x86, efi: add no_bricked_efi whitelist

PATCH In review Yasuaki Ishimatsu 4 participants 17 messages Nov 20, 2013 – Dec 3, 2013

Subsystems (2)

x86 EFI

lore

Share & tools

lore

Yasuaki Ishimatsu

author

2013-11-20 08:34 UTC #0 lore raw

PATCH 1/1 x86, efi: add no_bricked_efi whitelist

By following works, my system very often fails set_variable() to set new variable to efi variable storage and shows "efivars: set_variable() failed:
status=-28" message.

- commit 68d929862e29a8b52a7f2f2f86a0600423b093cd
efi: be more paranoid about available space when creating variables
- commit 31ff2f20d9003e74991d135f56e503fe776c127c
efi: Distinguish between "remaining space" and actually used space
- commit 8c58bf3eec3b8fc8162fe557e9361891c20758f2
x86,efi: Implement efi_no_storage_paranoia parameter
- commit f8b8404337de4e2466e2e1139ea68b1f8295974f
Modify UEFI anti-bricking code

When booting my system, remaining space of efi variable storage is about 5KB. So there is no room that sets a new variable to the storage. On my system, trigger of gc is when EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI. So if EFI_OUT_OF_RESOURCES occurs by the 5Kbyte threshold, nvram storage cannot be used until EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI.

This patch adds whitelist. If a server is in the whitelist, efi_no_storage_paranoia is set to true. And the system can use all efi variable storage.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

CC Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>

CC Richard Weinberger <richard-/[email protected]>

CC Lee, Chun-Y <jlee-IBi9RG/[email protected]>

CC Madper Xie <[email protected]>

CC Matt Fleming <[email protected]>

---
 arch/x86/platform/efi/efi.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

arch/x86/platform/efi/efi.c +29 -0

index c7e22ab..9fadf5d 100644
@@ -116,6 +116,15 @@ static int __init setup_storage_paranoia(char *arg)
 }
 early_param("efi_no_storage_paranoia", setup_storage_paranoia);
 
+struct no_bricked_efi {
+	char vendor[100];
+	u32 revision;
+};
+
+static struct no_bricked_efi efi_whitelist[] __initdata = {
+	{"FUJITSU LIMITED", 0},
+	{""}
+};
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {

@@ -704,6 +713,24 @@ static int __init efi_memmap_init(void)
 	return 0;
 }
 
+static bool __init is_no_bricked_efi(char *vendor, u32 revision)
+{
+	int i = 0;
+
+	while (efi_whitelist[i].vendor[0] != '\0') {
+		if (strncmp(efi_whitelist[i].vendor, vendor,
+			    sizeof(efi_whitelist[i].vendor))) {
+			i++;
+			continue;
+		}
+
+		if (revision >= efi_whitelist[i].revision)
+			return true;
+	}
+
+	return false;
+}
+
 void __init efi_init(void)
 {
 	efi_char16_t *c16;

@@ -778,6 +805,8 @@ void __init efi_init(void)
 #if EFI_DEBUG
 	print_efi_memmap();
 #endif
+	if (is_no_bricked_efi(vendor, efi.systab->hdr.revision))
+		efi_no_storage_paranoia = true;
 }
 
 void __init efi_late_init(void)

Richard Weinberger

2013-11-20 08:54 UTC #1 lore raw

Am Mittwoch, 20. November 2013, 17:34:18 schrieb Yasuaki Ishimatsu:

Quoted 39 lines

By following works, my system very often fails set_variable() to set new variable to efi variable storage and shows "efivars: set_variable() failed:
status=-28" message.
- commit 68d929862e29a8b52a7f2f2f86a0600423b093cd
efi: be more paranoid about available space when creating variables
- commit 31ff2f20d9003e74991d135f56e503fe776c127c
efi: Distinguish between "remaining space" and actually used space
- commit 8c58bf3eec3b8fc8162fe557e9361891c20758f2
x86,efi: Implement efi_no_storage_paranoia parameter
- commit f8b8404337de4e2466e2e1139ea68b1f8295974f
Modify UEFI anti-bricking code
When booting my system, remaining space of efi variable storage is about 5KB. So there is no room that sets a new variable to the storage. On my system, trigger of gc is when EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI. So if EFI_OUT_OF_RESOURCES occurs by the 5Kbyte threshold, nvram storage cannot be used until EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI.
This patch adds whitelist. If a server is in the whitelist, efi_no_storage_paranoia is set to true. And the system can use all efi variable storage.
Signed-off-by Yasuaki Ishimatsu <[email protected]>
CC Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>
CC Richard Weinberger <richard-/[email protected]>
CC Lee, Chun-Y <jlee-IBi9RG/[email protected]>
CC Madper Xie <[email protected]>
CC Matt Fleming <[email protected]>
---
 arch/x86/platform/efi/efi.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
arch/x86/platform/efi/efi.c +0 -0
index c7e22ab..9fadf5d 100644

arch/x86/platform/efi/efi.c static int __init setup_storage_paranoia(char *arg) · line 125

Quoted 10 lines

arch/x86/platform/efi/efi.c +7 -0

@@ -116,6 +116,15 @@ static int __init setup_storage_paranoia(char *arg)
 }
 early_param("efi_no_storage_paranoia", setup_storage_paranoia);
 
+struct no_bricked_efi {
+	char vendor[100];
+	u32 revision;
+};
+
+static struct no_bricked_efi efi_whitelist[] __initdata = {
+	{"FUJITSU LIMITED", 0},

So, no UEFI from Fujitsu on planet earth suffers from such issues? How can you guarantee that?

Thanks,
//richard

Yasuaki Ishimatsu

author

2013-11-21 09:25 UTC #2 lore raw

Quoted 43 lines · Richard Weinberger

Am Mittwoch, 20. November 2013, 17:34:18 schrieb Yasuaki Ishimatsu:
Quoted 42 lines
By following works, my system very often fails set_variable() to set new variable to efi variable storage and shows "efivars: set_variable() failed:
status=-28" message.
- commit 68d929862e29a8b52a7f2f2f86a0600423b093cd
efi: be more paranoid about available space when creating variables
- commit 31ff2f20d9003e74991d135f56e503fe776c127c
efi: Distinguish between "remaining space" and actually used space
- commit 8c58bf3eec3b8fc8162fe557e9361891c20758f2
x86,efi: Implement efi_no_storage_paranoia parameter
- commit f8b8404337de4e2466e2e1139ea68b1f8295974f
Modify UEFI anti-bricking code
When booting my system, remaining space of efi variable storage is about 5KB. So there is no room that sets a new variable to the storage. On my system, trigger of gc is when EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI. So if EFI_OUT_OF_RESOURCES occurs by the 5Kbyte threshold, nvram storage cannot be used until EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI.
This patch adds whitelist. If a server is in the whitelist, efi_no_storage_paranoia is set to true. And the system can use all efi variable storage.
Signed-off-by Yasuaki Ishimatsu <[email protected]>
CC Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>
CC Richard Weinberger <richard-/[email protected]>
CC Lee, Chun-Y <jlee-IBi9RG/[email protected]>
CC Madper Xie <[email protected]>
CC Matt Fleming <[email protected]>
---
  arch/x86/platform/efi/efi.c | 29 +++++++++++++++++++++++++++++
  1 file changed, 29 insertions(+)
arch/x86/platform/efi/efi.c +0 -0
index c7e22ab..9fadf5d 100644
@@ -116,6 +116,15 @@ static int __init setup_storage_paranoia(char *arg)
  }
  early_param("efi_no_storage_paranoia", setup_storage_paranoia);
 

Quoted 10 lines

arch/x86/platform/efi/efi.c static int __init setup_storage_paranoia(char *arg) · line 125
Quoted 7 lines
arch/x86/platform/efi/efi.c +7 -0
+struct no_bricked_efi {
+	char vendor[100];
+	u32 revision;
+};
+
+static struct no_bricked_efi efi_whitelist[] __initdata = {
+	{"FUJITSU LIMITED", 0},
So, no UEFI from Fujitsu on planet earth suffers from such issues? How can you guarantee that?

Thank you for comments.
I cannot guarantee that. How about using Product Name gotten by dmi_get_system_info(DMI_PRODUCT_NAME) instead of Vendor Name.?

Thanks,
Yasuaki Ishimatsu

Quoted 4 lines

Thanks,
//richard

Matthew Garrett

2013-11-20 16:02 UTC #3 lore raw

In reply to Yasuaki Ishimatsu

Quoted 6 lines · Yasuaki Ishimatsu

When booting my system, remaining space of efi variable storage is about 5KB. So there is no room that sets a new variable to the storage. On my system, trigger of gc is when EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI. So if EFI_OUT_OF_RESOURCES occurs by the 5Kbyte threshold, nvram storage cannot be used until EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI.

By "remaining space is about 5KB", do you mean that the rest of the space is filled with variables or do you mean that there is more free space but it hasn't been garbage collected?

Signature · 1 line

Matthew Garrett <[email protected]>

Yasuaki Ishimatsu

author

2013-11-21 09:35 UTC #4 lore raw

Quoted 9 lines · Matthew Garrett

Quoted 6 lines · Yasuaki Ishimatsu
When booting my system, remaining space of efi variable storage is about 5KB. So there is no room that sets a new variable to the storage. On my system, trigger of gc is when EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI. So if EFI_OUT_OF_RESOURCES occurs by the 5Kbyte threshold, nvram storage cannot be used until EFI_OUT_OF_RESOURCES occurs on pre OS environment with UEFI.

Quoted 4 lines

By "remaining space is about 5KB", do you mean that the rest of the space is filled with variables or do you mean that there is more free space but it hasn't been garbage collected?

Remaining space is free space that can be used by efi variable. But by 5KB threshold, we cannot use the space while running OS.

Thanks,
Yasuaki Ishimatsu

Matthew Garrett

2013-11-21 20:12 UTC #5 lore raw

Quoted 2 lines · Yasuaki Ishimatsu

Remaining space is free space that can be used by efi variable. But by 5KB threshold, we cannot use the space while running OS.

Is the situation something like(assuming a 128KB flash part):

1) 64KB of variables, 59KB of deleted variables, 5KB of free space,

2) 123KB of variables, 5KB of free space

Signature · 1 line

Matthew Garrett <[email protected]>

Yasuaki Ishimatsu

author

2013-11-22 00:00 UTC #6 lore raw

Quoted 8 lines · Matthew Garrett

Quoted 2 lines · Yasuaki Ishimatsu
Remaining space is free space that can be used by efi variable. But by 5KB threshold, we cannot use the space while running OS.
Is the situation something like(assuming a 128KB flash part):
1) 64KB of variables, 59KB of deleted variables, 5KB of free space,

My situation looks like this. The free space spreads out if gc runs.

Thanks,
Yasuaki Ishimatsu

Quoted 7 lines

or
2) 123KB of variables, 5KB of free space
?

Matthew Garrett

2013-11-22 00:03 UTC #7 lore raw

Quoted 11 lines · Yasuaki Ishimatsu

Quoted 8 lines · Matthew Garrett
Quoted 2 lines · Yasuaki Ishimatsu
Remaining space is free space that can be used by efi variable. But by 5KB threshold, we cannot use the space while running OS.
Is the situation something like(assuming a 128KB flash part):
1) 64KB of variables, 59KB of deleted variables, 5KB of free space,
My situation looks like this. The free space spreads out if gc runs.

Ok. In that case Joey's suggestion that we try forcing a GC run in the boot stub if free space < (threshold * 2) might be the best plan.

Signature · 1 line

Matthew Garrett <[email protected]>

Yasuaki Ishimatsu

author

2013-11-22 06:23 UTC #8 lore raw

Quoted 9 lines · Matthew Garrett

Quoted 8 lines · Yasuaki Ishimatsu
Quoted 7 lines · Matthew Garrett
Quoted 2 lines · Yasuaki Ishimatsu
Remaining space is free space that can be used by efi variable. But by 5KB threshold, we cannot use the space while running OS.
Is the situation something like(assuming a 128KB flash part):

Quoted 8 lines

Quoted 3 lines
Quoted 1 line
1) 64KB of variables, 59KB of deleted variables, 5KB of free space,
My situation looks like this. The free space spreads out if gc runs.
Ok. In that case Joey's suggestion that we try forcing a GC run in the boot stub if free space < (threshold * 2) might be the best plan.
2) 123KB of variables, 5KB of free space

I don't know how to implement your idea. But if GC run by your idea, the free space may spread out. So 1) case is solved. But it does not solve 2) case. In my opinion, if system has sane firmware, all nvram strage should be used. I will send whiltelist version which used DMI infomation. Please review it.

Thanks,
Yasuaki Ishimatsu

Yasuaki Ishimatsu

author

2013-11-22 06:29 UTC #9 lore raw

PATCH 1/1 x86, efi: add efi_whitelist_table to use all efi variable storage

By following works, my system very often fails set_variable() to set new variable to efi variable storage and shows "efivars: set_variable() failed:
status=-28" message.

This patch adds whitelist. If a server is in the whitelist, efi_no_storage_paranoia is set to true. And the system can use all efi variable storage.

Signed-off-by Yasuaki Ishimatsu <[email protected]>

CC Matthew Garrett <matthew.garrett-05XSO3Yj/[email protected]>

CC Richard Weinberger <richard-/[email protected]>

CC Lee, Chun-Y <jlee-IBi9RG/[email protected]>

CC Madper Xie <[email protected]>

CC Matt Fleming <[email protected]>

---
 arch/x86/platform/efi/efi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

arch/x86/platform/efi/efi.c +19 -0

index c7e22ab..f9fad62 100644
@@ -42,6 +42,7 @@
 #include <linux/io.h>
 #include <linux/reboot.h>
 #include <linux/bcd.h>
+#include <linux/dmi.h>
 
 #include <asm/setup.h>
 #include <asm/efi.h>

@@ -116,6 +117,24 @@ static int __init setup_storage_paranoia(char *arg)
 }
 early_param("efi_no_storage_paranoia", setup_storage_paranoia);
 
+static struct dmi_system_id efi_whitelist_table[] __initdata = {
+	{
+		/* FUJITSU PRIMEQUEST */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PRIMEQUEST"),
+		},
+	},
+	{}
+};
+
+static int __init init_efi_no_storage_paranoia(void)
+{
+	if (dmi_check_system(efi_whitelist_table))
+		efi_no_storage_paranoia = true;
+	return 0;
+}
+late_initcall(init_efi_no_storage_paranoia);
 
 static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
 {

Matthew Garrett

2013-11-22 17:47 UTC #10 lore raw

Re: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

arch/x86/platform/efi/efi.c static int __init setup_storage_paranoia(char *arg) · line 125

Quoted 2 lines · Yasuaki Ishimatsu

arch/x86/platform/efi/efi.c +2 -0

+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PRIMEQUEST"),

Can we guarantee that no Primequests have this issue? Can we guarantee that that no prior firmware versions have this issue? Can we guarantee that no future firmware versions will have this issue?

Signature · 1 line

Matthew Garrett <[email protected]>

Yasuaki Ishimatsu

author

2013-12-03 03:20 UTC #11 lore raw

Re: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

Quoted 9 lines · Matthew Garrett

arch/x86/platform/efi/efi.c static int __init setup_storage_paranoia(char *arg) · line 125
Quoted 2 lines · Yasuaki Ishimatsu
arch/x86/platform/efi/efi.c +2 -0
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PRIMEQUEST"),
Can we guarantee that no Primequests have this issue? Can we guarantee that that no prior firmware versions have this issue? Can we guarantee that no future firmware versions will have this issue?

Sorry for the late reply. I contacted our firmware team to confirm them. They said that current implementation, thereshold is 5KB, is strange.

Currently many EFI firmwares are based on EFI Development Kit (EDK). Accoding to EDK source, gc on boot time runs when free space of nvram is less than MaximumVariableSize of QueryVariableInfo(). In my system MaximumVariableSize is 1KB. So when free space reaches less than 5KB and I reboot the system, gc does not run on boot time in my system.

Threshold of gc depends on a system. So at least, 5KB threshold must be changed on each system.

Thanks,
Yasuaki Ishimatsu

Matthew Garrett

2013-12-03 03:45 UTC #12 lore raw

RE: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

The threshold of 5K is based on information we received from vendors. We won't be reducing it. Can you confirm that attempting to create a variable larger than the remaining space in boot services will trigger garbage collection?-- To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Yasuaki Ishimatsu

author

2013-12-03 04:49 UTC #13 lore raw

Re: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

Quoted 2 lines · Matthew Garrett

The threshold of 5K is based on information we received from vendors. We won't be reducing it. Can you confirm that attempting to create a variable larger than the remaining space in boot services will trigger garbage collection?

What boot service is the "boot services" in these sentences?

Thanks,
Yasuaki Ishimatsu

Matthew Garrett

2013-12-03 05:04 UTC #14 lore raw

Re: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

Quoted 1 line · Yasuaki Ishimatsu

What boot service is the "boot services" in these sentences?

The environment prior to ExitBootServices() being called.

Signature · 1 line

Matthew Garrett | [email protected]

Yasuaki Ishimatsu

author

2013-12-03 05:12 UTC #15 lore raw

Re: [PATCH] x86, efi: add efi_whitelist_table to use all efi variable storage

Quoted 6 lines · Matthew Garrett

Quoted 1 line · Yasuaki Ishimatsu
What boot service is the "boot services" in these sentences?
The environment prior to ExitBootServices() being called.

I see.
I'll check it.

Thank,
Yasuaki Ishimatsu

Matthew Garrett

2013-11-22 17:48 UTC #16 lore raw

In reply to Yasuaki Ishimatsu

Quoted 4 lines · Yasuaki Ishimatsu

I don't know how to implement your idea. But if GC run by your idea, the free space may spread out. So 1) case is solved. But it does not solve 2) case. In my opinion, if system has sane firmware, all nvram strage should be used. I will send whiltelist version which used DMI infomation. Please review it.

We don't want to solve (2). The benefit in doing so is small compared to the cost of maintaining a whitelist or risking breaking systems.

Signature · 1 line

Matthew Garrett <[email protected]>