Re: [edk2-devel] [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
Reviewed-by: Jiewen Yao
> -Original Message-
> From: Zhang, Qi1
> Sent: Friday, July 29, 2022 10:26 AM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 ; Yao, Jiewen ;
> Wang, Jian J ; Patil, Swapnil
>
> Subject: [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980
>
> As per TCG PC Client Device Driver Design Principle document,
> if tpm commands fails due to timeout condition, then it should
> have retry mechanism (3 retry attempts).
> Existing implementation of PtpCrbTpmCommand does not have retry
> mechanism if it fails with EFI_TIMEOUT.
>
> See TCG PC Client Device Driver Design Principles for TPM 2.0
> https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1
> _r4_211104_final.pdf
> Vision 1.1, Revision 0.04
> Section 7.2.1
>
> Signed-off-by: Qi Zhang
> Cc: Jiewen Yao
> Cc: Jian J Wang
> Reviewed-by: Jiewen Yao
> Tested-by: Swapnil Patil
> ---
> .../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 108 +++---
> 1 file changed, 69 insertions(+), 39 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1d99beaa10..840265292a 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -33,6 +33,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> //
>
> #define TPMCMDBUFLENGTH 0x500
>
>
>
> +//
>
> +// Max retry count according to Spec TCG PC Client Device Driver Design
> Principles
>
> +// for TPM2.0, Version 1.1, Revision 0.04, Section 7.2.1
>
> +//
>
> +#define RETRY_CNT_MAX 3
>
> +
>
> /**
>
>Check whether TPM PTP register exist.
>
>
>
> @@ -153,6 +159,7 @@ PtpCrbTpmCommand (
>UINT32 TpmOutSize;
>
>UINT16 Data16;
>
>UINT32 Data32;
>
> + UINT8 RetryCnt;
>
>
>
>DEBUG_CODE_BEGIN ();
>
>UINTN DebugSize;
>
> @@ -179,53 +186,76 @@ PtpCrbTpmCommand (
>DEBUG_CODE_END ();
>
>TpmOutSize = 0;
>
>
>
> - //
>
> - // STEP 0:
>
> - // if CapCRbIdelByPass == 0, enforce Idle state before sending command
>
> - //
>
> - if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)
> >CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
>
> + RetryCnt = 0;
>
> + while (TRUE) {
>
> +//
>
> +// STEP 0:
>
> +// if CapCRbIdelByPass == 0, enforce Idle state before sending command
>
> +//
>
> +if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)
> >CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
>
> + Status = PtpCrbWaitRegisterBits (
>
> + >CrbControlStatus,
>
> + PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>
> + 0,
>
> + PTP_TIMEOUT_C
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> +RetryCnt++;
>
> +if (RetryCnt < RETRY_CNT_MAX) {
>
> + MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
>
> + continue;
>
> +} else {
>
> + //
>
> + // Try to goIdle to recover TPM
>
> + //
>
> + Status = EFI_DEVICE_ERROR;
>
> + goto GoIdle_Exit;
>
> +}
>
> + }
>
> +}
>
> +
>
> +//
>
> +// STEP 1:
>
> +// Ready is any time the TPM is ready to receive a command, following a
> write
>
> +// of 1 by software to Request.cmdReady, as indicated by the Status field
>
> +// being cleared to 0.
>
> +//
>
> +MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
>
> Status = PtpCrbWaitRegisterBits (
>
> - >CrbControlStatus,
>
> - PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>
> + >CrbControlRequest,
>
> 0,
>
> + PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
>
> PTP_TIMEOUT_C
>
> );
>
> if (EFI_ERROR (Status)) {
>
> - //
>
> - // Try to goIdle to recover TPM
>
> - //
>
> - Status = EFI_DEVICE_ERROR;
>
> - goto GoIdle_Exit;
>
> + RetryCnt++;
>
> + if (RetryCnt < RETRY_CNT_MAX) {
>
> +MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
>
> +continue;
>
> + } else {
>
> +Status = EFI_DEVICE_ERROR;
>
> +goto GoIdle_Exit;
>
> + }
>
> }
>
> - }
>
>
>
> - //
>
> - // STEP 1:
>
> - // Ready is any time the TPM is ready to receive a command, following a
> write
>
> - // of 1 by software to Request.cmdReady, as indicated by the Status field
>
> - // being cleared to 0.
>
> - //
>
> - MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
>
> - Status = PtpCrbWaitRegisterBits (
>
> - >CrbControlRequest,
>
> -
Re: [edk2-devel] [PATCH v2 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
Hi Pierre,
Thank you for the suggestions. I will update the patch per your feedback and
send for review in v3.
Regards,
Kun
On 7/28/2022 6:07 AM, Pierre Gondois wrote:
Hello Kun,
With the changes below:
Reviewed-by: Pierre Gondois
Thanks!
On 7/28/22 06:31, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
Certain OSes will complain if the ECAM config space is not reserved in
the ACPI namespace.
This change adds a function to reserve PNP motherboard resources for a
given PCI node.
Co-authored-by: Joe Lopez
Signed-off-by: Kun Qin
---
Notes:
v2:
- Only create RES0 after config space checking [Pierre]
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
| 169
1 file changed, 169 insertions(+)
diff --git
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index ceffe2838c03..c03550baabf2 100644
---
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -616,6 +616,167 @@ GeneratePciCrs (
return Status;
}
+/** Generate a Pci Resource Template to hold Address Space Info
+
+ @param [in] PciNode RootNode of the AML tree.
+ @param [in, out] CrsNode CRS node of the AML tree to populate.
I think CrsNode is an 'out' only parameter. Also the description of
PciNode
seems incorrect.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid input parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PopulateBasicPciResObjects (
Would it be possible to rename it:
GenerateMotherboardDevice()
or with a name related to 'motherboard' ?
+ IN AML_OBJECT_NODE_HANDLE PciNode,
+ IN OUT AML_OBJECT_NODE_HANDLE *CrsNode
+ )
+{
+ EFI_STATUS Status;
+ UINT32 EisaId;
+ AML_OBJECT_NODE_HANDLE ResNode;
+
+ if (CrsNode == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // ASL: Device (PCIx) {}
+ Status = AmlCodeGenDevice ("RES0", PciNode, );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // ASL: Name (_HID, EISAID ("PNP0C02"))
+ Status = AmlGetEisaIdFromString ("PNP0C02", ); /* PNP
Motherboard Resources */
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // ASL: Name (_CRS, ResourceTemplate () {})
+ Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, CrsNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ return Status;
+}
+
+/** Generate a Pci Resource Template to hold Address Space Info
+
+ @param [in] Generator The SSDT Pci generator.
+ @param [in] CfgMgrProtocol Pointer to the Configuration
Manager
+ Protocol interface.
+ @param [in] PciInfo Pci device information.
+ @param [in, out] PciNode RootNode of the AML tree to
populate.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GeneratePciRes (
Would it be possible to rename it:
ReserveEcamSpace()
or with a name related to 'ecam' ?
+ IN ACPI_PCI_GENERATOR *Generator,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
CfgMgrProtocol,
+ IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo,
+ IN OUT AML_OBJECT_NODE_HANDLE PciNode
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE_HANDLE CrsNode;
+ BOOLEAN Translation;
+ UINT32 Index;
+ CM_ARM_OBJ_REF *RefInfo;
+ UINT32 RefCount;
+ CM_ARM_PCI_ADDRESS_MAP_INFO *AddrMapInfo;
+ BOOLEAN IsPosDecode;
+
+ // Get the array of CM_ARM_OBJ_REF referencing the
+ // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
+ Status = GetEArmObjCmRef (
+ CfgMgrProtocol,
+ PciInfo->AddressMapToken,
+ ,
+
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ for (Index = 0; Index < RefCount; Index++) {
+ // Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
+ Status = GetEArmObjPciAddressMapInfo (
+ CfgMgrProtocol,
+ RefInfo[Index].ReferenceToken,
+ ,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ Translation = (AddrMapInfo->CpuAddress !=
Re: [edk2-devel] [PATCH v2 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
Hi Pierre,
Thank you for your feedback. I will add more document/comments to the
newly define structure, as
well as the "break" as you suggested.
As per failure to locate "gEfiAcpiSdtProtocolGuid", my thought was that
this protocol could be enabled
per platform through
"gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol". So I did not
treat
the failure as a hard requirement (for the same reason, I did not add it
to the module Depex). Please let
me know whether you think this makes sense. Otherwise, I could replace
the "goto" logic with a check
for the same PCD and only conduct the routine if ACPI_SDT is expected.
Please also let me know if you have other suggestions.
Thanks,
Kun
On 7/28/2022 6:07 AM, Pierre Gondois wrote:
Hello Kun,
With the changes below:
Reviewed-by: Pierre Gondois
Thanks!
On 7/28/22 06:31, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
This change added an extra step to allow check for installed ACPI
tables.
For FADT, MADT, GTDT, DSDT, DBG2 and SPCR tables, either
pre-installed or
supplied through AcpiTableInfo can be accepted.
An extra check for FADT ACPI table existence during installation step is
also added.
Cc: Sami Mujawar
Cc: Alexei Fedorov
Co-authored-by: Joe Lopez
Signed-off-by: Kun Qin
---
Notes:
v2:
- Function description updates [Sami]
- Refactorized the table verification [Pierre]
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
| 182 +++-
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf
| 1 +
2 files changed, 103 insertions(+), 80 deletions(-)
diff --git
a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index ed62299f9bbd..4ad7c0c8dbfa 100644
---
a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++
b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
// Module specific include files.
@@ -22,6 +23,29 @@
#include
#include
+#define ACPI_TABLE_PRESENT_INFO_LIST BIT0
+#define ACPI_TABLE_PRESENT_INSTALLED BIT1
+
+#define ACPI_TABLE_VERIFY_FADT 0
+#define ACPI_TABLE_VERIFY_COUNT 6
+
+typedef struct {
+ ESTD_ACPI_TABLE_ID EstdTableId;
+ UINT32 AcpiTableSignature;
+ CHAR8 AcpiTableName[sizeof (UINT32) + 1];
+ BOOLEAN IsMandatory;
+ UINT16 Presence;
+} ACPI_TABLE_PRESENCE_INFO;
I think it needs some documentation (also for mAcpiVerifyTables).
+
+ACPI_TABLE_PRESENCE_INFO mAcpiVerifyTables[ACPI_TABLE_VERIFY_COUNT] = {
+ { EStdAcpiTableIdFadt,
EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, "FADT", TRUE, 0 },
+ { EStdAcpiTableIdMadt,
EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE, "MADT",
TRUE, 0 },
+ { EStdAcpiTableIdGtdt,
EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE, "GTDT",
TRUE, 0 },
+ { EStdAcpiTableIdDsdt,
EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
"DSDT", TRUE, 0 },
+ { EStdAcpiTableIdDbg2, EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
"DBG2", FALSE, 0 },
+ { EStdAcpiTableIdSpcr,
EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE, "SPCR",
FALSE, 0 },
+};
+
/** This macro expands to a function that retrieves the ACPI Table
List from the Configuration Manager.
*/
@@ -395,6 +419,7 @@ BuildAndInstallAcpiTable (
@retval EFI_SUCCESS Success.
@retval EFI_NOT_FOUND If mandatory table is not found.
+ @retval EFI_ALREADY_STARTED If mandatory table found in
AcpiTableInfo is already installed.
**/
STATIC
EFI_STATUS
@@ -404,75 +429,68 @@ VerifyMandatoryTablesArePresent (
IN UINT32 AcpiTableCount
)
{
- EFI_STATUS Status;
- BOOLEAN FadtFound;
- BOOLEAN MadtFound;
- BOOLEAN GtdtFound;
- BOOLEAN DsdtFound;
- BOOLEAN Dbg2Found;
- BOOLEAN SpcrFound;
+ EFI_STATUS Status;
+ UINTN Handle;
+ UINTN Index;
+ UINTN InstalledTableIndex;
+ EFI_ACPI_DESCRIPTION_HEADER *DescHeader;
+ EFI_ACPI_TABLE_VERSION Version;
+ EFI_ACPI_SDT_PROTOCOL *AcpiSdt;
- Status = EFI_SUCCESS;
- FadtFound = FALSE;
- MadtFound = FALSE;
- GtdtFound = FALSE;
- DsdtFound = FALSE;
- Dbg2Found = FALSE;
- SpcrFound = FALSE;
ASSERT (AcpiTableInfo != NULL);
+ Status = EFI_SUCCESS;
+
+ // Check against the statically initialized ACPI tables to see if
they are in ACPI info list
while (AcpiTableCount-- != 0) {
- switch (AcpiTableInfo[AcpiTableCount].AcpiTableSignature) {
- case EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
- FadtFound = TRUE;
- break;
- case
Re: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable & SmmFeatureControl capability
Due to the SMI latency impact for IA-32 processor, I will drop this change &
replace with the PCD check. I will resend the new patch for review.
Thanks,
Jiaxin
> -Original Message-
> From: Wu, Jiaxin
> Sent: Monday, July 18, 2022 3:32 PM
> To: Kinney, Michael D ; devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray
> Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable
> & SmmFeatureControl capability
>
> Hi Mike,
>
> Thanks the comments. Only IA-32 processor will check on every SMI since it
> needs configure Mtrr. Do you think the impact is acceptable or not?
>
> For fixed PCD solution, the original concern: the fixed PCD will be treated as
> global variable. Should we need consider no compiler optimization case or it
> must be optimized away condition checks?
>
> Thanks,
> Jiaxin
>
>
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Monday, July 18, 2022 8:13 AM
> > To: devel@edk2.groups.io; Wu, Jiaxin ; Kinney, Michael
> > D
> > Cc: Dong, Eric ; Ni, Ray
> > Subject: RE: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR
> enable
> > & SmmFeatureControl capability
> >
> > Are these checks made on every SMI?
> >
> > What is the impact to SMI latency to do the check dynamically?
> >
> > FeatureFlag and FixedAtBuild PCDs are declared as const global variables
> > which are used by optimizing compiler as constants in instructions or
> > optimize away condition checks all together. This option should still
> > be considered.
> >
> > Mike
> >
> > > -Original Message-
> > > From: devel@edk2.groups.io On Behalf Of Wu,
> > Jiaxin
> > > Sent: Sunday, July 17, 2022 1:38 AM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric ; Ni, Ray
> > > Subject: [edk2-devel] [PATCH v1] UefiCpuPkg: Dynamic check SMRR enable
> &
> > SmmFeatureControl capability
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3962
> > >
> > > Two SMM variables (mSmrrSupported & mSmmFeatureControlSupported)
> > are global
> > > variables, they control whether the SMRR and SMM Feature Control MSR
> will
> > > be restored respectively.
> > > To avoid the TOCTOU, dynamic check SMRR enable & SmmFeatureControl
> > capability.
> > >
> > > Change-Id: I6835e4b0e12c5e6f52effb60fd9224e3eb97fc0d
> > > Cc: Eric Dong
> > > Cc: Ray Ni
> > > Signed-off-by: Jiaxin Wu
> > > ---
> > > .../SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c| 248
> > -
> > > 1 file changed, 141 insertions(+), 107 deletions(-)
> > >
> > > diff --git
> > a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> > > b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> > > index 78de7f8407..b2f31c993f 100644
> > > ---
> > a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> > > +++
> > b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> > > @@ -35,26 +35,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > // MSRs required for configuration of SMM Code Access Check
> > > //
> > > #define SMM_FEATURES_LIB_IA32_MCA_CAP 0x17D
> > > #define SMM_CODE_ACCESS_CHK_BIT BIT58
> > >
> > > -//
> > > -// Set default value to assume SMRR is not supported
> > > -//
> > > -BOOLEAN mSmrrSupported = FALSE;
> > > -
> > > -//
> > > -// Set default value to assume MSR_SMM_FEATURE_CONTROL is not
> > supported
> > > -//
> > > -BOOLEAN mSmmFeatureControlSupported = FALSE;
> > > -
> > > -//
> > > -// Set default value to assume IA-32 Architectural MSRs are used
> > > -//
> > > -UINT32 mSmrrPhysBaseMsr =
> SMM_FEATURES_LIB_IA32_SMRR_PHYSBASE;
> > > -UINT32 mSmrrPhysMaskMsr =
> > SMM_FEATURES_LIB_IA32_SMRR_PHYSMASK;
> > > -
> > > //
> > > // Set default value to assume MTRRs need to be configured on each SMI
> > > //
> > > BOOLEAN mNeedConfigureMtrrs = TRUE;
> > >
> > > @@ -62,26 +46,39 @@ BOOLEAN mNeedConfigureMtrrs = TRUE;
> > > // Array for state of SMRR enable on all CPUs
> > > //
> > > BOOLEAN *mSmrrEnabled;
> > >
> > > /**
> > > - Performs library initialization.
> > > + Return if SMRR is supported
> > >
> > > - This initialization function contains common functionality shared
> > > betwen
> all
> > > - library instance constructors.
> > > + @param[in] SmrrPhysBaseMsr Pointer to SmrrPhysBaseMsr.
> > > + @param[in] SmrrPhysMaskMsr Pointer to SmrrPhysMaskMsr.
> > > +
> > > + @retval TRUE SMRR is supported.
> > > + @retval FALSE SMRR is not supported.
> > >
> > > **/
> > > -VOID
> > > -CpuFeaturesLibInitialization (
> > > - VOID
> > > +BOOLEAN
> > > +IsSmrrSupported (
> > > + IN UINT32 *SmrrPhysBaseMsrOPTIONAL,
> > > + IN UINT32 *SmrrPhysMaskMsrOPTIONAL
> > >)
> > > {
> > > + BOOLEAN ReturnValue;
> > > +
> > >UINT32 RegEax;
> > >UINT32 RegEdx;
> > >UINTN FamilyId;
> > >UINTN ModelId;
> > >
> > > + UINT64 FeatureControl;
> > > +
> > > + //
> > > + // Set default value to assume SMRR is not supported
> > > + //
> > > + ReturnValue = FALSE;
> > > +
> > >//
>
Re: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for GenCrc32 tool
Hi Konstantin, this is really an issue need to be solved.
When GenSec tool uses the guidtools to encode the section, it differentiates
how GenCrc32 is used from other tools..
GenSec defines a CRC32_SECTION_HEADER structure for GenCrc32, and calculate the
DataOffset of the Section with this header.
However other guidtools just use the EFI_GUID_DEFINED_SECTION header and
calculate the DataOffset with it.
"
typedef struct {
EFI_GUID_DEFINED_SECTION GuidSectionHeader;
UINT32CRC32Checksum;
} CRC32_SECTION_HEADER;
Crc32GuidSect->GuidSectionHeader.DataOffset = sizeof (CRC32_SECTION_HEADER);
"
Personally think that is the root cause of the 4 bytes issue.
I do not know why the GenCrc32 usage is different from other tools in origin
design.
Propose to solve this issue from GenSec side to keep all the guidtool with same
function.
Thanks,
Christine
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of
> Konstantin Aladyshev
> Sent: Tuesday, July 19, 2022 9:45 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C ; Gao, Liming
> ; Chen, Christine ;
> Konstantin Aladyshev
> Subject: [edk2-devel] [PATCH] BaseTools/VolInfo: Correct buffer for
> GenCrc32 tool
>
> If the guided section was encoded with GenCrc32 tool the resulting
> 'EFI_GUID_DEFINED_SECTION.DataOffset' field points to the start of the
> meaningfull data that follows the CRC32 value.
> But if we want to decode the section with GenCrc32 tool we need to provide
> a buffer that includes the CRC32 value itself.
>
> Signed-off-by: Konstantin Aladyshev
> mailto:aladyshe...@gmail.com>>
> ---
> BaseTools/Source/C/VolInfo/VolInfo.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/BaseTools/Source/C/VolInfo/VolInfo.c
> b/BaseTools/Source/C/VolInfo/VolInfo.c
> index f450796f9c..47a8f80f46 100644
> --- a/BaseTools/Source/C/VolInfo/VolInfo.c
> +++ b/BaseTools/Source/C/VolInfo/VolInfo.c
> @@ -1999,6 +1999,13 @@ Returns:
>); free (ExtractionTool); +if (!CompareGuid (+
> EfiGuid,+
> + )+ ) {+
> DataOffset -= 4;+} Status = PutFileImage (
>ToolInputFile,--
> 2.25.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#91530): https://edk2.groups.io/g/devel/message/91530
> Mute This Topic: https://groups.io/mt/92482555/4546272
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [yuwei.c...@intel.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91960): https://edk2.groups.io/g/devel/message/91960
Mute This Topic: https://groups.io/mt/92482555/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] IntelFsp2Pkg: Fix GenCfgOpt bug for FSPI_UPD support.
Patch merged:
https://github.com/tianocore/edk2/commit/0d0bfcb4571caa65b7875003f38e67e2ac7e5560
Thanks,
Chasel
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Chiu, Chasel
> Sent: Thursday, July 28, 2022 7:57 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star
> Subject: [edk2-devel] [PATCH] IntelFsp2Pkg: Fix GenCfgOpt bug for FSPI_UPD
> support.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> Fixed a logic bug in GenCfgOpt.py to skip FSPI_UPD when platforms do not
> support.
>
> Cc: Nate DeSimone
> Cc: Star Zeng
> Signed-off-by: Chasel Chiu
> ---
> IntelFsp2Pkg/Tools/GenCfgOpt.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> b/IntelFsp2Pkg/Tools/GenCfgOpt.py index 71c48f10e0..13be81ddbc 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -1301,7 +1301,7 @@ EndList
> elif '_S' in SignatureStr[6:6+2]:
> TxtBody.append("#define
> FSPS_UPD_SIGNATURE %s/* '%s' */\n\n" % (Item['value'],
> SignatureStr))elif '_I' in SignatureStr[6:6+2]:-
> if NoFSPI ==
> True:+ if NoFSPI == False:
> TxtBody.append("#define
> FSPI_UPD_SIGNATURE %s/* '%s' */\n\n" % (Item['value'],
> SignatureStr)) TxtBody.append("\n") --
> 2.35.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#91956): https://edk2.groups.io/g/devel/message/91956
> Mute This Topic: https://groups.io/mt/92683969/1777047
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.c...@intel.com] -=-
> =-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91959): https://edk2.groups.io/g/devel/message/91959
Mute This Topic: https://groups.io/mt/92683969/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] Capsule update with USBIO in FmpDxe
In \FmpDevicePkg\Library\FmpDeviceLibNull\FmpDeviceLib.c There are comments for
RegisterFmpInstaller:
/**
Used to pass the FMP install function to this lib. This allows the library to
have control of the handle that the FMP instance is installed on. This allows
the library to use DriverBinding protocol model to locate its device(s) in the
system.
@param[in] Func Function pointer to FMP install function.
@retval EFI_SUCCESS Library has saved function pointer and will call
function pointer on each DriverBinding Start.
@retval EFI_UNSUPPORTED Library doesn't use driver binding and only supports
a single instance.
@retval other error Error occurred. Don't install FMP
**/
EFI_STATUS
EFIAPI
RegisterFmpInstaller (
IN FMP_DEVICE_LIB_REGISTER_FMP_INSTALLER Func
)
{
//
// This is a system firmware update that does not use Driver Binding Protocol
//
return EFI_UNSUPPORTED;
}
However, since the ‘RegisterFmpInstaller’ of the example directly returns
EFI_UNSUPPORTED , it does not demonstrate how to handle the DriverBinding
protocol. In addition, I refer to the example of
\MdeModulePkg\Bus\Usb\UsbMouseAbsolutePointerDxe adding the driverbinding
protocol to locate the USBMouse device to the 'RegisterFmpInstaller' function.
And set the return value of RegisterFmpInstaller to EFI_SUCCESS. where
RegisterFmpInstaller is in the following file:
edk2-platforms\Platform\Intel\Vlv2TbltDevicePkg\Feature\Capsule\Library\FmpDeviceLibSample
Then tested on Minnowboard.
Test command: CapsuleApp.efi Red.cap
The test results are as follows:
*
During the Capsule update process, when FmpDeviceSetImageWithStatus is
executed, ‘USBMouseAbsolutePointerDriverBindingSupported' has not been
executed, that is, USB Device IO cannot be operated yet.
*
After USBMouseAbsolutePointerDriverBindingSupported is executed, USBIO can be
operated, but at this time 'FmpDeviceSetImageWithStatus' has been executed,
that is, capsule update time has passed and cannot be processed.
*
I also refer to the method of Locate spiFlash protocol in
‘PlatformFlashAccessLib.c’, instead of using DriverBinding model, I directly
Locate USBIo Protocol, which is written as follows:
Status = gBS->LocateProtocol(, NULL, (VOID**));
But the returned value is Status: EFI_NOT_FOUND. The file full path is as
follows:
\edk2-platforms\Platform\Intel\Vlv2TbltDevicePkg\Feature\Capsule\Library\PlatformFlashAccessLib\PlatformFlashAccessLib.c
My questions are as follows:
* To operate USB Io in FMPDxe driver, should I use driverbinding protocol in
RegisterFmpInstaller? Or can use LocateProtocol directly? Or is there any other
way to handle it?
* Are there example codes of how RegisterFmpInstaller handles the driverbinding
protocol or other method such as LocateProtocol?
* When processing the Capsule update (FmpDeviceSetImageWithStatus), the
USBMouseAbsolutePointerDriverBindingSupported has not been executed. How should
I deal with it?
Any suggestion is highly appreciated.
Thanks!
Best Regards,
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91958): https://edk2.groups.io/g/devel/message/91958
Mute This Topic: https://groups.io/mt/92684345/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] IntelFsp2Pkg: Fix GenCfgOpt bug for FSPI_UPD support.
Reviewed-by: Star Zeng
-Original Message-
From: Chiu, Chasel
Sent: Friday, July 29, 2022 10:57 AM
To: devel@edk2.groups.io
Cc: Chiu, Chasel ; Desimone, Nathaniel L
; Zeng, Star
Subject: [PATCH] IntelFsp2Pkg: Fix GenCfgOpt bug for FSPI_UPD support.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
Fixed a logic bug in GenCfgOpt.py to skip FSPI_UPD when platforms
do not support.
Cc: Nate DeSimone
Cc: Star Zeng
Signed-off-by: Chasel Chiu
---
IntelFsp2Pkg/Tools/GenCfgOpt.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index 71c48f10e0..13be81ddbc 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -1301,7 +1301,7 @@ EndList
elif '_S' in SignatureStr[6:6+2]:
TxtBody.append("#define FSPS_UPD_SIGNATURE %s
/* '%s' */\n\n" % (Item['value'], SignatureStr))
elif '_I' in SignatureStr[6:6+2]:
- if NoFSPI == True:
+ if NoFSPI == False:
TxtBody.append("#define FSPI_UPD_SIGNATURE
%s/* '%s' */\n\n" % (Item['value'], SignatureStr))
TxtBody.append("\n")
--
2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91957): https://edk2.groups.io/g/devel/message/91957
Mute This Topic: https://groups.io/mt/92683969/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] IntelFsp2Pkg: Fix GenCfgOpt bug for FSPI_UPD support.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
Fixed a logic bug in GenCfgOpt.py to skip FSPI_UPD when platforms
do not support.
Cc: Nate DeSimone
Cc: Star Zeng
Signed-off-by: Chasel Chiu
---
IntelFsp2Pkg/Tools/GenCfgOpt.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py b/IntelFsp2Pkg/Tools/GenCfgOpt.py
index 71c48f10e0..13be81ddbc 100644
--- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
+++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
@@ -1301,7 +1301,7 @@ EndList
elif '_S' in SignatureStr[6:6+2]:
TxtBody.append("#define FSPS_UPD_SIGNATURE %s
/* '%s' */\n\n" % (Item['value'], SignatureStr))
elif '_I' in SignatureStr[6:6+2]:
- if NoFSPI == True:
+ if NoFSPI == False:
TxtBody.append("#define FSPI_UPD_SIGNATURE
%s/* '%s' */\n\n" % (Item['value'], SignatureStr))
TxtBody.append("\n")
--
2.35.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91956): https://edk2.groups.io/g/devel/message/91956
Mute This Topic: https://groups.io/mt/92683969/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH v4] SecurityPkg: Add retry mechanism for tpm command
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980
As per TCG PC Client Device Driver Design Principle document,
if tpm commands fails due to timeout condition, then it should
have retry mechanism (3 retry attempts).
Existing implementation of PtpCrbTpmCommand does not have retry
mechanism if it fails with EFI_TIMEOUT.
See TCG PC Client Device Driver Design Principles for TPM 2.0
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1_r4_211104_final.pdf
Vision 1.1, Revision 0.04
Section 7.2.1
Signed-off-by: Qi Zhang
Cc: Jiewen Yao
Cc: Jian J Wang
Reviewed-by: Jiewen Yao
Tested-by: Swapnil Patil
---
.../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 108 +++---
1 file changed, 69 insertions(+), 39 deletions(-)
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 1d99beaa10..840265292a 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -33,6 +33,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
//
#define TPMCMDBUFLENGTH 0x500
+//
+// Max retry count according to Spec TCG PC Client Device Driver Design
Principles
+// for TPM2.0, Version 1.1, Revision 0.04, Section 7.2.1
+//
+#define RETRY_CNT_MAX 3
+
/**
Check whether TPM PTP register exist.
@@ -153,6 +159,7 @@ PtpCrbTpmCommand (
UINT32 TpmOutSize;
UINT16 Data16;
UINT32 Data32;
+ UINT8 RetryCnt;
DEBUG_CODE_BEGIN ();
UINTN DebugSize;
@@ -179,53 +186,76 @@ PtpCrbTpmCommand (
DEBUG_CODE_END ();
TpmOutSize = 0;
- //
- // STEP 0:
- // if CapCRbIdelByPass == 0, enforce Idle state before sending command
- //
- if ((GetCachedIdleByPass () == 0) && ((MmioRead32
((UINTN)>CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) ==
0)) {
+ RetryCnt = 0;
+ while (TRUE) {
+//
+// STEP 0:
+// if CapCRbIdelByPass == 0, enforce Idle state before sending command
+//
+if ((GetCachedIdleByPass () == 0) && ((MmioRead32
((UINTN)>CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) ==
0)) {
+ Status = PtpCrbWaitRegisterBits (
+ >CrbControlStatus,
+ PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ 0,
+ PTP_TIMEOUT_C
+ );
+ if (EFI_ERROR (Status)) {
+RetryCnt++;
+if (RetryCnt < RETRY_CNT_MAX) {
+ MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+ continue;
+} else {
+ //
+ // Try to goIdle to recover TPM
+ //
+ Status = EFI_DEVICE_ERROR;
+ goto GoIdle_Exit;
+}
+ }
+}
+
+//
+// STEP 1:
+// Ready is any time the TPM is ready to receive a command, following a
write
+// of 1 by software to Request.cmdReady, as indicated by the Status field
+// being cleared to 0.
+//
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
Status = PtpCrbWaitRegisterBits (
- >CrbControlStatus,
- PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ >CrbControlRequest,
0,
+ PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
PTP_TIMEOUT_C
);
if (EFI_ERROR (Status)) {
- //
- // Try to goIdle to recover TPM
- //
- Status = EFI_DEVICE_ERROR;
- goto GoIdle_Exit;
+ RetryCnt++;
+ if (RetryCnt < RETRY_CNT_MAX) {
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+continue;
+ } else {
+Status = EFI_DEVICE_ERROR;
+goto GoIdle_Exit;
+ }
}
- }
- //
- // STEP 1:
- // Ready is any time the TPM is ready to receive a command, following a write
- // of 1 by software to Request.cmdReady, as indicated by the Status field
- // being cleared to 0.
- //
- MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
- Status = PtpCrbWaitRegisterBits (
- >CrbControlRequest,
- 0,
- PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
- PTP_TIMEOUT_C
- );
- if (EFI_ERROR (Status)) {
-Status = EFI_DEVICE_ERROR;
-goto GoIdle_Exit;
- }
+Status = PtpCrbWaitRegisterBits (
+ >CrbControlStatus,
+ 0,
+ PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ PTP_TIMEOUT_C
+ );
+if (EFI_ERROR (Status)) {
+ RetryCnt++;
+ if (RetryCnt < RETRY_CNT_MAX) {
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+continue;
+ } else {
+Status = EFI_DEVICE_ERROR;
+goto GoIdle_Exit;
+ }
+}
- Status = PtpCrbWaitRegisterBits (
- >CrbControlStatus,
- 0,
- PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
-
Re: [edk2-devel] [edk2-staging][PATCH v3 15/15] edk2-staging/RedfishClientPkg: Introduce Bios feature driver
Thank you so much for your help to review my patch files. Will address comments
and push them to edk2-staging repo.
Nickle
-Original Message-
From: Chang, Abner
Sent: Thursday, July 28, 2022 11:48 AM
To: Wang, Nickle (Server BIOS) ; devel@edk2.groups.io
Cc: Yang, Atom ; Nick Ramirez
Subject: RE: [edk2-staging][PATCH v3 15/15] edk2-staging/RedfishClientPkg:
Introduce Bios feature driver
[AMD Official Use Only - General]
Reviewed-by: Abner Chang
> -Original Message-
> From: Nickle Wang
> Sent: Wednesday, July 27, 2022 9:38 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Yang, Atom
> ; Nick Ramirez
> Subject: [edk2-staging][PATCH v3 15/15] edk2-staging/RedfishClientPkg:
> Introduce Bios feature driver
>
> [CAUTION: External Email]
>
> Introduce new feature driver to support Bios version 1.0.9 schema.
> Update corresponding FDF and DSC file to enable this feature driver.
>
> Signed-off-by: Nickle Wang
> Cc: Abner Chang
> Cc: Yang Atom
> Cc: Nick Ramirez
> ---
> .../Features/Bios/v1_0_9/Common/BiosCommon.c | 741
>
> .../Features/Bios/v1_0_9/Common/BiosCommon.h | 30 +
> .../Features/Bios/v1_0_9/Dxe/BiosDxe.c| 789 ++
> .../Features/Bios/v1_0_9/Dxe/BiosDxe.inf | 52 ++
> RedfishClientPkg/RedfishClient.fdf.inc| 2 +
> .../RedfishClientComponents.dsc.inc | 2 +
> RedfishClientPkg/RedfishClientLibs.dsc.inc| 6 +-
> 7 files changed, 1621 insertions(+), 1 deletion(-)
> create mode 100644
> RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> create mode 100644
> RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.h
> create mode 100644 RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c
> create mode 100644
> RedfishClientPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.inf
>
> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> new file mode 100644
> index 00..d910d0d8a9
> --- /dev/null
> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
> @@ -0,0 +1,741 @@
> +/** @file
> + Redfish feature driver implementation - common functions
> +
> + (C) Copyright 2020-2022 Hewlett Packard Enterprise Development LP
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "BiosCommon.h"
> +
> +CHAR8 BiosEmptyJson[] = "{\"@odata.id\": \"\", \"@odata.type\":
> \"#Bios.v1_0_9.Bios\", \"Id\": \"\", \"Name\": \"\", \"Attributes\":{}}";
> +
> +REDFISH_RESOURCE_COMMON_PRIVATE *mRedfishResourcePrivate =
> NULL;
> +
> +/**
> + Consume resource from given URI.
> +
> + @param[in] ThisPointer to
> REDFISH_RESOURCE_COMMON_PRIVATE instance.
> + @param[in] JsonThe JSON to consume.
> + @param[in] HeaderEtag The Etag string returned in HTTP header.
> +
> + @retval EFI_SUCCESS Value is returned successfully.
> + @retval Others Some error happened.
> +
> +**/
> +EFI_STATUS
> +RedfishConsumeResourceCommon (
> + IN REDFISH_RESOURCE_COMMON_PRIVATE *Private,
> + IN CHAR8 *Json,
> + IN CHAR8 *HeaderEtag OPTIONAL
> + )
> +{
> + EFI_STATUS Status;
> + EFI_REDFISH_BIOS_V1_0_9 *Bios;
> + EFI_REDFISH_BIOS_V1_0_9_CS *BiosCs;
> + EFI_STRING ConfigureLang;
> + RedfishCS_Type_EmptyProp_CS_Data *EmptyPropCs;
> +
> +
> + if (Private == NULL || IS_EMPTY_STRING (Json)) {
> +return EFI_INVALID_PARAMETER;
> + }
> +
> + Bios = NULL;
> + BiosCs = NULL;
> + ConfigureLang = NULL;
> +
> + Status = Private->JsonStructProtocol->ToStructure (
> + Private->JsonStructProtocol,
> + NULL,
> + Json,
> + (EFI_REST_JSON_STRUCTURE_HEADER
> **)
> + );
> + if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a, ToStructure() failed: %r\n",
> __FUNCTION__, Status));
> +return Status;
> + }
> +
> + BiosCs = Bios->Bios;
> +
> + //
> + // Check ETAG to see if we need to consume it
> + //
> + if (CheckEtag (Private->Uri, HeaderEtag, BiosCs->odata_etag)) {
> +//
> +// No change
> +//
> +DEBUG ((DEBUG_INFO, "%a, ETAG: %s has no change, ignore consume
> action\n", __FUNCTION__, Private->Uri));
> +Status = EFI_ALREADY_STARTED;
> +goto ON_RELEASE;
> + }
> +
> + //
> + // Handle ATTRIBUTEREGISTRY
> + //
> + if (BiosCs->AttributeRegistry != NULL) {
> +//
> +// Find corresponding configure language for collection resource.
> +//
> +ConfigureLang = GetConfigureLang (BiosCs->odata_id,
> "AttributeRegistry");
> +if (ConfigureLang != NULL) {
> + Status = ApplyFeatureSettingsStringType (RESOURCE_SCHEMA,
> RESOURCE_SCHEMA_VERSION, ConfigureLang, BiosCs->AttributeRegistry);
> + if
[edk2-devel] [PATCH v2 1/1] OvmfPkg/QemuVideoDxe: Zero out PixelInformation in QueryMode
Ensure that the PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is zeroed out in
EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode() and
EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() when PixelFormat is
PixelBlueGreenRedReserved8BitPerColor.
According to UEFI 2.9 Section 12.9, PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is valid only if
PixelFormat is PixelBitMask. This means that firmware is not required
to fill out the PixelInformation field for other PixelFormat types,
which implies that the QemuVideoDxe implementation is technically
correct.
However, not zeroing out those fields will leak the contents of the
memory returned by the memory allocator, so it is better to explicitly
set them to zero.
In addition, the SCT test suite relies on PixelInformation always
having a consistent value, which causes failures.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Jordan Justen
Cc: Gerd Hoffmann
Cc: Jeff Booher-Kaeding
Cc: Samer El-Haj-Mahmoud
Cc: Sunny Wang
Cc: Jeremy Linton
Signed-off-by: Dimitrije Pavlov
Acked-by: Gerd Hoffmann
---
OvmfPkg/QemuVideoDxe/Gop.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 0c4dea7fb6f2..7a9fe208c99c 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -31,7 +31,14 @@ QemuVideoCompleteModeInfo (
Info->PixelInformation.ReservedMask = 0;
} else if (ModeData->ColorDepth == 32) {
DEBUG ((DEBUG_INFO, "PixelBlueGreenRedReserved8BitPerColor\n"));
-Info->PixelFormat = PixelBlueGreenRedReserved8BitPerColor;
+Info->PixelFormat =
PixelBlueGreenRedReserved8BitPerColor;
+Info->PixelInformation.RedMask = 0;
+Info->PixelInformation.GreenMask= 0;
+Info->PixelInformation.BlueMask = 0;
+Info->PixelInformation.ReservedMask = 0;
+ } else {
+DEBUG ((DEBUG_ERROR, "%a: Invalid ColorDepth %u", __FUNCTION__,
ModeData->ColorDepth));
+ASSERT (FALSE);
}
Info->PixelsPerScanLine = Info->HorizontalResolution;
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91953): https://edk2.groups.io/g/devel/message/91953
Mute This Topic: https://groups.io/mt/92679973/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/1] OvmfPkg/QemuVideoDxe: Zero out PixelInformation in QueryMode
Ensure that the PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is zeroed out in
EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode() and
EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() when PixelFormat is
PixelBlueGreenRedReserved8BitPerColor.
According to UEFI 2.9 Section 12.9, PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is valid only if
PixelFormat is PixelBitMask. This means that firmware is not required
to fill out the PixelInformation field for other PixelFormat types,
which implies that the QemuVideoDxe implementation is technically
correct.
However, not zeroing out those fields will leak the contents of the
memory returned by the memory allocator, so it is better to explicitly
set them to zero.
In addition, the SCT test suite relies on PixelInformation always
having a consistent value, which causes failures.
Cc: Ard Biesheuvel
Cc: Jiewen Yao
Cc: Jordan Justen
Cc: Gerd Hoffmann
Cc: Jeff Booher-Kaeding
Cc: Samer El-Haj-Mahmoud
Cc: Sunny Wang
Cc: Jeremy Linton
Signed-off-by: Dimitrije Pavlov
Acked-by: Gerd Hoffmann
---
OvmfPkg/QemuVideoDxe/Gop.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 0c4dea7fb6f2..7a9fe208c99c 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -31,7 +31,14 @@ QemuVideoCompleteModeInfo (
Info->PixelInformation.ReservedMask = 0;
} else if (ModeData->ColorDepth == 32) {
DEBUG ((DEBUG_INFO, "PixelBlueGreenRedReserved8BitPerColor\n"));
-Info->PixelFormat = PixelBlueGreenRedReserved8BitPerColor;
+Info->PixelFormat =
PixelBlueGreenRedReserved8BitPerColor;
+Info->PixelInformation.RedMask = 0;
+Info->PixelInformation.GreenMask= 0;
+Info->PixelInformation.BlueMask = 0;
+Info->PixelInformation.ReservedMask = 0;
+ } else {
+DEBUG ((DEBUG_ERROR, "%a: Invalid ColorDepth %u", __FUNCTION__,
ModeData->ColorDepth));
+ASSERT (FALSE);
}
Info->PixelsPerScanLine = Info->HorizontalResolution;
--
2.34.1
Re: [edk2-devel] [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow partial FreeBuffer
Adding Leif/Ard to CC incase they have any comments on this patch.
Thanks
Jeff
> -Original Message-
> From: Jeff Brasen
> Sent: Friday, June 17, 2022 9:39 AM
> To: devel@edk2.groups.io
> Cc: hao.a...@intel.com; ray...@intel.com
> Subject: RE: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe:
> Allow partial FreeBuffer
>
> Any thoughts on this patch?
>
> > -Original Message-
> > From: Jeff Brasen
> > Sent: Monday, February 14, 2022 11:46 AM
> > To: devel@edk2.groups.io
> > Cc: hao.a...@intel.com; ray...@intel.com; Jeff Brasen
> >
> > Subject: [PATCH v2] MdeModulePkg/NonDiscoverablePciDeviceDxe: Allow
> > partial FreeBuffer
> >
> > Add support for partial free of non cached buffers.
> > If a request for less than the full size is requested new allocations
> > for the remaining head and tail of the buffer are added to the list.
> > Added verification that Buffer is EFI_PAGE_SIZE aligned.
> > The XHCI driver does this if the page size for the controller is >4KB.
> >
> > Signed-off-by: Jeff Brasen
> > ---
> > .../NonDiscoverablePciDeviceIo.c | 53 ++-
> > 1 file changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> > index c1c5c6267c..77809cfedf 100644
> > ---
> >
> a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > PciDeviceIo.c
> > +++
> >
> b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverable
> > Pc
> > +++ iDeviceIo.c
> > @@ -960,12 +960,23 @@ NonCoherentPciIoFreeBuffer (
> >LIST_ENTRY *Entry;
> >EFI_STATUS Status;
> >NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
> > + NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *AllocHead;
> > + NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *AllocTail;
> >BOOLEAN Found;
> > + UINTNStartPages;
> > + UINTNEndPages;
> > +
> > + if (HostAddress != ALIGN_POINTER (HostAddress, EFI_PAGE_SIZE)) {
> > +ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> > +return EFI_INVALID_PARAMETER;
> > + }
> >
> >Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO (This);
> >
> >Found = FALSE;
> >Alloc = NULL;
> > + AllocHead = NULL;
> > + AllocTail = NULL;
> >
> >//
> >// Find the uncached allocation list entry associated @@ -976,9
> > +987,13 @@ NonCoherentPciIoFreeBuffer (
> > Entry = Entry->ForwardLink)
> >{
> > Alloc = BASE_CR (Entry,
> > NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION, List);
> > -if ((Alloc->HostAddress == HostAddress) && (Alloc->NumPages ==
> Pages))
> > {
> > +StartPages = 0;
> > +if (Alloc->HostAddress < HostAddress) {
> > + StartPages = (HostAddress - Alloc->HostAddress) / EFI_PAGE_SIZE;
> > +}
> > +if ((Alloc->HostAddress <= HostAddress) && (Alloc->NumPages >=
> > + (Pages + StartPages))) {
> >//
> > - // We are freeing the exact allocation we were given
> > + // We are freeing at least part of what we were given
> >// before by AllocateBuffer()
> >//
> >Found = TRUE;
> > @@ -991,7 +1006,41 @@ NonCoherentPciIoFreeBuffer (
> > return EFI_NOT_FOUND;
> >}
> >
> > + EndPages = Alloc->NumPages - (Pages + StartPages);
> > +
> > + if (StartPages != 0) {
> > +AllocHead = AllocatePool (sizeof *AllocHead);
> > +if (AllocHead == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > +}
> > +
> > +AllocHead->HostAddress = Alloc->HostAddress;
> > +AllocHead->NumPages = StartPages;
> > +AllocHead->Attributes = Alloc->Attributes; }
> > +
> > + if (EndPages != 0) {
> > +AllocTail = AllocatePool (sizeof *AllocTail);
> > +if (AllocTail == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > +}
> > +
> > +AllocTail->HostAddress = Alloc->HostAddress + ((Pages +
> > + StartPages) *
> > EFI_PAGE_SIZE);
> > +AllocTail->NumPages = EndPages;
> > +AllocTail->Attributes = Alloc->Attributes; }
> > +
> >RemoveEntryList (>List);
> > + //
> > + // Record this new sub allocations in the linked list, so we //
> > + can restore the memory space attributes later // if (AllocHead !=
> > + NULL) {
> > +InsertHeadList (>UncachedAllocationList, >List);
> > + } if (AllocTail != NULL) {
> > +InsertHeadList (>UncachedAllocationList, >List);
> > + }
> >
> >Status = gDS->SetMemorySpaceAttributes (
> >(EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress,
> > --
> > 2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91951): https://edk2.groups.io/g/devel/message/91951
Mute This Topic: https://groups.io/mt/89143704/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
Re: [edk2-devel] [edk2-platforms][PATCH v4 1/2] Ext4Pkg: Add symbolic links support
Looks very nice, tyvm. I did add a few more comments, but nothing critical at
all.
Reviewed-by: Marvin Häuser as-is or with my comments
addressed, either works.
> On 28. Jul 2022, at 17:26, Savva Mitrofanov wrote:
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677
>
> Provided support for symlink file type. Added routine which allows
> reading and following them through recursive open() call. As a security
> meausure implemented simple symlink loop check with nest level limit
> equal 8. Also this patch moves Ext4Open functionality to internal
> routine.
>
> Cc: Marvin Häuser
> Cc: Pedro Falcato
> Cc: Vitaly Cheptsov
> Signed-off-by: Savva Mitrofanov
> ---
> Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 13 +-
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 98 +-
> Features/Ext4Pkg/Ext4Dxe/File.c | 359 ++--
> Features/Ext4Pkg/Ext4Dxe/Inode.c| 53 +++
> 4 files changed, 485 insertions(+), 38 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> index a55cd2fa68ad..a73e3f8622f1 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
> @@ -171,7 +171,7 @@
> #define EXT4_DIRTY_FL 0x0100
> #define EXT4_COMPRBLK_FL 0x0200
> #define EXT4_NOCOMPR_FL 0x0400
> -#define EXT4_ECOMPR_FL0x0800
> +#define EXT4_ENCRYPT_FL 0x0800
> #define EXT4_BTREE_FL 0x1000
> #define EXT4_INDEX_FL 0x2000
> #define EXT4_JOURNAL_DATA_FL 0x4000
> @@ -332,11 +332,12 @@ STATIC_ASSERT (
> "ext4 block group descriptor struct has incorrect size"
> );
>
> -#define EXT4_DBLOCKS 12
> -#define EXT4_IND_BLOCK 12
> -#define EXT4_DIND_BLOCK 13
> -#define EXT4_TIND_BLOCK 14
> -#define EXT4_NR_BLOCKS 15
> +#define EXT4_DBLOCKS12
> +#define EXT4_IND_BLOCK 12
> +#define EXT4_DIND_BLOCK 13
> +#define EXT4_TIND_BLOCK 14
> +#define EXT4_NR_BLOCKS 15
> +#define EXT4_FAST_SYMLINK_MAX_SIZE EXT4_NR_BLOCKS * sizeof(UINT32)
>
> #define EXT4_GOOD_OLD_INODE_SIZE 128
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> index b1508482b0a7..c1df9d1149e4 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -31,7 +31,9 @@
>
> #include "Ext4Disk.h"
>
> +#define SYMLOOP_MAX8
> #define EXT4_NAME_MAX 255
> +#define EFI_PATH_MAX 4096
>
> #define EXT4_DRIVER_VERSION 0x
>
> @@ -324,11 +326,11 @@ number of read bytes.
> **/
> EFI_STATUS
> Ext4Read (
> - IN EXT4_PARTITION *Partition,
> - IN EXT4_FILE *File,
> - OUT VOID *Buffer,
> - IN UINT64 Offset,
> - IN OUT UINTN *Length
> + IN EXT4_PARTITION *Partition,
> + IN EXT4_FILE *File,
> + OUTVOID*Buffer,
> + IN UINT64 Offset,
> + IN OUT UINTN *Length
> );
>
> /**
> @@ -368,6 +370,7 @@ struct _Ext4File {
>
> UINT64OpenMode;
> UINT64Position;
> + UINT32SymLoops;
>
> EXT4_PARTITION*Partition;
>
> @@ -497,6 +500,45 @@ Ext4SetupFile (
> IN EXT4_PARTITION *Partition
> );
>
> +/**
> + Opens a new file relative to the source file's location.
> +
> + @param[out] FoundFile A pointer to the location to return the opened
> handle for the new
> + file.
> + @param[in] Source A pointer to the EXT4_FILE instance that is the file
> + handle to the source location. This would typically
> be an open
> + handle to a directory.
> + @param[in] FileName The Null-terminated string of the name of the file
> to be opened.
> + The file name may contain the following path
> modifiers: "\", ".",
> + and "..".
> + @param[in] OpenMode The mode to open the file. The only valid
> combinations that the
> + file may be opened with are: Read, Read/Write, or
> Create/Read/Write.
> + @param[in] Attributes Only valid for EFI_FILE_MODE_CREATE, in which case
> these are the
> + attribute bits for the newly created file.
> +
> + @retval EFI_SUCCESS The file was opened.
> + @retval EFI_NOT_FOUNDThe specified file could not be found on the
> device.
> + @retval EFI_NO_MEDIA The device has no medium.
> + @retval EFI_MEDIA_CHANGEDThe device has a different medium in it or
> the medium is no
> + longer supported.
> + @retval EFI_DEVICE_ERROR The device reported an error.
> + @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
> + @retval EFI_WRITE_PROTECTED An attempt was made to create a file, or open
> a file for write
> + when the media is write-protected.
> + @retval EFI_ACCESS_DENIEDThe service
Re: [edk2-devel] [edk2-platforms][PATCH v4 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE
Reviewed-by: Marvin Häuser
(Please just include this in future revisions, if any)
> On 28. Jul 2022, at 17:26, Savva Mitrofanov wrote:
>
> We shouldn't use direct casts, because in the future it could break
> the code, so using BASE_CR would be safe against possible structure
> changes and rearrangements
>
> Cc: Marvin Häuser
> Cc: Pedro Falcato
> Cc: Vitaly Cheptsov
> Signed-off-by: Savva Mitrofanov
> ---
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 2 ++
> Features/Ext4Pkg/Ext4Dxe/File.c| 16
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> index c1df9d1149e4..1e63e6282fa5 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -382,6 +382,8 @@ struct _Ext4File {
> EXT4_DENTRY *Dentry;
> };
>
> +#define EXT4_FILE_FROM_THIS(This) BASE_CR ((This), EXT4_FILE, Protocol)
> +
> #define EXT4_FILE_FROM_OPEN_FILES_NODE(Node)
> \
> BASE_CR(Node, EXT4_FILE, OpenFilesListNode)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
> index ae9230d6422b..b1744ad56c98 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/File.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/File.c
> @@ -562,7 +562,7 @@ Ext4Open (
> EXT4_FILE *FoundFile;
> EXT4_FILE *Source;
>
> - Source = (EXT4_FILE *)This;
> + Source = EXT4_FILE_FROM_THIS (This);
>
> //
> // Reset SymLoops counter
> @@ -599,7 +599,7 @@ Ext4Close (
> IN EFI_FILE_PROTOCOL *This
> )
> {
> - return Ext4CloseInternal ((EXT4_FILE *)This);
> + return Ext4CloseInternal (EXT4_FILE_FROM_THIS (This));
> }
>
> /**
> @@ -680,7 +680,7 @@ Ext4ReadFile (
> EXT4_PARTITION *Partition;
> EFI_STATUS Status;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
> Partition = File->Partition;
>
> ASSERT (Ext4FileIsOpenable (File));
> @@ -731,7 +731,7 @@ Ext4WriteFile (
> {
> EXT4_FILE *File;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
>
> if (!(File->OpenMode & EFI_FILE_MODE_WRITE)) {
> return EFI_ACCESS_DENIED;
> @@ -761,7 +761,7 @@ Ext4GetPosition (
> {
> EXT4_FILE *File;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
>
> if (Ext4FileIsDir (File)) {
> return EFI_UNSUPPORTED;
> @@ -794,7 +794,7 @@ Ext4SetPosition (
> {
> EXT4_FILE *File;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
>
> // Only seeks to 0 (so it resets the ReadDir operation) are allowed
> if (Ext4FileIsDir (File) && (Position != 0)) {
> @@ -1062,7 +1062,7 @@ Ext4GetInfo (
> EXT4_FILE *File;
> EXT4_PARTITION *Partition;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
> Partition = File->Partition;
>
> if (CompareGuid (InformationType, )) {
> @@ -1179,7 +1179,7 @@ Ext4SetInfo (
> EXT4_FILE *File;
> EXT4_PARTITION *Partition;
>
> - File = (EXT4_FILE *)This;
> + File = EXT4_FILE_FROM_THIS (This);
> Partition = File->Partition;
>
> if (Partition->ReadOnly) {
> --
> 2.37.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91949): https://edk2.groups.io/g/devel/message/91949
Mute This Topic: https://groups.io/mt/92672601/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms] [PATCH 1/1] Platform/Sgi: Add support to disable isolated cpus
the MADT table
+
+ Parse the IsolatedCpuInfo from the Hob list and updates the MADT table to
[SAMI] Nit. updates -> update
[Nishant] Will update in next patch version.
+ disable cpu's which are not available on the platfrom.
+
+ @param[in] AcpiHeader Points to the Madt table.
+ @param[in] HobData Points to the unusable cpuinfo in hoblist.
+**/
+STATIC
+VOID
+UpdateMadtTable (
+ IN EFI_ACPI_DESCRIPTION_HEADER *AcpiHeader,
+ IN SGI_PLATFORM_DESCRIPTOR *HobData
+ )
+{
+ UINT8 *StructureListHead;
+ UINT8 *StructureListTail;
+ EFI_ACPI_6_4_GIC_STRUCTURE *GicStructure;
+ BOOLEAN MpidPresent;
+
+ if (HobData->IsolatedCpuList.Count == 0) {
+return;
+ }
+
+ StructureListHead =
+((UINT8 *)AcpiHeader) +
+sizeof(EFI_ACPI_6_4_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER);
+ StructureListTail = (UINT8 *)AcpiHeader + AcpiHeader->Length;
+
+ // Locate ACPI GICC structure in the MADT table.
+ while (StructureListHead < StructureListTail) {
+if (StructureListHead[0] == EFI_ACPI_6_4_GIC) {
[SAMI] This is definitely not the way to parse an ACPI table. Please dont do
this.
Also, why are you not using DynamicTables framework? It is designed to handle
such cases.
[/SAMI]
[Nishant]
Could you please add more details on what is wrong with this approach?
[SAMI] The problem is it does not work with the ACPI table below:
[snip]
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20210930 (32-bit version)
* Copyright (c) 2000 - 2021 Intel Corporation
*
* Disassembly of apic.bin, Thu Jul 28 17:57:00 2022
*
* ACPI Data Table [APIC]
*
* Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue (in hex)
*/
[000h 4]Signature : "APIC"[Multiple APIC
Description Table (MADT)]
[004h 0004 4] Table Length : 05FC
[008h 0008 1] Revision : 05
[009h 0009 1] Checksum : 0B
[00Ah 0010 6] Oem ID : "ARMLTD"
[010h 0016 8] Oem Table ID : "ARMSGI "
[018h 0024 4] Oem Revision : 20220728
[01Ch 0028 4] Asl Compiler ID : "ARM "
[020h 0032 4]Asl Compiler Revision : 2999
[024h 0036 4] Local Apic Address :
[028h 0040 4]Flags (decoded below) :
PC-AT Compatibility : 0
[02Ch 0044 1]Subtable Type : 0B [Generic Interrupt Controller]
[02Dh 0045 1] Length : 50
[02Eh 0046 2] Reserved :
[030h 0048 4] CPU Interface Number :
[034h 0052 4]Processor UID :
[038h 0056 4]Flags (decoded below) : 0001
Processor Enabled : 1
Performance Interrupt Trigger Mode : 0
Virtual GIC Interrupt Trigger Mode : 0
[03Ch 0060 4] Parking Protocol Version :
[040h 0064 4]Performance Interrupt : 0017
[044h 0068 8] Parked Address :
[04Ch 0076 8] Base Address :
[054h 0084 8] Virtual GIC Base Address :
[05Ch 0092 8] Hypervisor GIC Base Address :
[064h 0100 4]Virtual GIC Interrupt : 0019
[068h 0104 8] Redistributor Base Address :
[070h 0112 8]ARM MPIDR :
[078h 0120 1] Efficiency Class : 00
[079h 0121 1] Reserved : 00
[07Ah 0122 2] SPE Overflow Interrupt :
[07Ch 0124 1]Subtable Type : 0B [Generic Interrupt Controller]
[07Dh 0125 1] Length : 50
[07Eh 0126 2] Reserved :
[080h 0128 4] CPU Interface Number :
[084h 0132 4]Processor UID : 0001
[088h 0136 4]Flags (decoded below) : 0001
Processor Enabled : 1
Performance Interrupt Trigger Mode : 0
Virtual GIC Interrupt Trigger Mode : 0
[08Ch 0140 4] Parking Protocol Version :
[090h 0144 4]Performance Interrupt : 0017
[094h 0148 8] Parked Address :
[09Ch 0156 8] Base Address :
[0A4h 0164 8] Virtual GIC Base Address :
[0ACh 0172 8] Hypervisor GIC Base Address :
[0B4h 0180 4]Virtual GIC Interrupt : 0019
[0B8h 0184 8] Redistributor Base Address :
[0C0h 0192 8]ARM MPIDR : 0001
[0C8h 0200 1] Efficiency Class : 00
[0C9h 0201 1] Reserved : 00
[0CAh 0202 2] SPE Overflow Interrupt :
[0CCh 0204 1]Subtable Type : 0B [Generic Interrupt Controller]
[0CDh 0205 1] Length : 50
[
Re: [edk2-devel] [PATCH v2] IntelFsp2Pkg: FSPI_UPD is not mandatory.
Patch merged:
https://github.com/tianocore/edk2/commit/3eca64f157c340f9bbf552d89a69698a3090c080
Thanks,
Chasel
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Chiu, Chasel
> Sent: Wednesday, July 27, 2022 10:15 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Desimone, Nathaniel L
> ; Zeng, Star
> Subject: [edk2-devel] [PATCH v2] IntelFsp2Pkg: FSPI_UPD is not mandatory.
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3993
> FSPI_UPD is required only When platforms implemented FSP_I component.
> Updated the scripts to allow FSPI_UPD not present scenario.
> Also fixed FSP_GLOBAL_DATA structure alignment issue and unnecessary non-
> backward compatibility change in previous FSP_I patch.
>
> Cc: Nate DeSimone
> Cc: Star Zeng
> Signed-off-by: Chasel Chiu
> ---
> IntelFsp2Pkg/Include/FspGlobalData.h | 43 +-
> -
> IntelFsp2Pkg/Tools/GenCfgOpt.py | 14 ++
> 2 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/IntelFsp2Pkg/Include/FspGlobalData.h
> b/IntelFsp2Pkg/Include/FspGlobalData.h
> index cf94f7b6a5..32c6d460e4 100644
> --- a/IntelFsp2Pkg/Include/FspGlobalData.h
> +++ b/IntelFsp2Pkg/Include/FspGlobalData.h
> @@ -42,58 +42,57 @@ typedef struct {
> #define FSP_PERFORMANCE_DATA_TIMER_MASK 0xFF typedef
> struct {- UINT32Signature;- UINT8 Version;- UINT8
> Reserved1[3];+
> UINT32 Signature;+ UINT8 Version;+ UINT8
>Reserved1[3];
> /// /// Offset 0x08 ///- UINTN CoreStack;- UINTN Reserved2;+
> UINTN
> CoreStack;+ VOID *SmmInitUpdPtr; /// /// IA32: Offset
> 0x10; X64:
> Offset 0x18 ///- UINT32StatusCode;- UINT8 ApiIdx;+ UINT32
> StatusCode;+ UINT8 ApiIdx; /// /// 0: FSP in API mode; 1:
> FSP in
> DISPATCH mode ///- UINT8 FspMode;- UINT8 OnSeparateStack;- UINT8
> Reserved3;- UINT32NumberOfPhases;- UINT32PhasesExecuted;- UINT32
> Reserved4[8];+ UINT8 FspMode;+ UINT8
> OnSeparateStack;+
> UINT8 Reserved2;+ UINT32 NumberOfPhases;+ UINT32
> PhasesExecuted;+ UINT32 Reserved3[8]; /// /// IA32: Offset
> 0x40; X64:
> Offset 0x48 /// Start of UINTN and pointer section- /// All UINTN and
> pointer
> members must be put in this section- /// except CoreStack and Reserved2. In
> addition, the number of- /// UINTN and pointer members must be even for
> natural alignment- /// in both IA32 and X64.+ /// All UINTN and pointer
> members are put in this section+ /// for maintaining natural alignment for
> both
> IA32 and X64 builds. /// FSP_PLAT_DATA PlatformData; VOID
> *TempRamInitUpdPtr; VOID *MemoryInitUpdPtr; VOID
> *SiliconInitUpdPtr;- VOID *SmmInitUpdPtr; ///- /// IA32:
> Offset 0x68;
> X64: Offset 0x98+ /// IA32: Offset 0x64; X64: Offset 0x90 /// To store
> function
> parameters pointer /// so it can be retrieved after stack switched. ///
> VOID
> *FunctionParameterPtr; FSP_INFO_HEADER*FspInfoHeader; VOID
> *UpdDataPtr;- UINTN Reserved5; /// /// End of UINTN and
> pointer
> section+ /// At this point, next field offset must be either *0h or *8h to+
> ///
> meet natural alignment requirement. ///- UINT8 Reserved6[16];+
> UINT8 Reserved4[16]; UINT32 PerfSig; UINT16
>PerfLen;-
> UINT16 Reserved7;+ UINT16 Reserved5; UINT32
> PerfIdx;+
> UINT32 Reserved6; UINT64 PerfData[32]; }
> FSP_GLOBAL_DATA;
> diff --git a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> index 128b896592..71c48f10e0 100644
> --- a/IntelFsp2Pkg/Tools/GenCfgOpt.py
> +++ b/IntelFsp2Pkg/Tools/GenCfgOpt.py
> @@ -959,8 +959,13 @@ EndList
> UpdTxtFile = '' FvDir = self._FvDir if
> GuidList[Index] not in
> self._MacroDict:-self.Error = "%s definition is missing in
> DSC file" %
> (GuidList[Index])-return 1+NoFSPI = False+
> if
> GuidList[Index] == 'FSP_I_UPD_TOOL_GUID':+NoFSPI = True+
> continue+else:+self.Error = "%s
> definition is missing in DSC
> file" % (GuidList[Index])+return 1 if
> UpdTxtFile == '':
> UpdTxtFile = os.path.join(FvDir, self._MacroDict[GuidList[Index]] + '.txt')@@
> -
> 1296,7 +1301,8 @@ EndList
> elif '_S' in SignatureStr[6:6+2]:
> TxtBody.append("#define
> FSPS_UPD_SIGNATURE %s/* '%s' */\n\n" % (Item['value'],
> SignatureStr))elif '_I' in SignatureStr[6:6+2]:-
> TxtBody.append("#define FSPI_UPD_SIGNATURE %s/* '%s'
> */\n\n"
> % (Item['value'], SignatureStr))+
Re: [edk2-devel] [Patch 1/1] Maintainers.txt: Add missing Github IDs for OvmfPkg TPM/TGC modules
On Wed, 27 Jul 2022 at 18:31, Michael D Kinney wrote: > > Cc: Ard Biesheuvel > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Gerd Hoffmann > Cc: Marc-André Lureau > Cc: Stefan Berger > Signed-off-by: Michael D Kinney Acked-by: Ard Biesheuvel > --- > Maintainers.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Maintainers.txt b/Maintainers.txt > index 071644c35612..a0d8b6968505 100644 > --- a/Maintainers.txt > +++ b/Maintainers.txt > @@ -495,8 +495,8 @@ F: OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > F: OvmfPkg/Library/Tcg2PhysicalPresenceLib*/ > F: OvmfPkg/PlatformPei/ClearCache.c > F: OvmfPkg/Tcg/ > -R: Marc-André Lureau > -R: Stefan Berger > +R: Marc-André Lureau [elmarco] > +R: Stefan Berger [stefanberger] > > OvmfPkg: Xen-related modules > F: OvmfPkg/Include/Guid/XenBusRootDevice.h > -- > 2.32.0.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91946): https://edk2.groups.io/g/devel/message/91946 Mute This Topic: https://groups.io/mt/92662539/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] VirtualKeyboardFeaturePkg: Pre OS virtual keyboard touch response are very slow with I2C touch panel
-Original Message-
From: Thangaraj, KalaiyarasanX
Sent: Wednesday, July 27, 2022 8:44 PM
To: devel@edk2.groups.io
Cc: Thangaraj, KalaiyarasanX ; Bi, Dandan
; Gao, Liming ; Pethaiyan,
Madhan ; Esakkithevar, Kathappan
; Balaji, Madhusudhan
Subject: [PATCH] VirtualKeyboardFeaturePkg: Pre OS virtual keyboard touch
response are very slow with I2C touch panel
On one Touch, multiple Reads happend and this reads varying based on Key Press
time.
Resulting in Multiple key press update on screen. This condition avoids
KeyPressed skips resulting due to faster key press and update only on valid key
press.
Cc: Dandan Bi
Cc: Liming Gao
Cc: Madhan Pethaiyan
Cc: Kathappan Esakkithevar
Cc: Madhusudhan Balaji
Signed-off-by: KalaiyarasanX Thangaraj
Reviewed-by: Madhusudhan Balaji
---
Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/Keyboard.c
| 12
Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/VirtualKeyboard.h
| 3 +++
2 files changed, 15 insertions(+)
diff --git
a/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/Keyboard.c
b/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/Keyboard.c
index 2b1216746b..aa4bfe3baa 100644
---
a/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/Keyboard.c
+++ b/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyb
+++ oardDxe/Keyboard.c
@@ -512,6 +512,18 @@ VkTimer (
if (!VkContext->TouchActive) {
VkContext->KeyPressed = FALSE;
}
+
+ //
+ // On one Touch, multiple Reads happend and this reads varying based on Key
Press time.
+ // Resulting in Multiple key press update on screen. This condition
+ avoids Key Press skips // resulting due to faster key press and update only
on valid key press.
+ //
+ if ((Point.CurrentX != PreviousX) || (Point.CurrentY != PreviousY)) {
+ VkContext->KeyPressed = FALSE;
+ }
+ PreviousX = Point.CurrentX;
+ PreviousY = Point.CurrentY;
+
ConvertCoordinate (VkContext, Point, , );
if (!VkContext->KeyPressed &&
diff --git
a/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/VirtualKeyboard.h
b/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/VirtualKeyboard.h
index 14a50fa5af..11e045f894 100644
---
a/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyboardDxe/VirtualKeyboard.h
+++ b/Features/Intel/UserInterface/VirtualKeyboardFeaturePkg/VirtualKeyb
+++ oardDxe/VirtualKeyboard.h
@@ -37,6 +37,9 @@ extern EFI_DRIVER_BINDING_PROTOCOL
gVirtualKeyboardDriverBinding;
extern EFI_COMPONENT_NAME_PROTOCOL gVirtualKeyboardComponentName;
extern EFI_COMPONENT_NAME2_PROTOCOL gVirtualKeyboardComponentName2;
+GLOBAL_REMOVE_IF_UNREFERENCED UINTN PreviousX;
+GLOBAL_REMOVE_IF_UNREFERENCED UINTN PreviousY;
+
///
/// Debug raw data points
///
--
2.26.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91945): https://edk2.groups.io/g/devel/message/91945
Mute This Topic: https://groups.io/mt/92651285/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH] MdePkg:Improved Smbios Type9 table and Smbios 3.5.0 spec changes
In Type9 structure since PeerGroups has a variable
number of entries, must not define new fields in the structure.So added an
extended structure and defined new fields added after PeerGroups. Also done
some improvements to Smbios 3.5.0 spec changes.
Signed-off-by:
sainadh nagolu
---
MdePkg/Include/IndustryStandard/SmBios.h | 62 +++-
1 file changed, 35 insertions(+), 27 deletions(-)
diff --git a/MdePkg/Include/IndustryStandard/SmBios.h
b/MdePkg/Include/IndustryStandard/SmBios.h
index c7a4971f14..f62ad7fa4d 100644
--- a/MdePkg/Include/IndustryStandard/SmBios.h
+++ b/MdePkg/Include/IndustryStandard/SmBios.h
@@ -1503,6 +1503,17 @@ typedef struct {
UINT8 DataBusWidth;
UINT8 PeerGroupingCount;
MISC_SLOT_PEER_GROUP PeerGroups[1];
+ //
+ // Since PeerGroups has a variable number of entries, must not define new
+ // fields in the structure. Remaining fields can be referenced using
+ // SMBIOS_TABLE_TYPE9_EXTENDED structure
+ //
+} SMBIOS_TABLE_TYPE9;
+
+///
+/// Extended structure for System Slots (Type 9)
+///
+typedef struct {
//
// Add for smbios 3.4
//
@@ -1513,7 +1524,7 @@ typedef struct {
// Add for smbios 3.5
//
UINT8 SlotHeight; ///< The enumeration
value from MISC_SLOT_HEIGHT.
-} SMBIOS_TABLE_TYPE9;
+} SMBIOS_TABLE_TYPE9_EXTENDED;
///
/// On Board Devices Information - Device Types.
@@ -2746,11 +2757,11 @@ typedef enum {
///
/// Firmware Inventory Firmware Characteristics (Type 45).
///
-typedef enum {
- CharacteristicsUpdatable = 0x00,
- CharacteristicsWriteProtected = 0x01,
- CharacteristicsReserved = 0x02/// 0x02 - 0x0F are reserved
-} FIRMWARE_INVENTORY_CHARACTERISTICS;
+typedef struct {
+ UINT16Updatable :1;
+ UINT16WriteProtected :1;
+ UINT16Reserved :14;
+} FIRMWARE_CHARACTERISTICS;
///
/// Firmware Inventory State Information (Type 45).
@@ -2763,7 +2774,7 @@ typedef enum {
FirmwareInventoryStateAbsent = 0x05,
FirmwareInventoryStateStandbyOffline = 0x06,
FirmwareInventoryStateStandbySpare = 0x07,
- FirmwareInventoryStateUnavailableOffline = 0x08,
+ FirmwareInventoryStateUnavailableOffline = 0x08
} FIRMWARE_INVENTORY_STATE;
///
@@ -2780,21 +2791,19 @@ typedef enum {
/// One Type 45 structure is provided for each firmware component.
///
typedef struct {
- SMBIOS_STRUCTUREHdr;
- SMBIOS_HANDLE RefHandle;
-
- UINT8 FirmwareComponentName;
- UINT8 FirmwareVersion;
- UINT8 FirmwareVersionFormat;///< The enumeration value
from FIRMWARE_INVENTORY_VERSION_FORMAT_TYPE
- UINT8 FirmwareId;
- UINT8 FirmwareIdFormat;
- UINT8 ReleaseDate;
- UINT8 Manufacturer;
- UINT8 LowestSupportedVersion;
- UINT64 ImageSize;
- UINT32 Characteristics;
- UINT8 State;
- UINT8 AssociatedComponentCount;
+ SMBIOS_STRUCTURE Hdr;
+ SMBIOS_TABLE_STRING FirmwareComponentName;
+ SMBIOS_TABLE_STRING FirmwareVersion;
+ UINT8 FirmwareVersionFormat;///< The enumeration
value from FIRMWARE_INVENTORY_VERSION_FORMAT_TYPE
+ SMBIOS_TABLE_STRING FirmwareId;
+ UINT8 FirmwareIdFormat; ///< The enumeration
value from FIRMWARE_INVENTORY_FIRMWARE_ID_FORMAT_TYPE.
+ SMBIOS_TABLE_STRING ReleaseDate;
+ SMBIOS_TABLE_STRING Manufacturer;
+ SMBIOS_TABLE_STRING LowestSupportedVersion;
+ UINT64ImageSize;
+ FIRMWARE_CHARACTERISTICS Characteristics;
+ UINT8 State;///< The enumeration
value from FIRMWARE_INVENTORY_STATE.
+ UINT8 AssociatedComponentCount;
///
/// zero or n-number of handles depends on AssociatedComponentCount
/// handles are of type SMBIOS_HANDLE
@@ -2820,11 +2829,10 @@ typedef enum {
/// parent structure.
///
typedef struct {
- SMBIOS_STRUCTUREHdr;
- SMBIOS_HANDLE RefHandle;
- UINT16 StringPropertyId;
- UINT8 StringPropertyValue;
- SMBIOS_HANDLE ParentHandle;
+ SMBIOS_STRUCTURE Hdr;
+ UINT16 StringPropertyId; ///< The enumeration value
from STRING_PROPERTY_ID.
+ SMBIOS_TABLE_STRINGStringPropertyValue;
+ SMBIOS_HANDLE ParentHandle;
} SMBIOS_TABLE_TYPE46;
///
--
2.36.0.windows.1
-The information contained in this message may be confidential and proprietary
to American Megatrends (AMI). This communication is intended to be read only by
the individual or entity to whom it is addressed or by their designee. If the
reader of this message is not the intended recipient, you are on notice that
any distribution of this message,
[edk2-devel] [edk2-platforms][PATCH v4 2/2] Ext4Pkg: Add base containing record macro for EXT4_FILE
We shouldn't use direct casts, because in the future it could break
the code, so using BASE_CR would be safe against possible structure
changes and rearrangements
Cc: Marvin Häuser
Cc: Pedro Falcato
Cc: Vitaly Cheptsov
Signed-off-by: Savva Mitrofanov
---
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 2 ++
Features/Ext4Pkg/Ext4Dxe/File.c| 16
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index c1df9d1149e4..1e63e6282fa5 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -382,6 +382,8 @@ struct _Ext4File {
EXT4_DENTRY *Dentry;
};
+#define EXT4_FILE_FROM_THIS(This) BASE_CR ((This), EXT4_FILE, Protocol)
+
#define EXT4_FILE_FROM_OPEN_FILES_NODE(Node)
\
BASE_CR(Node, EXT4_FILE, OpenFilesListNode)
diff --git a/Features/Ext4Pkg/Ext4Dxe/File.c b/Features/Ext4Pkg/Ext4Dxe/File.c
index ae9230d6422b..b1744ad56c98 100644
--- a/Features/Ext4Pkg/Ext4Dxe/File.c
+++ b/Features/Ext4Pkg/Ext4Dxe/File.c
@@ -562,7 +562,7 @@ Ext4Open (
EXT4_FILE *FoundFile;
EXT4_FILE *Source;
- Source = (EXT4_FILE *)This;
+ Source = EXT4_FILE_FROM_THIS (This);
//
// Reset SymLoops counter
@@ -599,7 +599,7 @@ Ext4Close (
IN EFI_FILE_PROTOCOL *This
)
{
- return Ext4CloseInternal ((EXT4_FILE *)This);
+ return Ext4CloseInternal (EXT4_FILE_FROM_THIS (This));
}
/**
@@ -680,7 +680,7 @@ Ext4ReadFile (
EXT4_PARTITION *Partition;
EFI_STATUS Status;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
Partition = File->Partition;
ASSERT (Ext4FileIsOpenable (File));
@@ -731,7 +731,7 @@ Ext4WriteFile (
{
EXT4_FILE *File;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
if (!(File->OpenMode & EFI_FILE_MODE_WRITE)) {
return EFI_ACCESS_DENIED;
@@ -761,7 +761,7 @@ Ext4GetPosition (
{
EXT4_FILE *File;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
if (Ext4FileIsDir (File)) {
return EFI_UNSUPPORTED;
@@ -794,7 +794,7 @@ Ext4SetPosition (
{
EXT4_FILE *File;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
// Only seeks to 0 (so it resets the ReadDir operation) are allowed
if (Ext4FileIsDir (File) && (Position != 0)) {
@@ -1062,7 +1062,7 @@ Ext4GetInfo (
EXT4_FILE *File;
EXT4_PARTITION *Partition;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
Partition = File->Partition;
if (CompareGuid (InformationType, )) {
@@ -1179,7 +1179,7 @@ Ext4SetInfo (
EXT4_FILE *File;
EXT4_PARTITION *Partition;
- File = (EXT4_FILE *)This;
+ File = EXT4_FILE_FROM_THIS (This);
Partition = File->Partition;
if (Partition->ReadOnly) {
--
2.37.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91943): https://edk2.groups.io/g/devel/message/91943
Mute This Topic: https://groups.io/mt/92672601/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-platforms][PATCH v4 1/2] Ext4Pkg: Add symbolic links support
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677
Provided support for symlink file type. Added routine which allows
reading and following them through recursive open() call. As a security
meausure implemented simple symlink loop check with nest level limit
equal 8. Also this patch moves Ext4Open functionality to internal
routine.
Cc: Marvin Häuser
Cc: Pedro Falcato
Cc: Vitaly Cheptsov
Signed-off-by: Savva Mitrofanov
---
Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 13 +-
Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 98 +-
Features/Ext4Pkg/Ext4Dxe/File.c | 359 ++--
Features/Ext4Pkg/Ext4Dxe/Inode.c| 53 +++
4 files changed, 485 insertions(+), 38 deletions(-)
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
index a55cd2fa68ad..a73e3f8622f1 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h
@@ -171,7 +171,7 @@
#define EXT4_DIRTY_FL 0x0100
#define EXT4_COMPRBLK_FL 0x0200
#define EXT4_NOCOMPR_FL 0x0400
-#define EXT4_ECOMPR_FL0x0800
+#define EXT4_ENCRYPT_FL 0x0800
#define EXT4_BTREE_FL 0x1000
#define EXT4_INDEX_FL 0x2000
#define EXT4_JOURNAL_DATA_FL 0x4000
@@ -332,11 +332,12 @@ STATIC_ASSERT (
"ext4 block group descriptor struct has incorrect size"
);
-#define EXT4_DBLOCKS 12
-#define EXT4_IND_BLOCK 12
-#define EXT4_DIND_BLOCK 13
-#define EXT4_TIND_BLOCK 14
-#define EXT4_NR_BLOCKS 15
+#define EXT4_DBLOCKS12
+#define EXT4_IND_BLOCK 12
+#define EXT4_DIND_BLOCK 13
+#define EXT4_TIND_BLOCK 14
+#define EXT4_NR_BLOCKS 15
+#define EXT4_FAST_SYMLINK_MAX_SIZE EXT4_NR_BLOCKS * sizeof(UINT32)
#define EXT4_GOOD_OLD_INODE_SIZE 128
diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
index b1508482b0a7..c1df9d1149e4 100644
--- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
+++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
@@ -31,7 +31,9 @@
#include "Ext4Disk.h"
+#define SYMLOOP_MAX8
#define EXT4_NAME_MAX 255
+#define EFI_PATH_MAX 4096
#define EXT4_DRIVER_VERSION 0x
@@ -324,11 +326,11 @@ number of read bytes.
**/
EFI_STATUS
Ext4Read (
- IN EXT4_PARTITION *Partition,
- IN EXT4_FILE *File,
- OUT VOID *Buffer,
- IN UINT64 Offset,
- IN OUT UINTN *Length
+ IN EXT4_PARTITION *Partition,
+ IN EXT4_FILE *File,
+ OUTVOID*Buffer,
+ IN UINT64 Offset,
+ IN OUT UINTN *Length
);
/**
@@ -368,6 +370,7 @@ struct _Ext4File {
UINT64OpenMode;
UINT64Position;
+ UINT32SymLoops;
EXT4_PARTITION*Partition;
@@ -497,6 +500,45 @@ Ext4SetupFile (
IN EXT4_PARTITION *Partition
);
+/**
+ Opens a new file relative to the source file's location.
+
+ @param[out] FoundFile A pointer to the location to return the opened handle
for the new
+ file.
+ @param[in] Source A pointer to the EXT4_FILE instance that is the file
+ handle to the source location. This would typically
be an open
+ handle to a directory.
+ @param[in] FileName The Null-terminated string of the name of the file to
be opened.
+ The file name may contain the following path
modifiers: "\", ".",
+ and "..".
+ @param[in] OpenMode The mode to open the file. The only valid
combinations that the
+ file may be opened with are: Read, Read/Write, or
Create/Read/Write.
+ @param[in] Attributes Only valid for EFI_FILE_MODE_CREATE, in which case
these are the
+ attribute bits for the newly created file.
+
+ @retval EFI_SUCCESS The file was opened.
+ @retval EFI_NOT_FOUNDThe specified file could not be found on the
device.
+ @retval EFI_NO_MEDIA The device has no medium.
+ @retval EFI_MEDIA_CHANGEDThe device has a different medium in it or the
medium is no
+ longer supported.
+ @retval EFI_DEVICE_ERROR The device reported an error.
+ @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
+ @retval EFI_WRITE_PROTECTED An attempt was made to create a file, or open a
file for write
+ when the media is write-protected.
+ @retval EFI_ACCESS_DENIEDThe service denied access to the file.
+ @retval EFI_OUT_OF_RESOURCES Not enough resources were available to open the
file.
+ @retval EFI_VOLUME_FULL The volume is full.
+
+**/
+EFI_STATUS
+Ext4OpenInternal (
+ OUT EXT4_FILE **FoundFile,
+ IN EXT4_FILE *Source,
+ IN CHAR16 *FileName,
+ IN UINT64 OpenMode,
+ IN UINT64 Attributes
+
[edk2-devel] [edk2-platforms][PATCH v4 0/2] Ext4Pkg: Add Symbolic Links support
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3677 Hi all, In the fourth version I corrected SymlinkSize selection logic in Ext4ReadFastSymlink(), previously fast-symlink's EXT4_INODE_SIZE is not necessarily validated when we checked it in Ext4SymlinkIsFastSymlink(), so we should truncate if necessary. Also I corrected MSVC compiler warning by assigning ExtAttrBlocks to UINT32 type. This patchset adds symbolic links support with simple recursion protection based on symbolic link nest level limitation, also I included patch which adds BASE_CR to extract EXT4_FILE private structure to prevent possible code corruption caused by structure changes and rearrangements in future. REF: https://github.com/savvamitrofanov/edk2-platforms/tree/ext4pkg_symlink_support Cc: Marvin Häuser Cc: Pedro Falcato Cc: Vitaly Cheptsov Savva Mitrofanov (2): Ext4Pkg: Add symbolic links support Ext4Pkg: Add base containing record macro for EXT4_FILE Features/Ext4Pkg/Ext4Dxe/Ext4Disk.h | 13 +- Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 100 +- Features/Ext4Pkg/Ext4Dxe/File.c | 369 ++-- Features/Ext4Pkg/Ext4Dxe/Inode.c| 53 +++ 4 files changed, 492 insertions(+), 43 deletions(-) -- 2.37.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#91941): https://edk2.groups.io/g/devel/message/91941 Mute This Topic: https://groups.io/mt/92672599/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 5/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added function to reserve ECAM space
Hello Kun,
With the changes below:
Reviewed-by: Pierre Gondois
Thanks!
On 7/28/22 06:31, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
Certain OSes will complain if the ECAM config space is not reserved in
the ACPI namespace.
This change adds a function to reserve PNP motherboard resources for a
given PCI node.
Co-authored-by: Joe Lopez
Signed-off-by: Kun Qin
---
Notes:
v2:
- Only create RES0 after config space checking [Pierre]
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c |
169
1 file changed, 169 insertions(+)
diff --git
a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
index ceffe2838c03..c03550baabf2 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
@@ -616,6 +616,167 @@ GeneratePciCrs (
return Status;
}
+/** Generate a Pci Resource Template to hold Address Space Info
+
+ @param [in] PciNode RootNode of the AML tree.
+ @param [in, out] CrsNode CRS node of the AML tree to populate.
I think CrsNode is an 'out' only parameter. Also the description of PciNode
seems incorrect.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid input parameter.
+ @retval EFI_OUT_OF_RESOURCESCould not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+PopulateBasicPciResObjects (
Would it be possible to rename it:
GenerateMotherboardDevice()
or with a name related to 'motherboard' ?
+ INAML_OBJECT_NODE_HANDLE PciNode,
+ IN OUT AML_OBJECT_NODE_HANDLE *CrsNode
+ )
+{
+ EFI_STATUS Status;
+ UINT32 EisaId;
+ AML_OBJECT_NODE_HANDLE ResNode;
+
+ if (CrsNode == NULL) {
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+ }
+
+ // ASL: Device (PCIx) {}
+ Status = AmlCodeGenDevice ("RES0", PciNode, );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ // ASL: Name (_HID, EISAID ("PNP0C02"))
+ Status = AmlGetEisaIdFromString ("PNP0C02", ); /* PNP Motherboard
Resources */
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ Status = AmlCodeGenNameInteger ("_HID", EisaId, ResNode, NULL);
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ // ASL: Name (_CRS, ResourceTemplate () {})
+ Status = AmlCodeGenNameResourceTemplate ("_CRS", ResNode, CrsNode);
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ return Status;
+}
+
+/** Generate a Pci Resource Template to hold Address Space Info
+
+ @param [in] Generator The SSDT Pci generator.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+Protocol interface.
+ @param [in] PciInfo Pci device information.
+ @param [in, out] PciNode RootNode of the AML tree to populate.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCESCould not allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+GeneratePciRes (
Would it be possible to rename it:
ReserveEcamSpace()
or with a name related to 'ecam' ?
+ INACPI_PCI_GENERATOR*Generator,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
+ IN CONST CM_ARM_PCI_CONFIG_SPACE_INFO *PciInfo,
+ IN OUT AML_OBJECT_NODE_HANDLEPciNode
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE_HANDLE CrsNode;
+ BOOLEAN Translation;
+ UINT32 Index;
+ CM_ARM_OBJ_REF *RefInfo;
+ UINT32 RefCount;
+ CM_ARM_PCI_ADDRESS_MAP_INFO *AddrMapInfo;
+ BOOLEAN IsPosDecode;
+
+ // Get the array of CM_ARM_OBJ_REF referencing the
+ // CM_ARM_PCI_ADDRESS_MAP_INFO objects.
+ Status = GetEArmObjCmRef (
+ CfgMgrProtocol,
+ PciInfo->AddressMapToken,
+ ,
+
+ );
+ if (EFI_ERROR (Status)) {
+ASSERT (0);
+return Status;
+ }
+
+ for (Index = 0; Index < RefCount; Index++) {
+// Get CM_ARM_PCI_ADDRESS_MAP_INFO structures one by one.
+Status = GetEArmObjPciAddressMapInfo (
+ CfgMgrProtocol,
+ RefInfo[Index].ReferenceToken,
+ ,
+ NULL
+ );
+if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+}
+
+Translation = (AddrMapInfo->CpuAddress != AddrMapInfo->PciAddress);
+if (AddrMapInfo->CpuAddress >= AddrMapInfo->PciAddress) {
+ IsPosDecode = TRUE;
+} else {
+ IsPosDecode =
Re: [edk2-devel] [PATCH v2 4/6] DynamicTablesPkg: DynamicTableManagerDxe: Added check for installed tables
Hello Kun,
With the changes below:
Reviewed-by: Pierre Gondois
Thanks!
On 7/28/22 06:31, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3997
This change added an extra step to allow check for installed ACPI tables.
For FADT, MADT, GTDT, DSDT, DBG2 and SPCR tables, either pre-installed or
supplied through AcpiTableInfo can be accepted.
An extra check for FADT ACPI table existence during installation step is
also added.
Cc: Sami Mujawar
Cc: Alexei Fedorov
Co-authored-by: Joe Lopez
Signed-off-by: Kun Qin
---
Notes:
v2:
- Function description updates [Sami]
- Refactorized the table verification [Pierre]
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c |
182 +++-
DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.inf |
1 +
2 files changed, 103 insertions(+), 80 deletions(-)
diff --git
a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
index ed62299f9bbd..4ad7c0c8dbfa 100644
--- a/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
+++ b/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
// Module specific include files.
@@ -22,6 +23,29 @@
#include
#include
+#define ACPI_TABLE_PRESENT_INFO_LIST BIT0
+#define ACPI_TABLE_PRESENT_INSTALLED BIT1
+
+#define ACPI_TABLE_VERIFY_FADT 0
+#define ACPI_TABLE_VERIFY_COUNT 6
+
+typedef struct {
+ ESTD_ACPI_TABLE_IDEstdTableId;
+ UINT32AcpiTableSignature;
+ CHAR8 AcpiTableName[sizeof (UINT32) + 1];
+ BOOLEAN IsMandatory;
+ UINT16Presence;
+} ACPI_TABLE_PRESENCE_INFO;
I think it needs some documentation (also for mAcpiVerifyTables).
+
+ACPI_TABLE_PRESENCE_INFO mAcpiVerifyTables[ACPI_TABLE_VERIFY_COUNT] = {
+ { EStdAcpiTableIdFadt, EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
"FADT", TRUE, 0 },
+ { EStdAcpiTableIdMadt, EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
"MADT", TRUE, 0 },
+ { EStdAcpiTableIdGtdt, EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
"GTDT", TRUE, 0 },
+ { EStdAcpiTableIdDsdt, EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
"DSDT", TRUE, 0 },
+ { EStdAcpiTableIdDbg2, EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE,
"DBG2", FALSE, 0 },
+ { EStdAcpiTableIdSpcr, EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE,
"SPCR", FALSE, 0 },
+};
+
/** This macro expands to a function that retrieves the ACPI Table
List from the Configuration Manager.
*/
@@ -395,6 +419,7 @@ BuildAndInstallAcpiTable (
@retval EFI_SUCCESS Success.
@retval EFI_NOT_FOUND If mandatory table is not found.
+ @retval EFI_ALREADY_STARTED If mandatory table found in AcpiTableInfo is
already installed.
**/
STATIC
EFI_STATUS
@@ -404,75 +429,68 @@ VerifyMandatoryTablesArePresent (
IN UINT32 AcpiTableCount
)
{
- EFI_STATUS Status;
- BOOLEAN FadtFound;
- BOOLEAN MadtFound;
- BOOLEAN GtdtFound;
- BOOLEAN DsdtFound;
- BOOLEAN Dbg2Found;
- BOOLEAN SpcrFound;
+ EFI_STATUS Status;
+ UINTNHandle;
+ UINTNIndex;
+ UINTNInstalledTableIndex;
+ EFI_ACPI_DESCRIPTION_HEADER *DescHeader;
+ EFI_ACPI_TABLE_VERSION Version;
+ EFI_ACPI_SDT_PROTOCOL*AcpiSdt;
- Status= EFI_SUCCESS;
- FadtFound = FALSE;
- MadtFound = FALSE;
- GtdtFound = FALSE;
- DsdtFound = FALSE;
- Dbg2Found = FALSE;
- SpcrFound = FALSE;
ASSERT (AcpiTableInfo != NULL);
+ Status = EFI_SUCCESS;
+
+ // Check against the statically initialized ACPI tables to see if they are
in ACPI info list
while (AcpiTableCount-- != 0) {
-switch (AcpiTableInfo[AcpiTableCount].AcpiTableSignature) {
- case EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE:
-FadtFound = TRUE;
-break;
- case EFI_ACPI_6_2_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE:
-MadtFound = TRUE;
-break;
- case EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE:
-GtdtFound = TRUE;
-break;
- case EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE:
-DsdtFound = TRUE;
-break;
- case EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE:
-Dbg2Found = TRUE;
-break;
- case EFI_ACPI_6_2_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_SIGNATURE:
-SpcrFound = TRUE;
-break;
- default:
-break;
+for (Index = 0; Index < ACPI_TABLE_VERIFY_COUNT; Index++) {
+ if (AcpiTableInfo[AcpiTableCount].AcpiTableSignature ==
Re: [edk2-devel] [PATCH v3] SecurityPkg: Add retry mechanism for tpm command
Hey
Please add the spec related info to the code as comments.
Thank you
Yao, Jiewen
> -Original Message-
> From: Zhang, Qi1
> Sent: Thursday, July 28, 2022 4:51 PM
> To: devel@edk2.groups.io
> Cc: Zhang, Qi1 ; Yao, Jiewen ;
> Wang, Jian J ; Patil, Swapnil
>
> Subject: [PATCH v3] SecurityPkg: Add retry mechanism for tpm command
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980
>
> As per TCG PC Client Device Driver Design Principle document,
> if tpm commands fails due to timeout condition, then it should
> have retry mechanism (3 retry attempts).
> Existing implementation of PtpCrbTpmCommand does not have retry
> mechanism if it fails with EFI_TIMEOUT.
>
> See TCG PC Client Device Driver Design Principles for TPM 2.0
> https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1
> _r4_211104_final.pdf
> Vision 1.1, Revision 0.04
> Section 7.2.1
>
> Signed-off-by: Qi Zhang
> Cc: Jiewen Yao
> Cc: Jian J Wang
> Reviewed-by: Jiewen Yao
> Tested-by: Swapnil Patil
> ---
> .../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 107 +++---
> 1 file changed, 68 insertions(+), 39 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> index 1d99beaa10..6b5994fde2 100644
> --- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> +++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
> @@ -33,6 +33,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> //
>
> #define TPMCMDBUFLENGTH 0x500
>
>
>
> +//
>
> +// Max retry count
>
> +//
>
> +#define RETRY_CNT_MAX 3
>
> +
>
> /**
>
>Check whether TPM PTP register exist.
>
>
>
> @@ -153,6 +158,7 @@ PtpCrbTpmCommand (
>UINT32 TpmOutSize;
>
>UINT16 Data16;
>
>UINT32 Data32;
>
> + UINT8 RetryCnt;
>
>
>
>DEBUG_CODE_BEGIN ();
>
>UINTN DebugSize;
>
> @@ -179,53 +185,76 @@ PtpCrbTpmCommand (
>DEBUG_CODE_END ();
>
>TpmOutSize = 0;
>
>
>
> - //
>
> - // STEP 0:
>
> - // if CapCRbIdelByPass == 0, enforce Idle state before sending command
>
> - //
>
> - if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)
> >CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
>
> + RetryCnt = 0;
>
> + while (TRUE) {
>
> +//
>
> +// STEP 0:
>
> +// if CapCRbIdelByPass == 0, enforce Idle state before sending command
>
> +//
>
> +if ((GetCachedIdleByPass () == 0) && ((MmioRead32 ((UINTN)
> >CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0)) {
>
> + Status = PtpCrbWaitRegisterBits (
>
> + >CrbControlStatus,
>
> + PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>
> + 0,
>
> + PTP_TIMEOUT_C
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> +RetryCnt++;
>
> +if (RetryCnt < RETRY_CNT_MAX) {
>
> + MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
>
> + continue;
>
> +} else {
>
> + //
>
> + // Try to goIdle to recover TPM
>
> + //
>
> + Status = EFI_DEVICE_ERROR;
>
> + goto GoIdle_Exit;
>
> +}
>
> + }
>
> +}
>
> +
>
> +//
>
> +// STEP 1:
>
> +// Ready is any time the TPM is ready to receive a command, following a
> write
>
> +// of 1 by software to Request.cmdReady, as indicated by the Status field
>
> +// being cleared to 0.
>
> +//
>
> +MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
>
> Status = PtpCrbWaitRegisterBits (
>
> - >CrbControlStatus,
>
> - PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
>
> + >CrbControlRequest,
>
> 0,
>
> + PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
>
> PTP_TIMEOUT_C
>
> );
>
> if (EFI_ERROR (Status)) {
>
> - //
>
> - // Try to goIdle to recover TPM
>
> - //
>
> - Status = EFI_DEVICE_ERROR;
>
> - goto GoIdle_Exit;
>
> + RetryCnt++;
>
> + if (RetryCnt < RETRY_CNT_MAX) {
>
> +MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
>
> +continue;
>
> + } else {
>
> +Status = EFI_DEVICE_ERROR;
>
> +goto GoIdle_Exit;
>
> + }
>
> }
>
> - }
>
>
>
> - //
>
> - // STEP 1:
>
> - // Ready is any time the TPM is ready to receive a command, following a
> write
>
> - // of 1 by software to Request.cmdReady, as indicated by the Status field
>
> - // being cleared to 0.
>
> - //
>
> - MmioWrite32 ((UINTN)>CrbControlRequest,
> PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
>
> - Status = PtpCrbWaitRegisterBits (
>
> - >CrbControlRequest,
>
> - 0,
>
> -
Re: [edk2-devel] [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello message to the serial port
Hi everyone, I checked Rebecca's patch with my Qemu use case. All fine - we can use that patch instead of the series I posted here. Even better, because it prints the version information. Thanks! Regards, Oliver On Tue, Jul 26, 2022 at 6:22 PM Sami Mujawar wrote: > Hi Oliver, > > > > Both your and Rebecca’s patches are using SerialPortWrite() which should > work regardless of the debug print level and for release builds as well. > > Rebecca’s patch additionally initialises the serial port which is required > if the UART has not been setup by any previous firmware. > > I believe Qemu might handoff with the UART initialised, therefore you > might not see the difference. However, this would be required for some > hardware platforms. > > > > I do not have a setup with Qemu ready to try. Is it possible for you to > apply Rebecca’s patch and check, please? > > Alternatively, I will prepare a setup tomorrow to confirm. > > > > Regards, > > > > Sami Mujawar > > > > *From: *Oliver Steffen > *Date: *Tuesday, 26 July 2022 at 16:50 > *To: *Sami Mujawar > *Cc: *"devel@edk2.groups.io" , Rebecca Cran < > rebe...@bsdio.com>, "quic_rc...@quicinc.com" , > Ard Biesheuvel , Chasel Chiu < > chasel.c...@intel.com>, Gerd Hoffmann , Leif Lindholm < > quic_llind...@quicinc.com>, Nate DeSimone , > Star Zeng , Andrew Fish , Laszlo > Ersek > *Subject: *Re: [PATCH 2/3] ArmPlatformPkg: PrePeiCore: write early hello > message to the serial port > > > > Hi Sami, > > Thank you for pointing to that patch. It pretty much does the same thing > and I would be OK with using it instead of these changes here. Except > that the message only shows up in debug mode and seems to depend on the > debug mask. It would be nice if it would show up all the time. > > We are building ArmVirt in debug, but with > DEBUG_PRINT_ERROR_LEVEL=0x8000, aka a "silent" debug build, and need > the message there too. > > What was proposed in this series prints the message independently of the > debug mask. > > > Regards, > Oliver > > > > On Tue, Jul 26, 2022 at 12:42 PM Sami Mujawar > wrote: > > Hi Oliver, > > There is a patch for review at > https://edk2.groups.io/g/devel/message/4 which prints the firmware > version string. I believe with Ard's comments addressed that patch can be > merged. > Would that patch solve your purpose to print an early message for debug > purpose? > > Regards, > > Sami Mujawar > > On 26/07/2022, 08:29, "Oliver Steffen" wrote: > > From: Laszlo Ersek > > Print the early hello message to the serial port. > > The FixedPcdGetSize() macro expands to an integer constant, therefore > an > optimizing compiler can eliminate the new code, if the platform DSC > doesn't override the empty string (size=1) default of > PcdEarlyHelloMessage. > > Signed-off-by: Laszlo Ersek > Signed-off-by: Oliver Steffen > --- > ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf | 2 ++ > ArmPlatformPkg/PrePeiCore/PrePeiCore.h | 1 + > ArmPlatformPkg/PrePeiCore/MainMPCore.c | 7 +++ > ArmPlatformPkg/PrePeiCore/MainUniCore.c | 7 +++ > 5 files changed, 19 insertions(+) > > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > index a5b4722459d1..ea7b220bc831 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreMPCore.inf > @@ -66,6 +66,8 @@ [FixedPcd] >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize >gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + >gArmTokenSpaceGuid.PcdGicDistributorBase >gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase >gArmTokenSpaceGuid.PcdGicSgiIntId > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > index 466a2b01c384..29fb8737cb2f 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf > @@ -64,4 +64,6 @@ [FixedPcd] >gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize >gArmPlatformTokenSpaceGuid.PcdCPUCoreSecondaryStackSize > > + gArmPlatformTokenSpaceGuid.PcdEarlyHelloMessage > + >gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack > diff --git a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > index 0345dd7bdd2a..ae8302becda2 100644 > --- a/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > +++ b/ArmPlatformPkg/PrePeiCore/PrePeiCore.h > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c > b/ArmPlatformPkg/PrePeiCore/MainMPCore.c > index b5d0d3a6442f..21c9d5f6da8f 100644 >
[edk2-devel] [PATCH v3] SecurityPkg: Add retry mechanism for tpm command
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3980
As per TCG PC Client Device Driver Design Principle document,
if tpm commands fails due to timeout condition, then it should
have retry mechanism (3 retry attempts).
Existing implementation of PtpCrbTpmCommand does not have retry
mechanism if it fails with EFI_TIMEOUT.
See TCG PC Client Device Driver Design Principles for TPM 2.0
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_Device_Driver_Design_Principles_TPM2p0_v1p1_r4_211104_final.pdf
Vision 1.1, Revision 0.04
Section 7.2.1
Signed-off-by: Qi Zhang
Cc: Jiewen Yao
Cc: Jian J Wang
Reviewed-by: Jiewen Yao
Tested-by: Swapnil Patil
---
.../Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c | 107 +++---
1 file changed, 68 insertions(+), 39 deletions(-)
diff --git a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
index 1d99beaa10..6b5994fde2 100644
--- a/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
+++ b/SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c
@@ -33,6 +33,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
//
#define TPMCMDBUFLENGTH 0x500
+//
+// Max retry count
+//
+#define RETRY_CNT_MAX 3
+
/**
Check whether TPM PTP register exist.
@@ -153,6 +158,7 @@ PtpCrbTpmCommand (
UINT32 TpmOutSize;
UINT16 Data16;
UINT32 Data32;
+ UINT8 RetryCnt;
DEBUG_CODE_BEGIN ();
UINTN DebugSize;
@@ -179,53 +185,76 @@ PtpCrbTpmCommand (
DEBUG_CODE_END ();
TpmOutSize = 0;
- //
- // STEP 0:
- // if CapCRbIdelByPass == 0, enforce Idle state before sending command
- //
- if ((GetCachedIdleByPass () == 0) && ((MmioRead32
((UINTN)>CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) ==
0)) {
+ RetryCnt = 0;
+ while (TRUE) {
+//
+// STEP 0:
+// if CapCRbIdelByPass == 0, enforce Idle state before sending command
+//
+if ((GetCachedIdleByPass () == 0) && ((MmioRead32
((UINTN)>CrbControlStatus) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) ==
0)) {
+ Status = PtpCrbWaitRegisterBits (
+ >CrbControlStatus,
+ PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ 0,
+ PTP_TIMEOUT_C
+ );
+ if (EFI_ERROR (Status)) {
+RetryCnt++;
+if (RetryCnt < RETRY_CNT_MAX) {
+ MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+ continue;
+} else {
+ //
+ // Try to goIdle to recover TPM
+ //
+ Status = EFI_DEVICE_ERROR;
+ goto GoIdle_Exit;
+}
+ }
+}
+
+//
+// STEP 1:
+// Ready is any time the TPM is ready to receive a command, following a
write
+// of 1 by software to Request.cmdReady, as indicated by the Status field
+// being cleared to 0.
+//
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
Status = PtpCrbWaitRegisterBits (
- >CrbControlStatus,
- PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ >CrbControlRequest,
0,
+ PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
PTP_TIMEOUT_C
);
if (EFI_ERROR (Status)) {
- //
- // Try to goIdle to recover TPM
- //
- Status = EFI_DEVICE_ERROR;
- goto GoIdle_Exit;
+ RetryCnt++;
+ if (RetryCnt < RETRY_CNT_MAX) {
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+continue;
+ } else {
+Status = EFI_DEVICE_ERROR;
+goto GoIdle_Exit;
+ }
}
- }
- //
- // STEP 1:
- // Ready is any time the TPM is ready to receive a command, following a write
- // of 1 by software to Request.cmdReady, as indicated by the Status field
- // being cleared to 0.
- //
- MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY);
- Status = PtpCrbWaitRegisterBits (
- >CrbControlRequest,
- 0,
- PTP_CRB_CONTROL_AREA_REQUEST_COMMAND_READY,
- PTP_TIMEOUT_C
- );
- if (EFI_ERROR (Status)) {
-Status = EFI_DEVICE_ERROR;
-goto GoIdle_Exit;
- }
+Status = PtpCrbWaitRegisterBits (
+ >CrbControlStatus,
+ 0,
+ PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
+ PTP_TIMEOUT_C
+ );
+if (EFI_ERROR (Status)) {
+ RetryCnt++;
+ if (RetryCnt < RETRY_CNT_MAX) {
+MmioWrite32 ((UINTN)>CrbControlRequest,
PTP_CRB_CONTROL_AREA_REQUEST_GO_IDLE);
+continue;
+ } else {
+Status = EFI_DEVICE_ERROR;
+goto GoIdle_Exit;
+ }
+}
- Status = PtpCrbWaitRegisterBits (
- >CrbControlStatus,
- 0,
- PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE,
- PTP_TIMEOUT_C
- );
- if (EFI_ERROR (Status)) {
-Status = EFI_DEVICE_ERROR;
-goto GoIdle_Exit;
+
[edk2-devel] [edk2-platforms:PATCH] Modify processor _UID ordering by CPU default fused in MADT
BIOS should not reordering cpu processor_uid
Signed-off-by: JackX Lin
Cc: Chasel Chiu
Cc: Dong Eric
Cc: Jiewen Yao
Cc: Ray Ni
Cc: Rangasai V Chaganty
Cc: Donald Kuo
Cc: Chandana C Kumar
Cc: Palakshareddy, Lavanya C
Cc: JackX Lin
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 99
---
1 file changed, 4 insertions(+), 95 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index c7e87cbd7d..d0e8891918 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -62,33 +62,6 @@ EFI_CPU_ID_ORDER_MAP*mCpuApicIdOrderTable = NULL;
UINTN mNumberOfCpus = 0;
UINTN mNumberOfEnabledCPUs = 0;
-
-/**
- The function is called by PerformQuickSort to compare int values.
-
- @param[in] LeftThe pointer to first buffer.
- @param[in] Right The pointer to second buffer.
-
- @return -1 Buffer1 is less than Buffer2.
- @return 1 Buffer1 is greater than Buffer2.
-
-**/
-INTN
-EFIAPI
-ApicIdCompareFunction (
- IN CONST VOID *Left,
- IN CONST VOID *Right
- )
-{
- UINT32 LeftApicId;
- UINT32 RightApicId;
-
- LeftApicId = ((EFI_CPU_ID_ORDER_MAP *) Left)->ApicId;
- RightApicId = ((EFI_CPU_ID_ORDER_MAP *) Right)->ApicId;
-
- return (LeftApicId > RightApicId)? 1 : (-1);
-}
-
/**
Print Cpu Apic ID Table
@@ -168,21 +141,16 @@ SortCpuLocalApicInTable (
EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer;
UINT32Index;
UINT32CurrProcessor;
- UINT32BspApicId;
- EFI_CPU_ID_ORDER_MAP TempVal;
EFI_CPU_ID_ORDER_MAP *CpuIdMapPtr;
UINT32CoreThreadMask;
- EFI_CPU_ID_ORDER_MAP *TempCpuApicIdOrderTable;
UINT32Socket;
- Index = 0;
Status = EFI_SUCCESS;
if (mCpuOrderSorted) {
return Status;
}
- TempCpuApicIdOrderTable = AllocateZeroPool (mNumberOfCpus * sizeof
(EFI_CPU_ID_ORDER_MAP));
CoreThreadMask = (UINT32) ((1 << mNumOfBitShift) - 1);
for (CurrProcessor = 0, Index = 0; CurrProcessor < mNumberOfCpus;
CurrProcessor++, Index++) {
@@ -192,7 +160,7 @@ SortCpuLocalApicInTable (
);
-CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) [Index];
+CpuIdMapPtr = (EFI_CPU_ID_ORDER_MAP *) [Index];
if ((ProcessorInfoBuffer.StatusFlag & PROCESSOR_ENABLED_BIT) != 0) {
CpuIdMapPtr->ApicId = (UINT32)ProcessorInfoBuffer.ProcessorId;
CpuIdMapPtr->Thread = ProcessorInfoBuffer.Location.Thread;
@@ -214,74 +182,16 @@ SortCpuLocalApicInTable (
} //end if PROC ENABLE
} //end for CurrentProcessor
- //keep for debug purpose
DEBUG ((DEBUG_INFO, "::ACPI:: APIC ID Order Table Init. CoreThreadMask =
%x, mNumOfBitShift = %x\n", CoreThreadMask, mNumOfBitShift));
- DebugDisplayReOrderTable (TempCpuApicIdOrderTable);
//
// Get Bsp Apic Id
//
- BspApicId = GetApicId ();
- DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", BspApicId));
-
- //
- //check to see if 1st entry is BSP, if not swap it
- //
- if (TempCpuApicIdOrderTable[0].ApicId != BspApicId) {
-for (Index = 0; Index < mNumberOfCpus; Index++) {
- if ((TempCpuApicIdOrderTable[Index].Flags == 1) &&
(TempCpuApicIdOrderTable[Index].ApicId == BspApicId)) {
-CopyMem (, [Index], sizeof
(EFI_CPU_ID_ORDER_MAP));
-CopyMem ([Index], [0],
sizeof (EFI_CPU_ID_ORDER_MAP));
-CopyMem ([0], , sizeof
(EFI_CPU_ID_ORDER_MAP));
-break;
- }
-}
-
-if (mNumberOfCpus <= Index) {
- DEBUG ((DEBUG_ERROR, "Asserting the SortCpuLocalApicInTable Index
Bufferflow\n"));
- return EFI_INVALID_PARAMETER;
-}
- }
-
- //
- // 1. Sort TempCpuApicIdOrderTable,
- //sort it by using ApicId from minimum to maximum (Socket0 to SocketN),
and the BSP must in the fist location of the table.
- //So, start sorting the table from the second element and total elements
are mNumberOfCpus-1.
- //
- PerformQuickSort ((TempCpuApicIdOrderTable + 1), (mNumberOfCpus - 1), sizeof
(EFI_CPU_ID_ORDER_MAP), (SORT_COMPARE) ApicIdCompareFunction);
+ DEBUG ((DEBUG_INFO, "BspApicId - 0x%x\n", GetApicId ()));
- //
- // 2. Sort and map the primary threads to the front of the
CpuApicIdOrderTable
- //
- for (CurrProcessor = 0, Index = 0; Index < mNumberOfCpus; Index++) {
-if ((TempCpuApicIdOrderTable[Index].Thread) == 0) { // primary thread
- CopyMem ([CurrProcessor],
[Index], sizeof
