diff mbox series

libxl: Disable relocating memory for qemu-xen in stubdomain too

Message ID 20231227023544.1253277-1-marmarek@invisiblethingslab.com (mailing list archive)
State New, archived
Headers show
Series libxl: Disable relocating memory for qemu-xen in stubdomain too | expand

Commit Message

Marek Marczykowski-Górecki Dec. 27, 2023, 2:35 a.m. UTC
According to comments (and experiments) qemu-xen cannot handle memory
reolcation done by hvmloader. The code was already disabled when running
qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
be consistent in this regard.

Reported-by: Neowutran <xen@neowutran.ovh>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 tools/libs/light/libxl_dm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Neowutran Dec. 27, 2023, 4:54 p.m. UTC | #1
On 2023-12-27 03:12, Marek Marczykowski-Górecki wrote:
> According to comments (and experiments) qemu-xen cannot handle memory
> reolcation done by hvmloader. The code was already disabled when running
> qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
> adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
> be consistent in this regard.
> 
> Reported-by: Neowutran <xen@neowutran.ovh>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  tools/libs/light/libxl_dm.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 14b593110f7c..ed620a9d8e14 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -2432,6 +2432,16 @@ 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. */
> +    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));
>      ret = xc_domain_set_target(ctx->xch, dm_domid, guest_domid);
>      if (ret<0) {
>          LOGED(ERROR, guest_domid, "setting target domain %d -> %d",
> -- 
> 2.41.0

Seems to work as expected

Thanks, 
Neowutran
Jason Andryuk Dec. 28, 2023, 12:18 a.m. UTC | #2
On Tue, Dec 26, 2023 at 11:49 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> According to comments (and experiments) qemu-xen cannot handle memory
> reolcation done by hvmloader. The code was already disabled when running
> qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
> adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
> be consistent in this regard.
>
> Reported-by: Neowutran <xen@neowutran.ovh>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Thanks,
Jason
Jan Beulich Jan. 4, 2024, 11:12 a.m. UTC | #3
On 27.12.2023 17:54, Neowutran wrote:
> On 2023-12-27 03:12, Marek Marczykowski-Górecki wrote:
>> According to comments (and experiments) qemu-xen cannot handle memory
>> reolcation done by hvmloader. The code was already disabled when running
>> qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
>> adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
>> be consistent in this regard.
>>
>> Reported-by: Neowutran <xen@neowutran.ovh>
>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>> ---
>>  tools/libs/light/libxl_dm.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
>> index 14b593110f7c..ed620a9d8e14 100644
>> --- a/tools/libs/light/libxl_dm.c
>> +++ b/tools/libs/light/libxl_dm.c
>> @@ -2432,6 +2432,16 @@ 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. */
>> +    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));
>>      ret = xc_domain_set_target(ctx->xch, dm_domid, guest_domid);
>>      if (ret<0) {
>>          LOGED(ERROR, guest_domid, "setting target domain %d -> %d",
>> -- 
>> 2.41.0
> 
> Seems to work as expected

May we translate this to Tested-by: ?

Jan
Andrew Cooper Jan. 4, 2024, 11:18 a.m. UTC | #4
On 27/12/2023 2:35 am, Marek Marczykowski-Górecki wrote:
> According to comments (and experiments) qemu-xen cannot handle memory
> reolcation done by hvmloader. The code was already disabled when running
> qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
> adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
> be consistent in this regard.
>
> Reported-by: Neowutran <xen@neowutran.ovh>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

It turns out that it's unconditionally clobbered in XenServer too.

https://github.com/xapi-project/xen-api/blob/53d2e8cdff76ca6805c7018948a9c414b9ac7c72/ocaml/xenopsd/scripts/qemu-wrapper#L146

Not that I'm surprised - this falls squarely into guest-physmap mess
which is broken in several well-documented ways already.

~Andrew
Anthony PERARD Feb. 21, 2024, 5:10 p.m. UTC | #5
On Wed, Dec 27, 2023 at 07:18:12PM -0500, Jason Andryuk wrote:
> On Tue, Dec 26, 2023 at 11:49 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > According to comments (and experiments) qemu-xen cannot handle memory
> > reolcation done by hvmloader. The code was already disabled when running
> > qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
> > adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
> > be consistent in this regard.
> >
> > Reported-by: Neowutran <xen@neowutran.ovh>
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> 
> Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,
diff mbox series

Patch

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 14b593110f7c..ed620a9d8e14 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2432,6 +2432,16 @@  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. */
+    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));
     ret = xc_domain_set_target(ctx->xch, dm_domid, guest_domid);
     if (ret<0) {
         LOGED(ERROR, guest_domid, "setting target domain %d -> %d",