Re: [PATCH v3 7/9] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

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

On 07/10/15 17:29, Julien Grall wrote:
> On 07/10/15 17:00, Ian Campbell wrote:
>> On Wed, 2015-10-07 at 16:48 +0100, Julien Grall wrote:
>>> On 07/10/15 16:38, Ian Campbell wrote:
>>>> On Wed, 2015-10-07 at 15:41 +0100, Julien Grall wrote:
>>>>> Note that with these changes, any read to those registers will list
>>>>> only
>>>>> the target vCPU used by Xen. We think this is likely to be OK because
>>>>> the
>>>>> GIC spec doesn't require to return exactly the value written and it
>>>>> can 
>>>>> be seen as if we decide to implement the register read-only.
>>>>
>>>> This still isn't true.
>>>
>>> I understood you previous answer on v2 as please replace "this is
>>> fine..." by "We think this is likely to be OK because..."....
>>
>> Sorry, I meant ... as "and go on to explain it as we've been discussing".
>>
>>>> The GIC spec requires one of two modes: Either the register is read
>>>> -only
>>>> _or_ it is writable and one can expect to read back what was written
>>>> (because the spec doesn't say otherwise and doing otherwise would
>>>> definitely be exceptional behaviour).
>>>
>>> How can you say that the spec requires the second mode? Nothing in the
>>> spec say that you can expect to read back what was written...
>>
>> Because that is so clearly and obviously the only sane default for a read
>> write register that nobody even bothers to say it. They specify when it is
>> not the case that you get back what you wrote by defining what you get on
>> read.
> 
> Well that's not obvious for me...
> 
>>> The spec only says what you the format of the register and what you may
>>> expect to read if the register is read-only. Other than that it's
>>> completely blur and you can implement whatever you want.
>>
>> Regardless, I am not acking in a patch which implies we think this
>> behaviour is justified by the spec as this one currently does.
> 
> TBH, I don't see how I can justify that what we are doing is fine except
> by saying "for simplicity".

I though a bit more about it. We are both interpreting the spec
differently and I think both are valid. So hardware people and OS
developer may also lean toward one of them.

Mine is more restrictive than yours, as you said on v1, it would hit
picky OS that follow your interpretation and read back the register to
check either the value is exactly the one written and/or the previous
value. Although it make the implementation simpler and save memory.

How about:

"Note that with these changes, any read to those registers will list
only the target vCPU used by Xen. As the spec is not clear whether this
is a valid choice or not, picky OSes (i.e which read back register to
check the value written) which have a different interpretation of the
spec may not boot anymore on Xen. Although, I think this is a fair trade
between memory usage in Xen (1KB on a domain using 4 vCPUs with no SPIs)
and a strict interpretation of the spec (though all the cases are not
clearly defined)".

Regards,

-- 
Julien Grall