RE: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm

[Wei Chen <Wei.Chen@xxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Wei
> Chen
> Sent: 2022年6月30日 19:25
> To: Jan Beulich <jbeulich@xxxxxxxx>; Julien Grall <julien@xxxxxxx>
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>;
> Roger Pau Monné <roger.pau@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Jiamei Xie
> <Jiamei.Xie@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: RE: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> Hi Julien and Jan,
> 
> > -----Original Message-----
> > From: Jan Beulich <jbeulich@xxxxxxxx>
> > Sent: 2022年6月24日 18:09
> > To: Julien Grall <julien@xxxxxxx>
> > Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> Bertrand
> > Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> > <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>;
> > Roger Pau Monné <roger.pau@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Jiamei Xie
> > <Jiamei.Xie@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Chen
> > <Wei.Chen@xxxxxxx>
> > Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> >
> > On 24.06.2022 12:05, Jan Beulich wrote:
> > > On 24.06.2022 11:49, Julien Grall wrote:
> > >> Hi Jan,
> > >>
> >
> > >>>>>>> --- a/xen/arch/arm/efi/Makefile
> > >>>>>>> +++ b/xen/arch/arm/efi/Makefile
> > >>>>>>> @@ -1,4 +1,12 @@
> > >>>>>>>    include $(srctree)/common/efi/efi-common.mk
> > >>>>>>>
> > >>>>>>> +ifeq ($(CONFIG_ARM_EFI),y)
> > >>>>>>>    obj-y += $(EFIOBJ-y)
> > >>>>>>>    obj-$(CONFIG_ACPI) +=  efi-dom0.init.o
> > >>>>>>> +else
> > >>>>>>> +# Add stub.o to EFIOBJ-y to re-use the clean-files in
> > >>>>>>> +# efi-common.mk. Otherwise the link of stub.c in arm/efi
> > >>>>>>> +# will not be cleaned in "make clean".
> > >>>>>>> +EFIOBJ-y += stub.o
> > >>>>>>> +obj-y += stub.o
> > >>>>>>> +endif
> > >>>>>>
> > >>>>>> This has caused
> > >>>>>>
> > >>>>>> ld: warning: arch/arm/efi/built_in.o uses 2-byte wchar_t yet the
> > output is
> > >>>>>> to use 4-byte wchar_t; use of wchar_t values across objects may
> > fail
> > >>>>>>
> > >>>>>> for the 32-bit Arm build that I keep doing every once in a while,
> > with
> > >>>>>> (if it matters) GNU ld 2.38. I guess you will want to consider
> > building
> > >>>>>> all of Xen with -fshort-wchar, or to avoid building stub.c with
> > that
> > >>>>>> option.
> > >>>>>>
> > >>>>>
> > >>>>> Thanks for pointing this out. I will try to use -fshort-wchar for
> > Arm32,
> > >>>>> if Arm maintainers agree.
> > >>>>
> > >>>> Looking at the code we don't seem to build Xen arm64 with -fshort-
> > wchar
> > >>>> (aside the EFI files). So it is not entirely clear why we would
> want
> > to
> > >>>> use -fshort-wchar for arm32.
> > >>>
> > >>> We don't use wchar_t outside of EFI code afaict. Hence to all other
> > code
> > >>> it should be benign whether -fshort-wchar is in use. So the
> suggestion
> > >>> to use the flag unilaterally on Arm32 is really just to silence the
> ld
> > >>> warning;
> > >>
> > >> Ok. This is odd. Why would ld warn on arm32 but not other arch?
> > >
> > > Arm32 embeds ABI information in a note section in each object file.
> >
> > Or a note-like one (just to avoid possible confusion); I think it's
> > ".ARM.attributes".
> >
> > Jan
> >
> > > The mismatch of the wchar_t part of this information is what causes
> > > ld to emit the warning.
> > >
> > >>> off the top of my head I can't see anything wrong with using
> > >>> the option also for Arm64 or even globally. Yet otoh we typically
> try
> > to
> > >>> not make changes for environments where they aren't really needed.
> > >>
> > >> I agree. If we need a workaround, then my preference would be to not
> > >> build stub.c with -fshort-wchar.
> > >
> > > This would need to be an Arm-special then, as on x86 it needs to be
> > built
> > > this way.
> 
> I have taken a look into this warning:
> This is because the "-fshort-wchar" flag causes GCC to generate
> code that is not binary compatible with code generated without
> that flag. Why this warning hasn't been triggered in Arm64 is
> because we don't use any wchar in Arm64 codes. We are also not
> using wchar in Arm32 codes, but Arm32 will embed ABI information
> in ".ARM.attributes" section. This section stores some object
> file attributes, like ABI version, CPU arch and etc. And wchar
> size is described in this section by "Tag_ABI_PCS_wchar_t" too.
> Tag_ABI_PCS_wchar_t is 2 for object files with "-fshort-wchar",
> but for object files without "-fshort-wchar" is 4. Arm32 GCC
> ld will check this tag, and throw above warning when it finds
> the object files have different Tag_ABI_PCS_wchar_t values.
> 
> As gnu-efi-3.0 use the GCC option "-fshort-wchar" to force wchar
> to use short integers (2 bytes) instead of integers (4 bytes).
> We can't remove this option from x86 and Arm64, because they need
> to interact with EFI firmware. So I have to options:
> 1. Remove "-fshort-wchar" from efi-common.mk and add it back by
>    x86 and arm64's EFI Makefile
> 2. Add "-no-wchar-size-warning" to Arm32's linker flags
> 

The 3rd Option is similar to Linux kernel:
Kbuild: use -fshort-wchar globally
https://patchwork.kernel.org/project/linux-kbuild/patch/20170726133655.2137437-1-arnd@xxxxxxxx/


> I personally prefer option#1, because Arm32 doesn't need to interact
> with EFI firmware, all it requires are some stub functions. And
> "-no-wchar-size-warning" may hide some warnings we should aware in
> future.
> 
> Cheers,
> Wei Chen
> 
> > >
> > > Jan
> > >