Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

[Masahiro Yamada]

On Sat, Nov 6, 2021 at 12:34 PM Seed Of Onan <seedofonan@yahoo.com> wrote:
>
> Note that everything else about configuring, building, and executing the kernel that results works great.
>
> The trouble appears to be because of the dependency test under CONFIG_EFI_STUB has $(cc-option,-mabi=ms) and (looking at that macro's definition in scripts/Kconfig.include) CLANG_FLAGS doesn't have -m64 (but certainly it is getting onto the command line of each invocation of gcc some other way) and gcc errors when combining -mabi=ms with (in this case defaulted) -mx32.
>
> I'm figuring that although it could be done in principle, the gcc folks would object to establishing a meaning to the -mabi=ms calling convention for x32 primarily because Microsoft doesn't support that and the "ms" here stands for "Microsoft", I presume.
>
> In any case, what do you think? Shouldn't the cc-option macro be more complete -- if "-m64" is being added because the build config system knows that it is needed, then it should be included in the command line used by cc-option macro? Perhaps the "-m64" happens to be in the same environment variable as the "-mabi=ms" part, in which case couldn't instead one just change the dependency test to be like $(cc-option,-m64 -mabi=ms)?
>
> Or am I missing the mark and there's a better fix in another place?
>
> Please help. Thank you,
> -Gary
>



I was not invoked in anyway in the following commit.

commit 8f24f8c2fc82f701866419dcb594e2cc1d3f46ba
Author: Ard Biesheuvel <ardb@kernel.org>
Date:   Tue Dec 24 16:10:12 2019 +0100

    efi/libstub: Annotate firmware routines as __efiapi



I added Ard to Cc.



At least, please describe your problem properly.
What error did you get? How to reproduce it?



-- 
Best Regards
Masahiro Yamada

Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

[Ard Biesheuvel]

On Sat, 6 Nov 2021 at 12:35, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sat, Nov 6, 2021 at 12:34 PM Seed Of Onan <seedofonan@yahoo.com> wrote:
> >
> > Note that everything else about configuring, building, and executing the kernel that results works great.
> >
> > The trouble appears to be because of the dependency test under CONFIG_EFI_STUB has $(cc-option,-mabi=ms) and (looking at that macro's definition in scripts/Kconfig.include) CLANG_FLAGS doesn't have -m64 (but certainly it is getting onto the command line of each invocation of gcc some other way) and gcc errors when combining -mabi=ms with (in this case defaulted) -mx32.
> >
> > I'm figuring that although it could be done in principle, the gcc folks would object to establishing a meaning to the -mabi=ms calling convention for x32 primarily because Microsoft doesn't support that and the "ms" here stands for "Microsoft", I presume.
> >
> > In any case, what do you think? Shouldn't the cc-option macro be more complete -- if "-m64" is being added because the build config system knows that it is needed, then it should be included in the command line used by cc-option macro? Perhaps the "-m64" happens to be in the same environment variable as the "-mabi=ms" part, in which case couldn't instead one just change the dependency test to be like $(cc-option,-m64 -mabi=ms)?
> >
> > Or am I missing the mark and there's a better fix in another place?
> >
> > Please help. Thank you,
> > -Gary
> >
>
>
>
> I was not invoked in anyway in the following commit.
>
> commit 8f24f8c2fc82f701866419dcb594e2cc1d3f46ba
> Author: Ard Biesheuvel <ardb@kernel.org>
> Date:   Tue Dec 24 16:10:12 2019 +0100
>
>     efi/libstub: Annotate firmware routines as __efiapi
>
>
>
> I added Ard to Cc.
>
>
>
> At least, please describe your problem properly.
> What error did you get? How to reproduce it?
>

IIUC, the issue is in

config EFI_STUB
       bool "EFI stub support"
       depends on EFI && !X86_USE_3DNOW
       depends on $(cc-option,-mabi=ms) || X86_32

where the cc-option check is failing due to the use of -mx32 for the
core kernel, whereas the EFI stub will be built with -m64 regardless.

Given that __attribute__((ms_abi)) should be supported on any GCC
versions 4.8 or newer, I think we can just drop this depends line
entirely. Would that solve your issue?

Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

[Ard Biesheuvel]

On Sun, 7 Nov 2021 at 05:24, Seed Of Onan <seedofonan@yahoo.com> wrote:
>
> > IIUC, the issue is in
> > config EFI_STUB
> >  :
> >    depends on $(cc-option,-mabi=ms) || X86_32
> > where the cc-option check is failing due to the use of
> > -mx32 for the core kernel, whereas the EFI stub will be
> > built with -m64 regardless.
> Yes, except, the kernel can't be built with -mx32, only -m32 or -m64. The -m64 option gets added to the compile flags in arch/x86/Makefile on 64bit builds.
>

Right, so it is just the compiler default

> > Given that __attribute__((ms_abi)) should be supported on any
> > GCC versions 4.8 or newer, I think we can just drop this
> > depends line entirely. Would that solve your issue?
> Yes it would, but it sure seems to me to be the wrong way -- gcc is not the only compiler, and the ms calling convention isn't required to build the kernel otherwise, is it?
>

I don't follow. All compilers that we support implement this so why test for it?

> I found in scripts/Kconfig.include a variable that is "-m64 if the compiler supports it, or an empty string otherwise":
>     depends on $(cc-option,$(m64-flag) -mabi=ms) || X86_32
>
> I don't get the "|| X86_32" part -- can it really be that when building an x86_32 kernel, there is no concern about the support for the calling convention needed by the efi stub? But whatever.

Of course we care about the calling convention. Why would you think
that we don't?

Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

[Ard Biesheuvel]

On Sun, 7 Nov 2021 at 08:58, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 7 Nov 2021 at 05:24, Seed Of Onan <seedofonan@yahoo.com> wrote:
> >
> > > IIUC, the issue is in
> > > config EFI_STUB
> > >  :
> > >    depends on $(cc-option,-mabi=ms) || X86_32
> > > where the cc-option check is failing due to the use of
> > > -mx32 for the core kernel, whereas the EFI stub will be
> > > built with -m64 regardless.
> > Yes, except, the kernel can't be built with -mx32, only -m32 or -m64. The -m64 option gets added to the compile flags in arch/x86/Makefile on 64bit builds.
> >
>
> Right, so it is just the compiler default
>
> > > Given that __attribute__((ms_abi)) should be supported on any
> > > GCC versions 4.8 or newer, I think we can just drop this
> > > depends line entirely. Would that solve your issue?
> > Yes it would, but it sure seems to me to be the wrong way -- gcc is not the only compiler, and the ms calling convention isn't required to build the kernel otherwise, is it?
> >
>
> I don't follow. All compilers that we support implement this so why test for it?
>
> > I found in scripts/Kconfig.include a variable that is "-m64 if the compiler supports it, or an empty string otherwise":
> >     depends on $(cc-option,$(m64-flag) -mabi=ms) || X86_32
> >
> > I don't get the "|| X86_32" part -- can it really be that when building an x86_32 kernel, there is no concern about the support for the calling convention needed by the efi stub? But whatever.
>
> Of course we care about the calling convention. Why would you think
> that we don't?

Note that 32-bit UEFI code uses __attribute__((regparm(0))) as the
calling convention not ms_abi

Re: Given an x32 userspace with a gcc to match (ie defaults to -mx32) then CONFIG_EFI_STUB is unavailable

[Ard Biesheuvel]

On Sun, 7 Nov 2021 at 23:54, Seed Of Onan <seedofonan@yahoo.com> wrote:
>
> What follows is just IMHO. Seriously -- I sincerely appreciate you even considering doing anything at all about my nothing issue, and I expect no further involvement in the decision as to what is done about it.
>
> > I don't follow. All compilers that we support implement this so why test for it?
> Okay. And how does one discover if one's compiler (and environment and platform) is one "that we support" for any given combination of kernel configurations and mix of options? For instance, is mine and how do I find out? I'm presently under the spell of thinking that the contents of Kconfig DEFINES what "we" support. So in answer to your question, I'm thinking we keep the existing test because it specifies a real, additional burden on any past/present/future compiler, and without it we are saying that any past/present/future compiler should (is supported to) work now because there is no longer any such additional burden -- older x86_64 compilers and environments that once failed the -mabi=ms test, they are now supported if no other test says otherwise. In short, I don't know if "we" should keep it (modified, I hope) or not, but I expect you (all) are better qualified to decide both that and the philosophical point of Kconfig in the first place.
>

It's very simple: we support GCC 5.1 or newer. But as you pointed out,
the -m64 option is missing from the ms_abi test, leading to false
negatives if m64 is not the default.

Anyone is free to shoot themselves in the foot if they want to try and
build the kernel with a non-supported compiler: the only impact that
dropping this config check will have is that instead of disabling
CONFIG_EFI_STUB automatically, it may get enabled and cause a build
error. This is not a big deal as a) the compiler was not supported to
begin with, and b) the user can just disable it, as the feature was
not available to them in the first place.

> > Note that 32-bit UEFI code uses __attribute__((regparm(0)))
> > as the calling convention not ms_abi
> Interesting, thanks. So that leads me to think the test should be, for hypothetical macro cc-attribute, like:
> (X86_64 && $(cc-attribute, ms_abi)) || (X86_32 && $(cc-attribute, regparm(0)))
>

Again, the check is pointless so it needs to be dropped. It only
decides whether or not EFI_STUB can be enabled or not, and with a
non-supported compiler, we are in DYI territory anyway, so the user
should just disable it themselves if their compiler is not compatible
with the option.