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

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

On Wed, 2015-10-07 at 20:13 +0100, Julien Grall wrote:
> On 07/10/15 17:29, Julien Grall wrote:
> > TBH, I don't see how I can justify that what we are doing is fine except
> > by saying "for simplicity".

I think it is also fine to say that we currently expect that in reality
OSes won't rely on being able to read-back from this register.

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

TBH I don't agree, your definition essentially turns any RW register which
does not specify precisely otherwise into a WO register.

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

That's OK. I'd prefer to drop the "picky" and to make the "(i.e. ....)" in
the middle to say something like "(i.e. OSes which perform read-modify
-write operations on these registers"), which is not a picky thing to want
to do even if we don't expect any OS to actually to it.

BTW, IHI 0069A says 8.9.17:

    When affinity routing is not enabled, holds the list of target PEs for
    the interrupt. That is, it holds the list of CPU interfaces to which the
    Distributor forwards the interrupt if it is asserted and has sufficient
    priority.

Which isn't actually inconsistent with the register reading back the set of
CPUs to which the interrupt is actually going to end up being routed (i.e.
the behaviour of this patch).

However I think this has gone on long enough and some text which says it
isn't clear is still a reasonable thing to write.

Ian.