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

If you think this is not justify by the spec, then maybe we should just
drop this patch. It's nice to have it but not strictly mandatory.

Regards,

-- 
Julien Grall