[PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Peng Fan]

On i.MX8, we implemented partition reboot which means Cortex-A reboot
will not impact M4 cores and System control Unit core. However GICv3
is not reset because we also need to support A72 Cluster reboot without
affecting A53 Cluster.

The gic-v3 controller is configured with EOImode to 1, so during xen
reboot, there is a function call "smp_call_function(halt_this_cpu, NULL, 0);"
, but halt_this_cpu never return, that means other CPUs have no chance to
deactivate the SGI interrupt, because the deactivate_irq operation is at
the end of do_sgi. During the next boot of Xen, CPU0 will issue
GIC_SGI_CALL_FUNCTION to other CPUs. As the Active state for SGI is left
untouched during the reboot, the GIC_SGI_CALL_FUNCTION will still be active
on the non-boot CPUs. This means the interrupt cannot be triggered again
until it get deactivated.

And according to IHI0069D_gic_architecture_specification, chapter
"8.11.3 GICR_ICACTIVER0, Interrupt Clear-Active Register 0", the RW
field of GICR_ICACTIVER0 resets to a value that is architecturally UNKNOWN.

So set a fixed value during gic-v3 initialization to make sure
interrupts are in deactivated state.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V2:
  refine commit log
  deactivate SGI/PPI.
  No need for SPI, because Some or all RW fields of this GICD_ICACTIVER
  have defined reset values

 xen/arch/arm/gic-v3.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6fbc106757..1c1d2604f3 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -833,6 +833,11 @@ static int gicv3_cpu_init(void)
         writel_relaxed(priority,
                 GICD_RDIST_SGI_BASE + GICR_IPRIORITYR0 + (i / 4) * 4);
 
+    /*
+     * The activate state is unknown at boot, so make sure all
+     * SGIs and PPIs are de-activated.
+     */
+    writel_relaxed(0xffffffff, GICD_RDIST_SGI_BASE + GICR_ICACTIVER0);
     /*
      * Disable all PPI interrupts, ensure all SGI interrupts are
      * enabled.
-- 
2.14.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Julien Grall]

Hi,

We spoke about SPIs in the previous version. Why aren't they 
de-activated here?

On 1/30/19 2:00 PM, Peng Fan wrote:
> On i.MX8, we implemented partition reboot which means Cortex-A reboot
> will not impact M4 cores and System control Unit core. However GICv3
> is not reset because we also need to support A72 Cluster reboot without
> affecting A53 Cluster.
> 
> The gic-v3 controller is configured with EOImode to 1, so during xen
> reboot, there is a function call "smp_call_function(halt_this_cpu, NULL, 0);"
> , but halt_this_cpu never return, that means other CPUs have no chance to
> deactivate the SGI interrupt, because the deactivate_irq operation is at
> the end of do_sgi. During the next boot of Xen, CPU0 will issue
> GIC_SGI_CALL_FUNCTION to other CPUs. As the Active state for SGI is left
> untouched during the reboot, the GIC_SGI_CALL_FUNCTION will still be active
> on the non-boot CPUs. This means the interrupt cannot be triggered again
> until it get deactivated.
> 
> And according to IHI0069D_gic_architecture_specification, chapter
> "8.11.3 GICR_ICACTIVER0, Interrupt Clear-Active Register 0", the RW
> field of GICR_ICACTIVER0 resets to a value that is architecturally UNKNOWN.
> 
> So set a fixed value during gic-v3 initialization to make sure
> interrupts are in deactivated state.

It is a bit unclear what you mean by "fixed value" here. The only thing 
you do is clearing active state. So a better wording is "So make sure 
all interrupts are deactivated at during initialization by clearing the 
state".

How about SPIs?

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Peng Fan]

Hi Julien

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@arm.com]
> Sent: 2019年2月1日 18:41
> To: Peng Fan <peng.fan@nxp.com>; sstabellini@kernel.org; jgross@suse.com
> Cc: xen-devel@lists.xenproject.org
> Subject: Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during
> initialization
> 
> Hi,
> 
> We spoke about SPIs in the previous version. Why aren't they de-activated
> here?

According to GIC-V3 " 8.9.5 GICD_ICACTIVER<n>, Interrupt Clear-Active Registers, n = 0 - 31"
"
Some or all RW fields of this register have defined reset values.
When this register has an architecturally-defined reset value, this field resets to 0.
"

So I think we no need to deactivated, because it has reset values 0.

> 
> On 1/30/19 2:00 PM, Peng Fan wrote:
> > On i.MX8, we implemented partition reboot which means Cortex-A reboot
> > will not impact M4 cores and System control Unit core. However GICv3
> > is not reset because we also need to support A72 Cluster reboot
> > without affecting A53 Cluster.
> >
> > The gic-v3 controller is configured with EOImode to 1, so during xen
> > reboot, there is a function call "smp_call_function(halt_this_cpu, NULL, 0);"
> > , but halt_this_cpu never return, that means other CPUs have no chance
> > to deactivate the SGI interrupt, because the deactivate_irq operation
> > is at the end of do_sgi. During the next boot of Xen, CPU0 will issue
> > GIC_SGI_CALL_FUNCTION to other CPUs. As the Active state for SGI is
> > left untouched during the reboot, the GIC_SGI_CALL_FUNCTION will still
> > be active on the non-boot CPUs. This means the interrupt cannot be
> > triggered again until it get deactivated.
> >
> > And according to IHI0069D_gic_architecture_specification, chapter
> > "8.11.3 GICR_ICACTIVER0, Interrupt Clear-Active Register 0", the RW
> > field of GICR_ICACTIVER0 resets to a value that is architecturally
> UNKNOWN.
> >
> > So set a fixed value during gic-v3 initialization to make sure
> > interrupts are in deactivated state.
> 
> It is a bit unclear what you mean by "fixed value" here. The only thing you do
> is clearing active state. So a better wording is "So make sure all interrupts are
> deactivated at during initialization by clearing the state".
> 
> How about SPIs?

Replied above. I could add the following to the patch if you think it needed, I am
not sure whether it is valid that SPI is in active state when Xen booting.

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 1c1d2604f3..5b9c5559a7 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -622,9 +622,12 @@ static void __init gicv3_dist_init(void)
         writel_relaxed(priority, GICD + GICD_IPRIORITYR + (i / 4) * 4);
     }

-    /* Disable all global interrupts */
+    /* Disable/deactivate all global interrupts */
     for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
+    {
         writel_relaxed(0xffffffff, GICD + GICD_ICENABLER + (i / 32) * 4);
+        writel_relaxed(0xffffffff, GICD + GICD_ICACTIVER + (i / 32) * 4);
+    }

     /*
      * Configure SPIs as non-secure Group-1. This will only matter

Thanks,
Peng.

> 
> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Julien Grall]

On 2/2/19 12:54 PM, Peng Fan wrote:
> Hi Julien

Hi Peng,

>> -----Original Message-----
>> From: Julien Grall [mailto:julien.grall@arm.com]
>> Sent: 2019年2月1日 18:41
>> To: Peng Fan <peng.fan@nxp.com>; sstabellini@kernel.org; jgross@suse.com
>> Cc: xen-devel@lists.xenproject.org
>> Subject: Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during
>> initialization
>>
>> Hi,
>>
>> We spoke about SPIs in the previous version. Why aren't they de-activated
>> here?
> 
> According to GIC-V3 " 8.9.5 GICD_ICACTIVER<n>, Interrupt Clear-Active Registers, n = 0 - 31"
> "
> Some or all RW fields of this register have defined reset values.
> When this register has an architecturally-defined reset value, this field resets to 0.
> "

I can't find this wording in the last spec (IHI0069E). Can you please 
give the version of the specific version of the spec when quoting it?

> 
> So I think we no need to deactivated, because it has reset values 0

The specification is about the reset value from the hardware, it does 
not tell you how the firmware (or the previous kernel when using kexec) 
is going to leave the interrupts. For instance, as I pointed out in a 
different thread, Xen may reboot with SPIs activated. This can happen if 
you receive the reboot/power off request while handling an SPI.

> 
>>
>> On 1/30/19 2:00 PM, Peng Fan wrote:
>>> On i.MX8, we implemented partition reboot which means Cortex-A reboot
>>> will not impact M4 cores and System control Unit core. However GICv3
>>> is not reset because we also need to support A72 Cluster reboot
>>> without affecting A53 Cluster.
>>>
>>> The gic-v3 controller is configured with EOImode to 1, so during xen
>>> reboot, there is a function call "smp_call_function(halt_this_cpu, NULL, 0);"
>>> , but halt_this_cpu never return, that means other CPUs have no chance
>>> to deactivate the SGI interrupt, because the deactivate_irq operation
>>> is at the end of do_sgi. During the next boot of Xen, CPU0 will issue
>>> GIC_SGI_CALL_FUNCTION to other CPUs. As the Active state for SGI is
>>> left untouched during the reboot, the GIC_SGI_CALL_FUNCTION will still
>>> be active on the non-boot CPUs. This means the interrupt cannot be
>>> triggered again until it get deactivated.
>>>
>>> And according to IHI0069D_gic_architecture_specification, chapter
>>> "8.11.3 GICR_ICACTIVER0, Interrupt Clear-Active Register 0", the RW
>>> field of GICR_ICACTIVER0 resets to a value that is architecturally
>> UNKNOWN.
>>>
>>> So set a fixed value during gic-v3 initialization to make sure
>>> interrupts are in deactivated state.
>>
>> It is a bit unclear what you mean by "fixed value" here. The only thing you do
>> is clearing active state. So a better wording is "So make sure all interrupts are
>> deactivated at during initialization by clearing the state".
>>
>> How about SPIs?
> 
> Replied above. I could add the following to the patch if you think it needed, I am
> not sure whether it is valid that SPI is in active state when Xen booting.

See above, I think this is valid and can happen today.

> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 1c1d2604f3..5b9c5559a7 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -622,9 +622,12 @@ static void __init gicv3_dist_init(void)
>           writel_relaxed(priority, GICD + GICD_IPRIORITYR + (i / 4) * 4);
>       }
> 
> -    /* Disable all global interrupts */
> +    /* Disable/deactivate all global interrupts */
>       for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
> +    {
>           writel_relaxed(0xffffffff, GICD + GICD_ICENABLER + (i / 32) * 4);
> +        writel_relaxed(0xffffffff, GICD + GICD_ICACTIVER + (i / 32) * 4);
> +    }
> 
>       /*
>        * Configure SPIs as non-secure Group-1. This will only matter

This chunk looks good. I will also write a patch for GICv2 doing the same.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Peng Fan]

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@arm.com]
> Sent: 2019年2月3日 0:04
> To: Peng Fan <peng.fan@nxp.com>; sstabellini@kernel.org; jgross@suse.com
> Cc: xen-devel@lists.xenproject.org
> Subject: Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during
> initialization
> 
> 
> 
> On 2/2/19 12:54 PM, Peng Fan wrote:
> > Hi Julien
> 
> Hi Peng,
> 
> >> -----Original Message-----
> >> From: Julien Grall [mailto:julien.grall@arm.com]
> >> Sent: 2019年2月1日 18:41
> >> To: Peng Fan <peng.fan@nxp.com>; sstabellini@kernel.org;
> >> jgross@suse.com
> >> Cc: xen-devel@lists.xenproject.org
> >> Subject: Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI
> >> during initialization
> >>
> >> Hi,
> >>
> >> We spoke about SPIs in the previous version. Why aren't they
> >> de-activated here?
> >
> > According to GIC-V3 " 8.9.5 GICD_ICACTIVER<n>, Interrupt Clear-Active
> Registers, n = 0 - 31"
> > "
> > Some or all RW fields of this register have defined reset values.
> > When this register has an architecturally-defined reset value, this field
> resets to 0.
> > "
> 
> I can't find this wording in the last spec (IHI0069E). Can you please give the
> version of the specific version of the spec when quoting it?

The spec I use is IHI0069D_gic_architecture_specification. Will add the version in
future patches when needed.

> 
> >
> > So I think we no need to deactivated, because it has reset values 0
> 
> The specification is about the reset value from the hardware, it does not tell
> you how the firmware (or the previous kernel when using kexec) is going to
> leave the interrupts. For instance, as I pointed out in a different thread, Xen
> may reboot with SPIs activated. This can happen if you receive the
> reboot/power off request while handling an SPI.

Thanks, just sent out v3 including the deactivation of SPI.

> 
> >
> >>
> >> On 1/30/19 2:00 PM, Peng Fan wrote:
> >>> On i.MX8, we implemented partition reboot which means Cortex-A
> >>> reboot will not impact M4 cores and System control Unit core.
> >>> However GICv3 is not reset because we also need to support A72
> >>> Cluster reboot without affecting A53 Cluster.
> >>>
> >>> The gic-v3 controller is configured with EOImode to 1, so during xen
> >>> reboot, there is a function call "smp_call_function(halt_this_cpu, NULL,
> 0);"
> >>> , but halt_this_cpu never return, that means other CPUs have no
> >>> chance to deactivate the SGI interrupt, because the deactivate_irq
> >>> operation is at the end of do_sgi. During the next boot of Xen, CPU0
> >>> will issue GIC_SGI_CALL_FUNCTION to other CPUs. As the Active state
> >>> for SGI is left untouched during the reboot, the
> >>> GIC_SGI_CALL_FUNCTION will still be active on the non-boot CPUs.
> >>> This means the interrupt cannot be triggered again until it get
> deactivated.
> >>>
> >>> And according to IHI0069D_gic_architecture_specification, chapter
> >>> "8.11.3 GICR_ICACTIVER0, Interrupt Clear-Active Register 0", the RW
> >>> field of GICR_ICACTIVER0 resets to a value that is architecturally
> >> UNKNOWN.
> >>>
> >>> So set a fixed value during gic-v3 initialization to make sure
> >>> interrupts are in deactivated state.
> >>
> >> It is a bit unclear what you mean by "fixed value" here. The only
> >> thing you do is clearing active state. So a better wording is "So
> >> make sure all interrupts are deactivated at during initialization by clearing
> the state".
> >>
> >> How about SPIs?
> >
> > Replied above. I could add the following to the patch if you think it
> > needed, I am not sure whether it is valid that SPI is in active state when Xen
> booting.
> 
> See above, I think this is valid and can happen today.
> 
> >
> > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index
> > 1c1d2604f3..5b9c5559a7 100644
> > --- a/xen/arch/arm/gic-v3.c
> > +++ b/xen/arch/arm/gic-v3.c
> > @@ -622,9 +622,12 @@ static void __init gicv3_dist_init(void)
> >           writel_relaxed(priority, GICD + GICD_IPRIORITYR + (i / 4) * 4);
> >       }
> >
> > -    /* Disable all global interrupts */
> > +    /* Disable/deactivate all global interrupts */
> >       for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i += 32 )
> > +    {
> >           writel_relaxed(0xffffffff, GICD + GICD_ICENABLER + (i / 32)
> > * 4);
> > +        writel_relaxed(0xffffffff, GICD + GICD_ICACTIVER + (i / 32) * 4);
> > +    }
> >
> >       /*
> >        * Configure SPIs as non-secure Group-1. This will only matter
> 
> This chunk looks good. I will also write a patch for GICv2 doing the same.

Thanks,
Peng.

> 
> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH for-4.12 v2] arm: gic-v3: deactivate SGI/PPI during initialization

[Stefano Stabellini]

On Sat, 2 Feb 2019, Julien Grall wrote:
> This chunk looks good. I will also write a patch for GICv2 doing the same.

No worries, Julien. I'll send the patch.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel