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

[Peng Fan <peng.fan@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@xxxxxxx]
> Sent: 2019年2月1日 18:41
> To: Peng Fan <peng.fan@xxxxxxx>; sstabellini@xxxxxxxxxx; jgross@xxxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel