Re: [PATCH v1 6/8] xen/arm: vgic: Optimize the way to store the target vCPU in the rank

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

On 29/09/15 15:23, Ian Campbell wrote:
> On Tue, 2015-09-29 at 14:36 +0100, Julien Grall wrote:
>> On 29/09/15 14:07, Ian Campbell wrote:
>>> On Fri, 2015-09-25 at 15:51 +0100, Julien Grall wrote:
>>>> Xen is currently directly storing the value of register
>>>> GICD_ITARGETSR
>>>> (for GICv2) and GICD_IROUTER (for GICv3) in the rank. This makes the
>>>> emulation of the registers access very simple but makes the code to
>>>> get
>>>> the target vCPU for a given IRQ more complex.
>>>>
>>>> While the target vCPU of an IRQ is retrieved everytime an IRQ is
>>>> injected to the guest, the access to the register occurs less often.
>>>>
>>>> So the data structure should be optimized for the most common case
>>>> rather than the inverse.
>>>>
>>>> This patch introduce the usage of an array to store the target vCPU
>>>> for
>>>> every interrupt in the rank. This will make the code to get the
>>>> target
>>>> very quick. The emulation code will now have to generate the
>>>> GICD_ITARGETSR
>>>> and GICD_IROUTER register for read access and split it to store in a
>>>> convenient way.
>>>>
>>>> Note that with these changes, any read to those registers will list
>>>> only
>>>> the target vCPU used by Xen. This is fine 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.
>>>
>>> I think this is probably OK, but skirting round what the spec actually
>>> says
>>> a fair bit.
>>
>> Well, nothing in the spec clearly explain the behavior of a read access
>> on the register. An implementation could decide to make some bits RO or
>> even not store everything.
>>
>> FWIW, KVM is using the same trick.
> 
> At least we'll both get screwed by a picky OS then ;-)

I think our implementation could fold into 4.3.12 IHI 0048B:

"It is IMPLEMENTATION DEFINED which, if any, SPIs are statically
configured in hardware. The CPU targets field for such an SPI is
read-only, and returns a value that indicates the CPU targets for the
interrupt."

We are implementing a weird read-only but at least an OS should not
trust the written value.

Regards,

-- 
Julien Grall