From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 11/24] xen/arm: Let the toolstack configure the number of SPIs Date: Thu, 29 Jan 2015 12:14:50 +0000 Message-ID: <54CA243A.8090700@linaro.org> References: <1421159133-31526-1-git-send-email-julien.grall@linaro.org> <1421159133-31526-12-git-send-email-julien.grall@linaro.org> <54C93168.30105@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YGo0H-0001Fu-08 for xen-devel@lists.xenproject.org; Thu, 29 Jan 2015 12:15:21 +0000 Received: by mail-wi0-f171.google.com with SMTP id l15so25277568wiw.4 for ; Thu, 29 Jan 2015 04:15:18 -0800 (PST) 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: Stefano Stabellini Cc: Wei Liu , ian.campbell@citrix.com, tim@xen.org, Ian Jackson , stefano.stabellini@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 29/01/15 12:01, Stefano Stabellini wrote: > On Wed, 28 Jan 2015, Julien Grall wrote: >> Hi Stefano, >> >> On 28/01/15 18:26, Stefano Stabellini wrote: >>> On Tue, 13 Jan 2015, Julien Grall wrote: >>>> Each domain may have a different number of IRQs depending on the devices >>>> assigned to it. >>>> >>>> Rather re-using the number of IRQs used by the hardwared GIC, let the >>>> toolstack specify the number of SPIs when the domain is created. This >>>> will avoid to waste memory. >>>> >>>> To calculate the number of SPIs, we assume that any IRQ given via the option >>>> "irqs=" in xl is mapped 1:1 to the guest. >>>> >>>> Signed-off-by: Julien Grall >>>> Cc: Ian Jackson >>>> Cc: Jan Beulich >>>> Cc: Wei Liu >>>> >>>> --- >>>> Changes in v3: >>>> - Fix typoes >>>> - A separate has been created to extend the DOMCTL create domain >>>> >>>> Changes in v2: >>>> - Patch added >>>> --- >>>> tools/libxc/xc_domain.c | 1 + >>>> tools/libxl/libxl_arm.c | 19 +++++++++++++++++++ >>>> xen/arch/arm/domain.c | 7 ++++++- >>>> xen/arch/arm/setup.c | 1 + >>>> xen/arch/arm/vgic.c | 10 +++++----- >>>> xen/include/asm-arm/domain.h | 2 ++ >>>> xen/include/asm-arm/setup.h | 1 + >>>> xen/include/asm-arm/vgic.h | 2 +- >>>> xen/include/public/arch-arm.h | 2 ++ >>>> 9 files changed, 38 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c >>>> index eebc121..eb066cf 100644 >>>> --- a/tools/libxc/xc_domain.c >>>> +++ b/tools/libxc/xc_domain.c >>>> @@ -67,6 +67,7 @@ int xc_domain_create(xc_interface *xch, >>>> /* No arch-specific configuration for now */ >>>> #elif defined (__arm__) || defined(__aarch64__) >>>> config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT; >>>> + config.nr_spis = 0; >>>> #else >>>> errno = ENOSYS; >>>> return -1; >>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c >>>> index cddce6e..53177eb 100644 >>>> --- a/tools/libxl/libxl_arm.c >>>> +++ b/tools/libxl/libxl_arm.c >>>> @@ -39,6 +39,25 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, >>>> libxl_domain_config *d_config, >>>> xc_domain_configuration_t *xc_config) >>>> { >>>> + uint32_t nr_spis = 0; >>>> + unsigned int i; >>>> + >>>> + for (i = 0; i < d_config->b_info.num_irqs; i++) { >>>> + int irq = d_config->b_info.irqs[i]; >>> >>> unsigned int >> >> I will use uint32_t. >> >>>> + int spi = irq - 32; >> >> Same here. >> >>> >>>> + if (irq < 32) >>>> + continue; >>>> + >>>> + if (nr_spis <= spi) >>>> + nr_spis = spi + 1; >>> >>> overflow check? >> >> If I use unsigned int, the overflow will go back to 0. While it won't >> affect the code, the domain creation will fail later (unable to assign >> the SPI). >> >> So is it useful to add a check here? > > The maximum number of allowed spis has to be lower than UINT_MAX, right? UINT_MAX + 1 would give 0. So it's still < UINT_MAX. When the toolstack will try to assign the IRQ. The hypervisor will see the SPI is too high for the vGIC. So it will reject. Anyway, technically it's even 1024. I forgot to add this check in the vgic code. I will do it in the next version. Regards, -- Julien Grall