Not all headers can be used by 32 bit boot code.
Allows to override some headers, we don't want to mess up with
main headers as most of the code is only 64 bit so the easy stuff should
be done for 64 bit declarations.
Boot headers should be 64 bit compatibles to avoid having multiple
declarations.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index d457876659..13d4583173 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -18,7 +18,7 @@ CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
 $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
 CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
 CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
-CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
+CFLAGS_x86_32 += -I$(srctree)/arch/x86/include/boot $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
 
 # override for 32bit binaries
 $(obj32): CFLAGS_stack_boundary :=
-- 
2.34.1

On 22.11.2024 10:33, Frediano Ziglio wrote:
> Not all headers can be used by 32 bit boot code.
> Allows to override some headers, we don't want to mess up with
> main headers as most of the code is only 64 bit so the easy stuff should
> be done for 64 bit declarations.
> Boot headers should be 64 bit compatibles to avoid having multiple
> declarations.

I'm afraid that in isolation it's not clear what is intended. Boot code is
all located in a single directory. Can't we use local headers there, using
#include "...", instead of ...

> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -18,7 +18,7 @@ CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
>  $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
>  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
>  CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
> -CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
> +CFLAGS_x86_32 += -I$(srctree)/arch/x86/include/boot $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__

... introducing a arch-wide subdir, which non-boot code could easily (ab)use?

Jan

On Tue, Dec 10, 2024 at 10:14 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.11.2024 10:33, Frediano Ziglio wrote:
> > Not all headers can be used by 32 bit boot code.
> > Allows to override some headers, we don't want to mess up with
> > main headers as most of the code is only 64 bit so the easy stuff should
> > be done for 64 bit declarations.
> > Boot headers should be 64 bit compatibles to avoid having multiple
> > declarations.
>
> I'm afraid that in isolation it's not clear what is intended. Boot code is
> all located in a single directory. Can't we use local headers there, using
> #include "...", instead of ...
>

That approach was refused. Some definitions are in the headers (like
CPU features for instance) but duplicating the definitions was
rejected as a solution.
So the idea is to reuse such definitions. But, as stated in the
comment, the "x86" includes are not for x86, but most of them are just
for x64.This for historic reasons. But most of the code is x64 only so
changing headers to be x86 compatible would complicate them for a
minimal gain.

> > --- a/xen/arch/x86/boot/Makefile
> > +++ b/xen/arch/x86/boot/Makefile
> > @@ -18,7 +18,7 @@ CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
> >  $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
> >  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
> >  CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
> > -CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
> > +CFLAGS_x86_32 += -I$(srctree)/arch/x86/include/boot $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
>
> ... introducing a arch-wide subdir, which non-boot code could easily (ab)use?
>

You would have to explicitly add the "boot" into the path, it does not
seem "easy" to me, but if you really want to do it you can do with
these or any other headers, like simply "#include "../arch/arm/..." in
x64 code. Same easiness.

> Jan

Frediano

On 10.12.2024 15:29, Frediano Ziglio wrote:
> On Tue, Dec 10, 2024 at 10:14 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.11.2024 10:33, Frediano Ziglio wrote:
>>> Not all headers can be used by 32 bit boot code.
>>> Allows to override some headers, we don't want to mess up with
>>> main headers as most of the code is only 64 bit so the easy stuff should
>>> be done for 64 bit declarations.
>>> Boot headers should be 64 bit compatibles to avoid having multiple
>>> declarations.
>>
>> I'm afraid that in isolation it's not clear what is intended. Boot code is
>> all located in a single directory. Can't we use local headers there, using
>> #include "...", instead of ...
>>
> 
> That approach was refused.

Can you provide a reference please?

> Some definitions are in the headers (like
> CPU features for instance) but duplicating the definitions was
> rejected as a solution.
> So the idea is to reuse such definitions. But, as stated in the
> comment, the "x86" includes are not for x86, but most of them are just
> for x64.This for historic reasons. But most of the code is x64 only so
> changing headers to be x86 compatible would complicate them for a
> minimal gain.

Yet the amount of what wants sharing ought to be low. It might even help
to clarify what it is that is meant to become shared.

>>> --- a/xen/arch/x86/boot/Makefile
>>> +++ b/xen/arch/x86/boot/Makefile
>>> @@ -18,7 +18,7 @@ CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
>>>  $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
>>>  CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
>>>  CFLAGS_x86_32 += -nostdinc -include $(filter %/include/xen/config.h,$(XEN_CFLAGS))
>>> -CFLAGS_x86_32 += $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
>>> +CFLAGS_x86_32 += -I$(srctree)/arch/x86/include/boot $(filter -I% -O%,$(XEN_CFLAGS)) -D__XEN__
>>
>> ... introducing a arch-wide subdir, which non-boot code could easily (ab)use?
> 
> You would have to explicitly add the "boot" into the path,

No different from "hvm" or "guest", just to give two examples.

> it does not
> seem "easy" to me, but if you really want to do it you can do with
> these or any other headers, like simply "#include "../arch/arm/..." in
> x64 code. Same easiness.

Well, no, the ".." in #include directives certainly stands out.

Jan