On 07/05/18 21:12, Brijesh Singh wrote: > The flash memory range is an IO address and should be presented as Memory > Mapped IO in EFI Runtime mapping. This information can be used by OS > when mapping the flash memory range. > > It is especially helpful in SEV guest case, in which IO addresses should > be mapped as unencrypted. If memory region is not marked as MMIO then OS > maps the range as encrypted. > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: Julien Grall <julien.gr...@linaro.org> > Cc: Justen Jordan L <jordan.l.jus...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh <brijesh.si...@amd.com> > --- > .../FwBlockService.c | 28 > ++++++++++++++++------ > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > index 558b395dff4a..28499991a43c 100644 > --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c > @@ -831,12 +831,13 @@ ValidateFvHeader ( > > STATIC > EFI_STATUS > -MarkMemoryRangeForRuntimeAccess ( > +MarkIoMemoryRangeForRuntimeAccess ( > EFI_PHYSICAL_ADDRESS BaseAddress, > UINTN Length > ) > { > EFI_STATUS Status; > + EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor; > > // > // Mark flash region as runtime memory > @@ -847,18 +848,31 @@ MarkMemoryRangeForRuntimeAccess ( > ); > > Status = gDS->AddMemorySpace ( > - EfiGcdMemoryTypeSystemMemory, > + EfiGcdMemoryTypeMemoryMappedIo, > BaseAddress, > Length, > EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > ); > ASSERT_EFI_ERROR (Status); > > - Status = gBS->AllocatePages ( > + Status = gDS->AllocateMemorySpace ( > AllocateAddress,
(1) This should be changed to "EfiGcdAllocateAddress". (I didn't notice this in your previous submission; however, to my excuse, I did suggest it correctly in <fbba354d-1df7-0da5-80a6-40df601b95ea@redhat.com">http://mid.mail-archive.com/fbba354d-1df7-0da5-80a6-40df601b95ea@redhat.com> :) ) Note that this omission does not invalidate your testing, because "AllocateAddress" (from "MdePkg/Include/Uefi/UefiSpec.h") has value 2, and "EfiGcdAllocateAddress" (from "MdePkg/Include/Pi/PiDxeCis.h") has value 2 as well. > - EfiRuntimeServicesData, > - EFI_SIZE_TO_PAGES (Length), > - &BaseAddress > + EfiGcdMemoryTypeMemoryMappedIo, > + 0, > + Length, > + &BaseAddress, > + gImageHandle, > + NULL > + ); > + ASSERT_EFI_ERROR (Status); > + > + Status = gDS->GetMemorySpaceDescriptor (BaseAddress, &GcdDescriptor); (2) The extra space before the equal sign (=) looks uncalled for. > + ASSERT_EFI_ERROR (Status); > + > + Status = gDS->SetMemorySpaceAttributes ( > + BaseAddress, > + Length, > + GcdDescriptor.Attributes | EFI_MEMORY_RUNTIME > ); > ASSERT_EFI_ERROR (Status); > > @@ -1091,7 +1105,7 @@ FvbInitialize ( > // > InstallProtocolInterfaces (FvbDevice); > > - MarkMemoryRangeForRuntimeAccess (BaseAddress, Length); > + MarkIoMemoryRangeForRuntimeAccess (BaseAddress, Length); > > // > // Set several PCD values to point to flash > Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel