Actually for this one, would you mind to replace the 'ASSERT_EFI_ERROR
(Status);' (the one after gDS->AddMemorySpace() and the one after
gDS->SetMemorySpaceAttributes()) by:
if (EFI_ERROR (Status)) {
return Status;
}
... or anything similar. It is mainly if the memory map changes between
DEBUG and RELEASE for various reasons, it would be nice to prevent a
crash in RELEASE.
Except this comment:
Reviewed-By: Olivier Martin <[email protected]>
On 23/02/15 14:29, Laszlo Ersek wrote:
> 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 <[email protected]>
> ---
>
> Notes:
> v3:
> - use PcdPciMmio32* [Olivier]
>
> v2:
> - new in v2, after testing VGA on KVM and picking a few brains
>
> ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |
> 7 +++++-
> ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c |
> 13 +++++++++-
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec |
> 25 ++++++++++++++++++++
> ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc |
> 3 +++
> 4 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git
> a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index 5497fa6..ecea088 100644
> ---
> a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++
> b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -24,6 +24,7 @@
> [Packages]
> MdePkg/MdePkg.dec
> ArmPlatformPkg/ArmPlatformPkg.dec
> + ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>
> [LibraryClasses]
> UefiDriverEntryPoint
> @@ -60,5 +61,9 @@
> gArmPlatformTokenSpaceGuid.PcdPciMmio32Size
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
> +[FeaturePcd]
> + gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
> +
> [depex]
> - gEfiMetronomeArchProtocolGuid
> + gEfiMetronomeArchProtocolGuid AND
> + gEfiCpuArchProtocolGuid
> diff --git
> a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
> b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
> index 50c12c2..0aa72a6 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/PciHostBridgeDxe/PciHostBridge.c
> @@ -97,6 +97,7 @@ InitializePciHostBridge (
> IN EFI_SYSTEM_TABLE *SystemTable
> )
> {
> + UINT64 MmioAttributes;
> EFI_STATUS Status;
> UINTN Loop1;
> UINTN Loop2;
> @@ -133,11 +134,21 @@ InitializePciHostBridge (
> );
> ASSERT_EFI_ERROR (Status);
>
> + MmioAttributes = FeaturePcdGet (PcdKludgeMapPciMmioAsCached) ?
> + EFI_MEMORY_WB : EFI_MEMORY_UC;
> +
> Status = gDS->AddMemorySpace (
> EfiGcdMemoryTypeMemoryMappedIo,
> PcdGet32 (PcdPciMmio32Base),
> PcdGet32 (PcdPciMmio32Size),
> - EFI_MEMORY_UC
> + MmioAttributes
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + Status = gDS->SetMemorySpaceAttributes (
> + PcdGet32 (PcdPciMmio32Base),
> + PcdGet32 (PcdPciMmio32Size),
> + MmioAttributes
> );
> ASSERT_EFI_ERROR (Status);
>
> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
> index 9941154..d53dab9 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
> @@ -56,3 +56,28 @@
>
>
> gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress|0x0|UINT64|0x00000004
> gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress|0x0|UINT64|0x00000005
> +
> +[PcdsFeatureFlag]
> + #
> + # "Map PCI MMIO as Cached"
> + #
> + # Due to the way Stage1 and Stage2 mappings are combined on Aarch64, and
> + # because KVM -- for the time being -- does not try to interfere with the
> + # Stage1 mappings, we must not set EFI_MEMORY_UC for emulated PCI MMIO
> + # regions.
> + #
> + # EFI_MEMORY_UC is mapped to Device-nGnRnE, and that Stage1 attribute would
> + # direct guest writes to host DRAM immediately, bypassing the cache
> + # regardless of Stage2 attributes. However, QEMU's reads of the same range
> + # can easily be served from the (stale) CPU cache.
> + #
> + # Setting this PCD to TRUE will use EFI_MEMORY_WB for mapping PCI MMIO
> + # regions, which ensures that guest writes to such regions go through the
> CPU
> + # cache. Strictly speaking this is wrong, but it is needed as a temporary
> + # workaround for emulated PCI devices. Setting the PCD to FALSE results in
> + # the theoretically correct EFI_MEMORY_UC mapping, and should be the long
> + # term choice, especially with assigned devices.
> + #
> + # The default is to turn off the kludge; DSC's can selectively enable it.
> + #
> +
> gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|FALSE|BOOLEAN|0x00000006
> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> index 1977610..66fe979 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> @@ -86,6 +86,9 @@
> # It could be set FALSE to save size.
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>
> + # Activate KVM workaround for now.
> + gArmVirtualizationTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
> +
> [PcdsFixedAtBuild.common]
> gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
>
-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered
in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England & Wales, Company No: 2548782
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
