| Message ID | 20240902133232.3302839-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xen/boot: Remove defs.h | expand |
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>
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
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__ */
... 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(-)