Re: [Xen-devel] [PATCH v3 09/11] tmem: Use 'struct tmem_oid' in tmem_handle and move it to sysctl header.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Aug 28, 2015 at 02:53:18PM -0400, Konrad Rzeszutek Wilk wrote:
> Instead of the three member uint64_t structure.
> 
> The structure is used by the control stack for
> XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_[PAGE|INV] only so
> moving it to the sysctl header.
> 
> Also modified tmemc_save_get_next_page to deal with
> the new type - and converted some of the on-stack
> usage of the array to use an pointer.
> 
> Further work will be to make the xen_sysctl_tmem_op have
> an union with proper type for the two: ..GET_NEXT_[PAGE|INV]
> operations.

.. with the changes that Jan requested the patch ended up looking
like:

From 4fd1cf85b10565b7b7e3f73d19fa30de808ff60f Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Mon, 31 Aug 2015 11:00:29 -0400
Subject: [PATCH v3.1 08/10] tmem: Use 'struct xen_tmem_oid' for every user.

Patch "tmem: Make the uint64_t oid[3] a proper structure:
xen_tmem_oid" converted the sysctl API to use an
proper structure. But it did not do it for the tmem hypercall.

This expands that and converts the tmem hypercall. For this
to work we define the struct in tmem.h and include it in
sysctl.h.

This change also included work to make the compat layer
happy. That was to declare the struct xen_tmem_oid to be
checked in xlat.lst - which will construct an typedef
in the compat file with the same type, hence allowing
copying of 'oid' member without type issues. The kicker
is that the compat layer adds the prefix 'xen' and since
our structure already has it - we must not include it.

The layout (and size) of this structure in memory for the
'struct tmem_op' (so guest facing) is the same! Verified
via pahole and with 32/64 bit guests.

--- /tmp/old    2015-08-27 16:34:00.535638730 -0400
+++ /tmp/new    2015-08-27 16:34:10.447705328 -0400
@@ -8,7 +8,7 @@
                        uint32_t   arg1;                 /*    28     4 */
                } creat;                                 /*          24 */
                struct {
-                       uint64_t   oid[3];               /*     8    24 */
+                       xen_tmem_oid_t oid;              /*     8    24 */
                        uint32_t   index;                /*    32     4 */
                        uint32_t   tmem_offset;          /*    36     4 */
                        uint32_t   pfn_offset;           /*    40     4 */

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 xen/common/compat/tmem_xen.c | 4 +---
 xen/common/tmem.c            | 2 +-
 xen/include/public/sysctl.h  | 7 +------
 xen/include/public/tmem.h    | 9 +++++++++
 xen/include/xlat.lst         | 1 +
 5 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/xen/common/compat/tmem_xen.c b/xen/common/compat/tmem_xen.c
index 97c7ff2..db08005 100644
--- a/xen/common/compat/tmem_xen.c
+++ b/xen/common/compat/tmem_xen.c
@@ -11,9 +11,7 @@
 #include <xen/hypercall.h>
 #include <compat/tmem.h>
 
-#define xen_tmem_op tmem_op
-/*CHECK_tmem_op;*/
-#undef xen_tmem_op
+CHECK_tmem_oid;
 
 /*
  * Local variables:
diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index 5260c6c..c5edab4 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -2719,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
             write_unlock(&tmem_rwlock);
             read_lock(&tmem_rwlock);
 
-            oidp = container_of(&op.u.gen.oid[0], struct xen_tmem_oid, oid[0]);
+            oidp = &op.u.gen.oid;
             switch ( op.cmd )
             {
             case TMEM_NEW_POOL:
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 2d7580b..3bdf0e1 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -34,6 +34,7 @@
 #include "xen.h"
 #include "domctl.h"
 #include "physdev.h"
+#include "tmem.h"
 
 #define XEN_SYSCTL_INTERFACE_VERSION 0x0000000C
 
@@ -737,12 +738,6 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
 #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE       32
 #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE     33
 
-struct xen_tmem_oid {
-    uint64_t oid[3];
-};
-typedef struct xen_tmem_oid xen_tmem_oid_t;
-DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_t);
-
 struct xen_sysctl_tmem_op {
     uint32_t cmd;       /* IN: XEN_SYSCTL_TMEM_OP_* . */
     int32_t pool_id;    /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
index e4ee704..913566a 100644
--- a/xen/include/public/tmem.h
+++ b/xen/include/public/tmem.h
@@ -73,6 +73,11 @@
 #define EFROZEN                 1000
 #define EEMPTY                  1001
 
+struct xen_tmem_oid {
+    uint64_t oid[3];
+};
+typedef struct xen_tmem_oid xen_tmem_oid_t;
+DEFINE_XEN_GUEST_HANDLE(xen_tmem_oid_t);
 
 #ifndef __ASSEMBLY__
 #if __XEN_INTERFACE_VERSION__ < 0x00040400
@@ -89,7 +94,11 @@ struct tmem_op {
             uint32_t arg1;
         } creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW */
         struct {
+#if __XEN_INTERFACE_VERSION__ < 0x00040600
             uint64_t oid[3];
+#else
+            xen_tmem_oid_t oid;
+#endif
             uint32_t index;
             uint32_t tmem_offset;
             uint32_t pfn_offset;
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 8cedee7..3795059 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -103,6 +103,7 @@
 !      sched_poll                      sched.h
 ?      sched_remote_shutdown           sched.h
 ?      sched_shutdown                  sched.h
+?      tmem_oid                        tmem.h
 !      tmem_op                         tmem.h
 ?      t_buf                           trace.h
 ?      vcpu_get_physid                 vcpu.h
-- 
2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel