On 06/28/2018 07:57 AM, Laszlo Ersek wrote:

[...]


--- 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.

Hmmm. Indeed, memory space added to GCD need not immediately show up in
the UEFI memory map, for the UEFI memory allocation services to allocate
from. IIRC, the PI spec says that *system memory* added like this *may*
show up immediately:

"""
If the memory range specified by BaseAddress and Length is of type
EfiGcdMemoryTypeSystemMemory or EfiGcdMemoryTypeMoreReliable, then the
memory range may be automatically allocated for use by the UEFI memory
services.
"""

and, indeed in edk2, it happens at once; but it doesn't apply to MMIO at
all.

So, can you replace the AllocatePages call with the following:

   Status = gDS->AllocateMemorySpace (
                   EfiGcdAllocateAddress,
                   EfiGcdMemoryTypeMemoryMappedIo,
                   0,                              // Alignment
                   EFI_SIZE_TO_PAGES (Length),
                   &BaseAddress,
                   gImageHandle,
                   NULL                            // DeviceHandle
                   );



I did realized this in my yesterday's debug. I was looking at other
drivers (mainly PciBridge...) on how it adds the MMIO. I no longer
get the ASSERT. thank you !

After changing this system boots fine but I am getting Linux kernel
crashes when we attempt to update the UEFI runtime variables. I am
still debugging and trying to to root cause it.




Good point, I will try it and let you know. As you say since SMM uses
UEFI page table

More correctly: SMM drivers use, at runtime as well, the page table in
SMRAM that was created by the firmware.

hence after fixing FtwNotificationEvent(..) we should be
good.

No, that's not precise -- FtwNotificationEvent() is not used in SMM *at
all*.

I checked that before I sent my previous email; namely,
FtwNotificationEvent() is in
"MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c", which is not
built into the SMM variant of the variable driver.

Please see the variable driver stacks in OVMF:

- non-SMM:

   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
   OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf

In the non-SMM case, we have a normal runtime driver,
VariableRuntimeDxe.inf, that implements the variable service. It relies
on the FTW driver (another runtime driver) for the FTW protocol.
Finally, both the variable driver and the FTW driver rely on the FVB
(firmware volume block) protocol, which may come from the QEMU flash
driver, or from the emulation driver. Again, all of these are runtime
drivers. "VariableRuntimeDxe.inf" includes the "VariableDxe.c" file,
hence FtwNotificationEvent() is relevant.

- SMM:

   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesSmm.inf
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf

In the SMM case, we have a *split* variable driver,
"VariableSmmRuntimeDxe.inf" (which is a normal runtime driver) and
"VariableSmm.inf" (a "traditional SMM" driver). The runtime half is
unprivileged and only formats a message for the privileged SMM half,
submits the message, and then parses the response. The rest of the
variable stack, namely the privileged half of the variable driver, and
the FTW driver, and the QEMU flash (FVB) driver, all stay within SMM --
they are traditional SMM drivers too.

"VariableSmmRuntimeDxe.inf" does not include "VariableDxe.c". Instead,
"VariableSmm.inf" (the privileged half) includes "VariableSmm.c", and
that source file waits for the *SMM* FVB protocol, in the
SmmFtwNotificationEvent() function. But, this notification function,
unlike the DXE counterpart FtwNotificationEvent(), does not care about
the GCD entry attributes -- because in SMM, separate (protected) page
tables are used anyway.

If you put FtwNotificationEvent() and SmmFtwNotificationEvent() side by
side, you'll see that they perform almost identical steps:
- "Ensure [SMM] FTW protocol is installed"
- "Find the proper [SMM] FVB protocol for variable"
- "Mark the variable storage region of the FLASH as RUNTIME" -- this is
   DXE only!
- VariableWriteServiceInitialize
- "Install the Variable Write Architectural protocol"

(I skipped RecordSecureBootPolicyVarData(), because that always happens
in the DXE half. Anyway it's irrelevant here.)

Thus, my expectation is that the current issue manifests itself only if
you build OVMF without -D SMM_REQUIRE.



Actually I was not aware that there are two different stack (SMM vs
non-SMM). And you are absolutely right. The issue does not happen
when using -D SMM_REQUIRE flag. So, I think we just need to focus
on making non SMM work.

-Brijesh

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to