On Tue, 12 Jun 2018 15:55:16 +0100,
Srinivas Kandagatla <[email protected]> wrote:
> 
> GICR_WAKER can be a secured register, check this before accessing it
> as its done in power management code.

NAK.

>From the GICv3 spec:

* When GICD_CTLR.DS==1, this register is always accessible.
* When GICD_CTLR.DS==0, this is a Secure register. This register is
  RAZ/WI to Non-secure accesses.

> Without this patch Qualcomm DB820c board crashes.

I suggest you find out how the GIC has been integrated on this
platform. If you take a fault on accessing this register, this very
much looks like an integration bug, and it should be quirked as such.

Thanks,

        M.
        
> 
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
>  drivers/irqchip/irq-gic-v3.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 5a67ec084588..38136d6e9ca5 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -656,6 +656,12 @@ static int gic_dist_supports_lpis(void)
>       return !!(readl_relaxed(gic_data.dist_base + GICD_TYPER) & 
> GICD_TYPER_LPIS) && !gicv3_nolpi;
>  }
>  
> +/* Check whether it's single security state view */
> +static bool gic_dist_security_disabled(void)
> +{
> +     return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
> +}
> +
>  static void gic_cpu_init(void)
>  {
>       void __iomem *rbase;
> @@ -664,7 +670,8 @@ static void gic_cpu_init(void)
>       if (gic_populate_rdist())
>               return;
>  
> -     gic_enable_redist(true);
> +     if (gic_dist_security_disabled())
> +             gic_enable_redist(true);
>  
>       rbase = gic_data_rdist_sgi_base();
>  
> @@ -819,11 +826,6 @@ static int gic_set_affinity(struct irq_data *d, const 
> struct cpumask *mask_val,
>  #endif
>  
>  #ifdef CONFIG_CPU_PM
> -/* Check whether it's single security state view */
> -static bool gic_dist_security_disabled(void)
> -{
> -     return readl_relaxed(gic_data.dist_base + GICD_CTLR) & GICD_CTLR_DS;
> -}
>  
>  static int gic_cpu_pm_notifier(struct notifier_block *self,
>                              unsigned long cmd, void *v)
> -- 
> 2.16.2
> 

-- 
Jazz is not dead, it just smell funny.

Reply via email to