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

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

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@lists.xen.org
https://lists.xen.org/xen-devel