On 06/27/2018 11:59 AM, Laszlo Ersek wrote:
On 06/27/18 18:34, Brijesh Singh wrote:
On 06/27/2018 07:54 AM, Laszlo Ersek wrote:
On 06/26/18 21:46, Brijesh Singh wrote:
After that, any access
to the flash will end up going through the encryption engine. I did try
hacking EDK2 to restore the C-bit
(I continue to be annoyed that the memory encryption bit is not exposed
in the GCD memory space attributes explicitly.)
but that was not sufficient because UEFI
runtime services are mapped as "encrypted" in OS page table
What do you mean here? Runtime services *code* or runtime services
*data*? Code must obviously be remain encrypted (otherwise we cannot
execute it in SEV). Runtime Services Data should also be mapped as
encrypted (it is normal RAM that is not used for guest<->hypervisor
exchange).
Sorry, I was meaning to say both the "code" and "data" are mapped as
encrypted by the OS.
hence we end up accessing the flash as encrypted when OS requests to
update the variables.
I don't understand the "hence" here; I don't see how the implication
follows. runtime services code and data should be encrypted. Runtime
MMIO should be un-encrypted.
Ohh, wait, in MarkMemoryRangeForRuntimeAccess(), we use
"EfiGcdMemoryTypeSystemMemory". I don't have a clue why that is a good
idea. That should have been EfiGcdMemoryTypeMemoryMappedIo.
Right, the memory is marked as 'system ram' and not 'mmio'.
Just to experiment, I did try changing it to 'mmio' to see if OS will
map this region as "unencrypted" but ovmf fails with below error
message after changing it from systemRAM->mmio
ConvertPages: failed to find range FFC00000 - FFFFFFFF
ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [FvbServicesRuntimeDxe]
/home/amd/workdir/upstream/edk2/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockServie.c(864):
!EFI_ERROR (Status)
This error occurs because (I think) you modified only the AddMemorySpace
call. If you change the GCD type on that, then please update the
subsequent AllocatePages as well, from EfiRuntimeServicesData to
EfiMemoryMappedIO.
Here is what I have.
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
@@ -847,7 +847,7 @@ MarkMemoryRangeForRuntimeAccess (
);
Status = gDS->AddMemorySpace (
- EfiGcdMemoryTypeSystemMemory,
+ EfiGcdMemoryTypeMemoryMappedIo,
BaseAddress,
Length,
EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
@@ -856,7 +856,7 @@ MarkMemoryRangeForRuntimeAccess (
Status = gBS->AllocatePages (
AllocateAddress,
- EfiRuntimeServicesData,
+ EfiMemoryMappedIO,
EFI_SIZE_TO_PAGES (Length),
&BaseAddress
);
I am still getting the error assertion failure. I can debug to see what
is going on.
The spec says about the latter enum constant, "Used by system firmware
to request that a memory-mapped IO region be mapped by the OS to a
virtual address so it can be accessed by EFI runtime services." It seems
appropriate (and I'm a bit confused why we haven't used the MMIO GCD and
UEFI enum values for the memory type, all this time.)
Since this efi runtime data is mapped as C=1 by the OS, hence when OS
asks efi to update the runtime variable we end up accessing the memory
region with C=1 (runtime services are executed using OS pagetable).
Indeed.
(And, this is only a problem when SMM is not used, i.e. when the full
variable driver stack is non-SMM, just DXE. In the SMM case, the SMM
page tables are used, and the OS cannot interfere with that.)
Good point, I will try it and let you know. As you say since SMM uses
UEFI page table hence after fixing FtwNotificationEvent(..) we should be
good.
Anyway, in the pure DXE / runtime driver case, do you think a guest
kernel patch will be necessary too? Perhaps if you change the UEFI
memmap entry type (see AllocatePages above) to MMIO, then the guest
kernel could technically honor that.
Theoretically speaking, if we are able to make this memory region as
mmio then OS should be able to map it with C=0.
-Brijesh
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
