* [PATCH v4 1/5] tools/debugger: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
@ 2021-06-08 12:35 ` Costin Lupu
2021-06-08 12:35 ` [PATCH v4 2/5] tools/libfsimage: Fix PATH_MAX " Costin Lupu
` (5 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Costin Lupu @ 2021-06-08 12:35 UTC (permalink / raw)
To: xen-devel; +Cc: Tim Deegan, Ian Jackson, Wei Liu
If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror. This
patch replaces usage of PAGE_* macros with KDD_PAGE_* macros in order to avoid
confusion between control domain page granularity (PAGE_* definitions) and
guest domain page granularity (which is what we are dealing with here).
We chose to define the KDD_PAGE_* macros instead of using XC_PAGE_* macros
because (1) the code in kdd.c should not include any Xen headers and (2) to add
consistency for code in both kdd.c and kdd-xen.c.
Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
Reviewed-by: Tim Deegan <tim@xen.org>
---
tools/debugger/kdd/kdd-xen.c | 15 ++++++---------
tools/debugger/kdd/kdd.c | 19 ++++++++-----------
tools/debugger/kdd/kdd.h | 7 +++++++
3 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index f3f9529f9f..e78c9311c4 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -48,9 +48,6 @@
#define MAPSIZE 4093 /* Prime */
-#define PAGE_SHIFT 12
-#define PAGE_SIZE (1U << PAGE_SHIFT)
-
struct kdd_guest {
struct xentoollog_logger xc_log; /* Must be first for xc log callbacks */
xc_interface *xc_handle;
@@ -72,7 +69,7 @@ static void flush_maps(kdd_guest *g)
int i;
for (i = 0; i < MAPSIZE; i++) {
if (g->maps[i] != NULL)
- munmap(g->maps[i], PAGE_SIZE);
+ munmap(g->maps[i], KDD_PAGE_SIZE);
g->maps[i] = NULL;
}
}
@@ -490,13 +487,13 @@ static uint32_t kdd_access_physical_page(kdd_guest *g, uint64_t addr,
uint32_t map_pfn, map_offset;
uint8_t *map;
- map_pfn = (addr >> PAGE_SHIFT);
- map_offset = addr & (PAGE_SIZE - 1);
+ map_pfn = (addr >> KDD_PAGE_SHIFT);
+ map_offset = addr & (KDD_PAGE_SIZE - 1);
/* Evict any mapping of the wrong frame from our slot */
if (g->pfns[map_pfn % MAPSIZE] != map_pfn
&& g->maps[map_pfn % MAPSIZE] != NULL) {
- munmap(g->maps[map_pfn % MAPSIZE], PAGE_SIZE);
+ munmap(g->maps[map_pfn % MAPSIZE], KDD_PAGE_SIZE);
g->maps[map_pfn % MAPSIZE] = NULL;
}
g->pfns[map_pfn % MAPSIZE] = map_pfn;
@@ -507,7 +504,7 @@ static uint32_t kdd_access_physical_page(kdd_guest *g, uint64_t addr,
else {
map = xc_map_foreign_range(g->xc_handle,
g->domid,
- PAGE_SIZE,
+ KDD_PAGE_SIZE,
PROT_READ|PROT_WRITE,
map_pfn);
@@ -533,7 +530,7 @@ uint32_t kdd_access_physical(kdd_guest *g, uint64_t addr,
{
uint32_t chunk, rv, done = 0;
while (len > 0) {
- chunk = PAGE_SIZE - (addr & (PAGE_SIZE - 1));
+ chunk = KDD_PAGE_SIZE - (addr & (KDD_PAGE_SIZE - 1));
if (chunk > len)
chunk = len;
rv = kdd_access_physical_page(g, addr, chunk, buf, write);
diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 17513c2650..320c623eda 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -288,9 +288,6 @@ static void kdd_log_pkt(kdd_state *s, const char *name, kdd_pkt *p)
* Memory access: virtual addresses and syntactic sugar.
*/
-#define PAGE_SHIFT (12)
-#define PAGE_SIZE (1ULL << PAGE_SHIFT)
-
static uint32_t kdd_read_physical(kdd_state *s, uint64_t addr,
uint32_t len, void *buf)
{
@@ -352,7 +349,7 @@ static uint64_t v2p(kdd_state *s, int cpuid, uint64_t va)
/* Walk the appropriate number of levels */
for (i = levels; i > 0; i--) {
- shift = PAGE_SHIFT + bits * (i-1);
+ shift = KDD_PAGE_SHIFT + bits * (i-1);
mask = ((1ULL << bits) - 1) << shift;
offset = ((va & mask) >> shift) * width;
KDD_DEBUG(s, "level %i: mask 0x%16.16"PRIx64" pa 0x%16.16"PRIx64
@@ -364,12 +361,12 @@ static uint64_t v2p(kdd_state *s, int cpuid, uint64_t va)
return -1ULL; // Not present
pa = entry & 0x000ffffffffff000ULL;
if (pse && (i == 2) && (entry & 0x80)) { // Superpage
- mask = ((1ULL << (PAGE_SHIFT + bits)) - 1);
+ mask = ((1ULL << (KDD_PAGE_SHIFT + bits)) - 1);
return (pa & ~mask) + (va & mask);
}
}
- return pa + (va & (PAGE_SIZE - 1));
+ return pa + (va & (KDD_PAGE_SIZE - 1));
}
static uint32_t kdd_access_virtual(kdd_state *s, int cpuid, uint64_t addr,
@@ -380,7 +377,7 @@ static uint32_t kdd_access_virtual(kdd_state *s, int cpuid, uint64_t addr,
/* Process one page at a time */
while (len > 0) {
- chunk = PAGE_SIZE - (addr & (PAGE_SIZE - 1));
+ chunk = KDD_PAGE_SIZE - (addr & (KDD_PAGE_SIZE - 1));
if (chunk > len)
chunk = len;
pa = v2p(s, cpuid, addr);
@@ -591,7 +588,7 @@ static void get_os_info_64(kdd_state *s)
uint64_t dbgkd_addr;
DBGKD_GET_VERSION64 dbgkd_get_version64;
/* Maybe 1GB is too big for the limit to search? */
- uint32_t search_limit = (1024 * 1024 * 1024) / PAGE_SIZE; /*1GB/PageSize*/
+ uint32_t search_limit = (1024 * 1024 * 1024) / KDD_PAGE_SIZE; /*1GB/PageSize*/
uint64_t efer;
/* if we are not in 64-bit mode, fail */
@@ -620,7 +617,7 @@ static void get_os_info_64(kdd_state *s)
* in 1GB range above the current page base address
*/
- base = idt0_addr & ~(PAGE_SIZE - 1);
+ base = idt0_addr & ~(KDD_PAGE_SIZE - 1);
while (search_limit) {
uint16_t val;
@@ -633,7 +630,7 @@ static void get_os_info_64(kdd_state *s)
if (val == MZ_HEADER) // MZ
break;
- base -= PAGE_SIZE;
+ base -= KDD_PAGE_SIZE;
search_limit -= 1;
}
@@ -720,7 +717,7 @@ static void find_os(kdd_state *s)
/* Try each page in the potential range of kernel load addresses */
for (limit = s->os.base + s->os.range;
s->os.base <= limit;
- s->os.base += PAGE_SIZE)
+ s->os.base += KDD_PAGE_SIZE)
if (check_os(s))
return;
}
diff --git a/tools/debugger/kdd/kdd.h b/tools/debugger/kdd/kdd.h
index b9a17440df..b476a76d93 100644
--- a/tools/debugger/kdd/kdd.h
+++ b/tools/debugger/kdd/kdd.h
@@ -39,6 +39,13 @@
#define PACKED __attribute__((packed))
+/* We define our page related constants here in order to specifically
+ * avoid using the Xen page macros (this is a restriction for the code
+ * in kdd.c which should not include any Xen headers) and to add
+ * consistency for code in both kdd.c and kdd-xen.c. */
+#define KDD_PAGE_SHIFT 12
+#define KDD_PAGE_SIZE (1U << KDD_PAGE_SHIFT)
+
/*****************************************************************************
* Serial line protocol: Sender sends a 16-byte header with an optional
* payload following it. Receiver responds to each packet with an
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v4 2/5] tools/libfsimage: Fix PATH_MAX redefinition error
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
2021-06-08 12:35 ` [PATCH v4 1/5] tools/debugger: Fix PAGE_SIZE redefinition error Costin Lupu
@ 2021-06-08 12:35 ` Costin Lupu
2021-06-08 12:35 ` [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE " Costin Lupu
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Costin Lupu @ 2021-06-08 12:35 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu, Julien Grall
If PATH_MAX is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror.
Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
tools/libfsimage/ext2fs/fsys_ext2fs.c | 2 ++
tools/libfsimage/reiserfs/fsys_reiserfs.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/tools/libfsimage/ext2fs/fsys_ext2fs.c b/tools/libfsimage/ext2fs/fsys_ext2fs.c
index a4ed10419c..5ed8fce90e 100644
--- a/tools/libfsimage/ext2fs/fsys_ext2fs.c
+++ b/tools/libfsimage/ext2fs/fsys_ext2fs.c
@@ -278,7 +278,9 @@ struct ext4_extent_header {
#define EXT2_SUPER_MAGIC 0xEF53 /* include/linux/ext2_fs.h */
#define EXT2_ROOT_INO 2 /* include/linux/ext2_fs.h */
+#ifndef PATH_MAX
#define PATH_MAX 1024 /* include/linux/limits.h */
+#endif
#define MAX_LINK_COUNT 5 /* number of symbolic links to follow */
/* made up, these are pointers into FSYS_BUF */
diff --git a/tools/libfsimage/reiserfs/fsys_reiserfs.c b/tools/libfsimage/reiserfs/fsys_reiserfs.c
index 916eb15292..10ca657476 100644
--- a/tools/libfsimage/reiserfs/fsys_reiserfs.c
+++ b/tools/libfsimage/reiserfs/fsys_reiserfs.c
@@ -284,7 +284,9 @@ struct reiserfs_de_head
#define S_ISDIR(mode) (((mode) & 0170000) == 0040000)
#define S_ISLNK(mode) (((mode) & 0170000) == 0120000)
+#ifndef PATH_MAX
#define PATH_MAX 1024 /* include/linux/limits.h */
+#endif
#define MAX_LINK_COUNT 5 /* number of symbolic links to follow */
/* The size of the node cache */
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
2021-06-08 12:35 ` [PATCH v4 1/5] tools/debugger: Fix PAGE_SIZE redefinition error Costin Lupu
2021-06-08 12:35 ` [PATCH v4 2/5] tools/libfsimage: Fix PATH_MAX " Costin Lupu
@ 2021-06-08 12:35 ` Costin Lupu
2021-07-08 17:30 ` Julien Grall
2021-07-13 6:47 ` Jan Beulich
2021-06-08 12:35 ` [PATCH v4 4/5] tools/libs/gnttab: " Costin Lupu
` (3 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: Costin Lupu @ 2021-06-08 12:35 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu
If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror. This
patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
confusion between control domain page granularity (PAGE_* definitions) and
guest domain page granularity.
The exception is in osdep_xenforeignmemory_map() where we need the system page
size to check whether the PFN array should be allocated with mmap() or with
dynamic allocation.
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 | 23 ++++++++++++-----------
tools/libs/foreignmemory/minios.c | 2 +-
tools/libs/foreignmemory/netbsd.c | 10 +++++-----
tools/libs/foreignmemory/private.h | 9 +--------
6 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 28ec311af1..7edc6f0dbf 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 << PAGE_SHIFT;
+ *size = fres.nr_frames << XC_PAGE_SHIFT;
return 0;
}
diff --git a/tools/libs/foreignmemory/freebsd.c b/tools/libs/foreignmemory/freebsd.c
index d94ea07862..2cf0fa1c38 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 << PAGE_SHIFT, prot, flags | MAP_SHARED, fd, 0);
+ addr = mmap(addr, num << XC_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 << PAGE_SHIFT);
+ (void)munmap(addr, num << XC_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 << PAGE_SHIFT);
+ return munmap(addr, num << XC_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 << PAGE_SHIFT) : 0;
+ return fres ? munmap(fres->addr, fres->nr_frames << XC_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 << PAGE_SHIFT,
+ fres->addr = mmap(fres->addr, fres->nr_frames << XC_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 c1f35e2db7..9062117407 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<<PAGE_SHIFT);
+ ioctlx.addr = (unsigned long)addr + (i<<XC_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 << PAGE_SHIFT, prot, flags | MAP_SHARED,
+ addr = mmap(addr, num << XC_PAGE_SHIFT, prot, flags | MAP_SHARED,
fd, 0);
if ( addr == MAP_FAILED )
return NULL;
@@ -198,9 +198,10 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
*/
privcmd_mmapbatch_t ioctlx;
xen_pfn_t *pfn;
- unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), PAGE_SHIFT);
+ unsigned int pfn_arr_size = ROUNDUP((num * sizeof(*pfn)), XC_PAGE_SHIFT);
+ int os_page_size = sysconf(_SC_PAGESIZE);
- if ( pfn_arr_size <= PAGE_SIZE )
+ if ( pfn_arr_size <= os_page_size )
pfn = alloca(num * sizeof(*pfn));
else
{
@@ -209,7 +210,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
if ( pfn == MAP_FAILED )
{
PERROR("mmap of pfn array failed");
- (void)munmap(addr, num << PAGE_SHIFT);
+ (void)munmap(addr, num << XC_PAGE_SHIFT);
return NULL;
}
}
@@ -242,7 +243,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
continue;
}
rc = map_foreign_batch_single(fd, dom, pfn + i,
- (unsigned long)addr + (i<<PAGE_SHIFT));
+ (unsigned long)addr + (i<<XC_PAGE_SHIFT));
if ( rc < 0 )
{
rc = -errno;
@@ -254,7 +255,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
break;
}
- if ( pfn_arr_size > PAGE_SIZE )
+ if ( pfn_arr_size > os_page_size )
munmap(pfn, pfn_arr_size);
if ( rc == -ENOENT && i == num )
@@ -270,7 +271,7 @@ void *osdep_xenforeignmemory_map(xenforeignmemory_handle *fmem,
{
int saved_errno = errno;
- (void)munmap(addr, num << PAGE_SHIFT);
+ (void)munmap(addr, num << XC_PAGE_SHIFT);
errno = saved_errno;
return NULL;
}
@@ -281,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 << PAGE_SHIFT);
+ return munmap(addr, num << XC_PAGE_SHIFT);
}
int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem,
@@ -293,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 << PAGE_SHIFT) : 0;
+ return fres ? munmap(fres->addr, fres->nr_frames << XC_PAGE_SHIFT) : 0;
}
int osdep_xenforeignmemory_map_resource(
@@ -312,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 << PAGE_SHIFT,
+ fres->addr = mmap(fres->addr, fres->nr_frames << XC_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 43341ca301..c5453736d5 100644
--- a/tools/libs/foreignmemory/minios.c
+++ b/tools/libs/foreignmemory/minios.c
@@ -55,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 << PAGE_SHIFT);
+ return munmap(addr, num << XC_PAGE_SHIFT);
}
/*
diff --git a/tools/libs/foreignmemory/netbsd.c b/tools/libs/foreignmemory/netbsd.c
index c0b1b8f79d..597db775d7 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 * PAGE_SIZE, prot,
+ addr = mmap(addr, num * XC_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 * PAGE_SIZE);
+ munmap(addr, num * XC_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 * PAGE_SIZE);
+ return munmap(addr, num * XC_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 << PAGE_SHIFT) : 0;
+ return fres ? munmap(fres->addr, fres->nr_frames << XC_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 << PAGE_SHIFT,
+ fres->addr = mmap(fres->addr, fres->nr_frames << XC_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 1ee3626dd2..65fe77aa5b 100644
--- a/tools/libs/foreignmemory/private.h
+++ b/tools/libs/foreignmemory/private.h
@@ -1,6 +1,7 @@
#ifndef XENFOREIGNMEMORY_PRIVATE_H
#define XENFOREIGNMEMORY_PRIVATE_H
+#include <xenctrl.h>
#include <xentoollog.h>
#include <xenforeignmemory.h>
@@ -10,14 +11,6 @@
#include <xen/xen.h>
#include <xen/sys/privcmd.h>
-#ifndef PAGE_SHIFT /* Mini-os, Yukk */
-#define PAGE_SHIFT 12
-#endif
-#ifndef __MINIOS__ /* Yukk */
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-#endif
-
struct xenforeignmemory_handle {
xentoollog_logger *logger, *logger_tofree;
unsigned flags;
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 ` [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE " Costin Lupu
@ 2021-07-08 17:30 ` Julien Grall
2021-07-13 6:47 ` Jan Beulich
1 sibling, 0 replies; 15+ messages in thread
From: Julien Grall @ 2021-07-08 17:30 UTC (permalink / raw)
To: Costin Lupu, xen-devel; +Cc: Ian Jackson, Wei Liu
Hi Costin,
On 08/06/2021 13:35, Costin Lupu wrote:
> If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
> header) then gcc will trigger a redefinition error because of -Werror. This
> patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
> confusion between control domain page granularity (PAGE_* definitions) and
> guest domain page granularity.
>
> The exception is in osdep_xenforeignmemory_map() where we need the system page
> size to check whether the PFN array should be allocated with mmap() or with
> dynamic allocation.
>
> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 ` [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE " Costin Lupu
2021-07-08 17:30 ` Julien Grall
@ 2021-07-13 6:47 ` Jan Beulich
2021-07-13 11:51 ` Costin Lupu
1 sibling, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2021-07-13 6:47 UTC (permalink / raw)
To: Costin Lupu, Ian Jackson, Wei Liu; +Cc: xen-devel
On 08.06.2021 14:35, Costin Lupu wrote:
> --- a/tools/libs/foreignmemory/private.h
> +++ b/tools/libs/foreignmemory/private.h
> @@ -1,6 +1,7 @@
> #ifndef XENFOREIGNMEMORY_PRIVATE_H
> #define XENFOREIGNMEMORY_PRIVATE_H
>
> +#include <xenctrl.h>
> #include <xentoollog.h>
>
> #include <xenforeignmemory.h>
At the risk of repeating what may have been discussed on irc already yesterday
(which I would not have seen), this is the cause for the present smoke test
failure:
In file included from /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:39,
from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
from private.h:4,
from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/xen/include/public/memory.h:407:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(const_uint8) buffer;
^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
from private.h:4,
from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:101:34: error: field 'arch' has incomplete type
struct xen_arch_domainconfig arch;
^~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:152:34: error: field 'arch_config' has incomplete type
struct xen_arch_domainconfig arch_config;
^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:182:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_pfn_t) array;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:263:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint8) dirty_bitmap;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:280:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(vcpu_guest_context_t) ctxt; /* IN/OUT */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:301:26: error: field 'nodemap' has incomplete type
struct xenctl_bitmap nodemap;/* IN */
^~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:337:26: error: field 'cpumap_hard' has incomplete type
struct xenctl_bitmap cpumap_hard;
^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:338:26: error: field 'cpumap_soft' has incomplete type
struct xenctl_bitmap cpumap_soft;
^~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:418:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:473:5: error: unknown type name 'int64_aligned_t'
int64_aligned_t time_offset_seconds; /* applied to domain wallclock time */
^~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:480:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint8) buffer; /* IN/OUT: data, or call
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:533:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:544:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint32) sdev_array; /* OUT */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:685:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:735:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint8) buffer; /* OUT: buffer to write record into */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:909:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint64) buffer;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:963:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_domctl_vcpu_msr_t) msrs; /* IN/OUT */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:984:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint) vdistance;
^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:38,
from private.h:4,
from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:56:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(char) buffer;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:73:26: error: field 'cpu_mask' has incomplete type
struct xenctl_bitmap cpu_mask;
^~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:155:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:165:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_domctl_getdomaininfo_t) buffer;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:174:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(const_char) keys;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:188:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_cpuinfo_t) info;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:217:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint64) trans_pt; /* Px transition table */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:225:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint64) triggers; /* Cx trigger counts */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:317:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint32) affected_cpus;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:474:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_lockprof_data_t) data;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:504:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_cputopo_t) cputopo;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:537:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_meminfo_t) meminfo;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:563:26: error: field 'cpumap' has incomplete type
struct xenctl_bitmap cpumap; /* OUT: IF */
^~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:665:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:707:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:738:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(physdev_pci_device_t) devs;
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:814:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint32) features; /* OUT: */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:887:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(char) name; /* IN: pointer to name. */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:912:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(uint8) payload; /* IN, the ELF file. */
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:975:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status; /* OUT. Must have enough
^~~~~~~~~~~~~~~~~~~
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:1059:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */
^~~~~~~~~~~~~~~~~~~
In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:55,
from private.h:4,
from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/arch-x86/xen-mca.h:431:5: error: unknown type name 'xenctl_bitmap_t'
xenctl_bitmap_t cpumap;
^~~~~~~~~~~~~~~
In file included from private.h:4,
from minios.c:29:
/home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:468:34: error: field 'arch_config' has incomplete type
struct xen_arch_domainconfig arch_config;
^~~~~~~~~~~
Clearly xenctrl.h cannot be included freely right now; it expects other
header to have been included first. Question is whether that's what needs
fixing, or whether the new #include wants prefixing by whatever prereq
headers that are needed. Or whether, considering that libxenforeignmemory.so
doesn't depend on libxc.so, including xenctrl.h is inappropriate here in the
first place, meaning that the tool stack's PAGE_SIZE abstraction may want to
move to a separate header which is not tied to any particular library.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
2021-07-13 6:47 ` Jan Beulich
@ 2021-07-13 11:51 ` Costin Lupu
0 siblings, 0 replies; 15+ messages in thread
From: Costin Lupu @ 2021-07-13 11:51 UTC (permalink / raw)
To: Jan Beulich, Ian Jackson, Wei Liu; +Cc: xen-devel, julien Grall
Hi Jan,
First of all sorry for breaking the stubdom build. Please see inline.
On 7/13/21 9:47 AM, Jan Beulich wrote:
> On 08.06.2021 14:35, Costin Lupu wrote:
>> --- a/tools/libs/foreignmemory/private.h
>> +++ b/tools/libs/foreignmemory/private.h
>> @@ -1,6 +1,7 @@
>> #ifndef XENFOREIGNMEMORY_PRIVATE_H
>> #define XENFOREIGNMEMORY_PRIVATE_H
>>
>> +#include <xenctrl.h>
>> #include <xentoollog.h>
>>
>> #include <xenforeignmemory.h>
>
> At the risk of repeating what may have been discussed on irc already yesterday
> (which I would not have seen), this is the cause for the present smoke test
> failure:
>
> In file included from /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:39,
> from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
> from private.h:4,
> from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/xen/include/public/memory.h:407:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(const_uint8) buffer;
> ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:36,
> from private.h:4,
> from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:101:34: error: field 'arch' has incomplete type
> struct xen_arch_domainconfig arch;
> ^~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:152:34: error: field 'arch_config' has incomplete type
> struct xen_arch_domainconfig arch_config;
> ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:182:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_pfn_t) array;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:263:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint8) dirty_bitmap;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:280:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(vcpu_guest_context_t) ctxt; /* IN/OUT */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:301:26: error: field 'nodemap' has incomplete type
> struct xenctl_bitmap nodemap;/* IN */
> ^~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:337:26: error: field 'cpumap_hard' has incomplete type
> struct xenctl_bitmap cpumap_hard;
> ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:338:26: error: field 'cpumap_soft' has incomplete type
> struct xenctl_bitmap cpumap_soft;
> ^~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:418:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_domctl_schedparam_vcpu_t) vcpus;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:473:5: error: unknown type name 'int64_aligned_t'
> int64_aligned_t time_offset_seconds; /* applied to domain wallclock time */
> ^~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:480:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint8) buffer; /* IN/OUT: data, or call
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:533:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:544:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint32) sdev_array; /* OUT */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:685:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* IN/OUT */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:735:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint8) buffer; /* OUT: buffer to write record into */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:909:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint64) buffer;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:963:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_domctl_vcpu_msr_t) msrs; /* IN/OUT */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/domctl.h:984:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint) vdistance;
> ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:38,
> from private.h:4,
> from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:56:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(char) buffer;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:73:26: error: field 'cpu_mask' has incomplete type
> struct xenctl_bitmap cpu_mask;
> ^~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:155:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:165:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_domctl_getdomaininfo_t) buffer;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:174:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(const_char) keys;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:188:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_cpuinfo_t) info;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:217:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint64) trans_pt; /* Px transition table */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:225:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint64) triggers; /* Cx trigger counts */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:317:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint32) affected_cpus;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:474:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_lockprof_data_t) data;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:504:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_cputopo_t) cputopo;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:537:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_meminfo_t) meminfo;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:563:26: error: field 'cpumap' has incomplete type
> struct xenctl_bitmap cpumap; /* OUT: IF */
> ^~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:665:13: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_sysctl_arinc653_schedule_t) schedule;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:707:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:738:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(physdev_pci_device_t) devs;
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:814:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint32) features; /* OUT: */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:887:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(char) name; /* IN: pointer to name. */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:912:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(uint8) payload; /* IN, the ELF file. */
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:975:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status; /* OUT. Must have enough
> ^~~~~~~~~~~~~~~~~~~
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/sysctl.h:1059:5: error: expected specifier-qualifier-list before 'XEN_GUEST_HANDLE_64'
> XEN_GUEST_HANDLE_64(xen_cpuid_leaf_t) cpuid_policy; /* OUT */
> ^~~~~~~~~~~~~~~~~~~
> In file included from /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:55,
> from private.h:4,
> from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/stubdom/include/xen/arch-x86/xen-mca.h:431:5: error: unknown type name 'xenctl_bitmap_t'
> xenctl_bitmap_t cpumap;
> ^~~~~~~~~~~~~~~
> In file included from private.h:4,
> from minios.c:29:
> /home/osstest/build.163627.build-amd64/xen/tools/include/xenctrl.h:468:34: error: field 'arch_config' has incomplete type
> struct xen_arch_domainconfig arch_config;
> ^~~~~~~~~~~
>
> Clearly xenctrl.h cannot be included freely right now; it expects other
> header to have been included first. Question is whether that's what needs
> fixing, or whether the new #include wants prefixing by whatever prereq
> headers that are needed. Or whether, considering that libxenforeignmemory.so
> doesn't depend on libxc.so, including xenctrl.h is inappropriate here in the
> first place, meaning that the tool stack's PAGE_SIZE abstraction may want to
> move to a separate header which is not tied to any particular library.
>
XEN_GUEST_HANDLE_64 is defined in xen.h. The xenctrl.h header does
include xen.h before including memory.h, where XEN_GUEST_HANDLE_64 is
used. However, xen.h is also included before that inclusion by
mini-os/os.h (included by minios.c) without defining __XEN_TOOLS__ and
thus disabling the definition of XEN_GUEST_HANDLE_64 from xen.h.
Although moving the PAGE_SIZE abstraction definitions in a header of its
own would fix this build issue, we would still have the problem
described above, i.e. we include xen.h without defining __XEN_TOOLS__,
but we would need toolstack definitions. This is a bit unclear for me,
shouldn't __XEN_TOOLS__ be defined for a stubdom?
Having said that, I can create a new header for the toolstack PAGE_SIZE
abstraction, but I would need some name suggestions. Would
xenctrl_page.h be ok? Would we keep the XC_PAGE_* names if we consider
that they will be toolstack specific (and not tied to xenctrl lib)?.
Cheers,
Costin
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
` (2 preceding siblings ...)
2021-06-08 12:35 ` [PATCH v4 3/5] tools/libs/foreignmemory: Fix PAGE_SIZE " Costin Lupu
@ 2021-06-08 12:35 ` Costin Lupu
2021-07-08 17:33 ` Julien Grall
2021-06-08 12:35 ` [PATCH v4 5/5] tools/ocaml: Fix redefinition errors Costin Lupu
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Costin Lupu @ 2021-06-08 12:35 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu
If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror. This
patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
confusion between control domain page granularity (PAGE_* definitions) and
guest domain page granularity.
The exception is in osdep_xenforeignmemory_map() where we need the system page
size to check whether the PFN array should be allocated with mmap() or with
dynamic allocation.
Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
---
tools/libs/gnttab/freebsd.c | 28 +++++++++++++---------------
tools/libs/gnttab/linux.c | 28 +++++++++++++---------------
tools/libs/gnttab/netbsd.c | 23 ++++++++++-------------
3 files changed, 36 insertions(+), 43 deletions(-)
diff --git a/tools/libs/gnttab/freebsd.c b/tools/libs/gnttab/freebsd.c
index 768af701c6..e42ac3fbf3 100644
--- a/tools/libs/gnttab/freebsd.c
+++ b/tools/libs/gnttab/freebsd.c
@@ -30,14 +30,11 @@
#include <xen/sys/gntdev.h>
+#include <xenctrl.h>
#include <xen-tools/libs.h>
#include "private.h"
-#define PAGE_SHIFT 12
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-
#define DEVXEN "/dev/xen/gntdev"
int osdep_gnttab_open(xengnttab_handle *xgt)
@@ -77,10 +74,11 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
int domids_stride;
unsigned int refs_size = ROUNDUP(count *
sizeof(struct ioctl_gntdev_grant_ref),
- PAGE_SHIFT);
+ XC_PAGE_SHIFT);
+ int os_page_size = getpagesize();
domids_stride = (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN) ? 0 : 1;
- if ( refs_size <= PAGE_SIZE )
+ if ( refs_size <= os_page_size )
map.refs = malloc(refs_size);
else
{
@@ -107,7 +105,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
goto out;
}
- addr = mmap(NULL, PAGE_SIZE * count, prot, MAP_SHARED, fd,
+ addr = mmap(NULL, XC_PAGE_SIZE * count, prot, MAP_SHARED, fd,
map.index);
if ( addr != MAP_FAILED )
{
@@ -116,7 +114,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
notify.index = map.index;
notify.action = 0;
- if ( notify_offset < PAGE_SIZE * count )
+ if ( notify_offset < XC_PAGE_SIZE * count )
{
notify.index += notify_offset;
notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -131,7 +129,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
if ( rv )
{
GTERROR(xgt->logger, "ioctl SET_UNMAP_NOTIFY failed");
- munmap(addr, count * PAGE_SIZE);
+ munmap(addr, count * XC_PAGE_SIZE);
addr = MAP_FAILED;
}
}
@@ -150,7 +148,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
}
out:
- if ( refs_size > PAGE_SIZE )
+ if ( refs_size > os_page_size )
munmap(map.refs, refs_size);
else
free(map.refs);
@@ -189,7 +187,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
}
/* Next, unmap the memory. */
- if ( (rc = munmap(start_address, count * PAGE_SIZE)) )
+ if ( (rc = munmap(start_address, count * XC_PAGE_SIZE)) )
return rc;
/* Finally, unmap the driver slots used to store the grant information. */
@@ -256,7 +254,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
goto out;
}
- area = mmap(NULL, count * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
+ area = mmap(NULL, count * XC_PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
fd, gref_info.index);
if ( area == MAP_FAILED )
@@ -268,7 +266,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
notify.index = gref_info.index;
notify.action = 0;
- if ( notify_offset < PAGE_SIZE * count )
+ if ( notify_offset < XC_PAGE_SIZE * count )
{
notify.index += notify_offset;
notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -283,7 +281,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
if ( err )
{
GSERROR(xgs->logger, "ioctl SET_UNMAP_NOTIFY failed");
- munmap(area, count * PAGE_SIZE);
+ munmap(area, count * XC_PAGE_SIZE);
area = NULL;
}
@@ -306,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 * PAGE_SIZE);
+ return munmap(start_address, count * XC_PAGE_SIZE);
}
/*
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
index 74331a4c7b..5628fd5719 100644
--- a/tools/libs/gnttab/linux.c
+++ b/tools/libs/gnttab/linux.c
@@ -32,14 +32,11 @@
#include <xen/sys/gntdev.h>
#include <xen/sys/gntalloc.h>
+#include <xenctrl.h>
#include <xen-tools/libs.h>
#include "private.h"
-#define PAGE_SHIFT 12
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-
#define DEVXEN "/dev/xen/"
#ifndef O_CLOEXEC
@@ -92,6 +89,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
int fd = xgt->fd;
struct ioctl_gntdev_map_grant_ref *map;
unsigned int map_size = sizeof(*map) + (count - 1) * sizeof(map->refs[0]);
+ int os_page_size = sysconf(_SC_PAGESIZE);
void *addr = NULL;
int domids_stride = 1;
int i;
@@ -99,11 +97,11 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
if (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN)
domids_stride = 0;
- if ( map_size <= PAGE_SIZE )
+ if ( map_size <= os_page_size )
map = alloca(map_size);
else
{
- map_size = ROUNDUP(map_size, PAGE_SHIFT);
+ map_size = ROUNDUP(map_size, XC_PAGE_SHIFT);
map = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0);
if ( map == MAP_FAILED )
@@ -127,7 +125,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
}
retry:
- addr = mmap(NULL, PAGE_SIZE * count, prot, MAP_SHARED, fd,
+ addr = mmap(NULL, XC_PAGE_SIZE * count, prot, MAP_SHARED, fd,
map->index);
if (addr == MAP_FAILED && errno == EAGAIN)
@@ -152,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 < PAGE_SIZE * count) {
+ if (notify_offset < XC_PAGE_SIZE * count) {
notify.index += notify_offset;
notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
}
@@ -164,7 +162,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
rv = ioctl(fd, IOCTL_GNTDEV_SET_UNMAP_NOTIFY, ¬ify);
if (rv) {
GTERROR(xgt->logger, "ioctl SET_UNMAP_NOTIFY failed");
- munmap(addr, count * PAGE_SIZE);
+ munmap(addr, count * XC_PAGE_SIZE);
addr = MAP_FAILED;
}
}
@@ -184,7 +182,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
}
out:
- if ( map_size > PAGE_SIZE )
+ if ( map_size > os_page_size )
munmap(map, map_size);
return addr;
@@ -220,7 +218,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
}
/* Next, unmap the memory. */
- if ( (rc = munmap(start_address, count * PAGE_SIZE)) )
+ if ( (rc = munmap(start_address, count * XC_PAGE_SIZE)) )
return rc;
/* Finally, unmap the driver slots used to store the grant information. */
@@ -466,7 +464,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
goto out;
}
- area = mmap(NULL, count * PAGE_SIZE, PROT_READ | PROT_WRITE,
+ area = mmap(NULL, count * XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, gref_info->index);
if (area == MAP_FAILED) {
@@ -477,7 +475,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
notify.index = gref_info->index;
notify.action = 0;
- if (notify_offset < PAGE_SIZE * count) {
+ if (notify_offset < XC_PAGE_SIZE * count) {
notify.index += notify_offset;
notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
}
@@ -489,7 +487,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
err = ioctl(fd, IOCTL_GNTALLOC_SET_UNMAP_NOTIFY, ¬ify);
if (err) {
GSERROR(xgs->logger, "ioctl SET_UNMAP_NOTIFY failed");
- munmap(area, count * PAGE_SIZE);
+ munmap(area, count * XC_PAGE_SIZE);
area = NULL;
}
@@ -510,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 * PAGE_SIZE);
+ return munmap(start_address, count * XC_PAGE_SIZE);
}
/*
diff --git a/tools/libs/gnttab/netbsd.c b/tools/libs/gnttab/netbsd.c
index f8d7c356eb..a4ad624b54 100644
--- a/tools/libs/gnttab/netbsd.c
+++ b/tools/libs/gnttab/netbsd.c
@@ -28,15 +28,12 @@
#include <sys/ioctl.h>
#include <sys/mman.h>
+#include <xenctrl.h>
#include <xen/xen.h>
#include <xen/xenio.h>
#include "private.h"
-#define PAGE_SHIFT 12
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-
#define DEVXEN "/kern/xen/privcmd"
int osdep_gnttab_open(xengnttab_handle *xgt)
@@ -87,19 +84,19 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
}
map.count = count;
- addr = mmap(NULL, count * PAGE_SIZE,
+ addr = mmap(NULL, count * XC_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 * PAGE_SIZE);
+ munmap((void *)map.va, count * XC_PAGE_SIZE);
addr = MAP_FAILED;
}
map.va = addr;
map.notify.offset = 0;
map.notify.action = 0;
- if ( notify_offset < PAGE_SIZE * count )
+ if ( notify_offset < XC_PAGE_SIZE * count )
{
map.notify.offset = notify_offset;
map.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -115,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 * PAGE_SIZE);
+ munmap(addr, count * XC_PAGE_SIZE);
addr = MAP_FAILED;
}
@@ -136,7 +133,7 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
}
/* Next, unmap the memory. */
- rc = munmap(start_address, count * PAGE_SIZE);
+ rc = munmap(start_address, count * XC_PAGE_SIZE);
return rc;
}
@@ -187,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 * PAGE_SIZE,
+ area = mmap(NULL, count * XC_PAGE_SIZE,
PROT_READ | PROT_WRITE, MAP_ANON | MAP_SHARED, -1, 0);
if ( area == MAP_FAILED )
@@ -200,7 +197,7 @@ void *osdep_gntshr_share_pages(xengntshr_handle *xgs,
alloc.notify.offset = 0;
alloc.notify.action = 0;
- if ( notify_offset < PAGE_SIZE * count )
+ if ( notify_offset < XC_PAGE_SIZE * count )
{
alloc.notify.offset = notify_offset;
alloc.notify.action |= UNMAP_NOTIFY_CLEAR_BYTE;
@@ -215,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 * PAGE_SIZE);
+ munmap(area, count * XC_PAGE_SIZE);
area = MAP_FAILED;
goto out;
}
@@ -230,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 * PAGE_SIZE);
+ return munmap(start_address, count * XC_PAGE_SIZE);
}
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
2021-06-08 12:35 ` [PATCH v4 4/5] tools/libs/gnttab: " Costin Lupu
@ 2021-07-08 17:33 ` Julien Grall
2021-07-09 8:59 ` Costin Lupu
0 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2021-07-08 17:33 UTC (permalink / raw)
To: Costin Lupu, xen-devel; +Cc: Ian Jackson, Wei Liu
Hi Costin,
On 08/06/2021 13:35, Costin Lupu wrote:
> If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
> header) then gcc will trigger a redefinition error because of -Werror. This
> patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
> confusion between control domain page granularity (PAGE_* definitions) and
> guest domain page granularity.
>
> The exception is in osdep_xenforeignmemory_map() where we need the system page
Did you mean osdep_gnttab_grant_map?
> size to check whether the PFN array should be allocated with mmap() or with
> dynamic allocation.
>
> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
Other than the question above:
Reviewed-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
2021-07-08 17:33 ` Julien Grall
@ 2021-07-09 8:59 ` Costin Lupu
2021-07-09 9:06 ` Julien Grall
0 siblings, 1 reply; 15+ messages in thread
From: Costin Lupu @ 2021-07-09 8:59 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Ian Jackson, Wei Liu
Hi Julien,
On 7/8/21 8:33 PM, Julien Grall wrote:
> Hi Costin,
>
> On 08/06/2021 13:35, Costin Lupu wrote:
>> If PAGE_SIZE is already defined in the system (e.g. in
>> /usr/include/limits.h
>> header) then gcc will trigger a redefinition error because of -Werror.
>> This
>> patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order
>> to avoid
>> confusion between control domain page granularity (PAGE_* definitions)
>> and
>> guest domain page granularity.
>>
>> The exception is in osdep_xenforeignmemory_map() where we need the
>> system page
>
> Did you mean osdep_gnttab_grant_map?
>
Argh, yes, sorry about that. Can we fix this on upstreaming or should I
send a new version?
>> size to check whether the PFN array should be allocated with mmap() or
>> with
>> dynamic allocation.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
>
> Other than the question above:
>
> Reviewed-by: Julien Grall <jgrall@amazon.com>
>
Cheers,
Costin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
2021-07-09 8:59 ` Costin Lupu
@ 2021-07-09 9:06 ` Julien Grall
0 siblings, 0 replies; 15+ messages in thread
From: Julien Grall @ 2021-07-09 9:06 UTC (permalink / raw)
To: Costin Lupu, xen-devel; +Cc: Ian Jackson, Wei Liu
On 09/07/2021 09:59, Costin Lupu wrote:
> Hi Julien,
Hi Costin,
>
> On 7/8/21 8:33 PM, Julien Grall wrote:
>> Hi Costin,
>>
>> On 08/06/2021 13:35, Costin Lupu wrote:
>>> If PAGE_SIZE is already defined in the system (e.g. in
>>> /usr/include/limits.h
>>> header) then gcc will trigger a redefinition error because of -Werror.
>>> This
>>> patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order
>>> to avoid
>>> confusion between control domain page granularity (PAGE_* definitions)
>>> and
>>> guest domain page granularity.
>>>
>>> The exception is in osdep_xenforeignmemory_map() where we need the
>>> system page
>>
>> Did you mean osdep_gnttab_grant_map?
>>
>
> Argh, yes, sorry about that. Can we fix this on upstreaming or should I
> send a new version?
I can do it on commit.
Cheers,
>
>>> size to check whether the PFN array should be allocated with mmap() or
>>> with
>>> dynamic allocation.
>>>
>>> Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
>>
>> Other than the question above:
>>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>>
>
> Cheers,
> Costin
>
--
Julien Grall
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v4 5/5] tools/ocaml: Fix redefinition errors
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
` (3 preceding siblings ...)
2021-06-08 12:35 ` [PATCH v4 4/5] tools/libs/gnttab: " Costin Lupu
@ 2021-06-08 12:35 ` Costin Lupu
2021-06-08 13:34 ` [PATCH v4 0/5] Fix redefinition errors for toolstack libs Christian Lindig
2021-07-09 14:58 ` Julien Grall
6 siblings, 0 replies; 15+ messages in thread
From: Costin Lupu @ 2021-06-08 12:35 UTC (permalink / raw)
To: xen-devel
Cc: Christian Lindig, David Scott, Ian Jackson, Wei Liu,
Julien Grall, Dario Faggioli
If PAGE_SIZE is already defined in the system (e.g. in /usr/include/limits.h
header) then gcc will trigger a redefinition error because of -Werror. This
patch replaces usage of PAGE_* macros with XC_PAGE_* macros in order to avoid
confusion between control domain page granularity (PAGE_* definitions) and
guest domain page granularity (which is what we are dealing with here).
Same issue applies for redefinitions of Val_none and Some_val macros which
can be already define in the OCaml system headers (e.g.
/usr/lib/ocaml/caml/mlvalues.h).
Signed-off-by: Costin Lupu <costin.lupu@cs.pub.ro>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Dario Faggioli <dfaggioli@suse.com>
---
tools/ocaml/libs/xc/xenctrl_stubs.c | 10 ++++------
tools/ocaml/libs/xentoollog/xentoollog_stubs.c | 4 ++++
tools/ocaml/libs/xl/xenlight_stubs.c | 4 ++++
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c b/tools/ocaml/libs/xc/xenctrl_stubs.c
index d05d7bb30e..f9e33e599a 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -36,14 +36,12 @@
#include "mmap_stubs.h"
-#define PAGE_SHIFT 12
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-
#define _H(__h) ((xc_interface *)(__h))
#define _D(__d) ((uint32_t)Int_val(__d))
+#ifndef Val_none
#define Val_none (Val_int(0))
+#endif
#define string_of_option_array(array, index) \
((Field(array, index) == Val_none) ? NULL : String_val(Field(Field(array, index), 0)))
@@ -818,7 +816,7 @@ CAMLprim value stub_xc_domain_memory_increase_reservation(value xch,
CAMLparam3(xch, domid, mem_kb);
int retval;
- unsigned long nr_extents = ((unsigned long)(Int64_val(mem_kb))) >> (PAGE_SHIFT - 10);
+ unsigned long nr_extents = ((unsigned long)(Int64_val(mem_kb))) >> (XC_PAGE_SHIFT - 10);
uint32_t c_domid = _D(domid);
caml_enter_blocking_section();
@@ -924,7 +922,7 @@ CAMLprim value stub_pages_to_kib(value pages)
{
CAMLparam1(pages);
- CAMLreturn(caml_copy_int64(Int64_val(pages) << (PAGE_SHIFT - 10)));
+ CAMLreturn(caml_copy_int64(Int64_val(pages) << (XC_PAGE_SHIFT - 10)));
}
diff --git a/tools/ocaml/libs/xentoollog/xentoollog_stubs.c b/tools/ocaml/libs/xentoollog/xentoollog_stubs.c
index bf64b211c2..e4306a0c2f 100644
--- a/tools/ocaml/libs/xentoollog/xentoollog_stubs.c
+++ b/tools/ocaml/libs/xentoollog/xentoollog_stubs.c
@@ -53,8 +53,12 @@ static char * dup_String_val(value s)
#include "_xtl_levels.inc"
/* Option type support as per http://www.linux-nantes.org/~fmonnier/ocaml/ocaml-wrapping-c.php */
+#ifndef Val_none
#define Val_none Val_int(0)
+#endif
+#ifndef Some_val
#define Some_val(v) Field(v,0)
+#endif
static value Val_some(value v)
{
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 352a00134d..45b8af61c7 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -227,8 +227,12 @@ static value Val_string_list(libxl_string_list *c_val)
}
/* Option type support as per http://www.linux-nantes.org/~fmonnier/ocaml/ocaml-wrapping-c.php */
+#ifndef Val_none
#define Val_none Val_int(0)
+#endif
+#ifndef Some_val
#define Some_val(v) Field(v,0)
+#endif
static value Val_some(value v)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v4 0/5] Fix redefinition errors for toolstack libs
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
` (4 preceding siblings ...)
2021-06-08 12:35 ` [PATCH v4 5/5] tools/ocaml: Fix redefinition errors Costin Lupu
@ 2021-06-08 13:34 ` Christian Lindig
2021-07-09 14:58 ` Julien Grall
6 siblings, 0 replies; 15+ messages in thread
From: Christian Lindig @ 2021-06-08 13:34 UTC (permalink / raw)
To: Costin Lupu; +Cc: xen-devel
[-- Attachment #1: Type: text/plain, Size: 682 bytes --]
On 8 Jun 2021, at 13:35, Costin Lupu <costin.lupu@cs.pub.ro<mailto:costin.lupu@cs.pub.ro>> wrote:
For replication I used gcc 10.3 on an Alpine system. In order to replicate the
redefinition error for PAGE_SIZE one should install the 'fortify-headers'
package which will change the chain of included headers by indirectly including
/usr/include/limits.h where PAGE_SIZE and PATH_MAX are defined.
[..]
tools/ocaml/libs/xc/xenctrl_stubs.c | 10 +++----
.../ocaml/libs/xentoollog/xentoollog_stubs.c | 4 +++
tools/ocaml/libs/xl/xenlight_stubs.c | 4 +++
Acked-by: Christian Lindig <christian.lindig@citrix.com<mailto:christian.lindig@citrix.com>>
[-- Attachment #2: Type: text/html, Size: 1603 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4 0/5] Fix redefinition errors for toolstack libs
2021-06-08 12:35 [PATCH v4 0/5] Fix redefinition errors for toolstack libs Costin Lupu
` (5 preceding siblings ...)
2021-06-08 13:34 ` [PATCH v4 0/5] Fix redefinition errors for toolstack libs Christian Lindig
@ 2021-07-09 14:58 ` Julien Grall
2021-07-09 15:08 ` Julien Grall
6 siblings, 1 reply; 15+ messages in thread
From: Julien Grall @ 2021-07-09 14:58 UTC (permalink / raw)
To: Costin Lupu, xen-devel
Cc: Tim Deegan, Ian Jackson, Wei Liu, Christian Lindig, David Scott,
Julien Grall
Hi Costin,
On 08/06/2021 13:35, Costin Lupu wrote:
> For replication I used gcc 10.3 on an Alpine system. In order to replicate the
> redefinition error for PAGE_SIZE one should install the 'fortify-headers'
> package which will change the chain of included headers by indirectly including
> /usr/include/limits.h where PAGE_SIZE and PATH_MAX are defined.
>
> Changes since v1:
> - Use XC_PAGE_* macros instead of PAGE_* macros
>
> Changes since v2:
> - Define KDD_PAGE_* macros for changes in debugger/kdd/
>
> Changes since v3:
> - Use sysconf(_SC_PAGESIZE) instead of getpagesize()
>
> Costin Lupu (5):
> tools/debugger: Fix PAGE_SIZE redefinition error
> tools/libfsimage: Fix PATH_MAX redefinition error
> tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
> tools/libs/gnttab: Fix PAGE_SIZE redefinition error
> tools/ocaml: Fix redefinition errors
Ian gave his acked-by on IRC, so I will commit them in a bit.
Cheers,
>
> tools/debugger/kdd/kdd-xen.c | 15 ++++------
> tools/debugger/kdd/kdd.c | 19 ++++++-------
> tools/debugger/kdd/kdd.h | 7 +++++
> tools/libfsimage/ext2fs/fsys_ext2fs.c | 2 ++
> tools/libfsimage/reiserfs/fsys_reiserfs.c | 2 ++
> tools/libs/foreignmemory/core.c | 2 +-
> tools/libs/foreignmemory/freebsd.c | 10 +++----
> tools/libs/foreignmemory/linux.c | 23 +++++++--------
> tools/libs/foreignmemory/minios.c | 2 +-
> tools/libs/foreignmemory/netbsd.c | 10 +++----
> tools/libs/foreignmemory/private.h | 9 +-----
> tools/libs/gnttab/freebsd.c | 28 +++++++++----------
> tools/libs/gnttab/linux.c | 28 +++++++++----------
> tools/libs/gnttab/netbsd.c | 23 +++++++--------
> tools/ocaml/libs/xc/xenctrl_stubs.c | 10 +++----
> .../ocaml/libs/xentoollog/xentoollog_stubs.c | 4 +++
> tools/ocaml/libs/xl/xenlight_stubs.c | 4 +++
> 17 files changed, 98 insertions(+), 100 deletions(-)
>
--
Julien Grall
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v4 0/5] Fix redefinition errors for toolstack libs
2021-07-09 14:58 ` Julien Grall
@ 2021-07-09 15:08 ` Julien Grall
0 siblings, 0 replies; 15+ messages in thread
From: Julien Grall @ 2021-07-09 15:08 UTC (permalink / raw)
To: Costin Lupu, xen-devel
Cc: Tim Deegan, Ian Jackson, Wei Liu, Christian Lindig, David Scott,
Julien Grall
Hi,
On 09/07/2021 15:58, Julien Grall wrote:
> Hi Costin,
>
> On 08/06/2021 13:35, Costin Lupu wrote:
>> For replication I used gcc 10.3 on an Alpine system. In order to
>> replicate the
>> redefinition error for PAGE_SIZE one should install the 'fortify-headers'
>> package which will change the chain of included headers by indirectly
>> including
>> /usr/include/limits.h where PAGE_SIZE and PATH_MAX are defined.
>>
>> Changes since v1:
>> - Use XC_PAGE_* macros instead of PAGE_* macros
>>
>> Changes since v2:
>> - Define KDD_PAGE_* macros for changes in debugger/kdd/
>>
>> Changes since v3:
>> - Use sysconf(_SC_PAGESIZE) instead of getpagesize()
>>
>> Costin Lupu (5):
>> tools/debugger: Fix PAGE_SIZE redefinition error
>> tools/libfsimage: Fix PATH_MAX redefinition error
>> tools/libs/foreignmemory: Fix PAGE_SIZE redefinition error
>> tools/libs/gnttab: Fix PAGE_SIZE redefinition error
>> tools/ocaml: Fix redefinition errors
>
> Ian gave his acked-by on IRC, so I will commit them in a bit.
The last patch technically needed an acked-by from Christian. AFAICT, he
gave it on v1 [1] but you didn't retain it as you switch to XC_PAGE_SHIFT.
The change looks non-controversial so I went ahead with committing
because it has sit for a while on the ML and it would help building Xen
new containers we are about to add in the CI.
Cheers,
[1] 0b82dc88-fe59-3074-471f-2e36ba3b840e@cs.pub.ro
>
> Cheers,
>
>>
>> tools/debugger/kdd/kdd-xen.c | 15 ++++------
>> tools/debugger/kdd/kdd.c | 19 ++++++-------
>> tools/debugger/kdd/kdd.h | 7 +++++
>> tools/libfsimage/ext2fs/fsys_ext2fs.c | 2 ++
>> tools/libfsimage/reiserfs/fsys_reiserfs.c | 2 ++
>> tools/libs/foreignmemory/core.c | 2 +-
>> tools/libs/foreignmemory/freebsd.c | 10 +++----
>> tools/libs/foreignmemory/linux.c | 23 +++++++--------
>> tools/libs/foreignmemory/minios.c | 2 +-
>> tools/libs/foreignmemory/netbsd.c | 10 +++----
>> tools/libs/foreignmemory/private.h | 9 +-----
>> tools/libs/gnttab/freebsd.c | 28 +++++++++----------
>> tools/libs/gnttab/linux.c | 28 +++++++++----------
>> tools/libs/gnttab/netbsd.c | 23 +++++++--------
>> tools/ocaml/libs/xc/xenctrl_stubs.c | 10 +++----
>> .../ocaml/libs/xentoollog/xentoollog_stubs.c | 4 +++
>> tools/ocaml/libs/xl/xenlight_stubs.c | 4 +++
>> 17 files changed, 98 insertions(+), 100 deletions(-)
>>
>
--
Julien Grall
^ permalink raw reply [flat|nested] 15+ messages in thread