Re: [Xen-devel] [Qemu-devel] [PATCH] xen: Fix ring.h header

[Markus Armbruster <armbru@xxxxxxxxxx>]

Anthony PERARD <anthony.perard@xxxxxxxxxx> writes:

> The xen_[rw]?mb() macros defined in ring.h can't be used and the fact
> that there are gated behind __XEN_INTERFACE_VERSION__ means that it
> needs to be defined somewhere. QEMU doesn't implement interfaces with
> the Xen hypervisor so defining __XEN_INTERFACE_VERSION__ is pointless.
> This leads to:
>     include/hw/xen/io/ring.h:47:5: error: "__XEN_INTERFACE_VERSION__"
>         is not defined, evaluates to 0 [-Werror=undef]
>
> Cleanup ring.h. The xen_*mb() macros are already defined in xenctrl.h
> which is included in xen_common.h.
>
> Reported-by: Markus Armbruster <armbru@xxxxxxxxxx>
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>
> Notes:
>     A similar patch have been sent to the canonical version:
>     https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg00370.html

That patch has an additional hunk:

  @@ -33,6 +33,13 @@
    * - standard integers types (uint8_t, uint16_t, etc)
    * They are provided by stdint.h of the standard headers.
    *
  + * Before using the different macros, you need to provide the following
  + * macros:
  + * - xen_mb()  a memory barrier
  + * - xen_rmb() a read memory barrier
  + * - xen_wmb() a write memory barrier
  + * Example of those can be found in xenctrl.h.
  + *
    * In addition, if you intend to use the FLEX macros, you also need to
    * provide the following, before invoking the FLEX macros:
    * - size_t

Suggest to include it in this patch, because (1) it's useful (I think),
and (2) it reduces differences to upstream Xen's version.

>
>  include/hw/xen/interface/io/ring.h | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/include/hw/xen/interface/io/ring.h 
> b/include/hw/xen/interface/io/ring.h
> index 1adacf09f9..704990a2c9 100644
> --- a/include/hw/xen/interface/io/ring.h
> +++ b/include/hw/xen/interface/io/ring.h
> @@ -42,12 +42,6 @@
>   * and grant_table.h from the Xen public headers.
>   */
>  
> -#if __XEN_INTERFACE_VERSION__ < 0x00030208
> -#define xen_mb()  mb()
> -#define xen_rmb() rmb()
> -#define xen_wmb() wmb()
> -#endif
> -
>  typedef unsigned int RING_IDX;
>  
>  /* Round a 32-bit unsigned constant down to the nearest power of two. */

Regardless of my suggestion:
Reviewed-by: Markus Armbruster <armbru@xxxxxxxxxx>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel