... rather than opencoding locally.
This involve collecting various macros scattered around Xen (min()/max()
macros from kernel.h, and _p() from lib.h) and moving them into macros.h
In reloc.c, replace ALIGN_UP() with ROUNDUP().
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Frediano Ziglio <frediano.ziglio@cloud.com>
---
xen/arch/x86/boot/cmdline.c | 4 ++++
xen/arch/x86/boot/defs.h | 19 ----------------
xen/arch/x86/boot/reloc.c | 11 +++++-----
xen/include/xen/kernel.h | 36 +-----------------------------
xen/include/xen/lib.h | 2 --
xen/include/xen/macros.h | 44 +++++++++++++++++++++++++++++++++++++
6 files changed, 55 insertions(+), 61 deletions(-)
diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c
index 28a47da7ab02..b7375d106678 100644
--- a/xen/arch/x86/boot/cmdline.c
+++ b/xen/arch/x86/boot/cmdline.c
@@ -31,6 +31,7 @@ asm (
);
#include <xen/kconfig.h>
+#include <xen/macros.h>
#include <xen/types.h>
#include "defs.h"
@@ -50,6 +51,9 @@ typedef struct __packed {
#endif
} early_boot_opts_t;
+/* Avoid pulling in all of ctypes.h for this. */
+#define tolower(c) ((c) | 0x20)
+
/*
* Space and TAB are obvious delimiters. However, I am
* adding "\n" and "\r" here too. Just in case when
diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
index cf9a80d116f3..4d519ac4f5ea 100644
--- a/xen/arch/x86/boot/defs.h
+++ b/xen/arch/x86/boot/defs.h
@@ -24,23 +24,4 @@
#define __packed __attribute__((__packed__))
#define __stdcall __attribute__((__stdcall__))
-#define ALIGN_UP(arg, align) \
- (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
-
-#define min(x,y) ({ \
- const typeof(x) _x = (x); \
- const typeof(y) _y = (y); \
- (void) (&_x == &_y); \
- _x < _y ? _x : _y; })
-
-#define max(x,y) ({ \
- const typeof(x) _x = (x); \
- const typeof(y) _y = (y); \
- (void) (&_x == &_y); \
- _x > _y ? _x : _y; })
-
-#define _p(val) ((void *)(unsigned long)(val))
-
-#define tolower(c) ((c) | 0x20)
-
#endif /* __BOOT_DEFS_H__ */
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index ac8b58b69581..eb9902d73fd9 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -26,6 +26,7 @@ asm (
" jmp reloc \n"
);
+#include <xen/macros.h>
#include <xen/types.h>
#include "defs.h"
@@ -76,7 +77,7 @@ static uint32_t alloc;
static uint32_t alloc_mem(uint32_t bytes)
{
- return alloc -= ALIGN_UP(bytes, 16);
+ return alloc -= ROUNDUP(bytes, 16);
}
static void zero_mem(uint32_t s, uint32_t bytes)
@@ -202,11 +203,11 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
zero_mem(ptr, sizeof(*mbi_out));
/* Skip Multiboot2 information fixed part. */
- ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
+ ptr = ROUNDUP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
/* Get the number of modules. */
for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
- tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+ tag = _p(ROUNDUP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
{
if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
++mbi_out->mods_count;
@@ -227,11 +228,11 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
}
/* Skip Multiboot2 information fixed part. */
- ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
+ ptr = ROUNDUP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
/* Put all needed data into mbi_out. */
for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
- tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
+ tag = _p(ROUNDUP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
{
switch ( tag->type )
{
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index bc2440b5f96e..c5b6cc977772 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -5,43 +5,9 @@
* 'kernel.h' contains some often-used function prototypes etc
*/
+#include <xen/macros.h>
#include <xen/types.h>
-/*
- * min()/max() macros that also do
- * strict type-checking.. See the
- * "unnecessary" pointer comparison.
- */
-#define min(x,y) ({ \
- const typeof(x) _x = (x); \
- const typeof(y) _y = (y); \
- (void) (&_x == &_y); \
- _x < _y ? _x : _y; })
-
-#define max(x,y) ({ \
- const typeof(x) _x = (x); \
- const typeof(y) _y = (y); \
- (void) (&_x == &_y); \
- _x > _y ? _x : _y; })
-
-/*
- * ..and if you can't take the strict
- * types, you can specify one yourself.
- *
- * Or not use min/max at all, of course.
- */
-#define min_t(type,x,y) \
- ({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
-#define max_t(type,x,y) \
- ({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
-
-/*
- * pre-processor, array size, and bit field width suitable variants;
- * please don't use in "normal" expressions.
- */
-#define MIN(x,y) ((x) < (y) ? (x) : (y))
-#define MAX(x,y) ((x) > (y) ? (x) : (y))
-
/**
* container_of - cast a member of a structure out to the containing structure
*
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 394319c81863..e884a02ee8ce 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -57,8 +57,6 @@ static inline void
debugtrace_printk(const char *fmt, ...) {}
#endif
-/* Allows us to use '%p' as general-purpose machine-word format char. */
-#define _p(_x) ((void *)(unsigned long)(_x))
extern void printk(const char *fmt, ...)
__attribute__ ((format (printf, 1, 2), cold));
diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h
index 44d723fd121a..19caaa8026ea 100644
--- a/xen/include/xen/macros.h
+++ b/xen/include/xen/macros.h
@@ -101,6 +101,50 @@
*/
#define sizeof_field(type, member) sizeof(((type *)NULL)->member)
+/* Cast an arbitrary integer to a pointer. */
+#define _p(x) ((void *)(unsigned long)(x))
+
+/*
+ * min()/max() macros that also do strict type-checking..
+ */
+#define min(x, y) \
+ ({ \
+ const typeof(x) _x = (x); \
+ const typeof(y) _y = (y); \
+ (void)(&_x == &_y); /* typecheck */ \
+ _x < _y ? _x : _y; \
+ })
+#define max(x, y) \
+ ({ \
+ const typeof(x) _x = (x); \
+ const typeof(y) _y = (y); \
+ (void)(&_x == &_y); /* typecheck */ \
+ _x > _y ? _x : _y; \
+ })
+
+/*
+ * ..and if you can't take the strict types, you can specify one yourself.
+ */
+#define min_t(type, x, y) \
+ ({ \
+ type __x = (x); \
+ type __y = (y); \
+ __x < __y ? __x: __y; \
+ })
+#define max_t(type, x, y) \
+ ({ \
+ type __x = (x); \
+ type __y = (y); \
+ __x > __y ? __x: __y; \
+ })
+
+/*
+ * pre-processor, array size, and bit field width suitable variants;
+ * please don't use in "normal" expressions.
+ */
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+#define MAX(x, y) ((x) > (y) ? (x) : (y))
+
#endif /* __ASSEMBLY__ */
#endif /* __MACROS_H__ */
--
2.39.2
On Mon, Sep 2, 2024 at 2:32 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> ... rather than opencoding locally.
>
> This involve collecting various macros scattered around Xen (min()/max()
> macros from kernel.h, and _p() from lib.h) and moving them into macros.h
>
> In reloc.c, replace ALIGN_UP() with ROUNDUP().
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Frediano Ziglio <frediano.ziglio@cloud.com>
> ---
> xen/arch/x86/boot/cmdline.c | 4 ++++
> xen/arch/x86/boot/defs.h | 19 ----------------
> xen/arch/x86/boot/reloc.c | 11 +++++-----
> xen/include/xen/kernel.h | 36 +-----------------------------
> xen/include/xen/lib.h | 2 --
> xen/include/xen/macros.h | 44 +++++++++++++++++++++++++++++++++++++
> 6 files changed, 55 insertions(+), 61 deletions(-)
>
> diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c
> index 28a47da7ab02..b7375d106678 100644
> --- a/xen/arch/x86/boot/cmdline.c
> +++ b/xen/arch/x86/boot/cmdline.c
> @@ -31,6 +31,7 @@ asm (
> );
>
> #include <xen/kconfig.h>
> +#include <xen/macros.h>
> #include <xen/types.h>
>
> #include "defs.h"
> @@ -50,6 +51,9 @@ typedef struct __packed {
> #endif
> } early_boot_opts_t;
>
> +/* Avoid pulling in all of ctypes.h for this. */
> +#define tolower(c) ((c) | 0x20)
> +
> /*
> * Space and TAB are obvious delimiters. However, I am
> * adding "\n" and "\r" here too. Just in case when
> diff --git a/xen/arch/x86/boot/defs.h b/xen/arch/x86/boot/defs.h
> index cf9a80d116f3..4d519ac4f5ea 100644
> --- a/xen/arch/x86/boot/defs.h
> +++ b/xen/arch/x86/boot/defs.h
> @@ -24,23 +24,4 @@
> #define __packed __attribute__((__packed__))
> #define __stdcall __attribute__((__stdcall__))
>
> -#define ALIGN_UP(arg, align) \
> - (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
> -
> -#define min(x,y) ({ \
> - const typeof(x) _x = (x); \
> - const typeof(y) _y = (y); \
> - (void) (&_x == &_y); \
> - _x < _y ? _x : _y; })
> -
> -#define max(x,y) ({ \
> - const typeof(x) _x = (x); \
> - const typeof(y) _y = (y); \
> - (void) (&_x == &_y); \
> - _x > _y ? _x : _y; })
> -
> -#define _p(val) ((void *)(unsigned long)(val))
> -
> -#define tolower(c) ((c) | 0x20)
> -
> #endif /* __BOOT_DEFS_H__ */
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index ac8b58b69581..eb9902d73fd9 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -26,6 +26,7 @@ asm (
> " jmp reloc \n"
> );
>
> +#include <xen/macros.h>
> #include <xen/types.h>
>
> #include "defs.h"
> @@ -76,7 +77,7 @@ static uint32_t alloc;
>
> static uint32_t alloc_mem(uint32_t bytes)
> {
> - return alloc -= ALIGN_UP(bytes, 16);
> + return alloc -= ROUNDUP(bytes, 16);
> }
>
> static void zero_mem(uint32_t s, uint32_t bytes)
> @@ -202,11 +203,11 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
> zero_mem(ptr, sizeof(*mbi_out));
>
> /* Skip Multiboot2 information fixed part. */
> - ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> + ptr = ROUNDUP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
>
> /* Get the number of modules. */
> for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
> - tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> + tag = _p(ROUNDUP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> {
> if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
> ++mbi_out->mods_count;
> @@ -227,11 +228,11 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out)
> }
>
> /* Skip Multiboot2 information fixed part. */
> - ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> + ptr = ROUNDUP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
>
> /* Put all needed data into mbi_out. */
> for ( tag = _p(ptr); (uint32_t)tag - mbi_in < mbi_fix->total_size;
> - tag = _p(ALIGN_UP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> + tag = _p(ROUNDUP((uint32_t)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> {
> switch ( tag->type )
> {
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index bc2440b5f96e..c5b6cc977772 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -5,43 +5,9 @@
> * 'kernel.h' contains some often-used function prototypes etc
> */
>
> +#include <xen/macros.h>
> #include <xen/types.h>
>
> -/*
> - * min()/max() macros that also do
> - * strict type-checking.. See the
> - * "unnecessary" pointer comparison.
> - */
> -#define min(x,y) ({ \
> - const typeof(x) _x = (x); \
> - const typeof(y) _y = (y); \
> - (void) (&_x == &_y); \
> - _x < _y ? _x : _y; })
> -
> -#define max(x,y) ({ \
> - const typeof(x) _x = (x); \
> - const typeof(y) _y = (y); \
> - (void) (&_x == &_y); \
> - _x > _y ? _x : _y; })
> -
> -/*
> - * ..and if you can't take the strict
> - * types, you can specify one yourself.
> - *
> - * Or not use min/max at all, of course.
> - */
> -#define min_t(type,x,y) \
> - ({ type __x = (x); type __y = (y); __x < __y ? __x: __y; })
> -#define max_t(type,x,y) \
> - ({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
> -
> -/*
> - * pre-processor, array size, and bit field width suitable variants;
> - * please don't use in "normal" expressions.
> - */
> -#define MIN(x,y) ((x) < (y) ? (x) : (y))
> -#define MAX(x,y) ((x) > (y) ? (x) : (y))
> -
> /**
> * container_of - cast a member of a structure out to the containing structure
> *
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 394319c81863..e884a02ee8ce 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -57,8 +57,6 @@ static inline void
> debugtrace_printk(const char *fmt, ...) {}
> #endif
>
> -/* Allows us to use '%p' as general-purpose machine-word format char. */
> -#define _p(_x) ((void *)(unsigned long)(_x))
> extern void printk(const char *fmt, ...)
> __attribute__ ((format (printf, 1, 2), cold));
>
> diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h
> index 44d723fd121a..19caaa8026ea 100644
> --- a/xen/include/xen/macros.h
> +++ b/xen/include/xen/macros.h
> @@ -101,6 +101,50 @@
> */
> #define sizeof_field(type, member) sizeof(((type *)NULL)->member)
>
> +/* Cast an arbitrary integer to a pointer. */
> +#define _p(x) ((void *)(unsigned long)(x))
Really minor, and not a regression, I personally prefer uintptr_t
instead of "unsigned long", beside portability (which is not an issue
in Xen) is more clear we are dealing with a number representing a
pointer.
> +
> +/*
> + * min()/max() macros that also do strict type-checking..
> + */
> +#define min(x, y) \
> + ({ \
> + const typeof(x) _x = (x); \
> + const typeof(y) _y = (y); \
> + (void)(&_x == &_y); /* typecheck */ \
> + _x < _y ? _x : _y; \
> + })
> +#define max(x, y) \
> + ({ \
> + const typeof(x) _x = (x); \
> + const typeof(y) _y = (y); \
> + (void)(&_x == &_y); /* typecheck */ \
> + _x > _y ? _x : _y; \
> + })
> +
> +/*
> + * ..and if you can't take the strict types, you can specify one yourself.
> + */
> +#define min_t(type, x, y) \
> + ({ \
> + type __x = (x); \
> + type __y = (y); \
> + __x < __y ? __x: __y; \
> + })
> +#define max_t(type, x, y) \
> + ({ \
> + type __x = (x); \
> + type __y = (y); \
> + __x > __y ? __x: __y; \
> + })
> +
> +/*
> + * pre-processor, array size, and bit field width suitable variants;
> + * please don't use in "normal" expressions.
> + */
> +#define MIN(x, y) ((x) < (y) ? (x) : (y))
> +#define MAX(x, y) ((x) > (y) ? (x) : (y))
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* __MACROS_H__ */
Beside the silly comment, I'm fine with the series.
Frediano
On 02/09/2024 4:47 pm, Frediano Ziglio wrote: > On Mon, Sep 2, 2024 at 2:32 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> diff --git a/xen/include/xen/macros.h b/xen/include/xen/macros.h >> index 44d723fd121a..19caaa8026ea 100644 >> --- a/xen/include/xen/macros.h >> +++ b/xen/include/xen/macros.h >> @@ -101,6 +101,50 @@ >> */ >> #define sizeof_field(type, member) sizeof(((type *)NULL)->member) >> >> +/* Cast an arbitrary integer to a pointer. */ >> +#define _p(x) ((void *)(unsigned long)(x)) > Really minor, and not a regression, I personally prefer uintptr_t > instead of "unsigned long", beside portability (which is not an issue > in Xen) is more clear we are dealing with a number representing a > pointer. This point has been brought up several times before. Yes we probably ought to be using uintptr_t, but Xen is currently consistent in its use of unsigned long for this. ~Andrew
On 02.09.2024 15:32, Andrew Cooper wrote: > ... rather than opencoding locally. > > This involve collecting various macros scattered around Xen (min()/max() > macros from kernel.h, and _p() from lib.h) and moving them into macros.h > > In reloc.c, replace ALIGN_UP() with ROUNDUP(). > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
© 2016 - 2024 Red Hat, Inc.