diff mbox series

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

Message ID 20220610055316.2197571-2-wei.chen@arm.com (mailing list archive)
State New, archived
Headers show
Series Device tree based NUMA support for Arm - Part#1 | expand

Commit Message

Wei Chen June 10, 2022, 5:53 a.m. UTC
x86 is using compiler feature testing to decide EFI build
enable or not. When EFI build is disabled, x86 will use an
efi/stub.c file to replace efi/runtime.c for build objects.
Following this idea, we introduce a stub file for Arm, but
use CONFIG_ARM_EFI to decide EFI build enable or not.

And the most functions in x86 EFI stub.c can be reused for
other architectures, like Arm. So we move them to common
and keep the x86 specific function in x86/efi/stub.c.

To avoid the symbol link conflict error when linking common
stub files to x86/efi. We add a regular file check in efi
stub files' link script. Depends on this check we can bypass
the link behaviors for existed stub files in x86/efi.

As there is no Arm specific EFI stub function for Arm in
current stage, Arm still can use the existed symbol link
method for EFI stub files.

Signed-off-by: Wei Chen <wei.chen@arm.com>
Tested-by: Jiamei Xie <jiamei.xie@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
v4 -> v5:
1. Add acked-by.
v3 -> v4:
1. Add indent to make ln and test to be aligned.
v2 -> v3:
1. Add existed file check, if a regular stub files,
   the common/stub files' link will be ignored.
2. Keep stub.c in x86/efi to include common/efi/stub.c
3. Restore efi_compat_xxx stub functions to x86/efi.c.
   Other architectures will not use efi_compat_xxx.
4. Remove ARM_EFI dependency from ARM_64.
5. Add comment for adding stub.o to EFIOBJ-y.
6. Merge patch#2 and patch#3 to one patch.
v1 -> v2:
1. Drop the copy of stub.c from Arm EFI.
2. Share common codes of x86 EFI stub for other architectures.
3. Use CONFIG_ARM_EFI to replace CONFIG_EFI
4. Remove help text and make CONFIG_ARM_EFI invisible.
5. Merge one following patch:
   xen/arm: introduce a stub file for non-EFI architectures
6. Use the common stub.c instead of creating new one.
---
 xen/arch/arm/Kconfig         |  4 ++++
 xen/arch/arm/Makefile        |  2 +-
 xen/arch/arm/efi/Makefile    |  8 ++++++++
 xen/arch/x86/efi/stub.c      | 32 +-------------------------------
 xen/common/efi/efi-common.mk |  3 ++-
 xen/common/efi/stub.c        | 32 ++++++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 33 deletions(-)
 create mode 100644 xen/common/efi/stub.c

Comments

Jan Beulich June 23, 2022, 12:53 p.m. UTC | #1
On 10.06.2022 07:53, Wei Chen wrote:
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -1,6 +1,5 @@
>  obj-$(CONFIG_ARM_32) += arm32/
>  obj-$(CONFIG_ARM_64) += arm64/
> -obj-$(CONFIG_ARM_64) += efi/
>  obj-$(CONFIG_ACPI) += acpi/
>  obj-$(CONFIG_HAS_PCI) += pci/
>  ifneq ($(CONFIG_NO_PLAT),y)
> @@ -20,6 +19,7 @@ obj-y += domain.o
>  obj-y += domain_build.init.o
>  obj-y += domctl.o
>  obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> +obj-y += efi/
>  obj-y += gic.o
>  obj-y += gic-v2.o
>  obj-$(CONFIG_GICV3) += gic-v3.o
> --- 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.

Jan
Wei Chen June 24, 2022, 7:18 a.m. UTC | #2
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年6月23日 20:54
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> On 10.06.2022 07:53, Wei Chen wrote:
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -1,6 +1,5 @@
> >  obj-$(CONFIG_ARM_32) += arm32/
> >  obj-$(CONFIG_ARM_64) += arm64/
> > -obj-$(CONFIG_ARM_64) += efi/
> >  obj-$(CONFIG_ACPI) += acpi/
> >  obj-$(CONFIG_HAS_PCI) += pci/
> >  ifneq ($(CONFIG_NO_PLAT),y)
> > @@ -20,6 +19,7 @@ obj-y += domain.o
> >  obj-y += domain_build.init.o
> >  obj-y += domctl.o
> >  obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> > +obj-y += efi/
> >  obj-y += gic.o
> >  obj-y += gic-v2.o
> >  obj-$(CONFIG_GICV3) += gic-v3.o
> > --- 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.

Cheers,
Wei Chen

> Jan
Julien Grall June 24, 2022, 8:35 a.m. UTC | #3
Hi,

On 24/06/2022 08:18, Wei Chen wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年6月23日 20:54
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
>> devel@lists.xenproject.org
>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>
>> On 10.06.2022 07:53, Wei Chen wrote:
>>> --- a/xen/arch/arm/Makefile
>>> +++ b/xen/arch/arm/Makefile
>>> @@ -1,6 +1,5 @@
>>>   obj-$(CONFIG_ARM_32) += arm32/
>>>   obj-$(CONFIG_ARM_64) += arm64/
>>> -obj-$(CONFIG_ARM_64) += efi/
>>>   obj-$(CONFIG_ACPI) += acpi/
>>>   obj-$(CONFIG_HAS_PCI) += pci/
>>>   ifneq ($(CONFIG_NO_PLAT),y)
>>> @@ -20,6 +19,7 @@ obj-y += domain.o
>>>   obj-y += domain_build.init.o
>>>   obj-y += domctl.o
>>>   obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>> +obj-y += efi/
>>>   obj-y += gic.o
>>>   obj-y += gic-v2.o
>>>   obj-$(CONFIG_GICV3) += gic-v3.o
>>> --- 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.

Cheers,
Jan Beulich June 24, 2022, 9:33 a.m. UTC | #4
On 24.06.2022 10:35, Julien Grall wrote:
> On 24/06/2022 08:18, Wei Chen wrote:
>>> -----Original Message-----
>>> From: Jan Beulich <jbeulich@suse.com>
>>> Sent: 2022年6月23日 20:54
>>> To: Wei Chen <Wei.Chen@arm.com>
>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
>>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
>>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
>>> devel@lists.xenproject.org
>>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>>
>>> On 10.06.2022 07:53, Wei Chen wrote:
>>>> --- a/xen/arch/arm/Makefile
>>>> +++ b/xen/arch/arm/Makefile
>>>> @@ -1,6 +1,5 @@
>>>>   obj-$(CONFIG_ARM_32) += arm32/
>>>>   obj-$(CONFIG_ARM_64) += arm64/
>>>> -obj-$(CONFIG_ARM_64) += efi/
>>>>   obj-$(CONFIG_ACPI) += acpi/
>>>>   obj-$(CONFIG_HAS_PCI) += pci/
>>>>   ifneq ($(CONFIG_NO_PLAT),y)
>>>> @@ -20,6 +19,7 @@ obj-y += domain.o
>>>>   obj-y += domain_build.init.o
>>>>   obj-y += domctl.o
>>>>   obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>>> +obj-y += efi/
>>>>   obj-y += gic.o
>>>>   obj-y += gic-v2.o
>>>>   obj-$(CONFIG_GICV3) += gic-v3.o
>>>> --- 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; 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.

Jan
Julien Grall June 24, 2022, 9:49 a.m. UTC | #5
Hi Jan,

On 24/06/2022 10:33, Jan Beulich wrote:
> On 24.06.2022 10:35, Julien Grall wrote:
>> On 24/06/2022 08:18, Wei Chen wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 2022年6月23日 20:54
>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
>>>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>>>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
>>>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
>>>> devel@lists.xenproject.org
>>>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>>>
>>>> On 10.06.2022 07:53, Wei Chen wrote:
>>>>> --- a/xen/arch/arm/Makefile
>>>>> +++ b/xen/arch/arm/Makefile
>>>>> @@ -1,6 +1,5 @@
>>>>>    obj-$(CONFIG_ARM_32) += arm32/
>>>>>    obj-$(CONFIG_ARM_64) += arm64/
>>>>> -obj-$(CONFIG_ARM_64) += efi/
>>>>>    obj-$(CONFIG_ACPI) += acpi/
>>>>>    obj-$(CONFIG_HAS_PCI) += pci/
>>>>>    ifneq ($(CONFIG_NO_PLAT),y)
>>>>> @@ -20,6 +19,7 @@ obj-y += domain.o
>>>>>    obj-y += domain_build.init.o
>>>>>    obj-y += domctl.o
>>>>>    obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>>>> +obj-y += efi/
>>>>>    obj-y += gic.o
>>>>>    obj-y += gic-v2.o
>>>>>    obj-$(CONFIG_GICV3) += gic-v3.o
>>>>> --- 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? Wei, do 
you think you can have a look?

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

Cheers,
Jan Beulich June 24, 2022, 10:05 a.m. UTC | #6
On 24.06.2022 11:49, Julien Grall wrote:
> Hi Jan,
> 
> On 24/06/2022 10:33, Jan Beulich wrote:
>> On 24.06.2022 10:35, Julien Grall wrote:
>>> On 24/06/2022 08:18, Wei Chen wrote:
>>>>> -----Original Message-----
>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>> Sent: 2022年6月23日 20:54
>>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
>>>>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>>>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>>>>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
>>>>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
>>>>> devel@lists.xenproject.org
>>>>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>>>>
>>>>> On 10.06.2022 07:53, Wei Chen wrote:
>>>>>> --- a/xen/arch/arm/Makefile
>>>>>> +++ b/xen/arch/arm/Makefile
>>>>>> @@ -1,6 +1,5 @@
>>>>>>    obj-$(CONFIG_ARM_32) += arm32/
>>>>>>    obj-$(CONFIG_ARM_64) += arm64/
>>>>>> -obj-$(CONFIG_ARM_64) += efi/
>>>>>>    obj-$(CONFIG_ACPI) += acpi/
>>>>>>    obj-$(CONFIG_HAS_PCI) += pci/
>>>>>>    ifneq ($(CONFIG_NO_PLAT),y)
>>>>>> @@ -20,6 +19,7 @@ obj-y += domain.o
>>>>>>    obj-y += domain_build.init.o
>>>>>>    obj-y += domctl.o
>>>>>>    obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>>>>> +obj-y += efi/
>>>>>>    obj-y += gic.o
>>>>>>    obj-y += gic-v2.o
>>>>>>    obj-$(CONFIG_GICV3) += gic-v3.o
>>>>>> --- 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.
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.

Jan
Jan Beulich June 24, 2022, 10:09 a.m. UTC | #7
On 24.06.2022 12:05, Jan Beulich wrote:
> On 24.06.2022 11:49, Julien Grall wrote:
>> Hi Jan,
>>
>> On 24/06/2022 10:33, Jan Beulich wrote:
>>> On 24.06.2022 10:35, Julien Grall wrote:
>>>> On 24/06/2022 08:18, Wei Chen wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jan Beulich <jbeulich@suse.com>
>>>>>> Sent: 2022年6月23日 20:54
>>>>>> To: Wei Chen <Wei.Chen@arm.com>
>>>>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien
>>>>>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
>>>>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
>>>>>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>; Wei
>>>>>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
>>>>>> devel@lists.xenproject.org
>>>>>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>>>>>
>>>>>> On 10.06.2022 07:53, Wei Chen wrote:
>>>>>>> --- a/xen/arch/arm/Makefile
>>>>>>> +++ b/xen/arch/arm/Makefile
>>>>>>> @@ -1,6 +1,5 @@
>>>>>>>    obj-$(CONFIG_ARM_32) += arm32/
>>>>>>>    obj-$(CONFIG_ARM_64) += arm64/
>>>>>>> -obj-$(CONFIG_ARM_64) += efi/
>>>>>>>    obj-$(CONFIG_ACPI) += acpi/
>>>>>>>    obj-$(CONFIG_HAS_PCI) += pci/
>>>>>>>    ifneq ($(CONFIG_NO_PLAT),y)
>>>>>>> @@ -20,6 +19,7 @@ obj-y += domain.o
>>>>>>>    obj-y += domain_build.init.o
>>>>>>>    obj-y += domctl.o
>>>>>>>    obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
>>>>>>> +obj-y += efi/
>>>>>>>    obj-y += gic.o
>>>>>>>    obj-y += gic-v2.o
>>>>>>>    obj-$(CONFIG_GICV3) += gic-v3.o
>>>>>>> --- 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.
> 
> Jan
>
Wei Chen June 28, 2022, 8:30 a.m. UTC | #8
Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 2022年6月24日 17:50
> To: Jan Beulich <jbeulich@suse.com>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Wei Chen
> <Wei.Chen@arm.com>
> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> Hi Jan,
> 
> On 24/06/2022 10:33, Jan Beulich wrote:
> > On 24.06.2022 10:35, Julien Grall wrote:
> >> On 24/06/2022 08:18, Wei Chen wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: 2022年6月23日 20:54
> >>>> To: Wei Chen <Wei.Chen@arm.com>
> >>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Julien
> >>>> Grall <julien@xen.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> >>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> >>>> <andrew.cooper3@citrix.com>; Roger Pau Monné <roger.pau@citrix.com>;
> Wei
> >>>> Liu <wl@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
> >>>> devel@lists.xenproject.org
> >>>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> >>>>
> >>>> On 10.06.2022 07:53, Wei Chen wrote:
> >>>>> --- a/xen/arch/arm/Makefile
> >>>>> +++ b/xen/arch/arm/Makefile
> >>>>> @@ -1,6 +1,5 @@
> >>>>>    obj-$(CONFIG_ARM_32) += arm32/
> >>>>>    obj-$(CONFIG_ARM_64) += arm64/
> >>>>> -obj-$(CONFIG_ARM_64) += efi/
> >>>>>    obj-$(CONFIG_ACPI) += acpi/
> >>>>>    obj-$(CONFIG_HAS_PCI) += pci/
> >>>>>    ifneq ($(CONFIG_NO_PLAT),y)
> >>>>> @@ -20,6 +19,7 @@ obj-y += domain.o
> >>>>>    obj-y += domain_build.init.o
> >>>>>    obj-y += domctl.o
> >>>>>    obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> >>>>> +obj-y += efi/
> >>>>>    obj-y += gic.o
> >>>>>    obj-y += gic-v2.o
> >>>>>    obj-$(CONFIG_GICV3) += gic-v3.o
> >>>>> --- 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? Wei, do
> you think you can have a look?
> 

Jan has already given some answers. I'll have a look and see if there's
a better way.

Cheers,
Wei Chen

> > 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.
> 
> Cheers,
> 
> --
> Julien Grall
Wei Chen June 30, 2022, 11:25 a.m. UTC | #9
Hi Julien and Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年6月24日 18:09
> To: Julien Grall <julien@xen.org>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Wei Chen
> <Wei.Chen@arm.com>
> 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
> >
Wei Chen June 30, 2022, 11:37 a.m. UTC | #10
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Wei
> Chen
> Sent: 2022年6月30日 19:25
> To: Jan Beulich <jbeulich@suse.com>; Julien Grall <julien@xen.org>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org
> Subject: RE: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> Hi Julien and Jan,
> 
> > -----Original Message-----
> > From: Jan Beulich <jbeulich@suse.com>
> > Sent: 2022年6月24日 18:09
> > To: Julien Grall <julien@xen.org>
> > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Bertrand
> > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> > <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> > Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> > <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Wei Chen
> > <Wei.Chen@arm.com>
> > 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@arndb.de/


> 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
> > >
Jan Beulich June 30, 2022, 12:36 p.m. UTC | #11
On 30.06.2022 13:25, Wei Chen wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> 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
Wei Chen July 1, 2022, 3:11 a.m. UTC | #12
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年6月30日 20:36
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Julien Grall
> <julien@xen.org>
> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> On 30.06.2022 13:25, Wei Chen wrote:
> >> From: Jan Beulich <jbeulich@suse.com>
> >> 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.
> 

Maybe I should describe it this way: Arm64 does not use wchar type
directly in any code for parameters, variables and return values.
So Arm64 object files are exactly the same with "-fshort-wchar" and
without "-fshort-wchar".

Although Xen's EFI code interacts with UEFI firmware, similar to RPC
function calls, these code also do not explicitly use wchar. But it
is reasonable to keep -fshort-wchar for Xen's EFI code, but as long
as we use wchar in EFI code in the future, we will definitely encounter
this warning like Arm32.

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

I am OK with option#3 to set "-fshort-wchar" as a global CFLAGS. This
flag will affect wchar performance (non-4bytes-alignment), but Xen
doesn't use wchar out of EFI. So setting it as a global flag should
be harmless. It can also avoid similar warnings from appearing again.

Cheers,
Wei Chen

> Jan
Jan Beulich July 1, 2022, 5:54 a.m. UTC | #13
On 01.07.2022 05:11, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年6月30日 20:36
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
>> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
>> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
>> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
>> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Julien Grall
>> <julien@xen.org>
>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>
>> On 30.06.2022 13:25, Wei Chen wrote:
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> 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.
>>
> 
> Maybe I should describe it this way: Arm64 does not use wchar type
> directly in any code for parameters, variables and return values.
> So Arm64 object files are exactly the same with "-fshort-wchar" and
> without "-fshort-wchar".
> 
> Although Xen's EFI code interacts with UEFI firmware, similar to RPC
> function calls, these code also do not explicitly use wchar.

How does it not? There are ample string literals as well as enough
uses of CHAR16 (the UEFI "abstraction" of wchar_t).

Jan
Jan Beulich July 1, 2022, 5:57 a.m. UTC | #14
On 30.06.2022 14:36, Jan Beulich wrote:
> On 30.06.2022 13:25, Wei Chen wrote:
>>> From: Jan Beulich <jbeulich@suse.com>
>>> 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.

Or make Arm32 use -fno-short-wchar (looking at gcc code this should
be possible) to override (undo) the earlier option (possibly isolated
to just stub.c). I'd prefer that over #3, really.

Jan
Wei Chen July 1, 2022, 6:10 a.m. UTC | #15
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 2022年7月1日 13:54
> To: Wei Chen <Wei.Chen@arm.com>
> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Julien Grall
> <julien@xen.org>
> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
> 
> >>>>> 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.
> >>
> >
> > Maybe I should describe it this way: Arm64 does not use wchar type
> > directly in any code for parameters, variables and return values.
> > So Arm64 object files are exactly the same with "-fshort-wchar" and
> > without "-fshort-wchar".
> >
> > Although Xen's EFI code interacts with UEFI firmware, similar to RPC
> > function calls, these code also do not explicitly use wchar.
> 
> How does it not? There are ample string literals as well as enough
> uses of CHAR16 (the UEFI "abstraction" of wchar_t).
> 

But I don't think CHAR16 will be affected by -fshort-wchar, because we
have specified CHAR16 as unsigned short type in typedef.

I'll try the -fno-short-wchar method from your subsequent mail, if it
works, that would be the least impactful way.

Cheers,
Wei Chen

> Jan
Jan Beulich July 1, 2022, 10:07 a.m. UTC | #16
On 01.07.2022 08:10, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 2022年7月1日 13:54
>> To: Wei Chen <Wei.Chen@arm.com>
>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand
>> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
>> <Volodymyr_Babchuk@epam.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
>> Roger Pau Monné <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Jiamei Xie
>> <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org; Julien Grall
>> <julien@xen.org>
>> Subject: Re: [PATCH v6 1/8] xen: reuse x86 EFI stub functions for Arm
>>
>>>>>>> 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.
>>>>
>>>
>>> Maybe I should describe it this way: Arm64 does not use wchar type
>>> directly in any code for parameters, variables and return values.
>>> So Arm64 object files are exactly the same with "-fshort-wchar" and
>>> without "-fshort-wchar".
>>>
>>> Although Xen's EFI code interacts with UEFI firmware, similar to RPC
>>> function calls, these code also do not explicitly use wchar.
>>
>> How does it not? There are ample string literals as well as enough
>> uses of CHAR16 (the UEFI "abstraction" of wchar_t).
>>
> 
> But I don't think CHAR16 will be affected by -fshort-wchar, because we
> have specified CHAR16 as unsigned short type in typedef.

Oh, I didn't recall that anomaly. Yes, then only string literals are
affected. Albeit I'd hope gnu-efi would at some point switch to using
wchar_t as the underlying type, as long as the compiler provides it.

Jan
diff mbox series

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ecfa6822e4..8a16d43bd5 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -6,6 +6,7 @@  config ARM_64
 	def_bool y
 	depends on !ARM_32
 	select 64BIT
+	select ARM_EFI
 	select HAS_FAST_MULTIPLY
 
 config ARM
@@ -33,6 +34,9 @@  config ACPI
 	  Advanced Configuration and Power Interface (ACPI) support for Xen is
 	  an alternative to device tree on ARM64.
 
+config ARM_EFI
+	bool
+
 config GICV3
 	bool "GICv3 driver"
 	depends on ARM_64 && !NEW_VGIC
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 1d862351d1..bb7a6151c1 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -1,6 +1,5 @@ 
 obj-$(CONFIG_ARM_32) += arm32/
 obj-$(CONFIG_ARM_64) += arm64/
-obj-$(CONFIG_ARM_64) += efi/
 obj-$(CONFIG_ACPI) += acpi/
 obj-$(CONFIG_HAS_PCI) += pci/
 ifneq ($(CONFIG_NO_PLAT),y)
@@ -20,6 +19,7 @@  obj-y += domain.o
 obj-y += domain_build.init.o
 obj-y += domctl.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
+obj-y += efi/
 obj-y += gic.o
 obj-y += gic-v2.o
 obj-$(CONFIG_GICV3) += gic-v3.o
diff --git a/xen/arch/arm/efi/Makefile b/xen/arch/arm/efi/Makefile
index 4313c39066..dffe72e589 100644
--- 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
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 9984932626..f2365bc041 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -1,7 +1,5 @@ 
 #include <xen/efi.h>
-#include <xen/errno.h>
 #include <xen/init.h>
-#include <xen/lib.h>
 #include <asm/asm_defns.h>
 #include <asm/efibind.h>
 #include <asm/page.h>
@@ -10,6 +8,7 @@ 
 #include <efi/eficon.h>
 #include <efi/efidevp.h>
 #include <efi/efiapi.h>
+#include "../../../common/efi/stub.c"
 
 /*
  * Here we are in EFI stub. EFI calls are not supported due to lack
@@ -45,11 +44,6 @@  void __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
     unreachable();
 }
 
-bool efi_enabled(unsigned int feature)
-{
-    return false;
-}
-
 void __init efi_init_memory(void) { }
 
 bool efi_boot_mem_unused(unsigned long *start, unsigned long *end)
@@ -62,32 +56,8 @@  bool efi_boot_mem_unused(unsigned long *start, unsigned long *end)
 
 void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t l4e) { }
 
-bool efi_rs_using_pgtables(void)
-{
-    return false;
-}
-
-unsigned long efi_get_time(void)
-{
-    BUG();
-    return 0;
-}
-
-void efi_halt_system(void) { }
-void efi_reset_system(bool warm) { }
-
-int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
-{
-    return -ENOSYS;
-}
-
 int efi_compat_get_info(uint32_t idx, union compat_pf_efi_info *)
     __attribute__((__alias__("efi_get_info")));
 
-int efi_runtime_call(struct xenpf_efi_runtime_call *op)
-{
-    return -ENOSYS;
-}
-
 int efi_compat_runtime_call(struct compat_pf_efi_runtime_call *)
     __attribute__((__alias__("efi_runtime_call")));
diff --git a/xen/common/efi/efi-common.mk b/xen/common/efi/efi-common.mk
index 4298ceaee7..ec2c34f198 100644
--- a/xen/common/efi/efi-common.mk
+++ b/xen/common/efi/efi-common.mk
@@ -9,7 +9,8 @@  CFLAGS-y += -iquote $(srcdir)
 # e.g.: It transforms "dir/foo/bar" into successively
 #       "dir foo bar", ".. .. ..", "../../.."
 $(obj)/%.c: $(srctree)/common/efi/%.c FORCE
-	$(Q)ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/source/common/efi/$(<F) $@
+	$(Q)test -f $@ || \
+	    ln -nfs $(subst $(space),/,$(patsubst %,..,$(subst /, ,$(obj))))/source/common/efi/$(<F) $@
 
 clean-files += $(patsubst %.o, %.c, $(EFIOBJ-y:.init.o=.o) $(EFIOBJ-))
 
diff --git a/xen/common/efi/stub.c b/xen/common/efi/stub.c
new file mode 100644
index 0000000000..15694632c2
--- /dev/null
+++ b/xen/common/efi/stub.c
@@ -0,0 +1,32 @@ 
+#include <xen/efi.h>
+#include <xen/errno.h>
+#include <xen/lib.h>
+
+bool efi_enabled(unsigned int feature)
+{
+    return false;
+}
+
+bool efi_rs_using_pgtables(void)
+{
+    return false;
+}
+
+unsigned long efi_get_time(void)
+{
+    BUG();
+    return 0;
+}
+
+void efi_halt_system(void) { }
+void efi_reset_system(bool warm) { }
+
+int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
+{
+    return -ENOSYS;
+}
+
+int efi_runtime_call(struct xenpf_efi_runtime_call *op)
+{
+    return -ENOSYS;
+}