From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Kilari Subject: Re: [RFC PATCH v3 07/18] xen/arm: ITS: implement hw_irq_controller for LPIs Date: Fri, 26 Jun 2015 19:55:03 +0530 Message-ID: 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" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55896DEC.2030101@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall 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 Hi Julien, >> + 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? > > Furthermore, when you set the LPI configuration (see lpi_set_config) you > are using a round robin to get the collection. This won't work anymore > if Xen decides to change the affinity... So you may want to drop > affinity support for now. > >> +} > > Missing newline. > > [..] > >> 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. Regards Vijay