Re: [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> In ARM/AARCH64 guests that run on KVM, we can now use virtio-gpu-pci, so
> PcdKludgeMapPciMmioAsCached is no longer necessary. Standard VGA continues
> to work on TCG without the kludge.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  ArmVirtPkg/ArmVirtPkg.dec                        | 24 --------------------
>  ArmVirtPkg/ArmVirtQemu.dsc                       |  3 ---
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                 |  3 ---
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |  1 -
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c      |  3 +--
>  5 files changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index 9f98f28f01e1..a5ec42166445 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,36 +58,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # EFI_VT_100_GUID.
>
>    #
>
>    gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
>
>
>
>  [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.
>
> -  #
>
> -  gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|FALSE|BOOLEAN|0x00000006
>
> -
>
> -  #
>
>    # Pure ACPI boot
>
>    #
>
>    # Inhibit installation of the FDT as a configuration table if this feature
>
>    # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
>
>    # description of the platform, and it is up to the OS to choose.
>
>    #
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 2eb2780a1fca..1d459756f61b 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -97,15 +97,12 @@ [PcdsFeatureFlag.common]
>
>
>    ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
>
>    #  It could be set FALSE to save size.
>
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>
>
>
> -  # Activate KVM workaround for now.
>
> -  gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
>
> -
>
>  !if $(PURE_ACPI_BOOT_ENABLE) == TRUE
>
>    gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
>
>  !endif
>
>
>
>  [PcdsFixedAtBuild.common]
>
>    gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index d9e62c78d28e..99d2feba202b 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -98,15 +98,12 @@ [PcdsFeatureFlag.common]
>
>
>    ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
>
>    #  It could be set FALSE to save size.
>
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>
>
>
> -  # Activate KVM workaround for now.
>
> -  gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
>
> -
>
>  [PcdsFixedAtBuild.common]
>
>    gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>
>  !if $(ARCH) == AARCH64
>
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
>
>  !endif
>
>
>
> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index 41e134b24b0e..8c75eda3deb5 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -53,13 +53,12 @@ [Protocols]
>    gFdtClientProtocolGuid                                ## CONSUMES
>
>
>
>  [Pcd]
>
>    gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
>
>
>  [FeaturePcd]
>
> -  gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
>
>    gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
>
>
>
>  [depex]
>
>    gEfiMetronomeArchProtocolGuid AND
>
>    gEfiCpuArchProtocolGuid AND
>
>    gFdtClientProtocolGuid
>
> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> index 4b2b6a562a3f..5063782bb392 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> @@ -362,14 +362,13 @@ InitializePciHostBridge (
>                    EfiGcdIoTypeIo,
>
>                    IoBase,
>
>                    IoSize
>
>                    );
>
>    ASSERT_EFI_ERROR (Status);
>
>
>
> -  MmioAttributes = FeaturePcdGet (PcdKludgeMapPciMmioAsCached) ?
>
> -                   EFI_MEMORY_WB : EFI_MEMORY_UC;
>
> +  MmioAttributes = EFI_MEMORY_UC;
>
>
>
>    Status = gDS->AddMemorySpace (
>
>                    EfiGcdMemoryTypeMemoryMappedIo,
>
>                    MmioBase,
>
>                    MmioSize,
>
>                    MmioAttributes
>
> --
> 2.9.2
>