On 02/23/15 16:28, Olivier Martin wrote:
> 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]>
Will do, thanks!
Laszlo
>
>
>
> 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
>
------------------------------------------------------------------------------
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
