[PATCH v3 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error

Costin Lupu posted 5 patches 3 years, 6 months ago
There is a newer version of this series
[PATCH v3 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
Posted by Costin Lupu 3 years, 6 months ago
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).

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..9ce27bee6e 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 = getpagesize();
     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, &notify);
         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, &notify);
     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


Re: [PATCH v3 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
Posted by Julien Grall 3 years, 6 months ago
Hi Costin,

On 10/05/2021 09: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 (which is what we are dealing with here).
> 
> 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();

Same remark as for patch #4. This at least want to be explained in the 
commit message.

Cheers,

-- 
Julien Grall

Re: [PATCH v3 4/5] tools/libs/gnttab: Fix PAGE_SIZE redefinition error
Posted by Costin Lupu 3 years, 5 months ago
Hi Julien,

On 5/17/21 9:16 PM, Julien Grall wrote:
> Hi Costin,
> 
> On 10/05/2021 09: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 (which is what we are dealing with here).
>>
>> 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();
> 
> Same remark as for patch #4. This at least want to be explained in the
> commit message.

Done.


Thanks,
Costin