Re: [PATCH] x86/boot: Reinstate -nostdinc for CFLAGS_x86_32

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Sep 03, 2024 at 12:51:28PM +0100, Andrew Cooper wrote:
> On 03/09/2024 12:20 pm, Anthony PERARD wrote:
> > On Tue, Sep 03, 2024 at 11:49:40AM +0100, Andrew Cooper wrote:
> >> Most of Xen is build using -nostdinc and a fully specified include path.
> >> However, the makefile line:
> >>
> >>   $(head-bin-objs): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
> >>
> >> discards XEN_CFLAGS and replaces them with CFLAGS_x86_32.
> >>
> >> Reinstate -nostdinc, and add the arch include path to the command line.  
> >> This
> >> is a latent bug for now, but it breaks properly with subsequent include
> >> changes.
> >>
> >> Fixes: d58a509e01c4 ("build,x86: remove the need for build32.mk")
> > I disagree with this. I'm pretty sure I've check that no command line
> > have changed.
> 
> Fine, I'll drop it.
> 
> >
> > Also, this commit shows that CFLAGS was only coming from root's
> > Config.mk:
> >> diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot
> >> deleted file mode 100644
> >> index e90680cd9f..0000000000
> >> --- a/xen/arch/x86/boot/build32.mk
> >> +++ /dev/null
> >> @@ -1,40 +0,0 @@
> >> -override XEN_TARGET_ARCH=x86_32
> >> -CFLAGS =
> >> -include $(XEN_ROOT)/Config.mk
> > So, I'm pretty sure, -nostdinc was never used before. But happy to be
> > told that I've come to the wrong conclusion. (We would need to check by
> > building with an old version without this commit to be sure.)
> 
> -nostdinc was added after the fact.  Which is fine.
> 
> But as a consequence of these being unlike the rest of Xen, somehow (and
> only on FreeBSD it seems), the regular build of Xen is fine, but
> tools/firmware/xen-dir for the shim is subject to -nostdinc in a way
> that breaks cmdline.c

FWIW, it did break for me on a normal build in xen/ on FreeBSD, no
need to run it from tools/firmware/xen-dir.

> > "xen/arch/x86/boot" as it's own sets of CFLAGS, which is annoying and I
> > haven't really tried to change that. This is also why XEN_CFLAGS is been
> > discarded.
> 
> This is necessary.  We're swapping -m64 for -m32 to build these two
> objects, and that invalidates a whole bunch of other CFLAGS.

See my previous reply, I guess figuring out which of those options also
need to be dropped as a result of dropping -m64 is likely too
complicated and fragile as we add new options.

Thanks, Roger.