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

Paolo Bonzini posted 1 patch 11 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231209143222.2916891-1-pbonzini@redhat.com
Maintainers: Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
include/hw/xen/xen_native.h |  2 +-
meson.build                 | 17 ++++++++++-------
2 files changed, 11 insertions(+), 8 deletions(-)
[PATCH] xen: fix condition for enabling the Xen accelerator
Posted by Paolo Bonzini 11 months, 3 weeks ago
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
Posted by Stefano Stabellini 11 months, 2 weeks ago
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
> 
> 
Re: [PATCH] xen: fix condition for enabling the Xen accelerator
Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago
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
Posted by Richard W.M. Jones 11 months, 3 weeks ago
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