From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [PATCH v6 4/6] xen/arm: vgic: Optimize the way to
 store the target vCPU in the rank
Date: Mon, 30 Nov 2015 13:32:32 +0000
Message-ID: <565C4FF0.5050609@citrix.com>
References: <1447864963-21577-1-git-send-email-julien.grall@citrix.com>
	<1447864963-21577-5-git-send-email-julien.grall@citrix.com>
	<1448451462.17688.73.camel@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=769104de0=julien.grall@citrix.com>)
	id 1a3Oah-0002Ox-22
	for xen-devel@lists.xenproject.org; Mon, 30 Nov 2015 13:34:03 +0000
In-Reply-To: <1448451462.17688.73.camel@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Campbell <ian.campbell@citrix.com>, xen-devel@lists.xenproject.org
Cc: stefano.stabellini@eu.citrix.com
List-Id: xen-devel@lists.xenproject.org

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@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> 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."

Regards,

-- 
Julien Grall