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

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

On Mon, 2015-10-05 at 13:51 +0100, Julien Grall wrote:
> Xen is currently directly storing the value of GICD_ITARGETSR register
> (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 vIRQ more complex.
> 
> While the target vCPU of an vIRQ is retrieved every time an vIRQ 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 introduces 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.
> 
> With the new way to store the target vCPU, the structure vgic_irq_rank
> is shrinked down from 320 bytes to 92 bytes. This is saving about 228

"shrunk"

> bytes of memory allocated separately per vCPU.
> 
> 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'd prefer if instead of "this is fine..." this said something like "We
think this is likely to be OK because...(stuff)...".

It is not the case that the spec actually says what is implemented here is
acceptable and I think that needs to be clear here along with why we
currently think it is something we can get away with, for future reference.

> +    /*
> +     * From the spec (see 8.9.13 in IHI 0069A), any write with an
> +     * invalid vCPU will lead to ignore the interrupt.

"will lead to the interrupt being ignored"