Re: [PATCH v2 2/4] x86/boot: Use header to allows inclusion of public xen.h header

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Tue, Dec 10, 2024 at 10:32 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 22.11.2024 10:33, Frediano Ziglio wrote:
> > This allows to include other headers and avoid duplicated declarations.
> >
> > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
>
> Again it's left unclear what the purpose / goal is.
>

Reduce duplication avoiding duplicate declarations. The alternative
would be to duplicate them, which was proposed already and refused as
duplication was not good.

> > --- /dev/null
> > +++ b/xen/arch/x86/include/boot/public/xen.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +/* This header allows the inclusion of public xen.h */
> > +
> > +#ifndef BOOT__PUBLIC__XEN_H
> > +#define BOOT__PUBLIC__XEN_H
> > +
> > +#if !defined(__XEN__) || defined(__XEN_TOOLS__) || __XEN__ != 1
> > +#error Unexpected defines
> > +#endif
>
> What is this to guard against? We're in the Xen tree, building Xen.
>

In include/public/arch-x86/xen.h file there are these declarations:

#if defined(__i386__)
# ifdef __XEN__
__DeFiNe__ __DECL_REG_LO8(which) uint32_t e ## which ## x
__DeFiNe__ __DECL_REG_LO16(name) union { uint32_t e ## name; }
# endif
#include "xen-x86_32.h"
# ifdef __XEN__
__UnDeF__ __DECL_REG_LO8
__UnDeF__ __DECL_REG_LO16
__DeFiNe__ __DECL_REG_LO8(which) e ## which ## x
__DeFiNe__ __DECL_REG_LO16(name) e ## name
# endif
#elif defined(__x86_64__)
#include "xen-x86_64.h"
#endif

This header allows us to include that part without compiler errors due
to __DeFiNe__ and __UnDeF__ not being C code.

> > +#include <xen/types.h>
> > +
> > +#ifdef __i386__
> > +
> > +# define __XEN_TOOLS__ 1
> > +# undef __XEN__
> > +# include <public/arch-x86/xen.h>
> > +# define __XEN__ 1
> > +# undef __XEN_TOOLS__
>
> Why would __XEN__ need un-defining and __XEN_TOOLS__ (seemingly wrongly)
> need defining? (As an aside, I don't think the expansion of either macro
> really matters. IOW I don#t see the need for the two 1-s.)
>
> Jan

The "1"-s are the default definitions, simply I define them
temporarily and then restore them.

Frediano