Re: [PATCH v3 2/4] tools: remove support for running a guest with qemu-traditional

[Anthony PERARD <anthony@xxxxxxxxxxxxxx>]

On Tue, Apr 29, 2025 at 01:06:32PM +0200, Juergen Gross wrote:
> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
> index 34f6753f61..227b5ceafb 100644
> --- a/tools/libacpi/mk_dsdt.c
> +++ b/tools/libacpi/mk_dsdt.c
> @@ -105,7 +81,7 @@ int main(int argc, char **argv)
>  {
>      unsigned int cpu, max_cpus;
>  #if defined(CONFIG_X86)
> -    dm_version dm_version = QEMU_XEN_TRADITIONAL;
> +    dm_version dm_version = QEMU_INVALID;
>      unsigned int slot, dev, intx, link;
>  
>      max_cpus = HVM_MAX_VCPUS;
> @@ -160,6 +134,11 @@ int main(int argc, char **argv)
>          }
>      }
>  
> +    if (dm_version == QEMU_INVALID) {

`dm_version` is only available if CONFIG_X86 is defined, so that doesn't
build on Arm.

> +        fprintf(stderr, "--dm_version is a mandatory parameter.\n");
> +        return -1;
> +    }
> +
>      /**** DSDT DefinitionBlock start ****/
>      /* (we append to existing DSDT definition block) */
>      indent_level++;
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 4627564c0d..645119b65a 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -2463,16 +2189,15 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
> libxl__stub_dm_spawn_state *sdss)
>                          "%s",
>                          
> libxl_bios_type_to_string(guest_config->b_info.u.hvm.bios));
>      }
> -    /* Disable relocating memory to make the MMIO hole larger
> -     * unless we're running qemu-traditional and vNUMA is not
> -     * configured. */
> +
> +    /*
> +     * Disable relocating memory, having a lager MMIO hole isn't

I think you wanted to write "larger" and not "lager".

> +     * implemented with qemu-xen.
> +     */
>      libxl__xs_printf(gc, XBT_NULL,
>                       libxl__sprintf(gc, "%s/hvmloader/allow-memory-relocate",
>                                      libxl__xs_get_dompath(gc, guest_domid)),
> -                     "%d",
> -                     guest_config->b_info.device_model_version
> -                        == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
> -                     !libxl__vnuma_configured(&guest_config->b_info));
> +                     "0");
>      ret = xc_domain_set_target(ctx->xch, dm_domid, guest_domid);
>      if (ret<0) {
>          LOGED(ERROR, guest_domid, "setting target domain %d -> %d",
> @@ -3196,26 +2917,19 @@ static void device_model_launch(libxl__egc *egc,
>          libxl__xs_printf(gc, XBT_NULL,
>                           GCSPRINTF("%s/hvmloader/bios", path),
>                           "%s", 
> libxl_bios_type_to_string(b_info->u.hvm.bios));
> -        /* Disable relocating memory to make the MMIO hole larger
> -         * unless we're running qemu-traditional and vNUMA is not
> -         * configured. */
> +        /*
> +         * Disable relocating memory, having a lager MMIO hole isn't

Here too, I think you wanted to write "larger" and not "lager".

> +         * implemented with qemu-xen.
> +         */
>          libxl__xs_printf(gc, XBT_NULL,
>                           GCSPRINTF("%s/hvmloader/allow-memory-relocate", 
> path),
> -                         "%d",
> -                         
> b_info->device_model_version==LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL 
> &&
> -                         !libxl__vnuma_configured(b_info));
> +                         "0");
>          free(path);
>      }

And with those fixed: Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
I guess I can fixed those on commit if that's fine by you?

Thanks,

-- 
Anthony PERARD