The patch adds SEV support in QemuFwCfgLib. When SEV is enabled:
* Pei phase support IO-style operations. This is mainly because we need to
use a bounce buffer inorder to support DMA operation. Allocate a memory
for bounce buffer can get painful in Pei phase hence if we detect FWCfg DMA
support then silently fallback to IO.
* Dxe phase supports both IO and DMA style operations.
---
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c | 73 +++++++++++++
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 112 ++++++++++++++++++++
.../Library/QemuFwCfgLib/QemuFwCfgLibInternal.h | 38 +++++++
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c | 93 +++++++++++++++++
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf | 2
OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c | 82 +++++++++++++++
7 files changed, 402 insertions(+)
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
index ac05f4c..be8e945 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
@@ -4,6 +4,7 @@
Copyright (C) 2013, Red Hat, Inc.
Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available
under the terms and conditions of the BSD License which accompanies this
@@ -14,14 +15,34 @@
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include "Uefi.h"
+
+#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/QemuFwCfgLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/BmDmaLib.h>
#include "QemuFwCfgLibInternal.h"
STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
STATIC BOOLEAN mQemuFwCfgDmaSupported;
+STATIC BOOLEAN mQemuFwCfgSevIsEnabled = FALSE;
+
+/**
+ Returns a boolean indicating whether the SEV is enabled
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+**/
+BOOLEAN
+InternalQemuFwCfgSevIsEnabled (
+ VOID
+ )
+{
+ return mQemuFwCfgSevIsEnabled;
+}
/**
Returns a boolean indicating if the firmware configuration interface
@@ -79,6 +100,9 @@ QemuFwCfgInitialize (
mQemuFwCfgDmaSupported = TRUE;
DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
}
+
+ mQemuFwCfgSevIsEnabled = MemEncryptSevIsEnabled ();
+
return RETURN_SUCCESS;
}
@@ -114,3 +138,52 @@ InternalQemuFwCfgDmaIsAvailable (
{
return mQemuFwCfgDmaSupported;
}
+
+/**
+ Allocate a bounce buffer for SEV DMA.
+
+ @param[in] NumPage Number of pages.
+ @param[out] Buffer Allocated DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaAllocateBuffer (
+ IN UINT32 NumPages,
+ OUT VOID **Buffer
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Allocate DMA bounce buffer
+ //
+ Status = BmDmaAllocateBuffer (TRUE, EfiBootServicesData, NumPages, Buffer);
+ if (EFI_ERROR(Status)) {
+ DEBUG ((EFI_D_ERROR, "SEV: Failed to allocate bounce buffer %d pages\n", NumPages));
+ ASSERT_EFI_ERROR (Status);
+ CpuDeadLoop ();
+ }
+
+ DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma allocate buffer 0x%Lx Pages %d\n", (UINTN)Buffer, NumPages));
+}
+
+/**
+ Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+
+ @param[in] NumPage Number of pages.
+ @param[in] Buffer DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaFreeBuffer (
+ IN VOID *Buffer,
+ IN UINT32 NumPages
+ )
+{
+ //
+ // Free the bounce buffer
+ //
+ DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma free buffer 0x%Lx Pages %d\n", (UINTN)Buffer, NumPages));
+ BmDmaFreeBuffer (Buffer, NumPages);
+}
+
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
index 346bb88..536887f 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
@@ -47,4 +47,6 @@
DebugLib
IoLib
MemoryAllocationLib
+ MemEncryptSevLib
+ BmDmaLib
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
index 1bf725d..d2560a3 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
@@ -47,6 +47,111 @@ QemuFwCfgSelectItem (
/**
+ Transfer an array of bytes, or skip a number of bytes, using the SEV DMA bounce
+ interface. The function is same as InternalQemuFwCfgDmaBytes with excpetion that
+ it uses bounce buffer
+
+ @param[in] Size Size in bytes to transfer or skip.
+
+ @param[in,out] HostBuffer Buffer to read data into or write data from. Ignored,
+ and may be NULL, if Size is zero, or Control is
+ FW_CFG_DMA_CTL_SKIP.
+
+ @param[in] Control One of the following:
+ FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
+ FW_CFG_DMA_CTL_READ - read from fw_cfg into Buffer.
+ FW_CFG_DMA_CTL_SKIP - skip bytes in fw_cfg.
+**/
+VOID
+InternalQemuFwCfgSevDmaBytes (
+ IN UINT32 Size,
+ IN OUT VOID *HostBuffer OPTIONAL,
+ IN UINT32 Control
+ )
+{
+ volatile FW_CFG_DMA_ACCESS *Access;
+ UINT32 AccessHigh, AccessLow;
+ UINT32 Status;
+ UINT32 NumPages;
+ VOID *DmaBuffer, *Buffer;
+
+ //
+ // Calculate number of pages we need to allocate for this operation
+ //
+ if (Control == FW_CFG_DMA_CTL_SKIP) {
+ //
+ // Control data does not need the actual buffer
+ //
+ NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access));
+ } else {
+ NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access) + Size);
+ }
+
+ //
+ // Allocate DMA bounce buffer
+ //
+ InternalQemuFwCfgSevDmaAllocateBuffer (NumPages, &DmaBuffer);
+
+ Access = (FW_CFG_DMA_ACCESS *)DmaBuffer;
+ Buffer = DmaBuffer + sizeof(*Access);
+
+ Access->Control = SwapBytes32 (Control);
+ Access->Length = SwapBytes32 (Size);
+ Access->Address = SwapBytes64 ((UINTN)Buffer);
+
+ //
+ // Copy data from Host buffer into DMA buffer
+ //
+ if (HostBuffer && (Control == FW_CFG_DMA_CTL_WRITE)) {
+ CopyMem (Buffer, HostBuffer, Size);
+ }
+
+ //
+ // Delimit the transfer from (a) modifications to Access, (b) in case of a
+ // write, from writes to Buffer by the caller.
+ //
+ MemoryFence ();
+
+ //
+ // Start the transfer.
+ //
+ AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
+ AccessLow = (UINT32)(UINTN)Access;
+ IoWrite32 (FW_CFG_IO_DMA_ADDRESS, SwapBytes32 (AccessHigh));
+ IoWrite32 (FW_CFG_IO_DMA_ADDRESS + 4, SwapBytes32 (AccessLow));
+
+ //
+ // Don't look at Access->Control before starting the transfer.
+ //
+ MemoryFence ();
+
+ //
+ // Wait for the transfer to complete.
+ //
+ do {
+ Status = SwapBytes32 (Access->Control);
+ ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
+ } while (Status != 0);
+
+ //
+ // After a read, the caller will want to use Buffer.
+ //
+ MemoryFence ();
+
+ //
+ // Copy data from DMA buffer into Host Buffer
+ //
+ if (HostBuffer && (Control == FW_CFG_DMA_CTL_READ)) {
+ CopyMem (HostBuffer, Buffer, Size);
+ }
+
+ //
+ // Free the DMA bounce buffer
+ //
+ InternalQemuFwCfgSevDmaFreeBuffer (DmaBuffer, NumPages);
+}
+
+/**
Transfer an array of bytes, or skip a number of bytes, using the DMA
interface.
@@ -79,6 +184,13 @@ InternalQemuFwCfgDmaBytes (
return;
}
+ //
+ // When SEV is enabled then use SEV version of DmaReadWrite
+ //
+ if (InternalQemuFwCfgSevIsEnabled ()) {
+ return InternalQemuFwCfgSevDmaBytes (Size, Buffer, Control);
+ }
+
Access.Control = SwapBytes32 (Control);
Access.Length = SwapBytes32 (Size);
Access.Address = SwapBytes64 ((UINTN)Buffer);
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
index 6e87c62..8e2ff45 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
@@ -2,6 +2,7 @@
Internal interfaces specific to the QemuFwCfgLib instances in OvmfPkg.
Copyright (C) 2016, Red Hat, Inc.
+ Copyright (C) 2017, Advanced Micro Devices.
This program and the accompanying materials are licensed and made available
under the terms and conditions of the BSD License which accompanies this
@@ -43,4 +44,41 @@ InternalQemuFwCfgDmaIsAvailable (
VOID
);
+/**
+ Returns a boolean indicating whether the SEV is enabled
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+**/
+BOOLEAN
+InternalQemuFwCfgSevIsEnabled (
+ VOID
+ );
+
+/**
+ Allocate a bounce buffer for SEV DMA.
+
+ @param[in] NumPage Number of pages.
+ @param[out] Buffer Allocated DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaAllocateBuffer (
+ IN UINT32 NumPages,
+ OUT VOID **Buffer
+ );
+
+/**
+ Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+
+ @param[in] NumPage Number of pages.
+ @param[in] Buffer DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaFreeBuffer (
+ IN VOID *Buffer,
+ IN UINT32 NumPages
+ );
+
#endif
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
index ac05f4c..3dc9270 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
@@ -4,6 +4,7 @@
Copyright (C) 2013, Red Hat, Inc.
Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available
under the terms and conditions of the BSD License which accompanies this
@@ -14,14 +15,55 @@
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/QemuFwCfgLib.h>
+#include <Register/Cpuid.h>
+#include <Register/AmdSevMap.h>
#include "QemuFwCfgLibInternal.h"
STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
STATIC BOOLEAN mQemuFwCfgDmaSupported;
+/**
+ Returns a boolean indicating whether the SEV is enabled
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+**/
+BOOLEAN
+InternalQemuFwCfgSevIsEnabled (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ MSR_SEV_STATUS_REGISTER Msr;
+ CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
+
+ //
+ // Check if memory encryption leaf exist
+ //
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
+ //
+ // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
+ //
+ AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL);
+
+ if (Eax.Bits.SevBit) {
+ //
+ // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
+ //
+ Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+ if (Msr.Bits.SevBit) {
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
+}
/**
Returns a boolean indicating if the firmware configuration interface
@@ -79,6 +121,17 @@ QemuFwCfgInitialize (
mQemuFwCfgDmaSupported = TRUE;
DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
}
+
+ //
+ // When SEV is enabled then we do not support DMA interface.
+ // This is because we need to use bounce buffer to support DMA operation in SEV guest.
+ // Allocating memory for bounce buffer can get painful in Pei phase
+ //
+ if (mQemuFwCfgDmaSupported && InternalQemuFwCfgSevIsEnabled ()) {
+ mQemuFwCfgDmaSupported = FALSE;
+ DEBUG ((DEBUG_INFO, "QemuFwCfg disabling DMA interface and defaulting to IO Port.\n"));
+ }
+
return RETURN_SUCCESS;
}
@@ -114,3 +167,43 @@ InternalQemuFwCfgDmaIsAvailable (
{
return mQemuFwCfgDmaSupported;
}
+
+/**
+ Allocate a bounce buffer for SEV DMA.
+
+ @param[in] NumPage Number of pages.
+ @param[out] Buffer Allocated DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaAllocateBuffer (
+ IN UINT32 NumPages,
+ OUT VOID **Buffer
+ )
+{
+ //
+ // We should never reach here
+ //
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+
+/**
+ Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+
+ @param[in] NumPage Number of pages.
+ @param[in] Buffer DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaFreeBuffer (
+ IN VOID *Buffer,
+ IN UINT32 NumPages
+ )
+{
+ //
+ // We should never reach here
+ //
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
index 4f966a8..83cc0de 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
@@ -39,7 +39,9 @@
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
OvmfPkg/OvmfPkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
[LibraryClasses]
BaseLib
diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
index 465ccbe..70b0a47 100644
--- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
+++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
@@ -16,8 +16,11 @@
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/BaseLib.h>
#include <Library/DebugLib.h>
#include <Library/QemuFwCfgLib.h>
+#include <Register/Cpuid.h>
+#include <Register/AmdSevMap.h>
#include "QemuFwCfgLibInternal.h"
@@ -94,3 +97,82 @@ InternalQemuFwCfgDmaIsAvailable (
{
return FALSE;
}
+
+/**
+ Returns a boolean indicating whether the SEV is enabled
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+**/
+BOOLEAN
+InternalQemuFwCfgSevIsEnabled (
+ VOID
+ )
+{
+ UINT32 RegEax;
+ MSR_SEV_STATUS_REGISTER Msr;
+ CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
+
+ //
+ // Check if memory encryption leaf exist
+ //
+ AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
+ if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
+ //
+ // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
+ //
+ AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL);
+
+ if (Eax.Bits.SevBit) {
+ //
+ // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
+ //
+ Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
+ if (Msr.Bits.SevBit) {
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
+}
+
+/**
+ Allocate a bounce buffer for SEV DMA.
+
+ @param[in] NumPage Number of pages.
+ @param[out] Buffer Allocated DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaAllocateBuffer (
+ IN UINT32 NumPages,
+ OUT VOID **Buffer
+ )
+{
+ //
+ // We should never reach here
+ //
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
+
+/**
+ Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
+
+ @param[in] NumPage Number of pages.
+ @param[in] Buffer DMA Buffer pointer
+
+**/
+VOID
+InternalQemuFwCfgSevDmaFreeBuffer (
+ IN VOID *Buffer,
+ IN UINT32 NumPages
+ )
+{
+ //
+ // We should never reach here
+ //
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+}
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/21/17 22:13, Brijesh Singh wrote:
> The patch adds SEV support in QemuFwCfgLib. When SEV is enabled:
>
> * Pei phase support IO-style operations. This is mainly because we need to
> use a bounce buffer inorder to support DMA operation. Allocate a memory
> for bounce buffer can get painful in Pei phase hence if we detect FWCfg DMA
> support then silently fallback to IO.
>
> * Dxe phase supports both IO and DMA style operations.
> ---
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c | 73 +++++++++++++
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 112 ++++++++++++++++++++
> .../Library/QemuFwCfgLib/QemuFwCfgLibInternal.h | 38 +++++++
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c | 93 +++++++++++++++++
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf | 2
> OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c | 82 +++++++++++++++
> 7 files changed, 402 insertions(+)
Can you please split this patch into 5 patches?
- new interfaces in QemuFwCfgLibInternal.h
- implementation of new APIs for SEC instance
- implementation of new APIs for PEI instance
- implementation of new APIs for DXE instance
- call new APIs from common code
If that is possible. Otherwise, it's quite hard to review & compare the
instances for the different firmware phases.
Anyway, some things I've noticed:
- InternalQemuFwCfgSevIsEnabled() should use global variables in the PEI
instance as well. Possibly the appropriate interface from
BaseMemcryptSevLib could be called from both the PEI and DXE instances
here. (I think the DXE instance already does that.)
- regarding InternalQemuFwCfgDmaBytes(): would it be possible to hook
the SEV-related tricks into the existent function, rather than
copy+modify the entire function as InternalQemuFwCfgSevDmaBytes()? It
seems to me that a conditional "prologue" and "epilogue" in
InternalQemuFwCfgDmaBytes() could do the trick.
... From skimming this patch, I think those are the only
functionality-related comments I have at this point, beyond the remarks
I made elsewhere in this series (like: line length, DEBUG_* macros, and
so on). Please recheck all patches for those comments.
Thanks!
Laszlo
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> index ac05f4c..be8e945 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> @@ -4,6 +4,7 @@
>
> Copyright (C) 2013, Red Hat, Inc.
> Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
>
> This program and the accompanying materials are licensed and made available
> under the terms and conditions of the BSD License which accompanies this
> @@ -14,14 +15,34 @@
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> **/
>
> +#include "Uefi.h"
> +
> +#include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> #include <Library/QemuFwCfgLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/MemEncryptSevLib.h>
> +#include <Library/BmDmaLib.h>
>
> #include "QemuFwCfgLibInternal.h"
>
> STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
> STATIC BOOLEAN mQemuFwCfgDmaSupported;
> +STATIC BOOLEAN mQemuFwCfgSevIsEnabled = FALSE;
> +
> +/**
> + Returns a boolean indicating whether the SEV is enabled
>
> + @retval TRUE SEV is enabled
> + @retval FALSE SEV is not enabled
> +**/
> +BOOLEAN
> +InternalQemuFwCfgSevIsEnabled (
> + VOID
> + )
> +{
> + return mQemuFwCfgSevIsEnabled;
> +}
>
> /**
> Returns a boolean indicating if the firmware configuration interface
> @@ -79,6 +100,9 @@ QemuFwCfgInitialize (
> mQemuFwCfgDmaSupported = TRUE;
> DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
> }
> +
> + mQemuFwCfgSevIsEnabled = MemEncryptSevIsEnabled ();
> +
> return RETURN_SUCCESS;
> }
>
> @@ -114,3 +138,52 @@ InternalQemuFwCfgDmaIsAvailable (
> {
> return mQemuFwCfgDmaSupported;
> }
> +
> +/**
> + Allocate a bounce buffer for SEV DMA.
> +
Please document that this function either succeeds or doesn't return.
> + @param[in] NumPage Number of pages.
> + @param[out] Buffer Allocated DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaAllocateBuffer (
> + IN UINT32 NumPages,
> + OUT VOID **Buffer
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // Allocate DMA bounce buffer
> + //
> + Status = BmDmaAllocateBuffer (TRUE, EfiBootServicesData, NumPages, Buffer);
> + if (EFI_ERROR(Status)) {
> + DEBUG ((EFI_D_ERROR, "SEV: Failed to allocate bounce buffer %d pages\n", NumPages));
> + ASSERT_EFI_ERROR (Status);
> + CpuDeadLoop ();
> + }
> +
> + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma allocate buffer 0x%Lx Pages %d\n", (UINTN)Buffer, NumPages));
> +}
> +
> +/**
> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
> +
> + @param[in] NumPage Number of pages.
> + @param[in] Buffer DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaFreeBuffer (
> + IN VOID *Buffer,
> + IN UINT32 NumPages
> + )
> +{
> + //
> + // Free the bounce buffer
> + //
> + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma free buffer 0x%Lx Pages %d\n", (UINTN)Buffer, NumPages));
> + BmDmaFreeBuffer (Buffer, NumPages);
> +}
> +
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> index 346bb88..536887f 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> @@ -47,4 +47,6 @@
> DebugLib
> IoLib
> MemoryAllocationLib
> + MemEncryptSevLib
> + BmDmaLib
>
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> index 1bf725d..d2560a3 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> @@ -47,6 +47,111 @@ QemuFwCfgSelectItem (
>
>
> /**
> + Transfer an array of bytes, or skip a number of bytes, using the SEV DMA bounce
> + interface. The function is same as InternalQemuFwCfgDmaBytes with excpetion that
> + it uses bounce buffer
> +
> + @param[in] Size Size in bytes to transfer or skip.
> +
> + @param[in,out] HostBuffer Buffer to read data into or write data from. Ignored,
> + and may be NULL, if Size is zero, or Control is
> + FW_CFG_DMA_CTL_SKIP.
> +
> + @param[in] Control One of the following:
> + FW_CFG_DMA_CTL_WRITE - write to fw_cfg from Buffer.
> + FW_CFG_DMA_CTL_READ - read from fw_cfg into Buffer.
> + FW_CFG_DMA_CTL_SKIP - skip bytes in fw_cfg.
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaBytes (
> + IN UINT32 Size,
> + IN OUT VOID *HostBuffer OPTIONAL,
> + IN UINT32 Control
> + )
> +{
> + volatile FW_CFG_DMA_ACCESS *Access;
> + UINT32 AccessHigh, AccessLow;
> + UINT32 Status;
> + UINT32 NumPages;
> + VOID *DmaBuffer, *Buffer;
> +
> + //
> + // Calculate number of pages we need to allocate for this operation
> + //
> + if (Control == FW_CFG_DMA_CTL_SKIP) {
> + //
> + // Control data does not need the actual buffer
> + //
> + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access));
> + } else {
> + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access) + Size);
> + }
> +
> + //
> + // Allocate DMA bounce buffer
> + //
> + InternalQemuFwCfgSevDmaAllocateBuffer (NumPages, &DmaBuffer);
> +
> + Access = (FW_CFG_DMA_ACCESS *)DmaBuffer;
> + Buffer = DmaBuffer + sizeof(*Access);
> +
> + Access->Control = SwapBytes32 (Control);
> + Access->Length = SwapBytes32 (Size);
> + Access->Address = SwapBytes64 ((UINTN)Buffer);
> +
> + //
> + // Copy data from Host buffer into DMA buffer
> + //
> + if (HostBuffer && (Control == FW_CFG_DMA_CTL_WRITE)) {
> + CopyMem (Buffer, HostBuffer, Size);
> + }
> +
> + //
> + // Delimit the transfer from (a) modifications to Access, (b) in case of a
> + // write, from writes to Buffer by the caller.
> + //
> + MemoryFence ();
> +
> + //
> + // Start the transfer.
> + //
> + AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
> + AccessLow = (UINT32)(UINTN)Access;
> + IoWrite32 (FW_CFG_IO_DMA_ADDRESS, SwapBytes32 (AccessHigh));
> + IoWrite32 (FW_CFG_IO_DMA_ADDRESS + 4, SwapBytes32 (AccessLow));
> +
> + //
> + // Don't look at Access->Control before starting the transfer.
> + //
> + MemoryFence ();
> +
> + //
> + // Wait for the transfer to complete.
> + //
> + do {
> + Status = SwapBytes32 (Access->Control);
> + ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
> + } while (Status != 0);
> +
> + //
> + // After a read, the caller will want to use Buffer.
> + //
> + MemoryFence ();
> +
> + //
> + // Copy data from DMA buffer into Host Buffer
> + //
> + if (HostBuffer && (Control == FW_CFG_DMA_CTL_READ)) {
> + CopyMem (HostBuffer, Buffer, Size);
> + }
> +
> + //
> + // Free the DMA bounce buffer
> + //
> + InternalQemuFwCfgSevDmaFreeBuffer (DmaBuffer, NumPages);
> +}
> +
> +/**
> Transfer an array of bytes, or skip a number of bytes, using the DMA
> interface.
>
> @@ -79,6 +184,13 @@ InternalQemuFwCfgDmaBytes (
> return;
> }
>
> + //
> + // When SEV is enabled then use SEV version of DmaReadWrite
> + //
> + if (InternalQemuFwCfgSevIsEnabled ()) {
> + return InternalQemuFwCfgSevDmaBytes (Size, Buffer, Control);
> + }
> +
> Access.Control = SwapBytes32 (Control);
> Access.Length = SwapBytes32 (Size);
> Access.Address = SwapBytes64 ((UINTN)Buffer);
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> index 6e87c62..8e2ff45 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> @@ -2,6 +2,7 @@
> Internal interfaces specific to the QemuFwCfgLib instances in OvmfPkg.
>
> Copyright (C) 2016, Red Hat, Inc.
> + Copyright (C) 2017, Advanced Micro Devices.
>
> This program and the accompanying materials are licensed and made available
> under the terms and conditions of the BSD License which accompanies this
> @@ -43,4 +44,41 @@ InternalQemuFwCfgDmaIsAvailable (
> VOID
> );
>
> +/**
> + Returns a boolean indicating whether the SEV is enabled
> +
> + @retval TRUE SEV is enabled
> + @retval FALSE SEV is not enabled
> +**/
> +BOOLEAN
> +InternalQemuFwCfgSevIsEnabled (
> + VOID
> + );
> +
> +/**
> + Allocate a bounce buffer for SEV DMA.
> +
> + @param[in] NumPage Number of pages.
> + @param[out] Buffer Allocated DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaAllocateBuffer (
> + IN UINT32 NumPages,
> + OUT VOID **Buffer
> + );
> +
> +/**
> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
> +
> + @param[in] NumPage Number of pages.
> + @param[in] Buffer DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaFreeBuffer (
> + IN VOID *Buffer,
> + IN UINT32 NumPages
> + );
> +
> #endif
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> index ac05f4c..3dc9270 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> @@ -4,6 +4,7 @@
>
> Copyright (C) 2013, Red Hat, Inc.
> Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
>
> This program and the accompanying materials are licensed and made available
> under the terms and conditions of the BSD License which accompanies this
> @@ -14,14 +15,55 @@
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> **/
>
> +#include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> #include <Library/QemuFwCfgLib.h>
> +#include <Register/Cpuid.h>
> +#include <Register/AmdSevMap.h>
>
> #include "QemuFwCfgLibInternal.h"
>
> STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
> STATIC BOOLEAN mQemuFwCfgDmaSupported;
>
> +/**
> + Returns a boolean indicating whether the SEV is enabled
> +
> + @retval TRUE SEV is enabled
> + @retval FALSE SEV is not enabled
> +**/
> +BOOLEAN
> +InternalQemuFwCfgSevIsEnabled (
> + VOID
> + )
> +{
> + UINT32 RegEax;
> + MSR_SEV_STATUS_REGISTER Msr;
> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
> +
> + //
> + // Check if memory encryption leaf exist
> + //
> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
> + //
> + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
> + //
> + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL);
> +
> + if (Eax.Bits.SevBit) {
> + //
> + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
> + //
> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> + if (Msr.Bits.SevBit) {
> + return TRUE;
> + }
> + }
> + }
> +
> + return FALSE;
> +}
>
> /**
> Returns a boolean indicating if the firmware configuration interface
> @@ -79,6 +121,17 @@ QemuFwCfgInitialize (
> mQemuFwCfgDmaSupported = TRUE;
> DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
> }
> +
> + //
> + // When SEV is enabled then we do not support DMA interface.
> + // This is because we need to use bounce buffer to support DMA operation in SEV guest.
> + // Allocating memory for bounce buffer can get painful in Pei phase
> + //
> + if (mQemuFwCfgDmaSupported && InternalQemuFwCfgSevIsEnabled ()) {
> + mQemuFwCfgDmaSupported = FALSE;
> + DEBUG ((DEBUG_INFO, "QemuFwCfg disabling DMA interface and defaulting to IO Port.\n"));
> + }
> +
> return RETURN_SUCCESS;
> }
>
> @@ -114,3 +167,43 @@ InternalQemuFwCfgDmaIsAvailable (
> {
> return mQemuFwCfgDmaSupported;
> }
> +
> +/**
> + Allocate a bounce buffer for SEV DMA.
> +
> + @param[in] NumPage Number of pages.
> + @param[out] Buffer Allocated DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaAllocateBuffer (
> + IN UINT32 NumPages,
> + OUT VOID **Buffer
> + )
> +{
> + //
> + // We should never reach here
> + //
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> +}
> +
> +/**
> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
> +
> + @param[in] NumPage Number of pages.
> + @param[in] Buffer DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaFreeBuffer (
> + IN VOID *Buffer,
> + IN UINT32 NumPages
> + )
> +{
> + //
> + // We should never reach here
> + //
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> +}
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> index 4f966a8..83cc0de 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> @@ -39,7 +39,9 @@
>
> [Packages]
> MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> OvmfPkg/OvmfPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
>
> [LibraryClasses]
> BaseLib
> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> index 465ccbe..70b0a47 100644
> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> @@ -16,8 +16,11 @@
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> **/
>
> +#include <Library/BaseLib.h>
> #include <Library/DebugLib.h>
> #include <Library/QemuFwCfgLib.h>
> +#include <Register/Cpuid.h>
> +#include <Register/AmdSevMap.h>
>
> #include "QemuFwCfgLibInternal.h"
>
> @@ -94,3 +97,82 @@ InternalQemuFwCfgDmaIsAvailable (
> {
> return FALSE;
> }
> +
> +/**
> + Returns a boolean indicating whether the SEV is enabled
> +
> + @retval TRUE SEV is enabled
> + @retval FALSE SEV is not enabled
> +**/
> +BOOLEAN
> +InternalQemuFwCfgSevIsEnabled (
> + VOID
> + )
> +{
> + UINT32 RegEax;
> + MSR_SEV_STATUS_REGISTER Msr;
> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
> +
> + //
> + // Check if memory encryption leaf exist
> + //
> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
> + //
> + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
> + //
> + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL);
> +
> + if (Eax.Bits.SevBit) {
> + //
> + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
> + //
> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> + if (Msr.Bits.SevBit) {
> + return TRUE;
> + }
> + }
> + }
> +
> + return FALSE;
> +}
> +
> +/**
> + Allocate a bounce buffer for SEV DMA.
> +
> + @param[in] NumPage Number of pages.
> + @param[out] Buffer Allocated DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaAllocateBuffer (
> + IN UINT32 NumPages,
> + OUT VOID **Buffer
> + )
> +{
> + //
> + // We should never reach here
> + //
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> +}
> +
> +/**
> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocateBuffer
> +
> + @param[in] NumPage Number of pages.
> + @param[in] Buffer DMA Buffer pointer
> +
> +**/
> +VOID
> +InternalQemuFwCfgSevDmaFreeBuffer (
> + IN VOID *Buffer,
> + IN UINT32 NumPages
> + )
> +{
> + //
> + // We should never reach here
> + //
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> +}
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On Mon, Mar 27, 2017 at 5:19 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/21/17 22:13, Brijesh Singh wrote:
> > The patch adds SEV support in QemuFwCfgLib. When SEV is enabled:
> >
> > * Pei phase support IO-style operations. This is mainly because we need
> to
> > use a bounce buffer inorder to support DMA operation. Allocate a
> memory
> > for bounce buffer can get painful in Pei phase hence if we detect
> FWCfg DMA
> > support then silently fallback to IO.
> >
> > * Dxe phase supports both IO and DMA style operations.
> > ---
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c | 73 +++++++++++++
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf | 2
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c | 112
> ++++++++++++++++++++
> > .../Library/QemuFwCfgLib/QemuFwCfgLibInternal.h | 38 +++++++
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c | 93
> +++++++++++++++++
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf | 2
> > OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c | 82
> +++++++++++++++
> > 7 files changed, 402 insertions(+)
>
> Can you please split this patch into 5 patches?
> - new interfaces in QemuFwCfgLibInternal.h
> - implementation of new APIs for SEC instance
> - implementation of new APIs for PEI instance
> - implementation of new APIs for DXE instance
> - call new APIs from common code
>
> If that is possible. Otherwise, it's quite hard to review & compare the
> instances for the different firmware phases.
>
>
Sure I will divide into multiple patches.
> Anyway, some things I've noticed:
>
> - InternalQemuFwCfgSevIsEnabled() should use global variables in the PEI
> instance as well. Possibly the appropriate interface from
> BaseMemcryptSevLib could be called from both the PEI and DXE instances
> here. (I think the DXE instance already does that.)
>
>
Id tou are okay with it then I will perfer to use BaseMemEncryptSevLib's
provided function in both PEI and DXE phases.
> - regarding InternalQemuFwCfgDmaBytes(): would it be possible to hook
> the SEV-related tricks into the existent function, rather than
> copy+modify the entire function as InternalQemuFwCfgSevDmaBytes()? It
> seems to me that a conditional "prologue" and "epilogue" in
> InternalQemuFwCfgDmaBytes() could do the trick.
>
>
I wanted to reuse the existing function but I was not sure how do you
wanted me to handle static allocation of "Access" variable inside
the InternalQemuFwCfgDmaBytes()
VOID
InternalQemuFwCfgDmaBytes (
IN UINT32 Size,
IN OUT VOID *Buffer OPTIONAL,
IN UINT32 Control
)
{
volatile FW_CFG_DMA_ACCESS Access;
UINT32 AccessHigh, AccessLow;
UINT32 Status;
The "Access" variable is statically allocated inside this function, in case
of SEV the Dma "Access" control variable need to be dynamically allocated
because hypervisor read/writes data into Access->status.
I could convert Access into FW_CFG_DMA_ACCESS pointer for non SEV case and
then integerate the SEV specific changes inside the same function.
I was just not sure which approach is prefrered hence decided to create a
new function for now and at least start the discussion.
> ... From skimming this patch, I think those are the only
> functionality-related comments I have at this point, beyond the remarks
> I made elsewhere in this series (like: line length, DEBUG_* macros, and
> so on). Please recheck all patches for those comments.
>
>
I will revist all the patches and fix those DEBUG_* macros.
> >
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> > index ac05f4c..be8e945 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
> > @@ -4,6 +4,7 @@
> >
> > Copyright (C) 2013, Red Hat, Inc.
> > Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> >
> > This program and the accompanying materials are licensed and made
> available
> > under the terms and conditions of the BSD License which accompanies
> this
> > @@ -14,14 +15,34 @@
> > WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > **/
> >
> > +#include "Uefi.h"
> > +
> > +#include <Library/BaseLib.h>
> > #include <Library/DebugLib.h>
> > #include <Library/QemuFwCfgLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/MemEncryptSevLib.h>
> > +#include <Library/BmDmaLib.h>
> >
> > #include "QemuFwCfgLibInternal.h"
> >
> > STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
> > STATIC BOOLEAN mQemuFwCfgDmaSupported;
> > +STATIC BOOLEAN mQemuFwCfgSevIsEnabled = FALSE;
> > +
> > +/**
> > + Returns a boolean indicating whether the SEV is enabled
> >
> > + @retval TRUE SEV is enabled
> > + @retval FALSE SEV is not enabled
> > +**/
> > +BOOLEAN
> > +InternalQemuFwCfgSevIsEnabled (
> > + VOID
> > + )
> > +{
> > + return mQemuFwCfgSevIsEnabled;
> > +}
> >
> > /**
> > Returns a boolean indicating if the firmware configuration interface
> > @@ -79,6 +100,9 @@ QemuFwCfgInitialize (
> > mQemuFwCfgDmaSupported = TRUE;
> > DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
> > }
> > +
> > + mQemuFwCfgSevIsEnabled = MemEncryptSevIsEnabled ();
> > +
> > return RETURN_SUCCESS;
> > }
> >
> > @@ -114,3 +138,52 @@ InternalQemuFwCfgDmaIsAvailable (
> > {
> > return mQemuFwCfgDmaSupported;
> > }
> > +
> > +/**
> > + Allocate a bounce buffer for SEV DMA.
> > +
>
> Please document that this function either succeeds or doesn't return.
>
> > + @param[in] NumPage Number of pages.
> > + @param[out] Buffer Allocated DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaAllocateBuffer (
> > + IN UINT32 NumPages,
> > + OUT VOID **Buffer
> > + )
> > +{
> > + EFI_STATUS Status;
> > +
> > + //
> > + // Allocate DMA bounce buffer
> > + //
> > + Status = BmDmaAllocateBuffer (TRUE, EfiBootServicesData, NumPages,
> Buffer);
> > + if (EFI_ERROR(Status)) {
> > + DEBUG ((EFI_D_ERROR, "SEV: Failed to allocate bounce buffer %d
> pages\n", NumPages));
> > + ASSERT_EFI_ERROR (Status);
> > + CpuDeadLoop ();
> > + }
> > +
> > + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma allocate buffer 0x%Lx Pages
> %d\n", (UINTN)Buffer, NumPages));
> > +}
> > +
> > +/**
> > + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
> eBuffer
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[in] Buffer DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaFreeBuffer (
> > + IN VOID *Buffer,
> > + IN UINT32 NumPages
> > + )
> > +{
> > + //
> > + // Free the bounce buffer
> > + //
> > + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma free buffer 0x%Lx Pages
> %d\n", (UINTN)Buffer, NumPages));
> > + BmDmaFreeBuffer (Buffer, NumPages);
> > +}
> > +
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> > index 346bb88..536887f 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
> > @@ -47,4 +47,6 @@
> > DebugLib
> > IoLib
> > MemoryAllocationLib
> > + MemEncryptSevLib
> > + BmDmaLib
> >
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > index 1bf725d..d2560a3 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
> > @@ -47,6 +47,111 @@ QemuFwCfgSelectItem (
> >
> >
> > /**
> > + Transfer an array of bytes, or skip a number of bytes, using the SEV
> DMA bounce
> > + interface. The function is same as InternalQemuFwCfgDmaBytes with
> excpetion that
> > + it uses bounce buffer
> > +
> > + @param[in] Size Size in bytes to transfer or skip.
> > +
> > + @param[in,out] HostBuffer Buffer to read data into or write data
> from. Ignored,
> > + and may be NULL, if Size is zero, or
> Control is
> > + FW_CFG_DMA_CTL_SKIP.
> > +
> > + @param[in] Control One of the following:
> > + FW_CFG_DMA_CTL_WRITE - write to fw_cfg from
> Buffer.
> > + FW_CFG_DMA_CTL_READ - read from fw_cfg into
> Buffer.
> > + FW_CFG_DMA_CTL_SKIP - skip bytes in fw_cfg.
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaBytes (
> > + IN UINT32 Size,
> > + IN OUT VOID *HostBuffer OPTIONAL,
> > + IN UINT32 Control
> > + )
> > +{
> > + volatile FW_CFG_DMA_ACCESS *Access;
> > + UINT32 AccessHigh, AccessLow;
> > + UINT32 Status;
> > + UINT32 NumPages;
> > + VOID *DmaBuffer, *Buffer;
> > +
> > + //
> > + // Calculate number of pages we need to allocate for this operation
> > + //
> > + if (Control == FW_CFG_DMA_CTL_SKIP) {
> > + //
> > + // Control data does not need the actual buffer
> > + //
> > + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access));
> > + } else {
> > + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access) + Size);
> > + }
> > +
> > + //
> > + // Allocate DMA bounce buffer
> > + //
> > + InternalQemuFwCfgSevDmaAllocateBuffer (NumPages, &DmaBuffer);
> > +
> > + Access = (FW_CFG_DMA_ACCESS *)DmaBuffer;
> > + Buffer = DmaBuffer + sizeof(*Access);
> > +
> > + Access->Control = SwapBytes32 (Control);
> > + Access->Length = SwapBytes32 (Size);
> > + Access->Address = SwapBytes64 ((UINTN)Buffer);
> > +
> > + //
> > + // Copy data from Host buffer into DMA buffer
> > + //
> > + if (HostBuffer && (Control == FW_CFG_DMA_CTL_WRITE)) {
> > + CopyMem (Buffer, HostBuffer, Size);
> > + }
> > +
> > + //
> > + // Delimit the transfer from (a) modifications to Access, (b) in case
> of a
> > + // write, from writes to Buffer by the caller.
> > + //
> > + MemoryFence ();
> > +
> > + //
> > + // Start the transfer.
> > + //
> > + AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
> > + AccessLow = (UINT32)(UINTN)Access;
> > + IoWrite32 (FW_CFG_IO_DMA_ADDRESS, SwapBytes32 (AccessHigh));
> > + IoWrite32 (FW_CFG_IO_DMA_ADDRESS + 4, SwapBytes32 (AccessLow));
> > +
> > + //
> > + // Don't look at Access->Control before starting the transfer.
> > + //
> > + MemoryFence ();
> > +
> > + //
> > + // Wait for the transfer to complete.
> > + //
> > + do {
> > + Status = SwapBytes32 (Access->Control);
> > + ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
> > + } while (Status != 0);
> > +
> > + //
> > + // After a read, the caller will want to use Buffer.
> > + //
> > + MemoryFence ();
> > +
> > + //
> > + // Copy data from DMA buffer into Host Buffer
> > + //
> > + if (HostBuffer && (Control == FW_CFG_DMA_CTL_READ)) {
> > + CopyMem (HostBuffer, Buffer, Size);
> > + }
> > +
> > + //
> > + // Free the DMA bounce buffer
> > + //
> > + InternalQemuFwCfgSevDmaFreeBuffer (DmaBuffer, NumPages);
> > +}
> > +
> > +/**
> > Transfer an array of bytes, or skip a number of bytes, using the DMA
> > interface.
> >
> > @@ -79,6 +184,13 @@ InternalQemuFwCfgDmaBytes (
> > return;
> > }
> >
> > + //
> > + // When SEV is enabled then use SEV version of DmaReadWrite
> > + //
> > + if (InternalQemuFwCfgSevIsEnabled ()) {
> > + return InternalQemuFwCfgSevDmaBytes (Size, Buffer, Control);
> > + }
> > +
> > Access.Control = SwapBytes32 (Control);
> > Access.Length = SwapBytes32 (Size);
> > Access.Address = SwapBytes64 ((UINTN)Buffer);
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> > index 6e87c62..8e2ff45 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
> > @@ -2,6 +2,7 @@
> > Internal interfaces specific to the QemuFwCfgLib instances in OvmfPkg.
> >
> > Copyright (C) 2016, Red Hat, Inc.
> > + Copyright (C) 2017, Advanced Micro Devices.
> >
> > This program and the accompanying materials are licensed and made
> available
> > under the terms and conditions of the BSD License which accompanies
> this
> > @@ -43,4 +44,41 @@ InternalQemuFwCfgDmaIsAvailable (
> > VOID
> > );
> >
> > +/**
> > + Returns a boolean indicating whether the SEV is enabled
> > +
> > + @retval TRUE SEV is enabled
> > + @retval FALSE SEV is not enabled
> > +**/
> > +BOOLEAN
> > +InternalQemuFwCfgSevIsEnabled (
> > + VOID
> > + );
> > +
> > +/**
> > + Allocate a bounce buffer for SEV DMA.
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[out] Buffer Allocated DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaAllocateBuffer (
> > + IN UINT32 NumPages,
> > + OUT VOID **Buffer
> > + );
> > +
> > +/**
> > + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
> eBuffer
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[in] Buffer DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaFreeBuffer (
> > + IN VOID *Buffer,
> > + IN UINT32 NumPages
> > + );
> > +
> > #endif
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> > index ac05f4c..3dc9270 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
> > @@ -4,6 +4,7 @@
> >
> > Copyright (C) 2013, Red Hat, Inc.
> > Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
> >
> > This program and the accompanying materials are licensed and made
> available
> > under the terms and conditions of the BSD License which accompanies
> this
> > @@ -14,14 +15,55 @@
> > WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > **/
> >
> > +#include <Library/BaseLib.h>
> > #include <Library/DebugLib.h>
> > #include <Library/QemuFwCfgLib.h>
> > +#include <Register/Cpuid.h>
> > +#include <Register/AmdSevMap.h>
> >
> > #include "QemuFwCfgLibInternal.h"
> >
> > STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
> > STATIC BOOLEAN mQemuFwCfgDmaSupported;
> >
> > +/**
> > + Returns a boolean indicating whether the SEV is enabled
> > +
> > + @retval TRUE SEV is enabled
> > + @retval FALSE SEV is not enabled
> > +**/
> > +BOOLEAN
> > +InternalQemuFwCfgSevIsEnabled (
> > + VOID
> > + )
> > +{
> > + UINT32 RegEax;
> > + MSR_SEV_STATUS_REGISTER Msr;
> > + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
> > +
> > + //
> > + // Check if memory encryption leaf exist
> > + //
> > + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> > + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
> > + //
> > + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
> > + //
> > + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL,
> NULL);
> > +
> > + if (Eax.Bits.SevBit) {
> > + //
> > + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
> > + //
> > + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> > + if (Msr.Bits.SevBit) {
> > + return TRUE;
> > + }
> > + }
> > + }
> > +
> > + return FALSE;
> > +}
> >
> > /**
> > Returns a boolean indicating if the firmware configuration interface
> > @@ -79,6 +121,17 @@ QemuFwCfgInitialize (
> > mQemuFwCfgDmaSupported = TRUE;
> > DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
> > }
> > +
> > + //
> > + // When SEV is enabled then we do not support DMA interface.
> > + // This is because we need to use bounce buffer to support DMA
> operation in SEV guest.
> > + // Allocating memory for bounce buffer can get painful in Pei phase
> > + //
> > + if (mQemuFwCfgDmaSupported && InternalQemuFwCfgSevIsEnabled ()) {
> > + mQemuFwCfgDmaSupported = FALSE;
> > + DEBUG ((DEBUG_INFO, "QemuFwCfg disabling DMA interface and
> defaulting to IO Port.\n"));
> > + }
> > +
> > return RETURN_SUCCESS;
> > }
> >
> > @@ -114,3 +167,43 @@ InternalQemuFwCfgDmaIsAvailable (
> > {
> > return mQemuFwCfgDmaSupported;
> > }
> > +
> > +/**
> > + Allocate a bounce buffer for SEV DMA.
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[out] Buffer Allocated DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaAllocateBuffer (
> > + IN UINT32 NumPages,
> > + OUT VOID **Buffer
> > + )
> > +{
> > + //
> > + // We should never reach here
> > + //
> > + ASSERT (FALSE);
> > + CpuDeadLoop ();
> > +}
> > +
> > +/**
> > + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
> eBuffer
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[in] Buffer DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaFreeBuffer (
> > + IN VOID *Buffer,
> > + IN UINT32 NumPages
> > + )
> > +{
> > + //
> > + // We should never reach here
> > + //
> > + ASSERT (FALSE);
> > + CpuDeadLoop ();
> > +}
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> > index 4f966a8..83cc0de 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
> > @@ -39,7 +39,9 @@
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > OvmfPkg/OvmfPkg.dec
> > + UefiCpuPkg/UefiCpuPkg.dec
> >
> > [LibraryClasses]
> > BaseLib
> > diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> > index 465ccbe..70b0a47 100644
> > --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> > +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
> > @@ -16,8 +16,11 @@
> > WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > **/
> >
> > +#include <Library/BaseLib.h>
> > #include <Library/DebugLib.h>
> > #include <Library/QemuFwCfgLib.h>
> > +#include <Register/Cpuid.h>
> > +#include <Register/AmdSevMap.h>
> >
> > #include "QemuFwCfgLibInternal.h"
> >
> > @@ -94,3 +97,82 @@ InternalQemuFwCfgDmaIsAvailable (
> > {
> > return FALSE;
> > }
> > +
> > +/**
> > + Returns a boolean indicating whether the SEV is enabled
> > +
> > + @retval TRUE SEV is enabled
> > + @retval FALSE SEV is not enabled
> > +**/
> > +BOOLEAN
> > +InternalQemuFwCfgSevIsEnabled (
> > + VOID
> > + )
> > +{
> > + UINT32 RegEax;
> > + MSR_SEV_STATUS_REGISTER Msr;
> > + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
> > +
> > + //
> > + // Check if memory encryption leaf exist
> > + //
> > + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
> > + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
> > + //
> > + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
> > + //
> > + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL,
> NULL);
> > +
> > + if (Eax.Bits.SevBit) {
> > + //
> > + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
> > + //
> > + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
> > + if (Msr.Bits.SevBit) {
> > + return TRUE;
> > + }
> > + }
> > + }
> > +
> > + return FALSE;
> > +}
> > +
> > +/**
> > + Allocate a bounce buffer for SEV DMA.
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[out] Buffer Allocated DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaAllocateBuffer (
> > + IN UINT32 NumPages,
> > + OUT VOID **Buffer
> > + )
> > +{
> > + //
> > + // We should never reach here
> > + //
> > + ASSERT (FALSE);
> > + CpuDeadLoop ();
> > +}
> > +
> > +/**
> > + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
> eBuffer
> > +
> > + @param[in] NumPage Number of pages.
> > + @param[in] Buffer DMA Buffer pointer
> > +
> > +**/
> > +VOID
> > +InternalQemuFwCfgSevDmaFreeBuffer (
> > + IN VOID *Buffer,
> > + IN UINT32 NumPages
> > + )
> > +{
> > + //
> > + // We should never reach here
> > + //
> > + ASSERT (FALSE);
> > + CpuDeadLoop ();
> > +}
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
>
>
--
Confusion is always the most honest response.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/27/17 21:24, Brijesh Singh wrote:
> On Mon, Mar 27, 2017 at 5:19 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>> - regarding InternalQemuFwCfgDmaBytes(): would it be possible to hook
>> the SEV-related tricks into the existent function, rather than
>> copy+modify the entire function as InternalQemuFwCfgSevDmaBytes()? It
>> seems to me that a conditional "prologue" and "epilogue" in
>> InternalQemuFwCfgDmaBytes() could do the trick.
>>
>>
> I wanted to reuse the existing function but I was not sure how do you
> wanted me to handle static allocation of "Access" variable inside
> the InternalQemuFwCfgDmaBytes()
>
> VOID
> InternalQemuFwCfgDmaBytes (
> IN UINT32 Size,
> IN OUT VOID *Buffer OPTIONAL,
> IN UINT32 Control
> )
> {
> volatile FW_CFG_DMA_ACCESS Access;
> UINT32 AccessHigh, AccessLow;
> UINT32 Status;
>
>
> The "Access" variable is statically allocated inside this function,
The storage duration of "Access" is not "static", it is "automatic".
(Put more colloquially, it is on the stack, it is not a global variable.)
But, I do understand the point; for the SEV case, you need it to be
dynamically allocated *and* exposed to the hypervisor, with
InternalQemuFwCfgSevDmaAllocateBuffer().
So here's my suggestion: in the following (suggested) list of patches:
- new interfaces in QemuFwCfgLibInternal.h
- implementation of new APIs for SEC instance
- implementation of new APIs for PEI instance
- implementation of new APIs for DXE instance
- call new APIs from common code
the last part should actually be two patches (so not 5 but 6 patches in
total).
First, please modify InternalQemuFwCfgDmaBytes() so that it works
through a pointer, but without other (functionality) changes:
volatile FW_CFG_DMA_ACCESS LocalAccess;
volatile FW_CFG_DMA_ACCESS *Access;
Access = &LocalAccess;
Second, add the SEV-specific prologue / epilogue that initially set the
"Access" pointer differently, and finally free / unmap it.
> in case
> of SEV the Dma "Access" control variable need to be dynamically allocated
> because hypervisor read/writes data into Access->status.
>
> I could convert Access into FW_CFG_DMA_ACCESS pointer for non SEV case and
> then integerate the SEV specific changes inside the same function.
Yes, that's the idea.
>
> I was just not sure which approach is prefrered hence decided to create a
> new function for now and at least start the discussion.
Thank you.
Laszlo
>
>
>
>
>> ... From skimming this patch, I think those are the only
>> functionality-related comments I have at this point, beyond the remarks
>> I made elsewhere in this series (like: line length, DEBUG_* macros, and
>> so on). Please recheck all patches for those comments.
>>
>>
> I will revist all the patches and fix those DEBUG_* macros.
>
>
>
>>>
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
>>> index ac05f4c..be8e945 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
>>> @@ -4,6 +4,7 @@
>>>
>>> Copyright (C) 2013, Red Hat, Inc.
>>> Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
>>> + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
>>>
>>> This program and the accompanying materials are licensed and made
>> available
>>> under the terms and conditions of the BSD License which accompanies
>> this
>>> @@ -14,14 +15,34 @@
>>> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>>> **/
>>>
>>> +#include "Uefi.h"
>>> +
>>> +#include <Library/BaseLib.h>
>>> #include <Library/DebugLib.h>
>>> #include <Library/QemuFwCfgLib.h>
>>> +#include <Library/UefiBootServicesTableLib.h>
>>> +#include <Library/MemEncryptSevLib.h>
>>> +#include <Library/BmDmaLib.h>
>>>
>>> #include "QemuFwCfgLibInternal.h"
>>>
>>> STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
>>> STATIC BOOLEAN mQemuFwCfgDmaSupported;
>>> +STATIC BOOLEAN mQemuFwCfgSevIsEnabled = FALSE;
>>> +
>>> +/**
>>> + Returns a boolean indicating whether the SEV is enabled
>>>
>>> + @retval TRUE SEV is enabled
>>> + @retval FALSE SEV is not enabled
>>> +**/
>>> +BOOLEAN
>>> +InternalQemuFwCfgSevIsEnabled (
>>> + VOID
>>> + )
>>> +{
>>> + return mQemuFwCfgSevIsEnabled;
>>> +}
>>>
>>> /**
>>> Returns a boolean indicating if the firmware configuration interface
>>> @@ -79,6 +100,9 @@ QemuFwCfgInitialize (
>>> mQemuFwCfgDmaSupported = TRUE;
>>> DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
>>> }
>>> +
>>> + mQemuFwCfgSevIsEnabled = MemEncryptSevIsEnabled ();
>>> +
>>> return RETURN_SUCCESS;
>>> }
>>>
>>> @@ -114,3 +138,52 @@ InternalQemuFwCfgDmaIsAvailable (
>>> {
>>> return mQemuFwCfgDmaSupported;
>>> }
>>> +
>>> +/**
>>> + Allocate a bounce buffer for SEV DMA.
>>> +
>>
>> Please document that this function either succeeds or doesn't return.
>>
>>> + @param[in] NumPage Number of pages.
>>> + @param[out] Buffer Allocated DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaAllocateBuffer (
>>> + IN UINT32 NumPages,
>>> + OUT VOID **Buffer
>>> + )
>>> +{
>>> + EFI_STATUS Status;
>>> +
>>> + //
>>> + // Allocate DMA bounce buffer
>>> + //
>>> + Status = BmDmaAllocateBuffer (TRUE, EfiBootServicesData, NumPages,
>> Buffer);
>>> + if (EFI_ERROR(Status)) {
>>> + DEBUG ((EFI_D_ERROR, "SEV: Failed to allocate bounce buffer %d
>> pages\n", NumPages));
>>> + ASSERT_EFI_ERROR (Status);
>>> + CpuDeadLoop ();
>>> + }
>>> +
>>> + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma allocate buffer 0x%Lx Pages
>> %d\n", (UINTN)Buffer, NumPages));
>>> +}
>>> +
>>> +/**
>>> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
>> eBuffer
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[in] Buffer DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaFreeBuffer (
>>> + IN VOID *Buffer,
>>> + IN UINT32 NumPages
>>> + )
>>> +{
>>> + //
>>> + // Free the bounce buffer
>>> + //
>>> + DEBUG ((EFI_D_VERBOSE, "QemuFwCfgSevDma free buffer 0x%Lx Pages
>> %d\n", (UINTN)Buffer, NumPages));
>>> + BmDmaFreeBuffer (Buffer, NumPages);
>>> +}
>>> +
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>>> index 346bb88..536887f 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
>>> @@ -47,4 +47,6 @@
>>> DebugLib
>>> IoLib
>>> MemoryAllocationLib
>>> + MemEncryptSevLib
>>> + BmDmaLib
>>>
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
>>> index 1bf725d..d2560a3 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c
>>> @@ -47,6 +47,111 @@ QemuFwCfgSelectItem (
>>>
>>>
>>> /**
>>> + Transfer an array of bytes, or skip a number of bytes, using the SEV
>> DMA bounce
>>> + interface. The function is same as InternalQemuFwCfgDmaBytes with
>> excpetion that
>>> + it uses bounce buffer
>>> +
>>> + @param[in] Size Size in bytes to transfer or skip.
>>> +
>>> + @param[in,out] HostBuffer Buffer to read data into or write data
>> from. Ignored,
>>> + and may be NULL, if Size is zero, or
>> Control is
>>> + FW_CFG_DMA_CTL_SKIP.
>>> +
>>> + @param[in] Control One of the following:
>>> + FW_CFG_DMA_CTL_WRITE - write to fw_cfg from
>> Buffer.
>>> + FW_CFG_DMA_CTL_READ - read from fw_cfg into
>> Buffer.
>>> + FW_CFG_DMA_CTL_SKIP - skip bytes in fw_cfg.
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaBytes (
>>> + IN UINT32 Size,
>>> + IN OUT VOID *HostBuffer OPTIONAL,
>>> + IN UINT32 Control
>>> + )
>>> +{
>>> + volatile FW_CFG_DMA_ACCESS *Access;
>>> + UINT32 AccessHigh, AccessLow;
>>> + UINT32 Status;
>>> + UINT32 NumPages;
>>> + VOID *DmaBuffer, *Buffer;
>>> +
>>> + //
>>> + // Calculate number of pages we need to allocate for this operation
>>> + //
>>> + if (Control == FW_CFG_DMA_CTL_SKIP) {
>>> + //
>>> + // Control data does not need the actual buffer
>>> + //
>>> + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access));
>>> + } else {
>>> + NumPages = EFI_SIZE_TO_PAGES (sizeof (*Access) + Size);
>>> + }
>>> +
>>> + //
>>> + // Allocate DMA bounce buffer
>>> + //
>>> + InternalQemuFwCfgSevDmaAllocateBuffer (NumPages, &DmaBuffer);
>>> +
>>> + Access = (FW_CFG_DMA_ACCESS *)DmaBuffer;
>>> + Buffer = DmaBuffer + sizeof(*Access);
>>> +
>>> + Access->Control = SwapBytes32 (Control);
>>> + Access->Length = SwapBytes32 (Size);
>>> + Access->Address = SwapBytes64 ((UINTN)Buffer);
>>> +
>>> + //
>>> + // Copy data from Host buffer into DMA buffer
>>> + //
>>> + if (HostBuffer && (Control == FW_CFG_DMA_CTL_WRITE)) {
>>> + CopyMem (Buffer, HostBuffer, Size);
>>> + }
>>> +
>>> + //
>>> + // Delimit the transfer from (a) modifications to Access, (b) in case
>> of a
>>> + // write, from writes to Buffer by the caller.
>>> + //
>>> + MemoryFence ();
>>> +
>>> + //
>>> + // Start the transfer.
>>> + //
>>> + AccessHigh = (UINT32)RShiftU64 ((UINTN)Access, 32);
>>> + AccessLow = (UINT32)(UINTN)Access;
>>> + IoWrite32 (FW_CFG_IO_DMA_ADDRESS, SwapBytes32 (AccessHigh));
>>> + IoWrite32 (FW_CFG_IO_DMA_ADDRESS + 4, SwapBytes32 (AccessLow));
>>> +
>>> + //
>>> + // Don't look at Access->Control before starting the transfer.
>>> + //
>>> + MemoryFence ();
>>> +
>>> + //
>>> + // Wait for the transfer to complete.
>>> + //
>>> + do {
>>> + Status = SwapBytes32 (Access->Control);
>>> + ASSERT ((Status & FW_CFG_DMA_CTL_ERROR) == 0);
>>> + } while (Status != 0);
>>> +
>>> + //
>>> + // After a read, the caller will want to use Buffer.
>>> + //
>>> + MemoryFence ();
>>> +
>>> + //
>>> + // Copy data from DMA buffer into Host Buffer
>>> + //
>>> + if (HostBuffer && (Control == FW_CFG_DMA_CTL_READ)) {
>>> + CopyMem (HostBuffer, Buffer, Size);
>>> + }
>>> +
>>> + //
>>> + // Free the DMA bounce buffer
>>> + //
>>> + InternalQemuFwCfgSevDmaFreeBuffer (DmaBuffer, NumPages);
>>> +}
>>> +
>>> +/**
>>> Transfer an array of bytes, or skip a number of bytes, using the DMA
>>> interface.
>>>
>>> @@ -79,6 +184,13 @@ InternalQemuFwCfgDmaBytes (
>>> return;
>>> }
>>>
>>> + //
>>> + // When SEV is enabled then use SEV version of DmaReadWrite
>>> + //
>>> + if (InternalQemuFwCfgSevIsEnabled ()) {
>>> + return InternalQemuFwCfgSevDmaBytes (Size, Buffer, Control);
>>> + }
>>> +
>>> Access.Control = SwapBytes32 (Control);
>>> Access.Length = SwapBytes32 (Size);
>>> Access.Address = SwapBytes64 ((UINTN)Buffer);
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
>>> index 6e87c62..8e2ff45 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h
>>> @@ -2,6 +2,7 @@
>>> Internal interfaces specific to the QemuFwCfgLib instances in OvmfPkg.
>>>
>>> Copyright (C) 2016, Red Hat, Inc.
>>> + Copyright (C) 2017, Advanced Micro Devices.
>>>
>>> This program and the accompanying materials are licensed and made
>> available
>>> under the terms and conditions of the BSD License which accompanies
>> this
>>> @@ -43,4 +44,41 @@ InternalQemuFwCfgDmaIsAvailable (
>>> VOID
>>> );
>>>
>>> +/**
>>> + Returns a boolean indicating whether the SEV is enabled
>>> +
>>> + @retval TRUE SEV is enabled
>>> + @retval FALSE SEV is not enabled
>>> +**/
>>> +BOOLEAN
>>> +InternalQemuFwCfgSevIsEnabled (
>>> + VOID
>>> + );
>>> +
>>> +/**
>>> + Allocate a bounce buffer for SEV DMA.
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[out] Buffer Allocated DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaAllocateBuffer (
>>> + IN UINT32 NumPages,
>>> + OUT VOID **Buffer
>>> + );
>>> +
>>> +/**
>>> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
>> eBuffer
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[in] Buffer DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaFreeBuffer (
>>> + IN VOID *Buffer,
>>> + IN UINT32 NumPages
>>> + );
>>> +
>>> #endif
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
>>> index ac05f4c..3dc9270 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPei.c
>>> @@ -4,6 +4,7 @@
>>>
>>> Copyright (C) 2013, Red Hat, Inc.
>>> Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.<BR>
>>> + Copyright (c) 2017, Advanced Micro Devices. All rights reserved.<BR>
>>>
>>> This program and the accompanying materials are licensed and made
>> available
>>> under the terms and conditions of the BSD License which accompanies
>> this
>>> @@ -14,14 +15,55 @@
>>> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>>> **/
>>>
>>> +#include <Library/BaseLib.h>
>>> #include <Library/DebugLib.h>
>>> #include <Library/QemuFwCfgLib.h>
>>> +#include <Register/Cpuid.h>
>>> +#include <Register/AmdSevMap.h>
>>>
>>> #include "QemuFwCfgLibInternal.h"
>>>
>>> STATIC BOOLEAN mQemuFwCfgSupported = FALSE;
>>> STATIC BOOLEAN mQemuFwCfgDmaSupported;
>>>
>>> +/**
>>> + Returns a boolean indicating whether the SEV is enabled
>>> +
>>> + @retval TRUE SEV is enabled
>>> + @retval FALSE SEV is not enabled
>>> +**/
>>> +BOOLEAN
>>> +InternalQemuFwCfgSevIsEnabled (
>>> + VOID
>>> + )
>>> +{
>>> + UINT32 RegEax;
>>> + MSR_SEV_STATUS_REGISTER Msr;
>>> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
>>> +
>>> + //
>>> + // Check if memory encryption leaf exist
>>> + //
>>> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
>>> + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
>>> + //
>>> + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
>>> + //
>>> + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL,
>> NULL);
>>> +
>>> + if (Eax.Bits.SevBit) {
>>> + //
>>> + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
>>> + //
>>> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
>>> + if (Msr.Bits.SevBit) {
>>> + return TRUE;
>>> + }
>>> + }
>>> + }
>>> +
>>> + return FALSE;
>>> +}
>>>
>>> /**
>>> Returns a boolean indicating if the firmware configuration interface
>>> @@ -79,6 +121,17 @@ QemuFwCfgInitialize (
>>> mQemuFwCfgDmaSupported = TRUE;
>>> DEBUG ((DEBUG_INFO, "QemuFwCfg interface (DMA) is supported.\n"));
>>> }
>>> +
>>> + //
>>> + // When SEV is enabled then we do not support DMA interface.
>>> + // This is because we need to use bounce buffer to support DMA
>> operation in SEV guest.
>>> + // Allocating memory for bounce buffer can get painful in Pei phase
>>> + //
>>> + if (mQemuFwCfgDmaSupported && InternalQemuFwCfgSevIsEnabled ()) {
>>> + mQemuFwCfgDmaSupported = FALSE;
>>> + DEBUG ((DEBUG_INFO, "QemuFwCfg disabling DMA interface and
>> defaulting to IO Port.\n"));
>>> + }
>>> +
>>> return RETURN_SUCCESS;
>>> }
>>>
>>> @@ -114,3 +167,43 @@ InternalQemuFwCfgDmaIsAvailable (
>>> {
>>> return mQemuFwCfgDmaSupported;
>>> }
>>> +
>>> +/**
>>> + Allocate a bounce buffer for SEV DMA.
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[out] Buffer Allocated DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaAllocateBuffer (
>>> + IN UINT32 NumPages,
>>> + OUT VOID **Buffer
>>> + )
>>> +{
>>> + //
>>> + // We should never reach here
>>> + //
>>> + ASSERT (FALSE);
>>> + CpuDeadLoop ();
>>> +}
>>> +
>>> +/**
>>> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
>> eBuffer
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[in] Buffer DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaFreeBuffer (
>>> + IN VOID *Buffer,
>>> + IN UINT32 NumPages
>>> + )
>>> +{
>>> + //
>>> + // We should never reach here
>>> + //
>>> + ASSERT (FALSE);
>>> + CpuDeadLoop ();
>>> +}
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>> index 4f966a8..83cc0de 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>> @@ -39,7 +39,9 @@
>>>
>>> [Packages]
>>> MdePkg/MdePkg.dec
>>> + MdeModulePkg/MdeModulePkg.dec
>>> OvmfPkg/OvmfPkg.dec
>>> + UefiCpuPkg/UefiCpuPkg.dec
>>>
>>> [LibraryClasses]
>>> BaseLib
>>> diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
>> b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
>>> index 465ccbe..70b0a47 100644
>>> --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
>>> +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c
>>> @@ -16,8 +16,11 @@
>>> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>>> **/
>>>
>>> +#include <Library/BaseLib.h>
>>> #include <Library/DebugLib.h>
>>> #include <Library/QemuFwCfgLib.h>
>>> +#include <Register/Cpuid.h>
>>> +#include <Register/AmdSevMap.h>
>>>
>>> #include "QemuFwCfgLibInternal.h"
>>>
>>> @@ -94,3 +97,82 @@ InternalQemuFwCfgDmaIsAvailable (
>>> {
>>> return FALSE;
>>> }
>>> +
>>> +/**
>>> + Returns a boolean indicating whether the SEV is enabled
>>> +
>>> + @retval TRUE SEV is enabled
>>> + @retval FALSE SEV is not enabled
>>> +**/
>>> +BOOLEAN
>>> +InternalQemuFwCfgSevIsEnabled (
>>> + VOID
>>> + )
>>> +{
>>> + UINT32 RegEax;
>>> + MSR_SEV_STATUS_REGISTER Msr;
>>> + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax;
>>> +
>>> + //
>>> + // Check if memory encryption leaf exist
>>> + //
>>> + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
>>> + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) {
>>> + //
>>> + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported)
>>> + //
>>> + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL,
>> NULL);
>>> +
>>> + if (Eax.Bits.SevBit) {
>>> + //
>>> + // Check MSR_0xC0010131 Bit 0 (Sev Enabled)
>>> + //
>>> + Msr.Uint32 = AsmReadMsr32 (MSR_SEV_STATUS);
>>> + if (Msr.Bits.SevBit) {
>>> + return TRUE;
>>> + }
>>> + }
>>> + }
>>> +
>>> + return FALSE;
>>> +}
>>> +
>>> +/**
>>> + Allocate a bounce buffer for SEV DMA.
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[out] Buffer Allocated DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaAllocateBuffer (
>>> + IN UINT32 NumPages,
>>> + OUT VOID **Buffer
>>> + )
>>> +{
>>> + //
>>> + // We should never reach here
>>> + //
>>> + ASSERT (FALSE);
>>> + CpuDeadLoop ();
>>> +}
>>> +
>>> +/**
>>> + Free the DMA buffer allocated using InternalQemuFwCfgSevDmaAllocat
>> eBuffer
>>> +
>>> + @param[in] NumPage Number of pages.
>>> + @param[in] Buffer DMA Buffer pointer
>>> +
>>> +**/
>>> +VOID
>>> +InternalQemuFwCfgSevDmaFreeBuffer (
>>> + IN VOID *Buffer,
>>> + IN UINT32 NumPages
>>> + )
>>> +{
>>> + //
>>> + // We should never reach here
>>> + //
>>> + ASSERT (FALSE);
>>> + CpuDeadLoop ();
>>> +}
>>>
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>>
>>
>>
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.