Re: [PATCH v3 1/5] tools/debugger: Fix PAGE_SIZE redefinition error

2021-05-11 Thread Tim Deegan
At 11:35 +0300 on 10 May (1620646515), 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 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 

Reviewed-by: Tim Deegan 

Thanks!

Tim.



[PATCH v3 1/5] tools/debugger: Fix PAGE_SIZE redefinition error

2021-05-10 Thread Costin Lupu
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 
---
 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 & 0x000ff000ULL;
 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