[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v6 4/6] xen/arm: vgic: Optimize the way to store the target vCPU in the rank
On Mon, 2015-11-30 at 13:32 +0000, Julien Grall wrote: > Hi Ian, > > On 25/11/15 11:37, Ian Campbell wrote: > > On Wed, 2015-11-18 at 16:42 +0000, 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 shrunk down from 320 bytes to 92 bytes. This is saving about 228 > > > bytes of memory allocated separately per vCPU. > > > > > > Note that with these changes, any read to those register will list > > > only > > > the target vCPU used by Xen. As the spec is not clear whether this is > > > a > > > valid choice or not, OSes which have a different interpretation of > > > the > > > spec (i.e OSes which perform read-modify-write operations on these > > > registers) may not boot anymore on Xen. Although, I think this is > > > fair > > > trade between memory usage in Xen (1KB less on a domain using 4 vCPUs > > > with no SPIs) and a strict interpretation of the spec (though all the > > > cases are not clearly defined). > > > > > > Furthermore, the implementation of the callback get_target_vcpu is > > > now > > > exactly the same. Consolidate the implementation in the common vGIC > > > code > > > and drop the callback. > > > > > > Finally take the opportunity to fix coding style and replace "irq" by > > > "virq" to make clear that we are dealing with virtual IRQ in section > > > we > > > are modifying. > > > > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx> > > > > Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > > > > I have one clarifying question, which may or may not be worth a > > followup: > > > > > + * Fetch an ITARGETSR register based on the offset from ITARGETSR0. > > > > Is the offset here in terms of bytes or in terms of entire ITARGETSR<n> > > registers (i.e. 4 bytes)? > > The offset is in term of bytes. > > > Might be worth clarifying the comment? > > I'm not sure, I think it's implicit with the following sentence in the > comment: > > "Note the offset will be aligned to the appropriate boundary." It's very implicit, since without knowing the answer it's not clear what an appropriate boundary is. How about: "Note the byte offset will be aligned to an ITARGETSR<n>" boundary" ? _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |