[edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

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

 OvmfPkg/OvmfPkgIa32.dsc             |   2 +
 OvmfPkg/OvmfPkgIa32X64.dsc          |   2 +
 OvmfPkg/OvmfPkgX64.dsc              |   2 +
 OvmfPkg/OvmfPkgIa32.fdf             |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf          |   1 +
 OvmfPkg/OvmfPkgX64.fdf              |   1 +
 OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf |  56 +++++++++
 OvmfPkg/VirtHstiDxe/VirtHstiDxe.h   |  94 +++++++++++++++
 OvmfPkg/VirtHstiDxe/Flash.c         |  90 +++++++++++++++
 OvmfPkg/VirtHstiDxe/QemuCommon.c    |  36 ++++++
 OvmfPkg/VirtHstiDxe/QemuPC.c        |  38 ++++++
 OvmfPkg/VirtHstiDxe/QemuQ35.c       |  71 ++++++++++++
 OvmfPkg/VirtHstiDxe/VirtHstiDxe.c   | 173 ++++++++++++++++++++++++++++
 13 files changed, 567 insertions(+)
 create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
 create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
 create mode 100644 OvmfPkg/VirtHstiDxe/Flash.c
 create mode 100644 OvmfPkg/VirtHstiDxe/QemuCommon.c
 create mode 100644 OvmfPkg/VirtHstiDxe/QemuPC.c
 create mode 100644 OvmfPkg/VirtHstiDxe/QemuQ35.c
 create mode 100644 OvmfPkg/VirtHstiDxe/VirtHstiDxe.c

-- 
2.44.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117050): https://edk2.groups.io/g/devel/message/117050
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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 1/4] OvmfPkg: Add VirtHstiDxe driver

[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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/4] OvmfPkg: Add VirtHstiDxe to OVMF firmware build

[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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 3/4] OvmfPkg/VirtHstiDxe: add varstore flash check

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 4/4] OvmfPkg/VirtHstiDxe: add code flash check

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Ard Biesheuvel]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Gerd Hoffmann]

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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] OvmfPkg: Add VirtHstiDxe driver

[Yao, Jiewen]

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]
-=-=-=-=-=-=-=-=-=-=-=-