[edk2-devel] [PATCH v8 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject

2021-02-21 Thread Ankur Arora
Add logic in EjectCpu() to do the actual the CPU ejection.

On the BSP, ejection happens by first selecting the CPU via
its QemuSelector and then sending the QEMU "eject" command.
QEMU in-turn signals the remote VCPU thread which context-switches
the CPU out of the SMI handler.

Meanwhile the CPU being ejected, waits around in its holding
area until it is context-switched out. Note that it is possible
that a slow CPU gets ejected before it reaches the wait loop.
However, this would never happen before it has executed the
"AllCpusInSync" loop in SmiRendezvous().
It can mean that an ejected CPU does not execute code after
that point but given that the CPU state will be destroyed by
QEMU, the missed cleanup is no great loss.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses the following reviewing comments from v6:
(1) s/CpuEject/EjectCpu/g
(2,2a,2c) Get rid of eject-worker and related.
(2b,2d) Use the PlatformSmmBspElection() logic to find out IsBSP.
(3,3b) Use CPU_HOT_EJECT_DATA->QemuSelector instead of ApicIdMap to
 do the actual ejection.
(4,5a,5b) Fix the format etc in the final unplugged log message
() Also as discussed elsewhere document the ordering requirements for
 mCpuHotEjectData->QemuSelector[] and mCpuHotEjectData->Handler.
() [from patch 2] Move definition of QEMU_CPUHP_STAT_EJECTED to this
 patch.
() s/QEMU_CPUHP_STAT_EJECTED/QEMU_CPUHP_STAT_EJECT/

 OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h  |   1 +
 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 127 +++--
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  31 +
 3 files changed, 152 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h 
b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index 2ec7a107a64d..d0e83102c13f 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
 #define QEMU_CPUHP_STAT_ENABLEDBIT0
 #define QEMU_CPUHP_STAT_INSERT BIT1
 #define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_EJECT  BIT3
 #define QEMU_CPUHP_STAT_FW_REMOVE  BIT4
 
 #define QEMU_CPUHP_RW_CMD_DATA   0x8
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 851e2b28aad9..0484be8fe43c 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -18,6 +18,7 @@
 #include  // CPU_HOT_EJECT_DATA
 #include // EFI_MM_CPU_IO_PROTOCOL
 #include   // EFI_SMM_CPU_SERVICE_PROTOCOL
+#include  // MSR_IA32_APIC_BASE_REGISTER
 #include// EFI_STATUS
 
 #include "ApicId.h"  // APIC_ID
@@ -191,12 +192,39 @@ RevokeNewSlot:
 }
 
 /**
+  EjectCpu needs to know the BSP at SMI exit at a point when
+  some of the EFI_SMM_CPU_SERVICE_PROTOCOL state has been torn
+  down.
+  Reuse the logic from OvmfPkg::PlatformSmmBspElection() to
+  do that.
+
+  @param[in] ProcessorNum  ProcessorNum denotes the processor handle number
+   in EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+STATIC
+BOOLEAN
+CheckIfBsp (
+  IN UINTN ProcessorNum
+  )
+{
+  MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
+  BOOLEAN IsBsp;
+
+  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+  IsBsp = (BOOLEAN)(ApicBaseMsr.Bits.BSP == 1);
+  return IsBsp;
+}
+
+/**
   CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
   on each CPU at exit from SMM.
 
-  If, the executing CPU is not being ejected, nothing to be done.
+  If, the executing CPU is neither the BSP, nor being ejected, nothing
+  to be done.
   If, the executing CPU is being ejected, wait in a halted loop
   until ejected.
+  If, the executing CPU is the BSP, set QEMU CPU status to eject
+  for CPUs being ejected.
 
   @param[in] ProcessorNum  ProcessorNum denotes the CPU exiting SMM,
and will be used as an index into
@@ -211,9 +239,99 @@ EjectCpu (
   )
 {
   UINT64 QemuSelector;
+  BOOLEAN IsBsp;
 
+  IsBsp = CheckIfBsp (ProcessorNum);
+
+  //
+  // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is updated
+  // on the BSP in the ongoing SMI iteration at two places:
+  //
+  // - UnplugCpus() where the BSP determines if a CPU is under ejection
+  //   or not. As the comment where mCpuHotEjectData->Handler is set-up
+  //   describes any such updates are guaranteed to be ordered-before the
+  //   dereference below.
+  //
+  // - EjectCpu() on the BSP updates QemuSelectorMap[ProcessorNum] for
+  //   CPUs after they have been hot-ejected.
+  //
+  //   The CPU under ejection: might be executing anywhere between the
+  //   "AllCpusInSync" exit loop in SmiRendezvous() to 

[edk2-devel] [PATCH v8 06/10] OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state

2021-02-21 Thread Ankur Arora
Init CPU_HOT_EJECT_DATA, which will be used to share CPU ejection
state between SmmCpuFeaturesLib (via PiSmmCpuDxeSmm) and CpuHotPlugSmm.

The init happens via SmmCpuFeaturesSmmRelocationComplete(), and so it
will run as part of the PiSmmCpuDxeSmm entry point function,
PiCpuSmmEntry(). Once inited, CPU_HOT_EJECT_DATA is exposed via
PcdCpuHotEjectDataAddress.

The CPU hot-eject handler (CPU_HOT_EJECT_DATA->Handler) is setup when
there is an ejection request via CpuHotplugSmm.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses the following review comments:
 (1) Detail in commit message about context in which CPU_HOT_EJECT_DATA
  is inited.
 (2) Add in sorted order MemoryAllocationLib in LibraryClasses
 (3) Sort added includes in SmmCpuFeaturesLib.c
 (4a-4b) Fixup linkage directives for mCpuHotEjectData.
 (5) s/CpuHotEjectData/mCpuHotEjectData/
 (6,10a,10b) Remove dependence on PcdCpuHotPlugSupport
 (7) Make the tense structure consistent in block comment for
  InitCpuHotEject().
 (8) s/SmmCpuFeaturesSmmInitHotEject/InitCpuHotEject/
 (9) s/mMaxNumberOfCpus/MaxNumberOfCpus/
 (11) Remove a bunch of obvious comments.
 (14a,14b,14c) Use SafeUint functions and rework the allocation logic
  so we can just use a single allocation.
 (12) Remove the AllocatePool() cast.
 (13) Use a CpuDeadLoop() in case of failure; albeit via a goto, not
  inline.
 (15) Initialize the mCpuHotEjectData->QemuSelectorMap locally.
 (16) Fix indentation in PcdSet64S.
 (17) Change the cast in PcdSet64S() to UINTN.
 (18) Use RETURN_STATUS instead of EFI_STATUS.
 (19,20) Move the Handler logic in SmmCpuFeaturesRendezvousExit() into
  into a separate patch.

 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  4 +
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 92 ++
 2 files changed, 96 insertions(+)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 97a10afb6e27..8a426a4c10fb 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -30,9 +30,13 @@ [LibraryClasses]
   BaseMemoryLib
   DebugLib
   MemEncryptSevLib
+  MemoryAllocationLib
   PcdLib
+  SafeIntLib
   SmmServicesTableLib
   UefiBootServicesTableLib
 
 [Pcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase
diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 7ef7ed98342e..adbfc90ad46e 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -11,10 +11,13 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -171,6 +174,92 @@ SmmCpuFeaturesHookReturnFromSmm (
   return OriginalInstructionPointer;
 }
 
+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData = NULL;
+
+/**
+  Initialize mCpuHotEjectData if PcdCpuMaxLogicalProcessorNumber > 1.
+
+  Also setup the corresponding PcdCpuHotEjectDataAddress.
+**/
+STATIC
+VOID
+InitCpuHotEjectData (
+  VOID
+  )
+{
+  UINTN  ArrayLen;
+  UINTN  BaseLen;
+  UINTN  TotalLen;
+  UINT32 Idx;
+  UINT32 MaxNumberOfCpus;
+  RETURN_STATUS  PcdStatus;
+
+  MaxNumberOfCpus = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+
+  if (MaxNumberOfCpus == 1) {
+return;
+  }
+
+  //
+  // We want the following lay out for CPU_HOT_EJECT_DATA:
+  //  UINTN alignment:  CPU_HOT_EJECT_DATA
+  //  --- padding if needed ---
+  //  UINT64 alignment:  CPU_HOT_EJECT_DATA->QemuSelectorMap[]
+  //
+  // Accordingly, we allocate:
+  //   sizeof(*mCpuHotEjectData) + (MaxNumberOfCpus *
+  // sizeof(mCpuHotEjectData->QemuSelectorMap[0])).
+  // Add sizeof(UINT64) to use as padding if needed.
+  //
+
+  if (RETURN_ERROR (SafeUintnMult (sizeof (*mCpuHotEjectData), 1, )) ||
+  RETURN_ERROR (SafeUintnMult (
+  sizeof (mCpuHotEjectData->QemuSelectorMap[0]),
+  MaxNumberOfCpus, )) ||
+  RETURN_ERROR (SafeUintnAdd (BaseLen, ArrayLen, ))||
+  RETURN_ERROR (SafeUintnAdd (TotalLen, sizeof (UINT64), ))) {
+DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_EJECT_DATA\n", __FUNCTION__));
+goto Fatal;
+  }
+
+  mCpuHotEjectData = AllocatePool (TotalLen);
+  if (mCpuHotEjectData == NULL) {
+ASSERT (mCpuHotEjectData != NULL);
+goto Fatal;
+  }
+
+  mCpuHotEjectData->Handler = NULL;
+  mCpuHotEjectData->ArrayLength = MaxNumberOfCpus;
+
+  mCpuHotEjectData->QemuSelectorMap 

[edk2-devel] [PATCH v8 10/10] OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

2021-02-21 Thread Ankur Arora
Advertise OVMF support for CPU hot-unplug and negotiate it
if QEMU requests the feature.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses the following review comments:
(1,3) s/hot unplug/hot-unplug/
(2) Get rid of the reference to the made up ICH9_APM_CNT_CPU_HOT_UNPLUG
(4,6) Remove the artificial tie in between
 ICH9_LPC_SMI_F_CPU_HOTPLUG, ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.
(5) Fully spell out "SMI on CPU hot-unplug".
(7) Emit separate messages on negotiation (or not) of
 ICH9_LPC_SMI_F_CPU_HOT_UNPLUG.

 OvmfPkg/SmmControl2Dxe/SmiFeatures.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c 
b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
index c9d875543205..b1d59a559dae 100644
--- a/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
+++ b/OvmfPkg/SmmControl2Dxe/SmiFeatures.c
@@ -29,6 +29,12 @@
 //
 #define ICH9_LPC_SMI_F_CPU_HOTPLUG BIT1
 
+// The following bit value stands for "enable CPU hot-unplug, and inject an SMI
+// with control value ICH9_APM_CNT_CPU_HOTPLUG upon hot-unplug", in the
+// "etc/smi/supported-features" and "etc/smi/requested-features" fw_cfg files.
+//
+#define ICH9_LPC_SMI_F_CPU_HOT_UNPLUG BIT2
+
 //
 // Provides a scratch buffer (allocated in EfiReservedMemoryType type memory)
 // for the S3 boot script fragment to write to and read from.
@@ -112,7 +118,8 @@ NegotiateSmiFeatures (
   QemuFwCfgReadBytes (sizeof mSmiFeatures, );
 
   //
-  // We want broadcast SMI, SMI on CPU hotplug, and nothing else.
+  // We want broadcast SMI, SMI on CPU hotplug, SMI on CPU hot-unplug
+  // and nothing else.
   //
   RequestedFeaturesMask = ICH9_LPC_SMI_F_BROADCAST;
   if (!MemEncryptSevIsEnabled ()) {
@@ -120,8 +127,10 @@ NegotiateSmiFeatures (
 // For now, we only support hotplug with SEV disabled.
 //
 RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOTPLUG;
+RequestedFeaturesMask |= ICH9_LPC_SMI_F_CPU_HOT_UNPLUG;
   }
   mSmiFeatures &= RequestedFeaturesMask;
+
   QemuFwCfgSelectItem (mRequestedFeaturesItem);
   QemuFwCfgWriteBytes (sizeof mSmiFeatures, );
 
@@ -166,6 +175,13 @@ NegotiateSmiFeatures (
   __FUNCTION__));
   }
 
+  if ((mSmiFeatures & ICH9_LPC_SMI_F_CPU_HOT_UNPLUG) == 0) {
+DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug not negotiated\n", __FUNCTION__));
+  } else {
+DEBUG ((DEBUG_INFO, "%a: CPU hot-unplug with SMI negotiated\n",
+  __FUNCTION__));
+  }
+
   //
   // Negotiation successful (although we may not have gotten the optimal
   // feature set).
-- 
2.9.3



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




[edk2-devel] [PATCH v8 07/10] OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler

2021-02-21 Thread Ankur Arora
Call the CPU hot-eject handler if one is installed. The condition for
installation is (PcdCpuMaxLogicalProcessorNumber > 1), and there's
a hot-unplug request.

The handler executes in context of SmmCpuFeaturesRendezvousExit(),
which is called at the tail end of SmiRendezvous() after the BSP has
given the signal to exit via the "AllCpusInSync" loop.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Address the following review comments from v6, patch-6:
 (19a) Move the call to the ejection handler to a separate patch.
 (19b) Describe the calling context of SmmCpuFeaturesRendezvousExit().
 (20) Add comment describing the state when the Handler is not armed.

 OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index adbfc90ad46e..99988285b6a2 100644
--- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -467,6 +467,21 @@ SmmCpuFeaturesRendezvousExit (
   IN UINTN  CpuIndex
   )
 {
+  //
+  // We only call the Handler if CPU hot-eject is enabled
+  // (PcdCpuMaxLogicalProcessorNumber > 1), and hot-eject is needed
+  // in this SMI exit (otherwise mCpuHotEjectData->Handler is not armed.)
+  //
+
+  if (mCpuHotEjectData != NULL) {
+CPU_HOT_EJECT_HANDLER Handler;
+
+Handler = mCpuHotEjectData->Handler;
+
+if (Handler != NULL) {
+  Handler (CpuIndex);
+}
+  }
 }
 
 /**
-- 
2.9.3



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




[edk2-devel] [PATCH v8 08/10] OvmfPkg/CpuHotplugSmm: add EjectCpu()

2021-02-21 Thread Ankur Arora
Add EjectCpu(), which handles the CPU ejection, and provides a holding
area for said CPUs. It is called via SmmCpuFeaturesRendezvousExit(),
at the tail end of the SMI handling.

Also UnplugCpus() now stashes QEMU Selectors of CPUs which need to be
ejected in CPU_HOT_EJECT_DATA.QemuSelectorMap. This is used by
EjectCpu() to identify CPUs marked for ejection.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Address these review comments from v6:
 (1) s/CpuEject/EjectCpu/g
 (2) Ensure that the added include is in sorted order.
 (3) Switch to a cheaper CpuSleep() based loop instead of
  CpuDeadLoop().  Also add the CpuLib LibraryClass.
 (4) Remove the nested else clause
 (5) Use Laszlo's much clearer comment when we try to map multiple
  QemuSelector to the same ProcessorNum.
 (6a) Fix indentation of the debug print in the block in (5).
 (6b,6c,6d) Fix printf types for ProcessorNum, use FMT_APIC_ID for
  APIC_ID and 0x%Lx for QemuSelector[].
 () As discussed elsewhere add an DEBUG_INFO print logging the
  correspondence between ProcessorNum, APIC_ID, QemuSelector.
 (7a,7b) Use EFI_ALREADY_STARTED instead of EFI_INVALID_PARAMETER and
  document it in the UnplugCpus() comment block.
 ()  As discussed elsewhere, add the import statement for
  PcdCpuHotEjectDataAddress.
 (9) Use Laszlo's comment in the PcdGet64(PcdCpuHotEjectDataAddress)
  description block.
 (10) Change mCpuHotEjectData init state checks from ASSERT to ones
  consistent with similar checks for mCpuHotPlugData.
 (11-14) Get rid of mCpuHotEjectData init loop: moved to a prior
  patch so it can be done at allocation time.
 (15) s/SmmCpuFeaturesSmiRendezvousExit/SmmCpuFeaturesRendezvousExit/
 (16,17) Document the ordering requirements of
  mCpuHotEjectData->Handler, and mCpuHotEjectData->QemuSelectorMap.

Not addressed:
 (8) Not removing the EjectCount variable as I'd like to minimize
  stores/loads to CPU_HOT_EJECT_DATA->Handler and so would like to do this
  a single time at the end of the iteration.  (It is safe to write multiple
  times to the handler in UnplugCpus() but given the ordering concerns
  around it, it seems cleaner to not access it unnecessarily.)

 OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf |   2 +
 OvmfPkg/CpuHotplugSmm/CpuHotplug.c  | 157 ++--
 2 files changed, 151 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf 
b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
index 04322b0d7855..ebcc7e2ac63a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf
@@ -40,6 +40,7 @@ [Packages]
 [LibraryClasses]
   BaseLib
   BaseMemoryLib
+  CpuLib
   DebugLib
   LocalApicLib
   MmServicesTableLib
@@ -54,6 +55,7 @@ [Protocols]
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress## CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress  ## CONSUMES
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase ## CONSUMES
 
 [FeaturePcd]
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index f07b5072749a..851e2b28aad9 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -10,10 +10,12 @@
 #include  // ICH9_APM_CNT
 #include  // QEMU_CPUHP_CMD_GET_PENDING
 #include  // CpuDeadLoop()
+#include   // CpuSleep()
 #include // ASSERT()
 #include   // gMmst
 #include   // PcdGetBool()
 #include   // SafeUintnSub()
+#include  // CPU_HOT_EJECT_DATA
 #include // EFI_MM_CPU_IO_PROTOCOL
 #include   // EFI_SMM_CPU_SERVICE_PROTOCOL
 #include// EFI_STATUS
@@ -32,11 +34,12 @@ STATIC EFI_MM_CPU_IO_PROTOCOL *mMmCpuIo;
 //
 STATIC EFI_SMM_CPU_SERVICE_PROTOCOL *mMmCpuService;
 //
-// This structure is a communication side-channel between the
+// These structures serve as communication side-channels between the
 // EFI_SMM_CPU_SERVICE_PROTOCOL consumer (i.e., this driver) and provider
 // (i.e., PiSmmCpuDxeSmm).
 //
 STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
+STATIC CPU_HOT_EJECT_DATA *mCpuHotEjectData;
 //
 // SMRAM arrays for fetching the APIC IDs of processors with pending events (of
 // known event types), for the time of just one MMI.
@@ -188,18 +191,72 @@ RevokeNewSlot:
 }
 
 /**
+  CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+  on each CPU at exit from SMM.
+
+  If, the executing CPU is not being ejected, nothing to be done.
+  If, the executing CPU is being ejected, wait in a halted loop
+  until ejected.
+
+  @param[in] ProcessorNum  ProcessorNum denotes the CPU exiting SMM,
+  

[edk2-devel] [PATCH v8 04/10] OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()

2021-02-21 Thread Ankur Arora
Introduce UnplugCpus() which maps each APIC ID being unplugged
onto the hardware ID of the processor and informs PiSmmCpuDxeSmm
of removal by calling EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().

With this change we handle the first phase of unplug where we collect
the CPUs that need to be unplugged and mark them for removal in SMM
data structures.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses these review comments from v6:
 (1) Drop the empty line in the comment block around UnplugCpus().
 (2) Make the "did not find APIC ID" DEBUG_VERBOSE instead of DEBUG_INFO.
 (3) Un-Indented ("Outdented") the line following the comment "Ignore the
  unplug if APIC ID.
 (4) Remove the empty line between Status assignment and check.
 (5) Drop the "goto Fatal" logic and just return Status directly.
 (6) Handle both Plugging and Unplugging of CPUs in one go.
 (7) Also nest the EFI_STATUS check.

 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 84 ++
 1 file changed, 84 insertions(+)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index 3192bfea1f15..f07b5072749a 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -188,6 +188,83 @@ RevokeNewSlot:
 }
 
 /**
+  Process to be hot-unplugged CPUs, per QemuCpuhpCollectApicIds().
+
+  For each such CPU, report the CPU to PiSmmCpuDxeSmm via
+  EFI_SMM_CPU_SERVICE_PROTOCOL. If the to be hot-unplugged CPU is
+  unknown, skip it silently.
+
+  @param[in] ToUnplugApicIdsThe APIC IDs of the CPUs that are about to be
+hot-unplugged.
+
+  @param[in] ToUnplugCount  The number of filled-in APIC IDs in
+ToUnplugApicIds.
+
+  @retval EFI_SUCCESS   Known APIC IDs have been removed from SMM data
+structures.
+
+  @return   Error codes propagated from
+mMmCpuService->RemoveProcessor().
+**/
+STATIC
+EFI_STATUS
+UnplugCpus (
+  IN APIC_ID  *ToUnplugApicIds,
+  IN UINT32   ToUnplugCount
+  )
+{
+  EFI_STATUS Status;
+  UINT32 ToUnplugIdx;
+  UINTN  ProcessorNum;
+
+  ToUnplugIdx = 0;
+  while (ToUnplugIdx < ToUnplugCount) {
+APIC_IDRemoveApicId;
+
+RemoveApicId = ToUnplugApicIds[ToUnplugIdx];
+
+//
+// mCpuHotPlugData->ApicId maps ProcessorNum -> ApicId. Use it to find
+// the ProcessorNum for the APIC ID to be removed.
+//
+for (ProcessorNum = 0;
+ ProcessorNum < mCpuHotPlugData->ArrayLength;
+ ProcessorNum++) {
+  if (mCpuHotPlugData->ApicId[ProcessorNum] == RemoveApicId) {
+break;
+  }
+}
+
+//
+// Ignore the unplug if APIC ID not found
+//
+if (ProcessorNum == mCpuHotPlugData->ArrayLength) {
+  DEBUG ((DEBUG_VERBOSE, "%a: did not find APIC ID " FMT_APIC_ID
+" to unplug\n", __FUNCTION__, RemoveApicId));
+  ToUnplugIdx++;
+  continue;
+}
+
+//
+// Mark ProcessorNum for removal from SMM data structures
+//
+Status = mMmCpuService->RemoveProcessor (mMmCpuService, ProcessorNum);
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: RemoveProcessor(" FMT_APIC_ID "): %r\n",
+__FUNCTION__, RemoveApicId, Status));
+  return Status;
+}
+
+ToUnplugIdx++;
+  }
+
+  //
+  // We've removed this set of APIC IDs from SMM data structures.
+  //
+  return EFI_SUCCESS;
+}
+
+/**
   CPU Hotplug MMI handler function.
 
   This is a root MMI handler.
@@ -309,6 +386,13 @@ CpuHotplugMmi (
 }
   }
 
+  if (ToUnplugCount > 0) {
+Status = UnplugCpus (mToUnplugApicIds, ToUnplugCount);
+if (EFI_ERROR (Status)) {
+  goto Fatal;
+}
+  }
+
   //
   // We've handled this MMI.
   //
-- 
2.9.3



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




[edk2-devel] [PATCH v8 05/10] OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA

2021-02-21 Thread Ankur Arora
Define CPU_HOT_EJECT_DATA and add PCD PcdCpuHotEjectDataAddress, which
will be used to share CPU ejection state between OvmfPkg/CpuHotPlugSmm
and PiSmmCpuDxeSmm.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses the following review comments in v6:
 (1) Dropped modifications to LibraryClasses in OvmfPkg.dec
 (2,3) Cleanup comments around PCD PcdCpuHotEjectDataAddress.
 (4) Move PCD PcdCpuHotEjectDataAddress declaration in CpuHotplugSmm.inf
  to a patch-7 where it actually gets used.
 (5a,5b) Change the comment in the top block to use Laszlo's language.
  Also detail when the PCD would contain a valid value.
 (6) Move Library/CpuHotEjectData.h to Pcd/CpuHotEjectData.h
 (7,15,16) Fixup guard macro to be C namespace compliant. Also fixup the
  comment style near the endif guard.
 (8-10) Rename CPU_HOT_EJECT_FN to a more EDK2 compliant style. Also add
  a comment block and fix spacing.
 () Rename ApicIdMap -> QemuSelectorMap while keeping the type as UINT64.
  Related to a comment in patch-8 ("... add worker to do CPU ejection".)
 (11a,11b) Rename CPU_EJECT_INVALID to CPU_EJECT_QEMU_SELECTOR_INVALID
  and add a comment about it.
 () Remove CPU_EJECT_WORKER based on review comment on a patch 8.
 (12,14) Remove CPU_HOT_EJECT_DATA fields Revision and Reserved.
  Reorder CPU_HOT_EJECT_DATA to minimize internal padding
  and ensure elements are properly aligned.
 (13a,13b) Change CpuIndex->ApicId map to ProcessorNum -> QemuSelector
 () Make CPU_HOT_EJECT_HANDLER->Handler,
  CPU_HOT_EJECT_HANDLER->QemuSelectorMap volatile.

 OvmfPkg/OvmfPkg.dec   |  4 +++
 OvmfPkg/Include/Pcd/CpuHotEjectData.h | 52 +++
 2 files changed, 56 insertions(+)
 create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4348bb45c64a..9629707020ba 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -352,6 +352,10 @@ [PcdsDynamic, PcdsDynamicEx]
   #  This PCD is only accessed if PcdSmmSmramRequire is TRUE (see below).
   gUefiOvmfPkgTokenSpaceGuid.PcdQ35SmramAtDefaultSmbase|FALSE|BOOLEAN|0x34
 
+  ## This PCD adds a communication channel between OVMF's SmmCpuFeaturesLib
+  #  instance in PiSmmCpuDxeSmm, and CpuHotplugSmm.
+  gUefiOvmfPkgTokenSpaceGuid.PcdCpuHotEjectDataAddress|0|UINT64|0x46
+
 [PcdsFeatureFlag]
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderPciTranslation|TRUE|BOOLEAN|0x1c
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuBootOrderMmioTranslation|FALSE|BOOLEAN|0x1d
diff --git a/OvmfPkg/Include/Pcd/CpuHotEjectData.h 
b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
new file mode 100644
index ..024a92726869
--- /dev/null
+++ b/OvmfPkg/Include/Pcd/CpuHotEjectData.h
@@ -0,0 +1,52 @@
+/** @file
+  Definition for the CPU_HOT_EJECT_DATA structure, which shares
+  CPU hot-eject state between OVMF's SmmCpuFeaturesLib instance in
+  PiSmmCpuDxeSmm, and CpuHotplugSmm.
+
+  CPU_HOT_EJECT_DATA is allocated in SMRAM, and pointed-to by
+  PcdCpuHotEjectDataAddress.
+
+  PcdCpuHotEjectDataAddress is valid when SMM_REQUIRE is TRUE
+  and MaxNumberOfCpus > 1.
+
+  Copyright (C) 2021, Oracle Corporation.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef CPU_HOT_EJECT_DATA_H_
+#define CPU_HOT_EJECT_DATA_H_
+
+/**
+  CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit()
+  on each CPU at exit from SMM.
+
+  @param[in] ProcessorNum  ProcessorNum denotes the CPU exiting SMM,
+   and will be used as an index into
+   CPU_HOT_EJECT_DATA->QemuSelectorMap. It is
+   identical to the processor handle in
+   EFI_SMM_CPU_SERVICE_PROTOCOL.
+**/
+typedef
+VOID
+(EFIAPI *CPU_HOT_EJECT_HANDLER) (
+  IN UINTN  ProcessorNum
+  );
+
+//
+// CPU_EJECT_QEMU_SELECTOR_INVALID marks CPUs not being ejected in
+// CPU_HOT_EJECT_DATA->QemuSelectorMap.
+//
+// QEMU CPU Selector is UINT32, so we choose an invalid value larger
+// than that type.
+//
+#define CPU_EJECT_QEMU_SELECTOR_INVALID   (MAX_UINT64)
+
+typedef struct {
+  volatile UINT64   *QemuSelectorMap; // Maps ProcessorNum -> QemuSelector
+  // for pending hot-ejects
+  volatile CPU_HOT_EJECT_HANDLER Handler; // Handler to do the CPU ejection
+  UINT32ArrayLength;  // Entries in the QemuSelectorMap
+} CPU_HOT_EJECT_DATA;
+
+#endif // CPU_HOT_EJECT_DATA_H_
-- 
2.9.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71916): https://edk2.groups.io/g/devel/message/71916
Mute This Topic: https://groups.io/mt/80819860/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 

[edk2-devel] [PATCH v8 02/10] OvmfPkg/CpuHotplugSmm: collect hot-unplug events

2021-02-21 Thread Ankur Arora
Process fw_remove events in QemuCpuhpCollectApicIds() and collect
corresponding APIC IDs for CPUs that are being hot-unplugged.

In addition, we now ignore CPUs which only have remove set. These
CPUs haven't been processed by OSPM yet.

This is based on the QEMU hot-unplug protocol documented here:
  
https://lore.kernel.org/qemu-devel/20201204170939.1815522-3-imamm...@redhat.com/

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Addresses the following review comments from v6:
 (1,4) Move (and also rename) QEMU_CPUHP_STAT_EJECTED to patch 8,
  where we actually use it.
 (2) Downgrade debug mask from DEBUG_INFO to DEBUG_VERBOSE.
 (3a,3b,3c) Keep the CurrentSelector increment operation at
  the tail of the loop.
 () As discussed elsewhere we also need to get the CpuSelector while
  collecting ApicIds in QemuCpuhpCollectApicIds(). This patch adds a
  separate parameter for the CpuSelector values, because that works
  better alongside the hotplug ExtendIds logic.

 OvmfPkg/CpuHotplugSmm/QemuCpuhp.h |  1 +
 OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h |  1 +
 OvmfPkg/CpuHotplugSmm/CpuHotplug.c| 21 +-
 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 84 ---
 4 files changed, 79 insertions(+), 28 deletions(-)

diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h 
b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 8adaa0ad91f0..1e23b150910e 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
@@ -55,6 +55,7 @@ QemuCpuhpCollectApicIds (
   OUT APIC_ID  *PluggedApicIds,
   OUT UINT32   *PluggedCount,
   OUT APIC_ID  *ToUnplugApicIds,
+  OUT UINT32   *ToUnplugSelector,
   OUT UINT32   *ToUnplugCount
   );
 
diff --git a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h 
b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
index a34a6d3fae61..2ec7a107a64d 100644
--- a/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
+++ b/OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h
@@ -34,6 +34,7 @@
 #define QEMU_CPUHP_STAT_ENABLEDBIT0
 #define QEMU_CPUHP_STAT_INSERT BIT1
 #define QEMU_CPUHP_STAT_REMOVE BIT2
+#define QEMU_CPUHP_STAT_FW_REMOVE  BIT4
 
 #define QEMU_CPUHP_RW_CMD_DATA   0x8
 
diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index bf68fcd42914..3192bfea1f15 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -52,6 +52,7 @@ STATIC CPU_HOT_PLUG_DATA *mCpuHotPlugData;
 //
 STATIC APIC_ID *mPluggedApicIds;
 STATIC APIC_ID *mToUnplugApicIds;
+STATIC UINT32  *mToUnplugSelector;
 //
 // Address of the non-SMRAM reserved memory page that contains the Post-SMM Pen
 // for hot-added CPUs.
@@ -289,6 +290,7 @@ CpuHotplugMmi (
  mPluggedApicIds,
  ,
  mToUnplugApicIds,
+ mToUnplugSelector,
  
  );
   if (EFI_ERROR (Status)) {
@@ -333,7 +335,9 @@ CpuHotplugEntry (
   )
 {
   EFI_STATUS Status;
+  UINTN  Len;
   UINTN  Size;
+  UINTN  SizeSel;
 
   //
   // This module should only be included when SMM support is required.
@@ -387,8 +391,9 @@ CpuHotplugEntry (
   //
   // Allocate the data structures that depend on the possible CPU count.
   //
-  if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, )) ||
-  RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Size, ))) {
+  if (RETURN_ERROR (SafeUintnSub (mCpuHotPlugData->ArrayLength, 1, )) ||
+  RETURN_ERROR (SafeUintnMult (sizeof (APIC_ID), Len, ))||
+  RETURN_ERROR (SafeUintnMult (sizeof (UINT32), Len, ))) {
 Status = EFI_ABORTED;
 DEBUG ((DEBUG_ERROR, "%a: invalid CPU_HOT_PLUG_DATA\n", __FUNCTION__));
 goto Fatal;
@@ -405,6 +410,12 @@ CpuHotplugEntry (
 DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
 goto ReleasePluggedApicIds;
   }
+  Status = gMmst->MmAllocatePool (EfiRuntimeServicesData, SizeSel,
+(VOID **));
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: MmAllocatePool(): %r\n", __FUNCTION__, Status));
+goto ReleaseToUnplugApicIds;
+  }
 
   //
   // Allocate the Post-SMM Pen for hot-added CPUs.
@@ -412,7 +423,7 @@ CpuHotplugEntry (
   Status = SmbaseAllocatePostSmmPen (,
  SystemTable->BootServices);
   if (EFI_ERROR (Status)) {
-goto ReleaseToUnplugApicIds;
+goto ReleaseToUnplugSelector;
   }
 
   //
@@ -472,6 +483,10 @@ ReleasePostSmmPen:
   SmbaseReleasePostSmmPen (mPostSmmPenAddress, SystemTable->BootServices);
   mPostSmmPenAddress = 0;
 
+ReleaseToUnplugSelector:
+  gMmst->MmFreePool (mToUnplugSelector);
+  mToUnplugSelector = NULL;
+
 

[edk2-devel] [PATCH v8 03/10] OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper

2021-02-21 Thread Ankur Arora
Add QemuCpuhpWriteCpuStatus() which will be used to update the QEMU
CPU status register. On error, it hangs in a similar fashion as
other helper functions.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
---

Notes:
Address this review comment:
 () Move QemuCpuhpWriteCpuStatus() (declaration and definition) between
  QemuCpuhpWriteCpuSelector() and QemuCpuhpWriteCommand() to match
  the order of the register descriptions in QEMU.

 OvmfPkg/CpuHotplugSmm/QemuCpuhp.h |  6 ++
 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c | 22 ++
 2 files changed, 28 insertions(+)

diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h 
b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
index 1e23b150910e..859412c1a173 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.h
@@ -42,6 +42,12 @@ QemuCpuhpWriteCpuSelector (
   );
 
 VOID
+QemuCpuhpWriteCpuStatus (
+  IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
+  IN UINT8CpuStatus
+  );
+
+VOID
 QemuCpuhpWriteCommand (
   IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
   IN UINT8Command
diff --git a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c 
b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
index 36372a5e6193..9434bb14dd4e 100644
--- a/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
+++ b/OvmfPkg/CpuHotplugSmm/QemuCpuhp.c
@@ -114,6 +114,28 @@ QemuCpuhpWriteCpuSelector (
 }
 
 VOID
+QemuCpuhpWriteCpuStatus (
+  IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
+  IN UINT8CpuStatus
+  )
+{
+  EFI_STATUS Status;
+
+  Status = MmCpuIo->Io.Write (
+ MmCpuIo,
+ MM_IO_UINT8,
+ ICH9_CPU_HOTPLUG_BASE + QEMU_CPUHP_R_CPU_STAT,
+ 1,
+ 
+ );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, Status));
+ASSERT (FALSE);
+CpuDeadLoop ();
+  }
+}
+
+VOID
 QemuCpuhpWriteCommand (
   IN CONST EFI_MM_CPU_IO_PROTOCOL *MmCpuIo,
   IN UINT8Command
-- 
2.9.3



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




[edk2-devel] [PATCH v8 01/10] OvmfPkg/CpuHotplugSmm: refactor hotplug logic

2021-02-21 Thread Ankur Arora
Refactor CpuHotplugMmi() to pull out the CPU hotplug logic into
ProcessHotAddedCpus(). This is in preparation for supporting CPU
hot-unplug.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Ard Biesheuvel 
Cc: Igor Mammedov 
Cc: Boris Ostrovsky 
Cc: Aaron Young 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3132
Signed-off-by: Ankur Arora 
Reviewed-by: Laszlo Ersek 
---

Notes:
Addresses these review comments from v6:
 (1) s/EFI_ERROR(/EFI_ERROR (/
 (2) Remove the empty line in the comment block above
  ProcessHotAddedCpus().
 () Nest the EFI_ERROR handling inside the (PluggedCount > 0) clause.

 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 210 ++---
 1 file changed, 126 insertions(+), 84 deletions(-)

diff --git a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c 
b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
index cfe698ed2b5e..bf68fcd42914 100644
--- a/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
+++ b/OvmfPkg/CpuHotplugSmm/CpuHotplug.c
@@ -62,6 +62,129 @@ STATIC UINT32 mPostSmmPenAddress;
 //
 STATIC EFI_HANDLE mDispatchHandle;
 
+/**
+  Process CPUs that have been hot-added, per QemuCpuhpCollectApicIds().
+
+  For each such CPU, relocate the SMBASE, and report the CPU to PiSmmCpuDxeSmm
+  via EFI_SMM_CPU_SERVICE_PROTOCOL. If the supposedly hot-added CPU is already
+  known, skip it silently.
+
+  @param[in] PluggedApicIdsThe APIC IDs of the CPUs that have been
+   hot-plugged.
+
+  @param[in] PluggedCount  The number of filled-in APIC IDs in
+   PluggedApicIds.
+
+  @retval EFI_SUCCESS  CPUs corresponding to all the APIC IDs are
+   populated.
+
+  @retval EFI_OUT_OF_RESOURCES Out of APIC ID space in "mCpuHotPlugData".
+
+  @return  Error codes propagated from SmbaseRelocate()
+   and mMmCpuService->AddProcessor().
+**/
+STATIC
+EFI_STATUS
+ProcessHotAddedCpus (
+  IN APIC_ID  *PluggedApicIds,
+  IN UINT32   PluggedCount
+  )
+{
+  EFI_STATUS Status;
+  UINT32 PluggedIdx;
+  UINT32 NewSlot;
+
+  //
+  // The Post-SMM Pen need not be reinstalled multiple times within a single
+  // root MMI handling. Even reinstalling once per root MMI is only prudence;
+  // in theory installing the pen in the driver's entry point function should
+  // suffice.
+  //
+  SmbaseReinstallPostSmmPen (mPostSmmPenAddress);
+
+  PluggedIdx = 0;
+  NewSlot = 0;
+  while (PluggedIdx < PluggedCount) {
+APIC_ID NewApicId;
+UINT32  CheckSlot;
+UINTN   NewProcessorNumberByProtocol;
+
+NewApicId = PluggedApicIds[PluggedIdx];
+
+//
+// Check if the supposedly hot-added CPU is already known to us.
+//
+for (CheckSlot = 0;
+ CheckSlot < mCpuHotPlugData->ArrayLength;
+ CheckSlot++) {
+  if (mCpuHotPlugData->ApicId[CheckSlot] == NewApicId) {
+break;
+  }
+}
+if (CheckSlot < mCpuHotPlugData->ArrayLength) {
+  DEBUG ((DEBUG_VERBOSE, "%a: APIC ID " FMT_APIC_ID " was hot-plugged "
+"before; ignoring it\n", __FUNCTION__, NewApicId));
+  PluggedIdx++;
+  continue;
+}
+
+//
+// Find the first empty slot in CPU_HOT_PLUG_DATA.
+//
+while (NewSlot < mCpuHotPlugData->ArrayLength &&
+   mCpuHotPlugData->ApicId[NewSlot] != MAX_UINT64) {
+  NewSlot++;
+}
+if (NewSlot == mCpuHotPlugData->ArrayLength) {
+  DEBUG ((DEBUG_ERROR, "%a: no room for APIC ID " FMT_APIC_ID "\n",
+__FUNCTION__, NewApicId));
+  return EFI_OUT_OF_RESOURCES;
+}
+
+//
+// Store the APIC ID of the new processor to the slot.
+//
+mCpuHotPlugData->ApicId[NewSlot] = NewApicId;
+
+//
+// Relocate the SMBASE of the new CPU.
+//
+Status = SmbaseRelocate (NewApicId, mCpuHotPlugData->SmBase[NewSlot],
+   mPostSmmPenAddress);
+if (EFI_ERROR (Status)) {
+  goto RevokeNewSlot;
+}
+
+//
+// Add the new CPU with EFI_SMM_CPU_SERVICE_PROTOCOL.
+//
+Status = mMmCpuService->AddProcessor (mMmCpuService, NewApicId,
+  );
+if (EFI_ERROR (Status)) {
+  DEBUG ((DEBUG_ERROR, "%a: AddProcessor(" FMT_APIC_ID "): %r\n",
+__FUNCTION__, NewApicId, Status));
+  goto RevokeNewSlot;
+}
+
+DEBUG ((DEBUG_INFO, "%a: hot-added APIC ID " FMT_APIC_ID ", SMBASE 0x%Lx, "
+  "EFI_SMM_CPU_SERVICE_PROTOCOL assigned number %Lu\n", __FUNCTION__,
+  NewApicId, (UINT64)mCpuHotPlugData->SmBase[NewSlot],
+  (UINT64)NewProcessorNumberByProtocol));
+
+NewSlot++;
+PluggedIdx++;
+  }
+
+  //
+  // We've processed this batch of hot-added CPUs.
+  //
+  return EFI_SUCCESS;
+
+RevokeNewSlot:
+  mCpuHotPlugData->ApicId[NewSlot] = MAX_UINT64;
+
+  return Status;
+}
 
 /**
   CPU Hotplug MMI handler function.
@@ -122,8 +245,6 @@ CpuHotplugMmi (
   UINT8  ApmControl;
   UINT32 PluggedCount;
   UINT32 ToUnplugCount;
-  UINT32 

[edk2-devel] [PATCH v8 00/10] support CPU hot-unplug

2021-02-21 Thread Ankur Arora
Hi,

[ Note, v8 is substantially same as v7, but fixes a couple of CI errors. ]

This series adds OVMF support for CPU hot-unplug.

QEMU secureboot hot-unplug logic corresponding to this is in upstream.
Also posted here:
  
https://lore.kernel.org/qemu-devel/20201207140739.3829993-1-imamm...@redhat.com/

Testing (with QEMU 5.2.50):
 - Stable with randomized CPU plug/unplug (guest maxcpus=33,128)
 - Synthetic tests with simultaneous multi CPU hot-unplug

Also at:
  github.com/terminus/edk2/ hot-unplug-v8

Changelog:

v8:
  - Fixes a couple of ECC issues in the code (in patches 7, 9)

v7:
  - Address review comments from v6.
  - Fix ejection bug where we were using APIC ID to do the ejection
rather than the Qemu Selector.
  - Describes safety properties and ordering needed for concurrent
accesses to CPU_HOT_EJECT_DATA->QemuSelectorMap, and
CPU_HOT_EJECT_DATA->Handler.
  URL: 
https://patchew.org/EDK2/20210219090444.1332380-1-ankur.a.ar...@oracle.com/

v6:
  - addresses v5 review comments.
  URL: 
https://patchew.org/EDK2/20210129005950.467638-1-ankur.a.ar...@oracle.com/

v5:
  - fixes ECC errors (all but one in "OvmfPkg/CpuHotplugSmm: add
add Qemu Cpu Status helper").
  URL: 
https://patchew.org/EDK2/20210126064440.299596-1-ankur.a.ar...@oracle.com/

v4:
  - Gets rid of unnecessary UefiCpuPkg changes
  URL: 
https://patchew.org/EDK2/20210118063457.358581-1-ankur.a.ar...@oracle.com/

v3:
  - Use a saner PCD based interface to share state between PiSmmCpuDxeSmm
and OvmfPkg/CpuHotplugSmm
  - Cleaner split of the hot-unplug code
  URL: 
https://patchew.org/EDK2/20210115074533.277448-1-ankur.a.ar...@oracle.com/

v2:
  - Do the ejection via SmmCpuFeaturesRendezvousExit()
  URL: 
https://patchew.org/EDK2/20210107195515.106158-1-ankur.a.ar...@oracle.com/

RFC:
  URL: 
https://patchew.org/EDK2/20201208053432.2690694-1-ankur.a.ar...@oracle.com/

Please review.

Thanks
Ankur

Ankur Arora (10):
  OvmfPkg/CpuHotplugSmm: refactor hotplug logic
  OvmfPkg/CpuHotplugSmm: collect hot-unplug events
  OvmfPkg/CpuHotplugSmm: add Qemu Cpu Status helper
  OvmfPkg/CpuHotplugSmm: introduce UnplugCpus()
  OvmfPkg/CpuHotplugSmm: define CPU_HOT_EJECT_DATA
  OvmfPkg/SmmCpuFeaturesLib: init CPU ejection state
  OvmfPkg/SmmCpuFeaturesLib: call CPU hot-eject handler
  OvmfPkg/CpuHotplugSmm: add EjectCpu()
  OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject
  OvmfPkg/SmmControl2Dxe: negotiate CPU hot-unplug

 OvmfPkg/OvmfPkg.dec|   4 +
 OvmfPkg/CpuHotplugSmm/CpuHotplugSmm.inf|   2 +
 .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|   4 +
 OvmfPkg/CpuHotplugSmm/QemuCpuhp.h  |   7 +
 OvmfPkg/Include/IndustryStandard/QemuCpuHotplug.h  |   2 +
 OvmfPkg/Include/Pcd/CpuHotEjectData.h  |  52 ++
 OvmfPkg/CpuHotplugSmm/CpuHotplug.c | 583 +
 OvmfPkg/CpuHotplugSmm/QemuCpuhp.c  | 106 +++-
 .../Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  | 138 +
 OvmfPkg/SmmControl2Dxe/SmiFeatures.c   |  18 +-
 10 files changed, 796 insertions(+), 120 deletions(-)
 create mode 100644 OvmfPkg/Include/Pcd/CpuHotEjectData.h

-- 
2.9.3



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




[edk2-devel] MP Services Protocol : Support Hyper threading or not

2021-02-21 Thread Tiger Liu(BJ-RD)
Dear All:
I have a question about MP Services Protocol.
Does it support Hyper-threading technology?

For example:
A Quad-cores CPU(with HT feature support), actually it is equal to 8 logical 
cores.
Take assumption 8 logical cores are all healthy and being enabled.

So:

1.  Will EFI_MP_SERVICES_PROTOCOL.GetNumberOfProcessors()return total 8 
logical processor cores?

2.  Will EFI_MP_SERVICES_PROTOCOL.StartupAllAPs() make all 8 logical cores 
do some tasks simultaneously?

Thanks


?
?
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.


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




Re: [edk2-devel] [PATCH - resend] MdeModulePkg/BootLogoLib: Center logo 38.2% from top of screen

2021-02-21 Thread Wu, Hao A
> -Original Message-
> From: Patrick Rudolph 
> Sent: Thursday, February 18, 2021 8:43 PM
> To: gaoliming 
> Cc: devel@edk2.groups.io; tcrawf...@system76.com; Wang, Jian J
> ; Wu, Hao A ; Gao, Zhichao
> ; Ni, Ray 
> Subject: Re: [edk2-devel] [PATCH - resend] MdeModulePkg/BootLogoLib:
> Center logo 38.2% from top of screen
> 
> Hi,
> Please find the issue created here:
> https://bugzilla.tianocore.org/show_bug.cgi?id=3226


Hello Patrick,

Could you help to send an updated version of the patch to include the above BZ 
tracker information in the commit log message?
Thanks in advance.

Best Regards,
Hao Wu


> 
> On Thu, Feb 18, 2021 at 4:32 AM gaoliming 
> wrote:
> >
> > Patrick:
> >   I am OK for this extension to meet with Microsoft recommendation.
> > This change is a new feature. Can you submit one BZ
> > (https://bugzilla.tianocore.org/) for it?
> >
> > Thanks
> > Liming
> > > -邮件原件-
> > > 发件人: bounce+27952+71716+4905953+8761...@groups.io
> > >  代表 Patrick
> Rudolph
> > > 发送时间: 2021年2月17日 18:11
> > > 收件人: devel@edk2.groups.io
> > > 抄送: tcrawf...@system76.com; jian.j.w...@intel.com;
> > > hao.a...@intel.com; zhichao@intel.com; ray...@intel.com
> > > 主题: [edk2-devel] [PATCH - resend] MdeModulePkg/BootLogoLib:
> Center
> > > logo 38.2% from top of screen
> > >
> > > From: Tim Crawford 
> > >
> > > Use Microsoft's recommended positioning [1] for the boot logo.
> > >
> > > > We recommend that the logo is placed with its center at 38.2% from
> > > > the screen's top edge. This positioning is based on the golden
> > > > ratio's visual aesthetics and matches the Windows 10 design
> proportions.
> > >
> > > [1]:
> > > https://docs.microsoft.com/en-us/windows-
> hardware/drivers/bringup/bo
> > > ot-s creen-components#position-the-logo-during-post
> > >
> > > Based on Tim Crawford  initial commit.
> > >
> > > Signed-off-by: Patrick Rudolph 
> > > ---
> > >  MdeModulePkg/Include/Protocol/PlatformLogo.h   | 3 ++-
> > >  MdeModulePkg/Library/BootLogoLib/BootLogoLib.c | 4 
> > >  MdeModulePkg/Logo/Logo.c   | 2 +-
> > >  3 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> > > b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> > > index 55c9e08696..21a4c79e1d 100644
> > > --- a/MdeModulePkg/Include/Protocol/PlatformLogo.h
> > > +++ b/MdeModulePkg/Include/Protocol/PlatformLogo.h
> > > @@ -29,7 +29,8 @@ typedef enum {
> > >EdkiiPlatformLogoDisplayAttributeCenterBottom,
> > >
> > >EdkiiPlatformLogoDisplayAttributeLeftBottom,
> > >
> > >EdkiiPlatformLogoDisplayAttributeCenterLeft,
> > >
> > > -  EdkiiPlatformLogoDisplayAttributeCenter
> > >
> > > +  EdkiiPlatformLogoDisplayAttributeCenter,
> > >
> > > +  EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended
> > >
> > >  } EDKII_PLATFORM_LOGO_DISPLAY_ATTRIBUTE;
> > >
> > >
> > >
> > >  /**
> > >
> > > diff --git a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> > > b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> > > index 134660f28d..d40c65b59f 100644
> > > --- a/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> > > +++ b/MdeModulePkg/Library/BootLogoLib/BootLogoLib.c
> > > @@ -173,6 +173,10 @@ BootLogoEnableLogo (
> > >DestX = 0;
> > >
> > >DestY = (SizeOfY - Image.Height) / 2;
> > >
> > >break;
> > >
> > > +case EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended:
> > >
> > > +  DestX = (SizeOfX - Image.Width) / 2;
> > >
> > > +  DestY = (SizeOfY * 382) / 1000 - Image.Height / 2;
> > >
> > > +  break;
> > >
> > >  case EdkiiPlatformLogoDisplayAttributeCenter:
> > >
> > >DestX = (SizeOfX - Image.Width) / 2;
> > >
> > >DestY = (SizeOfY - Image.Height) / 2;
> > >
> > > diff --git a/MdeModulePkg/Logo/Logo.c b/MdeModulePkg/Logo/Logo.c
> > > index c647253ecd..131a1b456a 100644
> > > --- a/MdeModulePkg/Logo/Logo.c
> > > +++ b/MdeModulePkg/Logo/Logo.c
> > > @@ -26,7 +26,7 @@ EFI_HII_HANDLEmHiiHandle;
> > >  LOGO_ENTRYmLogos[] = {
> > >
> > >{
> > >
> > >  IMAGE_TOKEN (IMG_LOGO),
> > >
> > > -EdkiiPlatformLogoDisplayAttributeCenter,
> > >
> > > +EdkiiPlatformLogoDisplayAttributeMicrosoftRecommended,
> > >
> > >  0,
> > >
> > >  0
> > >
> > >}
> > >
> > > --
> > > 2.26.2
> > >
> > >
> > >
> > > -=-=-=-=-=-=
> > > Groups.io Links: You receive all messages sent to this group.
> > > View/Reply Online (#71716):
> > > https://edk2.groups.io/g/devel/message/71716
> > > Mute This Topic: https://groups.io/mt/80700289/4905953
> > > Group Owner: devel+ow...@edk2.groups.io
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [gaolim...@byosoft.com.cn]
> > > -=-=-=-=-=-=
> > >
> >
> >
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71909): https://edk2.groups.io/g/devel/message/71909
Mute This Topic: https://groups.io/mt/80727906/21656
Group Owner: devel+ow...@edk2.groups.io

Re: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET instructions to Nasm.inc

2021-02-21 Thread Zhiguang Liu
Hi Sheng Wei,
I don't have the access to push patch.

Hi Liming,
Can you help merge this patch? Thanks

Thanks
Zhiguang

> -Original Message-
> From: Sheng, W 
> Sent: Monday, February 22, 2021 10:12 AM
> To: gaoliming ; devel@edk2.groups.io; Liu,
> Zhiguang 
> Cc: Kinney, Michael D ; Yao, Jiewen
> 
> Subject: RE: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET
> instructions to Nasm.inc
> 
> Hi Zhiguang,
> Could you help to merge the patch to the master branch ?
> 
> @gaoliming
> Thank you for giving the review-by.
> BR
> Sheng Wei
> 
> > -Original Message-
> > From: gaoliming 
> > Sent: 2021年2月20日 13:35
> > To: devel@edk2.groups.io; Sheng, W 
> > Cc: Kinney, Michael D ; Liu, Zhiguang
> > ; Yao, Jiewen 
> > Subject: 回复: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET
> > instructions to Nasm.inc
> >
> > Reviewed-by: Liming Gao 
> >
> > > -邮件原件-
> > > 发件人: bounce+27952+71865+4905953+8761...@groups.io
> > >  代表 Sheng Wei
> > > 发送时间: 2021年2月20日 11:15
> > > 收件人: devel@edk2.groups.io
> > > 抄送: Michael D Kinney ; Liming Gao
> > > ; Zhiguang Liu ;
> > > Jiewen Yao 
> > > 主题: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET instructions
> > > to Nasm.inc
> > >
> > > This is to add instruction SAVEPREVSSP, CLRSSBSY and RSTORSSP_RAX in
> > > Nasm.
> > > The open CI is using NASM 2.14.02.
> > > CET instructions are supported since NASM 2.15.01.
> > >
> > > DB-encoded CET instructions need to be removed after open CI update
> > > to NASM 2.15.01.
> > > The BZ ticket is https://bugzilla.tianocore.org/show_bug.cgi?id=3227 .
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3192
> > >
> > > Signed-off-by: Sheng Wei 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Cc: Zhiguang Liu 
> > > Cc: Jiewen Yao 
> > > ---
> > >  MdePkg/Include/Ia32/Nasm.inc | 12 
> > > MdePkg/Include/X64/Nasm.inc  | 12 
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/MdePkg/Include/Ia32/Nasm.inc
> > > b/MdePkg/Include/Ia32/Nasm.inc index 31ce861f1e..fa42f9d3e9 100644
> > > --- a/MdePkg/Include/Ia32/Nasm.inc
> > > +++ b/MdePkg/Include/Ia32/Nasm.inc
> > > @@ -9,6 +9,18 @@
> > >  ;
> > >
> > ;-
> > --
> > ---
> > >
> > > +%macro SAVEPREVSSP 0
> > > +DB 0xF3, 0x0F, 0x01, 0xEA
> > > +%endmacro
> > > +
> > > +%macro CLRSSBSY_EAX0
> > > +DB 0x67, 0xF3, 0x0F, 0xAE, 0x30 %endmacro
> > > +
> > > +%macro RSTORSSP_EAX0
> > > +DB 0x67, 0xF3, 0x0F, 0x01, 0x28 %endmacro
> > > +
> > >  %macro SETSSBSY0
> > >  DB 0xF3, 0x0F, 0x01, 0xE8
> > >  %endmacro
> > > diff --git a/MdePkg/Include/X64/Nasm.inc
> > b/MdePkg/Include/X64/Nasm.inc
> > > index 42412735ea..e57a803c81 100644
> > > --- a/MdePkg/Include/X64/Nasm.inc
> > > +++ b/MdePkg/Include/X64/Nasm.inc
> > > @@ -9,6 +9,18 @@
> > >  ;
> > >
> > ;-
> > --
> > ---
> > >
> > > +%macro SAVEPREVSSP 0
> > > +DB 0xF3, 0x0F, 0x01, 0xEA
> > > +%endmacro
> > > +
> > > +%macro CLRSSBSY_RAX0
> > > +DB 0xF3, 0x0F, 0xAE, 0x30
> > > +%endmacro
> > > +
> > > +%macro RSTORSSP_RAX0
> > > +DB 0xF3, 0x0F, 0x01, 0x28
> > > +%endmacro
> > > +
> > >  %macro SETSSBSY0
> > >  DB 0xF3, 0x0F, 0x01, 0xE8
> > >  %endmacro
> > > --
> > > 2.16.2.windows.1
> > >
> > >
> > >
> > > 
> > >
> >
> >



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




Re: [edk2-devel] [PATCH v5 2/2] UefiCpuPkg/CpuExceptionHandlerLib: Clear CET shadow stack token busy bit

2021-02-21 Thread Sheng Wei
Hi Jiewen,
Thank you for review the patch.
Could you give review-by on this patch?
Thank you.
BR
Sheng Wei


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Sheng
> Wei
> Sent: 2021年2月20日 11:15
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray ; Laszlo
> Ersek ; Kumar, Rahul1 ; Yao,
> Jiewen ; Feng, Roger 
> Subject: [edk2-devel] [PATCH v5 2/2] UefiCpuPkg/CpuExceptionHandlerLib:
> Clear CET shadow stack token busy bit
> 
> If CET shadows stack feature enabled in SMM and stack switch is enabled.
> When code execute from SMM handler to SMM exception, CPU will check
> SMM exception shadow stack token busy bit if it is cleared or not.
> If it is set, it will trigger #DF exception.
> If it is not set, CPU will set the busy bit when enter SMM exception.
> So, the busy bit should be cleared when return back form SMM exception to
> SMM handler. Otherwise, keeping busy bit 1 will cause to trigger #DF
> exception when enter SMM exception next time.
> So, we use instruction SAVEPREVSSP, CLRSSBSY and RSTORSSP to clear the
> shadow stack token busy bit before RETF instruction in SMM exception.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3192
> 
> Signed-off-by: Sheng Wei 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> Cc: Jiewen Yao 
> Cc: Roger Feng 
> ---
>  .../DxeCpuExceptionHandlerLib.inf  |  3 ++
>  .../PeiCpuExceptionHandlerLib.inf  |  3 ++
>  .../SecPeiCpuExceptionHandlerLib.inf   |  4 ++
>  .../SmmCpuExceptionHandlerLib.inf  |  3 ++
>  .../X64/Xcode5ExceptionHandlerAsm.nasm | 46
> +-
>  .../Xcode5SecPeiCpuExceptionHandlerLib.inf |  4 ++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c   | 15 ++-
>  7 files changed, 75 insertions(+), 3 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.
> inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib
> .inf
> index 07b34c92a8..e7a81bebdb 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.
> inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLi
> +++ b.inf
> @@ -43,6 +43,9 @@
>gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
>gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
> 
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard##
> CONSUMES
> +
>  [Packages]
>MdePkg/MdePkg.dec
>MdeModulePkg/MdeModulePkg.dec
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> index feae7b3e06..cf5bfe4083 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLi
> +++ b.inf
> @@ -57,3 +57,6 @@
>  [Pcd]
>gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard# CONSUMES
> 
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard##
> CONSUMES
> +
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> index 967cb61ba6..8ae4feae62 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandle
> +++ rLib.inf
> @@ -49,3 +49,7 @@
>LocalApicLib
>PeCoffGetEntryPointLib
>VmgExitLib
> +
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard##
> CONSUMES
> +
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> index ea5b10b5c8..c9f20da058 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> +++ b.inf
> @@ -53,3 +53,6 @@
>DebugLib
>VmgExitLib
> 
> +[FeaturePcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard##
> CONSUMES
> +
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> index 26cae56cc5..ebe0eec874 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandl
> +++ erAsm.nasm
> @@ -13,6 +13,7 @@
>  ; Notes:
>  ;
>  
> ;--
> +%include "Nasm.inc"
> 
>  ;
>  ; CommonExceptionHandler()
> @@ -23,6 +24,7 @@
>  extern ASM_PFX(mErrorCodeFlag); Error code flags for exceptions
>  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag  

Re: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET instructions to Nasm.inc

2021-02-21 Thread Sheng Wei
Hi Zhiguang,
Could you help to merge the patch to the master branch ?

@gaoliming
Thank you for giving the review-by.
BR
Sheng Wei

> -Original Message-
> From: gaoliming 
> Sent: 2021年2月20日 13:35
> To: devel@edk2.groups.io; Sheng, W 
> Cc: Kinney, Michael D ; Liu, Zhiguang
> ; Yao, Jiewen 
> Subject: 回复: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET
> instructions to Nasm.inc
> 
> Reviewed-by: Liming Gao 
> 
> > -邮件原件-
> > 发件人: bounce+27952+71865+4905953+8761...@groups.io
> >  代表 Sheng Wei
> > 发送时间: 2021年2月20日 11:15
> > 收件人: devel@edk2.groups.io
> > 抄送: Michael D Kinney ; Liming Gao
> > ; Zhiguang Liu ;
> > Jiewen Yao 
> > 主题: [edk2-devel] [PATCH v5 1/2] MdePkg/Include: Add CET instructions
> > to Nasm.inc
> >
> > This is to add instruction SAVEPREVSSP, CLRSSBSY and RSTORSSP_RAX in
> > Nasm.
> > The open CI is using NASM 2.14.02.
> > CET instructions are supported since NASM 2.15.01.
> >
> > DB-encoded CET instructions need to be removed after open CI update to
> > NASM 2.15.01.
> > The BZ ticket is https://bugzilla.tianocore.org/show_bug.cgi?id=3227 .
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3192
> >
> > Signed-off-by: Sheng Wei 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Zhiguang Liu 
> > Cc: Jiewen Yao 
> > ---
> >  MdePkg/Include/Ia32/Nasm.inc | 12 
> > MdePkg/Include/X64/Nasm.inc  | 12 
> >  2 files changed, 24 insertions(+)
> >
> > diff --git a/MdePkg/Include/Ia32/Nasm.inc
> > b/MdePkg/Include/Ia32/Nasm.inc index 31ce861f1e..fa42f9d3e9 100644
> > --- a/MdePkg/Include/Ia32/Nasm.inc
> > +++ b/MdePkg/Include/Ia32/Nasm.inc
> > @@ -9,6 +9,18 @@
> >  ;
> >
> ;---
> ---
> >
> > +%macro SAVEPREVSSP 0
> > +DB 0xF3, 0x0F, 0x01, 0xEA
> > +%endmacro
> > +
> > +%macro CLRSSBSY_EAX0
> > +DB 0x67, 0xF3, 0x0F, 0xAE, 0x30
> > +%endmacro
> > +
> > +%macro RSTORSSP_EAX0
> > +DB 0x67, 0xF3, 0x0F, 0x01, 0x28
> > +%endmacro
> > +
> >  %macro SETSSBSY0
> >  DB 0xF3, 0x0F, 0x01, 0xE8
> >  %endmacro
> > diff --git a/MdePkg/Include/X64/Nasm.inc
> b/MdePkg/Include/X64/Nasm.inc
> > index 42412735ea..e57a803c81 100644
> > --- a/MdePkg/Include/X64/Nasm.inc
> > +++ b/MdePkg/Include/X64/Nasm.inc
> > @@ -9,6 +9,18 @@
> >  ;
> >
> ;---
> ---
> >
> > +%macro SAVEPREVSSP 0
> > +DB 0xF3, 0x0F, 0x01, 0xEA
> > +%endmacro
> > +
> > +%macro CLRSSBSY_RAX0
> > +DB 0xF3, 0x0F, 0xAE, 0x30
> > +%endmacro
> > +
> > +%macro RSTORSSP_RAX0
> > +DB 0xF3, 0x0F, 0x01, 0x28
> > +%endmacro
> > +
> >  %macro SETSSBSY0
> >  DB 0xF3, 0x0F, 0x01, 0xE8
> >  %endmacro
> > --
> > 2.16.2.windows.1
> >
> >
> >
> > 
> >
> 
> 



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




0001-MdePkg-Include-Add-CET-instructions-to-Nasm.inc.patch
Description: 0001-MdePkg-Include-Add-CET-instructions-to-Nasm.inc.patch


Re: [edk2-devel] [PATCH v2 0/2] TigerlakeOpenBoard: Fix build errors with GCC5

2021-02-21 Thread Heng Luo
Thanks Takuto Naito!

For the series..
Reviewed-by: Heng Luo 

> -Original Message-
> From: Takuto Naito 
> Sent: Sunday, February 21, 2021 11:10 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V ; Desimone,
> Nathaniel L ; Luo, Heng 
> Subject: [PATCH v2 0/2] TigerlakeOpenBoard: Fix build errors with GCC5
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224
> TigerlakeOpenBoard: Fix build errors with GCC5
> 
> v2:
> - Split the v1 patch into 2 patches.
>   One is for Platform/Intel/TigerlakeOpenBoardPkg,
>   another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.
> 
> https://github.com/naitaku/edk2-platforms/tree/tigerlake_fix_build_error_v2
> 
> Cc: Sai Chaganty 
> Cc: Nate DeSimone 
> Cc: Heng Luo 
> 
> Takuto Naito (2):
>   TigerlakeOpenBoardPkg: Fix build errors with GCC5
>   TigerlakeSiliconPkg/IpBlock: Fix build errors with GCC5
> 
>  .../PeiFspPolicyInitLib.inf   |   2 +-
>  .../BasePlatformHookLib/BasePlatformHookLib.c | 188 --
>  .../DxeSiliconPolicyUpdateLate.c  |   2 +-
>  .../DxePchPcieRpPolicyLib.c   |   2 +-
>  4 files changed, 3 insertions(+), 191 deletions(-)
> 
> --
> 2.30.1



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




Re: [edk2-devel] [PATCH v2 1/2] TigerlakeOpenBoardPkg: Fix build errors with GCC5

2021-02-21 Thread Heng Luo
Reviewed-by: Heng Luo 

> -Original Message-
> From: Takuto Naito 
> Sent: Sunday, February 21, 2021 11:10 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V ; Desimone,
> Nathaniel L ; Luo, Heng 
> Subject: [PATCH v2 1/2] TigerlakeOpenBoardPkg: Fix build errors with GCC5
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224
> 
> - Fix the path of TigerLakeFspBinPkg
> - Fix misuse of RETURN_ERROR
> - Remove unused function CheckNationalSio.
> 
> Cc: Sai Chaganty 
> Cc: Nate DeSimone 
> Cc: Heng Luo 
> Signed-off-by: Takuto Naito 
> ---
> 
> Notes:
> v2:
> - Split the v1 patch into 2 patches,
>   One is for Platform/Intel/TigerlakeOpenBoardPkg,
>   another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.
> 
>  .../PeiFspPolicyInitLib.inf   |   2 +-
>  .../BasePlatformHookLib/BasePlatformHookLib.c | 188 --
>  .../DxeSiliconPolicyUpdateLate.c  |   2 +-
>  3 files changed, 2 insertions(+), 190 deletions(-)
> 
> diff --git
> a/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLi
> b/PeiFspPolicyInitLib.inf
> b/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLi
> b/PeiFspPolicyInitLib.inf
> index 9d85d855f5..708fbac08f 100644
> ---
> a/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLi
> b/PeiFspPolicyInitLib.inf
> +++
> b/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLi
> b/PeiFspPolicyInitLib.inf
> @@ -52,7 +52,7 @@
>MdeModulePkg/MdeModulePkg.dec
>IntelFsp2Pkg/IntelFsp2Pkg.dec
>TigerlakeSiliconPkg/SiPkg.dec
> -  TigerLakeFspBinPkg/TigerLakeFspBinPkg.dec
> +  TigerLakeFspBinPkg/Client/TigerLakeFspBinPkg.dec
>TigerlakeOpenBoardPkg/OpenBoardPkg.dec
>UefiCpuPkg/UefiCpuPkg.dec
>IntelSiliconPkg/IntelSiliconPkg.dec
> diff --git
> a/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePl
> atformHookLib.c
> b/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePl
> atformHookLib.c
> index 6209e50450..cc5337698b 100644
> ---
> a/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePl
> atformHookLib.c
> +++
> b/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePl
> atformHookLib.c
> @@ -94,194 +94,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_SIO_TABLE
> mSioTableWinbond_x374[] = {
>{0x30, 0x01}// Enable it with Activation bit
>  };
> 
> -/**
> -  Detect if a National 393 SIO is docked. If yes, enable the docked SIO
> -  and its serial port, and disable the onboard serial port.
> -
> -  @retval EFI_SUCCESS Operations performed successfully.
> -**/
> -STATIC
> -VOID
> -CheckNationalSio (
> -  VOID
> -  )
> -{
> -  UINT8   Data8;
> -
> -  //
> -  // Pc87393 access is through either (0x2e, 0x2f) or (0x4e, 0x4f).
> -  // We use (0x2e, 0x2f) which is determined by BADD default strapping
> -  //
> -
> -  //
> -  // Read the Pc87393 signature
> -  //
> -  IoWrite8 (0x2e, 0x20);
> -  Data8 = IoRead8 (0x2f);
> -
> -  if (Data8 == 0xea) {
> -//
> -// Signature matches - National PC87393 SIO is docked
> -//
> -
> -//
> -// Enlarge the LPC decode scope to accommodate the Docking LPC Switch
> -// Register (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS is allocated at
> -// SIO_BASE_ADDRESS + 0x10)
> -//
> -PchLpcGenIoRangeSet ((FixedPcdGet16 (PcdSioBaseAddress) &
> (UINT16)~0x7F), 0x20);
> -
> -//
> -// Enable port switch
> -//
> -IoWrite8 (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS, 0x06);
> -
> -//
> -// Turn on docking power
> -//
> -IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0x8c);
> -
> -IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0x9c);
> -
> -IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0xBc);
> -
> -//
> -// Enable port switch
> -//
> -IoWrite8 (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS, 0x7);
> -
> -//
> -// GPIO setting
> -//
> -IoWrite8 (0x2e, 0x24);
> -IoWrite8 (0x2f, 0x29);
> -
> -//
> -// Enable chip clock
> -//
> -IoWrite8 (0x2e, 0x29);
> -IoWrite8 (0x2f, 0x1e);
> -
> -
> -//
> -// Enable serial port
> -//
> -
> -//
> -// Select com1
> -//
> -IoWrite8 (0x2e, 0x7);
> -IoWrite8 (0x2f, 0x3);
> -
> -//
> -// Base address: 0x3f8
> -//
> -IoWrite8 (0x2e, 0x60);
> -IoWrite8 (0x2f, 0x03);
> -IoWrite8 (0x2e, 0x61);
> -IoWrite8 (0x2f, 0xf8);
> -
> -//
> -// Interrupt: 4
> -//
> -IoWrite8 (0x2e, 0x70);
> -IoWrite8 (0x2f, 0x04);
> -
> -//
> -// Enable bank selection
> -//
> -IoWrite8 (0x2e, 0xf0);
> -IoWrite8 (0x2f, 0x82);
> -
> -//
> -// Activate
> -//
> -IoWrite8 (0x2e, 0x30);
> -IoWrite8 (0x2f, 0x01);
> -
> -//
> -// Disable onboard serial port
> -//
> -IoWrite8 (FixedPcdGet16 (PcdLpcSioConfigDefaultPort), 0x55);
> -
> -//
> -// Power 

Re: [edk2-devel] [PATCH v2 2/2] TigerlakeSiliconPkg/IpBlock: Fix build errors with GCC5

2021-02-21 Thread Heng Luo
Reviewed-by: Heng Luo 

> -Original Message-
> From: Takuto Naito 
> Sent: Sunday, February 21, 2021 11:10 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V ; Desimone,
> Nathaniel L ; Luo, Heng 
> Subject: [PATCH v2 2/2] TigerlakeSiliconPkg/IpBlock: Fix build errors with 
> GCC5
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224
> 
> - Fix the Teton Glacier Endpoint entry in mPciDeviceTable
> 
> Cc: Sai Chaganty 
> Cc: Nate DeSimone 
> Cc: Heng Luo 
> Signed-off-by: Takuto Naito 
> ---
> 
> Notes:
> v2:
> - Split the v1 patch into 2 patches,
>   One is for Platform/Intel/TigerlakeOpenBoardPkg,
>   another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.
> 
>  .../DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c   | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRp
> PolicyLib/DxePchPcieRpPolicyLib.c
> b/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRp
> PolicyLib/DxePchPcieRpPolicyLib.c
> index 577e436e32..1553d2e2aa 100644
> ---
> a/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRp
> PolicyLib/DxePchPcieRpPolicyLib.c
> +++
> b/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRp
> PolicyLib/DxePchPcieRpPolicyLib.c
> @@ -98,7 +98,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> PCH_PCIE_DEVICE_OVERRIDE mPcieDeviceTable[] = {
>//
>// Teton Glacier Endpoint
>//
> -  { 0x8086, 0x0975, 0xff, 0, 0, 0, PchPcieL1SubstatesOverride, 0, 0xff, 
> 0x3C, 0, 5,
> 0, 0, 0, 0 },
> +  { 0x8086, 0x0975, 0xff, 0, 0, 0, PchPcieL1SubstatesOverride, 0, 0xff, 
> 0x3C, 0, 5,
> 0, 0 },
> 
>//
>// End of Table
> --
> 2.30.1



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




Re: [edk2-devel] [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

2021-02-21 Thread Rebecca Cran

On 2/21/21 2:21 PM, Leif Lindholm wrote:

On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:

-UINT8
-GetChassisType (
-  VOID
-  )
-{
-  EFI_STATUS  Status;
-  UINT8   ChassisType;
-
-  Status = OemGetChassisType ();
-  if (EFI_ERROR (Status)) {
-return 0;
-  }
-
-  return ChassisType;
-}
-


This function is outright deleted, not renamed.
Does this belong in another patch?


I think it belongs here: it was just a helper function for when 
OemGetChassisType returned EFI_STATUS. Since it now returns the same 
data as this function, we don't need it anymore.


--
Rebecca Cran


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




Re: [edk2-devel] [PATCH v2 3/3] ArmPkg: Update OemGetChassisType function to return MISC_CHASSIS_TYPE

2021-02-21 Thread Leif Lindholm
On Sat, Feb 20, 2021 at 20:29:00 -0700, Rebecca Cran wrote:
> Update OemGetChassisType in OemMiscLib to return MISC_CHASSIS_TYPE
> instead of EFI_STATUS, which matches other OemMiscLib functions.
> 
> Signed-off-by: Rebecca Cran 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Include/Library/OemMiscLib.h |  8 +++-
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 11 ---
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h 
> b/ArmPkg/Include/Library/OemMiscLib.h
> index 31dfe7dac2a6..5f71b2ad48c7 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -128,14 +128,12 @@ OemGetMaxProcessors (
>  
>  /** Gets the type of chassis for the system.
>  
> -  @param ChassisType The type of the chassis.
> -
> -  @retval EFI_SUCCESS The chassis type was fetched successfully.
> +  @retval The type of the chassis.
>  **/
> -EFI_STATUS
> +MISC_CHASSIS_TYPE
>  EFIAPI
>  OemGetChassisType (
> -  OUT UINT8 *ChassisType
> +  VOID
>);
>  
>  /** Returns whether the specified processor is present or not.
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c 
> b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index b02a568426dd..6b233742feb0 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -95,19 +95,16 @@ OemGetMaxProcessors (
>  
>  /** Gets the type of chassis for the system.
>  
> -  @param ChassisType The type of the chassis.
> -
> -  @retval EFI_SUCCESS The chassis type was fetched successfully.
> +  @retval The type of the chassis.
>  **/
> -EFI_STATUS
> +MISC_CHASSIS_TYPE
>  EFIAPI
>  OemGetChassisType (
> -  UINT8 *ChassisType
> +  VOID
>)
>  {
>ASSERT (FALSE);
> -  *ChassisType = MiscChassisTypeUnknown;
> -  return EFI_SUCCESS;
> +  return MiscChassisTypeUnknown;
>  }
>  
>  /** Returns whether the specified processor is present or not.
> -- 
> 2.26.2
> 


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




Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Reduce DEBUG message verbosity

2021-02-21 Thread Leif Lindholm
On Sun, Feb 21, 2021 at 21:50:20 +0100, Ard Biesheuvel wrote:
> On Sun, 21 Feb 2021 at 21:43, Leif Lindholm  wrote:
> >
> > On Sun, Feb 21, 2021 at 11:50:27 +0100, Ard Biesheuvel wrote:
> > > On Sat, 20 Feb 2021 at 22:46, Leif Lindholm  wrote:
> > > >
> > > > *How* annoying was this?
> > > >
> > > > This is kind of useful information, well at the "would be good to see
> > > > in a regular DEBUG build" level.
> > > >
> > > > This change will have suddenly effectively hidden a message that was
> > > > already present in many platforms, where they were not (very) annoyingly
> > > > repetitive during a normal boot.
> > > >
> > > > It feels the test suite is not the thing that we need to optimise
> > > > debug output for.
> > > >
> > > > Is there some alternative way we can rate limit this?
> > >
> > > Given that the sole purpose of this library is to paper over the fact
> > > that the platform violates the spec, and lacks the ability to tell
> > > time, I think it makes little sense to obsess over how wrong the value
> > > is that it returns.
> >
> > I'm not obsessing about anything, I'm saying that using tediousness
> > when running a testsuite as an argument, potential issues with the
> > only aspect that makes this implementation useful without having to
> > rebuild firmware every few weeks are now hidden.
> >
> > If we truly don't care about the feature, nuke it. Don't hide when it
> > breaks.
> >
> 
> Nuke what exactly? I think the library has a use, I simply don't see
> the point of reporting 25-50 times every boot that no timestamp was
> recorded in a variable.

Nuke the variable and always use the build timestamp EPOCH.
I would prefer that to silent failure.

Or we can figure out and alternative way to stop it spamming the
console.

The library is declared as a BASE library, but that's clearly untrue
as it depends on UefiRuntimeLib.

So we could, for example, break out the "check for EPOCH in variable"
into a protocol and print the message only once at protocol init.
Or we could add a Pcd for "no point, I have no persistent storage".

/
Leif


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




Re: [edk2-devel] [PATCH v2 2/3] ArmPkg: Rename some functions and parameters in OemMiscLib

2021-02-21 Thread Leif Lindholm
On Sat, Feb 20, 2021 at 20:28:59 -0700, Rebecca Cran wrote:
> o Rename 'Offset' parameter in OemUpdateSmbiosInfo to 'Field'.
> o Rename OemGetProcessorMaxSockets to OemGetMaxProcessors.
> o Rename OemIsSocketPresent to OemIsProcessorPresent.
> o Update Universal/Smbios to follow the changes to OemMiscLib.
> 
> Signed-off-by: Rebecca Cran 
> ---
>  ArmPkg/Include/Library/OemMiscLib.h  
>   | 12 +++---
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c  
>   | 13 +++
>  ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c 
>   | 40 ++--
>  
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type03/MiscChassisManufacturerFunction.c
>  | 28 +-
>  4 files changed, 33 insertions(+), 60 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h 
> b/ArmPkg/Include/Library/OemMiscLib.h
> index ad0e77685dbe..31dfe7dac2a6 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -116,13 +116,13 @@ OemGetCacheInformation (
>IN OUT SMBIOS_TABLE_TYPE7 *SmbiosCacheTable
>);
>  
> -/** Gets the maximum number of sockets supported by the platform.
> +/** Gets the maximum number of processors supported by the platform.
>  
> -  @return The maximum number of sockets.
> +  @return The maximum number of processors.
>  **/
>  UINT8
>  EFIAPI
> -OemGetProcessorMaxSockets (
> +OemGetMaxProcessors (
>VOID
>);
>  
> @@ -146,7 +146,7 @@ OemGetChassisType (
>  **/
>  BOOLEAN
>  EFIAPI
> -OemIsSocketPresent (
> +OemIsProcessorPresent (
>IN UINTN ProcessorIndex
>);
>  
> @@ -154,14 +154,14 @@ OemIsSocketPresent (
>  
>@param mHiiHandleThe HII handle.
>@param TokenToUpdate The string to update.
> -  @param OffsetThe field to get information about.
> +  @param Field The field to get information about.
>  **/
>  VOID
>  EFIAPI
>  OemUpdateSmbiosInfo (
>IN EFI_HII_HANDLEHiiHandle,
>IN EFI_STRING_ID TokenToUpdate,
> -  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
> +  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
>);
>  
>  #endif // OEM_MISC_LIB_H_
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c 
> b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index 6b179941e414..b02a568426dd 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -13,7 +13,6 @@
>  #include 
>  #include 
>  #include 
> -
>  #include 
>  
>  
> @@ -80,13 +79,13 @@ OemGetCacheInformation (
>return TRUE;
>  }
>  
> -/** Gets the maximum number of sockets supported by the platform.
> +/** Gets the maximum number of processors supported by the platform.
>  
> -  @return The maximum number of sockets.
> +  @return The maximum number of processors.
>  **/
>  UINT8
>  EFIAPI
> -OemGetProcessorMaxSockets (
> +OemGetMaxProcessors (
>VOID
>)
>  {
> @@ -119,7 +118,7 @@ OemGetChassisType (
>  **/
>  BOOLEAN
>  EFIAPI
> -OemIsSocketPresent (
> +OemIsProcessorPresent (
>IN UINTN ProcessorIndex
>)
>  {
> @@ -131,14 +130,14 @@ OemIsSocketPresent (
>  
>@param mHiiHandleThe HII handle.
>@param TokenToUpdate The string to update.
> -  @param OffsetThe field to get information about.
> +  @param Field The field to get information about.
>  **/
>  VOID
>  EFIAPI
>  OemUpdateSmbiosInfo (
>IN EFI_HII_HANDLE mHiiHandle,
>IN EFI_STRING_ID TokenToUpdate,
> -  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Offset
> +  IN OEM_MISC_SMBIOS_HII_STRING_FIELD Field
>)
>  {
>ASSERT (FALSE);
> diff --git a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c 
> b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> index d03de12a820e..0cb56c53975e 100644
> --- a/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> +++ b/ArmPkg/Universal/Smbios/ProcessorSubClassDxe/ProcessorSubClass.c
> @@ -439,8 +439,8 @@ AddSmbiosCacheTypeTable (
>  strings following the data fields.
>  
>@param[out] Type4RecordThe Type 4 structure to allocate and initialize
> -  @param[in]  ProcessorIndex The index of the processor socket
> -  @param[in]  Populated  Whether the specified processor socket is
> +  @param[in]  ProcessorIndex The index of the processor
> +  @param[in]  Populated  Whether the specified processor is
>   populated.
>  
>@retval EFI_SUCCESS  The Type 4 structure was successfully
> @@ -460,7 +460,7 @@ AllocateType4AndSetProcessorInformationStrings (
>EFI_STRING_ID   SerialNumber;
>EFI_STRING_ID   AssetTag;
>EFI_STRING_ID   PartNumber;
> -  EFI_STRING  ProcessorSocketStr;
> +  EFI_STRING  ProcessorStr;
>EFI_STRING  ProcessorManuStr;
>EFI_STRING  ProcessorVersionStr;
>EFI_STRING  SerialNumberStr;
> @@ -468,7 +468,7 @@ AllocateType4AndSetProcessorInformationStrings (
>EFI_STRING  

Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Reduce DEBUG message verbosity

2021-02-21 Thread Ard Biesheuvel
On Sun, 21 Feb 2021 at 21:43, Leif Lindholm  wrote:
>
> On Sun, Feb 21, 2021 at 11:50:27 +0100, Ard Biesheuvel wrote:
> > On Sat, 20 Feb 2021 at 22:46, Leif Lindholm  wrote:
> > >
> > > *How* annoying was this?
> > >
> > > This is kind of useful information, well at the "would be good to see
> > > in a regular DEBUG build" level.
> > >
> > > This change will have suddenly effectively hidden a message that was
> > > already present in many platforms, where they were not (very) annoyingly
> > > repetitive during a normal boot.
> > >
> > > It feels the test suite is not the thing that we need to optimise
> > > debug output for.
> > >
> > > Is there some alternative way we can rate limit this?
> >
> > Given that the sole purpose of this library is to paper over the fact
> > that the platform violates the spec, and lacks the ability to tell
> > time, I think it makes little sense to obsess over how wrong the value
> > is that it returns.
>
> I'm not obsessing about anything, I'm saying that using tediousness
> when running a testsuite as an argument, potential issues with the
> only aspect that makes this implementation useful without having to
> rebuild firmware every few weeks are now hidden.
>
> If we truly don't care about the feature, nuke it. Don't hide when it
> breaks.
>

Nuke what exactly? I think the library has a use, I simply don't see
the point of reporting 25-50 times every boot that no timestamp was
recorded in a variable.


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




Re: [edk2-devel] [PATCH v2 1/3] ArmPkg: Fix ordering of return type and EFIAPI specifier in OemMiscLib

2021-02-21 Thread Leif Lindholm
On Sat, Feb 20, 2021 at 20:28:58 -0700, Rebecca Cran wrote:
> The return type should be on the line before any EFIAPI specifier.
> 
> Signed-off-by: Rebecca Cran 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Include/Library/OemMiscLib.h | 14 +++---
>  ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c | 12 ++--
>  2 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/ArmPkg/Include/Library/OemMiscLib.h 
> b/ArmPkg/Include/Library/OemMiscLib.h
> index e70019d05f15..ad0e77685dbe 100644
> --- a/ArmPkg/Include/Library/OemMiscLib.h
> +++ b/ArmPkg/Include/Library/OemMiscLib.h
> @@ -71,8 +71,8 @@ typedef enum
>  
>@return   CPU frequency in Hz
>  **/
> -EFIAPI
>  UINTN
> +EFIAPI
>  OemGetCpuFreq (
>IN UINT8 ProcessorIndex
>);
> @@ -87,8 +87,8 @@ OemGetCpuFreq (
>  
>@return  TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetProcessorInformation (
>IN UINTN ProcessorIndex,
>IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
> @@ -106,8 +106,8 @@ OemGetProcessorInformation (
>  
>@return TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetCacheInformation (
>IN UINT8   ProcessorIndex,
>IN UINT8   CacheLevel,
> @@ -120,8 +120,8 @@ OemGetCacheInformation (
>  
>@return The maximum number of sockets.
>  **/
> -EFIAPI
>  UINT8
> +EFIAPI
>  OemGetProcessorMaxSockets (
>VOID
>);
> @@ -132,8 +132,8 @@ OemGetProcessorMaxSockets (
>  
>@retval EFI_SUCCESS The chassis type was fetched successfully.
>  **/
> -EFIAPI
>  EFI_STATUS
> +EFIAPI
>  OemGetChassisType (
>OUT UINT8 *ChassisType
>);
> @@ -144,8 +144,8 @@ OemGetChassisType (
>  
>@return TRUE is the processor is present, FALSE otherwise.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemIsSocketPresent (
>IN UINTN ProcessorIndex
>);
> @@ -156,8 +156,8 @@ OemIsSocketPresent (
>@param TokenToUpdate The string to update.
>@param OffsetThe field to get information about.
>  **/
> -EFIAPI
>  VOID
> +EFIAPI
>  OemUpdateSmbiosInfo (
>IN EFI_HII_HANDLEHiiHandle,
>IN EFI_STRING_ID TokenToUpdate,
> diff --git a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c 
> b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> index 73cebef2d1b9..6b179941e414 100644
> --- a/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> +++ b/ArmPkg/Universal/Smbios/OemMiscLibNull/OemMiscLib.c
> @@ -23,8 +23,8 @@
>  
>@return   CPU frequency in Hz
>  **/
> -EFIAPI
>  UINTN
> +EFIAPI
>  OemGetCpuFreq (
>IN UINT8 ProcessorIndex
>)
> @@ -43,8 +43,8 @@ OemGetCpuFreq (
>  
>@return  TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetProcessorInformation (
>IN UINTN ProcessorIndex,
>IN OUT PROCESSOR_STATUS_DATA *ProcessorStatus,
> @@ -66,8 +66,8 @@ OemGetProcessorInformation (
>  
>@return TRUE on success, FALSE on failure.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemGetCacheInformation (
>IN UINT8   ProcessorIndex,
>IN UINT8   CacheLevel,
> @@ -84,8 +84,8 @@ OemGetCacheInformation (
>  
>@return The maximum number of sockets.
>  **/
> -EFIAPI
>  UINT8
> +EFIAPI
>  OemGetProcessorMaxSockets (
>VOID
>)
> @@ -117,8 +117,8 @@ OemGetChassisType (
>  
>@return TRUE is the processor is present, FALSE otherwise.
>  **/
> -EFIAPI
>  BOOLEAN
> +EFIAPI
>  OemIsSocketPresent (
>IN UINTN ProcessorIndex
>)
> @@ -133,8 +133,8 @@ OemIsSocketPresent (
>@param TokenToUpdate The string to update.
>@param OffsetThe field to get information about.
>  **/
> -EFIAPI
>  VOID
> +EFIAPI
>  OemUpdateSmbiosInfo (
>IN EFI_HII_HANDLE mHiiHandle,
>IN EFI_STRING_ID TokenToUpdate,
> -- 
> 2.26.2
> 


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




Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Reduce DEBUG message verbosity

2021-02-21 Thread Leif Lindholm
On Sun, Feb 21, 2021 at 11:50:27 +0100, Ard Biesheuvel wrote:
> On Sat, 20 Feb 2021 at 22:46, Leif Lindholm  wrote:
> >
> > *How* annoying was this?
> >
> > This is kind of useful information, well at the "would be good to see
> > in a regular DEBUG build" level.
> >
> > This change will have suddenly effectively hidden a message that was
> > already present in many platforms, where they were not (very) annoyingly
> > repetitive during a normal boot.
> >
> > It feels the test suite is not the thing that we need to optimise
> > debug output for.
> >
> > Is there some alternative way we can rate limit this?
> 
> Given that the sole purpose of this library is to paper over the fact
> that the platform violates the spec, and lacks the ability to tell
> time, I think it makes little sense to obsess over how wrong the value
> is that it returns.

I'm not obsessing about anything, I'm saying that using tediousness
when running a testsuite as an argument, potential issues with the
only aspect that makes this implementation useful without having to
rebuild firmware every few weeks are now hidden.

If we truly don't care about the feature, nuke it. Don't hide when it
breaks.

/
Leif


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




[edk2-devel] [PATCH v2 2/2] TigerlakeSiliconPkg/IpBlock: Fix build errors with GCC5

2021-02-21 Thread Takuto Naito
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224

- Fix the Teton Glacier Endpoint entry in mPciDeviceTable

Cc: Sai Chaganty 
Cc: Nate DeSimone 
Cc: Heng Luo 
Signed-off-by: Takuto Naito 
---

Notes:
v2:
- Split the v1 patch into 2 patches,
  One is for Platform/Intel/TigerlakeOpenBoardPkg,
  another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.

 .../DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c
 
b/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c
index 577e436e32..1553d2e2aa 100644
--- 
a/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c
+++ 
b/Silicon/Intel/TigerlakeSiliconPkg/IpBlock/PcieRp/LibraryPrivate/DxePchPcieRpPolicyLib/DxePchPcieRpPolicyLib.c
@@ -98,7 +98,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED PCH_PCIE_DEVICE_OVERRIDE 
mPcieDeviceTable[] = {
   //
   // Teton Glacier Endpoint
   //
-  { 0x8086, 0x0975, 0xff, 0, 0, 0, PchPcieL1SubstatesOverride, 0, 0xff, 0x3C, 
0, 5, 0, 0, 0, 0 },
+  { 0x8086, 0x0975, 0xff, 0, 0, 0, PchPcieL1SubstatesOverride, 0, 0xff, 0x3C, 
0, 5, 0, 0 },
 
   //
   // End of Table
-- 
2.30.1



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




[edk2-devel] [PATCH v2 1/2] TigerlakeOpenBoardPkg: Fix build errors with GCC5

2021-02-21 Thread Takuto Naito
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224

- Fix the path of TigerLakeFspBinPkg
- Fix misuse of RETURN_ERROR
- Remove unused function CheckNationalSio.

Cc: Sai Chaganty 
Cc: Nate DeSimone 
Cc: Heng Luo 
Signed-off-by: Takuto Naito 
---

Notes:
v2:
- Split the v1 patch into 2 patches,
  One is for Platform/Intel/TigerlakeOpenBoardPkg,
  another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.

 .../PeiFspPolicyInitLib.inf   |   2 +-
 .../BasePlatformHookLib/BasePlatformHookLib.c | 188 --
 .../DxeSiliconPolicyUpdateLate.c  |   2 +-
 3 files changed, 2 insertions(+), 190 deletions(-)

diff --git 
a/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf
 
b/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf
index 9d85d855f5..708fbac08f 100644
--- 
a/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf
+++ 
b/Platform/Intel/TigerlakeOpenBoardPkg/FspWrapper/Library/PeiFspPolicyInitLib/PeiFspPolicyInitLib.inf
@@ -52,7 +52,7 @@
   MdeModulePkg/MdeModulePkg.dec
   IntelFsp2Pkg/IntelFsp2Pkg.dec
   TigerlakeSiliconPkg/SiPkg.dec
-  TigerLakeFspBinPkg/TigerLakeFspBinPkg.dec
+  TigerLakeFspBinPkg/Client/TigerLakeFspBinPkg.dec
   TigerlakeOpenBoardPkg/OpenBoardPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
   IntelSiliconPkg/IntelSiliconPkg.dec
diff --git 
a/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePlatformHookLib.c
 
b/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePlatformHookLib.c
index 6209e50450..cc5337698b 100644
--- 
a/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePlatformHookLib.c
+++ 
b/Platform/Intel/TigerlakeOpenBoardPkg/Library/BasePlatformHookLib/BasePlatformHookLib.c
@@ -94,194 +94,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_SIO_TABLE 
mSioTableWinbond_x374[] = {
   {0x30, 0x01}// Enable it with Activation bit
 };
 
-/**
-  Detect if a National 393 SIO is docked. If yes, enable the docked SIO
-  and its serial port, and disable the onboard serial port.
-
-  @retval EFI_SUCCESS Operations performed successfully.
-**/
-STATIC
-VOID
-CheckNationalSio (
-  VOID
-  )
-{
-  UINT8   Data8;
-
-  //
-  // Pc87393 access is through either (0x2e, 0x2f) or (0x4e, 0x4f).
-  // We use (0x2e, 0x2f) which is determined by BADD default strapping
-  //
-
-  //
-  // Read the Pc87393 signature
-  //
-  IoWrite8 (0x2e, 0x20);
-  Data8 = IoRead8 (0x2f);
-
-  if (Data8 == 0xea) {
-//
-// Signature matches - National PC87393 SIO is docked
-//
-
-//
-// Enlarge the LPC decode scope to accommodate the Docking LPC Switch
-// Register (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS is allocated at
-// SIO_BASE_ADDRESS + 0x10)
-//
-PchLpcGenIoRangeSet ((FixedPcdGet16 (PcdSioBaseAddress) & (UINT16)~0x7F), 
0x20);
-
-//
-// Enable port switch
-//
-IoWrite8 (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS, 0x06);
-
-//
-// Turn on docking power
-//
-IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0x8c);
-
-IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0x9c);
-
-IoWrite8 (FixedPcdGet16 (PcdSioBaseAddress) + 0x0E, 0xBc);
-
-//
-// Enable port switch
-//
-IoWrite8 (SIO_DOCKING_LPC_SWITCH_REGISTER_ADDRESS, 0x7);
-
-//
-// GPIO setting
-//
-IoWrite8 (0x2e, 0x24);
-IoWrite8 (0x2f, 0x29);
-
-//
-// Enable chip clock
-//
-IoWrite8 (0x2e, 0x29);
-IoWrite8 (0x2f, 0x1e);
-
-
-//
-// Enable serial port
-//
-
-//
-// Select com1
-//
-IoWrite8 (0x2e, 0x7);
-IoWrite8 (0x2f, 0x3);
-
-//
-// Base address: 0x3f8
-//
-IoWrite8 (0x2e, 0x60);
-IoWrite8 (0x2f, 0x03);
-IoWrite8 (0x2e, 0x61);
-IoWrite8 (0x2f, 0xf8);
-
-//
-// Interrupt: 4
-//
-IoWrite8 (0x2e, 0x70);
-IoWrite8 (0x2f, 0x04);
-
-//
-// Enable bank selection
-//
-IoWrite8 (0x2e, 0xf0);
-IoWrite8 (0x2f, 0x82);
-
-//
-// Activate
-//
-IoWrite8 (0x2e, 0x30);
-IoWrite8 (0x2f, 0x01);
-
-//
-// Disable onboard serial port
-//
-IoWrite8 (FixedPcdGet16 (PcdLpcSioConfigDefaultPort), 0x55);
-
-//
-// Power Down UARTs
-//
-IoWrite8 (PcdGet16 (PcdLpcSioIndexPort), 0x2);
-IoWrite8 (PcdGet16 (PcdLpcSioDataPort), 0x00);
-
-//
-// Dissable COM1 decode
-//
-IoWrite8 (PcdGet16 (PcdLpcSioIndexPort), 0x24);
-IoWrite8 (PcdGet16 (PcdLpcSioDataPort), 0);
-
-//
-// Disable COM2 decode
-//
-IoWrite8 (PcdGet16 (PcdLpcSioIndexPort), 0x25);
-IoWrite8 (PcdGet16 (PcdLpcSioDataPort), 0);
-
-//
-// Disable interrupt
-//
-IoWrite8 (PcdGet16 (PcdLpcSioIndexPort), 0x28);
-IoWrite8 (PcdGet16 (PcdLpcSioDataPort), 0x0);
-
-IoWrite8 (FixedPcdGet16 (PcdLpcSioConfigDefaultPort), 

[edk2-devel] [PATCH v2 0/2] TigerlakeOpenBoard: Fix build errors with GCC5

2021-02-21 Thread Takuto Naito
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3224
TigerlakeOpenBoard: Fix build errors with GCC5

v2:
- Split the v1 patch into 2 patches.
  One is for Platform/Intel/TigerlakeOpenBoardPkg,
  another one is for edk2-platforms\Silicon\Intel\TigerlakeSiliconPkg.

https://github.com/naitaku/edk2-platforms/tree/tigerlake_fix_build_error_v2

Cc: Sai Chaganty 
Cc: Nate DeSimone 
Cc: Heng Luo 

Takuto Naito (2):
  TigerlakeOpenBoardPkg: Fix build errors with GCC5
  TigerlakeSiliconPkg/IpBlock: Fix build errors with GCC5

 .../PeiFspPolicyInitLib.inf   |   2 +-
 .../BasePlatformHookLib/BasePlatformHookLib.c | 188 --
 .../DxeSiliconPolicyUpdateLate.c  |   2 +-
 .../DxePchPcieRpPolicyLib.c   |   2 +-
 4 files changed, 3 insertions(+), 191 deletions(-)

-- 
2.30.1



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




Re: [edk2-devel] [edk2-platform][PATCH v1 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Reduce DEBUG message verbosity

2021-02-21 Thread Ard Biesheuvel
On Sat, 20 Feb 2021 at 22:46, Leif Lindholm  wrote:
>
> *How* annoying was this?
>
> This is kind of useful information, well at the "would be good to see
> in a regular DEBUG build" level.
>
> This change will have suddenly effectively hidden a message that was
> already present in many platforms, where they were not (very) annoyingly
> repetitive during a normal boot.
>
> It feels the test suite is not the thing that we need to optimise
> debug output for.
>
> Is there some alternative way we can rate limit this?
>

Given that the sole purpose of this library is to paper over the fact
that the platform violates the spec, and lacks the ability to tell
time, I think it makes little sense to obsess over how wrong the value
is that it returns.


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