From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs Date: Fri, 26 Jun 2015 17:15:30 +0200 Message-ID: <558D6C92.8080908@citrix.com> References: <1434974517-12136-1-git-send-email-vijay.kilari@gmail.com> <1434974517-12136-8-git-send-email-vijay.kilari@gmail.com> <55896DEC.2030101@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Vijay Kilari Cc: Ian Campbell , Stefano Stabellini , Prasun Kapoor , Vijaya Kumar K , Tim Deegan , "xen-devel@lists.xen.org" , Stefano Stabellini , manish.jaggi@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org On 26/06/2015 16:25, Vijay Kilari wrote: > Hi Julien, Hi Vijay, >>> + its_send_inv(its_dev, col, virq); >>> +} >>> + >>> +void its_set_affinity(struct irq_desc *desc, int cpu) >>> +{ >>> + struct its_device *its_dev = get_irq_device(desc); >>> + struct its_collection *target_col; >>> + >>> + /* Physical collection id */ >>> + target_col = &its_dev->its->collections[cpu]; >>> + its_send_movi(its_dev, target_col, irq_to_virq(desc)); >> >> The field "virq" in the structure irq_guest refers to the guest virtual >> IRQ and not the event ID. As Ian suggested in the proposal [1], please >> use an union to make this code clears. > > Apart from adding union, do you recommend to add macros irq_to_vid() > and irq_to_virq() and use appropriately? If it makes to code clearer yes. Although it may not be necessary if we move the value into irq_desc (see below). [..] >>> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c >>> index 2dd43ee..9dbdf7d 100644 >>> --- a/xen/arch/arm/irq.c >>> +++ b/xen/arch/arm/irq.c >>> @@ -36,6 +36,7 @@ struct irq_guest >>> { >>> struct domain *d; >>> unsigned int virq; >>> + struct its_device *dev; >> >> I know that this was suggested in the proposal [1]. But the goal of >> irq_guest is to store anything specific to the guest. The event ID and >> the its_device assigned are known when the device is added to Xen and >> hence can be set in irq_desc (with a small memory impact, but we have >> plenty of memory on ARM64). > > Do you mean adding its_device to irq_desc instead of irq_guest? > If so, irq_desc is common for both x86 & ARM. There is an arch specific structure for irq_desc (see arch_irq_desc). Regards, -- Julien Grall