Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h
@ 2020-07-30 18:18 Julien Grall
  2020-07-30 18:18 ` [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics Julien Grall
                   ` (6 more replies)
  0 siblings, 7 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, julien, Jun Nakajima, Wei Liu,
	Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
	George Dunlap, Jan Beulich, Volodymyr Babchuk,
	Roger Pau Monné

From: Julien Grall <jgrall@amazon.com>

Hi all,

A lot of the helpers implemented in asm-*/guest_access.h are implemented
the same way. This series aims to avoid the duplication and implement
them only once in xen/guest_access.h.

Cheers,

Julien Grall (7):
  xen/guest_access: Add emacs magics
  xen/arm: kernel: Re-order the includes
  xen/arm: decode: Re-order the includes
  xen/arm: guestcopy: Re-order the includes
  xen: include xen/guest_access.h rather than asm/guest_access.h
  xen/guest_access: Consolidate guest access helpers in
    xen/guest_access.h
  xen/guest_access: Fix coding style in xen/guest_access.h

 xen/arch/arm/decode.c                |   7 +-
 xen/arch/arm/domain.c                |   2 +-
 xen/arch/arm/guest_walk.c            |   3 +-
 xen/arch/arm/guestcopy.c             |   5 +-
 xen/arch/arm/kernel.c                |  12 +--
 xen/arch/arm/vgic-v3-its.c           |   2 +-
 xen/arch/x86/hvm/svm/svm.c           |   2 +-
 xen/arch/x86/hvm/viridian/viridian.c |   2 +-
 xen/arch/x86/hvm/vmx/vmx.c           |   2 +-
 xen/common/libelf/libelf-loader.c    |   2 +-
 xen/include/asm-arm/guest_access.h   | 115 -----------------------
 xen/include/asm-x86/guest_access.h   | 116 ++----------------------
 xen/include/xen/guest_access.h       | 131 +++++++++++++++++++++++++++
 xen/lib/x86/private.h                |   2 +-
 14 files changed, 161 insertions(+), 242 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-31 13:04   ` Bertrand Marquis
  2020-07-30 18:18 ` [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes Julien Grall
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, Roger Pau Monné

From: Julien Grall <jgrall@amazon.com>

Add emacs magics for xen/guest_access.h and
asm-x86/guest_access.h.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

---
    Changes in v2:
        - Remove the word "missing"
---
 xen/include/asm-x86/guest_access.h | 8 ++++++++
 xen/include/xen/guest_access.h     | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
index 2be3577bd340..3ffde205f6a1 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -160,3 +160,11 @@
 })
 
 #endif /* __ASM_X86_GUEST_ACCESS_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
index 09989df819ce..ef9aaa3efcfe 100644
--- a/xen/include/xen/guest_access.h
+++ b/xen/include/xen/guest_access.h
@@ -33,3 +33,11 @@ char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf,
                                   size_t size, size_t max_size);
 
 #endif /* __XEN_GUEST_ACCESS_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
  2020-07-30 18:18 ` [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:36   ` Stefano Stabellini
  2020-07-31 12:58   ` Bertrand Marquis
  2020-07-30 18:18 ` [RESEND][PATCH v2 3/7] xen/arm: decode: " Julien Grall
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien

From: Julien Grall <jgrall@amazon.com>

We usually have xen/ includes first and then asm/. They are also ordered
alphabetically among themselves.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/kernel.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 8eff0748367d..f95fa392af44 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -3,20 +3,20 @@
  *
  * Copyright (C) 2011 Citrix Systems, Inc.
  */
+#include <xen/domain_page.h>
 #include <xen/errno.h>
+#include <xen/gunzip.h>
 #include <xen/init.h>
 #include <xen/lib.h>
+#include <xen/libfdt/libfdt.h>
 #include <xen/mm.h>
-#include <xen/domain_page.h>
 #include <xen/sched.h>
-#include <asm/byteorder.h>
-#include <asm/setup.h>
-#include <xen/libfdt/libfdt.h>
-#include <xen/gunzip.h>
 #include <xen/vmap.h>
 
+#include <asm/byteorder.h>
 #include <asm/guest_access.h>
 #include <asm/kernel.h>
+#include <asm/setup.h>
 
 #define UIMAGE_MAGIC          0x27051956
 #define UIMAGE_NMLEN          32
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 3/7] xen/arm: decode: Re-order the includes
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
  2020-07-30 18:18 ` [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics Julien Grall
  2020-07-30 18:18 ` [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
  2020-07-30 18:18 ` [RESEND][PATCH v2 4/7] xen/arm: guestcopy: " Julien Grall
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien

From: Julien Grall <jgrall@amazon.com>

We usually have xen/ includes first and then asm/. They are also ordered
alphabetically among themselves.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/decode.c | 5 +++--
 xen/arch/arm/kernel.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 8b1e15d11892..144793c8cea0 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -17,11 +17,12 @@
  * GNU General Public License for more details.
  */
 
-#include <xen/types.h>
+#include <xen/lib.h>
 #include <xen/sched.h>
+#include <xen/types.h>
+
 #include <asm/current.h>
 #include <asm/guest_access.h>
-#include <xen/lib.h>
 
 #include "decode.h"
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index f95fa392af44..032923853f2c 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -5,6 +5,7 @@
  */
 #include <xen/domain_page.h>
 #include <xen/errno.h>
+#include <xen/guest_access.h>
 #include <xen/gunzip.h>
 #include <xen/init.h>
 #include <xen/lib.h>
@@ -14,7 +15,6 @@
 #include <xen/vmap.h>
 
 #include <asm/byteorder.h>
-#include <asm/guest_access.h>
 #include <asm/kernel.h>
 #include <asm/setup.h>
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 4/7] xen/arm: guestcopy: Re-order the includes
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
                   ` (2 preceding siblings ...)
  2020-07-30 18:18 ` [RESEND][PATCH v2 3/7] xen/arm: decode: " Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 12:53   ` Bertrand Marquis
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel; +Cc: Volodymyr Babchuk, Julien Grall, Stefano Stabellini, julien

From: Julien Grall <jgrall@amazon.com>

We usually have xen/ includes first and then asm/. They are also ordered
alphabetically among themselves.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/guestcopy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 7a0f3e9d5fc6..c8023e2bca5d 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -1,7 +1,8 @@
-#include <xen/lib.h>
 #include <xen/domain_page.h>
+#include <xen/lib.h>
 #include <xen/mm.h>
 #include <xen/sched.h>
+
 #include <asm/current.h>
 #include <asm/guest_access.h>
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
                   ` (3 preceding siblings ...)
  2020-07-30 18:18 ` [RESEND][PATCH v2 4/7] xen/arm: guestcopy: " Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
                     ` (3 more replies)
  2020-07-30 18:18 ` [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall
  2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
  6 siblings, 4 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Kevin Tian, Stefano Stabellini, julien, Jun Nakajima, Wei Liu,
	Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
	George Dunlap, Jan Beulich, Volodymyr Babchuk,
	Roger Pau Monné

From: Julien Grall <jgrall@amazon.com>

Only a few places are actually including asm/guest_access.h. While this
is fine today, a follow-up patch will want to move most of the helpers
from asm/guest_access.h to xen/guest_access.h.

To prepare the move, everyone should include xen/guest_access.h rather
than asm/guest_access.h.

Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The
inclusion is now removed as no-one but the latter should include the
former.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v2:
        - Remove some changes that weren't meant to be here.
---
 xen/arch/arm/decode.c                | 2 +-
 xen/arch/arm/domain.c                | 2 +-
 xen/arch/arm/guest_walk.c            | 3 ++-
 xen/arch/arm/guestcopy.c             | 2 +-
 xen/arch/arm/vgic-v3-its.c           | 2 +-
 xen/arch/x86/hvm/svm/svm.c           | 2 +-
 xen/arch/x86/hvm/viridian/viridian.c | 2 +-
 xen/arch/x86/hvm/vmx/vmx.c           | 2 +-
 xen/common/libelf/libelf-loader.c    | 2 +-
 xen/include/asm-arm/guest_access.h   | 1 -
 xen/lib/x86/private.h                | 2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 144793c8cea0..792c2e92a7eb 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -17,12 +17,12 @@
  * GNU General Public License for more details.
  */
 
+#include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
 #include <xen/types.h>
 
 #include <asm/current.h>
-#include <asm/guest_access.h>
 
 #include "decode.h"
 
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 31169326b2e3..9258f6d3faa2 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -12,6 +12,7 @@
 #include <xen/bitops.h>
 #include <xen/errno.h>
 #include <xen/grant_table.h>
+#include <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <xen/init.h>
 #include <xen/lib.h>
@@ -26,7 +27,6 @@
 #include <asm/current.h>
 #include <asm/event.h>
 #include <asm/gic.h>
-#include <asm/guest_access.h>
 #include <asm/guest_atomics.h>
 #include <asm/irq.h>
 #include <asm/p2m.h>
diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index a1cdd7f4afea..b4496c4c86c6 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -16,8 +16,9 @@
  */
 
 #include <xen/domain_page.h>
+#include <xen/guest_access.h>
 #include <xen/sched.h>
-#include <asm/guest_access.h>
+
 #include <asm/guest_walk.h>
 #include <asm/short-desc.h>
 
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index c8023e2bca5d..32681606d8fc 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -1,10 +1,10 @@
 #include <xen/domain_page.h>
+#include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/mm.h>
 #include <xen/sched.h>
 
 #include <asm/current.h>
-#include <asm/guest_access.h>
 
 #define COPY_flush_dcache   (1U << 0)
 #define COPY_from_guest     (0U << 1)
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 6e153c698d56..58d939b85f92 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -32,6 +32,7 @@
 #include <xen/bitops.h>
 #include <xen/config.h>
 #include <xen/domain_page.h>
+#include <xen/guest_access.h>
 #include <xen/lib.h>
 #include <xen/init.h>
 #include <xen/softirq.h>
@@ -39,7 +40,6 @@
 #include <xen/sched.h>
 #include <xen/sizes.h>
 #include <asm/current.h>
-#include <asm/guest_access.h>
 #include <asm/mmio.h>
 #include <asm/gic_v3_defs.h>
 #include <asm/gic_v3_its.h>
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index ca3bbfcbb355..7301f3cd6004 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -16,6 +16,7 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <xen/guest_access.h>
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/trace.h>
@@ -34,7 +35,6 @@
 #include <asm/cpufeature.h>
 #include <asm/processor.h>
 #include <asm/amd.h>
-#include <asm/guest_access.h>
 #include <asm/debugreg.h>
 #include <asm/msr.h>
 #include <asm/i387.h>
diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 977c1bc54fad..dc7183a54627 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -5,12 +5,12 @@
  * Hypervisor Top Level Functional Specification for more information.
  */
 
+#include <xen/guest_access.h>
 #include <xen/sched.h>
 #include <xen/version.h>
 #include <xen/hypercall.h>
 #include <xen/domain_page.h>
 #include <xen/param.h>
-#include <asm/guest_access.h>
 #include <asm/guest/hyperv-tlfs.h>
 #include <asm/paging.h>
 #include <asm/p2m.h>
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index eb54aadfbafb..cb5df1e81c9c 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -15,6 +15,7 @@
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <xen/guest_access.h>
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/param.h>
@@ -31,7 +32,6 @@
 #include <asm/regs.h>
 #include <asm/cpufeature.h>
 #include <asm/processor.h>
-#include <asm/guest_access.h>
 #include <asm/debugreg.h>
 #include <asm/msr.h>
 #include <asm/p2m.h>
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index 0f468727d04a..629cc0d3e611 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -16,7 +16,7 @@
  */
 
 #ifdef __XEN__
-#include <asm/guest_access.h>
+#include <xen/guest_access.h>
 #endif
 
 #include "libelf-private.h"
diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
index 31b9f03f0015..b9a89c495527 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -1,7 +1,6 @@
 #ifndef __ASM_ARM_GUEST_ACCESS_H__
 #define __ASM_ARM_GUEST_ACCESS_H__
 
-#include <xen/guest_access.h>
 #include <xen/errno.h>
 #include <xen/sched.h>
 
diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
index b793181464f3..2d53bd3ced23 100644
--- a/xen/lib/x86/private.h
+++ b/xen/lib/x86/private.h
@@ -4,12 +4,12 @@
 #ifdef __XEN__
 
 #include <xen/bitops.h>
+#include <xen/guest_access.h>
 #include <xen/kernel.h>
 #include <xen/lib.h>
 #include <xen/nospec.h>
 #include <xen/types.h>
 
-#include <asm/guest_access.h>
 #include <asm/msr-index.h>
 
 #define copy_to_buffer_offset copy_to_guest_offset
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
                   ` (4 preceding siblings ...)
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 11:45   ` Jan Beulich
  2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
  6 siblings, 2 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, Volodymyr Babchuk,
	Roger Pau Monné

From: Julien Grall <jgrall@amazon.com>

Most of the helpers to access guest memory are implemented the same way
on Arm and x86. The only differences are:
    - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE()
      and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It
      is still fine to use the Arm implementation on x86.
    - __clear_guest_offset(): Interestingly the prototype does not match
      between the x86 and Arm. However, the Arm one is bogus. So the x86
      implementation can be used.
    - guest_handle{,_subrange}_okay(): They are validly differing
      because Arm is only supporting auto-translated guest and therefore
      handles are always valid.

In the past, the ia64 and ppc64 port use a different model to access
guest parameter. They have been long gone now.

Given Xen currently only support 2 archictures, it is too soon to have a
directory asm-generic as it is not possible to differentiate it with the
existing directory xen/. If/When there is a 3rd port, we can decide to
create the new directory if that new port decide to use a different way
to access guest parameter.

For now, consolidate it in xen/guest_access.h.

While it would be possible to adjust the coding style at the same, this
is left for a follow-up patch so 'diff' can be used to check the
consolidation was done correctly.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v2:
        - Expand the commit message explaining why asm-generic is not
        created.
---
 xen/include/asm-arm/guest_access.h | 114 ---------------------------
 xen/include/asm-x86/guest_access.h | 108 --------------------------
 xen/include/xen/guest_access.h     | 119 +++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 222 deletions(-)

diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
index b9a89c495527..53766386d3d8 100644
--- a/xen/include/asm-arm/guest_access.h
+++ b/xen/include/asm-arm/guest_access.h
@@ -23,88 +23,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf,
 #define __raw_copy_from_guest raw_copy_from_guest
 #define __raw_clear_guest raw_clear_guest
 
-/* Remainder copied from x86 -- could be common? */
-
-/* Is the guest handle a NULL reference? */
-#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
-
-/* Offset the given guest handle into the array it refers to. */
-#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
-#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
-
-/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
- * to the specified type of XEN_GUEST_HANDLE_PARAM. */
-#define guest_handle_cast(hnd, type) ({         \
-    type *_x = (hnd).p;                         \
-    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
-})
-
-/* Convert a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
-#define guest_handle_to_param(hnd, type) ({                  \
-    typeof((hnd).p) _x = (hnd).p;                            \
-    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
-    /* type checking: make sure that the pointers inside     \
-     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
-     * the same type, then return hnd */                     \
-    (void)(&_x == &_y.p);                                    \
-    _y;                                                      \
-})
-
-#define guest_handle_for_field(hnd, type, fld)          \
-    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
-
-#define guest_handle_from_ptr(ptr, type)        \
-    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
-#define const_guest_handle_from_ptr(ptr, type)  \
-    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
-
-/*
- * Copy an array of objects to guest context via a guest handle,
- * specifying an offset into the guest array.
- */
-#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
-    const typeof(*(ptr)) *_s = (ptr);                   \
-    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
-    /* Check that the handle is not for a const type */ \
-    void *__maybe_unused _t = (hnd).p;                  \
-    (void)((hnd).p == _s);                              \
-    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
-})
-
-/*
- * Clear an array of objects in guest context via a guest handle,
- * specifying an offset into the guest array.
- */
-#define clear_guest_offset(hnd, off, nr) ({    \
-    void *_d = (hnd).p;                        \
-    raw_clear_guest(_d+(off), nr);             \
-})
-
-/*
- * Copy an array of objects from guest context via a guest handle,
- * specifying an offset into the guest array.
- */
-#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
-    const typeof(*(ptr)) *_s = (hnd).p;                 \
-    typeof(*(ptr)) *_d = (ptr);                         \
-    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
-})
-
-/* Copy sub-field of a structure to guest context via a guest handle. */
-#define copy_field_to_guest(hnd, ptr, field) ({         \
-    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
-    void *_d = &(hnd).p->field;                         \
-    (void)(&(hnd).p->field == _s);                      \
-    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
-})
-
-/* Copy sub-field of a structure from guest context via a guest handle. */
-#define copy_field_from_guest(ptr, hnd, field) ({       \
-    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
-    typeof(&(ptr)->field) _d = &(ptr)->field;           \
-    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
-})
-
 /*
  * Pre-validate a guest handle.
  * Allows use of faster __copy_* functions.
@@ -113,38 +31,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf,
 #define guest_handle_okay(hnd, nr) (1)
 #define guest_handle_subrange_okay(hnd, first, last) (1)
 
-#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
-    const typeof(*(ptr)) *_s = (ptr);                   \
-    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
-    /* Check that the handle is not for a const type */ \
-    void *__maybe_unused _t = (hnd).p;                  \
-    (void)((hnd).p == _s);                              \
-    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
-})
-
-#define __clear_guest_offset(hnd, off, ptr, nr) ({      \
-    __raw_clear_guest(_d+(off), nr);  \
-})
-
-#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
-    const typeof(*(ptr)) *_s = (hnd).p;                 \
-    typeof(*(ptr)) *_d = (ptr);                         \
-    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
-})
-
-#define __copy_field_to_guest(hnd, ptr, field) ({       \
-    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
-    void *_d = &(hnd).p->field;                         \
-    (void)(&(hnd).p->field == _s);                      \
-    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
-})
-
-#define __copy_field_from_guest(ptr, hnd, field) ({     \
-    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
-    typeof(&(ptr)->field) _d = &(ptr)->field;           \
-    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
-})
-
 #endif /* __ASM_ARM_GUEST_ACCESS_H__ */
 /*
  * Local variables:
diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
index 3ffde205f6a1..08c9fbbc78e1 100644
--- a/xen/include/asm-x86/guest_access.h
+++ b/xen/include/asm-x86/guest_access.h
@@ -38,81 +38,6 @@
      clear_user_hvm((dst), (len)) :             \
      clear_user((dst), (len)))
 
-/* Is the guest handle a NULL reference? */
-#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
-
-/* Offset the given guest handle into the array it refers to. */
-#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
-#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
-
-/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
- * to the specified type of XEN_GUEST_HANDLE_PARAM. */
-#define guest_handle_cast(hnd, type) ({         \
-    type *_x = (hnd).p;                         \
-    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
-})
-
-/* Convert a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
-#define guest_handle_to_param(hnd, type) ({                  \
-    /* type checking: make sure that the pointers inside     \
-     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
-     * the same type, then return hnd */                     \
-    (void)((typeof(&(hnd).p)) 0 ==                           \
-        (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
-    (hnd);                                                   \
-})
-
-#define guest_handle_for_field(hnd, type, fld)          \
-    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
-
-#define guest_handle_from_ptr(ptr, type)        \
-    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
-#define const_guest_handle_from_ptr(ptr, type)  \
-    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
-
-/*
- * Copy an array of objects to guest context via a guest handle,
- * specifying an offset into the guest array.
- */
-#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
-    const typeof(*(ptr)) *_s = (ptr);                   \
-    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
-    /* Check that the handle is not for a const type */ \
-    void *__maybe_unused _t = (hnd).p;                  \
-    (void)((hnd).p == _s);                              \
-    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
-})
-
-/*
- * Copy an array of objects from guest context via a guest handle,
- * specifying an offset into the guest array.
- */
-#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
-    const typeof(*(ptr)) *_s = (hnd).p;                 \
-    typeof(*(ptr)) *_d = (ptr);                         \
-    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
-})
-
-#define clear_guest_offset(hnd, off, nr) ({    \
-    void *_d = (hnd).p;                        \
-    raw_clear_guest(_d+(off), nr);             \
-})
-
-/* Copy sub-field of a structure to guest context via a guest handle. */
-#define copy_field_to_guest(hnd, ptr, field) ({         \
-    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
-    void *_d = &(hnd).p->field;                         \
-    (void)(&(hnd).p->field == _s);                      \
-    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
-})
-
-/* Copy sub-field of a structure from guest context via a guest handle. */
-#define copy_field_from_guest(ptr, hnd, field) ({       \
-    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
-    typeof(&(ptr)->field) _d = &(ptr)->field;           \
-    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
-})
-
 /*
  * Pre-validate a guest handle.
  * Allows use of faster __copy_* functions.
@@ -126,39 +51,6 @@
                      (last)-(first)+1,                  \
                      sizeof(*(hnd).p)))
 
-#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
-    const typeof(*(ptr)) *_s = (ptr);                   \
-    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
-    /* Check that the handle is not for a const type */ \
-    void *__maybe_unused _t = (hnd).p;                  \
-    (void)((hnd).p == _s);                              \
-    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
-})
-
-#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
-    const typeof(*(ptr)) *_s = (hnd).p;                 \
-    typeof(*(ptr)) *_d = (ptr);                         \
-    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
-})
-
-#define __clear_guest_offset(hnd, off, nr) ({    \
-    void *_d = (hnd).p;                          \
-    __raw_clear_guest(_d+(off), nr);             \
-})
-
-#define __copy_field_to_guest(hnd, ptr, field) ({       \
-    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
-    void *_d = &(hnd).p->field;                         \
-    (void)(&(hnd).p->field == _s);                      \
-    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
-})
-
-#define __copy_field_from_guest(ptr, hnd, field) ({     \
-    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
-    typeof(&(ptr)->field) _d = &(ptr)->field;           \
-    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
-})
-
 #endif /* __ASM_X86_GUEST_ACCESS_H__ */
 /*
  * Local variables:
diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
index ef9aaa3efcfe..4957b8d1f2b8 100644
--- a/xen/include/xen/guest_access.h
+++ b/xen/include/xen/guest_access.h
@@ -11,6 +11,86 @@
 #include <xen/types.h>
 #include <public/xen.h>
 
+/* Is the guest handle a NULL reference? */
+#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
+
+/* Offset the given guest handle into the array it refers to. */
+#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
+#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
+
+/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
+ * to the specified type of XEN_GUEST_HANDLE_PARAM. */
+#define guest_handle_cast(hnd, type) ({         \
+    type *_x = (hnd).p;                         \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+})
+
+/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
+#define guest_handle_to_param(hnd, type) ({                  \
+    typeof((hnd).p) _x = (hnd).p;                            \
+    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
+    /* type checking: make sure that the pointers inside     \
+     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
+     * the same type, then return hnd */                     \
+    (void)(&_x == &_y.p);                                    \
+    _y;                                                      \
+})
+
+#define guest_handle_for_field(hnd, type, fld)          \
+    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
+
+#define guest_handle_from_ptr(ptr, type)        \
+    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
+#define const_guest_handle_from_ptr(ptr, type)  \
+    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
+
+/*
+ * Copy an array of objects to guest context via a guest handle,
+ * specifying an offset into the guest array.
+ */
+#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
+    const typeof(*(ptr)) *_s = (ptr);                   \
+    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
+    /* Check that the handle is not for a const type */ \
+    void *__maybe_unused _t = (hnd).p;                  \
+    (void)((hnd).p == _s);                              \
+    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
+})
+
+/*
+ * Clear an array of objects in guest context via a guest handle,
+ * specifying an offset into the guest array.
+ */
+#define clear_guest_offset(hnd, off, nr) ({    \
+    void *_d = (hnd).p;                        \
+    raw_clear_guest(_d+(off), nr);             \
+})
+
+/*
+ * Copy an array of objects from guest context via a guest handle,
+ * specifying an offset into the guest array.
+ */
+#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
+    const typeof(*(ptr)) *_s = (hnd).p;                 \
+    typeof(*(ptr)) *_d = (ptr);                         \
+    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
+})
+
+/* Copy sub-field of a structure to guest context via a guest handle. */
+#define copy_field_to_guest(hnd, ptr, field) ({         \
+    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
+    void *_d = &(hnd).p->field;                         \
+    (void)(&(hnd).p->field == _s);                      \
+    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
+})
+
+/* Copy sub-field of a structure from guest context via a guest handle. */
+#define copy_field_from_guest(ptr, hnd, field) ({       \
+    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
+    typeof(&(ptr)->field) _d = &(ptr)->field;           \
+    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
+})
+
 #define copy_to_guest(hnd, ptr, nr)                     \
     copy_to_guest_offset(hnd, 0, ptr, nr)
 
@@ -20,6 +100,45 @@
 #define clear_guest(hnd, nr)                            \
     clear_guest_offset(hnd, 0, nr)
 
+/*
+ * The __copy_* functions should only be used after the guest handle has
+ * been pre-validated via guest_handle_okay() and
+ * guest_handle_subrange_okay().
+ */
+
+#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
+    const typeof(*(ptr)) *_s = (ptr);                   \
+    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
+    /* Check that the handle is not for a const type */ \
+    void *__maybe_unused _t = (hnd).p;                  \
+    (void)((hnd).p == _s);                              \
+    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
+})
+
+#define __clear_guest_offset(hnd, off, nr) ({    \
+    void *_d = (hnd).p;                          \
+    __raw_clear_guest(_d + (off), nr);           \
+})
+
+#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
+    const typeof(*(ptr)) *_s = (hnd).p;                 \
+    typeof(*(ptr)) *_d = (ptr);                         \
+    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
+})
+
+#define __copy_field_to_guest(hnd, ptr, field) ({       \
+    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
+    void *_d = &(hnd).p->field;                         \
+    (void)(&(hnd).p->field == _s);                      \
+    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
+})
+
+#define __copy_field_from_guest(ptr, hnd, field) ({     \
+    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
+    typeof(&(ptr)->field) _d = &(ptr)->field;           \
+    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
+})
+
 #define __copy_to_guest(hnd, ptr, nr)                   \
     __copy_to_guest_offset(hnd, 0, ptr, nr)
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style in xen/guest_access.h
  2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
                   ` (5 preceding siblings ...)
  2020-07-30 18:18 ` [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall
@ 2020-07-30 18:18 ` Julien Grall
  2020-07-30 19:38   ` Stefano Stabellini
                     ` (2 more replies)
  6 siblings, 3 replies; 24+ messages in thread
From: Julien Grall @ 2020-07-30 18:18 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, julien, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich

From: Julien Grall <jgrall@amazon.com>

    * Add space before and after operator
    * Align \
    * Format comments

No functional changes expected.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/include/xen/guest_access.h | 36 +++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
index 4957b8d1f2b8..52fc7a063249 100644
--- a/xen/include/xen/guest_access.h
+++ b/xen/include/xen/guest_access.h
@@ -18,20 +18,24 @@
 #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
 #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
 
-/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
- * to the specified type of XEN_GUEST_HANDLE_PARAM. */
+/*
+ * Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
+ * to the specified type of XEN_GUEST_HANDLE_PARAM.
+ */
 #define guest_handle_cast(hnd, type) ({         \
     type *_x = (hnd).p;                         \
-    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
+    (XEN_GUEST_HANDLE_PARAM(type)) { _x };      \
 })
 
 /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
 #define guest_handle_to_param(hnd, type) ({                  \
     typeof((hnd).p) _x = (hnd).p;                            \
     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
-    /* type checking: make sure that the pointers inside     \
+    /*                                                       \
+     * type checking: make sure that the pointers inside     \
      * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
-     * the same type, then return hnd */                     \
+     * the same type, then return hnd.                       \
+     */                                                      \
     (void)(&_x == &_y.p);                                    \
     _y;                                                      \
 })
@@ -106,13 +110,13 @@
  * guest_handle_subrange_okay().
  */
 
-#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
-    const typeof(*(ptr)) *_s = (ptr);                   \
-    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
-    /* Check that the handle is not for a const type */ \
-    void *__maybe_unused _t = (hnd).p;                  \
-    (void)((hnd).p == _s);                              \
-    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
+#define __copy_to_guest_offset(hnd, off, ptr, nr) ({        \
+    const typeof(*(ptr)) *_s = (ptr);                       \
+    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;              \
+    /* Check that the handle is not for a const type */     \
+    void *__maybe_unused _t = (hnd).p;                      \
+    (void)((hnd).p == _s);                                  \
+    __raw_copy_to_guest(_d + (off), _s, sizeof(*_s) * (nr));\
 })
 
 #define __clear_guest_offset(hnd, off, nr) ({    \
@@ -120,10 +124,10 @@
     __raw_clear_guest(_d + (off), nr);           \
 })
 
-#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
-    const typeof(*(ptr)) *_s = (hnd).p;                 \
-    typeof(*(ptr)) *_d = (ptr);                         \
-    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
+#define __copy_from_guest_offset(ptr, hnd, off, nr) ({          \
+    const typeof(*(ptr)) *_s = (hnd).p;                         \
+    typeof(*(ptr)) *_d = (ptr);                                 \
+    __raw_copy_from_guest(_d, _s + (off), sizeof (*_d) * (nr)); \
 })
 
 #define __copy_field_to_guest(hnd, ptr, field) ({       \
-- 
2.17.1



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes
  2020-07-30 18:18 ` [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes Julien Grall
@ 2020-07-30 19:36   ` Stefano Stabellini
  2020-07-31 12:58   ` Bertrand Marquis
  1 sibling, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:36 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> We usually have xen/ includes first and then asm/. They are also ordered
> alphabetically among themselves.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/kernel.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 8eff0748367d..f95fa392af44 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -3,20 +3,20 @@
>   *
>   * Copyright (C) 2011 Citrix Systems, Inc.
>   */
> +#include <xen/domain_page.h>
>  #include <xen/errno.h>
> +#include <xen/gunzip.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
> +#include <xen/libfdt/libfdt.h>
>  #include <xen/mm.h>
> -#include <xen/domain_page.h>
>  #include <xen/sched.h>
> -#include <asm/byteorder.h>
> -#include <asm/setup.h>
> -#include <xen/libfdt/libfdt.h>
> -#include <xen/gunzip.h>
>  #include <xen/vmap.h>
>  
> +#include <asm/byteorder.h>
>  #include <asm/guest_access.h>
>  #include <asm/kernel.h>
> +#include <asm/setup.h>
>  
>  #define UIMAGE_MAGIC          0x27051956
>  #define UIMAGE_NMLEN          32
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 3/7] xen/arm: decode: Re-order the includes
  2020-07-30 18:18 ` [RESEND][PATCH v2 3/7] xen/arm: decode: " Julien Grall
@ 2020-07-30 19:37   ` Stefano Stabellini
  0 siblings, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> We usually have xen/ includes first and then asm/. They are also ordered
> alphabetically among themselves.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Might wanna mention the change from asm/guest_access.h to
xen/guest_access.h. Anyway:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/decode.c | 5 +++--
>  xen/arch/arm/kernel.c | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index 8b1e15d11892..144793c8cea0 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -17,11 +17,12 @@
>   * GNU General Public License for more details.
>   */
>  
> -#include <xen/types.h>
> +#include <xen/lib.h>
>  #include <xen/sched.h>
> +#include <xen/types.h>
> +
>  #include <asm/current.h>
>  #include <asm/guest_access.h>
> -#include <xen/lib.h>
>  
>  #include "decode.h"
>  
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index f95fa392af44..032923853f2c 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -5,6 +5,7 @@
>   */
>  #include <xen/domain_page.h>
>  #include <xen/errno.h>
> +#include <xen/guest_access.h>
>  #include <xen/gunzip.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
> @@ -14,7 +15,6 @@
>  #include <xen/vmap.h>
>  
>  #include <asm/byteorder.h>
> -#include <asm/guest_access.h>
>  #include <asm/kernel.h>
>  #include <asm/setup.h>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 4/7] xen/arm: guestcopy: Re-order the includes
  2020-07-30 18:18 ` [RESEND][PATCH v2 4/7] xen/arm: guestcopy: " Julien Grall
@ 2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 12:53   ` Bertrand Marquis
  1 sibling, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> We usually have xen/ includes first and then asm/. They are also ordered
> alphabetically among themselves.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  xen/arch/arm/guestcopy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index 7a0f3e9d5fc6..c8023e2bca5d 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -1,7 +1,8 @@
> -#include <xen/lib.h>
>  #include <xen/domain_page.h>
> +#include <xen/lib.h>
>  #include <xen/mm.h>
>  #include <xen/sched.h>
> +
>  #include <asm/current.h>
>  #include <asm/guest_access.h>
>  
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
@ 2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 11:36   ` Jan Beulich
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: Kevin Tian, Stefano Stabellini, Jun Nakajima, Wei Liu,
	Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
	George Dunlap, Jan Beulich, xen-devel, Volodymyr Babchuk,
	Roger Pau Monné

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Only a few places are actually including asm/guest_access.h. While this
> is fine today, a follow-up patch will want to move most of the helpers
> from asm/guest_access.h to xen/guest_access.h.
> 
> To prepare the move, everyone should include xen/guest_access.h rather
> than asm/guest_access.h.
> 
> Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The
> inclusion is now removed as no-one but the latter should include the
> former.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Remove some changes that weren't meant to be here.
> ---
>  xen/arch/arm/decode.c                | 2 +-
>  xen/arch/arm/domain.c                | 2 +-
>  xen/arch/arm/guest_walk.c            | 3 ++-
>  xen/arch/arm/guestcopy.c             | 2 +-
>  xen/arch/arm/vgic-v3-its.c           | 2 +-
>  xen/arch/x86/hvm/svm/svm.c           | 2 +-
>  xen/arch/x86/hvm/viridian/viridian.c | 2 +-
>  xen/arch/x86/hvm/vmx/vmx.c           | 2 +-
>  xen/common/libelf/libelf-loader.c    | 2 +-
>  xen/include/asm-arm/guest_access.h   | 1 -
>  xen/lib/x86/private.h                | 2 +-
>  11 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index 144793c8cea0..792c2e92a7eb 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -17,12 +17,12 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <xen/guest_access.h>
>  #include <xen/lib.h>
>  #include <xen/sched.h>
>  #include <xen/types.h>
>  
>  #include <asm/current.h>
> -#include <asm/guest_access.h>
>  
>  #include "decode.h"
>  
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 31169326b2e3..9258f6d3faa2 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -12,6 +12,7 @@
>  #include <xen/bitops.h>
>  #include <xen/errno.h>
>  #include <xen/grant_table.h>
> +#include <xen/guest_access.h>
>  #include <xen/hypercall.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
> @@ -26,7 +27,6 @@
>  #include <asm/current.h>
>  #include <asm/event.h>
>  #include <asm/gic.h>
> -#include <asm/guest_access.h>
>  #include <asm/guest_atomics.h>
>  #include <asm/irq.h>
>  #include <asm/p2m.h>
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index a1cdd7f4afea..b4496c4c86c6 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -16,8 +16,9 @@
>   */
>  
>  #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
>  #include <xen/sched.h>
> -#include <asm/guest_access.h>
> +
>  #include <asm/guest_walk.h>
>  #include <asm/short-desc.h>
>  
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index c8023e2bca5d..32681606d8fc 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -1,10 +1,10 @@
>  #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
>  #include <xen/lib.h>
>  #include <xen/mm.h>
>  #include <xen/sched.h>
>  
>  #include <asm/current.h>
> -#include <asm/guest_access.h>
>  
>  #define COPY_flush_dcache   (1U << 0)
>  #define COPY_from_guest     (0U << 1)
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 6e153c698d56..58d939b85f92 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -32,6 +32,7 @@
>  #include <xen/bitops.h>
>  #include <xen/config.h>
>  #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
>  #include <xen/lib.h>
>  #include <xen/init.h>
>  #include <xen/softirq.h>
> @@ -39,7 +40,6 @@
>  #include <xen/sched.h>
>  #include <xen/sizes.h>
>  #include <asm/current.h>
> -#include <asm/guest_access.h>
>  #include <asm/mmio.h>
>  #include <asm/gic_v3_defs.h>
>  #include <asm/gic_v3_its.h>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index ca3bbfcbb355..7301f3cd6004 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -16,6 +16,7 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <xen/guest_access.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  #include <xen/trace.h>
> @@ -34,7 +35,6 @@
>  #include <asm/cpufeature.h>
>  #include <asm/processor.h>
>  #include <asm/amd.h>
> -#include <asm/guest_access.h>
>  #include <asm/debugreg.h>
>  #include <asm/msr.h>
>  #include <asm/i387.h>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index 977c1bc54fad..dc7183a54627 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -5,12 +5,12 @@
>   * Hypervisor Top Level Functional Specification for more information.
>   */
>  
> +#include <xen/guest_access.h>
>  #include <xen/sched.h>
>  #include <xen/version.h>
>  #include <xen/hypercall.h>
>  #include <xen/domain_page.h>
>  #include <xen/param.h>
> -#include <asm/guest_access.h>
>  #include <asm/guest/hyperv-tlfs.h>
>  #include <asm/paging.h>
>  #include <asm/p2m.h>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index eb54aadfbafb..cb5df1e81c9c 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -15,6 +15,7 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <xen/guest_access.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  #include <xen/param.h>
> @@ -31,7 +32,6 @@
>  #include <asm/regs.h>
>  #include <asm/cpufeature.h>
>  #include <asm/processor.h>
> -#include <asm/guest_access.h>
>  #include <asm/debugreg.h>
>  #include <asm/msr.h>
>  #include <asm/p2m.h>
> diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
> index 0f468727d04a..629cc0d3e611 100644
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -16,7 +16,7 @@
>   */
>  
>  #ifdef __XEN__
> -#include <asm/guest_access.h>
> +#include <xen/guest_access.h>
>  #endif
>  
>  #include "libelf-private.h"
> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
> index 31b9f03f0015..b9a89c495527 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -1,7 +1,6 @@
>  #ifndef __ASM_ARM_GUEST_ACCESS_H__
>  #define __ASM_ARM_GUEST_ACCESS_H__
>  
> -#include <xen/guest_access.h>
>  #include <xen/errno.h>
>  #include <xen/sched.h>
>  
> diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
> index b793181464f3..2d53bd3ced23 100644
> --- a/xen/lib/x86/private.h
> +++ b/xen/lib/x86/private.h
> @@ -4,12 +4,12 @@
>  #ifdef __XEN__
>  
>  #include <xen/bitops.h>
> +#include <xen/guest_access.h>
>  #include <xen/kernel.h>
>  #include <xen/lib.h>
>  #include <xen/nospec.h>
>  #include <xen/types.h>
>  
> -#include <asm/guest_access.h>
>  #include <asm/msr-index.h>
>  
>  #define copy_to_buffer_offset copy_to_guest_offset
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall
@ 2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 11:45   ` Jan Beulich
  1 sibling, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, xen-devel,
	Volodymyr Babchuk, Roger Pau Monné

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Most of the helpers to access guest memory are implemented the same way
> on Arm and x86. The only differences are:
>     - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE()

It is actually just guest_handle_to_param() ?


>       and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It
>       is still fine to use the Arm implementation on x86.
>     - __clear_guest_offset(): Interestingly the prototype does not match
>       between the x86 and Arm. However, the Arm one is bogus. So the x86
>       implementation can be used.
>     - guest_handle{,_subrange}_okay(): They are validly differing
>       because Arm is only supporting auto-translated guest and therefore
>       handles are always valid.
> 
> In the past, the ia64 and ppc64 port use a different model to access
> guest parameter. They have been long gone now.
> 
> Given Xen currently only support 2 archictures, it is too soon to have a
> directory asm-generic as it is not possible to differentiate it with the
> existing directory xen/. If/When there is a 3rd port, we can decide to
> create the new directory if that new port decide to use a different way
> to access guest parameter.
> 
> For now, consolidate it in xen/guest_access.h.
> 
> While it would be possible to adjust the coding style at the same, this
> is left for a follow-up patch so 'diff' can be used to check the
> consolidation was done correctly.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Looks good to me

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Expand the commit message explaining why asm-generic is not
>         created.
> ---
>  xen/include/asm-arm/guest_access.h | 114 ---------------------------
>  xen/include/asm-x86/guest_access.h | 108 --------------------------
>  xen/include/xen/guest_access.h     | 119 +++++++++++++++++++++++++++++
>  3 files changed, 119 insertions(+), 222 deletions(-)
> 
> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
> index b9a89c495527..53766386d3d8 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -23,88 +23,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf,
>  #define __raw_copy_from_guest raw_copy_from_guest
>  #define __raw_clear_guest raw_clear_guest
>  
> -/* Remainder copied from x86 -- could be common? */
> -
> -/* Is the guest handle a NULL reference? */
> -#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
> -
> -/* Offset the given guest handle into the array it refers to. */
> -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
> -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
> -
> -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> - * to the specified type of XEN_GUEST_HANDLE_PARAM. */
> -#define guest_handle_cast(hnd, type) ({         \
> -    type *_x = (hnd).p;                         \
> -    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
> -})
> -
> -/* Convert a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> -#define guest_handle_to_param(hnd, type) ({                  \
> -    typeof((hnd).p) _x = (hnd).p;                            \
> -    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
> -    /* type checking: make sure that the pointers inside     \
> -     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> -     * the same type, then return hnd */                     \
> -    (void)(&_x == &_y.p);                                    \
> -    _y;                                                      \
> -})
> -
> -#define guest_handle_for_field(hnd, type, fld)          \
> -    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
> -
> -#define guest_handle_from_ptr(ptr, type)        \
> -    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
> -#define const_guest_handle_from_ptr(ptr, type)  \
> -    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
> -
> -/*
> - * Copy an array of objects to guest context via a guest handle,
> - * specifying an offset into the guest array.
> - */
> -#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
> -})
> -
> -/*
> - * Clear an array of objects in guest context via a guest handle,
> - * specifying an offset into the guest array.
> - */
> -#define clear_guest_offset(hnd, off, nr) ({    \
> -    void *_d = (hnd).p;                        \
> -    raw_clear_guest(_d+(off), nr);             \
> -})
> -
> -/*
> - * Copy an array of objects from guest context via a guest handle,
> - * specifying an offset into the guest array.
> - */
> -#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> -})
> -
> -/* Copy sub-field of a structure to guest context via a guest handle. */
> -#define copy_field_to_guest(hnd, ptr, field) ({         \
> -    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> -    void *_d = &(hnd).p->field;                         \
> -    (void)(&(hnd).p->field == _s);                      \
> -    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
> -})
> -
> -/* Copy sub-field of a structure from guest context via a guest handle. */
> -#define copy_field_from_guest(ptr, hnd, field) ({       \
> -    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> -    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> -    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
> -})
> -
>  /*
>   * Pre-validate a guest handle.
>   * Allows use of faster __copy_* functions.
> @@ -113,38 +31,6 @@ int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void *buf,
>  #define guest_handle_okay(hnd, nr) (1)
>  #define guest_handle_subrange_okay(hnd, first, last) (1)
>  
> -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
> -})
> -
> -#define __clear_guest_offset(hnd, off, ptr, nr) ({      \
> -    __raw_clear_guest(_d+(off), nr);  \
> -})
> -
> -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> -})
> -
> -#define __copy_field_to_guest(hnd, ptr, field) ({       \
> -    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> -    void *_d = &(hnd).p->field;                         \
> -    (void)(&(hnd).p->field == _s);                      \
> -    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
> -})
> -
> -#define __copy_field_from_guest(ptr, hnd, field) ({     \
> -    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> -    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> -    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
> -})
> -
>  #endif /* __ASM_ARM_GUEST_ACCESS_H__ */
>  /*
>   * Local variables:
> diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
> index 3ffde205f6a1..08c9fbbc78e1 100644
> --- a/xen/include/asm-x86/guest_access.h
> +++ b/xen/include/asm-x86/guest_access.h
> @@ -38,81 +38,6 @@
>       clear_user_hvm((dst), (len)) :             \
>       clear_user((dst), (len)))
>  
> -/* Is the guest handle a NULL reference? */
> -#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
> -
> -/* Offset the given guest handle into the array it refers to. */
> -#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
> -#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
> -
> -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> - * to the specified type of XEN_GUEST_HANDLE_PARAM. */
> -#define guest_handle_cast(hnd, type) ({         \
> -    type *_x = (hnd).p;                         \
> -    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
> -})
> -
> -/* Convert a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> -#define guest_handle_to_param(hnd, type) ({                  \
> -    /* type checking: make sure that the pointers inside     \
> -     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> -     * the same type, then return hnd */                     \
> -    (void)((typeof(&(hnd).p)) 0 ==                           \
> -        (typeof(&((XEN_GUEST_HANDLE_PARAM(type)) {}).p)) 0); \
> -    (hnd);                                                   \
> -})
> -
> -#define guest_handle_for_field(hnd, type, fld)          \
> -    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
> -
> -#define guest_handle_from_ptr(ptr, type)        \
> -    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
> -#define const_guest_handle_from_ptr(ptr, type)  \
> -    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
> -
> -/*
> - * Copy an array of objects to guest context via a guest handle,
> - * specifying an offset into the guest array.
> - */
> -#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
> -})
> -
> -/*
> - * Copy an array of objects from guest context via a guest handle,
> - * specifying an offset into the guest array.
> - */
> -#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> -})
> -
> -#define clear_guest_offset(hnd, off, nr) ({    \
> -    void *_d = (hnd).p;                        \
> -    raw_clear_guest(_d+(off), nr);             \
> -})
> -
> -/* Copy sub-field of a structure to guest context via a guest handle. */
> -#define copy_field_to_guest(hnd, ptr, field) ({         \
> -    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> -    void *_d = &(hnd).p->field;                         \
> -    (void)(&(hnd).p->field == _s);                      \
> -    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
> -})
> -
> -/* Copy sub-field of a structure from guest context via a guest handle. */
> -#define copy_field_from_guest(ptr, hnd, field) ({       \
> -    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> -    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> -    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
> -})
> -
>  /*
>   * Pre-validate a guest handle.
>   * Allows use of faster __copy_* functions.
> @@ -126,39 +51,6 @@
>                       (last)-(first)+1,                  \
>                       sizeof(*(hnd).p)))
>  
> -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
> -})
> -
> -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> -})
> -
> -#define __clear_guest_offset(hnd, off, nr) ({    \
> -    void *_d = (hnd).p;                          \
> -    __raw_clear_guest(_d+(off), nr);             \
> -})
> -
> -#define __copy_field_to_guest(hnd, ptr, field) ({       \
> -    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> -    void *_d = &(hnd).p->field;                         \
> -    (void)(&(hnd).p->field == _s);                      \
> -    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
> -})
> -
> -#define __copy_field_from_guest(ptr, hnd, field) ({     \
> -    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> -    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> -    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
> -})
> -
>  #endif /* __ASM_X86_GUEST_ACCESS_H__ */
>  /*
>   * Local variables:
> diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
> index ef9aaa3efcfe..4957b8d1f2b8 100644
> --- a/xen/include/xen/guest_access.h
> +++ b/xen/include/xen/guest_access.h
> @@ -11,6 +11,86 @@
>  #include <xen/types.h>
>  #include <public/xen.h>
>  
> +/* Is the guest handle a NULL reference? */
> +#define guest_handle_is_null(hnd)        ((hnd).p == NULL)
> +
> +/* Offset the given guest handle into the array it refers to. */
> +#define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
> +#define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
> +
> +/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> + * to the specified type of XEN_GUEST_HANDLE_PARAM. */
> +#define guest_handle_cast(hnd, type) ({         \
> +    type *_x = (hnd).p;                         \
> +    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
> +})
> +
> +/* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> +#define guest_handle_to_param(hnd, type) ({                  \
> +    typeof((hnd).p) _x = (hnd).p;                            \
> +    XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
> +    /* type checking: make sure that the pointers inside     \
> +     * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> +     * the same type, then return hnd */                     \
> +    (void)(&_x == &_y.p);                                    \
> +    _y;                                                      \
> +})
> +
> +#define guest_handle_for_field(hnd, type, fld)          \
> +    ((XEN_GUEST_HANDLE(type)) { &(hnd).p->fld })
> +
> +#define guest_handle_from_ptr(ptr, type)        \
> +    ((XEN_GUEST_HANDLE_PARAM(type)) { (type *)ptr })
> +#define const_guest_handle_from_ptr(ptr, type)  \
> +    ((XEN_GUEST_HANDLE_PARAM(const_##type)) { (const type *)ptr })
> +
> +/*
> + * Copy an array of objects to guest context via a guest handle,
> + * specifying an offset into the guest array.
> + */
> +#define copy_to_guest_offset(hnd, off, ptr, nr) ({      \
> +    const typeof(*(ptr)) *_s = (ptr);                   \
> +    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> +    /* Check that the handle is not for a const type */ \
> +    void *__maybe_unused _t = (hnd).p;                  \
> +    (void)((hnd).p == _s);                              \
> +    raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));  \
> +})
> +
> +/*
> + * Clear an array of objects in guest context via a guest handle,
> + * specifying an offset into the guest array.
> + */
> +#define clear_guest_offset(hnd, off, nr) ({    \
> +    void *_d = (hnd).p;                        \
> +    raw_clear_guest(_d+(off), nr);             \
> +})
> +
> +/*
> + * Copy an array of objects from guest context via a guest handle,
> + * specifying an offset into the guest array.
> + */
> +#define copy_from_guest_offset(ptr, hnd, off, nr) ({    \
> +    const typeof(*(ptr)) *_s = (hnd).p;                 \
> +    typeof(*(ptr)) *_d = (ptr);                         \
> +    raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> +})
> +
> +/* Copy sub-field of a structure to guest context via a guest handle. */
> +#define copy_field_to_guest(hnd, ptr, field) ({         \
> +    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> +    void *_d = &(hnd).p->field;                         \
> +    (void)(&(hnd).p->field == _s);                      \
> +    raw_copy_to_guest(_d, _s, sizeof(*_s));             \
> +})
> +
> +/* Copy sub-field of a structure from guest context via a guest handle. */
> +#define copy_field_from_guest(ptr, hnd, field) ({       \
> +    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> +    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> +    raw_copy_from_guest(_d, _s, sizeof(*_d));           \
> +})
> +
>  #define copy_to_guest(hnd, ptr, nr)                     \
>      copy_to_guest_offset(hnd, 0, ptr, nr)
>  
> @@ -20,6 +100,45 @@
>  #define clear_guest(hnd, nr)                            \
>      clear_guest_offset(hnd, 0, nr)
>  
> +/*
> + * The __copy_* functions should only be used after the guest handle has
> + * been pre-validated via guest_handle_okay() and
> + * guest_handle_subrange_okay().
> + */
> +
> +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
> +    const typeof(*(ptr)) *_s = (ptr);                   \
> +    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> +    /* Check that the handle is not for a const type */ \
> +    void *__maybe_unused _t = (hnd).p;                  \
> +    (void)((hnd).p == _s);                              \
> +    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
> +})
> +
> +#define __clear_guest_offset(hnd, off, nr) ({    \
> +    void *_d = (hnd).p;                          \
> +    __raw_clear_guest(_d + (off), nr);           \
> +})
> +
> +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
> +    const typeof(*(ptr)) *_s = (hnd).p;                 \
> +    typeof(*(ptr)) *_d = (ptr);                         \
> +    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> +})
> +
> +#define __copy_field_to_guest(hnd, ptr, field) ({       \
> +    const typeof(&(ptr)->field) _s = &(ptr)->field;     \
> +    void *_d = &(hnd).p->field;                         \
> +    (void)(&(hnd).p->field == _s);                      \
> +    __raw_copy_to_guest(_d, _s, sizeof(*_s));           \
> +})
> +
> +#define __copy_field_from_guest(ptr, hnd, field) ({     \
> +    const typeof(&(ptr)->field) _s = &(hnd).p->field;   \
> +    typeof(&(ptr)->field) _d = &(ptr)->field;           \
> +    __raw_copy_from_guest(_d, _s, sizeof(*_d));         \
> +})
> +
>  #define __copy_to_guest(hnd, ptr, nr)                   \
>      __copy_to_guest_offset(hnd, 0, ptr, nr)
>  
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style in xen/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
@ 2020-07-30 19:38   ` Stefano Stabellini
  2020-07-31 11:41   ` Jan Beulich
  2020-07-31 12:46   ` Bertrand Marquis
  2 siblings, 0 replies; 24+ messages in thread
From: Stefano Stabellini @ 2020-07-30 19:38 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, xen-devel

On Thu, 30 Jul 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
>     * Add space before and after operator
>     * Align \
>     * Format comments
> 
> No functional changes expected.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/include/xen/guest_access.h | 36 +++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
> index 4957b8d1f2b8..52fc7a063249 100644
> --- a/xen/include/xen/guest_access.h
> +++ b/xen/include/xen/guest_access.h
> @@ -18,20 +18,24 @@
>  #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
>  #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
>  
> -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> - * to the specified type of XEN_GUEST_HANDLE_PARAM. */
> +/*
> + * Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> + * to the specified type of XEN_GUEST_HANDLE_PARAM.
> + */
>  #define guest_handle_cast(hnd, type) ({         \
>      type *_x = (hnd).p;                         \
> -    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
> +    (XEN_GUEST_HANDLE_PARAM(type)) { _x };      \
>  })
>  
>  /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
>  #define guest_handle_to_param(hnd, type) ({                  \
>      typeof((hnd).p) _x = (hnd).p;                            \
>      XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
> -    /* type checking: make sure that the pointers inside     \
> +    /*                                                       \
> +     * type checking: make sure that the pointers inside     \
>       * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> -     * the same type, then return hnd */                     \
> +     * the same type, then return hnd.                       \
> +     */                                                      \
>      (void)(&_x == &_y.p);                                    \
>      _y;                                                      \
>  })
> @@ -106,13 +110,13 @@
>   * guest_handle_subrange_okay().
>   */
>  
> -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
> +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({        \
> +    const typeof(*(ptr)) *_s = (ptr);                       \
> +    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;              \
> +    /* Check that the handle is not for a const type */     \
> +    void *__maybe_unused _t = (hnd).p;                      \
> +    (void)((hnd).p == _s);                                  \
> +    __raw_copy_to_guest(_d + (off), _s, sizeof(*_s) * (nr));\
>  })
>  
>  #define __clear_guest_offset(hnd, off, nr) ({    \
> @@ -120,10 +124,10 @@
>      __raw_clear_guest(_d + (off), nr);           \
>  })
>  
> -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({          \
> +    const typeof(*(ptr)) *_s = (hnd).p;                         \
> +    typeof(*(ptr)) *_d = (ptr);                                 \
> +    __raw_copy_from_guest(_d, _s + (off), sizeof (*_d) * (nr)); \
>  })
>  
>  #define __copy_field_to_guest(hnd, ptr, field) ({       \
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
@ 2020-07-31 11:36   ` Jan Beulich
  2020-07-31 12:45   ` Bertrand Marquis
  2020-07-31 12:45   ` Bertrand Marquis
  3 siblings, 0 replies; 24+ messages in thread
From: Jan Beulich @ 2020-07-31 11:36 UTC (permalink / raw)
  To: Julien Grall
  Cc: Kevin Tian, Stefano Stabellini, Wei Liu, Paul Durrant,
	Andrew Cooper, Julien Grall, Ian Jackson, George Dunlap,
	Jun Nakajima, xen-devel, Volodymyr Babchuk, Roger Pau Monné

On 30.07.2020 20:18, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Only a few places are actually including asm/guest_access.h. While this
> is fine today, a follow-up patch will want to move most of the helpers
> from asm/guest_access.h to xen/guest_access.h.
> 
> To prepare the move, everyone should include xen/guest_access.h rather
> than asm/guest_access.h.
> 
> Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The
> inclusion is now removed as no-one but the latter should include the
> former.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Is there any chance you could take measures to avoid new inclusions
of asm/guest_access.h to appear?

Jan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style in xen/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
  2020-07-30 19:38   ` Stefano Stabellini
@ 2020-07-31 11:41   ` Jan Beulich
  2020-07-31 12:46   ` Bertrand Marquis
  2 siblings, 0 replies; 24+ messages in thread
From: Jan Beulich @ 2020-07-31 11:41 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, xen-devel

On 30.07.2020 20:18, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
>     * Add space before and after operator
>     * Align \
>     * Format comments

How about also

    * remove/replace leading underscores

?

Jan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
@ 2020-07-31 11:45   ` Jan Beulich
  1 sibling, 0 replies; 24+ messages in thread
From: Jan Beulich @ 2020-07-31 11:45 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, xen-devel, Volodymyr Babchuk,
	Roger Pau Monné

On 30.07.2020 20:18, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Most of the helpers to access guest memory are implemented the same way
> on Arm and x86. The only differences are:
>     - guest_handle_{from, to}_param(): while on x86 XEN_GUEST_HANDLE()
>       and XEN_GUEST_HANDLE_PARAM() are the same, they are not on Arm. It
>       is still fine to use the Arm implementation on x86.

Is the description stale? I don't think there's any guest_handle_from_param()
anymore.

>     - __clear_guest_offset(): Interestingly the prototype does not match
>       between the x86 and Arm. However, the Arm one is bogus. So the x86
>       implementation can be used.
>     - guest_handle{,_subrange}_okay(): They are validly differing
>       because Arm is only supporting auto-translated guest and therefore
>       handles are always valid.
> 
> In the past, the ia64 and ppc64 port use a different model to access
> guest parameter. They have been long gone now.
> 
> Given Xen currently only support 2 archictures, it is too soon to have a
> directory asm-generic as it is not possible to differentiate it with the
> existing directory xen/. If/When there is a 3rd port, we can decide to
> create the new directory if that new port decide to use a different way
> to access guest parameter.
> 
> For now, consolidate it in xen/guest_access.h.
> 
> While it would be possible to adjust the coding style at the same, this
> is left for a follow-up patch so 'diff' can be used to check the
> consolidation was done correctly.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Apart from the above
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
  2020-07-31 11:36   ` Jan Beulich
@ 2020-07-31 12:45   ` Bertrand Marquis
  2020-07-31 12:45   ` Bertrand Marquis
  3 siblings, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:45 UTC (permalink / raw)
  To: Julien Grall
  Cc: Kevin Tian, Stefano Stabellini, Jan Beulich, Wei Liu,
	Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
	George Dunlap, Jun Nakajima, Xen-devel, Volodymyr Babchuk,
	Roger Pau Monné



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
>
> From: Julien Grall <jgrall@amazon.com>
>
> Only a few places are actually including asm/guest_access.h. While this
> is fine today, a follow-up patch will want to move most of the helpers
> from asm/guest_access.h to xen/guest_access.h.
>
> To prepare the move, everyone should include xen/guest_access.h rather
> than asm/guest_access.h.
>
> Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The
> inclusion is now removed as no-one but the latter should include the
> former.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

>
> ---
>    Changes in v2:
>        - Remove some changes that weren't meant to be here.
> ---
> xen/arch/arm/decode.c                | 2 +-
> xen/arch/arm/domain.c                | 2 +-
> xen/arch/arm/guest_walk.c            | 3 ++-
> xen/arch/arm/guestcopy.c             | 2 +-
> xen/arch/arm/vgic-v3-its.c           | 2 +-
> xen/arch/x86/hvm/svm/svm.c           | 2 +-
> xen/arch/x86/hvm/viridian/viridian.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c           | 2 +-
> xen/common/libelf/libelf-loader.c    | 2 +-
> xen/include/asm-arm/guest_access.h   | 1 -
> xen/lib/x86/private.h                | 2 +-
> 11 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index 144793c8cea0..792c2e92a7eb 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -17,12 +17,12 @@
>  * GNU General Public License for more details.
>  */
>
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/sched.h>
> #include <xen/types.h>
>
> #include <asm/current.h>
> -#include <asm/guest_access.h>
>
> #include "decode.h"
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 31169326b2e3..9258f6d3faa2 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -12,6 +12,7 @@
> #include <xen/bitops.h>
> #include <xen/errno.h>
> #include <xen/grant_table.h>
> +#include <xen/guest_access.h>
> #include <xen/hypercall.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> @@ -26,7 +27,6 @@
> #include <asm/current.h>
> #include <asm/event.h>
> #include <asm/gic.h>
> -#include <asm/guest_access.h>
> #include <asm/guest_atomics.h>
> #include <asm/irq.h>
> #include <asm/p2m.h>
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index a1cdd7f4afea..b4496c4c86c6 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -16,8 +16,9 @@
>  */
>
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/sched.h>
> -#include <asm/guest_access.h>
> +
> #include <asm/guest_walk.h>
> #include <asm/short-desc.h>
>
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index c8023e2bca5d..32681606d8fc 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -1,10 +1,10 @@
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/mm.h>
> #include <xen/sched.h>
>
> #include <asm/current.h>
> -#include <asm/guest_access.h>
>
> #define COPY_flush_dcache   (1U << 0)
> #define COPY_from_guest     (0U << 1)
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 6e153c698d56..58d939b85f92 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -32,6 +32,7 @@
> #include <xen/bitops.h>
> #include <xen/config.h>
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/init.h>
> #include <xen/softirq.h>
> @@ -39,7 +40,6 @@
> #include <xen/sched.h>
> #include <xen/sizes.h>
> #include <asm/current.h>
> -#include <asm/guest_access.h>
> #include <asm/mmio.h>
> #include <asm/gic_v3_defs.h>
> #include <asm/gic_v3_its.h>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index ca3bbfcbb355..7301f3cd6004 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -16,6 +16,7 @@
>  * this program; If not, see <http://www.gnu.org/licenses/>.
>  */
>
> +#include <xen/guest_access.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/trace.h>
> @@ -34,7 +35,6 @@
> #include <asm/cpufeature.h>
> #include <asm/processor.h>
> #include <asm/amd.h>
> -#include <asm/guest_access.h>
> #include <asm/debugreg.h>
> #include <asm/msr.h>
> #include <asm/i387.h>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index 977c1bc54fad..dc7183a54627 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -5,12 +5,12 @@
>  * Hypervisor Top Level Functional Specification for more information.
>  */
>
> +#include <xen/guest_access.h>
> #include <xen/sched.h>
> #include <xen/version.h>
> #include <xen/hypercall.h>
> #include <xen/domain_page.h>
> #include <xen/param.h>
> -#include <asm/guest_access.h>
> #include <asm/guest/hyperv-tlfs.h>
> #include <asm/paging.h>
> #include <asm/p2m.h>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index eb54aadfbafb..cb5df1e81c9c 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -15,6 +15,7 @@
>  * this program; If not, see <http://www.gnu.org/licenses/>.
>  */
>
> +#include <xen/guest_access.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/param.h>
> @@ -31,7 +32,6 @@
> #include <asm/regs.h>
> #include <asm/cpufeature.h>
> #include <asm/processor.h>
> -#include <asm/guest_access.h>
> #include <asm/debugreg.h>
> #include <asm/msr.h>
> #include <asm/p2m.h>
> diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
> index 0f468727d04a..629cc0d3e611 100644
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -16,7 +16,7 @@
>  */
>
> #ifdef __XEN__
> -#include <asm/guest_access.h>
> +#include <xen/guest_access.h>
> #endif
>
> #include "libelf-private.h"
> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
> index 31b9f03f0015..b9a89c495527 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -1,7 +1,6 @@
> #ifndef __ASM_ARM_GUEST_ACCESS_H__
> #define __ASM_ARM_GUEST_ACCESS_H__
>
> -#include <xen/guest_access.h>
> #include <xen/errno.h>
> #include <xen/sched.h>
>
> diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
> index b793181464f3..2d53bd3ced23 100644
> --- a/xen/lib/x86/private.h
> +++ b/xen/lib/x86/private.h
> @@ -4,12 +4,12 @@
> #ifdef __XEN__
>
> #include <xen/bitops.h>
> +#include <xen/guest_access.h>
> #include <xen/kernel.h>
> #include <xen/lib.h>
> #include <xen/nospec.h>
> #include <xen/types.h>
>
> -#include <asm/guest_access.h>
> #include <asm/msr-index.h>
>
> #define copy_to_buffer_offset copy_to_guest_offset
> --
> 2.17.1
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
                     ` (2 preceding siblings ...)
  2020-07-31 12:45   ` Bertrand Marquis
@ 2020-07-31 12:45   ` Bertrand Marquis
  3 siblings, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:45 UTC (permalink / raw)
  To: Julien Grall
  Cc: Kevin Tian, Stefano Stabellini, Jan Beulich, Wei Liu,
	Paul Durrant, Andrew Cooper, Julien Grall, Ian Jackson,
	George Dunlap, Jun Nakajima, Xen-devel, nd, Volodymyr Babchuk,
	Roger Pau Monné



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Only a few places are actually including asm/guest_access.h. While this
> is fine today, a follow-up patch will want to move most of the helpers
> from asm/guest_access.h to xen/guest_access.h.
> 
> To prepare the move, everyone should include xen/guest_access.h rather
> than asm/guest_access.h.
> 
> Interestingly, asm-arm/guest_access.h includes xen/guest_access.h. The
> inclusion is now removed as no-one but the latter should include the
> former.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

(sorry forgot to remove the disclaimer in the previous one)
> 
> ---
>    Changes in v2:
>        - Remove some changes that weren't meant to be here.
> ---
> xen/arch/arm/decode.c                | 2 +-
> xen/arch/arm/domain.c                | 2 +-
> xen/arch/arm/guest_walk.c            | 3 ++-
> xen/arch/arm/guestcopy.c             | 2 +-
> xen/arch/arm/vgic-v3-its.c           | 2 +-
> xen/arch/x86/hvm/svm/svm.c           | 2 +-
> xen/arch/x86/hvm/viridian/viridian.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c           | 2 +-
> xen/common/libelf/libelf-loader.c    | 2 +-
> xen/include/asm-arm/guest_access.h   | 1 -
> xen/lib/x86/private.h                | 2 +-
> 11 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index 144793c8cea0..792c2e92a7eb 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -17,12 +17,12 @@
>  * GNU General Public License for more details.
>  */
> 
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/sched.h>
> #include <xen/types.h>
> 
> #include <asm/current.h>
> -#include <asm/guest_access.h>
> 
> #include "decode.h"
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 31169326b2e3..9258f6d3faa2 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -12,6 +12,7 @@
> #include <xen/bitops.h>
> #include <xen/errno.h>
> #include <xen/grant_table.h>
> +#include <xen/guest_access.h>
> #include <xen/hypercall.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> @@ -26,7 +27,6 @@
> #include <asm/current.h>
> #include <asm/event.h>
> #include <asm/gic.h>
> -#include <asm/guest_access.h>
> #include <asm/guest_atomics.h>
> #include <asm/irq.h>
> #include <asm/p2m.h>
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index a1cdd7f4afea..b4496c4c86c6 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -16,8 +16,9 @@
>  */
> 
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/sched.h>
> -#include <asm/guest_access.h>
> +
> #include <asm/guest_walk.h>
> #include <asm/short-desc.h>
> 
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index c8023e2bca5d..32681606d8fc 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -1,10 +1,10 @@
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/mm.h>
> #include <xen/sched.h>
> 
> #include <asm/current.h>
> -#include <asm/guest_access.h>
> 
> #define COPY_flush_dcache   (1U << 0)
> #define COPY_from_guest     (0U << 1)
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 6e153c698d56..58d939b85f92 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c
> @@ -32,6 +32,7 @@
> #include <xen/bitops.h>
> #include <xen/config.h>
> #include <xen/domain_page.h>
> +#include <xen/guest_access.h>
> #include <xen/lib.h>
> #include <xen/init.h>
> #include <xen/softirq.h>
> @@ -39,7 +40,6 @@
> #include <xen/sched.h>
> #include <xen/sizes.h>
> #include <asm/current.h>
> -#include <asm/guest_access.h>
> #include <asm/mmio.h>
> #include <asm/gic_v3_defs.h>
> #include <asm/gic_v3_its.h>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index ca3bbfcbb355..7301f3cd6004 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -16,6 +16,7 @@
>  * this program; If not, see <http://www.gnu.org/licenses/>.
>  */
> 
> +#include <xen/guest_access.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/trace.h>
> @@ -34,7 +35,6 @@
> #include <asm/cpufeature.h>
> #include <asm/processor.h>
> #include <asm/amd.h>
> -#include <asm/guest_access.h>
> #include <asm/debugreg.h>
> #include <asm/msr.h>
> #include <asm/i387.h>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index 977c1bc54fad..dc7183a54627 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -5,12 +5,12 @@
>  * Hypervisor Top Level Functional Specification for more information.
>  */
> 
> +#include <xen/guest_access.h>
> #include <xen/sched.h>
> #include <xen/version.h>
> #include <xen/hypercall.h>
> #include <xen/domain_page.h>
> #include <xen/param.h>
> -#include <asm/guest_access.h>
> #include <asm/guest/hyperv-tlfs.h>
> #include <asm/paging.h>
> #include <asm/p2m.h>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index eb54aadfbafb..cb5df1e81c9c 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -15,6 +15,7 @@
>  * this program; If not, see <http://www.gnu.org/licenses/>.
>  */
> 
> +#include <xen/guest_access.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/param.h>
> @@ -31,7 +32,6 @@
> #include <asm/regs.h>
> #include <asm/cpufeature.h>
> #include <asm/processor.h>
> -#include <asm/guest_access.h>
> #include <asm/debugreg.h>
> #include <asm/msr.h>
> #include <asm/p2m.h>
> diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
> index 0f468727d04a..629cc0d3e611 100644
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -16,7 +16,7 @@
>  */
> 
> #ifdef __XEN__
> -#include <asm/guest_access.h>
> +#include <xen/guest_access.h>
> #endif
> 
> #include "libelf-private.h"
> diff --git a/xen/include/asm-arm/guest_access.h b/xen/include/asm-arm/guest_access.h
> index 31b9f03f0015..b9a89c495527 100644
> --- a/xen/include/asm-arm/guest_access.h
> +++ b/xen/include/asm-arm/guest_access.h
> @@ -1,7 +1,6 @@
> #ifndef __ASM_ARM_GUEST_ACCESS_H__
> #define __ASM_ARM_GUEST_ACCESS_H__
> 
> -#include <xen/guest_access.h>
> #include <xen/errno.h>
> #include <xen/sched.h>
> 
> diff --git a/xen/lib/x86/private.h b/xen/lib/x86/private.h
> index b793181464f3..2d53bd3ced23 100644
> --- a/xen/lib/x86/private.h
> +++ b/xen/lib/x86/private.h
> @@ -4,12 +4,12 @@
> #ifdef __XEN__
> 
> #include <xen/bitops.h>
> +#include <xen/guest_access.h>
> #include <xen/kernel.h>
> #include <xen/lib.h>
> #include <xen/nospec.h>
> #include <xen/types.h>
> 
> -#include <asm/guest_access.h>
> #include <asm/msr-index.h>
> 
> #define copy_to_buffer_offset copy_to_guest_offset
> -- 
> 2.17.1
> 
> 



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style in xen/guest_access.h
  2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
  2020-07-30 19:38   ` Stefano Stabellini
  2020-07-31 11:41   ` Jan Beulich
@ 2020-07-31 12:46   ` Bertrand Marquis
  2 siblings, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:46 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, Xen-devel, nd



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
>    * Add space before and after operator
>    * Align \
>    * Format comments
> 
> No functional changes expected.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

> ---
> xen/include/xen/guest_access.h | 36 +++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
> index 4957b8d1f2b8..52fc7a063249 100644
> --- a/xen/include/xen/guest_access.h
> +++ b/xen/include/xen/guest_access.h
> @@ -18,20 +18,24 @@
> #define guest_handle_add_offset(hnd, nr) ((hnd).p += (nr))
> #define guest_handle_subtract_offset(hnd, nr) ((hnd).p -= (nr))
> 
> -/* Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> - * to the specified type of XEN_GUEST_HANDLE_PARAM. */
> +/*
> + * Cast a guest handle (either XEN_GUEST_HANDLE or XEN_GUEST_HANDLE_PARAM)
> + * to the specified type of XEN_GUEST_HANDLE_PARAM.
> + */
> #define guest_handle_cast(hnd, type) ({         \
>     type *_x = (hnd).p;                         \
> -    (XEN_GUEST_HANDLE_PARAM(type)) { _x };            \
> +    (XEN_GUEST_HANDLE_PARAM(type)) { _x };      \
> })
> 
> /* Cast a XEN_GUEST_HANDLE to XEN_GUEST_HANDLE_PARAM */
> #define guest_handle_to_param(hnd, type) ({                  \
>     typeof((hnd).p) _x = (hnd).p;                            \
>     XEN_GUEST_HANDLE_PARAM(type) _y = { _x };                \
> -    /* type checking: make sure that the pointers inside     \
> +    /*                                                       \
> +     * type checking: make sure that the pointers inside     \
>      * XEN_GUEST_HANDLE and XEN_GUEST_HANDLE_PARAM are of    \
> -     * the same type, then return hnd */                     \
> +     * the same type, then return hnd.                       \
> +     */                                                      \
>     (void)(&_x == &_y.p);                                    \
>     _y;                                                      \
> })
> @@ -106,13 +110,13 @@
>  * guest_handle_subrange_okay().
>  */
> 
> -#define __copy_to_guest_offset(hnd, off, ptr, nr) ({    \
> -    const typeof(*(ptr)) *_s = (ptr);                   \
> -    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;          \
> -    /* Check that the handle is not for a const type */ \
> -    void *__maybe_unused _t = (hnd).p;                  \
> -    (void)((hnd).p == _s);                              \
> -    __raw_copy_to_guest(_d+(off), _s, sizeof(*_s)*(nr));\
> +#define __copy_to_guest_offset(hnd, off, ptr, nr) ({        \
> +    const typeof(*(ptr)) *_s = (ptr);                       \
> +    char (*_d)[sizeof(*_s)] = (void *)(hnd).p;              \
> +    /* Check that the handle is not for a const type */     \
> +    void *__maybe_unused _t = (hnd).p;                      \
> +    (void)((hnd).p == _s);                                  \
> +    __raw_copy_to_guest(_d + (off), _s, sizeof(*_s) * (nr));\
> })
> 
> #define __clear_guest_offset(hnd, off, nr) ({    \
> @@ -120,10 +124,10 @@
>     __raw_clear_guest(_d + (off), nr);           \
> })
> 
> -#define __copy_from_guest_offset(ptr, hnd, off, nr) ({  \
> -    const typeof(*(ptr)) *_s = (hnd).p;                 \
> -    typeof(*(ptr)) *_d = (ptr);                         \
> -    __raw_copy_from_guest(_d, _s+(off), sizeof(*_d)*(nr));\
> +#define __copy_from_guest_offset(ptr, hnd, off, nr) ({          \
> +    const typeof(*(ptr)) *_s = (hnd).p;                         \
> +    typeof(*(ptr)) *_d = (ptr);                                 \
> +    __raw_copy_from_guest(_d, _s + (off), sizeof (*_d) * (nr)); \
> })
> 
> #define __copy_field_to_guest(hnd, ptr, field) ({       \
> -- 
> 2.17.1
> 
> 



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 4/7] xen/arm: guestcopy: Re-order the includes
  2020-07-30 18:18 ` [RESEND][PATCH v2 4/7] xen/arm: guestcopy: " Julien Grall
  2020-07-30 19:37   ` Stefano Stabellini
@ 2020-07-31 12:53   ` Bertrand Marquis
  2020-07-31 12:56     ` Bertrand Marquis
  1 sibling, 1 reply; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:53 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
>
> From: Julien Grall <jgrall@amazon.com>
>
> We usually have xen/ includes first and then asm/. They are also ordered
> alphabetically among themselves.
>
> Signed-off-by: Julien Grall <jgrall@amazon.com>

This could have been merged in patch 3.

But anyway:
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>


> ---
> xen/arch/arm/guestcopy.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
> index 7a0f3e9d5fc6..c8023e2bca5d 100644
> --- a/xen/arch/arm/guestcopy.c
> +++ b/xen/arch/arm/guestcopy.c
> @@ -1,7 +1,8 @@
> -#include <xen/lib.h>
> #include <xen/domain_page.h>
> +#include <xen/lib.h>
> #include <xen/mm.h>
> #include <xen/sched.h>
> +
> #include <asm/current.h>
> #include <asm/guest_access.h>
>
> --
> 2.17.1
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 4/7] xen/arm: guestcopy: Re-order the includes
  2020-07-31 12:53   ` Bertrand Marquis
@ 2020-07-31 12:56     ` Bertrand Marquis
  0 siblings, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:56 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk, nd



> On 31 Jul 2020, at 14:53, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> 
> 
> 
>> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
>> 
>> From: Julien Grall <jgrall@amazon.com>
>> 
>> We usually have xen/ includes first and then asm/. They are also ordered
>> alphabetically among themselves.
>> 
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> This could have been merged in patch 3.
> 
> But anyway:
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> 
>> ---
>> xen/arch/arm/guestcopy.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
>> index 7a0f3e9d5fc6..c8023e2bca5d 100644
>> --- a/xen/arch/arm/guestcopy.c
>> +++ b/xen/arch/arm/guestcopy.c
>> @@ -1,7 +1,8 @@
>> -#include <xen/lib.h>
>> #include <xen/domain_page.h>
>> +#include <xen/lib.h>
>> #include <xen/mm.h>
>> #include <xen/sched.h>
>> +
>> #include <asm/current.h>
>> #include <asm/guest_access.h>
>> 
>> --
>> 2.17.1
>> 
>> 
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

sorry for the notice, i need to find a way to turn it off automatically :-)





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes
  2020-07-30 18:18 ` [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes Julien Grall
  2020-07-30 19:36   ` Stefano Stabellini
@ 2020-07-31 12:58   ` Bertrand Marquis
  1 sibling, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 12:58 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Julien Grall, Stefano Stabellini, Volodymyr Babchuk, nd



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> We usually have xen/ includes first and then asm/. They are also ordered
> alphabetically among themselves.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

> ---
> xen/arch/arm/kernel.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 8eff0748367d..f95fa392af44 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -3,20 +3,20 @@
>  *
>  * Copyright (C) 2011 Citrix Systems, Inc.
>  */
> +#include <xen/domain_page.h>
> #include <xen/errno.h>
> +#include <xen/gunzip.h>
> #include <xen/init.h>
> #include <xen/lib.h>
> +#include <xen/libfdt/libfdt.h>
> #include <xen/mm.h>
> -#include <xen/domain_page.h>
> #include <xen/sched.h>
> -#include <asm/byteorder.h>
> -#include <asm/setup.h>
> -#include <xen/libfdt/libfdt.h>
> -#include <xen/gunzip.h>
> #include <xen/vmap.h>
> 
> +#include <asm/byteorder.h>
> #include <asm/guest_access.h>
> #include <asm/kernel.h>
> +#include <asm/setup.h>
> 
> #define UIMAGE_MAGIC          0x27051956
> #define UIMAGE_NMLEN          32
> -- 
> 2.17.1
> 
> 



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics
  2020-07-30 18:18 ` [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics Julien Grall
@ 2020-07-31 13:04   ` Bertrand Marquis
  0 siblings, 0 replies; 24+ messages in thread
From: Bertrand Marquis @ 2020-07-31 13:04 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall,
	Ian Jackson, George Dunlap, Jan Beulich, Xen-devel, nd,
	Roger Pau Monné



> On 30 Jul 2020, at 20:18, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Add emacs magics for xen/guest_access.h and
> asm-x86/guest_access.h.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Most of file in Xen source code seem to have a white line before the “emacs magics”.
If this is something that should be enforced, it should be done here.

If not the change seems ok :-)

> 
> ---
>    Changes in v2:
>        - Remove the word "missing"
> ---
> xen/include/asm-x86/guest_access.h | 8 ++++++++
> xen/include/xen/guest_access.h     | 8 ++++++++
> 2 files changed, 16 insertions(+)
> 
> diff --git a/xen/include/asm-x86/guest_access.h b/xen/include/asm-x86/guest_access.h
> index 2be3577bd340..3ffde205f6a1 100644
> --- a/xen/include/asm-x86/guest_access.h
> +++ b/xen/include/asm-x86/guest_access.h
> @@ -160,3 +160,11 @@
> })
> 
> #endif /* __ASM_X86_GUEST_ACCESS_H__ */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/xen/guest_access.h b/xen/include/xen/guest_access.h
> index 09989df819ce..ef9aaa3efcfe 100644
> --- a/xen/include/xen/guest_access.h
> +++ b/xen/include/xen/guest_access.h
> @@ -33,3 +33,11 @@ char *safe_copy_string_from_guest(XEN_GUEST_HANDLE(char) u_buf,
>                                   size_t size, size_t max_size);
> 
> #endif /* __XEN_GUEST_ACCESS_H__ */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 2.17.1
> 
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, back to index

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 18:18 [RESEND][PATCH v2 0/7] xen: Consolidate asm-*/guest_access.h in xen/guest_access.h Julien Grall
2020-07-30 18:18 ` [RESEND][PATCH v2 1/7] xen/guest_access: Add emacs magics Julien Grall
2020-07-31 13:04   ` Bertrand Marquis
2020-07-30 18:18 ` [RESEND][PATCH v2 2/7] xen/arm: kernel: Re-order the includes Julien Grall
2020-07-30 19:36   ` Stefano Stabellini
2020-07-31 12:58   ` Bertrand Marquis
2020-07-30 18:18 ` [RESEND][PATCH v2 3/7] xen/arm: decode: " Julien Grall
2020-07-30 19:37   ` Stefano Stabellini
2020-07-30 18:18 ` [RESEND][PATCH v2 4/7] xen/arm: guestcopy: " Julien Grall
2020-07-30 19:37   ` Stefano Stabellini
2020-07-31 12:53   ` Bertrand Marquis
2020-07-31 12:56     ` Bertrand Marquis
2020-07-30 18:18 ` [RESEND][PATCH v2 5/7] xen: include xen/guest_access.h rather than asm/guest_access.h Julien Grall
2020-07-30 19:37   ` Stefano Stabellini
2020-07-31 11:36   ` Jan Beulich
2020-07-31 12:45   ` Bertrand Marquis
2020-07-31 12:45   ` Bertrand Marquis
2020-07-30 18:18 ` [RESEND][PATCH v2 6/7] xen/guest_access: Consolidate guest access helpers in xen/guest_access.h Julien Grall
2020-07-30 19:37   ` Stefano Stabellini
2020-07-31 11:45   ` Jan Beulich
2020-07-30 18:18 ` [RESEND][PATCH v2 7/7] xen/guest_access: Fix coding style " Julien Grall
2020-07-30 19:38   ` Stefano Stabellini
2020-07-31 11:41   ` Jan Beulich
2020-07-31 12:46   ` Bertrand Marquis

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git