Re: [Xen-devel] [RFC PATCH 11/24] ARM: vGICv3: handle virtual LPI pending and property tables

[Julien Grall <julien.grall@xxxxxxx>]

On 31/01/2017 09:10, Andre Przywara wrote:
> Hi Julien,
Hi Andre,

> On 02/11/16 17:18, Julien Grall wrote:
> > On 28/09/16 19:24, Andre Przywara wrote:
> > > +    return (reg & ~field_mask) | field;
> > > +}
> > > +
> > > +/* We want to avoid outer shareable. */
> > > +static uint64_t vgic_sanitise_shareability(uint64_t field)
> > > +{
> > > +    switch (field) {
> > > +    case GIC_BASER_OuterShareable:
> > > +        return GIC_BASER_InnerShareable;
> > > +    default:
> > > +        return field;
> > > +    }
> > > +}
> > I am not sure to understand why we need to sanitise the value here. From
> > my understanding of the spec (see 8.11.18 in IHI 0069C) we should
> > support any shareability/cacheability, correct?
> No, actually an ITS is free to support only _one_ of those attributes,
> up to the point where it is read-only:
>
> "It is IMPLEMENTATION DEFINED whether this field has a fixed value or
> can be programmed by software. Implementing this field with a fixed
> value is deprecated."
>
> So we support more than one value, but refuse any really not useful
> ones. This goes in line with the KVM implementation.
Looking at your quote from the spec, this behavior is deprecated. Why do 
we want to implement a deprecated behavior?
> For the rest of the comments regarding the memory tables setup:
> I effectively rewrote this in the new series, so I think the majority of
> the comments don't apply anymore, hopefully the rewrite actually fixed
> the issues you mentioned. So I refrain from any comments now and look
> forward to a review of the new approach ;-)
I will give a look to the new implementation.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel