Re: [PATCH 05/15] tools/libs/light: Increase nr_spi to 160

[Henry Wang <xin.wang2@xxxxxxx>]

Hi Anthony,

(+Arm maintainers)

On 5/1/2024 9:58 PM, Anthony PERARD wrote:
> On Wed, Apr 24, 2024 at 11:34:39AM +0800, Henry Wang wrote:
> > Increase number of spi to 160 i.e. gic_number_lines() for Xilinx ZynqMP - 32.
> > This was done to allocate and assign IRQs to a running domain.
> >
> > Signed-off-by: Vikram Garhwal <fnu.vikram@xxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> > ---
> >   tools/libs/light/libxl_arm.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> > index dd5c9f4917..50dbd0f2a9 100644
> > --- a/tools/libs/light/libxl_arm.c
> > +++ b/tools/libs/light/libxl_arm.c
> > @@ -181,7 +181,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> >   
> >       LOG(DEBUG, "Configure the domain");
> >   
> > -    config->arch.nr_spis = nr_spis;
> > +    /* gic_number_lines() is 192 for Xilinx ZynqMP. min nr_spis = 192 - 32. */
> > +    config->arch.nr_spis = MAX(nr_spis, 160);
> Is there a way that that Xen or libxl could find out what the minimum
> number of SPI needs to be?
I am afraid currently there is none.

> Are we going to have to increase that minimum
> number every time a new platform comes along?
>
> It doesn't appear that libxl is using that `nr_spis` value and it is
> probably just given to Xen. So my guess is that Xen could simply take
> care of the minimum value, gic_number_lines() seems to be a Xen
> function.
Xen will take care of the value of nr_spis for dom0 in create_dom0()
dom0_cfg.arch.nr_spis = min(gic_number_lines(), (unsigned int) 992) - 32;
and also for dom0less domUs in create_domUs().

However, it looks like Xen will not take care of the mininum value for 
libxl guests, the value from config->arch.nr_spis in guest config file 
will be directly passed to the domain_vgic_init() function from 
arch_domain_create().
I agree with you that we shouldn't just bump the number everytime when 
we have a new platform. Therefore, would it be a good idea to move the 
logic in this patch to arch_sanitise_domain_config()?
Kind regards,
Henry

> Thanks,