[U-Boot] efi.h: Do not use config options

Message ID 20180612055420.58267-1-agraf@suse.de
State New
Delegated to: Alexander Graf
Headers show
Series
  • [U-Boot] efi.h: Do not use config options
Related show

Commit Message

Alexander Graf June 12, 2018, 5:54 a.m.
Currently efi.h determines a few bits of its environment according to
config options. This falls apart with the efi stub support which may
result in efi.h getting pulled into the stub as well as real U-Boot
code. In that case, one may be 32bit while the other one is 64bit.

This patch changes the conditionals to use compiler provided defines
instead. That way we always adhere to the build environment we're in
and the definitions adjust automatically.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/efi.h    | 17 ++++-------------
 lib/efi/Makefile |  4 ++--
 2 files changed, 6 insertions(+), 15 deletions(-)

Comments

Bin Meng June 12, 2018, 9:21 a.m. | #1
On Tue, Jun 12, 2018 at 1:54 PM, Alexander Graf <agraf@suse.de> wrote:
> Currently efi.h determines a few bits of its environment according to
> config options. This falls apart with the efi stub support which may
> result in efi.h getting pulled into the stub as well as real U-Boot
> code. In that case, one may be 32bit while the other one is 64bit.
>
> This patch changes the conditionals to use compiler provided defines
> instead. That way we always adhere to the build environment we're in
> and the definitions adjust automatically.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  include/efi.h    | 17 ++++-------------
>  lib/efi/Makefile |  4 ++--
>  2 files changed, 6 insertions(+), 15 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>

Alex, will you take this one via the efi tree, or via the x86 tree
since the EFI app/stub changes are dependent on this one?

Regards,
Bin
Bin Meng June 12, 2018, 3:33 p.m. | #2
Hi Alex,

On Tue, Jun 12, 2018 at 5:21 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Tue, Jun 12, 2018 at 1:54 PM, Alexander Graf <agraf@suse.de> wrote:
>> Currently efi.h determines a few bits of its environment according to
>> config options. This falls apart with the efi stub support which may
>> result in efi.h getting pulled into the stub as well as real U-Boot
>> code. In that case, one may be 32bit while the other one is 64bit.
>>
>> This patch changes the conditionals to use compiler provided defines
>> instead. That way we always adhere to the build environment we're in
>> and the definitions adjust automatically.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  include/efi.h    | 17 ++++-------------
>>  lib/efi/Makefile |  4 ++--
>>  2 files changed, 6 insertions(+), 15 deletions(-)
>>
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>
> Alex, will you take this one via the efi tree, or via the x86 tree
> since the EFI app/stub changes are dependent on this one?
>

I included your patch into my EFI app/stub series v2, for review and testing.

Regards,
Bin
Alexander Graf June 12, 2018, 7:13 p.m. | #3
On 12.06.18 17:33, Bin Meng wrote:
> Hi Alex,
> 
> On Tue, Jun 12, 2018 at 5:21 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> On Tue, Jun 12, 2018 at 1:54 PM, Alexander Graf <agraf@suse.de> wrote:
>>> Currently efi.h determines a few bits of its environment according to
>>> config options. This falls apart with the efi stub support which may
>>> result in efi.h getting pulled into the stub as well as real U-Boot
>>> code. In that case, one may be 32bit while the other one is 64bit.
>>>
>>> This patch changes the conditionals to use compiler provided defines
>>> instead. That way we always adhere to the build environment we're in
>>> and the definitions adjust automatically.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>>  include/efi.h    | 17 ++++-------------
>>>  lib/efi/Makefile |  4 ++--
>>>  2 files changed, 6 insertions(+), 15 deletions(-)
>>>
>>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> Tested-by: Bin Meng <bmeng.cn@gmail.com>
>>
>> Alex, will you take this one via the efi tree, or via the x86 tree
>> since the EFI app/stub changes are dependent on this one?
>>
> 
> I included your patch into my EFI app/stub series v2, for review and testing.

I don't mind either way. It probably fits better into your series though
which again should probably go via your tree :).


Alex

Patch

diff --git a/include/efi.h b/include/efi.h
index 98bddbac1a..5e1e8ac78c 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -19,12 +19,12 @@ 
 #include <linux/string.h>
 #include <linux/types.h>
 
-#if CONFIG_EFI_STUB_64BIT || (!defined(CONFIG_EFI_STUB) && defined(__x86_64__))
-/* EFI uses the Microsoft ABI which is not the default for GCC */
+/* EFI on x86_64 uses the Microsoft ABI which is not the default for GCC */
+#ifdef __x86_64__
 #define EFIAPI __attribute__((ms_abi))
 #else
 #define EFIAPI asmlinkage
-#endif
+#endif /* __x86_64__ */
 
 struct efi_device_path;
 
@@ -32,16 +32,7 @@  typedef struct {
 	u8 b[16];
 } efi_guid_t;
 
-#define EFI_BITS_PER_LONG	BITS_PER_LONG
-
-/*
- * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set
- * in lib/efi/Makefile, when building the stub.
- */
-#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
-#undef EFI_BITS_PER_LONG
-#define EFI_BITS_PER_LONG	64
-#endif
+#define EFI_BITS_PER_LONG	(sizeof(long) * 8)
 
 /* Bit mask for EFI status code with error */
 #define EFI_ERROR_MASK (1UL << (EFI_BITS_PER_LONG - 1))
diff --git a/lib/efi/Makefile b/lib/efi/Makefile
index 18d081ac46..ece7907227 100644
--- a/lib/efi/Makefile
+++ b/lib/efi/Makefile
@@ -7,9 +7,9 @@  obj-$(CONFIG_EFI_STUB) += efi_info.o
 
 CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \
 	$(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32)
-CFLAGS_efi_stub.o := -fpic -fshort-wchar -DEFI_STUB
+CFLAGS_efi_stub.o := -fpic -fshort-wchar
 CFLAGS_REMOVE_efi.o := -mregparm=3 \
 	$(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32)
-CFLAGS_efi.o := -fpic -fshort-wchar -DEFI_STUB
+CFLAGS_efi.o := -fpic -fshort-wchar
 
 extra-$(CONFIG_EFI_STUB) += efi_stub.o efi.o