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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.06.2022 13:25, Wei Chen wrote:
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 2022年6月24日 18:09
>>
>> On 24.06.2022 12:05, Jan Beulich wrote:
>>> On 24.06.2022 11:49, Julien Grall wrote:
>>>>>>>>> --- 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.

I don't think that's quite right - you actually say below that we
do use it there when interacting with UEFI. There's no warning
there solely because the information isn't embedded in the object
files there, from all I can tell.

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

I don't mind #1, but I think your subsequently proposed #3 would be
the first thing to try. There may be caveats, so if that doesn't work
out I'd suggest falling back to #1. Albeit ideally the flag setting
wouldn't be moved back (it _is_ a common EFI thing, after all), but
rather Arm32 arranging for its addition to be suppressed.

Jan