[PATCH] xen: fix condition for enabling the Xen accelerator

[PATCH] xen: fix condition for enabling the Xen accelerator

[Paolo Bonzini]

A misspelled condition in xen_native.h is hiding a bug in the enablement of
Xen for qemu-system-aarch64.  The bug becomes apparent when building for
Xen 4.18.

While the i386 emulator provides the xenpv machine type for multiple architectures,
and therefore can be compiled with Xen enabled even when the host is Arm, the
opposite is not true: qemu-system-aarch64 can only be compiled with Xen support
enabled when the host is Arm.

Expand the computation of accelerator_targets['CONFIG_XEN'] similar to what is
already there for KVM, and fix xen_native.h.

Cc: Stefano Stabellini <stefano.stabellini@amd.com>
Cc: Richard W.M. Jones <rjones@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Reported-by: Michael Young <m.a.young@durham.ac.uk>
Supersedes: <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.young@durham.ac.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/xen/xen_native.h |  2 +-
 meson.build                 | 17 ++++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
index 6f09c48823b..1a5ad693a4d 100644
--- a/include/hw/xen/xen_native.h
+++ b/include/hw/xen/xen_native.h
@@ -532,7 +532,7 @@ static inline int xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
 }
 #endif
 
-#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41700
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41700
 #define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x02000000)
 #define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x00100000)
 #define GUEST_VIRTIO_MMIO_SPI_FIRST   33
diff --git a/meson.build b/meson.build
index ec01f8b138a..67f4ede8aea 100644
--- a/meson.build
+++ b/meson.build
@@ -123,21 +123,24 @@ if get_option('kvm').allowed() and targetos == 'linux'
   kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"'
 endif
 config_host_data.set('CONFIG_KVM_TARGETS', kvm_targets_c)
-
 accelerator_targets = { 'CONFIG_KVM': kvm_targets }
 
+if cpu in ['x86', 'x86_64']
+  xen_targets = ['i386-softmmu', 'x86_64-softmmu']
+elif cpu in ['arm', 'aarch64']
+  # i386 emulator provides xenpv machine type for multiple architectures
+  xen_targets = ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu']
+else
+  xen_targets = []
+endif
+accelerator_targets += { 'CONFIG_XEN': xen_targets }
+
 if cpu in ['aarch64']
   accelerator_targets += {
     'CONFIG_HVF': ['aarch64-softmmu']
   }
 endif
 
-if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
-  # i386 emulator provides xenpv machine type for multiple architectures
-  accelerator_targets += {
-    'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
-  }
-endif
 if cpu in ['x86', 'x86_64']
   accelerator_targets += {
     'CONFIG_HVF': ['x86_64-softmmu'],
-- 
2.43.0

Re: [PATCH] xen: fix condition for enabling the Xen accelerator

[Richard W.M. Jones]

On Sat, Dec 09, 2023 at 03:32:22PM +0100, Paolo Bonzini wrote:
> A misspelled condition in xen_native.h is hiding a bug in the enablement of
> Xen for qemu-system-aarch64.  The bug becomes apparent when building for
> Xen 4.18.
>
> While the i386 emulator provides the xenpv machine type for multiple
> architectures, and therefore can be compiled with Xen enabled even
> when the host is Arm, the opposite is not true: qemu-system-aarch64
> can only be compiled with Xen support enabled when the host is Arm.
>
> Expand the computation of accelerator_targets['CONFIG_XEN'] similar
> to what is already there for KVM, and fix xen_native.h.

Here's a Fedora scratch build with Xen 4.18.0 which includes this patch:

https://koji.fedoraproject.org/koji/taskinfo?taskID=110105806

Rich.

> Cc: Stefano Stabellini <stefano.stabellini@amd.com>
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Reported-by: Michael Young <m.a.young@durham.ac.uk>
> Supersedes: <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.young@durham.ac.uk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/hw/xen/xen_native.h |  2 +-
>  meson.build                 | 17 ++++++++++-------
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 6f09c48823b..1a5ad693a4d 100644
> --- a/include/hw/xen/xen_native.h
> +++ b/include/hw/xen/xen_native.h
> @@ -532,7 +532,7 @@ static inline int xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
>  }
>  #endif
>  
> -#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41700
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41700
>  #define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x02000000)
>  #define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x00100000)
>  #define GUEST_VIRTIO_MMIO_SPI_FIRST   33
> diff --git a/meson.build b/meson.build
> index ec01f8b138a..67f4ede8aea 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -123,21 +123,24 @@ if get_option('kvm').allowed() and targetos == 'linux'
>    kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"'
>  endif
>  config_host_data.set('CONFIG_KVM_TARGETS', kvm_targets_c)
> -
>  accelerator_targets = { 'CONFIG_KVM': kvm_targets }
>  
> +if cpu in ['x86', 'x86_64']
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu']
> +elif cpu in ['arm', 'aarch64']
> +  # i386 emulator provides xenpv machine type for multiple architectures
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu']
> +else
> +  xen_targets = []
> +endif
> +accelerator_targets += { 'CONFIG_XEN': xen_targets }
> +
>  if cpu in ['aarch64']
>    accelerator_targets += {
>      'CONFIG_HVF': ['aarch64-softmmu']
>    }
>  endif
>  
> -if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
> -  # i386 emulator provides xenpv machine type for multiple architectures
> -  accelerator_targets += {
> -    'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
> -  }
> -endif
>  if cpu in ['x86', 'x86_64']
>    accelerator_targets += {
>      'CONFIG_HVF': ['x86_64-softmmu'],
> -- 
> 2.43.0

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit

Re: [PATCH] xen: fix condition for enabling the Xen accelerator

[Philippe Mathieu-Daudé]

On 9/12/23 15:32, Paolo Bonzini wrote:
> A misspelled condition in xen_native.h is hiding a bug in the enablement of
> Xen for qemu-system-aarch64.  The bug becomes apparent when building for
> Xen 4.18.

This looks like 2 issue, first meson, then
CONFIG_XEN_CTRL_INTERFACE_VERSION (the superseded patch).

> While the i386 emulator provides the xenpv machine type for multiple architectures,
> and therefore can be compiled with Xen enabled even when the host is Arm, the
> opposite is not true: qemu-system-aarch64 can only be compiled with Xen support
> enabled when the host is Arm.
> 
> Expand the computation of accelerator_targets['CONFIG_XEN'] similar to what is
> already there for KVM, and fix xen_native.h.
> 
> Cc: Stefano Stabellini <stefano.stabellini@amd.com>
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Reported-by: Michael Young <m.a.young@durham.ac.uk>
> Supersedes: <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.young@durham.ac.uk>

(strip the Supersedes: tag)

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/hw/xen/xen_native.h |  2 +-
>   meson.build                 | 17 ++++++++++-------
>   2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 6f09c48823b..1a5ad693a4d 100644
> --- a/include/hw/xen/xen_native.h
> +++ b/include/hw/xen/xen_native.h
> @@ -532,7 +532,7 @@ static inline int xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
>   }
>   #endif
>   
> -#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41700
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41700

I don't know about this, but for the rest (meson):
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] xen: fix condition for enabling the Xen accelerator

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2999 bytes --]

On Sat, 9 Dec 2023, Paolo Bonzini wrote:
> A misspelled condition in xen_native.h is hiding a bug in the enablement of
> Xen for qemu-system-aarch64.  The bug becomes apparent when building for
> Xen 4.18.
> 
> While the i386 emulator provides the xenpv machine type for multiple architectures,
> and therefore can be compiled with Xen enabled even when the host is Arm, the
> opposite is not true: qemu-system-aarch64 can only be compiled with Xen support
> enabled when the host is Arm.
> 
> Expand the computation of accelerator_targets['CONFIG_XEN'] similar to what is
> already there for KVM, and fix xen_native.h.
> 
> Cc: Stefano Stabellini <stefano.stabellini@amd.com>
> Cc: Richard W.M. Jones <rjones@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Reported-by: Michael Young <m.a.young@durham.ac.uk>
> Supersedes: <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.young@durham.ac.uk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  include/hw/xen/xen_native.h |  2 +-
>  meson.build                 | 17 ++++++++++-------
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 6f09c48823b..1a5ad693a4d 100644
> --- a/include/hw/xen/xen_native.h
> +++ b/include/hw/xen/xen_native.h
> @@ -532,7 +532,7 @@ static inline int xendevicemodel_set_irq_level(xendevicemodel_handle *dmod,
>  }
>  #endif
>  
> -#if CONFIG_XEN_CTRL_INTERFACE_VERSION <= 41700
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 41700
>  #define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x02000000)
>  #define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x00100000)
>  #define GUEST_VIRTIO_MMIO_SPI_FIRST   33
> diff --git a/meson.build b/meson.build
> index ec01f8b138a..67f4ede8aea 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -123,21 +123,24 @@ if get_option('kvm').allowed() and targetos == 'linux'
>    kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"'
>  endif
>  config_host_data.set('CONFIG_KVM_TARGETS', kvm_targets_c)
> -
>  accelerator_targets = { 'CONFIG_KVM': kvm_targets }
>  
> +if cpu in ['x86', 'x86_64']
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu']
> +elif cpu in ['arm', 'aarch64']
> +  # i386 emulator provides xenpv machine type for multiple architectures
> +  xen_targets = ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu']
> +else
> +  xen_targets = []
> +endif
> +accelerator_targets += { 'CONFIG_XEN': xen_targets }
> +
>  if cpu in ['aarch64']
>    accelerator_targets += {
>      'CONFIG_HVF': ['aarch64-softmmu']
>    }
>  endif
>  
> -if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
> -  # i386 emulator provides xenpv machine type for multiple architectures
> -  accelerator_targets += {
> -    'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'],
> -  }
> -endif
>  if cpu in ['x86', 'x86_64']
>    accelerator_targets += {
>      'CONFIG_HVF': ['x86_64-softmmu'],
> -- 
> 2.43.0
> 
>