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

Re: [XEN PATCH v8 27/47] build: grab common EFI source files in arch specific dir


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Tue, 18 Jan 2022 11:06:26 +0000
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, "Bertrand Marquis" <bertrand.marquis@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 18 Jan 2022 11:07:02 +0000
  • Ironport-data: A9a23:Skpvpao4MyXqAd8O7/WCD5dNs+peBmJZYxIvgKrLsJaIsI4StFCzt garIBmEOvzfazegfdgibNi/90ID6pbcxt4wSgI/rH9jFCkWo5uZCYyVIHmrMnLJJKUvbq7GA +byyDXkBJppJpMJjk71atANlZT4vE2xbuKU5NTsY0idfic5Dndx4f5fs7Rh2NQw2IHpW1rlV e7a+KUzBnf0g1aYDUpMg06zgEsHUCPa4W5wUvQWPJinjXeG/5UnJMt3yZKZdhMUdrJ8DO+iL 9sv+Znilo/vE7XBPfv++lrzWhVirrc/pmFigFIOM0SpqkAqSiDfTs/XnRfTAKtao2zhojx/9 DlCnZyvWQsjJaDiosoUbTRnOnhmIep86JaSdBBTseTLp6HHW37lwvEoB0AqJ4wIvO1wBAmi9 9RBdmpLNErawbvrnvTrEYGAhex6RCXvFIoZpnFnyyCfFfs8SIrPa67L+cVZzHE7gcUm8fP2O ZBHMGc2M02ojxtnC1xJL6lltdiU33jEfh5Zg1+fhoBp/D2GpOB2+Oe0a4eEEjCQfu1rmUKfq nPD7n7OKBgQP9yCyhKI6nupwOTImEvTR4Y6BLC+sPlwjzW71mEVTREbS1a/if24kVKlHcJSL VQO/SgjprR081akJvH3UgekuneCslgZUsBJDuwhwAiXz+zf5APxLmkbTBZRZdo+rsg0SDc2k FiTkLvBBzZirbmUQnK17aqPoHW5Pi19BXQZeSYOQA8B4t/iiII+lBTCSpBkCqHdpt/oHTD9x RiaoS54gK8c5fPnzI3iowqB2Wj14MGUEEhlvW07Q15J8Ct5e56pbZelx2L15MhtE6/AVkXGh mgtzp32AP81MbmBkymEQeMoFb6v5uqYPDC0vWODD6XN5Bz2pSf9INk4DCVWYR4wb51aIWOBj Fr74FsJvPdu0G2Wgbibim5bI+Aj1uDeGNvsTZg4hfIeM8EqJGdrEMyDDHN8PlwBcmBxycnT2 r/BKK5A6Er274w9k1JaoM9HgNcWKtgWnz+7eHwC503PPUCiTHCUU6wZF1CFc/o06qiJyC2Mr YoFa5fWmksDC7ejCsUyzWL1BQpVRZTcLcqnw/G7i8bZelY2cI3fI6K5LUwdl3xNwP0Oy7agE oCVUU5E0lvv7UAr2i3RAk2PnIjHBM4lxVpiZHREFQ/xhxALPNjzhI9CKcpfVeR3pYRLkK8vJ 9FYKproPxi6Ymmdk9jrRcOj/NUKmdXCrV/mAhdJlxBkLsEwHFKYq4a0FuYtnQFXZheKWQIFi +XI/mvmrVArHWyO1e7aN6CiyU2fp38YlL4gVkfEOIALKk7t7JJrO2r6ifpue5MALhDKxz270 QeKAEhH+bmR8tFtqNSZ17qZq4qJEvdlGhYIFWfs8rvrZzLR+XCuwNEcXb/QLyzdTm795I6re f5Rk6PnKPQCkVsT69h8Hr9nwLgQ/dzqo7MGnA1oEG+SNwagC696I2nA1s5K7/UfyrhcsAqwe 0SO5tgFZunZZJK7SAYcfVN3YP6C2PcYnijpwc40eEiqtjVq+LenUFlJO0XegiJqM7YoYpgux v0suZBK5lXn2AYqKNuPkgtd63+Ici4bS6wiu5wXXN3rhw4sxg0QaJDQEHarspSGatEKOUg2O D6EwqHFgu0ElEbFdnMyE1nL3PZc2stS6EwbkgdaKgTbgMfBi982wAZVoGY+QQlixxla1/5+Z zpwPEpvKKTSpzpliaCvhYx3992t0PFBxnHM9g==
  • Ironport-hdrordr: A9a23:z46oZqNLsjutOMBcTsWjsMiBIKoaSvp037BN7TEXdfU1SL39qy nKpp8mPHDP5Ar5NEtOpTniAsm9qBHnm6KdiLN5Vd3OYOCMggqVBbAnwYz+wyDxXw3Sn9QtsJ uIqpIOa+EY22IK7/rH3A==
  • Ironport-sdr: ChdYE/vdGSAz2qcyWQuw83sP9mfLWSz5EIR5TZnfeXePX8l/rjgiwijEW+5wtmDdML3C8FVEkQ m8aUjNVzCAfirI/NYszpFnsDakBkQs5eIGO+EoQpGAAz2SETJ+hAz2V3OtziNHehG+TTLfwTsn qmnhXo9K9cvT3OSgmyopfhbRRSfKwzU+cj+ANud3gEVIhyWurDP7LA5AjrlvIPXXj6gA0EFRhe RmkWHbT4gHJEAV6AsZOkTjp/9NAxlqSGVs9UIRr0r7lC0uPJ0psptkELxmH4DBZ9nyG7hRgQC2 R1HdP4Dtjt1eQww4PGtjtvnE
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Tue, Dec 21, 2021 at 02:53:49PM +0100, Jan Beulich wrote:
> On 25.11.2021 14:39, Anthony PERARD wrote:
> > Rather than preparing the efi source file, we will make the symbolic
> > link as needed from the build location.
> > 
> > The `ln` command is run every time to allow to update the link in case
> > the source tree change location.
> 
> Btw, since symlinks aren't being liked, would there be a way to make
> things work using vpath?

No, that's not possible. With "vpath = /build/xen/common/efi", make
would look at path "/build/xen/common/efi/arch/x86/efi/runtime.c" to
build "arch/x86/efi/runtime.o".

> > This patch also introduce "efi_common.mk" which allow to reuse the
> > common make instructions without having to duplicate them into each
> > arch.
> > 
> > And now that we have a list of common source file, we can start to
> > remove the links to the source files on clean.
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> > 
> > Notes:
> >     v8:
> >     - use symbolic link instead of making a copy of the source
> >     - introduce efi_common.mk
> >     - remove links to source file on clean
> >     - use -iquote for "efi.h" headers in common/efi
> > 
> >  xen/Makefile                 |  5 -----
> >  xen/arch/arm/efi/Makefile    |  4 ++--
> >  xen/arch/x86/Makefile        |  1 +
> >  xen/arch/x86/efi/Makefile    |  5 +----
> >  xen/common/efi/efi_common.mk | 12 ++++++++++++
> 
> Could I talk you into avoiding _ when - is suitable, which is the case not
> only for (non-exported) make variables, but also file names?

I'll try. I'll change this one.

> > --- a/xen/arch/arm/efi/Makefile
> > +++ b/xen/arch/arm/efi/Makefile
> > @@ -1,4 +1,4 @@
> > -CFLAGS-y += -fshort-wchar
> > +include $(srctree)/common/efi/efi_common.mk
> >  
> > -obj-y += boot.init.o pe.init.o ebmalloc.o runtime.o
> > +obj-y += $(EFIOBJ-y)
> >  obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index e8151bf4b111..eabd8d3919a4 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -79,6 +79,7 @@ endif
> >  
> >  # Allows "clean" to descend into boot/
> >  subdir- += boot
> > +subdir- += efi
> 
> This renders the comment stale - please generalize it.
> 
> Also, any reason a similar adjustment isn't needed for Arm?

Yes, there's already "obj- += efi/" on the arm side. On x86, efi/ is
only in ALL_OBJS which make.clean doesn't use.

> > --- /dev/null
> > +++ b/xen/common/efi/efi_common.mk
> > @@ -0,0 +1,12 @@
> > +EFIOBJ-y := boot.init.o pe.init.o ebmalloc.o runtime.o
> > +EFIOBJ-$(CONFIG_COMPAT) += compat.o
> > +
> > +CFLAGS-y += -fshort-wchar
> > +CFLAGS-y += -iquote $(srctree)/common/efi
> > +
> > +$(obj)/%.c: $(abs_srctree)/common/efi/%.c FORCE
> > +   $(Q)ln -nfs $< $@
> 
> Like was the case before, I think it would be better if the links were
> relative ones, at least when srctree == objtree (but ideally always).

I can give it a try.

> > +clean-files += $(patsubst %.o,%.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-))
> 
> Nit: Please be consistent (at least within a single line) about blanks
> following commas.

You mean to never put spaces after a comma because they are sometime
significant? That's the only way I know how to be consistent.

Thanks,

-- 
Anthony PERARD



 


Rackspace

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