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

[Wei Chen <Wei.Chen@xxxxxxx>]

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

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
> >