All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory
@ 2021-08-09 14:47 Costin Lupu
  2021-08-09 14:47 ` [PATCH 1/4] public: Add page related definitions for accessing " Costin Lupu
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Costin Lupu @ 2021-08-09 14:47 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
	Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Wei Liu,
	Roger Pau Monné,
	Juergen Gross

This series tries to fix a side-effect introduced by commits 0dbb4be7 and
d1b32abd which added a dependency to xenctrl for foreignmemory and gnntab
libraries library only because they needed to use the XC_PAGE_* values.

These changes introduce the XEN_PAGE_* definitions that will be used by any
toolstack component that doesn't need a dependency to xenctrl library.  

Costin Lupu (4):
  public: Add page related definitions for accessing guests memory
  libs/ctrl: Use Xen values for XC_PAGE_* definitions
  libs/foreignmemory: Use XEN_PAGE_* definitions
  libs/gnttab: Use XEN_PAGE_* definitions

 tools/include/xenctrl.h            |  7 +++---
 tools/libs/foreignmemory/core.c    |  2 +-
 tools/libs/foreignmemory/freebsd.c | 10 ++++----
 tools/libs/foreignmemory/linux.c   | 18 +++++++-------
 tools/libs/foreignmemory/minios.c  | 10 +-------
 tools/libs/foreignmemory/netbsd.c  | 10 ++++----
 tools/libs/foreignmemory/private.h |  2 +-
 tools/libs/foreignmemory/solaris.c |  6 ++---
 tools/libs/gnttab/freebsd.c        | 20 ++++++++--------
 tools/libs/gnttab/linux.c          | 20 ++++++++--------
 tools/libs/gnttab/netbsd.c         | 20 ++++++++--------
 xen/include/public/arch-arm/page.h | 34 ++++++++++++++++++++++++++
 xen/include/public/arch-x86/page.h | 34 ++++++++++++++++++++++++++
 xen/include/public/page.h          | 38 ++++++++++++++++++++++++++++++
 14 files changed, 165 insertions(+), 66 deletions(-)
 create mode 100644 xen/include/public/arch-arm/page.h
 create mode 100644 xen/include/public/arch-x86/page.h
 create mode 100644 xen/include/public/page.h

-- 
2.20.1



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

* [PATCH 1/4] public: Add page related definitions for accessing guests memory
  2021-08-09 14:47 [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory Costin Lupu
@ 2021-08-09 14:47 ` Costin Lupu
  2021-08-10  6:41   ` Jan Beulich
  2021-08-09 14:47 ` [PATCH 2/4] libs/ctrl: Use Xen values for XC_PAGE_* definitions Costin Lupu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Costin Lupu @ 2021-08-09 14:47 UTC (permalink / raw)
  To: xen-devel
  Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
	Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Wei Liu,
	Roger Pau Monné

These changes introduce the page related definitions needed for mapping and
accessing guests memory. These values are intended to be used by any toolstack
component that needs to map guests memory. Until now, the values were defined
by the xenctrl.h header, therefore whenever a component had to use them it also
had to add a dependency for the xenctrl library.

For this patch we set the same values for both x86 and ARM architectures.

Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
 xen/include/public/arch-arm/page.h | 34 ++++++++++++++++++++++++++
 xen/include/public/arch-x86/page.h | 34 ++++++++++++++++++++++++++
 xen/include/public/page.h          | 38 ++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+)
 create mode 100644 xen/include/public/arch-arm/page.h
 create mode 100644 xen/include/public/arch-x86/page.h
 create mode 100644 xen/include/public/page.h

diff --git a/xen/include/public/arch-arm/page.h b/xen/include/public/arch-arm/page.h
new file mode 100644
index 0000000000..e970feb49c
--- /dev/null
+++ b/xen/include/public/arch-arm/page.h
@@ -0,0 +1,34 @@
+/******************************************************************************
+ * page.h
+ *
+ * Page definitions for accessing guests memory on ARM
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2021, Costin Lupu
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_ARM_PAGE_H__
+#define __XEN_PUBLIC_ARCH_ARM_PAGE_H__
+
+#define XEN_PAGE_SHIFT           12
+#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
+#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))
+
+#endif /* __XEN_PUBLIC_ARCH_ARM_PAGE_H__ */
diff --git a/xen/include/public/arch-x86/page.h b/xen/include/public/arch-x86/page.h
new file mode 100644
index 0000000000..b1924ea3cb
--- /dev/null
+++ b/xen/include/public/arch-x86/page.h
@@ -0,0 +1,34 @@
+/******************************************************************************
+ * page.h
+ *
+ * Page definitions for accessing guests memory on x86
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2021, Costin Lupu
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_X86_PAGE_H__
+#define __XEN_PUBLIC_ARCH_X86_PAGE_H__
+
+#define XEN_PAGE_SHIFT           12
+#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
+#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))
+
+#endif /* __XEN_PUBLIC_ARCH_X86_PAGE_H__ */
diff --git a/xen/include/public/page.h b/xen/include/public/page.h
new file mode 100644
index 0000000000..d3e95fdb4a
--- /dev/null
+++ b/xen/include/public/page.h
@@ -0,0 +1,38 @@
+/******************************************************************************
+ * page.h
+ *
+ * Page definitions for accessing guests memory
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2021, Costin Lupu
+ */
+
+#ifndef __XEN_PUBLIC_PAGE_H__
+#define __XEN_PUBLIC_PAGE_H__
+
+#if defined(__i386__) || defined(__x86_64__)
+#include "arch-x86/page.h"
+#elif defined (__arm__) || defined (__aarch64__)
+#include "arch-arm/page.h"
+#else
+#error "Unsupported architecture"
+#endif
+
+#endif /* __XEN_PUBLIC_PAGE_H__ */
-- 
2.20.1



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

* [PATCH 2/4] libs/ctrl: Use Xen values for XC_PAGE_* definitions
  2021-08-09 14:47 [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory Costin Lupu
  2021-08-09 14:47 ` [PATCH 1/4] public: Add page related definitions for accessing " Costin Lupu
@ 2021-08-09 14:47 ` Costin Lupu
  2021-08-09 14:47 ` [PATCH 3/4] libs/foreignmemory: Use XEN_PAGE_* definitions Costin Lupu
  2021-08-09 14:47 ` [PATCH 4/4] libs/gnttab: " Costin Lupu
  3 siblings, 0 replies; 7+ messages in thread
From: Costin Lupu @ 2021-08-09 14:47 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Julien Grall

We use the values provided by the Xen public interface for defining the
XC_PAGE_* macros.

Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
 tools/include/xenctrl.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 14adaa0c10..90bb969fa0 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -54,10 +54,11 @@
 #include <xen/foreign/x86_64.h>
 #include <xen/arch-x86/xen-mca.h>
 #endif
+#include <xen/page.h>
 
-#define XC_PAGE_SHIFT           12
-#define XC_PAGE_SIZE            (1UL << XC_PAGE_SHIFT)
-#define XC_PAGE_MASK            (~(XC_PAGE_SIZE-1))
+#define XC_PAGE_SHIFT           XEN_PAGE_SHIFT
+#define XC_PAGE_SIZE            XEN_PAGE_SIZE
+#define XC_PAGE_MASK            XEN_PAGE_MASK
 
 #define INVALID_MFN  (~0UL)
 
-- 
2.20.1



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

* [PATCH 3/4] libs/foreignmemory: Use XEN_PAGE_* definitions
  2021-08-09 14:47 [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory Costin Lupu
  2021-08-09 14:47 ` [PATCH 1/4] public: Add page related definitions for accessing " Costin Lupu
  2021-08-09 14:47 ` [PATCH 2/4] libs/ctrl: Use Xen values for XC_PAGE_* definitions Costin Lupu
@ 2021-08-09 14:47 ` Costin Lupu
  2021-08-09 14:47 ` [PATCH 4/4] libs/gnttab: " Costin Lupu
  3 siblings, 0 replies; 7+ messages in thread
From: Costin Lupu @ 2021-08-09 14:47 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Julien Grall

These changes refine the changes in 0dbb4be7 which added a dependency to
xenctrl library. We use the XEN_PAGE_* definitions instead of the XC_PAGE_*
definitions and therefore we get rid of the unnecessary dependency.

Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
 tools/libs/foreignmemory/core.c    |  2 +-
 tools/libs/foreignmemory/freebsd.c | 10 +++++-----
 tools/libs/foreignmemory/linux.c   | 18 +++++++++---------
 tools/libs/foreignmemory/minios.c  | 10 +---------
 tools/libs/foreignmemory/netbsd.c  | 10 +++++-----
 tools/libs/foreignmemory/private.h |  2 +-
 tools/libs/foreignmemory/solaris.c |  6 +++---
 7 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 7edc6f0dbf..ad1ad9fc67 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -202,7 +202,7 @@ int xenforeignmemory_resource_size(
     if ( rc )
         return rc;
 
-    *size = fres.nr_frames << XC_PAGE_SHIFT;
+    *size = fres.nr_frames << XEN_PAGE_SHIFT;
     return 0;
 }
 
diff --git a/tools/libs/foreignmemory/freebsd.c b/tools/libs/foreignmemory/freebsd.c
index 2cf0fa1c38..9439c4ca6a 100644
--- a/tools/libs/foreignmemory/freebsd.c
+++ b/tools/libs/foreignmemory/freebsd.c
@@ -63,7 +63,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     privcmd_mmapbatch_t ioctlx;
     int rc;
 
-    addr = mmap(addr, num << XC_PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
+    addr = mmap(addr, num << XEN_PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
     if ( addr == MAP_FAILED )
         return NULL;
 
@@ -78,7 +78,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
 
-        (void)munmap(addr, num << XC_PAGE_SHIFT);
+        (void)munmap(addr, num << XEN_PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
     }
@@ -89,7 +89,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << XC_PAGE_SHIFT);
+    return munmap(addr, num << XEN_PAGE_SHIFT);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -101,7 +101,7 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(xenforeignmemory_handle *fmem,
                                         xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(xenforeignmemory_handle *fmem,
@@ -120,7 +120,7 @@ int osdep_xenforeignmemory_map_resource(xenforeignmemory_handle *fmem,
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c
index 9062117407..9dabf28cae 100644
--- a/tools/libs/foreignmemory/linux.c
+++ b/tools/libs/foreignmemory/linux.c
@@ -134,7 +134,7 @@ static int retry_paged(int fd, uint32_t dom, void *addr,
         /* At least one gfn is still in paging state */
         ioctlx.num = 1;
         ioctlx.dom = dom;
-        ioctlx.addr = (unsigned long)addr + (i<<XC_PAGE_SHIFT);
+        ioctlx.addr = (unsigned long)addr + (i<<XEN_PAGE_SHIFT);
         ioctlx.arr = arr + i;
         ioctlx.err = err + i;
 
@@ -168,7 +168,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     size_t i;
     int rc;
 
-    addr = mmap(addr, num << XC_PAGE_SHIFT, prot, flags | MAP_SHARED,
+    addr = mmap(addr, num << XEN_PAGE_SHIFT, prot, flags | MAP_SHARED,
                 fd, 0);
     if ( addr == MAP_FAILED )
         return NULL;
@@ -198,7 +198,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
          */
         privcmd_mmapbatch_t ioctlx;
         xen_pfn_t *pfn;
-        unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
+        unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XEN_PAGE_SHIFT);
         int os_page_size = sysconf(_SC_PAGESIZE);
 
         if ( pfn_arr_size <= os_page_size )
@@ -210,7 +210,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
             if ( pfn == MAP_FAILED )
             {
                 PERROR("mmap of pfn array failed");
-                (void)munmap(addr, num << XC_PAGE_SHIFT);
+                (void)munmap(addr, num << XEN_PAGE_SHIFT);
                 return NULL;
             }
         }
@@ -243,7 +243,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
                     continue;
                 }
                 rc = map_foreign_batch_single(fd, dom, pfn + i,
-                        (unsigned long)addr + (i<<XC_PAGE_SHIFT));
+                        (unsigned long)addr + (i<<XEN_PAGE_SHIFT));
                 if ( rc < 0 )
                 {
                     rc = -errno;
@@ -271,7 +271,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
 
-        (void)munmap(addr, num << XC_PAGE_SHIFT);
+        (void)munmap(addr, num << XEN_PAGE_SHIFT);
         errno = saved_errno;
         return NULL;
     }
@@ -282,7 +282,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << XC_PAGE_SHIFT);
+    return munmap(addr, num << XEN_PAGE_SHIFT);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -294,7 +294,7 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(
     xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(
@@ -313,7 +313,7 @@ int osdep_xenforeignmemory_map_resource(
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/minios.c b/tools/libs/foreignmemory/minios.c
index f2f4dfb2be..2454eb9af3 100644
--- a/tools/libs/foreignmemory/minios.c
+++ b/tools/libs/foreignmemory/minios.c
@@ -17,14 +17,6 @@
  * Copyright 2007-2008 Samuel Thibault <samuel.thibault@eu.citrix.com>.
  */
 
-/*
- * xenctrl.h currently defines __XEN_TOOLS__ which affects what is
- * exposed by Xen headers. As the define needs to be set consistently,
- * we want to include xenctrl.h before the mini-os headers (they include
- * public headers).
- */
-#include <xenctrl.h>
-
 #include <mini-os/types.h>
 #include <mini-os/os.h>
 #include <mini-os/mm.h>
@@ -63,7 +55,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num << XC_PAGE_SHIFT);
+    return munmap(addr, num << XEN_PAGE_SHIFT);
 }
 
 /*
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index 597db775d7..ba69b9c6bb 100644
--- a/tools/libs/foreignmemory/netbsd.c
+++ b/tools/libs/foreignmemory/netbsd.c
@@ -76,7 +76,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 {
     int fd = fmem->fd;
     privcmd_mmapbatch_v2_t ioctlx;
-    addr = mmap(addr, num * XC_PAGE_SIZE, prot,
+    addr = mmap(addr, num * XEN_PAGE_SIZE, prot,
                 flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( addr == MAP_FAILED ) {
         PERROR("osdep_xenforeignmemory_map: mmap failed");
@@ -93,7 +93,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
     {
         int saved_errno = errno;
         PERROR("osdep_xenforeignmemory_map: ioctl failed");
-        munmap(addr, num * XC_PAGE_SIZE);
+        munmap(addr, num * XEN_PAGE_SIZE);
         errno = saved_errno;
         return NULL;
     }
@@ -104,7 +104,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num * XC_PAGE_SIZE);
+    return munmap(addr, num * XEN_PAGE_SIZE);
 }
 
 int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -117,7 +117,7 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
 int osdep_xenforeignmemory_unmap_resource(
     xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres)
 {
-    return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
+    return fres ? munmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT) : 0;
 }
 
 int osdep_xenforeignmemory_map_resource(
@@ -136,7 +136,7 @@ int osdep_xenforeignmemory_map_resource(
         /* Request for resource size.  Skip mmap(). */
         goto skip_mmap;
 
-    fres->addr = mmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT,
+    fres->addr = mmap(fres->addr, fres->nr_frames << XEN_PAGE_SHIFT,
                       fres->prot, fres->flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( fres->addr == MAP_FAILED )
         return -1;
diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h
index 8540303adc..1200f98067 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -1,7 +1,6 @@
 #ifndef XENFOREIGNMEMORY_PRIVATE_H
 #define XENFOREIGNMEMORY_PRIVATE_H
 
-#include <xenctrl.h>
 #include <xentoollog.h>
 
 #include <xenforeignmemory.h>
@@ -9,6 +8,7 @@
 #include <xentoolcore_internal.h>
 
 #include <xen/xen.h>
+#include <xen/page.h>
 #include <xen/sys/privcmd.h>
 
 struct xenforeignmemory_handle {
diff --git a/tools/libs/foreignmemory/solaris.c b/tools/libs/foreignmemory/solaris.c
index 958fb01f6d..4466780bd8 100644
--- a/tools/libs/foreignmemory/solaris.c
+++ b/tools/libs/foreignmemory/solaris.c
@@ -72,7 +72,7 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
 {
     int fd = fmem->fd;
     privcmd_mmapbatch_t ioctlx;
-    addr = mmap(addr, num*XC_PAGE_SIZE, prot, flags | MAP_SHARED, fd, 0);
+    addr = mmap(addr, num*XEN_PAGE_SIZE, prot, flags | MAP_SHARED, fd, 0);
     if ( addr == MAP_FAILED )
         return NULL;
 
@@ -84,7 +84,7 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
     {
         int saved_errno = errno;
 
-        (void)munmap(addr, num*XC_PAGE_SIZE);
+        (void)munmap(addr, num*XEN_PAGE_SIZE);
         errno = saved_errno;
         return NULL;
     }
@@ -94,7 +94,7 @@ void *osdep_map_foreign_batch(xenforeignmem_handle *fmem, uint32_t dom,
 int osdep_xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
                                  void *addr, size_t num)
 {
-    return munmap(addr, num*XC_PAGE_SIZE);
+    return munmap(addr, num*XEN_PAGE_SIZE);
 }
 
 /*
-- 
2.20.1



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

* [PATCH 4/4] libs/gnttab: Use XEN_PAGE_* definitions
  2021-08-09 14:47 [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory Costin Lupu
                   ` (2 preceding siblings ...)
  2021-08-09 14:47 ` [PATCH 3/4] libs/foreignmemory: Use XEN_PAGE_* definitions Costin Lupu
@ 2021-08-09 14:47 ` Costin Lupu
  3 siblings, 0 replies; 7+ messages in thread
From: Costin Lupu @ 2021-08-09 14:47 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Juergen Gross, Julien Grall

These changes refine the changes in d1b32abd which added a dependency to
xenctrl library. We use the XEN_PAGE_* definitions instead of the XC_PAGE_*
definitions and therefore we get rid of the unnecessary dependency.

Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
 tools/libs/gnttab/freebsd.c | 20 ++++++++++----------
 tools/libs/gnttab/linux.c   | 20 ++++++++++----------
 tools/libs/gnttab/netbsd.c  | 20 ++++++++++----------
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
index e42ac3fbf3..7ecb0e3b38 100644
--- a/tools/libs/gnttab/freebsd.c
+++ b/tools/libs/gnttab/freebsd.c
@@ -28,9 +28,9 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
+#include <xen/page.h>
 #include <xen/sys/gntdev.h>
 
-#include <xenctrl.h>
 #include <xen-tools/libs.h>
 
 #include "private.h"
@@ -74,7 +74,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
     int domids_stride;
     unsigned int refs_size = ROUNDUP(count *
                                      sizeof(struct ioctl_gntdev_grant_ref),
-                                     XC_PAGE_SHIFT);
+                                     XEN_PAGE_SHIFT);
     int os_page_size = getpagesize();
 
     domids_stride = (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN) ? 0 : 1;
@@ -105,7 +105,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
         goto out;
     }
 
-    addr = mmap(NULL, XC_PAGE_SIZE * count, prot, MAP_SHARED, fd,
+    addr = mmap(NULL, XEN_PAGE_SIZE * count, prot, MAP_SHARED, fd,
                 map.index);
     if ( addr != MAP_FAILED )
     {
@@ -114,7 +114,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 
         notify.index = map.index;
         notify.action = 0;
-        if ( notify_offset < XC_PAGE_SIZE * count )
+        if ( notify_offset < XEN_PAGE_SIZE * count )
         {
             notify.index += notify_offset;
             notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -129,7 +129,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
         if ( rv )
         {
             GTERROR(xgt->logger, "ioctl SET_UNMAP_NOTIFY failed");
-            munmap(addr, count * XC_PAGE_SIZE);
+            munmap(addr, count * XEN_PAGE_SIZE);
             addr = MAP_FAILED;
         }
     }
@@ -187,7 +187,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
     }
 
     /* Next, unmap the memory. */
-    if ( (rc = munmap(start_address, count * XC_PAGE_SIZE)) )
+    if ( (rc = munmap(start_address, count * XEN_PAGE_SIZE)) )
         return rc;
 
     /* Finally, unmap the driver slots used to store the grant information. */
@@ -254,7 +254,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
         goto out;
     }
 
-    area = mmap(NULL, count * XC_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
+    area = mmap(NULL, count * XEN_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
                 fd, gref_info.index);
 
     if ( area == MAP_FAILED )
@@ -266,7 +266,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 
     notify.index = gref_info.index;
     notify.action = 0;
-    if ( notify_offset < XC_PAGE_SIZE * count )
+    if ( notify_offset < XEN_PAGE_SIZE * count )
     {
         notify.index += notify_offset;
         notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -281,7 +281,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
     if ( err )
     {
         GSERROR(xgs->logger, "ioctl SET_UNMAP_NOTIFY failed");
-        munmap(area, count * XC_PAGE_SIZE);
+        munmap(area, count * XEN_PAGE_SIZE);
         area = NULL;
     }
 
@@ -304,7 +304,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 int osdep_gntshr_unshare(xengntshr_handle *xgs,
                          void *start_address, uint32_t count)
 {
-    return munmap(start_address, count * XC_PAGE_SIZE);
+    return munmap(start_address, count * XEN_PAGE_SIZE);
 }
 
 /*
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
index 5628fd5719..11f1acb771 100644
--- a/tools/libs/gnttab/linux.c
+++ b/tools/libs/gnttab/linux.c
@@ -29,10 +29,10 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
+#include <xen/page.h>
 #include <xen/sys/gntdev.h>
 #include <xen/sys/gntalloc.h>
 
-#include <xenctrl.h>
 #include <xen-tools/libs.h>
 
 #include "private.h"
@@ -101,7 +101,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
         map = alloca(map_size);
     else
     {
-        map_size = ROUNDUP(map_size, XC_PAGE_SHIFT);
+        map_size = ROUNDUP(map_size, XEN_PAGE_SHIFT);
         map = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
                    MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0);
         if ( map == MAP_FAILED )
@@ -125,7 +125,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
     }
 
  retry:
-    addr = mmap(NULL, XC_PAGE_SIZE * count, prot, MAP_SHARED, fd,
+    addr = mmap(NULL, XEN_PAGE_SIZE * count, prot, MAP_SHARED, fd,
                 map->index);
 
     if (addr == MAP_FAILED && errno == EAGAIN)
@@ -150,7 +150,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
         struct ioctl_gntdev_unmap_notify notify;
         notify.index = map->index;
         notify.action = 0;
-        if (notify_offset < XC_PAGE_SIZE * count) {
+        if (notify_offset < XEN_PAGE_SIZE * count) {
             notify.index += notify_offset;
             notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
         }
@@ -162,7 +162,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
             rv = ioctl(fd, IOCTL_GNTDEV_SET_UNMAP_NOTIFY, &notify);
         if (rv) {
             GTERROR(xgt->logger, "ioctl SET_UNMAP_NOTIFY failed");
-            munmap(addr, count * XC_PAGE_SIZE);
+            munmap(addr, count * XEN_PAGE_SIZE);
             addr = MAP_FAILED;
         }
     }
@@ -218,7 +218,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
     }
 
     /* Next, unmap the memory. */
-    if ( (rc = munmap(start_address, count * XC_PAGE_SIZE)) )
+    if ( (rc = munmap(start_address, count * XEN_PAGE_SIZE)) )
         return rc;
 
     /* Finally, unmap the driver slots used to store the grant information. */
@@ -464,7 +464,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
         goto out;
     }
 
-    area = mmap(NULL, count * XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
+    area = mmap(NULL, count * XEN_PAGE_SIZE, PROT_READ | PROT_WRITE,
         MAP_SHARED, fd, gref_info->index);
 
     if (area == MAP_FAILED) {
@@ -475,7 +475,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 
     notify.index = gref_info->index;
     notify.action = 0;
-    if (notify_offset < XC_PAGE_SIZE * count) {
+    if (notify_offset < XEN_PAGE_SIZE * count) {
         notify.index += notify_offset;
         notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
     }
@@ -487,7 +487,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
         err = ioctl(fd, IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, &notify);
     if (err) {
         GSERROR(xgs->logger, "ioctl SET_UNMAP_NOTIFY failed");
-        munmap(area, count * XC_PAGE_SIZE);
+        munmap(area, count * XEN_PAGE_SIZE);
         area = NULL;
     }
 
@@ -508,7 +508,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 int osdep_gntshr_unshare(xengntshr_handle *xgs,
                          void *start_address, uint32_t count)
 {
-    return munmap(start_address, count * XC_PAGE_SIZE);
+    return munmap(start_address, count * XEN_PAGE_SIZE);
 }
 
 /*
diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
index a4ad624b54..beb94be468 100644
--- a/tools/libs/gnttab/netbsd.c
+++ b/tools/libs/gnttab/netbsd.c
@@ -28,8 +28,8 @@
 #include <sys/ioctl.h>
 #include <sys/mman.h>
 
-#include <xenctrl.h>
 #include <xen/xen.h>
+#include <xen/page.h>
 #include <xen/xenio.h>
 
 #include "private.h"
@@ -84,19 +84,19 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
     }
 
     map.count = count;
-    addr = mmap(NULL, count * XC_PAGE_SIZE,
+    addr = mmap(NULL, count * XEN_PAGE_SIZE,
                 prot, flags | MAP_ANON | MAP_SHARED, -1, 0);
     if ( map.va == MAP_FAILED )
     {
         GTERROR(xgt->logger, "osdep_gnttab_grant_map: mmap failed");
-        munmap((void *)map.va, count * XC_PAGE_SIZE);
+        munmap((void *)map.va, count * XEN_PAGE_SIZE);
         addr = MAP_FAILED;
     }
     map.va = addr;
 
     map.notify.offset = 0;
     map.notify.action = 0;
-    if ( notify_offset < XC_PAGE_SIZE * count )
+    if ( notify_offset < XEN_PAGE_SIZE * count )
     {
         map.notify.offset = notify_offset;
         map.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -112,7 +112,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
     {
         GTERROR(xgt->logger,
             "ioctl IOCTL_GNTDEV_MMAP_GRANT_REF failed: %d", rv);
-        munmap(addr, count * XC_PAGE_SIZE);
+        munmap(addr, count * XEN_PAGE_SIZE);
         addr = MAP_FAILED;
     }
 
@@ -133,7 +133,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
     }
 
     /* Next, unmap the memory. */
-    rc = munmap(start_address, count * XC_PAGE_SIZE);
+    rc = munmap(start_address, count * XEN_PAGE_SIZE);
 
     return rc;
 }
@@ -184,7 +184,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
     alloc.domid = domid;
     alloc.flags = writable ? GNTDEV_ALLOC_FLAG_WRITABLE : 0;
     alloc.count = count;
-    area = mmap(NULL, count * XC_PAGE_SIZE,
+    area = mmap(NULL, count * XEN_PAGE_SIZE,
                 PROT_READ | PROT_WRITE, MAP_ANON | MAP_SHARED, -1, 0);
 
     if ( area == MAP_FAILED )
@@ -197,7 +197,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 
     alloc.notify.offset = 0;
     alloc.notify.action = 0;
-    if ( notify_offset < XC_PAGE_SIZE * count )
+    if ( notify_offset < XEN_PAGE_SIZE * count )
     {
         alloc.notify.offset = notify_offset;
         alloc.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -212,7 +212,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
     if ( err )
     {
         GSERROR(xgs->logger, "IOCTL_GNTDEV_ALLOC_GRANT_REF failed");
-        munmap(area, count * XC_PAGE_SIZE);
+        munmap(area, count * XEN_PAGE_SIZE);
         area = MAP_FAILED;
         goto out;
     }
@@ -227,7 +227,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
 int osdep_gntshr_unshare(xengntshr_handle *xgs,
                          void *start_address, uint32_t count)
 {
-    return munmap(start_address, count * XC_PAGE_SIZE);
+    return munmap(start_address, count * XEN_PAGE_SIZE);
 }
 
 /*
-- 
2.20.1



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

* Re: [PATCH 1/4] public: Add page related definitions for accessing guests memory
  2021-08-09 14:47 ` [PATCH 1/4] public: Add page related definitions for accessing " Costin Lupu
@ 2021-08-10  6:41   ` Jan Beulich
  2021-08-19 18:28     ` Costin Lupu
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2021-08-10  6:41 UTC (permalink / raw)
  To: Costin Lupu
  Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
	Andrew Cooper, George Dunlap, Ian Jackson, Wei Liu,
	Roger Pau Monné,
	xen-devel

On 09.08.2021 16:47, Costin Lupu wrote:
> These changes introduce the page related definitions needed for mapping and
> accessing guests memory. These values are intended to be used by any toolstack
> component that needs to map guests memory. Until now, the values were defined
> by the xenctrl.h header, therefore whenever a component had to use them it also
> had to add a dependency for the xenctrl library.
> 
> For this patch we set the same values for both x86 and ARM architectures.
> 
> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
> ---
>  xen/include/public/arch-arm/page.h | 34 ++++++++++++++++++++++++++
>  xen/include/public/arch-x86/page.h | 34 ++++++++++++++++++++++++++
>  xen/include/public/page.h          | 38 ++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
>  create mode 100644 xen/include/public/arch-arm/page.h
>  create mode 100644 xen/include/public/arch-x86/page.h
>  create mode 100644 xen/include/public/page.h

I'm not convinced of these warranting introduction of new headers, but
I'm also not meaning to say that I'm strictly opposed. I don't recall
this aspect having had any consideration, yet.

> --- /dev/null
> +++ b/xen/include/public/arch-arm/page.h
> @@ -0,0 +1,34 @@
> +/******************************************************************************
> + * page.h
> + *
> + * Page definitions for accessing guests memory on ARM
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2021, Costin Lupu
> + */
> +
> +#ifndef __XEN_PUBLIC_ARCH_ARM_PAGE_H__
> +#define __XEN_PUBLIC_ARCH_ARM_PAGE_H__
> +
> +#define XEN_PAGE_SHIFT           12
> +#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
> +#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))

The latter two, being identical ...

> --- /dev/null
> +++ b/xen/include/public/arch-x86/page.h
> @@ -0,0 +1,34 @@
> +/******************************************************************************
> + * page.h
> + *
> + * Page definitions for accessing guests memory on x86
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2021, Costin Lupu
> + */
> +
> +#ifndef __XEN_PUBLIC_ARCH_X86_PAGE_H__
> +#define __XEN_PUBLIC_ARCH_X86_PAGE_H__
> +
> +#define XEN_PAGE_SHIFT           12
> +#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
> +#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))

... not just for x86, but in general, should imo move ...

> --- /dev/null
> +++ b/xen/include/public/page.h
> @@ -0,0 +1,38 @@
> +/******************************************************************************
> + * page.h
> + *
> + * Page definitions for accessing guests memory
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2021, Costin Lupu
> + */
> +
> +#ifndef __XEN_PUBLIC_PAGE_H__
> +#define __XEN_PUBLIC_PAGE_H__
> +
> +#if defined(__i386__) || defined(__x86_64__)
> +#include "arch-x86/page.h"
> +#elif defined (__arm__) || defined (__aarch64__)
> +#include "arch-arm/page.h"
> +#else
> +#error "Unsupported architecture"
> +#endif

... here. I don't think though that 1UL is an appropriate construct
to use: Imo the smallest type this should evaluate to is xen_ulong_t,
the constant should also be usable in assembly sources, and it would
better also suitably sign-extend when used in e.g. XEN_PAGE_MASK.

Jan

> +#endif /* __XEN_PUBLIC_PAGE_H__ */
> 



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

* Re: [PATCH 1/4] public: Add page related definitions for accessing guests memory
  2021-08-10  6:41   ` Jan Beulich
@ 2021-08-19 18:28     ` Costin Lupu
  0 siblings, 0 replies; 7+ messages in thread
From: Costin Lupu @ 2021-08-19 18:28 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
	Andrew Cooper, George Dunlap, Ian Jackson, Wei Liu,
	Roger Pau Monné,
	xen-devel

Hi Jan,

Thanks for your comments. I've just sent a v2.

Cheers,
Costin

On 8/10/21 9:41 AM, Jan Beulich wrote:
> On 09.08.2021 16:47, Costin Lupu wrote:
>> These changes introduce the page related definitions needed for mapping and
>> accessing guests memory. These values are intended to be used by any toolstack
>> component that needs to map guests memory. Until now, the values were defined
>> by the xenctrl.h header, therefore whenever a component had to use them it also
>> had to add a dependency for the xenctrl library.
>>
>> For this patch we set the same values for both x86 and ARM architectures.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
>> ---
>>  xen/include/public/arch-arm/page.h | 34 ++++++++++++++++++++++++++
>>  xen/include/public/arch-x86/page.h | 34 ++++++++++++++++++++++++++
>>  xen/include/public/page.h          | 38 ++++++++++++++++++++++++++++++
>>  3 files changed, 106 insertions(+)
>>  create mode 100644 xen/include/public/arch-arm/page.h
>>  create mode 100644 xen/include/public/arch-x86/page.h
>>  create mode 100644 xen/include/public/page.h
> 
> I'm not convinced of these warranting introduction of new headers, but
> I'm also not meaning to say that I'm strictly opposed. I don't recall
> this aspect having had any consideration, yet.
> 
>> --- /dev/null
>> +++ b/xen/include/public/arch-arm/page.h
>> @@ -0,0 +1,34 @@
>> +/******************************************************************************
>> + * page.h
>> + *
>> + * Page definitions for accessing guests memory on ARM
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to
>> + * deal in the Software without restriction, including without limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Copyright (c) 2021, Costin Lupu
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_ARCH_ARM_PAGE_H__
>> +#define __XEN_PUBLIC_ARCH_ARM_PAGE_H__
>> +
>> +#define XEN_PAGE_SHIFT           12
>> +#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
>> +#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))
> 
> The latter two, being identical ...
> 
>> --- /dev/null
>> +++ b/xen/include/public/arch-x86/page.h
>> @@ -0,0 +1,34 @@
>> +/******************************************************************************
>> + * page.h
>> + *
>> + * Page definitions for accessing guests memory on x86
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to
>> + * deal in the Software without restriction, including without limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Copyright (c) 2021, Costin Lupu
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_ARCH_X86_PAGE_H__
>> +#define __XEN_PUBLIC_ARCH_X86_PAGE_H__
>> +
>> +#define XEN_PAGE_SHIFT           12
>> +#define XEN_PAGE_SIZE            (1UL << XEN_PAGE_SHIFT)
>> +#define XEN_PAGE_MASK            (~(XEN_PAGE_SIZE - 1))
> 
> ... not just for x86, but in general, should imo move ...
> 
>> --- /dev/null
>> +++ b/xen/include/public/page.h
>> @@ -0,0 +1,38 @@
>> +/******************************************************************************
>> + * page.h
>> + *
>> + * Page definitions for accessing guests memory
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to
>> + * deal in the Software without restriction, including without limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Copyright (c) 2021, Costin Lupu
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_PAGE_H__
>> +#define __XEN_PUBLIC_PAGE_H__
>> +
>> +#if defined(__i386__) || defined(__x86_64__)
>> +#include "arch-x86/page.h"
>> +#elif defined (__arm__) || defined (__aarch64__)
>> +#include "arch-arm/page.h"
>> +#else
>> +#error "Unsupported architecture"
>> +#endif
> 
> ... here. I don't think though that 1UL is an appropriate construct
> to use: Imo the smallest type this should evaluate to is xen_ulong_t,
> the constant should also be usable in assembly sources, and it would
> better also suitably sign-extend when used in e.g. XEN_PAGE_MASK.
> 
> Jan
> 
>> +#endif /* __XEN_PUBLIC_PAGE_H__ */
>>
> 


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

end of thread, other threads:[~2021-08-19 18:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09 14:47 [PATCH 0/4] Introduce XEN_PAGE_* definitions for mapping guests memory Costin Lupu
2021-08-09 14:47 ` [PATCH 1/4] public: Add page related definitions for accessing " Costin Lupu
2021-08-10  6:41   ` Jan Beulich
2021-08-19 18:28     ` Costin Lupu
2021-08-09 14:47 ` [PATCH 2/4] libs/ctrl: Use Xen values for XC_PAGE_* definitions Costin Lupu
2021-08-09 14:47 ` [PATCH 3/4] libs/foreignmemory: Use XEN_PAGE_* definitions Costin Lupu
2021-08-09 14:47 ` [PATCH 4/4] libs/gnttab: " Costin Lupu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.