[PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects

Philippe Mathieu-Daudé posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200728100925.10454-1-philmd@redhat.com
include/sysemu/xen.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
[PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago
CONFIG_XEN is generated by configure and stored in "config-target.h",
which is (obviously) only include for target-specific objects.
This is a problem for target-agnostic objects as CONFIG_XEN is never
defined and xen_enabled() is always inlined as 'false'.

Fix by following the KVM schema, defining CONFIG_XEN_IS_POSSIBLE
when we don't know to force the call of the non-inlined function,
returning the xen_allowed boolean.

Fixes: da278d58a092 ("accel: Move Xen accelerator code under accel/xen/")
Reported-by: Paul Durrant <pdurrant@amazon.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/sysemu/xen.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index 1ca292715e..385a1fa2bf 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -8,7 +8,15 @@
 #ifndef SYSEMU_XEN_H
 #define SYSEMU_XEN_H
 
-#ifdef CONFIG_XEN
+#ifdef NEED_CPU_H
+# ifdef CONFIG_XEN
+#  define CONFIG_XEN_IS_POSSIBLE
+# endif
+#else
+# define CONFIG_XEN_IS_POSSIBLE
+#endif
+
+#ifdef CONFIG_XEN_IS_POSSIBLE
 
 bool xen_enabled(void);
 
@@ -18,7 +26,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
                    struct MemoryRegion *mr, Error **errp);
 #endif
 
-#else /* !CONFIG_XEN */
+#else /* !CONFIG_XEN_IS_POSSIBLE */
 
 #define xen_enabled() 0
 #ifndef CONFIG_USER_ONLY
@@ -33,6 +41,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
 }
 #endif
 
-#endif /* CONFIG_XEN */
+#endif /* CONFIG_XEN_IS_POSSIBLE */
 
 #endif
-- 
2.21.3


Re: [PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
Posted by no-reply@patchew.org 3 years, 8 months ago
Patchew URL: https://patchew.org/QEMU/20200728100925.10454-1-philmd@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20200728100925.10454-1-philmd@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
Posted by Anthony PERARD 3 years, 7 months ago
On Tue, Jul 28, 2020 at 12:09:25PM +0200, Philippe Mathieu-Daudé wrote:
> CONFIG_XEN is generated by configure and stored in "config-target.h",
> which is (obviously) only include for target-specific objects.
> This is a problem for target-agnostic objects as CONFIG_XEN is never
> defined and xen_enabled() is always inlined as 'false'.
> 
> Fix by following the KVM schema, defining CONFIG_XEN_IS_POSSIBLE
> when we don't know to force the call of the non-inlined function,
> returning the xen_allowed boolean.
> 
> Fixes: da278d58a092 ("accel: Move Xen accelerator code under accel/xen/")
> Reported-by: Paul Durrant <pdurrant@amazon.com>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Re: [PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
Posted by Peter Maydell 3 years, 7 months ago
On Mon, 3 Aug 2020 at 17:35, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Tue, Jul 28, 2020 at 12:09:25PM +0200, Philippe Mathieu-Daudé wrote:
> > CONFIG_XEN is generated by configure and stored in "config-target.h",
> > which is (obviously) only include for target-specific objects.
> > This is a problem for target-agnostic objects as CONFIG_XEN is never
> > defined and xen_enabled() is always inlined as 'false'.
> >
> > Fix by following the KVM schema, defining CONFIG_XEN_IS_POSSIBLE
> > when we don't know to force the call of the non-inlined function,
> > returning the xen_allowed boolean.
> >
> > Fixes: da278d58a092 ("accel: Move Xen accelerator code under accel/xen/")
> > Reported-by: Paul Durrant <pdurrant@amazon.com>
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Note that rc3 is tomorrow so if you want this in 5.1 it would
be a good idea to send a pullreq with it today...

thanks
-- PMM

RE: [PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
Posted by Paul Durrant 3 years, 8 months ago
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: 28 July 2020 11:09
> To: qemu-devel@nongnu.org
> Cc: Paul Durrant <paul@xen.org>; Paolo Bonzini <pbonzini@redhat.com>; xen-devel@lists.xenproject.org;
> Stefano Stabellini <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com>; Philippe
> Mathieu-Daudé <philmd@redhat.com>; Paul Durrant <pdurrant@amazon.com>; Peter Maydell
> <peter.maydell@linaro.org>
> Subject: [PATCH-for-5.1] accel/xen: Fix xen_enabled() behavior on target-agnostic objects
> 
> CONFIG_XEN is generated by configure and stored in "config-target.h",
> which is (obviously) only include for target-specific objects.
> This is a problem for target-agnostic objects as CONFIG_XEN is never
> defined and xen_enabled() is always inlined as 'false'.
> 
> Fix by following the KVM schema, defining CONFIG_XEN_IS_POSSIBLE
> when we don't know to force the call of the non-inlined function,
> returning the xen_allowed boolean.
> 
> Fixes: da278d58a092 ("accel: Move Xen accelerator code under accel/xen/")
> Reported-by: Paul Durrant <pdurrant@amazon.com>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Paul Durrant <paul@xen.org>

> ---
>  include/sysemu/xen.h | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
> index 1ca292715e..385a1fa2bf 100644
> --- a/include/sysemu/xen.h
> +++ b/include/sysemu/xen.h
> @@ -8,7 +8,15 @@
>  #ifndef SYSEMU_XEN_H
>  #define SYSEMU_XEN_H
> 
> -#ifdef CONFIG_XEN
> +#ifdef NEED_CPU_H
> +# ifdef CONFIG_XEN
> +#  define CONFIG_XEN_IS_POSSIBLE
> +# endif
> +#else
> +# define CONFIG_XEN_IS_POSSIBLE
> +#endif
> +
> +#ifdef CONFIG_XEN_IS_POSSIBLE
> 
>  bool xen_enabled(void);
> 
> @@ -18,7 +26,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
>                     struct MemoryRegion *mr, Error **errp);
>  #endif
> 
> -#else /* !CONFIG_XEN */
> +#else /* !CONFIG_XEN_IS_POSSIBLE */
> 
>  #define xen_enabled() 0
>  #ifndef CONFIG_USER_ONLY
> @@ -33,6 +41,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
>  }
>  #endif
> 
> -#endif /* CONFIG_XEN */
> +#endif /* CONFIG_XEN_IS_POSSIBLE */
> 
>  #endif
> --
> 2.21.3