From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4DA8B21A04E1D for ; Thu, 11 May 2017 08:53:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A964F80C2D; Thu, 11 May 2017 15:53:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A964F80C2D Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com A964F80C2D Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-15.phx2.redhat.com [10.3.116.15]) by smtp.corp.redhat.com (Postfix) with ESMTP id A8AA288B39; Thu, 11 May 2017 15:53:40 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org References: <1494454162-9940-1-git-send-email-brijesh.singh@amd.com> <1494454162-9940-7-git-send-email-brijesh.singh@amd.com> Cc: Thomas.Lendacky@amd.com, Jordan Justen , Jiewen Yao , leo.duran@amd.com From: Laszlo Ersek Message-ID: <1ebdde6a-bad7-ed9a-b2af-7334477c8ae3@redhat.com> Date: Thu, 11 May 2017 17:53:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 11 May 2017 15:53:42 +0000 (UTC) Subject: Re: [RFC v4 06/13] OvmfPkg:AmdSevDxe: add AmdSevDxe driver X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 May 2017 15:53:43 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 05/11/17 17:19, Laszlo Ersek wrote: > comments below: > > On 05/11/17 00:09, Brijesh Singh wrote: >> When SEV is enabled, the MMIO memory range must be mapped as unencrypted >> (i.e C-bit cleared) and DMA must be performed on unencrypted memory. >> >> The patch adds a DXE driver that runs early in boot and clears the memory >> encryption attribute from MMIO/NonExistent memory ranges and installs a >> IOMMU protocol to provide the DMA support for PCIHostBridge and other drivers. > > (1) Please mention that the C bit is cleared for MMIO GCD entries in > order to cover the ranges that were added during the PEI phase (through > memory resource descriptor HOBs). > > Also mention that the NonExistent ranges are processed in order to > cover, in advance, MMIO ranges added later in the DXE phase by various > device drivers, via the appropriate DXE memory space services. > > Finally, please mention that the approach is not transparent for later > addition of system memory ranges to the GCD memory space map. (Such > ranges should be encrypted.) OVMF does not do such a thing at the > moment, so this approach should be OK. > > I think we should also credit Jiewen for both ideas, namely the IOMMU > stuff and the handling of NonExistent ranges (in anticipation of future > MMIO additions), so please add > > Suggested-by: Jiewen Yao (5) Please mention that the driver is being added to the APRIORI DXE file for a separate reason as well (not just for the early clearing of the C bit on MMIO/NonExistent): OvmfPkg's DXE phase modules that tailor their behavior to SEV presence will assume that the IOMMU protocol exported by this driver is available *at once*. (If you fix this up as well, you can add my R-b -- see under (4).) Thanks, Laszlo > >> >> The driver produces IOMMU protocol introduce by Jiewen >> https://lists.01.org/pipermail/edk2-devel/2017-May/010462.html >> >> >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Leo Duran >> Cc: Jiewen Yao >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Brijesh Singh >> --- >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.fdf | 2 + >> OvmfPkg/OvmfPkgX64.fdf | 2 + >> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 49 +++ >> OvmfPkg/AmdSevDxe/AmdSevIommu.h | 43 ++ >> OvmfPkg/AmdSevDxe/AmdSevMmio.h | 41 ++ >> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 52 +++ >> OvmfPkg/AmdSevDxe/AmdSevIommu.c | 459 ++++++++++++++++++++ >> OvmfPkg/AmdSevDxe/AmdSevMmio.c | 50 +++ >> 10 files changed, 700 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 9403f76ce862..ee6f98d68b73 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -827,6 +827,7 @@ [Components.X64] >> !endif >> >> OvmfPkg/PlatformDxe/Platform.inf >> + OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> >> !if $(SMM_REQUIRE) == TRUE >> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index e137143f7afa..b5f26e06e60b 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -825,6 +825,7 @@ [Components] >> !endif >> >> OvmfPkg/PlatformDxe/Platform.inf >> + OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> >> !if $(SMM_REQUIRE) == TRUE >> OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf >> index 5233314139bc..12871860d001 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >> @@ -190,6 +190,7 @@ [FV.DXEFV] >> APRIORI DXE { >> INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf >> INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf >> + INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> !if $(SMM_REQUIRE) == FALSE >> INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf >> !endif >> @@ -351,6 +352,7 @@ [FV.DXEFV] >> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf >> INF OvmfPkg/PlatformDxe/Platform.inf >> +INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> >> !if $(SMM_REQUIRE) == TRUE >> INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >> index 36150101e784..ae6e66a1c08d 100644 >> --- a/OvmfPkg/OvmfPkgX64.fdf >> +++ b/OvmfPkg/OvmfPkgX64.fdf >> @@ -190,6 +190,7 @@ [FV.DXEFV] >> APRIORI DXE { >> INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf >> INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf >> + INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> !if $(SMM_REQUIRE) == FALSE >> INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf >> !endif >> @@ -351,6 +352,7 @@ [FV.DXEFV] >> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf >> INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf >> INF OvmfPkg/PlatformDxe/Platform.inf >> +INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> >> !if $(SMM_REQUIRE) == TRUE >> INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> new file mode 100644 >> index 000000000000..775dda9be386 >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf >> @@ -0,0 +1,49 @@ >> +#/** @file >> +# >> +# Driver clears the encryption attribute from MMIO regions and installs IOMMU >> +# protcol to provides DMA support for PciHostBridge and others >> +# >> +# Copyright (c) 2017, AMD Inc. All rights reserved.
>> +# >> +# This program and the accompanying materials >> +# are licensed and made available under the terms and conditions of the BSD >> +# License which accompanies this distribution. The full text of the license may >> +# be found at http://opensource.org/licenses/bsd-license.php >> +# >> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> +# >> +#**/ >> + >> +[Defines] >> + INF_VERSION = 1.25 >> + BASE_NAME = AmdSevDxe >> + FILE_GUID = 2ec9da37-ee35-4de9-86c5-6d9a81dc38a7 >> + MODULE_TYPE = DXE_DRIVER >> + VERSION_STRING = 1.0 >> + ENTRY_POINT = AmdSevDxeEntryPoint >> + >> +[Sources] >> + AmdSevDxe.c >> + AmdSevIommu.c >> + AmdSevMmio.c >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> + OvmfPkg/OvmfPkg.dec >> + >> +[LibraryClasses] >> + BaseLib >> + UefiLib >> + UefiDriverEntryPoint >> + UefiBootServicesTableLib >> + DxeServicesTableLib >> + DebugLib >> + MemEncryptSevLib >> + >> +[Protocols] >> + gEdkiiIoMmuProtocolGuid ## PRODUCES >> + >> +[Depex] >> + TRUE >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevIommu.h b/OvmfPkg/AmdSevDxe/AmdSevIommu.h >> new file mode 100644 >> index 000000000000..5712cb57052d >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevIommu.h >> @@ -0,0 +1,43 @@ >> +/** @file >> + >> + The protocol provides support to allocate, free, map and umap a DMA buffer for >> + bus master (e.g PciHostBridge). When SEV is enabled, the DMA operations must >> + be performed on unencrypted buffer hence protocol clear the encryption bit >> + from the DMA buffer. >> + >> + Copyright (c) 2017, Intel Corporation. All rights reserved.
>> + Copyright (c) 2017, AMD Inc. All rights reserved.
>> + This program and the accompanying materials are licensed and made available >> + under the terms and conditions of the BSD License which accompanies this >> + distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + >> +**/ >> + >> +#ifndef __AMDSEVIOMMU_H_ >> +#define __AMDSEVIOMMU_H >> + >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/** >> + Install IOMMU protocol to provide the DMA support for PciHostBridge and >> + MemEncryptSevLib. >> + >> +**/ >> +VOID >> +EFIAPI >> +AmdSevInstallIommuProtocol ( >> + VOID >> + ); >> + >> +#endif >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevMmio.h b/OvmfPkg/AmdSevDxe/AmdSevMmio.h >> new file mode 100644 >> index 000000000000..c6191025d921 >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevMmio.h >> @@ -0,0 +1,41 @@ >> +/** @file >> + >> + Implements routines to clear C-bit from MMIO Memory Range >> + >> + Copyright (c) 2017, AMD Inc. All rights reserved.
>> + >> + This program and the accompanying materials are licensed and made available >> + under the terms and conditions of the BSD License which accompanies this >> + distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + >> +**/ >> + >> +#ifndef __AMDSEVMMIO_H_ >> +#define __AMDSEVMMIO_H >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/** >> + >> + Iterate through the GCD map and clear the C-bit from MMIO and NonExistent >> + memory space. The NonExistent memory space will be used for mapping the MMIO >> + space added later (eg PciRootBridge). By clearing both known NonExistent >> + memory space can gurantee that any MMIO mapped later will have C-bit cleared. >> +*/ >> +VOID >> +EFIAPI >> +AmdSevClearEncMaskMmioRange ( >> + VOID >> + ); >> + >> +#endif >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c >> new file mode 100644 >> index 000000000000..e22e7ef7314f >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c >> @@ -0,0 +1,52 @@ >> +/** @file >> + >> + AMD Sev Dxe driver. The driver runs early in DXE phase and clears C-bit from >> + MMIO space and installs EDKII_IOMMU_PROTOCOL to provide the support for DMA >> + operations when SEV is enabled. >> + >> + Copyright (c) 2017, AMD Inc. All rights reserved.
>> + >> + This program and the accompanying materials >> + are licensed and made available under the terms and conditions of the BSD >> + License which accompanies this distribution. The full text of the license may >> + be found at http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + >> +**/ >> + >> +#include >> + >> +#include >> + >> +#include "AmdSevMmio.h" >> +#include "AmdSevIommu.h" >> + >> +EFI_STATUS >> +EFIAPI >> +AmdSevDxeEntryPoint ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + // >> + // Do nothing when SEV is not enabled >> + // >> + if (!MemEncryptSevIsEnabled ()) { >> + return EFI_SUCCESS; >> + } > > (2) The status code should be EFI_UNSUPPORTED or EFI_ABORTED. Returning > with EFI_SUCCESS will uselessly keep the driver in memory. > >> + >> + // >> + // Clear C-bit from MMIO Memory Range >> + // >> + AmdSevClearEncMaskMmioRange (); >> + >> + // >> + // Install IOMMU protocol to provide DMA support for PCIHostBridgeIo and >> + // AmdSevMemEncryptLib. > > (3) What is AmdSevMemEncryptLib? Is this comment perhaps stale? > >> + // >> + AmdSevInstallIommuProtocol (); >> + >> + return EFI_SUCCESS; >> +} >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevIommu.c b/OvmfPkg/AmdSevDxe/AmdSevIommu.c >> new file mode 100644 >> index 000000000000..9b35469ca34f >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevIommu.c >> @@ -0,0 +1,459 @@ >> +/** @file >> + AmdSevIommu related function >> + >> + The protocol provides support to allocate, free, map and umap a DMA buffer for >> + bus master (e.g PciHostBridge). When SEV is enabled, the DMA operations must >> + be performed on unencrypted buffer hence we use a bounce buffer to map the host >> + buffer into an unencrypted buffer. >> + >> + Copyright (c) 2017, AMD Inc. All rights reserved.
>> + >> + This program and the accompanying materials are licensed and made available >> + under the terms and conditions of the BSD License which accompanies this >> + distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + >> +**/ >> + >> +#include "AmdSevIommu.h" >> + >> +typedef struct { >> + EDKII_IOMMU_OPERATION Operation; >> + UINTN NumberOfBytes; >> + UINTN NumberOfPages; >> + EFI_PHYSICAL_ADDRESS HostAddress; >> + EFI_PHYSICAL_ADDRESS DeviceAddress; >> +} MAP_INFO; >> + >> +#define NO_MAPPING (VOID *) (UINTN) -1 >> + >> +/** >> + Provides the controller-specific addresses required to access system memory from a >> + DMA bus master. On SEV guest, the DMA operations must be performed on shared >> + buffer hence we allocate a bounce buffer to map the HostAddress to a DeviceAddress. >> + The Encryption attribute is removed from the DeviceAddress buffer. >> + >> + @param This The protocol instance pointer. >> + @param Operation Indicates if the bus master is going to read or >> + write to system memory. >> + @param HostAddress The system memory address to map to the PCI controller. >> + @param NumberOfBytes On input the number of bytes to map. On output >> + the number of bytes >> + that were mapped. >> + @param DeviceAddress The resulting map address for the bus master PCI >> + controller to use to >> + access the hosts HostAddress. >> + @param Mapping A resulting value to pass to Unmap(). >> + >> + @retval EFI_SUCCESS The range was mapped for the returned NumberOfBytes. >> + @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a common buffer. >> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >> + @retval EFI_OUT_OF_RESOURCES The request could not be completed due to a lack >> + of resources. >> + @retval EFI_DEVICE_ERROR The system hardware could not map the requested address. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +IoMmuMap ( >> + IN EDKII_IOMMU_PROTOCOL *This, >> + IN EDKII_IOMMU_OPERATION Operation, >> + IN VOID *HostAddress, >> + IN OUT UINTN *NumberOfBytes, >> + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, >> + OUT VOID **Mapping >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS PhysicalAddress; >> + MAP_INFO *MapInfo; >> + EFI_PHYSICAL_ADDRESS DmaMemoryTop; >> + EFI_ALLOCATE_TYPE AllocateType; >> + >> + if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL || >> + Mapping == NULL) { >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + // >> + // Make sure that Operation is valid >> + // >> + if ((UINT32) Operation >= EdkiiIoMmuOperationMaximum) { >> + return EFI_INVALID_PARAMETER; >> + } >> + PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress; >> + >> + DmaMemoryTop = (UINTN)-1; >> + AllocateType = AllocateAnyPages; >> + >> + if (((Operation != EdkiiIoMmuOperationBusMasterRead64 && >> + Operation != EdkiiIoMmuOperationBusMasterWrite64 && >> + Operation != EdkiiIoMmuOperationBusMasterCommonBuffer64)) && >> + ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) { >> + // >> + // If the root bridge or the device cannot handle performing DMA above >> + // 4GB but any part of the DMA transfer being mapped is above 4GB, then >> + // map the DMA transfer to a buffer below 4GB. >> + // >> + DmaMemoryTop = SIZE_4GB - 1; >> + AllocateType = AllocateMaxAddress; >> + >> + if (Operation == EdkiiIoMmuOperationBusMasterCommonBuffer || >> + Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) { >> + // >> + // Common Buffer operations can not be remapped. If the common buffer >> + // if above 4GB, then it is not possible to generate a mapping, so return >> + // an error. >> + // >> + return EFI_UNSUPPORTED; >> + } >> + } >> + >> + // >> + // CommandBuffer was allocated by us (AllocateBuffer) and is already in >> + // unencryted buffer so no need to create bounce buffer >> + // >> + if (Operation == EdkiiIoMmuOperationBusMasterCommonBuffer || >> + Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) { >> + *Mapping = NO_MAPPING; >> + *DeviceAddress = PhysicalAddress; >> + >> + return EFI_SUCCESS; >> + } >> + >> + // >> + // Allocate a MAP_INFO structure to remember the mapping when Unmap() is >> + // called later. >> + // >> + MapInfo = AllocatePool (sizeof (MAP_INFO)); >> + if (MapInfo == NULL) { >> + *NumberOfBytes = 0; >> + return EFI_OUT_OF_RESOURCES; >> + } >> + >> + // >> + // Initialize the MAP_INFO structure >> + // >> + MapInfo->Operation = Operation; >> + MapInfo->NumberOfBytes = *NumberOfBytes; >> + MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes); >> + MapInfo->HostAddress = PhysicalAddress; >> + MapInfo->DeviceAddress = DmaMemoryTop; >> + >> + // >> + // Allocate a buffer to map the transfer to. >> + // >> + Status = gBS->AllocatePages ( >> + AllocateType, >> + EfiBootServicesData, >> + MapInfo->NumberOfPages, >> + &MapInfo->DeviceAddress >> + ); >> + if (EFI_ERROR (Status)) { >> + FreePool (MapInfo); >> + *NumberOfBytes = 0; >> + return Status; >> + } >> + >> + // >> + // Clear the memory encryption mask from the device buffer >> + // >> + Status = MemEncryptSevClearPageEncMask (0, MapInfo->DeviceAddress, MapInfo->NumberOfPages, TRUE); >> + ASSERT_EFI_ERROR(Status); >> + >> + // >> + // If this is a read operation from the Bus Master's point of view, >> + // then copy the contents of the real buffer into the mapped buffer >> + // so the Bus Master can read the contents of the real buffer. >> + // >> + if (Operation == EdkiiIoMmuOperationBusMasterRead || >> + Operation == EdkiiIoMmuOperationBusMasterRead64) { >> + CopyMem ( >> + (VOID *) (UINTN) MapInfo->DeviceAddress, >> + (VOID *) (UINTN) MapInfo->HostAddress, >> + MapInfo->NumberOfBytes >> + ); >> + } >> + >> + // >> + // The DeviceAddress is the address of the maped buffer below 4GB >> + // >> + *DeviceAddress = MapInfo->DeviceAddress; >> + >> + // >> + // Return a pointer to the MAP_INFO structure in Mapping >> + // >> + *Mapping = MapInfo; >> + >> + DEBUG ((DEBUG_VERBOSE, "%a Host 0x%Lx Device 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n", >> + __FUNCTION__, MapInfo->DeviceAddress, MapInfo->HostAddress, >> + MapInfo->NumberOfPages, MapInfo->NumberOfBytes)); >> + >> + return EFI_SUCCESS; >> +} >> + >> +/** >> + Completes the Map() operation and releases any corresponding resources. >> + >> + @param This The protocol instance pointer. >> + @param Mapping The mapping value returned from Map(). >> + >> + @retval EFI_SUCCESS The range was unmapped. >> + @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map(). >> + @retval EFI_DEVICE_ERROR The data was not committed to the target system memory. >> +**/ >> +EFI_STATUS >> +EFIAPI >> +IoMmuUnmap ( >> + IN EDKII_IOMMU_PROTOCOL *This, >> + IN VOID *Mapping >> + ) >> +{ >> + MAP_INFO *MapInfo; >> + EFI_STATUS Status; >> + >> + if (Mapping == NULL) { >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + // >> + // See if the Map() operation associated with this Unmap() required a mapping >> + // buffer. If a mapping buffer was not required, then this function simply >> + // buffer. If a mapping buffer was not required, then this function simply >> + // >> + if (Mapping == NO_MAPPING) { >> + return EFI_SUCCESS; >> + } >> + >> + MapInfo = (MAP_INFO *)Mapping; >> + >> + // >> + // If this is a write operation from the Bus Master's point of view, >> + // then copy the contents of the mapped buffer into the real buffer >> + // so the processor can read the contents of the real buffer. >> + // >> + if (MapInfo->Operation == EdkiiIoMmuOperationBusMasterWrite || >> + MapInfo->Operation == EdkiiIoMmuOperationBusMasterWrite64) { >> + CopyMem ( >> + (VOID *) (UINTN) MapInfo->HostAddress, >> + (VOID *) (UINTN) MapInfo->DeviceAddress, >> + MapInfo->NumberOfBytes >> + ); >> + } >> + >> + DEBUG ((DEBUG_VERBOSE, "%a Host 0x%Lx Device 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n", >> + __FUNCTION__, MapInfo->DeviceAddress, MapInfo->HostAddress, >> + MapInfo->NumberOfPages, MapInfo->NumberOfBytes)); >> + // >> + // Restore the memory encryption mask >> + // >> + Status = MemEncryptSevSetPageEncMask (0, MapInfo->DeviceAddress, MapInfo->NumberOfPages, TRUE); >> + ASSERT_EFI_ERROR(Status); >> + >> + // >> + // Free the mapped buffer and the MAP_INFO structure. >> + // >> + gBS->FreePages (MapInfo->DeviceAddress, MapInfo->NumberOfPages); >> + FreePool (Mapping); >> + return EFI_SUCCESS; >> +} >> + >> +/** >> + Allocates pages that are suitable for an OperationBusMasterCommonBuffer or >> + OperationBusMasterCommonBuffer64 mapping. >> + >> + @param This The protocol instance pointer. >> + @param Type This parameter is not used and must be ignored. >> + @param MemoryType The type of memory to allocate, EfiBootServicesData >> + or EfiRuntimeServicesData. >> + @param Pages The number of pages to allocate. >> + @param HostAddress A pointer to store the base system memory address >> + of the allocated range. >> + @param Attributes The requested bit mask of attributes for the allocated range. >> + >> + @retval EFI_SUCCESS The requested memory pages were allocated. >> + @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute >> + bits are MEMORY_WRITE_COMBINE and MEMORY_CACHED. >> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. >> + @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +IoMmuAllocateBuffer ( >> + IN EDKII_IOMMU_PROTOCOL *This, >> + IN EFI_ALLOCATE_TYPE Type, >> + IN EFI_MEMORY_TYPE MemoryType, >> + IN UINTN Pages, >> + IN OUT VOID **HostAddress, >> + IN UINT64 Attributes >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS PhysicalAddress; >> + >> + // >> + // Validate Attributes >> + // >> + if ((Attributes & EDKII_IOMMU_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER) != 0) { >> + return EFI_UNSUPPORTED; >> + } >> + >> + // >> + // Check for invalid inputs >> + // >> + if (HostAddress == NULL) { >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + // >> + // The only valid memory types are EfiBootServicesData and >> + // EfiRuntimeServicesData >> + // >> + if (MemoryType != EfiBootServicesData && >> + MemoryType != EfiRuntimeServicesData) { >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + PhysicalAddress = (UINTN)-1; >> + if ((Attributes & EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) { >> + // >> + // Limit allocations to memory below 4GB >> + // >> + PhysicalAddress = SIZE_4GB - 1; >> + } >> + Status = gBS->AllocatePages ( >> + AllocateMaxAddress, >> + MemoryType, >> + Pages, >> + &PhysicalAddress >> + ); >> + if (!EFI_ERROR (Status)) { >> + *HostAddress = (VOID *) (UINTN) PhysicalAddress; >> + >> + // >> + // Clear memory encryption mask >> + // >> + Status = MemEncryptSevClearPageEncMask (0, PhysicalAddress, Pages, TRUE); >> + ASSERT_EFI_ERROR(Status); >> + } >> + >> + DEBUG ((DEBUG_VERBOSE, "%a Address 0x%Lx Pages 0x%Lx\n", __FUNCTION__, PhysicalAddress, Pages)); >> + return Status; >> +} >> + >> +/** >> + Frees memory that was allocated with AllocateBuffer(). >> + >> + @param This The protocol instance pointer. >> + @param Pages The number of pages to free. >> + @param HostAddress The base system memory address of the allocated range. >> + >> + @retval EFI_SUCCESS The requested memory pages were freed. >> + @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages >> + was not allocated with AllocateBuffer(). >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +IoMmuFreeBuffer ( >> + IN EDKII_IOMMU_PROTOCOL *This, >> + IN UINTN Pages, >> + IN VOID *HostAddress >> + ) >> +{ >> + EFI_STATUS Status; >> + >> + // >> + // Set memory encryption mask >> + // >> + Status = MemEncryptSevSetPageEncMask (0, (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress, Pages, TRUE); >> + ASSERT_EFI_ERROR(Status); >> + >> + DEBUG ((DEBUG_VERBOSE, "%a Address 0x%Lx Pages 0x%Lx\n", __FUNCTION__, (UINTN)HostAddress, Pages)); >> + return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages); >> +} >> + >> + >> +/** >> + Set IOMMU attribute for a system memory. >> + >> + If the IOMMU protocol exists, the system memory cannot be used >> + for DMA by default. >> + >> + When a device requests a DMA access for a system memory, >> + the device driver need use SetAttribute() to update the IOMMU >> + attribute to request DMA access (read and/or write). >> + >> + The DeviceHandle is used to identify which device submits the request. >> + The IOMMU implementation need translate the device path to an IOMMU device ID, >> + and set IOMMU hardware register accordingly. >> + 1) DeviceHandle can be a standard PCI device. >> + The memory for BusMasterRead need set EDKII_IOMMU_ACCESS_READ. >> + The memory for BusMasterWrite need set EDKII_IOMMU_ACCESS_WRITE. >> + The memory for BusMasterCommonBuffer need set EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE. >> + After the memory is used, the memory need set 0 to keep it being protected. >> + 2) DeviceHandle can be an ACPI device (ISA, I2C, SPI, etc). >> + The memory for DMA access need set EDKII_IOMMU_ACCESS_READ and/or EDKII_IOMMU_ACCESS_WRITE. >> + >> + @param[in] This The protocol instance pointer. >> + @param[in] DeviceHandle The device who initiates the DMA access request. >> + @param[in] Mapping The mapping value returned from Map(). >> + @param[in] IoMmuAccess The IOMMU access. >> + >> + @retval EFI_SUCCESS The IoMmuAccess is set for the memory range specified by DeviceAddress and Length. >> + @retval EFI_INVALID_PARAMETER DeviceHandle is an invalid handle. >> + @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map(). >> + @retval EFI_INVALID_PARAMETER IoMmuAccess specified an illegal combination of access. >> + @retval EFI_UNSUPPORTED DeviceHandle is unknown by the IOMMU. >> + @retval EFI_UNSUPPORTED The bit mask of IoMmuAccess is not supported by the IOMMU. >> + @retval EFI_UNSUPPORTED The IOMMU does not support the memory range specified by Mapping. >> + @retval EFI_OUT_OF_RESOURCES There are not enough resources available to modify the IOMMU access. >> + @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +IoMmuSetAttribute ( >> + IN EDKII_IOMMU_PROTOCOL *This, >> + IN EFI_HANDLE DeviceHandle, >> + IN VOID *Mapping, >> + IN UINT64 IoMmuAccess >> + ) >> +{ >> + return EFI_UNSUPPORTED; >> +} >> + >> +EDKII_IOMMU_PROTOCOL mAmdSev = { >> + EDKII_IOMMU_PROTOCOL_REVISION, >> + IoMmuSetAttribute, >> + IoMmuMap, >> + IoMmuUnmap, >> + IoMmuAllocateBuffer, >> + IoMmuFreeBuffer, >> +}; >> + >> +/** >> + Initialize Iommu Protocol. >> + >> +**/ >> +VOID >> +EFIAPI >> +AmdSevInstallIommuProtocol ( >> + VOID >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_HANDLE Handle; >> + >> + Handle = NULL; >> + Status = gBS->InstallMultipleProtocolInterfaces ( >> + &Handle, >> + &gEdkiiIoMmuProtocolGuid, &mAmdSev, >> + NULL >> + ); >> + ASSERT_EFI_ERROR (Status); >> +} >> diff --git a/OvmfPkg/AmdSevDxe/AmdSevMmio.c b/OvmfPkg/AmdSevDxe/AmdSevMmio.c >> new file mode 100644 >> index 000000000000..b623f82b7baa >> --- /dev/null >> +++ b/OvmfPkg/AmdSevDxe/AmdSevMmio.c >> @@ -0,0 +1,50 @@ >> +/** @file >> + >> + Implements routines to clear C-bit from MMIO Memory Range >> + >> + Copyright (c) 2017, AMD Inc. All rights reserved.
>> + >> + This program and the accompanying materials >> + are licensed and made available under the terms and conditions of the BSD License >> + which accompanies this distribution. The full text of the license may be found at >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. >> + >> +**/ >> + >> +#include "AmdSevMmio.h" >> + >> +/** >> + >> + Iterate through the GCD map and clear the C-bit from MMIO and NonExistent >> + memory space. The NonExistent memory space will be used for mapping the MMIO >> + space added later (eg PciRootBridge). By clearing both known NonExistent >> + memory space can gurantee that any MMIO mapped later will have C-bit cleared. >> +*/ >> +VOID >> +EFIAPI >> +AmdSevClearEncMaskMmioRange ( >> + VOID >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *AllDescMap; >> + UINTN NumEntries; >> + UINTN Index; >> + >> + Status = gDS->GetMemorySpaceMap (&NumEntries, &AllDescMap); >> + if (Status == EFI_SUCCESS) { >> + for (Index = 0; Index < NumEntries; Index++) { >> + CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Desc; >> + >> + Desc = &AllDescMap[Index]; >> + if (Desc->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo || >> + Desc->GcdMemoryType == EfiGcdMemoryTypeNonExistent) { >> + Status = MemEncryptSevClearPageEncMask (0, Desc->BaseAddress, EFI_SIZE_TO_PAGES(Desc->Length), FALSE); >> + ASSERT_EFI_ERROR(Status); >> + } >> + } > > (4) Right here I think you have a memory leak; gDS->GetMemorySpaceMap() > allocates AllDescMap dynamically (on success). Please free it with > FreePool(). > > > Regarding the IOMMU protocol implementation, I'm going to rely on > Jiewen's review -- thank you Jiewen very much for that! > > With the above fixed: > > Reviewed-by: Laszlo Ersek > > Thanks, > Laszlo > > >> + } >> +} >> >