[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v6 21/31] build: set XEN_BUILD_EFI earlier


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Mon, 9 Aug 2021 16:59:34 +0100
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 09 Aug 2021 16:02:30 +0000
  • Ironport-hdrordr: A9a23:QLkMuarUOv5FAAtj2w90qQEaV5opeYIsimQD101hICG8cqSj9v xGuM5rsiMc6QxhPE3I9urtBEDtexzhHNtOkO8s1NSZLWzbUQmTXeJfBOLZqlWKcUDDH6xmpM VdmsBFeaTN5DNB7foSjjPXL+od
  • Ironport-sdr: MArjyXIyB8gpVuQpVEtHmH3HcZurhbvc4H6QoFpDryHP8gob+EI8YhRGqyB531I3cSby3L3HPF /WnjfGqRhgg9tzpmRat8wR1/Frv2YLjz7kzixkv3aujO1PxMiUuCnzEgF0ReKnejHYdbkLHfaX vWNBhtlaxKC84WWaCRnOkxroJJcaJgNsC0CwqXqChB8iWfye863AIS5ULbrmSIKDAxuVj9cvGS 5Cgpl6OVYNc5uVPuP6PGBp27HwfIHifJMMVAJJkWCOXKUI36SYf7XgjH1eqT2CXEzWUFRf8vc7 VdzrRyqFoH8aI+fVGkhsTCnj
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Aug 05, 2021 at 09:27:18AM +0200, Jan Beulich wrote:
> On 01.07.2021 16:10, Anthony PERARD wrote:
> > We are going to need the variable XEN_BUILD_EFI earlier.
> > 
> > This early check is using "try-run" to allow to have a temporary
> > output file in case it is needed for $(CC) to build the *.c file.
> > 
> > The "efi/check.o" file is still needed in "arch/x86/Makefile" so the
> > check is currently duplicated.
> 
> Why is this? Can't you ...
> 
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -126,7 +126,7 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
> >  ifneq ($(efi-y),)
> >  
> >  # Check if the compiler supports the MS ABI.
> > -export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o 
> > efi/check.o 2>/dev/null && echo y)
> > +XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 
> > 2>/dev/null && echo y)
> >  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
> 
> ... use here what you ...
> 
> > --- a/xen/arch/x86/arch.mk
> > +++ b/xen/arch/x86/arch.mk
> > @@ -60,5 +60,10 @@ ifeq ($(CONFIG_UBSAN),y)
> >  $(call cc-option-add,CFLAGS_UBSAN,CC,-fno-sanitize=alignment)
> >  endif
> >  
> > +ifneq ($(CONFIG_PV_SHIM_EXCLUSIVE),y)
> > +# Check if the compiler supports the MS ABI.
> > +export XEN_BUILD_EFI := $(call try-run,$(CC) $(CFLAGS) -c 
> > arch/x86/efi/check.c -o "$$TMPO",y)
> > +endif
> 
> ... export here?

The problem with the check for EFI support is that there several step,
with a step depending on the binary produced by the previous one.

XEN_BUILD_EFI
    In addition to check "__ms_abi__" attribute is supported by $CC, the
    file "efi/check.o" is produced.
XEN_BUILD_PE
    It is using "efi/check.o" to check for PE support and produce
    "efi/check.efi".
"efi/check.efi" is also used by the Makefile for additional checks
(mkreloc).


So, if I let the duplicated check for $(XEN_BUILD_EFI) is that it felt
wrong to produce "efi/check.o" in "arch/x86/arch.mk" and then later use
it in "arch/x86/Makefile". I could maybe move the command that create
efi/check.o in the $(XEN_BUILD_PE) check, or I could try to move most of
the checks done for EFI into x86/arch.mk. Or maybe just creating the
"efi/check.o" file in x86/arch.mk and use it in x86/Makefile, with a
comment.

What do you think?

Thanks,

-- 
Anthony PERARD



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.