[PATCH] fix qemu build with xen-4.18.0

[PATCH] fix qemu build with xen-4.18.0

[Michael Young]

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

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

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

[Richard W.M. Jones]

(Adding Xen maintainers)

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
> 
> Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

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

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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

[Richard W.M. Jones]

On Fri, Dec 08, 2023 at 08:47:07AM +0000, Richard W.M. Jones wrote:
> (Adding Xen maintainers)
> 
> 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
> > 
> > Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> 
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

I added this patch to Fedora, which has Xen 4.18 and where
builds were previously failing, and now it's working:

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

So also adding:

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

> > ---
> >  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
> >  #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
> > -- 
> > 2.43.0
> > 
> 
> -- 
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> libguestfs lets you edit virtual machines.  Supports shell scripting,
> bindings from many languages.  http://libguestfs.org
> 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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

[Richard W.M. Jones]

On Fri, Dec 08, 2023 at 08:47:07AM +0000, Richard W.M. Jones wrote:
> (Adding Xen maintainers)
> 
> 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
> > 
> > Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> 
> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Actually, see Dan Berrange's answer in this thread.

Rich.

> > ---
> >  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
> >  #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
> > -- 
> > 2.43.0
> > 
> 
> -- 
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> libguestfs lets you edit virtual machines.  Supports shell scripting,
> bindings from many languages.  http://libguestfs.org
> 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

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

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

[Peter Maydell]

On Fri, 8 Dec 2023 at 09:25, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> CC'ing the Xen folks
>
> On Thu, Dec 07, 2023 at 11:12:48PM +0000, Michael Young wrote:
> > 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.

Wouldn't that suggest we want "< 41700" ? Or did 4.17 have
some issue that means we need the back-compat there too?

thanks
-- PMM

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

[Daniel P. Berrangé]

On Fri, Dec 08, 2023 at 10:59:03AM +0000, Peter Maydell wrote:
> On Fri, 8 Dec 2023 at 09:25, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > CC'ing the Xen folks
> >
> > On Thu, Dec 07, 2023 at 11:12:48PM +0000, Michael Young wrote:
> > > 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.
> 
> Wouldn't that suggest we want "< 41700" ? Or did 4.17 have
> some issue that means we need the back-compat there too?

Oh yes, and if we change it from '<=' to '<', then we get the same
build problem for  qemu-system-aarch64 on x86_64 host, when built
against xen-devel 4.17, which is what I'd expect.

So our accident <= instead of < has masked the pre-existing flaw.

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

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

[Stefano Stabellini]

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

On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
> 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.

Hi Daniel, thanks for looking into it.

- you are building on a x86_64 host
- the target is aarch64
- the target is the aarch64 Xen PVH machine (xen_arm.c)

But is the resulting QEMU binary expected to be an x86 binary? Or are
you cross compiling ARM binaries on a x86 host?

In other word, is the resulting QEMU binary expected to run on ARM or
x86?


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

The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
Xen on x86.  So this is only expected to work if you are
cross-compiling. But you can cross-compile both Xen and QEMU, and I am
pretty sure that Yocto is able to build Xen, Xen userspace tools, and
QEMU for Xen/ARM on an x86 host today.


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

It looks like a host vs. target architecture mismatch: the #if defined
(__aarch64__) check should pass I think.


The following is a guess. Maybe Xen gets enabled because you have x86
Xen installed, and you are building QEMU for an aarch64 target (aarch64
emulation, running on a x86 host). So this is not meant to work for
xen_arm.c and it would be better to disable xen_arm.c.

On the other hand if you are trying to cross-build a QEMU binary meant
to run on an aarch64 host, cross-building it on an x86_64 host, then yes
this is meant to work and we need to figure out why the #if defined
(__aarch64__) is not passing.



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

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

[Anthony PERARD]

On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
> > 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.
> 
> Hi Daniel, thanks for looking into it.
> 
> - you are building on a x86_64 host
> - the target is aarch64
> - the target is the aarch64 Xen PVH machine (xen_arm.c)
> 
> But is the resulting QEMU binary expected to be an x86 binary? Or are
> you cross compiling ARM binaries on a x86 host?
> 
> In other word, is the resulting QEMU binary expected to run on ARM or
> x86?
> 
> 
> > Are we expecting to build Xen support for non-arch native QEMU
> > system binaries or not ?
> 
> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
> Xen on x86.  So this is only expected to work if you are
> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
> QEMU for Xen/ARM on an x86 host today.
> 
> 
> > 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 ?
> 
> It looks like a host vs. target architecture mismatch: the #if defined
> (__aarch64__) check should pass I think.


Building qemu with something like:
    ./configure --enable-xen --cpu=x86_64
used to work. Can we fix that? It still works with v8.1.0.
At least, it works on x86, I never really try to build qemu for arm.
Notice that there's no "--target-list" on the configure command line.
I don't know if --cpu is useful here.

Looks like the first commit where the build doesn't work is
7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").

Could we get that fixed?

I'm sure distribution will appreciate to be able to build a single qemu
package for xen and other, rather than having a dedicated qemu-xen
package.

Cheers,

-- 
Anthony PERARD

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

[Peter Maydell]

On Tue, 12 Dec 2023 at 14:20, Anthony PERARD <anthony.perard@citrix.com> wrote:
> Building qemu with something like:
>     ./configure --enable-xen --cpu=x86_64
> used to work. Can we fix that? It still works with v8.1.0.
> At least, it works on x86, I never really try to build qemu for arm.
> Notice that there's no "--target-list" on the configure command line.
> I don't know if --cpu is useful here.

You should almost never need to specify --cpu : configure
will work out your host CPU architecture by looking at what
the host compiler defines.

thanks
-- PMM

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

[Volodymyr Babchuk]

Hi Anthony

Anthony PERARD <anthony.perard@citrix.com> writes:

> On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
>> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
>> > 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.
>> 
>> Hi Daniel, thanks for looking into it.
>> 
>> - you are building on a x86_64 host
>> - the target is aarch64
>> - the target is the aarch64 Xen PVH machine (xen_arm.c)
>> 
>> But is the resulting QEMU binary expected to be an x86 binary? Or are
>> you cross compiling ARM binaries on a x86 host?
>> 
>> In other word, is the resulting QEMU binary expected to run on ARM or
>> x86?
>> 
>> 
>> > Are we expecting to build Xen support for non-arch native QEMU
>> > system binaries or not ?
>> 
>> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
>> Xen on x86.  So this is only expected to work if you are
>> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
>> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
>> QEMU for Xen/ARM on an x86 host today.
>> 
>> 
>> > 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 ?
>> 
>> It looks like a host vs. target architecture mismatch: the #if defined
>> (__aarch64__) check should pass I think.
>
>
> Building qemu with something like:
>     ./configure --enable-xen --cpu=x86_64
> used to work. Can we fix that? It still works with v8.1.0.
> At least, it works on x86, I never really try to build qemu for arm.
> Notice that there's no "--target-list" on the configure command line.
> I don't know if --cpu is useful here.
>
> Looks like the first commit where the build doesn't work is
> 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").

I am currently trying to upstream this patch. It is in the QEMU mailing
list but it was never accepted. It is not reviewed in fact. I'll take a
look at it, but I don't understand how did you get in the first place.

-- 
WBR, Volodymyr

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

[Stefan Hajnoczi]

On Tue, 12 Dec 2023 at 10:36, Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
> Hi Anthony
>
> Anthony PERARD <anthony.perard@citrix.com> writes:
>
> > On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
> >> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
> >> > 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.
> >>
> >> Hi Daniel, thanks for looking into it.
> >>
> >> - you are building on a x86_64 host
> >> - the target is aarch64
> >> - the target is the aarch64 Xen PVH machine (xen_arm.c)
> >>
> >> But is the resulting QEMU binary expected to be an x86 binary? Or are
> >> you cross compiling ARM binaries on a x86 host?
> >>
> >> In other word, is the resulting QEMU binary expected to run on ARM or
> >> x86?
> >>
> >>
> >> > Are we expecting to build Xen support for non-arch native QEMU
> >> > system binaries or not ?
> >>
> >> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
> >> Xen on x86.  So this is only expected to work if you are
> >> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
> >> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
> >> QEMU for Xen/ARM on an x86 host today.
> >>
> >>
> >> > 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 ?
> >>
> >> It looks like a host vs. target architecture mismatch: the #if defined
> >> (__aarch64__) check should pass I think.
> >
> >
> > Building qemu with something like:
> >     ./configure --enable-xen --cpu=x86_64
> > used to work. Can we fix that? It still works with v8.1.0.
> > At least, it works on x86, I never really try to build qemu for arm.
> > Notice that there's no "--target-list" on the configure command line.
> > I don't know if --cpu is useful here.
> >
> > Looks like the first commit where the build doesn't work is
> > 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").
>
> I am currently trying to upstream this patch. It is in the QEMU mailing
> list but it was never accepted. It is not reviewed in fact. I'll take a
> look at it, but I don't understand how did you get in the first place.

Hi Volodymyr,
Paolo Bonzini sent a pull request with similar code changes this
morning and I have merged it into the qemu.git/staging branch:
https://gitlab.com/qemu-project/qemu/-/commit/eaae59af4035770975b0ce9364b587223a909501

If you spot something that is not correct, please reply here.

Thanks!

Stefan

>
> --
> WBR, Volodymyr

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

[Volodymyr Babchuk]

Hi Stefan,

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Tue, 12 Dec 2023 at 10:36, Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com> wrote:
>>
>> Hi Anthony
>>
>> Anthony PERARD <anthony.perard@citrix.com> writes:
>>
>> > On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
>> >> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
>> >> > 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.
>> >>
>> >> Hi Daniel, thanks for looking into it.
>> >>
>> >> - you are building on a x86_64 host
>> >> - the target is aarch64
>> >> - the target is the aarch64 Xen PVH machine (xen_arm.c)
>> >>
>> >> But is the resulting QEMU binary expected to be an x86 binary? Or are
>> >> you cross compiling ARM binaries on a x86 host?
>> >>
>> >> In other word, is the resulting QEMU binary expected to run on ARM or
>> >> x86?
>> >>
>> >>
>> >> > Are we expecting to build Xen support for non-arch native QEMU
>> >> > system binaries or not ?
>> >>
>> >> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
>> >> Xen on x86.  So this is only expected to work if you are
>> >> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
>> >> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
>> >> QEMU for Xen/ARM on an x86 host today.
>> >>
>> >>
>> >> > 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 ?
>> >>
>> >> It looks like a host vs. target architecture mismatch: the #if defined
>> >> (__aarch64__) check should pass I think.
>> >
>> >
>> > Building qemu with something like:
>> >     ./configure --enable-xen --cpu=x86_64
>> > used to work. Can we fix that? It still works with v8.1.0.
>> > At least, it works on x86, I never really try to build qemu for arm.
>> > Notice that there's no "--target-list" on the configure command line.
>> > I don't know if --cpu is useful here.
>> >
>> > Looks like the first commit where the build doesn't work is
>> > 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").
>>
>> I am currently trying to upstream this patch. It is in the QEMU mailing
>> list but it was never accepted. It is not reviewed in fact. I'll take a
>> look at it, but I don't understand how did you get in the first place.
>
> Hi Volodymyr,
> Paolo Bonzini sent a pull request with similar code changes this
> morning and I have merged it into the qemu.git/staging branch:
> https://urldefense.com/v3/__https://gitlab.com/qemu-project/qemu/-/commit/eaae59af4035770975b0ce9364b587223a909501__;!!GF_29dbcQIUBPA!yFgSxAEgXPjckF8piSt0T77bbeggSgwC-6-xDuZmzq4a8U7HEP8XxGnxwIhgA9iyFVie-fdVgAVA5wVipnewbLNp$
> [gitlab[.]com]
>
> If you spot something that is not correct, please reply here.
>

No, it is all fine in that pull request. I was talking about patch
"xen_arm: Add virtual PCIe host bridge support" which is still on
review:
https://patchwork.kernel.org/project/qemu-devel/patch/20231202014108.2017803-7-volodymyr_babchuk@epam.com/

I was surprised when Anthony mentioned that this patch breaks the
build, because the patch is not included in QEMU tree.

-- 
WBR, Volodymyr

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

[Stefan Hajnoczi]

On Tue, 12 Dec 2023 at 11:02, Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi Stefan,
>
> Stefan Hajnoczi <stefanha@gmail.com> writes:
>
> > On Tue, 12 Dec 2023 at 10:36, Volodymyr Babchuk
> > <Volodymyr_Babchuk@epam.com> wrote:
> >>
> >> Hi Anthony
> >>
> >> Anthony PERARD <anthony.perard@citrix.com> writes:
> >>
> >> > On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
> >> >> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
> >> >> > 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.
> >> >>
> >> >> Hi Daniel, thanks for looking into it.
> >> >>
> >> >> - you are building on a x86_64 host
> >> >> - the target is aarch64
> >> >> - the target is the aarch64 Xen PVH machine (xen_arm.c)
> >> >>
> >> >> But is the resulting QEMU binary expected to be an x86 binary? Or are
> >> >> you cross compiling ARM binaries on a x86 host?
> >> >>
> >> >> In other word, is the resulting QEMU binary expected to run on ARM or
> >> >> x86?
> >> >>
> >> >>
> >> >> > Are we expecting to build Xen support for non-arch native QEMU
> >> >> > system binaries or not ?
> >> >>
> >> >> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
> >> >> Xen on x86.  So this is only expected to work if you are
> >> >> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
> >> >> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
> >> >> QEMU for Xen/ARM on an x86 host today.
> >> >>
> >> >>
> >> >> > 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 ?
> >> >>
> >> >> It looks like a host vs. target architecture mismatch: the #if defined
> >> >> (__aarch64__) check should pass I think.
> >> >
> >> >
> >> > Building qemu with something like:
> >> >     ./configure --enable-xen --cpu=x86_64
> >> > used to work. Can we fix that? It still works with v8.1.0.
> >> > At least, it works on x86, I never really try to build qemu for arm.
> >> > Notice that there's no "--target-list" on the configure command line.
> >> > I don't know if --cpu is useful here.
> >> >
> >> > Looks like the first commit where the build doesn't work is
> >> > 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").
> >>
> >> I am currently trying to upstream this patch. It is in the QEMU mailing
> >> list but it was never accepted. It is not reviewed in fact. I'll take a
> >> look at it, but I don't understand how did you get in the first place.
> >
> > Hi Volodymyr,
> > Paolo Bonzini sent a pull request with similar code changes this
> > morning and I have merged it into the qemu.git/staging branch:
> > https://urldefense.com/v3/__https://gitlab.com/qemu-project/qemu/-/commit/eaae59af4035770975b0ce9364b587223a909501__;!!GF_29dbcQIUBPA!yFgSxAEgXPjckF8piSt0T77bbeggSgwC-6-xDuZmzq4a8U7HEP8XxGnxwIhgA9iyFVie-fdVgAVA5wVipnewbLNp$
> > [gitlab[.]com]
> >
> > If you spot something that is not correct, please reply here.
> >
>
> No, it is all fine in that pull request. I was talking about patch
> "xen_arm: Add virtual PCIe host bridge support" which is still on
> review:
> https://patchwork.kernel.org/project/qemu-devel/patch/20231202014108.2017803-7-volodymyr_babchuk@epam.com/
>
> I was surprised when Anthony mentioned that this patch breaks the
> build, because the patch is not included in QEMU tree.

Ah, I jumped straight to the last email and didn't realize :).

Stefan

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

[Anthony PERARD]

On Tue, Dec 12, 2023 at 03:35:50PM +0000, Volodymyr Babchuk wrote:
> Hi Anthony
> 
> Anthony PERARD <anthony.perard@citrix.com> writes:
> 
> > On Fri, Dec 08, 2023 at 02:49:27PM -0800, Stefano Stabellini wrote:
> >> On Fri, 8 Dec 2023, Daniel P. Berrangé wrote:
> >> > 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.
> >> 
> >> Hi Daniel, thanks for looking into it.
> >> 
> >> - you are building on a x86_64 host
> >> - the target is aarch64
> >> - the target is the aarch64 Xen PVH machine (xen_arm.c)
> >> 
> >> But is the resulting QEMU binary expected to be an x86 binary? Or are
> >> you cross compiling ARM binaries on a x86 host?
> >> 
> >> In other word, is the resulting QEMU binary expected to run on ARM or
> >> x86?
> >> 
> >> 
> >> > Are we expecting to build Xen support for non-arch native QEMU
> >> > system binaries or not ?
> >> 
> >> The ARM xenpvh machine (xen_arm.c) is meant to work with Xen on ARM, not
> >> Xen on x86.  So this is only expected to work if you are
> >> cross-compiling. But you can cross-compile both Xen and QEMU, and I am
> >> pretty sure that Yocto is able to build Xen, Xen userspace tools, and
> >> QEMU for Xen/ARM on an x86 host today.
> >> 
> >> 
> >> > 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 ?
> >> 
> >> It looks like a host vs. target architecture mismatch: the #if defined
> >> (__aarch64__) check should pass I think.
> >
> >
> > Building qemu with something like:
> >     ./configure --enable-xen --cpu=x86_64
> > used to work. Can we fix that? It still works with v8.1.0.
> > At least, it works on x86, I never really try to build qemu for arm.
> > Notice that there's no "--target-list" on the configure command line.
> > I don't know if --cpu is useful here.
> >
> > Looks like the first commit where the build doesn't work is
> > 7899f6589b78 ("xen_arm: Add virtual PCIe host bridge support").
> 
> I am currently trying to upstream this patch. It is in the QEMU mailing
> list but it was never accepted. It is not reviewed in fact. I'll take a
> look at it, but I don't understand how did you get in the first place.

Sorry, I got the wrong commit pasted, I actually meant:
0c8ab1cddd6c ("xen_arm: Create virtio-mmio devices during initialization")

-- 
Anthony PERARD