Re: [PATCH 2/2] xen/arm: gic-v3: Allow Xen to run on hardware reporting GICv4

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 15/09/15 10:49, Ian Campbell wrote:
> On Mon, 2015-09-14 at 16:32 +0100, Julien Grall wrote:
>> It seems that there is some hardware which report start to report GICv4
> 
> s/report start to reports/reports/ ?
> 
> Also, this is an odd way to express it, what you mean is that some hardware
> is now shipping with GICv4. Unless you are trying to imply that they are
> claiming to be GICv4 without actually being so?
> 
> (If we agree on some wording I can modify this text on commit, subject to
> the discussion below).
> 
>> in the GIC*_PIDR2 register.
>>
>> As GICv4 is a superset of GICv3, it should just work on Xen.
>>
>> Reported-by: Andre Przywara <andre.przywara@arm.com>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> ---
>>  xen/arch/arm/gic-v3.c             | 4 ++--
>>  xen/include/asm-arm/gic_v3_defs.h | 1 +
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 4d623bf..1e3c19b 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -640,7 +640,7 @@ static int __init gicv3_populate_rdist(void)
>>          void __iomem *ptr = gicv3.rdist_regions[i].map_base;
>>  
>>          reg = readl_relaxed(ptr + GICR_PIDR2) & GIC_PIDR2_ARCH_MASK;
>> -        if ( reg != GIC_PIDR2_ARCH_GICv3 )
>> +        if ( reg != GIC_PIDR2_ARCH_GICv3 && reg != GIC_PIDR2_ARCH_GICv4 )
> 
> Once we have GICv5, 6, etc this is going to get unwieldy, shall we switch
> to a switch now?

The current GICv3 documentation is actually subtitled:
"GIC architecture version 3.0 and version 4.0", so I'd view them both
together. We can care about GICv >4 once this appears, but for now I'd
just stick with that simple if extension.
I just stumbled upon this because the Linux driver compares against v3
and v4 with a very similar statement (and-ed compares in if).

> 
>>          {
>>              dprintk(XENLOG_ERR,
>>                      "GICv3: No redistributor present @%"PRIpaddr"\n",
> 
> I wonder if GICv3 ought to become GICv%d, on the other hand this is really
> the GICv3 driver driving a v4 in v3 "mode", so maybe v3 is the best
> logging.

I'd keep v3 as well, as this part is really a GICv3 property.
Maybe we should quote the spec in the commit message:
"Because GICv4 is an extension of GICv3, all references to GICv3 in this
manual apply equally to GICv4."

Cheers,
Andre.