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

[Henry Wang <xin.wang2@xxxxxxx>]

Hi Julien,

On 5/7/2024 10:35 PM, Julien Grall wrote:
> Hi,
>
> On 06/05/2024 06:17, Henry Wang wrote:
> > 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()?
> Xen domains are supposed to be platform agnostics and therefore the 
> numbers of SPIs should not be based on the HW.
> Furthermore, with your proposal we would end up to allocate data 
> structure for N SPIs when a domain may never needs any SPIs (such as 
> if passthrough is not in-use). This is more likely for domain created 
> by the toolstack than from Xen directly.
Agreed on both comments.

> Instead, we should introduce a new XL configuration to let the user 
> decide the number of SPIs. I would suggest to name "nr_spis" to match 
> the DT bindings.
Sure, I will introduce a new xl config for this to replace this patch. 
Thank you for the suggestion.
Kind regards,
Henry

> Cheers,