1. Patchew
  2. Xen
  3. View series

[PATCH for-4.15 v3] xen/dmop: Strip __XEN_TOOLS__ header guard from public ABI

Andrew Cooper posted 1 patch 3 years, 1 month ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210311132400.24411-1-andrew.cooper3@citrix.com
docs/designs/dmop.pandoc         | 12 +++++++++---
tools/libs/devicemodel/private.h |  2 --
xen/include/public/hvm/dm_op.h   |  5 -----
3 files changed, 9 insertions(+), 10 deletions(-)
[PATCH for-4.15 v3] xen/dmop: Strip __XEN_TOOLS__ header guard from public ABI
Posted by Andrew Cooper 3 years, 1 month ago 
__XEN_TOOLS__ is really there to separate the unstable from stable hypercalls.
Exactly as with c/s f40e1c52e4, stable interfaces shouldn't contain this
guard.

That change actually broke the build with:

  include/xendevicemodel.h:52:5: error: unknown type name 'ioservid_t'
       ioservid_t *id);
       ^

as libxendevicemodel.h now uses a type it can't see a typedef for.  However,
nothing noticed because the header.chk logic is also broken (fixed
subsequently).

Strip the guard from the public header, and remove compensation from
devicemodel's private.h.  Fix the dmop design doc to discuss both reasons
behind the the ABI design.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Paul Durrant <paul@xen.org>
CC: Ian Jackson <iwj@xenproject.org>

v3:
 * Clarifications to the commit message, and dmop.pandoc
v2:
 * Patch dmop.pandoc as well.
---
 docs/designs/dmop.pandoc         | 12 +++++++++---
 tools/libs/devicemodel/private.h |  2 --
 xen/include/public/hvm/dm_op.h   |  5 -----
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/docs/designs/dmop.pandoc b/docs/designs/dmop.pandoc
index 8e9f95af47..49e52b1bcc 100644
--- a/docs/designs/dmop.pandoc
+++ b/docs/designs/dmop.pandoc
@@ -4,9 +4,15 @@ DMOP
 Introduction
 ------------
 
-The aim of DMOP is to prevent a compromised device model from compromising
-domains other than the one it is providing emulation for (which is therefore
-likely already compromised).
+The DMOP hypercall has a new ABI design to solve problems in the Xen
+ecosystem.  First, the ABI is fully stable, to reduce the coupling between
+device models and the version of Xen.  Specifically, device model software
+using DMOP (be it user, stub domain or kernel software) need not be recompiled
+to match the version of the running hypervisor.
+
+Secondly, for device models in userspace, the ABI is designed specifically to
+allow a kernel to audit the memory ranges used, without having to know the
+internal structure of sub-ops.
 
 The problem occurs when you a device model issues an hypercall that
 includes references to user memory other than the operation structure
diff --git a/tools/libs/devicemodel/private.h b/tools/libs/devicemodel/private.h
index c4a225f8af..c24f3396bb 100644
--- a/tools/libs/devicemodel/private.h
+++ b/tools/libs/devicemodel/private.h
@@ -1,8 +1,6 @@
 #ifndef XENDEVICEMODEL_PRIVATE_H
 #define XENDEVICEMODEL_PRIVATE_H
 
-#define __XEN_TOOLS__ 1
-
 #include <xentoollog.h>
 #include <xendevicemodel.h>
 #include <xencall.h>
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index ef7fbc0d3d..fa3f083fed 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -25,9 +25,6 @@
 #define __XEN_PUBLIC_HVM_DM_OP_H__
 
 #include "../xen.h"
-
-#if defined(__XEN__) || defined(__XEN_TOOLS__)
-
 #include "../event_channel.h"
 
 #ifndef uint64_aligned_t
@@ -491,8 +488,6 @@ struct xen_dm_op {
     } u;
 };
 
-#endif /* __XEN__ || __XEN_TOOLS__ */
-
 struct xen_dm_op_buf {
     XEN_GUEST_HANDLE(void) h;
     xen_ulong_t size;
-- 
2.11.0
Re: [PATCH for-4.15 v3] xen/dmop: Strip __XEN_TOOLS__ header guard from public ABI
Posted by Paul Durrant 3 years, 1 month ago 
On 11/03/2021 13:24, Andrew Cooper wrote:
> __XEN_TOOLS__ is really there to separate the unstable from stable hypercalls.
> Exactly as with c/s f40e1c52e4, stable interfaces shouldn't contain this
> guard.
> 
> That change actually broke the build with:
> 
>    include/xendevicemodel.h:52:5: error: unknown type name 'ioservid_t'
>         ioservid_t *id);
>         ^
> 
> as libxendevicemodel.h now uses a type it can't see a typedef for.  However,
> nothing noticed because the header.chk logic is also broken (fixed
> subsequently).
> 
> Strip the guard from the public header, and remove compensation from
> devicemodel's private.h.  Fix the dmop design doc to discuss both reasons
> behind the the ABI design.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Release-Acked-by: Ian Jackson <iwj@xenproject.org>

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

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Paul Durrant <paul@xen.org>
> CC: Ian Jackson <iwj@xenproject.org>
> 
> v3:
>   * Clarifications to the commit message, and dmop.pandoc
> v2:
>   * Patch dmop.pandoc as well.
> ---
>   docs/designs/dmop.pandoc         | 12 +++++++++---
>   tools/libs/devicemodel/private.h |  2 --
>   xen/include/public/hvm/dm_op.h   |  5 -----
>   3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/designs/dmop.pandoc b/docs/designs/dmop.pandoc
> index 8e9f95af47..49e52b1bcc 100644
> --- a/docs/designs/dmop.pandoc
> +++ b/docs/designs/dmop.pandoc
> @@ -4,9 +4,15 @@ DMOP
>   Introduction
>   ------------
>   
> -The aim of DMOP is to prevent a compromised device model from compromising
> -domains other than the one it is providing emulation for (which is therefore
> -likely already compromised).
> +The DMOP hypercall has a new ABI design to solve problems in the Xen
> +ecosystem.  First, the ABI is fully stable, to reduce the coupling between
> +device models and the version of Xen.  Specifically, device model software
> +using DMOP (be it user, stub domain or kernel software) need not be recompiled
> +to match the version of the running hypervisor.
> +
> +Secondly, for device models in userspace, the ABI is designed specifically to
> +allow a kernel to audit the memory ranges used, without having to know the
> +internal structure of sub-ops.
>   
>   The problem occurs when you a device model issues an hypercall that
>   includes references to user memory other than the operation structure
> diff --git a/tools/libs/devicemodel/private.h b/tools/libs/devicemodel/private.h
> index c4a225f8af..c24f3396bb 100644
> --- a/tools/libs/devicemodel/private.h
> +++ b/tools/libs/devicemodel/private.h
> @@ -1,8 +1,6 @@
>   #ifndef XENDEVICEMODEL_PRIVATE_H
>   #define XENDEVICEMODEL_PRIVATE_H
>   
> -#define __XEN_TOOLS__ 1
> -
>   #include <xentoollog.h>
>   #include <xendevicemodel.h>
>   #include <xencall.h>
> diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
> index ef7fbc0d3d..fa3f083fed 100644
> --- a/xen/include/public/hvm/dm_op.h
> +++ b/xen/include/public/hvm/dm_op.h
> @@ -25,9 +25,6 @@
>   #define __XEN_PUBLIC_HVM_DM_OP_H__
>   
>   #include "../xen.h"
> -
> -#if defined(__XEN__) || defined(__XEN_TOOLS__)
> -
>   #include "../event_channel.h"
>   
>   #ifndef uint64_aligned_t
> @@ -491,8 +488,6 @@ struct xen_dm_op {
>       } u;
>   };
>   
> -#endif /* __XEN__ || __XEN_TOOLS__ */
> -
>   struct xen_dm_op_buf {
>       XEN_GUEST_HANDLE(void) h;
>       xen_ulong_t size;
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.