Mailing List Archive: [PATCH v6 2/4] tools: Move MB/GB() to common-macros.h

Consolidate to a single set of common macros for tools.

MB() will gain another use in libelf, so this movement makes it
available.

Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v4:
New

v5:
Add Jan's R-b & Req-b
---
tools/firmware/hvmloader/util.h | 3 ---
tools/helpers/init-xenstore-domain.c | 1 -
tools/include/xen-tools/common-macros.h | 4 ++++
tools/libs/light/libxl_internal.h | 4 ----
4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 87be213dec..14078bde1e 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -38,9 +38,6 @@ void __bug(const char *file, int line) __attribute__((noreturn));
#define BUG() __bug(__FILE__, __LINE__)
#define BUG_ON(p) do { if (p) BUG(); } while (0)

-#define MB(mb) (mb##ULL << 20)
-#define GB(gb) (gb##ULL << 30)
-
static inline int test_bit(unsigned int b, const void *p)
{
return !!(((const uint8_t *)p)[b>>3] & (1u<<(b&7)));
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index 5405842dfe..f38ba8d6b5 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -20,7 +20,6 @@
#include "init-dom-json.h"

#define LAPIC_BASE_ADDRESS 0xfee00000UL
-#define GB(x) ((uint64_t)x << 30)

static uint32_t domid = ~0;
static char *kernel;
diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index 81fba2e9f5..07aed92684 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -91,6 +91,10 @@
#define __AC(X, Y) (X ## Y)
#define _AC(X, Y) __AC(X, Y)

+/* Size macros. */
+#define MB(_mb) (_AC(_mb, ULL) << 20)
+#define GB(_gb) (_AC(_gb, ULL) << 30)
+
#define get_unaligned_t(type, ptr) ({ \
const struct { type x; } __packed *ptr_ = (typeof(ptr_))(ptr); \
ptr_->x; \
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 094d0df9b1..803dbc1a03 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -126,10 +126,6 @@
#define PVSHIM_BASENAME "xen-shim"
#define PVSHIM_CMDLINE "pv-shim console=xen,pv"

-/* Size macros. */
-#define MB(_mb) (_AC(_mb, ULL) << 20)
-#define GB(_gb) (_AC(_gb, ULL) << 30)
-
#define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))

#define LIBXL__LOGGING_ENABLED
--
2.44.0

On Wed, Mar 27, 2024 at 05:51:00PM -0400, Jason Andryuk wrote:
> Consolidate to a single set of common macros for tools.
>
> MB() will gain another use in libelf, so this movement makes it
> available.
>
> Requested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---

So, this patch fixes potential use issues with the macros in hvmloader
and init-xenstore-domain. While it's not perfect, it still better.

I'll try "MB(memory + 0)" with the different macros:

> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 87be213dec..14078bde1e 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -38,9 +38,6 @@ void __bug(const char *file, int line) __attribute__((noreturn));
> #define BUG() __bug(__FILE__, __LINE__)
> #define BUG_ON(p) do { if (p) BUG(); } while (0)
>
> -#define MB(mb) (mb##ULL << 20)
> -#define GB(gb) (gb##ULL << 30)

With this change we have this change for MB(memory + 0) when applied:
- (memory + 0ULL << 20)
+ ((memory + 0ULL) << 20)

> static inline int test_bit(unsigned int b, const void *p)
> {
> return !!(((const uint8_t *)p)[b>>3] & (1u<<(b&7)));
> diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
> index 5405842dfe..f38ba8d6b5 100644
> --- a/tools/helpers/init-xenstore-domain.c
> +++ b/tools/helpers/init-xenstore-domain.c
> @@ -20,7 +20,6 @@
> #include "init-dom-json.h"
>
> #define LAPIC_BASE_ADDRESS 0xfee00000UL
> -#define GB(x) ((uint64_t)x << 30)

With this change we have this change for GB(memory + 0) when applied:
- ((uint64_t)memory + 0 << 30)
+ ((memory + 0ULL) << 30)

So overall, patch makes things better and less duplication:
Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

--
Anthony PERARD