ArmVirtualizationPkg/PciHostBridgeDxe: MMIO aperture must not be uncached

Quite non-intuitively, we must allow guest-side writes to emulated PCI
MMIO regions to go through the CPU cache, otherwise QEMU, whose accesses
always go through the cache, may see stale data in the region.

This change makes no difference for QEMU/TCG, but it is important for
QEMU/KVM, at the moment.

Because gDS->SetMemorySpaceAttributes() is ultimately implemented by
EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes() -- see
"MdeModulePkg/Core/Dxe/Gcd/Gcd.c" and "ArmPkg/Drivers/CpuDxe/" -- we add
the CPU architectural protocol to the module's DepEx.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Olivier Martin <olivier.martin@arm.com>
git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16904 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
index 99411548aff6a5bcd37df903db7cd6d54bef7a99..d53dab9f65e115e6e24e364a14f6bc41b0d93d13 100644 (file)
--- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
+++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
@@ -56,3 +56,28 @@
 \r
   gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress|0x0|UINT64|0x00000004\r
   gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress|0x0|UINT64|0x00000005\r
 \r
   gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress|0x0|UINT64|0x00000004\r
   gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress|0x0|UINT64|0x00000005\r

+\r
+[PcdsFeatureFlag]\r
+  #\r
+  # "Map PCI MMIO as Cached"\r
+  #\r
+  # Due to the way Stage1 and Stage2 mappings are combined on Aarch64, and\r
+  # because KVM -- for the time being -- does not try to interfere with the\r
+  # Stage1 mappings, we must not set EFI_MEMORY_UC for emulated PCI MMIO\r
+  # regions.\r
+  #\r
+  # EFI_MEMORY_UC is mapped to Device-nGnRnE, and that Stage1 attribute would\r
+  # direct guest writes to host DRAM immediately, bypassing the cache\r
+  # regardless of Stage2 attributes. However, QEMU's reads of the same range\r
+  # can easily be served from the (stale) CPU cache.\r
+  #\r
+  # Setting this PCD to TRUE will use EFI_MEMORY_WB for mapping PCI MMIO\r
+  # regions, which ensures that guest writes to such regions go through the CPU\r
+  # cache. Strictly speaking this is wrong, but it is needed as a temporary\r
+  # workaround for emulated PCI devices. Setting the PCD to FALSE results in\r
+  # the theoretically correct EFI_MEMORY_UC mapping, and should be the long\r
+  # term choice, especially with assigned devices.\r
+  #\r
+  # The default is to turn off the kludge; DSC's can selectively enable it.\r
+  #\r
+  gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|FALSE|BOOLEAN|0x00000006\r

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
index 19776100a5ef3972619da5951b898b1be533f09d..66fe9798c6a94347b0baead53e8ce46688fd8bec 100644 (file)
--- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
+++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
@@ -86,6 +86,9 @@
   #  It could be set FALSE to save size.\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE\r
 \r
   #  It could be set FALSE to save size.\r
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE\r
 \r

+  # Activate KVM workaround for now.\r
+  gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE\r
+\r

 [PcdsFixedAtBuild.common]\r
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F\r
 \r
 [PcdsFixedAtBuild.common]\r
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F\r
 \r

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
index 452465afa8f34b5aa1012e9b94ac1099e8efe5ec..17d4db85bebf4013bd415989d03645fcef9bbb7c 100644 (file)
--- a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
+++ b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
@@ -97,6 +97,7 @@ InitializePciHostBridge (
   IN EFI_SYSTEM_TABLE  *SystemTable\r
   )\r
 {\r
   IN EFI_SYSTEM_TABLE  *SystemTable\r
   )\r
 {\r

+  UINT64                      MmioAttributes;\r

   EFI_STATUS                  Status;\r
   UINTN                       Loop1;\r
   UINTN                       Loop2;\r
   EFI_STATUS                  Status;\r
   UINTN                       Loop1;\r
   UINTN                       Loop2;\r


@@ -133,17 +134,31 @@ InitializePciHostBridge (
                   );\r
   ASSERT_EFI_ERROR (Status);\r
 \r
                   );\r
   ASSERT_EFI_ERROR (Status);\r
 \r

+  MmioAttributes = FeaturePcdGet (PcdKludgeMapPciMmioAsCached) ?\r
+                   EFI_MEMORY_WB : EFI_MEMORY_UC;\r
+\r

   Status = gDS->AddMemorySpace (\r
                   EfiGcdMemoryTypeMemoryMappedIo,\r
                   PcdGet32 (PcdPciMmio32Base),\r
                   PcdGet32 (PcdPciMmio32Size),\r
   Status = gDS->AddMemorySpace (\r
                   EfiGcdMemoryTypeMemoryMappedIo,\r
                   PcdGet32 (PcdPciMmio32Base),\r
                   PcdGet32 (PcdPciMmio32Size),\r

-                  EFI_MEMORY_UC\r
+                  MmioAttributes\r

                   );\r
   if (EFI_ERROR (Status)) {\r
     DEBUG ((EFI_D_ERROR, "%a: AddMemorySpace: %r\n", __FUNCTION__, Status));\r
     return Status;\r
   }\r
 \r
                   );\r
   if (EFI_ERROR (Status)) {\r
     DEBUG ((EFI_D_ERROR, "%a: AddMemorySpace: %r\n", __FUNCTION__, Status));\r
     return Status;\r
   }\r
 \r

+  Status = gDS->SetMemorySpaceAttributes (\r
+                  PcdGet32 (PcdPciMmio32Base),\r
+                  PcdGet32 (PcdPciMmio32Size),\r
+                  MmioAttributes\r
+                  );\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((EFI_D_ERROR, "%a: SetMemorySpaceAttributes: %r\n", __FUNCTION__,\r
+      Status));\r
+    return Status;\r
+  }\r
+\r

   //\r
   // Create Host Bridge Device Handle\r
   //\r
   //\r
   // Create Host Bridge Device Handle\r
   //\r

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
index 5497fa61d2aa23ee32ec04fa65cb145ffd68704f..ecea0882722d32a2df88d6e3c1a106c8a8f27249 100644 (file)
--- a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -24,6 +24,7 @@
 [Packages]\r
   MdePkg/MdePkg.dec\r
   ArmPlatformPkg/ArmPlatformPkg.dec\r
 [Packages]\r
   MdePkg/MdePkg.dec\r
   ArmPlatformPkg/ArmPlatformPkg.dec\r

+  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec\r

 \r
 [LibraryClasses]\r
   UefiDriverEntryPoint\r
 \r
 [LibraryClasses]\r
   UefiDriverEntryPoint\r


@@ -60,5 +61,9 @@
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Size\r
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress\r
 \r
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Size\r
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress\r
 \r

+[FeaturePcd]\r
+  gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached\r
+\r

 [depex]\r
 [depex]\r

-  gEfiMetronomeArchProtocolGuid\r
+  gEfiMetronomeArchProtocolGuid AND\r
+  gEfiCpuArchProtocolGuid\r