Re: [PATCH v1 04/10] hw/arm: xenpvh: Add support for SMP guests

[Stefano Stabellini]

On Tue, 13 Aug 2024, Edgar E. Iglesias wrote:
> On Mon, Aug 12, 2024 at 06:47:17PM -0700, Stefano Stabellini wrote:
> > On Mon, 12 Aug 2024, Edgar E. Iglesias wrote:
> > > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> > > 
> > > Add SMP support for Xen PVH ARM guests. Create max_cpus ioreq
> > > servers to handle hotplug.
> > > 
> > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > > ---
> > >  hw/arm/xen_arm.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > > index 5f75cc3779..ef8315969c 100644
> > > --- a/hw/arm/xen_arm.c
> > > +++ b/hw/arm/xen_arm.c
> > > @@ -173,7 +173,7 @@ static void xen_arm_init(MachineState *machine)
> > >  
> > >      xen_init_ram(machine);
> > >  
> > > -    xen_register_ioreq(xam->state, machine->smp.cpus, 
> > > &xen_memory_listener);
> > > +    xen_register_ioreq(xam->state, machine->smp.max_cpus, 
> > > &xen_memory_listener);
> > >  
> > >      xen_create_virtio_mmio_devices(xam);
> > >  
> > > @@ -218,7 +218,8 @@ static void xen_arm_machine_class_init(ObjectClass 
> > > *oc, void *data)
> > >      MachineClass *mc = MACHINE_CLASS(oc);
> > >      mc->desc = "Xen PVH ARM machine";
> > >      mc->init = xen_arm_init;
> > > -    mc->max_cpus = 1;
> > > +    /* MAX number of vcpus supported by Xen.  */
> > > +    mc->max_cpus = GUEST_MAX_VCPUS;
> > 
> > Will this cause allocations of data structures with 128 elements?
> > Looking at hw/xen/xen-hvm-common.c:xen_do_ioreq_register it seems
> > possible? Or hw/xen/xen-hvm-common.c:xen_do_ioreq_register is called
> 
> Yes, in theory there's probably overhead with this but as you correctly
> noted below, a PVH aware xl will set the max_cpus option to a lower value.
> 
> With a non-pvh aware xl, I was a little worried about the overhead
> but I couldn't see any visible slow-down on ARM neither in boot or in network
> performance (I didn't run very sophisticated benchmarks).
 
What do you mean by "non-pvh aware xl"? All useful versions of xl
support pvh?

> > later on with the precise vCPU value which should be provided to QEMU
> > via the -smp command line option
> > (tools/libs/light/libxl_dm.c:libxl__build_device_model_args_new)?
> 
> Yes, a pvh aware xl will for example pass -smp 2,maxcpus=4 based on
> values from the xl.cfg. If the user doesn't set maxvcpus in xl.cfg, xl
> will set maxvcpus to the same value as vcpus.

OK good. In that case if this is just an initial value meant to be
overwritten, I think it is best to keep it as 1.