Re: [Xen-devel] [PATCH v3 11/24] xen/arm: Let the toolstack configure the number of SPIs

[Julien Grall <julien.grall@xxxxxxxxxx>]

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 <julien.grall@xxxxxxxxxx>
>> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>>
>> ---
>>     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?

[..]

>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index d302fc9..101b4e9 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -121,6 +121,8 @@ struct arch_domain
>>      unsigned int evtchn_irq;
>>  }  __cacheline_aligned;
>>  
>> +#define domain_is_configured(d) ((d)->arch.is_configured)
> 
> This is unused. Moreover it cannot find "is_configured" anywhere.

Left-over of a previous version. Sorry.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel