Re: [PATCH v3 1/3] xen/arm: gic: Check the size of the CPU and vCPU interface retrieved from DT

[Julien Grall <julien.grall@citrix.com>]

On 06/10/15 15:55, Ian Campbell wrote:
> On Tue, 2015-10-06 at 15:39 +0100, Julien Grall wrote:
>>>> +        csize = SZ_8K;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Check if the CPU interface and virtual CPU interface have the
>>>> +     * same size.
>>>> +     */
>>>> +    if ( csize != vsize )
>>>> +        printk(XENLOG_WARNING "GICv2: WARNING: "
>>>> +               "Sizes of GICC (%#"PRIpaddr") and GICV (%#"PRIpaddr")
>>>> don't match\n",
>>>> +               csize, vsize);
>>>
>>> Should we also force them to be equal? Either
>>> 	csize = vsize = min(csize,vsize)
>>
>> If we restrict csize we will get to some other troubles later because
>> vsize may be only 4KB.
> 
> Does Xen work with that? I suppose so.

Well csize > 8KB is a mandatory because we are using GICC_DIR.

The GICC region mapped in DOM0 is bound to csize because we create the
"reg" property based on the host DT.

I'm thinking to turn this warning into a panic as IHMO csize != vsize
should never happen or else we would do something wrong later in Xen.

>>>
>>> WRT to the XXX I think I'd be happier if this was < SZ_8K for each.
>>> Otherwise some future GIC which is compatible but has extensions to the
>>> register space would needlessly require changes here. But I can live
>>> with
>>> this.
>>
>> The GICv2 CPU interface is always at least 8KB. Having an higher value
>> may mean that the GIC is aliased.
> 
> Or that this is a GICvN which has 8KB of GICv2 compatible registers and
> then some extensions.
> 
> In either that situation or the aliasing one it would be safe to expose the
> first 8KB as a gic-v2 to the guest.
> 
>> GICv2 on GICv3 is only used for guest. I prefer to restrict the usage to
>> known and safe value until we have someone using different size.
>>
>> This will avoid to expose unwanted data/value to a guest.
> 
> Right, I'm not saying we should expose the whole region, just the known to
> be gic-v2 compatible first 8KB.
> 
> NB I'm talking about domU here, things are more complicated with dom0 and
> in that case you are right that it would be a bad idea.

Thinking a bit more about this. csize is only required when GICv2 is
used for DOM0. On GICv3 we will always expose a vGICv3 to DOM0. So we
don't need to check csize.

Although, we do have to check that vsize is >= 8KB.

I will rework this patch series.

Regards,

-- 
Julien Grall