|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/4] x86/boot: Use <xen/macros.h>
On Mon, Sep 2, 2024 at 2:32 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> ---
> 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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |