Re: [PATCH] fix qemu build with xen-4.18.0

[Daniel P . Berrangé]

CC'ing the Xen folks

On Thu, Dec 07, 2023 at 11:12:48PM +0000, Michael Young wrote:
> Builds of qemu-8.2.0rc2 with xen-4.18.0 are currently failing
> with errors like
> ../hw/arm/xen_arm.c:74:5: error: ‘GUEST_VIRTIO_MMIO_SPI_LAST’ undeclared 
> (first use in this function)
>    74 |    (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> as there is an incorrect comparision in include/hw/xen/xen_native.h
> which means that settings like GUEST_VIRTIO_MMIO_SPI_LAST
> aren't being defined for xen-4.18.0

The conditions in arch-arm.h for xen 4.18 show:

$ cppi arch-arm.h | grep -E '(#.*if)|MMIO'
#ifndef __XEN_PUBLIC_ARCH_ARM_H__
# if defined(__XEN__) || defined(__XEN_TOOLS__) || defined(__GNUC__)
# endif
# ifndef __ASSEMBLY__
#  if defined(__XEN__) || defined(__XEN_TOOLS__)
#   if defined(__GNUC__) && !defined(__STRICT_ANSI__)
#   endif
#  endif /* __XEN__ || __XEN_TOOLS__ */
# endif
# if defined(__XEN__) || defined(__XEN_TOOLS__)
#  define PSR_MODE_BIT  0x10U /* Set iff AArch32 */
/* Virtio MMIO mappings */
#  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
#  define GUEST_VIRTIO_MMIO_SPI_LAST    43
# endif
# ifndef __ASSEMBLY__
# endif
#endif /*  __XEN_PUBLIC_ARCH_ARM_H__ */

So the MMIO constants are available if __XEN__ or __XEN_TOOLS__
are defined. This is no different to the condition that was
present in Xen 4.17.

What you didn't mention was that the Fedora build failure is
seen on an x86_64 host, when building the aarch64 target QEMU,
and I think this is the key issue.

The constants are defined in arch-arm.h, which is only included
under:

  #if defined(__i386__) || defined(__x86_64__)
  #include "arch-x86/xen.h"
  #elif defined(__arm__) || defined (__aarch64__)
  #include "arch-arm.h"
  #else
  #error "Unsupported architecture"
  #endif


When we are building on an x86_64 host, we not going to get
arch-arm.h included, even if we're trying to build the aarch64
system emulator.

I don't know how this is supposed to work ?

Are we expecting to build Xen support for non-arch native QEMU
system binaries or not ?

> Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> ---
>  include/hw/xen/xen_native.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h
> index 6f09c48823..04b1ef4d34 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

This change is not correct

We can see the upstream change was introduced in 4.17:

  $ git describe  2128143c114
  4.16.0-rc4-967-g2128143c11

IOW, if we have 4.17 or newer these constants already
exist. If we have 4.16 or older, then we need to define
them to provide back compat.

>  #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

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|