Re: [PATCH] xen/arm64: Avoid sending SGI when kicking secondary cpus with spin_table

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-04-08 at 13:23 +0100, Julien Grall wrote:
> Hi Chen,
> 
> Subject: I think you can drop the "_" in spin_table.
> 
> On 07/04/15 08:33, Chen Baozi wrote:
> > From: Chen Baozi <baozich@gmail.com>
> > 
> > On arm64, either firmware or xen's smp_up_cpu gate uses WFE on secondary
> > cpus to stand-by when booting. Thus, using SEV is enough for the boot
> > cpu to kick other secondaries. Further more, the current implementation
> > of cpu_up_send_sgi would pass a NULL cpumask pointer to send_SGI, which
> > then lead a data fault on GICv3 send_SGI implementation.
> 
> I'm not familiar with spin table on ARM64, so I will let Ian answer
> about it.

For arm32 it's sadly all a bit adhoc and not terribly well documented.
(If I'm wrong I'd love a pointer to the doc). But for arm64 it does seem
to be documented (linux/Documentation/arm64/booting.txt)

So it seems possible that the SGI dates to older arm based systems.

> Aside that, the GICv3 implementation looks buggy to me.
> The GIC code provides two helpers which lead to pass NULL to the
> callback send_SGI:
> 	- send_SGI_self: AFAICT nobody is using it

I think we ended up with this by analogy to x86's send_IPI_self,
probably expecting we would need it there too.

> 	- send_SGI_allbutself: Only used by the smp boot code
> 
> 
> I think the former can be dropped or modify to send_SGI_one.

Yes, either would be ok.

> For the later, I can't find why we need to send an SGI on ARM too. Ian,
> Stefano, any idea?

IIRC there were some platform firmware (u-boot? boot-wrapper? vexpress
first stage firmware?) which needed it (because they used wfi not wfe?).

Or perhaps we cargo-culted from somewhere, or maybe it's just always
been wrong.

ISTR that there is some subtle difference with the Event register
between armv7 and armv8, which might explain this. I don't quite recall
what though.

So, I dunno, it seems like there is a good chance we could remove this,
but that might break some random platform which we have forgotten about.

The GIC NULL issue thing should probably fixed either way, but we could
also try dropping the send SGI from both arm32 and 64 and see what
happens...

Ian.