* [edk2-devel] [PATCH 1/4] OvmfPkg: Add VirtHstiDxe driver
2024-03-22 14:27 [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
@ 2024-03-22 14:27 ` Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 2/4] OvmfPkg: Add VirtHstiDxe to OVMF firmware build Gerd Hoffmann
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-03-22 14:27 UTC (permalink / raw)
To: devel
Cc: Oliver Steffen, Konstantin Kostiuk, Ard Biesheuvel, Jiewen Yao,
Gerd Hoffmann
From: Konstantin Kostiuk <kkostiuk@redhat.com>
The driver supports qemu machine types 'pc' and 'q35'.
This patch adds some helper functions to manage the bitmasks.
The implemented features depend on both OVMF build configuration
and qemu VM configuration.
For q35 a single security feature is supported and checked: In
SMM-enabled builds the driver will verify smram is properly locked.
That test should never fail.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
Initial-patch-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf | 50 ++++++++
OvmfPkg/VirtHstiDxe/VirtHstiDxe.h | 67 +++++++++++
OvmfPkg/VirtHstiDxe/QemuPC.c | 38 +++++++
OvmfPkg/VirtHstiDxe/QemuQ35.c | 58 ++++++++++
OvmfPkg/VirtHstiDxe/VirtHstiDxe.c | 169 ++++++++++++++++++++++++++++
5 files changed, 382 insertions(+)
create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
create mode 100644 OvmfPkg/VirtHstiDxe/QemuPC.c
create mode 100644 OvmfPkg/VirtHstiDxe/QemuQ35.c
create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
new file mode 100644
index 000000000000..8c63ff6a8953
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
@@ -0,0 +1,50 @@
+## @file
+# Component description file for Virt Hsti Driver
+#
+# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) Microsoft Corporation.<BR>
+# Copyright (c) 2024, Red Hat. Inc
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = VirtHstiDxe
+ FILE_GUID = 60740CF3-D428-4500-80E6-04A5798241ED
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = VirtHstiDxeEntrypoint
+
+[Sources]
+ VirtHstiDxe.h
+ VirtHstiDxe.c
+ QemuPC.c
+ QemuQ35.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ UefiDriverEntryPoint
+ UefiLib
+ BaseLib
+ BaseMemoryLib
+ MemoryAllocationLib
+ DebugLib
+ HobLib
+ HstiLib
+ PcdLib
+ PciLib
+ UefiBootServicesTableLib
+
+[Guids]
+ gUefiOvmfPkgPlatformInfoGuid
+
+[FeaturePcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
+
+[Depex]
+ TRUE
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
new file mode 100644
index 000000000000..26109bf322e9
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
@@ -0,0 +1,67 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#define VIRT_HSTI_SECURITY_FEATURE_SIZE 2
+
+#define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
+
+typedef struct {
+ // ADAPTER_INFO_PLATFORM_SECURITY
+ UINT32 Version;
+ UINT32 Role;
+ CHAR16 ImplementationID[256];
+ UINT32 SecurityFeaturesSize;
+ // bitfields
+ UINT8 SecurityFeaturesRequired[VIRT_HSTI_SECURITY_FEATURE_SIZE];
+ UINT8 SecurityFeaturesImplemented[VIRT_HSTI_SECURITY_FEATURE_SIZE];
+ UINT8 SecurityFeaturesVerified[VIRT_HSTI_SECURITY_FEATURE_SIZE];
+ CHAR16 ErrorString[1];
+} VIRT_ADAPTER_INFO_PLATFORM_SECURITY;
+
+VOID
+VirtHstiSetSupported (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ );
+
+BOOLEAN
+VirtHstiIsSupported (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ );
+
+VOID
+VirtHstiTestResult (
+ CHAR16 *ErrorMsg,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ );
+
+/* QemuQ35.c */
+
+VIRT_ADAPTER_INFO_PLATFORM_SECURITY *
+VirtHstiQemuQ35Init (
+ VOID
+ );
+
+VOID
+VirtHstiQemuQ35Verify (
+ VOID
+ );
+
+/* QemuPC.c */
+
+VIRT_ADAPTER_INFO_PLATFORM_SECURITY *
+VirtHstiQemuPCInit (
+ VOID
+ );
+
+VOID
+VirtHstiQemuPCVerify (
+ VOID
+ );
diff --git a/OvmfPkg/VirtHstiDxe/QemuPC.c b/OvmfPkg/VirtHstiDxe/QemuPC.c
new file mode 100644
index 000000000000..aa0459e8b6c6
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/QemuPC.c
@@ -0,0 +1,38 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HstiLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PciLib.h>
+
+#include <IndustryStandard/Hsti.h>
+#include <IndustryStandard/Q35MchIch9.h>
+
+#include "VirtHstiDxe.h"
+
+STATIC VIRT_ADAPTER_INFO_PLATFORM_SECURITY mHstiPC = {
+ PLATFORM_SECURITY_VERSION_VNEXTCS,
+ PLATFORM_SECURITY_ROLE_PLATFORM_REFERENCE,
+ { L"OVMF (Qemu PC)" },
+ VIRT_HSTI_SECURITY_FEATURE_SIZE,
+};
+
+VIRT_ADAPTER_INFO_PLATFORM_SECURITY *
+VirtHstiQemuPCInit (
+ VOID
+ )
+{
+ return &mHstiPC;
+}
+
+VOID
+VirtHstiQemuPCVerify (
+ VOID
+ )
+{
+}
diff --git a/OvmfPkg/VirtHstiDxe/QemuQ35.c b/OvmfPkg/VirtHstiDxe/QemuQ35.c
new file mode 100644
index 000000000000..75e9731b4a52
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/QemuQ35.c
@@ -0,0 +1,58 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HstiLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PciLib.h>
+
+#include <IndustryStandard/Hsti.h>
+#include <IndustryStandard/Q35MchIch9.h>
+
+#include "VirtHstiDxe.h"
+
+STATIC VIRT_ADAPTER_INFO_PLATFORM_SECURITY mHstiQ35 = {
+ PLATFORM_SECURITY_VERSION_VNEXTCS,
+ PLATFORM_SECURITY_ROLE_PLATFORM_REFERENCE,
+ { L"OVMF (Qemu Q35)" },
+ VIRT_HSTI_SECURITY_FEATURE_SIZE,
+};
+
+VIRT_ADAPTER_INFO_PLATFORM_SECURITY *
+VirtHstiQemuQ35Init (
+ VOID
+ )
+{
+ if (FeaturePcdGet (PcdSmmSmramRequire)) {
+ VirtHstiSetSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK);
+ }
+
+ return &mHstiQ35;
+}
+
+VOID
+VirtHstiQemuQ35Verify (
+ VOID
+ )
+{
+ if (VirtHstiIsSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK)) {
+ CHAR16 *ErrorMsg = NULL;
+ UINT8 SmramVal;
+ UINT8 EsmramcVal;
+
+ SmramVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_SMRAM));
+ EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC));
+
+ if (!(EsmramcVal & MCH_ESMRAMC_T_EN)) {
+ ErrorMsg = L"q35 smram access is open";
+ } else if (!(SmramVal & MCH_SMRAM_D_LCK)) {
+ ErrorMsg = L"q35 smram config is not locked";
+ }
+
+ VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK);
+ }
+}
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
new file mode 100644
index 000000000000..74e5e6bd9d4f
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
@@ -0,0 +1,169 @@
+/** @file
+ This file contains DXE driver for publishing empty HSTI table
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2024, Red Hat. Inc
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/HobLib.h>
+#include <Library/HstiLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/PlatformInitLib.h>
+
+#include <IndustryStandard/Hsti.h>
+#include <IndustryStandard/I440FxPiix4.h>
+#include <IndustryStandard/Q35MchIch9.h>
+
+#include "VirtHstiDxe.h"
+
+VOID
+VirtHstiSetSupported (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ )
+{
+ ASSERT (ByteIndex < VIRT_HSTI_SECURITY_FEATURE_SIZE);
+ VirtHsti->SecurityFeaturesRequired[ByteIndex] |= BitMask;
+ VirtHsti->SecurityFeaturesImplemented[ByteIndex] |= BitMask;
+}
+
+BOOLEAN
+VirtHstiIsSupported (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ )
+{
+ ASSERT (ByteIndex < VIRT_HSTI_SECURITY_FEATURE_SIZE);
+ return VirtHsti->SecurityFeaturesImplemented[ByteIndex] & BitMask;
+}
+
+VOID
+VirtHstiTestResult (
+ CHAR16 *ErrorMsg,
+ IN UINT32 ByteIndex,
+ IN UINT8 BitMask
+ )
+{
+ EFI_STATUS Status;
+
+ ASSERT (ByteIndex < VIRT_HSTI_SECURITY_FEATURE_SIZE);
+
+ if (ErrorMsg) {
+ DEBUG ((DEBUG_ERROR, "VirtHsti: Test failed: %s\n", ErrorMsg));
+ Status = HstiLibAppendErrorString (
+ PLATFORM_SECURITY_ROLE_PLATFORM_REFERENCE,
+ NULL,
+ ErrorMsg
+ );
+ ASSERT_EFI_ERROR (Status);
+ } else {
+ Status = HstiLibSetFeaturesVerified (
+ PLATFORM_SECURITY_ROLE_PLATFORM_REFERENCE,
+ NULL,
+ ByteIndex,
+ BitMask
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+}
+
+STATIC
+UINT16
+VirtHstiGetHostBridgeDevId (
+ VOID
+ )
+{
+ EFI_HOB_GUID_TYPE *GuidHob;
+ EFI_HOB_PLATFORM_INFO *PlatformInfo;
+
+ GuidHob = GetFirstGuidHob (&gUefiOvmfPkgPlatformInfoGuid);
+ ASSERT (GuidHob);
+ PlatformInfo = (EFI_HOB_PLATFORM_INFO *)GET_GUID_HOB_DATA (GuidHob);
+ return PlatformInfo->HostBridgeDevId;
+}
+
+STATIC
+VOID
+EFIAPI
+VirtHstiOnReadyToBoot (
+ EFI_EVENT Event,
+ VOID *Context
+ )
+{
+ switch (VirtHstiGetHostBridgeDevId ()) {
+ case INTEL_82441_DEVICE_ID:
+ VirtHstiQemuPCVerify ();
+ break;
+ case INTEL_Q35_MCH_DEVICE_ID:
+ VirtHstiQemuQ35Verify ();
+ break;
+ default:
+ ASSERT (FALSE);
+ }
+
+ if (Event != NULL) {
+ gBS->CloseEvent (Event);
+ }
+}
+
+/**
+ The driver's entry point.
+
+ @param[in] ImageHandle The firmware allocated handle for the EFI image.
+ @param[in] SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The entry point is executed successfully.
+ @retval other Some error occurs when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+VirtHstiDxeEntrypoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti;
+ UINT16 DevId;
+ EFI_STATUS Status;
+ EFI_EVENT Event;
+
+ DevId = VirtHstiGetHostBridgeDevId ();
+ switch (DevId) {
+ case INTEL_82441_DEVICE_ID:
+ VirtHsti = VirtHstiQemuPCInit ();
+ break;
+ case INTEL_Q35_MCH_DEVICE_ID:
+ VirtHsti = VirtHstiQemuQ35Init ();
+ break;
+ default:
+ DEBUG ((DEBUG_INFO, "%a: unknown platform (0x%x)\n", __func__, DevId));
+ return EFI_UNSUPPORTED;
+ }
+
+ Status = HstiLibSetTable (VirtHsti, sizeof (*VirtHsti));
+ if (EFI_ERROR (Status)) {
+ if (Status != EFI_ALREADY_STARTED) {
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+
+ EfiCreateEventReadyToBootEx (
+ TPL_NOTIFY,
+ VirtHstiOnReadyToBoot,
+ NULL,
+ &Event
+ );
+
+ return EFI_SUCCESS;
+}
--
2.44.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117047): https://edk2.groups.io/g/devel/message/117047
Mute This Topic: https://groups.io/mt/105086164/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 11+ messages in thread* [edk2-devel] [PATCH 2/4] OvmfPkg: Add VirtHstiDxe to OVMF firmware build
2024-03-22 14:27 [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 1/4] " Gerd Hoffmann
@ 2024-03-22 14:27 ` Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 3/4] OvmfPkg/VirtHstiDxe: add varstore flash check Gerd Hoffmann
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-03-22 14:27 UTC (permalink / raw)
To: devel
Cc: Oliver Steffen, Konstantin Kostiuk, Ard Biesheuvel, Jiewen Yao,
Gerd Hoffmann
From: Konstantin Kostiuk <kkostiuk@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 2 ++
OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
OvmfPkg/OvmfPkgX64.dsc | 2 ++
OvmfPkg/OvmfPkgIa32.fdf | 1 +
OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
OvmfPkg/OvmfPkgX64.fdf | 1 +
6 files changed, 9 insertions(+)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 713f08764b07..52b05cb373fb 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -188,6 +188,7 @@ [LibraryClasses]
PeiHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/PeiHardwareInfoLib.inf
DxeHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/DxeHardwareInfoLib.inf
ImagePropertiesRecordLib|MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
+ HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
@@ -828,6 +829,7 @@ [Components]
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+ OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
#
# ISA Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 90b15dc27097..be386914b047 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -193,6 +193,7 @@ [LibraryClasses]
PeiHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/PeiHardwareInfoLib.inf
DxeHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/DxeHardwareInfoLib.inf
ImagePropertiesRecordLib|MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
+ HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
!endif
@@ -842,6 +843,7 @@ [Components.X64]
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+ OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
#
# ISA Support
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 56c920168d25..ae8c7e2fbdcb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -205,6 +205,7 @@ [LibraryClasses]
PeiHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/PeiHardwareInfoLib.inf
DxeHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/DxeHardwareInfoLib.inf
ImagePropertiesRecordLib|MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.inf
+ HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
!if $(SMM_REQUIRE) == FALSE
LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
@@ -910,6 +911,7 @@ [Components]
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+ OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
#
# ISA Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 6c56c5e53f21..6eb26f7d4613 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -316,6 +316,7 @@ [FV.DXEFV]
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
+INF OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index ee8068ad55dc..080784f722a7 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -323,6 +323,7 @@ [FV.DXEFV]
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
+INF OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index eb3fb90cb8b6..5bc979bc91b7 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -350,6 +350,7 @@ [FV.DXEFV]
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
+INF OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
--
2.44.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117048): https://edk2.groups.io/g/devel/message/117048
Mute This Topic: https://groups.io/mt/105086166/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 11+ messages in thread* [edk2-devel] [PATCH 3/4] OvmfPkg/VirtHstiDxe: add varstore flash check
2024-03-22 14:27 [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 1/4] " Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 2/4] OvmfPkg: Add VirtHstiDxe to OVMF firmware build Gerd Hoffmann
@ 2024-03-22 14:27 ` Gerd Hoffmann
2024-03-22 14:27 ` [edk2-devel] [PATCH 4/4] OvmfPkg/VirtHstiDxe: add code " Gerd Hoffmann
2024-04-17 8:18 ` [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
4 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-03-22 14:27 UTC (permalink / raw)
To: devel
Cc: Oliver Steffen, Gerd Hoffmann, Ard Biesheuvel, Jiewen Yao,
Konstantin Kostiuk
Detects qemu config issue: vars pflash is not in secure mode (write
access restricted to smm). Applies to Q35 with SMM only.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf | 4 ++
OvmfPkg/VirtHstiDxe/VirtHstiDxe.h | 16 ++++-
OvmfPkg/VirtHstiDxe/Flash.c | 90 +++++++++++++++++++++++++++++
OvmfPkg/VirtHstiDxe/QemuQ35.c | 13 +++++
4 files changed, 122 insertions(+), 1 deletion(-)
create mode 100644 OvmfPkg/VirtHstiDxe/Flash.c
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
index 8c63ff6a8953..9cb2ed1f0c64 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
@@ -22,6 +22,7 @@ [Sources]
VirtHstiDxe.c
QemuPC.c
QemuQ35.c
+ Flash.c
[Packages]
MdePkg/MdePkg.dec
@@ -46,5 +47,8 @@ [Guids]
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+
[Depex]
TRUE
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
index 26109bf322e9..ca4e376582ad 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
@@ -6,7 +6,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define VIRT_HSTI_SECURITY_FEATURE_SIZE 2
-#define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
+#define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
+#define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
typedef struct {
// ADAPTER_INFO_PLATFORM_SECURITY
@@ -65,3 +66,16 @@ VOID
VirtHstiQemuPCVerify (
VOID
);
+
+/* Flash.c */
+
+#define QEMU_FIRMWARE_FLASH_UNKNOWN 0
+#define QEMU_FIRMWARE_FLASH_IS_ROM 1
+#define QEMU_FIRMWARE_FLASH_IS_RAM 2
+#define QEMU_FIRMWARE_FLASH_READ_ONLY 3
+#define QEMU_FIRMWARE_FLASH_WRITABLE 4
+
+UINT32
+VirtHstiQemuFirmwareFlashCheck (
+ UINT32 Address
+ );
diff --git a/OvmfPkg/VirtHstiDxe/Flash.c b/OvmfPkg/VirtHstiDxe/Flash.c
new file mode 100644
index 000000000000..e93356793f8c
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/Flash.c
@@ -0,0 +1,90 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+
+#include "VirtHstiDxe.h"
+
+#define WRITE_BYTE_CMD 0x10
+#define BLOCK_ERASE_CMD 0x20
+#define CLEAR_STATUS_CMD 0x50
+#define READ_STATUS_CMD 0x70
+#define READ_DEVID_CMD 0x90
+#define BLOCK_ERASE_CONFIRM_CMD 0xd0
+#define READ_ARRAY_CMD 0xff
+#define CLEARED_ARRAY_STATUS 0x00
+
+/* based on QemuFlashDetected (QemuFlashFvbServicesRuntimeDxe) */
+UINT32
+VirtHstiQemuFirmwareFlashCheck (
+ UINT32 Address
+ )
+{
+ volatile UINT8 *Ptr;
+
+ UINTN Offset;
+ UINT8 OriginalUint8;
+ UINT8 ProbeUint8;
+
+ for (Offset = 0; Offset < EFI_PAGE_SIZE; Offset++) {
+ Ptr = (UINT8 *)(UINTN)(Address + Offset);
+ ProbeUint8 = *Ptr;
+ if ((ProbeUint8 != CLEAR_STATUS_CMD) &&
+ (ProbeUint8 != READ_STATUS_CMD) &&
+ (ProbeUint8 != CLEARED_ARRAY_STATUS))
+ {
+ break;
+ }
+ }
+
+ if (Offset >= EFI_PAGE_SIZE) {
+ DEBUG ((DEBUG_INFO, "%a: check failed\n", __func__));
+ return QEMU_FIRMWARE_FLASH_UNKNOWN;
+ }
+
+ OriginalUint8 = *Ptr;
+ *Ptr = CLEAR_STATUS_CMD;
+ ProbeUint8 = *Ptr;
+ if ((OriginalUint8 != CLEAR_STATUS_CMD) &&
+ (ProbeUint8 == CLEAR_STATUS_CMD))
+ {
+ *Ptr = OriginalUint8;
+ DEBUG ((DEBUG_INFO, "%a: %p behaves as RAM\n", __func__, Ptr));
+ return QEMU_FIRMWARE_FLASH_IS_RAM;
+ }
+
+ *Ptr = READ_STATUS_CMD;
+ ProbeUint8 = *Ptr;
+ if (ProbeUint8 == OriginalUint8) {
+ DEBUG ((DEBUG_INFO, "%a: %p behaves as ROM\n", __func__, Ptr));
+ return QEMU_FIRMWARE_FLASH_IS_ROM;
+ }
+
+ if (ProbeUint8 == READ_STATUS_CMD) {
+ *Ptr = OriginalUint8;
+ DEBUG ((DEBUG_INFO, "%a: %p behaves as RAM\n", __func__, Ptr));
+ return QEMU_FIRMWARE_FLASH_IS_RAM;
+ }
+
+ if (ProbeUint8 == CLEARED_ARRAY_STATUS) {
+ *Ptr = WRITE_BYTE_CMD;
+ *Ptr = OriginalUint8;
+ *Ptr = READ_STATUS_CMD;
+ ProbeUint8 = *Ptr;
+ *Ptr = READ_ARRAY_CMD;
+ if (ProbeUint8 & 0x10 /* programming error */) {
+ DEBUG ((DEBUG_INFO, "%a: %p behaves as FLASH, write-protected\n", __func__, Ptr));
+ return QEMU_FIRMWARE_FLASH_READ_ONLY;
+ } else {
+ DEBUG ((DEBUG_INFO, "%a: %p behaves as FLASH, writable\n", __func__, Ptr));
+ return QEMU_FIRMWARE_FLASH_WRITABLE;
+ }
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: check failed\n", __func__));
+ return QEMU_FIRMWARE_FLASH_UNKNOWN;
+}
diff --git a/OvmfPkg/VirtHstiDxe/QemuQ35.c b/OvmfPkg/VirtHstiDxe/QemuQ35.c
index 75e9731b4a52..203122627d2d 100644
--- a/OvmfPkg/VirtHstiDxe/QemuQ35.c
+++ b/OvmfPkg/VirtHstiDxe/QemuQ35.c
@@ -29,6 +29,7 @@ VirtHstiQemuQ35Init (
{
if (FeaturePcdGet (PcdSmmSmramRequire)) {
VirtHstiSetSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK);
+ VirtHstiSetSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH);
}
return &mHstiQ35;
@@ -55,4 +56,16 @@ VirtHstiQemuQ35Verify (
VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK);
}
+
+ if (VirtHstiIsSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH)) {
+ CHAR16 *ErrorMsg = NULL;
+
+ switch (VirtHstiQemuFirmwareFlashCheck (PcdGet32 (PcdOvmfFdBaseAddress))) {
+ case QEMU_FIRMWARE_FLASH_WRITABLE:
+ ErrorMsg = L"qemu vars pflash is not secure";
+ break;
+ }
+
+ VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH);
+ }
}
--
2.44.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117049): https://edk2.groups.io/g/devel/message/117049
Mute This Topic: https://groups.io/mt/105086167/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 11+ messages in thread* [edk2-devel] [PATCH 4/4] OvmfPkg/VirtHstiDxe: add code flash check
2024-03-22 14:27 [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
` (2 preceding siblings ...)
2024-03-22 14:27 ` [edk2-devel] [PATCH 3/4] OvmfPkg/VirtHstiDxe: add varstore flash check Gerd Hoffmann
@ 2024-03-22 14:27 ` Gerd Hoffmann
2024-04-17 8:18 ` [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
4 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-03-22 14:27 UTC (permalink / raw)
To: devel
Cc: Oliver Steffen, Gerd Hoffmann, Ard Biesheuvel, Jiewen Yao,
Konstantin Kostiuk
Detects qemu config issue: code pflash is writable.
Checked for both PC and Q35.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf | 2 ++
OvmfPkg/VirtHstiDxe/VirtHstiDxe.h | 13 +++++++++++
OvmfPkg/VirtHstiDxe/QemuCommon.c | 36 +++++++++++++++++++++++++++++
OvmfPkg/VirtHstiDxe/VirtHstiDxe.c | 4 ++++
4 files changed, 55 insertions(+)
create mode 100644 OvmfPkg/VirtHstiDxe/QemuCommon.c
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
index 9cb2ed1f0c64..376cd28aeb7e 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
@@ -22,6 +22,7 @@ [Sources]
VirtHstiDxe.c
QemuPC.c
QemuQ35.c
+ QemuCommon.c
Flash.c
[Packages]
@@ -49,6 +50,7 @@ [FeaturePcd]
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase
[Depex]
TRUE
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
index ca4e376582ad..b915f5cdf5ac 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
@@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
#define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
+#define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH BIT2
typedef struct {
// ADAPTER_INFO_PLATFORM_SECURITY
@@ -67,6 +68,18 @@ VirtHstiQemuPCVerify (
VOID
);
+/* QemuCommon.c */
+
+VOID
+VirtHstiQemuCommonInit (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti
+ );
+
+VOID
+VirtHstiQemuCommonVerify (
+ VOID
+ );
+
/* Flash.c */
#define QEMU_FIRMWARE_FLASH_UNKNOWN 0
diff --git a/OvmfPkg/VirtHstiDxe/QemuCommon.c b/OvmfPkg/VirtHstiDxe/QemuCommon.c
new file mode 100644
index 000000000000..4ab3fe2d6e63
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/QemuCommon.c
@@ -0,0 +1,36 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+
+#include "VirtHstiDxe.h"
+
+VOID
+VirtHstiQemuCommonInit (
+ VIRT_ADAPTER_INFO_PLATFORM_SECURITY *VirtHsti
+ )
+{
+ VirtHstiSetSupported (VirtHsti, 0, VIRT_HSTI_BYTE0_READONLY_CODE_FLASH);
+}
+
+VOID
+VirtHstiQemuCommonVerify (
+ VOID
+ )
+{
+ CHAR16 *ErrorMsg;
+
+ switch (VirtHstiQemuFirmwareFlashCheck (PcdGet32 (PcdBfvBase))) {
+ case QEMU_FIRMWARE_FLASH_WRITABLE:
+ ErrorMsg = L"qemu code pflash is writable";
+ break;
+ default:
+ ErrorMsg = NULL;
+ }
+
+ VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_READONLY_CODE_FLASH);
+}
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
index 74e5e6bd9d4f..b6e53a1219d1 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.c
@@ -104,9 +104,11 @@ VirtHstiOnReadyToBoot (
switch (VirtHstiGetHostBridgeDevId ()) {
case INTEL_82441_DEVICE_ID:
VirtHstiQemuPCVerify ();
+ VirtHstiQemuCommonVerify ();
break;
case INTEL_Q35_MCH_DEVICE_ID:
VirtHstiQemuQ35Verify ();
+ VirtHstiQemuCommonVerify ();
break;
default:
ASSERT (FALSE);
@@ -142,9 +144,11 @@ VirtHstiDxeEntrypoint (
switch (DevId) {
case INTEL_82441_DEVICE_ID:
VirtHsti = VirtHstiQemuPCInit ();
+ VirtHstiQemuCommonInit (VirtHsti);
break;
case INTEL_Q35_MCH_DEVICE_ID:
VirtHsti = VirtHstiQemuQ35Init ();
+ VirtHstiQemuCommonInit (VirtHsti);
break;
default:
DEBUG ((DEBUG_INFO, "%a: unknown platform (0x%x)\n", __func__, DevId));
--
2.44.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117051): https://edk2.groups.io/g/devel/message/117051
Mute This Topic: https://groups.io/mt/105086175/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-03-22 14:27 [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
` (3 preceding siblings ...)
2024-03-22 14:27 ` [edk2-devel] [PATCH 4/4] OvmfPkg/VirtHstiDxe: add code " Gerd Hoffmann
@ 2024-04-17 8:18 ` Gerd Hoffmann
2024-04-17 11:38 ` Ard Biesheuvel
2024-04-17 13:20 ` Yao, Jiewen
4 siblings, 2 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-04-17 8:18 UTC (permalink / raw)
To: devel, Ard Biesheuvel, jiewen; +Cc: Oliver Steffen
On Fri, Mar 22, 2024 at 03:27:31PM +0100, Gerd Hoffmann wrote:
>
>
> Gerd Hoffmann (2):
> OvmfPkg/VirtHstiDxe: add varstore flash check
> OvmfPkg/VirtHstiDxe: add code flash check
>
> Konstantin Kostiuk (2):
> OvmfPkg: Add VirtHstiDxe driver
> OvmfPkg: Add VirtHstiDxe to OVMF firmware build
Ping. Any comments on this series?
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117909): https://edk2.groups.io/g/devel/message/117909
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-04-17 8:18 ` [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
@ 2024-04-17 11:38 ` Ard Biesheuvel
2024-04-18 11:09 ` Gerd Hoffmann
2024-04-17 13:20 ` Yao, Jiewen
1 sibling, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2024-04-17 11:38 UTC (permalink / raw)
To: devel, kraxel; +Cc: jiewen, Oliver Steffen
On Wed, 17 Apr 2024 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Fri, Mar 22, 2024 at 03:27:31PM +0100, Gerd Hoffmann wrote:
> >
> >
> > Gerd Hoffmann (2):
> > OvmfPkg/VirtHstiDxe: add varstore flash check
> > OvmfPkg/VirtHstiDxe: add code flash check
> >
> > Konstantin Kostiuk (2):
> > OvmfPkg: Add VirtHstiDxe driver
> > OvmfPkg: Add VirtHstiDxe to OVMF firmware build
>
> Ping. Any comments on this series?
>
Dunno. What does it do?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117917): https://edk2.groups.io/g/devel/message/117917
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-04-17 11:38 ` Ard Biesheuvel
@ 2024-04-18 11:09 ` Gerd Hoffmann
0 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2024-04-18 11:09 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, jiewen, Oliver Steffen
On Wed, Apr 17, 2024 at 01:38:20PM +0200, Ard Biesheuvel wrote:
> On Wed, 17 Apr 2024 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Fri, Mar 22, 2024 at 03:27:31PM +0100, Gerd Hoffmann wrote:
> > >
> > >
> > > Gerd Hoffmann (2):
> > > OvmfPkg/VirtHstiDxe: add varstore flash check
> > > OvmfPkg/VirtHstiDxe: add code flash check
> > >
> > > Konstantin Kostiuk (2):
> > > OvmfPkg: Add VirtHstiDxe driver
> > > OvmfPkg: Add VirtHstiDxe to OVMF firmware build
> >
> > Ping. Any comments on this series?
> >
>
> Dunno. What does it do?
It implements https://learn.microsoft.com/en-us/windows-hardware/test/hlk/testref/hardware-security-testability-specification
for OVMF.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117982): https://edk2.groups.io/g/devel/message/117982
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-04-17 8:18 ` [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
2024-04-17 11:38 ` Ard Biesheuvel
@ 2024-04-17 13:20 ` Yao, Jiewen
2024-04-18 11:45 ` Gerd Hoffmann
1 sibling, 1 reply; 11+ messages in thread
From: Yao, Jiewen @ 2024-04-17 13:20 UTC (permalink / raw)
To: devel@edk2.groups.io, kraxel@redhat.com, Ard Biesheuvel,
jiewen@dobby.home.kraxel.org
Cc: Oliver Steffen, Yao, Jiewen
That is good start. The SMRAM lock and Flash lock seem good to me.
Comment:
1) Do we really need to add "Q35" for the policy?
#define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
#define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
I feel we had better remove it, since SMM_SMRAM_LOCK and SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.
2) Would you please let me know what "READONLY_CODE_FLASH" really means?
#define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
#define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH BIT2
Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
Or does it just mean NO write in normal operation mode, but still writable in SMM mode?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Wednesday, April 17, 2024 4:18 PM
> To: devel@edk2.groups.io; Ard Biesheuvel <ardb@kernel.org>;
> jiewen@dobby.home.kraxel.org
> Cc: Oliver Steffen <osteffen@redhat.com>
> Subject: Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
>
> On Fri, Mar 22, 2024 at 03:27:31PM +0100, Gerd Hoffmann wrote:
> >
> >
> > Gerd Hoffmann (2):
> > OvmfPkg/VirtHstiDxe: add varstore flash check
> > OvmfPkg/VirtHstiDxe: add code flash check
> >
> > Konstantin Kostiuk (2):
> > OvmfPkg: Add VirtHstiDxe driver
> > OvmfPkg: Add VirtHstiDxe to OVMF firmware build
>
> Ping. Any comments on this series?
>
> take care,
> Gerd
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117919): https://edk2.groups.io/g/devel/message/117919
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-04-17 13:20 ` Yao, Jiewen
@ 2024-04-18 11:45 ` Gerd Hoffmann
2024-04-18 14:01 ` Yao, Jiewen
0 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2024-04-18 11:45 UTC (permalink / raw)
To: Yao, Jiewen; +Cc: devel@edk2.groups.io, Ard Biesheuvel, Oliver Steffen
On Wed, Apr 17, 2024 at 01:20:57PM +0000, Yao, Jiewen wrote:
> That is good start. The SMRAM lock and Flash lock seem good to me.
>
> Comment:
> 1) Do we really need to add "Q35" for the policy?
> #define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
> #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
>
> I feel we had better remove it, since SMM_SMRAM_LOCK and SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.
Well, SMM mode is supported for the qemu 'q35' machine type only, the
'pc' machine type doesn't provide enough memory for SMM. Which why I've
added 'Q35' to the name.
The SMM_SMRAM_LOCK test actually is q35-specific because the control
registers are chipset specific. But, yes, the concept is not q35
specific.
I can drop 'Q35' if you prefer it that way.
> 2) Would you please let me know what "READONLY_CODE_FLASH" really means?
>
> #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
> #define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH BIT2
>
> Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
> Or does it just mean NO write in normal operation mode, but still writable in SMM mode?
With qemu being configured properly flash behavior should be this:
| OVMF_CODE.fd | OVMF_VARS.fd
-------------------------------+----------------+----------------
SMM_REQUIRE=TRUE, SMM mode | read-only | writable
SMM_REQUIRE=TRUE, normal mode | read-only (1) | read-only (2)
SMM_REQUIRE=FALSE | read-only (3) | writable
VIRT_HSTI_BYTE0_READONLY_CODE_FLASH will verify (1) + (3).
VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH will verify (2).
(probably a good idea to add that as comment to the patches).
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117983): https://edk2.groups.io/g/devel/message/117983
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
2024-04-18 11:45 ` Gerd Hoffmann
@ 2024-04-18 14:01 ` Yao, Jiewen
0 siblings, 0 replies; 11+ messages in thread
From: Yao, Jiewen @ 2024-04-18 14:01 UTC (permalink / raw)
To: kraxel@redhat.com; +Cc: devel@edk2.groups.io, Ard Biesheuvel, Oliver Steffen
1) Yes, I highly recommend remove Q35 keyword.
2) Got it. I think we had better add such info in the code as comment as well.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Thursday, April 18, 2024 7:45 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb@kernel.org>; Oliver Steffen
> <osteffen@redhat.com>
> Subject: Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver
>
> On Wed, Apr 17, 2024 at 01:20:57PM +0000, Yao, Jiewen wrote:
> > That is good start. The SMRAM lock and Flash lock seem good to me.
> >
> > Comment:
> > 1) Do we really need to add "Q35" for the policy?
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SMRAM_LOCK BIT0
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
> >
> > I feel we had better remove it, since SMM_SMRAM_LOCK and
> SMM_SECURE_VARS_FLASH are common features for almost all X86 platforms.
>
> Well, SMM mode is supported for the qemu 'q35' machine type only, the
> 'pc' machine type doesn't provide enough memory for SMM. Which why I've
> added 'Q35' to the name.
>
> The SMM_SMRAM_LOCK test actually is q35-specific because the control
> registers are chipset specific. But, yes, the concept is not q35
> specific.
>
> I can drop 'Q35' if you prefer it that way.
>
> > 2) Would you please let me know what "READONLY_CODE_FLASH" really
> means?
> >
> > #define VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH BIT1
> > #define VIRT_HSTI_BYTE0_READONLY_CODE_FLASH BIT2
> >
> > Does READONLY_CODE_FLASH mean NO write to flash even in SMM mode?
> > Or does it just mean NO write in normal operation mode, but still writable in
> SMM mode?
>
> With qemu being configured properly flash behavior should be this:
>
> | OVMF_CODE.fd | OVMF_VARS.fd
> -------------------------------+----------------+----------------
> SMM_REQUIRE=TRUE, SMM mode | read-only | writable
> SMM_REQUIRE=TRUE, normal mode | read-only (1) | read-only (2)
> SMM_REQUIRE=FALSE | read-only (3) | writable
>
> VIRT_HSTI_BYTE0_READONLY_CODE_FLASH will verify (1) + (3).
> VIRT_HSTI_BYTE0_Q35_SMM_SECURE_VARS_FLASH will verify (2).
>
> (probably a good idea to add that as comment to the patches).
>
> take care,
> Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117993): https://edk2.groups.io/g/devel/message/117993
Mute This Topic: https://groups.io/mt/105086174/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 11+ messages in thread