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

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

On Wed, 2015-10-07 at 16:07 +0100, Julien Grall wrote:
> 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.

That sounds fine.

> 
> > > > 
> > > > 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.

Right.

> I will rework this patch series.

Great.