[edk2-devel] [PATCH 1/5] OvmfPkg: Handle Cloud Hypervisor host bridge

2021-12-01 Thread sebastien . boeuf 
From: Sebastien Boeuf 

Handle things differently when the detected host bridge matches the
Cloud Hypervisor PCI host bridge identifier.

Signed-off-by: Rob Bradford 
Signed-off-by: Sebastien Boeuf 
---
 OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c   |  1 +
 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c  |  1 +
 OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h  |  1 +
 OvmfPkg/Include/IndustryStandard/CloudHv.h| 35 +++
 OvmfPkg/Include/OvmfPlatforms.h   |  1 +
 .../Library/AcpiTimerLib/BaseAcpiTimerLib.c   |  3 +
 .../AcpiTimerLib/BaseRomAcpiTimerLib.c|  4 ++
 .../Library/AcpiTimerLib/DxeAcpiTimerLib.c|  3 +
 .../PlatformBootManagerLib/BdsPlatform.c  |  1 +
 .../ResetSystemLib/BaseResetShutdown.c|  3 +
 .../Library/ResetSystemLib/DxeResetShutdown.c | 11 +++-
 OvmfPkg/PlatformPei/MemDetect.c   | 63 ++-
 OvmfPkg/PlatformPei/Platform.c| 11 +++-
 13 files changed, 105 insertions(+), 33 deletions(-)
 create mode 100644 OvmfPkg/Include/IndustryStandard/CloudHv.h

diff --git a/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c 
b/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c
index 97ca21945f..50c9322911 100644
--- a/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c
+++ b/OvmfPkg/Csm/CsmSupportLib/LegacyInterrupt.c
@@ -189,6 +189,7 @@ LegacyInterruptInstall (
   mLegacyInterruptDevice = LEGACY_INT_DEV_PIIX4;
   break;
 case INTEL_Q35_MCH_DEVICE_ID:
+case CLOUDHV_DEVICE_ID: // Cloud Hypervisor host bridge
   mLegacyInterruptDevice = LEGACY_INT_DEV_Q35;
   break;
 default:
diff --git a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c 
b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c
index fe9ae27c9d..9d6d6faf48 100644
--- a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c
+++ b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.c
@@ -474,6 +474,7 @@ LegacyRegionInit (
 mRegisterValues = mRegisterValues440;
 break;
   case INTEL_Q35_MCH_DEVICE_ID:
+  case CLOUDHV_DEVICE_ID: // Cloud Hypervisor host bridge
 mRegisterValues = mRegisterValuesQ35;
 break;
   default:
diff --git a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h 
b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
index e18cb97949..71df8f5fb2 100644
--- a/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
+++ b/OvmfPkg/Csm/CsmSupportLib/LegacyRegion.h
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/OvmfPkg/Include/IndustryStandard/CloudHv.h 
b/OvmfPkg/Include/IndustryStandard/CloudHv.h
new file mode 100644
index 00..6ab18ad50d
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/CloudHv.h
@@ -0,0 +1,35 @@
+/** @file
+  Various defines related to Cloud Hypervisor
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef __CLOUDHV_H__
+#define __CLOUDHV_H__
+
+//
+// Host Bridge Device ID
+//
+#define CLOUDHV_DEVICE_ID 0x0d57
+
+//
+// ACPI timer address
+//
+#define CLOUDHV_ACPI_TIMER_IO_ADDRESS 0xb008
+
+//
+// ACPI shutdown device address
+//
+#define CLOUDHV_ACPI_SHUTDOWN_IO_ADDRESS 0x03c0
+
+//
+// 32-bit MMIO memory hole base address
+//
+#define CLOUDHV_MMIO_HOLE_ADDRESS 0xc000
+
+//
+// 32-bit MMIO memory hole size
+//
+#define CLOUDHV_MMIO_HOLE_SIZE 0x3800
+
+#endif // __CLOUDHV_H__
diff --git a/OvmfPkg/Include/OvmfPlatforms.h b/OvmfPkg/Include/OvmfPlatforms.h
index 3b85593b70..ad0b0d2803 100644
--- a/OvmfPkg/Include/OvmfPlatforms.h
+++ b/OvmfPkg/Include/OvmfPlatforms.h
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 //
 // OVMF Host Bridge DID Address
diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c 
b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
index 7c593e8be1..e182ac2b7d 100644
--- a/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
+++ b/OvmfPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.c
@@ -55,6 +55,9 @@ AcpiTimerLibConstructor (
   AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);
   AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;
   break;
+case CLOUDHV_DEVICE_ID:
+  mAcpiTimerIoAddr =  CLOUDHV_ACPI_TIMER_IO_ADDRESS;
+  return RETURN_SUCCESS;
 default:
   DEBUG ((DEBUG_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
 __FUNCTION__, HostBridgeDevId));
diff --git a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c 
b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
index 52f3ea2dbf..a223153b2b 100644
--- a/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
+++ b/OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c
@@ -53,6 +53,8 @@ AcpiTimerLibConstructor (
   AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);
   AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;
   break;
+case CLOUDHV_DEVICE_ID:
+  return RETURN_SUCCESS;
 default:
   DEBUG ((DEBUG_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
 __FUNCTION__, HostBridgeDevId));
@@ -107,6 +109,8 @@ InternalAcpiGetTimerTick (
 case INTEL_Q35_MCH_DEVICE_ID:
   Pmba = POWER_MGMT_REGISTER_Q35 (ICH9_PMBASE);
   break;
+case CLOUDHV_DEVICE_ID:
+  return

Re: [edk2-devel] [PATCH 1/5] OvmfPkg: Handle Cloud Hypervisor host bridge

2021-12-01 Thread Gerd Hoffmann 
  Hi,

> +case CLOUDHV_DEVICE_ID: // Cloud Hypervisor host bridge

No need for the comment ...

> +++ b/OvmfPkg/Include/IndustryStandard/CloudHv.h
> @@ -0,0 +1,35 @@
> +/** @file
> +  Various defines related to Cloud Hypervisor
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#ifndef __CLOUDHV_H__
> +#define __CLOUDHV_H__
> +
> +//
> +// Host Bridge Device ID
> +//
> +#define CLOUDHV_DEVICE_ID 0x0d57

... this is enough documentation IMHO.

> +  if (mAcpiPmBaseAddress == 0) {
> +IoWrite8 (CLOUDHV_ACPI_SHUTDOWN_IO_ADDRESS, 5 << 2 | 1 << 5);

This looks like the sleep control register of the hw-reduced acpi pm
profile.  I'd suggest to use a new variable then, for example use

mAcpiHwReducedSleepCtl = CLOUDHV_ACPI_SHUTDOWN_IO_ADDRESS

> +case 0x: /* microvm */
> +  return;

Huh?

> @@ -778,21 +782,22 @@ QemuInitializeRam (
>if (IsMtrrSupported ()) {
>  MtrrGetAllMtrrs (&MtrrSettings);
>  
> -//
> -// MTRRs disabled, fixed MTRRs disabled, default type is uncached
> -//
> -ASSERT ((MtrrSettings.MtrrDefType & BIT11) == 0);
> -ASSERT ((MtrrSettings.MtrrDefType & BIT10) == 0);
> -ASSERT ((MtrrSettings.MtrrDefType & 0xFF) == 0);
> -
> -//
> -// flip default type to writeback
> -//
> -SetMem (&MtrrSettings.Fixed, sizeof MtrrSettings.Fixed, 0x06);
> -ZeroMem (&MtrrSettings.Variables, sizeof MtrrSettings.Variables);
> -MtrrSettings.MtrrDefType |= BIT11 | BIT10 | 6;
> -MtrrSetAllMtrrs (&MtrrSettings);
> +if (mHostBridgeDevId != CLOUDHV_DEVICE_ID) {

Do you need the MtrrGetAllMtrrs() call?  If not you can just use this:

   if (IsMtrrSupported () && mHostBridgeDevId != CLOUDHV_DEVICE_ID) 


> @@ -372,14 +375,18 @@ MiscInitialization (
>   MICROVM_PSEUDO_DEVICE_ID);
>ASSERT_RETURN_ERROR (PcdStatus);
>return;
> +case CLOUDHV_DEVICE_ID:
> +  DEBUG ((DEBUG_INFO, "%a: Cloud Hypervisor host bridge\n", 
> __FUNCTION__));
> +  PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId,
> + CLOUDHV_DEVICE_ID);
> +  ASSERT_RETURN_ERROR (PcdStatus);
> +  return;
>  default:
>DEBUG ((DEBUG_ERROR, "%a: Unknown Host Bridge Device ID: 0x%04x\n",
>  __FUNCTION__, mHostBridgeDevId));
>ASSERT (FALSE);
>return;
>}
> -  PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId, mHostBridgeDevId);
> -  ASSERT_RETURN_ERROR (PcdStatus);

Removing this check looks suspicious.

If it is not needed here for some reason move that change to a separate
patch with a commit message explaining things.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84204): https://edk2.groups.io/g/devel/message/84204
Mute This Topic: https://groups.io/mt/87428911/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-