qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Michael Young <m.a.young@durham.ac.uk>
Cc: qemu-devel@nongnu.org,
	Stefano Stabellini <sstabellini@kernel.org>,
	Anthony PERARD <anthony.perard@citrix.com>
Subject: Re: [PATCH] fix qemu build with xen-4.18.0
Date: Fri, 8 Dec 2023 09:25:17 +0000	[thread overview]
Message-ID: <ZXLg_YCHM-P6drQV@redhat.com> (raw)
In-Reply-To: <277e21fc78b75ec459efc7f5fde628a0222c63b0.1701989261.git.m.a.young@durham.ac.uk>

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



  parent reply	other threads:[~2023-12-08  9:25 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-07 23:12 [PATCH] fix qemu build with xen-4.18.0 Michael Young
2023-12-08  8:47 ` Richard W.M. Jones
2023-12-08  9:59   ` Richard W.M. Jones
2023-12-08 10:00   ` Richard W.M. Jones
2023-12-08  9:25 ` Daniel P. Berrangé [this message]
2023-12-08 10:59   ` Peter Maydell
2023-12-08 11:34     ` Daniel P. Berrangé
2023-12-08 22:49   ` Stefano Stabellini
2023-12-12 14:19     ` Anthony PERARD
2023-12-12 14:36       ` Peter Maydell
2023-12-12 15:35       ` Volodymyr Babchuk
2023-12-12 15:47         ` Stefan Hajnoczi
2023-12-12 16:02           ` Volodymyr Babchuk
2023-12-12 16:29             ` Stefan Hajnoczi
2023-12-12 16:00         ` Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZXLg_YCHM-P6drQV@redhat.com \
    --to=berrange@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=m.a.young@durham.ac.uk \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).