Re: [PATCH 0/4] EFI 1:1 mapping
On Sun, Jun 02, 2013 at 11:56:20PM +0100, Matthew Garrett wrote: I've just run Windows 8 under a hacked up copy of OVMF that dumps the data passed to SetVirtualAddressMap. It seems that Windows *is* mapping the runtime services to higher addresses - so presumably the 1:1 mapping is in addition to the virtual mapping. But but, once we call SetVirtualAddressMap with the set of addresses of the runtime services, only those can be used after, right? If so, we can't have both (this is at least my understanding)... Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Modify UEFI anti-bricking code
On Mon, 2013-06-03 at 13:17 +0100, Matt Fleming wrote: Do we really want to drop this hunk? The point of this code was to inform firmware vendors that their implementation is returning funky results, and that they should look into why it's doing that. We're not doing anything with that information now, and I don't think we can do anything meaningful with it - some implementations report the maximum size as the maximum supported by the implementation, and some report it as the maximum that can be written given the amount of space currently available. So yeah, it's against the spec to report 0 here, but any other value is equally useless to us, so it seems like something we'd be complaining about for no reason. What's the origin of this guid? And should we be adding it to include/linux/efi.h? It's a scratch guid that's randomly generated to avoid colliding with any other guid - we want to avoid this ever being a real variable. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH 0/4] EFI 1:1 mapping
On Mon, Jun 03, 2013 at 09:18:06AM -0700, James Bottomley wrote: I don't entirely buy that. All EFI programs run with the physical address map, therefore every API an EFI program uses is also tested, at boot time only, obviously. That seems optimistic. Windows never calls QueryVariableInfo() during boot services, so what makes you think doing so has ever been tested? However, the ExitBootServices() code seems to be much simpler, so I don't think it will cause too many bugs. The UEFI test suites also seem to try UEFI calls before and after ExitBootServices(), so I think relying on a 1:1 mapping looks safer to me. I have no expectation that the majority of system vendors run the test suite, but I have every expectation that every system vendor runs Windows. We should behave as close to the tested mechanism as possible, ie do what Windows does - and that includes calling SetVirtualAddressMap(). -- Matthew Garrett | mj...@srcf.ucam.org -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Modify UEFI anti-bricking code
On Tue, 2013-06-04 at 00:13 +0800, joeyli wrote: Oliver raised a question for if power fails between that succesful attempt and the deletion? It's a pretty tiny window, but sure. Making sure we delete it seems sensible. In that case we should make the GUID a #define rather than write it out twice. -- Matthew Garrett | mj...@srcf.ucam.org
Re: [PATCH 0/4] EFI 1:1 mapping
On Mon, 2013-06-03 at 17:24 +0100, Matthew Garrett wrote: On Mon, Jun 03, 2013 at 09:18:06AM -0700, James Bottomley wrote: I don't entirely buy that. All EFI programs run with the physical address map, therefore every API an EFI program uses is also tested, at boot time only, obviously. That seems optimistic. Windows never calls QueryVariableInfo() during boot services, so what makes you think doing so has ever been tested? It's used by the UEFI shell package ... every system which boots to the shell automatically tests this. I know no locked down UEFI system ships with a shell but almost every system in test has a Shell in some form, so I think its fairly safe to call it from boot services. However, the ExitBootServices() code seems to be much simpler, so I don't think it will cause too many bugs. The UEFI test suites also seem to try UEFI calls before and after ExitBootServices(), so I think relying on a 1:1 mapping looks safer to me. I have no expectation that the majority of system vendors run the test suite, but I have every expectation that every system vendor runs Windows. We should behave as close to the tested mechanism as possible, ie do what Windows does - and that includes calling SetVirtualAddressMap(). OK, so we basically agree to disagree. When I looked at the actual SetVirtualAddressMap() implementation, my heart skipped several beats: it's a massive set of pointer chasing heuristics which is bound to be incorrect in some instance, just because its so complex and easy to get wrong. Every time it's incorrect, we'll get a physical pointer used in a virtual space and an oops within the UEFI code. Conversely, I think the engineering risk that a particular UEFI call is expecting to have had SetVirtualAddressMap called is much lower. However, what about a compromise: why don't we implement 1:1 mapping and then call SetVirtualAddressMap with the 1:1 map ... in theory the pointer chases should then be nops (it will be replacing the physical address with the same virtual address), so everything should just work and anything the UEFI vendor missed will still work because the physical address will work also in this scenario. James -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] EFI 1:1 mapping
On Mon, 2013-06-03 at 17:42 +0100, Matthew Garrett wrote: On Mon, Jun 03, 2013 at 09:35:07AM -0700, James Bottomley wrote: On Mon, 2013-06-03 at 17:24 +0100, Matthew Garrett wrote: That seems optimistic. Windows never calls QueryVariableInfo() during boot services, so what makes you think doing so has ever been tested? It's used by the UEFI shell package ... every system which boots to the shell automatically tests this. I know no locked down UEFI system ships with a shell but almost every system in test has a Shell in some form, so I think its fairly safe to call it from boot services. Why do you persist in this belief that all system vendors are going to have run a shell, let alone any kind of test suite? That runs counter to everything we've learned about x86 firmware. People verify that it runs Windows and then ship it. I don't, but I find it hard to believe no vendor ever runs an EFI shell on their systems. The feedback I got from a couple of OEMs is that they use the shell mostly for internal testing. However, what about a compromise: why don't we implement 1:1 mapping and then call SetVirtualAddressMap with the 1:1 map ... in theory the pointer chases should then be nops (it will be replacing the physical address with the same virtual address), so everything should just work and anything the UEFI vendor missed will still work because the physical address will work also in this scenario. The problem there is that you're saying In theory. We know that Windows doesn't behave this way, so we have no legitimate expectation that it'll work. We know that it doesn't on some Apple hardware. Fine, you say we need to call SetVirtualAddressMap because windows does, I agree, I'm just saying we get additional safety from calling it with the 1:1 map ... I don't see what the problem is. James -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] EFI 1:1 mapping
On Mon, 2013-06-03 at 19:11 +0100, Matthew Garrett wrote: The problem there is that you're saying In theory. We know that Windows doesn't behave this way, so we have no legitimate expectation that it'll work. We know that it doesn't on some Apple hardware. Fine, you say we need to call SetVirtualAddressMap because windows does, I agree, I'm just saying we get additional safety from calling it with the 1:1 map ... I don't see what the problem is. No. I'm saying that calling it with the 1:1 map is something very different to the behaviour of Windows, and I'm saying that doing so is known to cause variable writes on some Apple hardware to stop working. If we're aiming for maximum compatibility, we need to call SetVirtualAddressMap() with addresses above the canonicalisation hole. OK, so tell me this problem: it's a new one one me. I think you're saying if we don't call SetVirtualAddressMap with a mapping above a certain value, some Apple system breaks somehow? (how?). James -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Modify UEFI anti-bricking code
於 一，2013-06-03 於 16:31 +，Matthew Garrett 提到：
On Tue, 2013-06-04 at 00:13 +0800, joeyli wrote:
Oliver raised a question for if power fails between that succesful
attempt and the deletion?
It's a pretty tiny window, but sure. Making sure we delete it seems
sensible. In that case we should make the GUID a #define rather than
write it out twice.
Base on your patch, the following diff moved DUMMY GUID to #define, and
add a static efi name string:
Thanks a lot!
Joey Lee
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 63e167a..cc3cfe8 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -53,6 +53,11 @@
#define EFI_DEBUG 1
+#define EFI_DUMMY_GUID \
+ EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e, 0x97, 0xed, 0x50, 0xf0,
0x9f, 0x92, 0xa9)
+
+static efi_char16_t efi_dummy_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
+
struct efi __read_mostly efi = {
.mps= EFI_INVALID_TABLE_ADDR,
.acpi = EFI_INVALID_TABLE_ADDR,
@@ -981,6 +986,9 @@ void __init efi_enter_virtual_mode(void)
runtime_code_page_mkexec();
kfree(new_memmap);
+
+ /* clean DUMMY object */
+ efi.set_variable(efi_dummy_name, EFI_DUMMY_GUID, 0, 0, NULL);
}
/*
@@ -1051,21 +1059,17 @@ efi_status_t efi_query_variable_store(u32 attributes,
unsigned long size)
*/
unsigned long dummy_size = remaining_size + 1024;
void *dummy = kmalloc(dummy_size, GFP_ATOMIC);
- efi_char16_t efi_name[6] = { 'D', 'U', 'M', 'M', 'Y', 0 };
- efi_guid_t guid = EFI_GUID(0x4424ac57, 0xbe4b, 0x47dd, 0x9e,
- 0x97, 0xed, 0x50, 0xf0, 0x9f, 0x92,
- 0xa9);
- status = efi.set_variable(efi_name, guid, attributes,
- dummy_size, dummy);
+ status = efi.set_variable(efi_dummy_name, EFI_DUMMY_GUID,
+ attributes, dummy_size, dummy);
if (status == EFI_SUCCESS) {
/*
* This should have failed, so if it didn't make sure
* that we delete it...
*/
- efi.set_variable(efi_name, guid, attributes, 0,
-dummy);
+ efi.set_variable(efi_dummy_name, EFI_DUMMY_GUID,
+attributes, 0, dummy);
}
/*
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
