REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3648

Register SmmExitBootServices and SmmLegacyBoot callback function to unregister this handler.

Signed-off-by: Hao Shi <hao.shi@...>

---
.../UserAuthenticationSmm.c | 34 +++++++++++++++++++
.../UserAuthenticationSmm.inf | 2 ++
2 files changed, 36 insertions(+)

diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c
index 07e834eb..30f889dd 100644
--- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c
+++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.c
@@ -13,6 +13,7 @@ UINTN mAdminPasswordTryCount = 0;

BOOLEAN mNeedReVerify = TRUE;
BOOLEAN mPasswordVerified = FALSE;
+EFI_HANDLE mSmmHandle = NULL;

/**
Verify if the password is correct.
@@ -612,6 +613,30 @@ EXIT:
return EFI_SUCCESS;
}

+/**
+ Performs Exit Boot Services UserAuthentication actions
+
+ @param[in] Protocol Points to the protocol's unique identifier.
+ @param[in] Interface Points to the interface instance.
+ @param[in] Handle The handle on which the interface was installed.
+
+ @retval EFI_SUCCESS Notification runs successfully.
+**/
+EFI_STATUS
+EFIAPI
+UaExitBootServices (
+ IN CONST EFI_GUID *Protocol,
+ IN VOID *Interface,
+ IN EFI_HANDLE Handle
+ )
+{
+ DEBUG ((DEBUG_INFO, "Unregister User Authentication Smi\n"));
+
+ gSmst->SmiHandlerUnRegister(mSmmHandle);
+
+ return EFI_SUCCESS;
+}
+
/**
Main entry for this driver.

@@ -633,6 +658,7 @@ PasswordSmmInit (
EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
CHAR16 PasswordHistoryName[sizeof(USER_AUTHENTICATION_VAR_NAME)/sizeof(CHAR16) + 5];
UINTN Index;
+ EFI_EVENT ExitBootServicesEvent;

ASSERT (PASSWORD_HASH_SIZE == SHA256_DIGEST_SIZE);
ASSERT (PASSWORD_HISTORY_CHECK_COUNT < 0xFFFF);
@@ -663,6 +689,14 @@ PasswordSmmInit (
if (EFI_ERROR (Status)) {
return Status;
}
+ mSmmHandle = SmmHandle;
+ //
+ // Register for SmmExitBootServices and SmmLegacyBoot notification.
+ //
+ Status = gSmst->SmmRegisterProtocolNotify (&gEdkiiSmmExitBootServicesProtocolGuid, UaExitBootServices, &ExitBootServicesEvent);
+ ASSERT_EFI_ERROR (Status);
+ Status = gSmst->SmmRegisterProtocolNotify (&gEdkiiSmmLegacyBootProtocolGuid, UaExitBootServices, &ExitBootServicesEvent);
+ ASSERT_EFI_ERROR (Status);

if (IsPasswordCleared()) {
DEBUG ((DEBUG_INFO, "IsPasswordCleared\n"));
diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf
index 0b33b194..d73a2fe2 100644
--- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf
+++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxeSmm/UserAuthenticationSmm.inf
@@ -48,6 +48,8 @@
[Protocols]
gEdkiiVariableLockProtocolGuid ## CONSUMES
gEfiSmmVariableProtocolGuid ## CONSUMES
+ gEdkiiSmmExitBootServicesProtocolGuid ## CONSUMES
+ gEdkiiSmmLegacyBootProtocolGuid ## CONSUMES

[Depex]
gEfiSmmVariableProtocolGuid AND gEfiVariableWriteArchProtocolGuid
--
2.26.2.windows.1

HI Brijesh
Thanks for the explanation.

All fields in TDX metadata are required. So the current SEV proposal (3 fields) does not work for TDX. The extra fields are used to guide VMM on how to copy the binary, allocate memory, and how to measure the component. And we are adding more extension to them to support more use cases.

Based upon the factor that TDX and SEV are using different architecture to boot, I am not sure why we have to combine them into one table. (TDX need assist from TDX module - run in host CPU, and SEV need assist from PSP - a coprocessor)

I strongly recommend to use two tables. TDX only defines TDX required info. SEV only defines SEV required info. Each one can add its own extension separately.
Otherwise, there will be unnecessary information in the one metadata, which will increase the image size.

Thank you
Yao Jiewen

On September 23, 2021 10:04 PM, Brijesh Singh wrote:

SEV hardware does not have a concept of the metadata. To boot SEV guest we
need to pass some information to VMM and in past those information were
passed through SNP_BOOT_BLOCK (GUIDed structure) but Gerd recommended
that it will be good idea if both SEV and TDX uses a common metadata approach
to pass these information. I personally think it was a good suggestion. So, in SNP
series I went ahead and created a generic metadata structure and hope that
TDX will build on it. The user of the metadata structure is VMM (qemu, etc);
while launching the guest the VMM knows whether its creating the SEV or TDX
guest and will process the entries accordingly.

As per the number of fields in the metadata is concerns, I felt 3 fields (start, size
and type) should be good enough for all the cases. There was a question from
Gerd to Min asking why do you need the dataoffset/rawdatasize etc and I don't
remember seeing the answer for it.

The discussion is in this link. https://edk2.groups.io/g/devel/message/80289

As I said in the start, SNP hardware does not
enforce metadata layout so I am flexible to add more field or remove or keep it
separate.

thanks

On 9/23/21 8:38 AM, Yao, Jiewen wrote:

Good point, Min.

If

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
com%2FAMDESE%2Fovmf%2Fblob%2Fsnp-
v8%2FOvmfPkg%2FResetVector%2FX64%2FOvmfMetadata.asm&amp;data=0
4%7C01%7Cbrijesh.singh%40amd.com%7C52f6327efa33480bf4a308d97e977
ff0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637680011416
117826%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=zDfikRYhxd8E
RY%2Fw6kJLhJKRNWbYTl4D6PpqK%2BVNsus%3D&amp;reserved=0 is the
proposal, then I have more comment:

Type: OVMF_SECTION_TYPE_CODE, OVMF_SECTION_TYPE_VARS are NOT

used for SEV. I am not sure why they are there.

Type: OVMF_SECTION_TYPE_CPUID should be SEV specific. TDX does not

need CPUID page.

Type: OVMF_SECTION_TYPE_SEC_MEM also seems for SEV. TDX does not

need this special memory, such as Page table. It is already covered by code.

Type: OVMF_SECTION_TYPE_SNP_SECRETS /

OVMF_SECTION_TYPE_SNP_SEC_MEM is SEV specific.

The SEV table is totally different with TDX metadata table. I really cannot see

the benefit to merge into one table.

Thank you
Yao Jiewen

-----Original Message-----
From: Xu, Min M <min.m.xu@...>
Sent: Thursday, September 23, 2021 9:20 PM
To: devel@edk2.groups.io; brijesh.singh@...; Yao, Jiewen
<jiewen.yao@...>; Gerd Hoffmann <kraxel@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Erdem Aktas <erdemaktas@...>;
James Bottomley <jejb@...>; Tom Lendacky
<thomas.lendacky@...>
Subject: RE: [edk2-devel] [PATCH V7 1/1] OvmfPkg: Enable TDX in
ResetVector

I suggest SEV and TDX keep their own metadata in separate files. This
is because SEV and TDX has different item structure.

From the OvmfMetadata definition in SEV
(https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
thub.com%2FAMDESE%2Fovmf%2Fblob%2Fsnp-

&amp;data=04%7C01%7Cbrijesh.sin

gh%40amd.com%7C52f6327efa33480bf4a308d97e977ff0%7C3dd8961fe488
4e608e1

1a82d994e183d%7C0%7C0%7C637680011416117826%7CUnknown%7CTWF
pbGZsb3d8ey

JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
7C10

00&amp;sdata=at43H9sgmlaD773wsU5%2BoPPSjImo0UiCxQ0nmwdV9ds%3D
&amp;res

erved=0
v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm) there are 3 fields in
the item. (Base/Size/Type).

But for TDX, there are 6 fields
(DataOffset/RawDataSize/MemoryAddress/MemorySize/Type/Attribute) in
one item.
That is because TDX-QEMU not only initialize the memory region, but
also does more tasks (measurement) if the Attribute indicates.
DataOffset/RawDataSize is used by the TDX-QEMU to do the measurement
if the Attribute field is MR.EXTEND.
MemoryAddress/MemorySize indicates the TDX-QEMU how to initialize the
memory region.

We can add more fields in the item to make it workable for both SEV
and TDX, (for example, add DataOffset/RawDataSize/Attribute), but it
also restrict the changes in the future if more fields is needed
(TDX's change will impact the existing SEV-QEMU).

On September 23, 2021 8:55 PM, Brijesh Singh wrote:

Like Gerd I would prefer to have one metadata table in the reset GUID.
The metadata table will contain multiple entries; lot of entries are
common between SNP and TDX. Some entries will have specific meaning
for the

platform.

Those special entries should be marked using the
OVMF_SECTION_TYPE_{TDX,SNP}_XXXX. It is perfectly fine to have a
more

than

one entry for the same region with different type, e.g

GhcbBookkeepingSnp:

  GHCB_BOOKKEPING_BASE_ADDRESS

  GHCB_BOOKKEEPING_SIZE

  OVMF_SECTION_TYPE_SNP_MEM

TdxMailBoxExt:

  GHCB_BOOKKEPING_BASE_ADDRESS

  GHCB_BOOKKEEPING_SIZE

  OVMF_SECTION_TYPE_TDX_MAILBOX

If we want all the OVMF_SECTION_TYPE_SNP_xxx should be defined in a
separate file then that is also doable. I put everything in one
place because I

was

trying to keep entry order similar to what is present in MEMFD.

thanks

On 9/23/21 6:39 AM, Yao, Jiewen wrote:

I strongly recommend to separate SEV and TDX in all context, if it
is

something

SEV or TDX specific.

Then each file has clear ownership.
If it is something generic for both SEV and TDX, it can in one file.

For example, SecPeiTempRam/SecPageTable can be in common file.
But SevSnpSecrets/GhcbBookkeeping should be in SEV file.

Thank you
Yao Jiewen

-----Original Message-----
From: Gerd Hoffmann <kraxel@...>
Sent: Thursday, September 23, 2021 4:48 PM
To: Xu, Min M <min.m.xu@...>
Cc: devel@edk2.groups.io; Ard Biesheuvel
<ardb+tianocore@...>; Justen, Jordan L
<jordan.l.justen@...>; Brijesh Singh
<brijesh.singh@...>; Erdem Aktas <erdemaktas@...>;

James

Bottomley <jejb@...>; Yao, Jiewen
<jiewen.yao@...>; Tom Lendacky <thomas.lendacky@...>
Subject: Re: [PATCH V7 1/1] OvmfPkg: Enable TDX in ResetVector

On Thu, Sep 23, 2021 at 12:38:24AM +0000, Xu, Min M wrote:

On September 22, 2021 3:49 PM, Gerd Hoffmann wrote:

Hi,

+%ifdef ARCH_X64
+;
+; TDX Metadata offset block
+;
+; TdxMetadata.asm is included in ARCH_X64 because Inte TDX is
+only ; available in ARCH_X64. Below block describes the offset
+of ; TdxMetadata block in Ovmf image ; ; GUID :
+e47a6535-984a-4798-865e-4685a7bf8ec2
+;
+tdxMetadataOffsetStart:
+ DD tdxMetadataOffsetStart - TdxMetadataGuid - 16
+ DW tdxMetadataOffsetEnd - tdxMetadataOffsetStart
+ DB 0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+ DB 0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+tdxMetadataOffsetEnd:
+
+%endif

This should be switched to common ovmf metadata (see patches 4-7
of the SEV-SNP series).

Min: please have a look at these patches.

Hi, Gerd
I checked the patches 4-7 of the SEV-SNP series. The common
OvmfMetadata is designed for both SEV and TDX, right?

That is the idea, yes.

If so, then it means the SEV and TDX metadata will be mixed in
this OvmfMetadata.

Yes.

I am thinking there will always be different fields for SEV and TDX.
For example, SEV has PcdOvmfSecGhcbPageTable but TDX doesn't need
that page. If the common OvmfMetadata is consumed by TDX-QEMU,

then

PcdOvmfSecGhcbPageTableBase will be initialized too.
That doesn't make sense.

We have different range types. OVMF_* are the common areas.
SEV_* will be used by sev only, TDX_* will be used by tdx only.
TDX and SEV entries are allowed to overlap, i.e.
PcdOvmfSecGhcbPageTableBase should have some SEV_* type for sev (I
think this needs fixing in the series), and tdx can use the page
for something else by adding an
TDX_* entry for the same range.

I am thinking that SEV and TDX can keep their own Metadata (in
separate files, SevMetadata.asm and TdxMetadata.asm) which are
pointed by the SEV or TDX offsets in the GUID-ed chain in ResetVector.

I'd very much prefer to have a single table to avoid duplication
for the common memory areas and keep the reset vector small.

Having separate SevMetadata.asm + TdxMetadata.asm files (then have
OvmfMetadata.asm include these two) is an option. I think this
isn't needed, we can also just group the entries in OvmfMetadata.asm.

In this case, SEV and TDX can design their own metadata flexibly,
for example, the attribute, the item structure, add/remove/update
the items, etc.

Why have two ways to do the same thing?

take care,
Gerd

SEV hardware does not have a concept of the metadata. To boot SEV guest we need to pass some information to VMM and in past those information were passed through SNP_BOOT_BLOCK (GUIDed structure) but Gerd recommended that it will be good idea if both SEV and TDX uses a common metadata approach to pass these information. I personally think it was a good suggestion. So, in SNP series I went ahead and created a generic metadata structure and hope that TDX will build on it. The user of the metadata structure is VMM (qemu, etc); while launching the guest the VMM knows whether its creating the SEV or TDX guest and will process the entries accordingly.

As per the number of fields in the metadata is concerns, I felt 3 fields (start, size and type) should be good enough for all the cases. There was a question from Gerd to Min asking why do you need the dataoffset/rawdatasize etc and I don't remember seeing the answer for it. As I said in the start, SNP hardware does not enforce metadata layout so I am flexible to add more field or remove or keep it separate.

thanks

On Thu, Sep 23, 2021 at 06:54:24 -0600, Rebecca Cran wrote:

On 9/23/21 4:30 AM, Leif Lindholm wrote:

Not just an improvement, but an actual bugfix.

I would propose a subject line change though:
Qemu: SbsaQemu: Set the DSDT revision value to 2 to use 64-bit math
->
Platform/Qemu: fix SbsaQemu DSDT format version

If you're OK with the change, I can fold that in.
Reviewed-by: Leif Lindholm <leif@...>

Yes, please do.

Pushed as 8227e9e9f6a8.
Thanks!

/
Leif

Hi Nhi,

Following up with the comment I also made for 12/28:

On Wed, Sep 15, 2021 at 22:55:00 +0700, Nhi Pham wrote:

diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h b/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h
new file mode 100644
index 000000000000..66286bfff145
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h
@@ -0,0 +1,317 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef PLATFORM_AC01_H_
+#define PLATFORM_AC01_H_
+
+//
+// Number of supported sockets in the platform
+//
+#define PLATFORM_CPU_MAX_SOCKET 2
+
+//
+// Maximum number of CPMs in the chip.
+//
+#define PLATFORM_CPU_MAX_CPM (FixedPcdGet32 (PcdClusterCount))
+
+//
+// Number of cores per CPM.
+//
+#define PLATFORM_CPU_NUM_CORES_PER_CPM (FixedPcdGet32 (PcdCoreCount) / PLATFORM_CPU_MAX_CPM)
+
+//
+// Socket bit offset of core UID.
+//
+#define PLATFORM_SOCKET_UID_BIT_OFFSET 16
+
+//
+// CPM bit offset of core UID.
+//
+#define PLATFORM_CPM_UID_BIT_OFFSET 8
+
+//
+// Maximum number of cores supported.
+//
+#define PLATFORM_CPU_MAX_NUM_CORES (PLATFORM_CPU_MAX_SOCKET * PLATFORM_CPU_MAX_CPM * PLATFORM_CPU_NUM_CORES_PER_CPM)
+
+//
+// Maximum number of memory region
+//
+#define PLATFORM_DRAM_INFO_MAX_REGION 16
+
+//
+// Maximum number of DDR slots supported
+//
+#define PLATFORM_DIMM_INFO_MAX_SLOT 32
+
+//
+// Maximum number of memory supported.
+//
+#define PLATFORM_MAX_MEMORY_REGION 4
+
+//
+// The Array of Soc Gpio Base Address
+//
+#define GPIO_DWAPB_BASE_ADDR 0x1000026f0000, 0x1000026e0000, 0x1000027b0000, 0x1000026d0000, 0x5000026f0000, 0x5000026e0000, 0x5000027b0000, 0x5000026d0000

Like I said in 12/28 - I'm OK with this use of lists in macros, but
all such macros must be

However, this and the subsequent macro are not used until 6/28 - so
please also move them to that patch.
Keep the R-b for both patches once you've done that, but please also
make a pass and see if there are further things defined in this patch
that are not used until later.

Best Regards,

Leif

+
+//
+// The Array of Soc Gpi Base Address
+//
+#define GPI_DWAPB_BASE_ADDR 0x1000026d0000, 0x5000026d0000
+
+//
+// Number of Pins Per Each Contoller
+//
+#define GPIO_DWAPB_PINS_PER_CONTROLLER 8
+
+//
+// Number of Pins Each Socket
+//
+#define GPIO_DWAPB_PINS_PER_SOCKET 32
+
+//
+// The maximum number of I2C bus
+//
+#define MAX_PLATFORM_I2C_BUS_NUM 2
+
+//
+// The base address of DW I2C
+//
+#define PLATFORM_I2C_REGISTER_BASE 0x1000026B0000ULL, 0x100002750000ULL
+
+//
+// Offset of failsafe testing feature
+//
+#define NV_UEFI_FAILURE_FAILSAFE_OFFSET 0x1F8
+
+//
+// Maximum number of memory controller supports NVDIMM-N per socket
+//
+#define PLATFORM_NVDIMM_MCU_MAX_PER_SK 2
+
+//
+// Maximum number of NVDIMM-N per memory controller
+//
+#define PLATFORM_NVDIMM_NUM_MAX_PER_MCU 1
+
+//
+// Maximum number of NVDIMM region per socket
+//
+#define PLATFORM_NVDIMM_REGION_MAX_PER_SK 2
+
+//
+// Socket 0 base address of NVDIMM non-hashed region 0
+//
+#define PLATFORM_NVDIMM_SK0_NHASHED_REGION0 0x0B0000000000ULL
+
+//
+// Socket 0 base address of NVDIMM non-hashed region 1
+//
+#define PLATFORM_NVDIMM_SK0_NHASHED_REGION1 0x0F0000000000ULL
+
+//
+// Socket 1 base address of NVDIMM non-hashed region 0
+//
+#define PLATFORM_NVDIMM_SK1_NHASHED_REGION0 0x430000000000ULL
+
+//
+// Socket 1 base address of NVDIMM non-hashed region 1
+//
+#define PLATFORM_NVDIMM_SK1_NHASHED_REGION1 0x470000000000ULL
+
+//
+// DIMM ID of NVDIMM-N device 1
+//
+#define PLATFORM_NVDIMM_NVD1_DIMM_ID 6
+
+//
+// DIMM ID of NVDIMM-N device 2
+//
+#define PLATFORM_NVDIMM_NVD2_DIMM_ID 14
+
+//
+// DIMM ID of NVDIMM-N device 3
+//
+#define PLATFORM_NVDIMM_NVD3_DIMM_ID 22
+
+//
+// DIMM ID of NVDIMM-N device 4
+//
+#define PLATFORM_NVDIMM_NVD4_DIMM_ID 30
+
+//
+// NFIT device handle of NVDIMM-N device 1
+//
+#define PLATFORM_NVDIMM_NVD1_DEVICE_HANDLE 0x0330
+
+//
+// NFIT device handle of NVDIMM-N device 2
+//
+#define PLATFORM_NVDIMM_NVD2_DEVICE_HANDLE 0x0770
+
+//
+// NFIT device handle of NVDIMM-N device 3
+//
+#define PLATFORM_NVDIMM_NVD3_DEVICE_HANDLE 0x1330
+
+//
+// NFIT device handle of NVDIMM-N device 4
+//
+#define PLATFORM_NVDIMM_NVD4_DEVICE_HANDLE 0x1770
+
+//
+// Interleave ways of non-hashed NVDIMM-N
+//
+#define PLATFORM_NVDIMM_NHASHED_INTERLEAVE_WAYS 1
+
+//
+// Interleave ways of hashed NVDIMM-N
+//
+#define PLATFORM_NVDIMM_HASHED_INTERLEAVE_WAYS 2
+
+//
+// Region offset of hashed NVDIMM-N
+//
+#define PLATFORM_NVDIMM_HASHED_REGION_OFFSET 512
+
+//
+// The base address of master socket GIC redistributor registers
+//
+#define GICR_MASTER_BASE_REG 0x100100140000
+
+//
+// The base address of GIC distributor registers
+//
+#define GICD_BASE_REG 0x100100000000
+
+//
+// The base address of slave socket GIC redistributor registers
+//
+#define GICR_SLAVE_BASE_REG 0x500100140000
+
+//
+// The base address of slave socket GIC distributor registers
+//
+#define GICD_SLAVE_BASE_REG 0x500100000000
+
+//
+// CSR Address base for slave socket
+//
+#define SLAVE_SOCKET_BASE_ADDRESS_OFFSET 0x400000000000
+
+//
+// Socket 0 first RC
+//
+#define SOCKET0_FIRST_RC 2
+
+//
+// Socket 0 last RC
+//
+#define SOCKET0_LAST_RC 7
+
+//
+// Socket 1 first RC
+//
+#define SOCKET1_FIRST_RC 10
+
+//
+// Socket 1 last RC
+//
+#define SOCKET1_LAST_RC 15
+
+//
+// SMpro EFUSE Shadow register
+//
+#define SMPRO_EFUSE_SHADOW0 (FixedPcdGet64 (PcdSmproEfuseShadow0))
+
+//
+// 2P Configuration Register
+//
+#define CFG2P_OFFSET 0x200
+
+//
+// Slave socket present
+//
+#define SLAVE_PRESENT_N BIT1
+
+//
+// Max number for AC01 PCIE Root Complexes per socket
+//
+#define AC01_MAX_RCS_PER_SOCKET 8
+
+//
+// Max number for AC01 PCIE Root Complexes
+//
+#define AC01_MAX_PCIE_ROOT_COMPLEX 16
+
+//
+// Max number for AC01 PCIE Root Bridge under each Root Complex
+//
+#define AC01_MAX_PCIE_ROOT_BRIDGE 1
+
+//
+// The base address of {TCU, CSR, MMCONFIG} Registers
+//
+#define AC01_PCIE_REGISTER_BASE 0x33FFE0000000, 0x37FFE0000000, 0x3BFFE0000000, 0x3FFFE0000000, 0x23FFE0000000, 0x27FFE0000000, 0x2BFFE0000000, 0x2FFFE0000000, 0x73FFE0000000, 0x77FFE0000000, 0x7BFFE0000000, 0x7FFFE0000000, 0x63FFE0000000, 0x67FFE0000000, 0x6BFFE0000000, 0x6FFFE0000000
+
+//
+// The base address of MMIO Registers
+//
+#define AC01_PCIE_MMIO_BASE 0x300000000000, 0x340000000000, 0x380000000000, 0x3C0000000000, 0x200000000000, 0x240000000000, 0x280000000000, 0x2C0000000000, 0x700000000000, 0x740000000000, 0x780000000000, 0x7C0000000000, 0x600000000000, 0x640000000000, 0x680000000000, 0x6C0000000000
+
+//
+// The base address of MMIO32 Registers
+//
+#define AC01_PCIE_MMIO32_BASE 0x000020000000, 0x000028000000, 0x000030000000, 0x000038000000, 0x000001000000, 0x000008000000, 0x000010000000, 0x000018000000, 0x000060000000, 0x000068000000, 0x000070000000, 0x000078000000, 0x000040000000, 0x000048000000, 0x000050000000, 0x000058000000
+
+//
+// The base address of MMIO32 Registers
+//
+#define AC01_PCIE_MMIO32_BASE_1P 0x000040000000, 0x000050000000, 0x000060000000, 0x000070000000, 0x000001000000, 0x000010000000, 0x000020000000, 0x000030000000, 0, 0, 0, 0, 0, 0, 0, 0
+
+//
+// DSDT RCA2 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCA2_QMEM 0x0000000000000000, 0x0000000060000000, 0x000000006FFFFFFF, 0x0000000000000000, 0x0000000010000000
+
+//
+// DSDT RCA3 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCA3_QMEM 0x0000000000000000, 0x0000000070000000, 0x000000007FFFFFFF, 0x0000000000000000, 0x0000000010000000
+
+//
+// DSDT RCB0 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCB0_QMEM 0x0000000000000000, 0x0000000001000000, 0x000000000FFFFFFF, 0x0000000000000000, 0x000000000F000000
+
+//
+// DSDT RCB1 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCB1_QMEM 0x0000000000000000, 0x0000000010000000, 0x000000001FFFFFFF, 0x0000000000000000, 0x0000000010000000
+
+//
+// DSDT RCB2 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCB2_QMEM 0x0000000000000000, 0x0000000020000000, 0x000000002FFFFFFF, 0x0000000000000000, 0x0000000010000000
+
+//
+// DSDT RCB3 PCIe Meme32 Attribute
+//
+#define AC01_PCIE_RCB3_QMEM 0x0000000000000000, 0x0000000030000000, 0x000000003FFFFFFF, 0x0000000000000000, 0x0000000010000000
+
+//
+// The start of TBU PMU IRQ array.
+//
+#define AC01_SMMU_TBU_PMU_IRQS 224, 230, 236, 242, 160, 170, 180, 190, 544, 550, 556, 562, 480, 490, 500, 510
+
+//
+// The start of TCU PMU IRQ array
+//
+#define AC01_SMMU_TCU_PMU_IRQS 256, 257, 258, 259, 260, 261, 262, 263, 576, 577, 578, 579, 580, 581, 582, 583
+
+#endif /* PLATFORM_AC01_H_ */

Hi Nhi,

I think all of my comments up to here have been reasonably simple.
This patch I have not reviewed before, so they could be more detailed.

If you'd like to submit a v4 of 1-11, we could possibly merge those
and reduce the churn a bit.

Anyway, review following:

On Wed, Sep 15, 2021 at 22:55:11 +0700, Nhi Pham wrote:

From: Vu Nguyen <vunguyen@...>

Provides essential functions to initialize the PCIe Root Complex of
Ampere Altra processor.

Cc: Thang Nguyen <thang@...>
Cc: Chuong Tran <chuong@...>
Cc: Phong Vo <phong@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>

Signed-off-by: Vu Nguyen <vunguyen@...>
---
Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec | 6 +
Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc | 1 +
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/Ac01PcieLib.inf | 67 +
Silicon/Ampere/AmpereAltraPkg/Include/Ac01PcieCommon.h | 128 ++
Silicon/Ampere/AmpereAltraPkg/Include/Library/Ac01PcieLib.h | 163 ++
Silicon/Ampere/AmpereAltraPkg/Include/Library/BoardPcieLib.h | 92 ++
Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h | 19 +-
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.h | 649 ++++++++
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreCapCfg.h | 63 +
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.h | 30 +
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c | 1659 ++++++++++++++++++++
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreLib.c | 556 +++++++
Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.c | 646 ++++++++
13 files changed, 4077 insertions(+), 2 deletions(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
index d5b12a81e9bf..93da9b38def5 100644
--- a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+++ b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
@@ -37,6 +37,12 @@ [LibraryClasses]
## @libraryclass Defines a set of methods to communicate with secure parition over MM interface.
MmCommunicationLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/MmCommunicationLib.h

+ ## @libraryclass Defines a set of methods to initialize Pcie
+ Ac01PcieLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/Ac01PcieLib.h
+
+ ## @libraryclass Defines a set of platform PCIe functions
+ BoardPcieLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/BoardPcieLib.h
+
## @libraryclass Defines a set of methods to access flash memory.
FlashLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h

diff --git a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
index 4af85f721cb3..d4e1b84c0276 100644
--- a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
+++ b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
@@ -82,6 +82,7 @@ [LibraryClasses.common]
NVParamLib|Silicon/Ampere/AmpereAltraPkg/Library/NVParamLib/NVParamLib.inf
MailboxInterfaceLib|Silicon/Ampere/AmpereAltraPkg/Library/MailboxInterfaceLib/MailboxInterfaceLib.inf
SystemFirmwareInterfaceLib|Silicon/Ampere/AmpereAltraPkg/Library/SystemFirmwareInterfaceLib/SystemFirmwareInterfaceLib.inf
+ Ac01PcieLib|Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/Ac01PcieLib.inf
AmpereCpuLib|Silicon/Ampere/AmpereAltraPkg/Library/AmpereCpuLib/AmpereCpuLib.inf
TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
I2cLib|Silicon/Ampere/AmpereAltraPkg/Library/DwI2cLib/DwI2cLib.inf
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/Ac01PcieLib.inf b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/Ac01PcieLib.inf
new file mode 100644
index 000000000000..e2b0f293eb24
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/Ac01PcieLib.inf
@@ -0,0 +1,67 @@
+## @file
+#
+# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = Ac01PcieLib
+ FILE_GUID = 8ABFA0FC-313E-11E8-B467-0ED5F89F718B
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = Ac01PcieLib
+
+[Sources]
+ PcieCore.c
+ PcieCore.h
+ PcieCoreCapCfg.h
+ PcieCoreLib.c
+ PciePatchAcpi.c
+ PciePatchAcpi.h
+
+[Packages]
+ EmbeddedPkg/EmbeddedPkg.dec
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Ampere/AmpereAltraBinPkg/AmpereAltraBinPkg.dec
+ Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+ Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
+
+[LibraryClasses]
+ AcpiLib
+ AmpereCpuLib
+ ArmLib
+ BaseLib
+ BaseMemoryLib
+ BoardPcieLib
+ DebugLib
+ GpioLib
+ IoLib
+ MemoryAllocationLib
+ PcdLib
+ PciePhyLib
+ SerialPortLib
+ SystemFirmwareInterfaceLib
+ TimerLib
+ UefiBootServicesTableLib
+ UefiLib
+ UefiRuntimeServicesTableLib
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+
+[Protocols]
+ gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+ gEfiAcpiSdtProtocolGuid # PROTOCOL ALWAYS_CONSUMED
+
+[Guids]
+ gPlatformHobGuid
+
+[Depex]
+ gEfiAcpiTableProtocolGuid AND gEfiAcpiSdtProtocolGuid
diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Ac01PcieCommon.h b/Silicon/Ampere/AmpereAltraPkg/Include/Ac01PcieCommon.h
new file mode 100644
index 000000000000..b72c305f4637
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Ac01PcieCommon.h
@@ -0,0 +1,128 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AC01_PCIE_COMMON_H_
+#define AC01_PCIE_COMMON_H_
+
+#define PRESET_INVALID 0xFF
+
+//
+// PCIe link width
+//
+enum {
+ LNKW_NONE = 0,
+ LNKW_X1 = 0x1,
+ LNKW_X2 = 0x2,
+ LNKW_X4 = 0x4,
+ LNKW_X8 = 0x8,
+ LNKW_X16 = 0x10,
+};

Coding style: enums should
- be typedef:d to an UPPER_CASE_NAME
- have CamelCaseEnumerations
- end with a maximum element.

(https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/appendix_a_common_examples#type-declarations)

+
+//
+// PCIe link speed
+//
+enum {
+ SPEED_NONE = 0,
+ SPEED_GEN1 = 0x1,
+ SPEED_GEN2 = 0x2,
+ SPEED_GEN3 = 0x4,
+ SPEED_GEN4 = 0x8,
+};
+
+//
+// PCIe controller number
+//
+enum {
+ PCIE_0 = 0,
+ PCIE_1,
+ PCIE_2,
+ PCIE_3,
+ PCIE_4,
+ MAX_PCIE_A = PCIE_4,
+ PCIE_5,
+ PCIE_6,
+ PCIE_7,
+ MAX_PCIE,
+ MAX_PCIE_B = MAX_PCIE
+};
+
+//
+// Root Complex type
+//
+enum {
+ RCA,
+ RCB
+};
+
+//
+// Root Complex number
+//
+enum {
+ RCA0 = 0,
+ RCA1,
+ RCA2,
+ RCA3,
+ MAX_RCA,
+ RCB0 = MAX_RCA,
+ RCB1,
+ RCB2,
+ RCB3,
+ MAX_RCB,
+ MAX_RC = MAX_RCB
+};
+
+//
+// Data structure to store the PCIe controller information
+//
+typedef struct {
+ UINT64 CsrAddr; // Pointer to CSR Address

Not a pointer, an address. So comment should say "Base address of CSR
block"? I'd be tempted to suggest renaming the variable CsrBase.

+ UINT64 SnpsRamAddr; // Pointer to Synopsys SRAM address
+ UINT8 MaxGen; // Max speed Gen-1/-2/-3/-4
+ UINT8 CurGen; // Current speed Gen-1/-2/-3/-4

CurrentGen.
CamelCase may be annoying, but CmlCsWAbrvs is worse.

+ UINT8 MaxWidth; // Max lanes x2/x4/x8/x16
+ UINT8 CurWidth; // Current lanes x2/x4/x8/x16
+ UINT8 ID; // ID of the controller within Root Complex
+ UINT8 DevNum; // Device number as part of Bus:Dev:Func
+ BOOLEAN Active; // Active? Used in bi-furcation mode
+ BOOLEAN LinkUp; // PHY and PCIE linkup
+ BOOLEAN HotPlug; // Hotplug support
+} AC01_PCIE;

_CONTROLLER

+
+//
+// Data structure to store the Root Complex information
+//
+typedef struct {
+ UINT64 BaseAddr;
+ UINT64 TcuAddr;
+ UINT64 HBAddr;
+ UINT64 MsgAddr;
+ UINT64 SerdesAddr;
+ UINT64 MmcfgAddr;
+ UINT64 MmioAddr;
+ UINT64 MmioSize;
+ UINT64 Mmio32Addr;
+ UINT64 Mmio32Size;
+ UINT64 IoAddr;
+ AC01_PCIE Pcie[MAX_PCIE_B];
+ UINT8 MaxPcieController;
+ UINT8 Type;
+ UINT8 ID;
+ UINT8 DevMapHigh:3; // Copy of High Devmap programmed to Host bridge
+ UINT8 DevMapLow:3; // Copy of Low Devmap programmed to Host bridge
+ UINT8 DefaultDevMapHigh:3; // Default of High devmap based on board settings
+ UINT8 DefaultDevMapLow:3; // Default of Low devmap based on board settings
+ UINT8 Socket;
+ BOOLEAN Active;
+ UINT8 Logical;
+ VOID *RootBridge; // Pointer to Stack PCI_ROOT_BRIDGE
+ UINT32 Flags; // Flags
+ UINT8 PresetGen3[MAX_PCIE_B]; // Preset for Gen3
+ UINT8 PresetGen4[MAX_PCIE_B]; // Preset for Gen4
+} AC01_RC;

_ROOT_COMPLEX

+
+#endif
diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Library/Ac01PcieLib.h b/Silicon/Ampere/AmpereAltraPkg/Include/Library/Ac01PcieLib.h
new file mode 100644
index 000000000000..47c6452a0499
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Library/Ac01PcieLib.h
@@ -0,0 +1,163 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AC01_PCIE_LIB_H_
+#define AC01_PCIE_LIB_H_
+
+#include <Library/PciHostBridgeLib.h>
+#include <Protocol/PciHostBridgeResourceAllocation.h>
+
+/**
+ Get RootBridge disable status.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval BOOLEAN Return RootBridge disable status.
+**/
+BOOLEAN
+Ac01PcieCheckRootBridgeDisabled (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ );
+
+/**
+ Prepare to start PCIE core BSP driver
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetup (
+ VOID
+ );
+
+/**
+ Prepare to end PCIE core BSP driver.
+**/
+VOID
+Ac01PcieEnd (
+ VOID
+ );
+
+/**
+ Get Total HostBridge.
+
+ @retval UINTN Return Total HostBridge.
+**/
+UINT8
+Ac01PcieGetTotalHBs (
+ VOID
+ );
+
+/**
+ Get Total RootBridge per HostBridge.
+
+ @param[in] RCIndex Index to identify of Root Complex.
+
+ @retval UINTN Return Total RootBridge per HostBridge.
+**/
+UINT8
+Ac01PcieGetTotalRBsPerHB (
+ IN UINTN RCIndex
+ );
+
+/**
+ Get RootBridge Attribute.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval UINTN Return RootBridge Attribute.
+**/
+UINTN
+Ac01PcieGetRootBridgeAttribute (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ );
+
+/**
+ Get RootBridge Segment number
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval UINTN Return RootBridge Segment number.
+**/
+UINTN
+Ac01PcieGetRootBridgeSegmentNumber (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ );
+
+/**
+ Initialize Host bridge
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetupHostBridge (
+ IN UINTN HBIndex
+ );
+
+/**
+ Initialize Root bridge
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+ @param[in] RootBridgeInstance The pointer of instance of the Root bridge IO.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetupRootBridge (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex,
+ IN PCI_ROOT_BRIDGE *RootBridge
+ );
+
+/**
+ Reads/Writes an PCI configuration register.
+
+ @param[in] RootInstance Pointer to RootInstance structure.
+ @param[in] Address Address which want to read or write to.
+ @param[in] Write Indicate that this is a read or write command.
+ @param[in] Width Specify the width of the data.
+ @param[in, out] Data The buffer to hold the data.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieConfigRW (
+ IN VOID *RootInstance,
+ IN UINT64 Address,
+ IN BOOLEAN Write,
+ IN UINTN Width,
+ IN OUT VOID *Data
+ );
+
+/**
+ Callback funciton for EndEnumeration notification from PCI stack.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+ @param[in] Phase The phase of enumeration as informed from PCI stack.
+**/
+VOID
+Ac01PcieHostBridgeNotifyPhase (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex,
+ IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PHASE Phase
+ );
+
+#endif /* AC01_PCIE_LIB_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Library/BoardPcieLib.h b/Silicon/Ampere/AmpereAltraPkg/Include/Library/BoardPcieLib.h
new file mode 100644
index 000000000000..772a85dd5677
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Library/BoardPcieLib.h
@@ -0,0 +1,92 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef BOARD_PCIE_LIB_H_
+#define BOARD_PCIE_LIB_H_
+
+#include <Ac01PcieCommon.h>
+
+/**
+ Override the segment number for a root complex with a board specific number.
+
+ @param[in] RC Root Complex instance with properties.
+ @param[out] SegmentNumber Return segment number.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+EFIAPI
+BoardPcieGetRCSegmentNumber (
+ IN AC01_RC *RC,
+ OUT UINTN *SegmentNumber
+ );
+
+/**
+ Check if SMM PMU enabled in board screen.
+
+ @param[out] IsSmmuPmuEnabled TRUE if the SMMU PMU enabled, otherwise FALSE.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+EFIAPI
+BoardPcieCheckSmmuPmuEnabled (
+ OUT BOOLEAN *IsSmmuPmuEnabled
+ );
+
+/**
+ Build PCIe menu screen.
+
+ @param[in] RCList List of Root Complex with properties.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+EFIAPI
+BoardPcieScreenInitialize (
+ IN AC01_RC *RCList
+ );
+
+/**
+ Parse platform Root Complex information.
+
+ @param[out] RC Root Complex instance to store the platform information.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+EFIAPI
+BoardPcieParseRCParams (
+ OUT AC01_RC *RC
+ );
+
+/**
+ Assert PERST of input PCIe controller
+
+ @param[in] RC Root Complex instance.
+ @param[in] PcieIndex PCIe controller index of input Root Complex.
+ @param[in] Bifurcation Bifurcation mode of input Root Complex.
+ @param[in] IsPullToHigh Target status for the PERST.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+EFIAPI
+BoardPcieAssertPerst (
+ IN AC01_RC *RC,
+ IN UINT32 PcieIndex,
+ IN UINT8 Bifurcation,
+ IN BOOLEAN IsPullToHigh
+ );
+
+#endif /* BOARD_PCIE_LIB_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h b/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h
index 66286bfff145..ed25c6eaa3b8 100644
--- a/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Platform/Ac01.h
@@ -264,15 +264,30 @@
//
#define AC01_PCIE_MMIO_BASE 0x300000000000, 0x340000000000, 0x380000000000, 0x3C0000000000, 0x200000000000, 0x240000000000, 0x280000000000, 0x2C0000000000, 0x700000000000, 0x740000000000, 0x780000000000, 0x7C0000000000, 0x600000000000, 0x640000000000, 0x680000000000, 0x6C0000000000

Hmm, some of these items would appear to have esacped to an earlier
commit that doesn't use/need them.
("Ampere: Initial support for Ampere Altra processor and Mt. Jade platform")

They should be moved to their appropriate place in the timeline.

But this also made me notice something that had passed me by on
initial review of that patch. I will make a separate comment on patch
1 for this, so it doesn't get lost.

But to state it here. The use of lists in #defines kind of pushed me
off the chair at first notice (I must have parsed them as comments on
the first pass). But I'm coming around to it, and I don't see it
explicitly banned by the coding style.

*However* these currently have misleading names.
The names *must* highlight that they are lists.
I guess the least impact would be to append _LIST to the name of any
macro holding a list. Saves on figuring out practical plural forms...

+//
+// The size of MMIO space
+//
+#define AC01_PCIE_MMIO_SIZE 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000, 0x3FFE0000000
+
//
// The base address of MMIO32 Registers
//
-#define AC01_PCIE_MMIO32_BASE 0x000020000000, 0x000028000000, 0x000030000000, 0x000038000000, 0x000001000000, 0x000008000000, 0x000010000000, 0x000018000000, 0x000060000000, 0x000068000000, 0x000070000000, 0x000078000000, 0x000040000000, 0x000048000000, 0x000050000000, 0x000058000000
+#define AC01_PCIE_MMIO32_BASE 0x000020000000, 0x000028000000, 0x000030000000, 0x000038000000, 0x000004000000, 0x000008000000, 0x000010000000, 0x000018000000, 0x000060000000, 0x000068000000, 0x000070000000, 0x000078000000, 0x000040000000, 0x000048000000, 0x000050000000, 0x000058000000
+
+//
+// The size of MMIO32 space
+//
+#define AC01_PCIE_MMIO32_SIZE 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x4000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000, 0x8000000

//
// The base address of MMIO32 Registers
//
-#define AC01_PCIE_MMIO32_BASE_1P 0x000040000000, 0x000050000000, 0x000060000000, 0x000070000000, 0x000001000000, 0x000010000000, 0x000020000000, 0x000030000000, 0, 0, 0, 0, 0, 0, 0, 0
+#define AC01_PCIE_MMIO32_BASE_1P 0x000040000000, 0x000050000000, 0x000060000000, 0x000070000000, 0x000008000000, 0x000010000000, 0x000020000000, 0x000030000000, 0, 0, 0, 0, 0, 0, 0, 0
+
+//
+// The size of MMIO32 1P space
+//
+#define AC01_PCIE_MMIO32_SIZE_1P 0x10000000, 0x10000000, 0x10000000, 0x10000000, 0x8000000, 0x10000000, 0x10000000, 0x10000000, 0, 0, 0, 0, 0, 0, 0, 0

//
// DSDT RCA2 PCIe Meme32 Attribute
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.h b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.h
new file mode 100644
index 000000000000..f84f7cec9fae
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.h
@@ -0,0 +1,649 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AC01_PCIE_CORE_H_
+#define AC01_PCIE_CORE_H_
+
+#include <IndustryStandard/Pci.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PciHostBridgeLib.h>
+#include <Library/PciLib.h>
+#include <Library/TimerLib.h>
+#include <Library/UefiLib.h>
+
+#include <Ac01PcieCommon.h>
+#include "PcieCoreCapCfg.h"
+
+#ifdef ENABLE_DEBUG_CFG
+#define DEBUG_PCIE_CFG(arg...) \
+ if (DebugCodeEnabled()) { \
+ DEBUG ((DEBUG_INFO,"PCICore (CFG): ")); \
+ DEBUG ((DEBUG_INFO,## arg)); \
+ }

Urgh, do we really need this debug infrastructure overlay?

+#else
+#define DEBUG_PCIE_CFG(arg...)
+#endif
+
+#ifdef ENABLE_DEBUG_CSR
+#define DEBUG_PCIE_CSR(arg...) \
+ if (DebugCodeEnabled()) { \
+ DEBUG ((DEBUG_INFO,"PCICore (CSR): ")); \
+ DEBUG ((DEBUG_INFO,## arg)); \
+ }
+#else
+#define DEBUG_PCIE_CSR(arg...)
+#endif
+
+#ifdef ENABLE_DEBUG_PHY
+#define DEBUG_PCIE_PHY(arg...) \
+ if (DebugCodeEnabled()) { \
+ DEBUG ((DEBUG_INFO,"PCICore (PHY): ")); \
+ DEBUG ((DEBUG_INFO,## arg)); \
+ }
+#else
+#define DEBUG_PCIE_PHY(arg...)
+#endif
+
+#ifdef ENABLE_DEBUG_INFO
+#define DEBUG_PCIE_INFO(arg...) \
+ if (DebugCodeEnabled()) { \
+ DEBUG ((DEBUG_INFO,"PCICore (DBG): ")); \
+ DEBUG ((DEBUG_INFO,## arg)); \
+ }
+#else
+#define DEBUG_PCIE_INFO(arg...)
+#endif
+
+#define DEBUG_PCIE_ERR(arg...) \
+ DEBUG ((DEBUG_ERROR,"PCICore (ERROR): ")); \
+ DEBUG ((DEBUG_ERROR,## arg));
+
+#ifndef BIT
+#define BIT(nr) (1 << (nr))
+#endif
+
+
+#define SLOT_POWER_LIMIT 75 // Watt
+
+#define MAX_REINIT 3
+
+#define LINK_CHECK_SUCCESS 0
+#define LINK_CHECK_FAILED -1
+#define LINK_CHECK_WRONG_PARAMETER 1
+
+#define PFA_REG_ENABLE 0
+#define PFA_REG_READ 1
+#define PFA_REG_CLEAR 2
+
+#define AMPERE_PCIE_VENDORID 0x1DEF
+#define AC01_HOST_BRIDGE_DEVICEID_RCA 0xE100
+#define AC01_HOST_BRIDGE_DEVICEID_RCB 0xE110
+#define AC01_PCIE_BRIDGE_DEVICEID_RCA 0xE101
+#define AC01_PCIE_BRIDGE_DEVICEID_RCB 0xE111
+
+#define MEMRDY_TIMEOUT 10 // 10 us
+#define PIPE_CLOCK_TIMEOUT 20000 // 20,000 us
+#define LTSSM_TRANSITION_TIMEOUT 100000 // 100 ms in total
+#define EP_LINKUP_TIMEOUT (10 * 1000) // 10ms
+#define LINK_WAIT_INTERVAL_US 50
+
+#define LINK_POLL_US_TIMER 1
+#define IO_SPACE 0x2000
+
+#define TCU_OFFSET 0
+#define HB_CSR_OFFSET 0x01000000
+#define PCIE0_CSR_OFFSET 0x01010000
+#define SNPSRAM_OFFSET 0x9000
+#define SERDES_CSR_OFFSET 0x01200000
+#define MMCONFIG_OFFSET 0x10000000
+
+
+/* DATA LINK registers */
+#define DLINK_VENDOR_CAP_ID 0x25
+#define DLINK_VSEC 0x80000001
+#define DATA_LINK_FEATURE_CAP_OFF 0X4
+
+/* PL16 CAP registers */
+#define PL16_CAP_ID 0x26
+#define PL16G_CAP_OFF_20H_REG_OFF 0x20
+#define PL16G_STATUS_REG_OFF 0x0C
+#define PL16G_STATUS_EQ_CPL_GET(val) (val & 0x1)
+#define PL16G_STATUS_EQ_CPL_P1_GET(val) ((val & 0x2) >> 1)
+#define PL16G_STATUS_EQ_CPL_P2_GET(val) ((val & 0x4) >> 2)
+#define PL16G_STATUS_EQ_CPL_P3_GET(val) ((val & 0x8) >> 3)
+#define DSP_16G_TX_PRESET0_SET(dst,src) (((dst) & ~0xF) | (((UINT32) (src)) & 0xF))
+#define DSP_16G_TX_PRESET1_SET(dst,src) (((dst) & ~0xF00) | (((UINT32) (src) << 8) & 0xF00))
+#define DSP_16G_TX_PRESET2_SET(dst,src) (((dst) & ~0xF0000) | (((UINT32) (src) << 16) & 0xF0000))
+#define DSP_16G_TX_PRESET3_SET(dst,src) (((dst) & ~0xF000000) | (((UINT32) (src) << 24) & 0xF000000))
+#define DSP_16G_RXTX_PRESET0_SET(dst,src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+#define DSP_16G_RXTX_PRESET1_SET(dst,src) (((dst) & ~0xFF00) | (((UINT32) (src) << 8) & 0xFF00))
+#define DSP_16G_RXTX_PRESET2_SET(dst,src) (((dst) & ~0xFF0000) | (((UINT32) (src) << 16) & 0xFF0000))
+#define DSP_16G_RXTX_PRESET3_SET(dst,src) (((dst) & ~0xFF000000) | (((UINT32) (src) << 24) & 0xFF000000))
+
+/* PCIe PF0_PORT_LOGIC registers */
+#define PORT_LOCIG_VC0_P_RX_Q_CTRL_OFF 0x748
+#define PORT_LOCIG_VC0_NP_RX_Q_CTRL_OFF 0x74C
+
+/* TCU registers */
+#define SMMU_GBPA 0x044
+
+/* SNPSRAM Synopsys Memory Read/Write Margin registers */
+#define SPRF_RMR 0x0
+#define SPSRAM_RMR 0x4
+#define TPRF_RMR 0x8
+#define TPSRAM_RMR 0xC
+
+//
+// Host bridge registers
+//
+#define HBRCAPDMR 0x0
+#define HBRCBPDMR 0x4
+#define HBPDVIDR 0x10
+#define HBPRBNR 0x14
+#define HBPREVIDR 0x18
+#define HBPSIDR 0x1C
+#define HBPCLSSR 0x20
+
+// HBRCAPDMR
+#define RCAPCIDEVMAP_SET(dst, src) (((dst) & ~0x7) | (((UINT32) (src)) & 0x7))
+#define RCAPCIDEVMAP_GET(val) ((val) & 0x7)
+
+// HBRCBPDMR
+#define RCBPCIDEVMAPLO_SET(dst, src) (((dst) & ~0x7) | (((UINT32) (src)) & 0x7))
+#define RCBPCIDEVMAPLO_GET(val) ((val) & 0x7)
+
+#define RCBPCIDEVMAPHI_SET(dst, src) (((dst) & ~0x70) | (((UINT32) (src) << 4) & 0x70))
+#define RCBPCIDEVMAPHI_GET(val) (((val) & 0x7) >> 4)
+
+// HBPDVIDR
+#define PCIVENDID_SET(dst, src) (((dst) & ~0xFFFF) | (((UINT32) (src)) & 0xFFFF))
+#define PCIVENDID_GET(val) ((val) & 0xFFFF)
+
+#define PCIDEVID_SET(dst, src) (((dst) & ~0xFFFF0000) | (((UINT32) (src) << 16) & 0xFFFF0000))
+#define PCIDEVID_GET(val) (((val) & 0xFFFF0000) >> 16)
+
+// HBPRBNR
+#define PCIRBNUM_SET(dst, src) (((dst) & ~0x1F) | (((UINT32) (src)) & 0x1F))
+
+// HBPREVIDR
+#define PCIREVID_SET(dst, src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+
+// HBPSIDR
+#define PCISUBSYSVENDID_SET(dst, src) (((dst) & ~0xFFFF) | (((UINT32) (src)) & 0xFFFF))
+
+#define PCISUBSYSID_SET(dst, src) (((dst) & ~0xFFFF0000) | (((UINT32) (src) << 16) & 0xFFFF0000))
+
+// HBPCLSSR
+#define CACHELINESIZE_SET(dst, src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+
+//
+// PCIE core register
+//
+#define LINKCTRL 0x0
+#define LINKSTAT 0x4
+#define IRQSEL 0xC
+#define HOTPLUGSTAT 0x28
+#define IRQENABLE 0x30
+#define IRQEVENTSTAT 0x38
+#define BLOCKEVENTSTAT 0x3c
+#define RESET 0xC000
+#define CLOCK 0xC004
+#define MEMRDYR 0xC104
+#define RAMSDR 0xC10C
+
+// LINKCTRL
+#define LTSSMENB_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+#define DEVICETYPE_SET(dst, src) (((dst) & ~0xF0) | (((UINT32) (src) << 4) & 0xF0))
+#define DEVICETYPE_GET(val) (((val) & 0xF0) >> 4)
+
+// LINKSTAT
+#define PHY_STATUS_MASK (1 << 2)
+#define SMLH_LTSSM_STATE_MASK 0x3f00
+#define SMLH_LTSSM_STATE_GET(val) ((val & 0x3F00) >> 8)
+#define RDLH_SMLH_LINKUP_STATUS_GET(val) (val & 0x3)
+#define PHY_STATUS_MASK_BIT 0x04
+#define SMLH_LINK_UP_MASK_BIT 0x02
+#define RDLH_LINK_UP_MASK_BIT 0x01
+
+// IRQSEL
+#define AER_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+#define PME_SET(dst, src) (((dst) & ~0x2) | (((UINT32) (src) << 1) & 0x2))
+#define LINKAUTOBW_SET(dst, src) (((dst) & ~0x4) | (((UINT32) (src) << 2) & 0x4))
+#define BWMGMT_SET(dst, src) (((dst) & ~0x8) | (((UINT32) (src) << 3) & 0x8))
+#define EQRQST_SET(dst, src) (((dst) & ~0x10) | (((UINT32) (src) << 4) & 0x10))
+#define INTPIN_SET(dst, src) (((dst) & ~0xFF00) | (((UINT32) (src) << 8) & 0xFF00))
+
+// SLOTCAP
+#define SLOT_HPC_SET(dst, src) (((dst) & ~0x40) | (((UINT32) (src) << 6) & 0x40))
+
+// SLOT_CAPABILITIES_REG, PCIE_CAP_SLOT_POWER_LIMIT_VALUE bits[14:7]
+#define SLOT_CAP_SLOT_POWER_LIMIT_VALUE_SET(dst, src) \
+ (((dst) & ~0x7F80) | (((UINT32)(src) << 7) & 0x7F80))
+
+// HOTPLUGSTAT
+#define PWR_IND_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+#define ATTEN_IND_SET(dst, src) (((dst) & ~0x2) | (((UINT32) (src) << 1) & 0x2))
+#define PWR_CTRL_SET(dst, src) (((dst) & ~0x4) | (((UINT32) (src) << 2) & 0x4))
+#define EML_CTRL_SET(dst, src) (((dst) & ~0x8) | (((UINT32) (src) << 3) & 0x8))
+
+// IRQENABLE
+#define LINKUP_SET(dst, src) (((dst) & ~0x40) | (((UINT32) (src) << 6) & 0x40))
+
+// IRQEVENTSTAT
+#define BLOCK_INT_MASK (1 << 4)
+#define INT_MASK (1 << 3)
+
+// BLOCKEVENTSTAT
+#define LINKUP_MASK (1 << 0)
+
+// RESET
+#define DWCPCIE_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+#define RESET_MASK 0x1
+
+// CLOCK
+#define AXIPIPE_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+// RAMSDR
+#define SD_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+//
+// PHY registers
+//
+#define RSTCTRL 0x0
+#define PHYCTRL 0x4
+#define RAMCTRL 0x8
+#define RAMSTAT 0xC
+#define PLLCTRL 0x10
+#define PHYLPKCTRL 0x14
+#define PHYTERMOFFSET0 0x18
+#define PHYTERMOFFSET1 0x1C
+#define PHYTERMOFFSET2 0x20
+#define PHYTERMOFFSET3 0x24
+#define RXTERM 0x28
+#define PHYDIAGCTRL 0x2C
+
+// RSTCTRL
+#define PHY_RESET_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+// PHYCTRL
+#define PWR_STABLE_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+//
+// PCIe config space registers
+//
+#define TYPE1_DEV_ID_VEND_ID_REG 0
+#define TYPE1_CLASS_CODE_REV_ID_REG 0x8
+#define TYPE1_CAP_PTR_REG 0x34
+#define SEC_LAT_TIMER_SUB_BUS_SEC_BUS_PRI_BUS_REG 0x18
+#define BRIDGE_CTRL_INT_PIN_INT_LINE_REG 0x3c
+#define CON_STATUS_REG (PM_CAP + 0x4)
+#define LINK_CAPABILITIES_REG (PCIE_CAP + 0xc)
+#define LINK_CONTROL_LINK_STATUS_REG (PCIE_CAP + 0x10)
+#define SLOT_CAPABILITIES_REG (PCIE_CAP + 0x14)
+#define DEVICE_CONTROL2_DEVICE_STATUS2_REG (PCIE_CAP + 0x28)
+#define LINK_CAPABILITIES2_REG (PCIE_CAP + 0x2c)
+#define LINK_CONTROL2_LINK_STATUS2_REG (PCIE_CAP + 0x30)
+#define UNCORR_ERR_STATUS_OFF (AER_CAP + 0x4)
+#define UNCORR_ERR_MASK_OFF (AER_CAP + 0x8)
+#define RESOURCE_CON_REG_VC0 (VC_CAP + 0x14)
+#define RESOURCE_CON_REG_VC1 (VC_CAP + 0x20)
+#define RESOURCE_STATUS_REG_VC1 (VC_CAP + 0x24)
+#define SD_CONTROL1_REG (RAS_DES_CAP+0xA0)
+#define CCIX_TP_CAP_TP_HDR2_OFF (CCIX_TP_CAP + 0x8)
+#define ESM_MNDTRY_RATE_CAP_OFF (CCIX_TP_CAP + 0xc)
+#define ESM_STAT_OFF (CCIX_TP_CAP + 0x14)
+#define ESM_CNTL_OFF (CCIX_TP_CAP + 0x18)
+#define ESM_LN_EQ_CNTL_25G_0_OFF (CCIX_TP_CAP + 0x2c)
+#define PORT_LINK_CTRL_OFF 0x710
+#define FILTER_MASK_2_OFF 0x720
+#define GEN2_CTRL_OFF 0x80c
+#define GEN3_RELATED_OFF 0x890
+#define GEN3_EQ_CONTROL_OFF 0x8A8
+#define MISC_CONTROL_1_OFF 0x8bc
+#define AMBA_ERROR_RESPONSE_DEFAULT_OFF 0x8d0
+#define AMBA_LINK_TIMEOUT_OFF 0x8d4
+#define AMBA_ORDERING_CTRL_OFF 0x8d8
+#define DTIM_CTRL0_OFF 0xab0
+#define AUX_CLK_FREQ_OFF 0xb40
+#define CCIX_CTRL_OFF 0xc20
+
+#define DEV_MASK 0x00F8000
+#define BUS_MASK 0xFF00000
+#define BUS_NUM(Addr) ((((UINT64)(Addr)) & BUS_MASK) >> 20)
+#define DEV_NUM(Addr) ((((UINT64)(Addr)) & DEV_MASK) >> 15)
+#define CFG_REG(Addr) (((UINT64)Addr) & 0x7FFF)
+
+// TYPE1_DEV_ID_VEND_ID_REG
+#define VENDOR_ID_SET(dst, src) (((dst) & ~0xFFFF) | (((UINT32) (src)) & 0xFFFF))
+#define DEVICE_ID_SET(dst, src) (((dst) & ~0xFFFF0000) | (((UINT32) (src) << 16) & 0xFFFF0000))
+
+// TYPE1_CLASS_CODE_REV_ID_REG
+#define BASE_CLASS_CODE_SET(dst, src) (((dst) & ~0xFF000000) | (((UINT32) (src) << 24) & 0xFF000000))
+#define SUBCLASS_CODE_SET(dst, src) (((dst) & ~0xFF0000) | (((UINT32) (src) << 16) & 0xFF0000))
+#define PROGRAM_INTERFACE_SET(dst, src) (((dst) & ~0xFF00) | (((UINT32) (src) << 8) & 0xFF00))
+#define REVISION_ID_SET(dst, src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+
+// SEC_LAT_TIMER_SUB_BUS_SEC_BUS_PRI_BUS_REG
+#define DEFAULT_SUB_BUS 0xFF
+#define SUB_BUS_SET(dst, src) (((dst) & ~0xFF0000) | (((UINT32) (src) << 16) & 0xFF0000))
+#define SEC_BUS_SET(dst, src) (((dst) & ~0xFF00) | (((UINT32) (src) << 8) & 0xFF00))
+#define PRIM_BUS_SET(dst, src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+
+// BRIDGE_CTRL_INT_PIN_INT_LINE_REG
+#define INT_PIN_SET(dst, src) (((dst) & ~0xFF00) | (((UINT32) (src) << 8) & 0xFF00))
+
+// CON_STATUS_REG
+#define POWER_STATE_SET(dst, src) (((dst) & ~0x3) | (((UINT32) (src)) & 0x3))
+
+// DEVICE_CONTROL2_DEVICE_STATUS2_REG
+#define CAP_CPL_TIMEOUT_VALUE_SET(dst, src) (((dst) & ~0xF) | (((UINT32) (src)) & 0xF))
+
+// LINK_CAPABILITIES_REG
+#define CAP_ID 0x10
+#define LINK_CAPABILITIES_REG_OFF 0xC
+#define LINK_CONTROL_LINK_STATUS_OFF 0x10
+#define CAP_MAX_LINK_WIDTH_X1 0x1
+#define CAP_MAX_LINK_WIDTH_X2 0x2
+#define CAP_MAX_LINK_WIDTH_X4 0x4
+#define CAP_MAX_LINK_WIDTH_X8 0x8
+#define CAP_MAX_LINK_WIDTH_X16 0x10
+#define CAP_MAX_LINK_WIDTH_GET(val) ((val & 0x3F0) >> 4)
+#define CAP_MAX_LINK_WIDTH_SET(dst, src) (((dst) & ~0x3F0) | (((UINT32) (src) << 4) & 0x3F0))
+#define MAX_LINK_SPEED_25 0x1
+#define MAX_LINK_SPEED_50 0x2
+#define MAX_LINK_SPEED_80 0x3
+#define MAX_LINK_SPEED_160 0x4
+#define MAX_LINK_SPEED_320 0x5
+#define CAP_MAX_LINK_SPEED_GET(val) ((val & 0xF))
+#define CAP_MAX_LINK_SPEED_SET(dst, src) (((dst) & ~0xF) | (((UINT32) (src)) & 0xF))
+#define CAP_SLOT_CLK_CONFIG_SET(dst, src) (((dst) & ~0x10000000) | (((UINT32) (src) << 28) & 0x10000000))
+#define NO_ASPM_SUPPORTED 0x0
+#define L0S_SUPPORTED 0x1
+#define L1_SUPPORTED 0x2
+#define L0S_L1_SUPPORTED 0x3
+#define CAP_ACTIVE_STATE_LINK_PM_SUPPORT_SET(dst, src) (((dst) & ~0xC00) | (((UINT32)(src) << 10) & 0xC00))
+
+// LINK_CONTROL_LINK_STATUS_REG
+#define CAP_DLL_ACTIVE_GET(val) ((val & 0x20000000) >> 29)
+#define CAP_NEGO_LINK_WIDTH_GET(val) ((val & 0x3F00000) >> 20)
+#define CAP_LINK_SPEED_GET(val) ((val & 0xF0000) >> 16)
+#define CAP_LINK_SPEED_SET(dst, src) (((dst) & ~0xF0000) | (((UINT32) (src) << 16) & 0xF0000))
+#define CAP_LINK_SPEED_TO_VECTOR(val) BIT((val)-1)
+#define CAP_EN_CLK_POWER_MAN_GET(val) ((val & 0x100) >> 8)
+#define CAP_EN_CLK_POWER_MAN_SET(dst, src) (((dst) & ~0x100) | (((UINT32) (src) << 8) & 0x100))
+#define CAP_RETRAIN_LINK_SET(dst, src) (((dst) & ~0x20) | (((UINT32) (src) << 5) & 0x20))
+#define CAP_COMMON_CLK_SET(dst, src) (((dst) & ~0x40) | (((UINT32) (src) << 6) & 0x40))
+#define CAP_LINK_TRAINING_GET(val) ((val & 0x8000000) >> 27)
+#define CAP_LINK_DISABLE_SET(dst, src) (((dst) & ~0x10) | (((UINT32)(src) << 4) & 0x10))
+
+// LINK_CAPABILITIES2_REG
+#define LINK_SPEED_VECTOR_25 BIT(0)

Use BIT0-BIT63 from Base.h instead of the macro when referring to constants.

+#define LINK_SPEED_VECTOR_50 BIT(1)
+#define LINK_SPEED_VECTOR_80 BIT(2)
+#define LINK_SPEED_VECTOR_160 BIT(3)
+#define LINK_SPEED_VECTOR_320 BIT(4)
+#define CAP_SUPPORT_LINK_SPEED_VECTOR_GET(val) ((val & 0xFE) >> 1)
+#define CAP_SUPPORT_LINK_SPEED_VECTOR_SET(dst, src) (((dst) & ~0xFE) | (((UINT32) (src) << 1) & 0xFE))

Umm, all of these LINK_SPEED_VECTOR macros exist only in this file.
If they are intended to be used only in later patches, please hold
them back until then.

+#define CAP_EQ_CPL_GET(val) ((val & 0x20000) >> 17)
+#define CAP_EQ_CPL_P1_GET(val) ((val & 0x40000) >> 18)
+#define CAP_EQ_CPL_P2_GET(val) ((val & 0x80000) >> 19)
+#define CAP_EQ_CPL_P3_GET(val) ((val & 0x100000) >> 20)

Also unused.
Could this (very long) header be pruned of contents not actually used?

+
+// LINK_CONTROL2_LINK_STATUS2_REG
+#define CAP_TARGET_LINK_SPEED_SET(dst, src) (((dst) & ~0xF) | (((UINT32) (src)) & 0xF))
+
+// Secondary Capability
+#define SPCIE_CAP_ID 0x19
+#define CAP_OFF_0C 0x0C
+#define LINK_CONTROL3_REG_OFF 0x4
+#define DSP_TX_PRESET0_SET(dst,src) (((dst) & ~0xF) | (((UINT32) (src)) & 0xF))
+#define DSP_TX_PRESET1_SET(dst,src) (((dst) & ~0xF0000) | (((UINT32) (src) << 16) & 0xF0000))
+
+// UNCORR_ERR_STATUS_OFF
+#define CMPLT_TIMEOUT_ERR_STATUS_GET(val) ((val & 0x4000) >> 14)
+#define CMPLT_TIMEOUT_ERR_STATUS_SET(dst, src) (((dst) & ~0x4000) | (((UINT32) (src) << 14) & 0x4000))
+
+// UNCORR_ERR_MASK_OFF
+#define CMPLT_TIMEOUT_ERR_MASK_SET(dst, src) (((dst) & ~0x4000) | (((UINT32) (src) << 14) & 0x4000))
+#define SDES_ERR_MASK_SET(dst, src) (((dst) & ~0x20) | (((UINT32)(src) << 5) & 0x20))
+
+// RESOURCE_STATUS_REG_VC1
+#define VC_NEGO_PENDING_VC1_GET(val) ((val & 0x20000) >> 17)
+
+// SD_CONTROL1_REG
+#define FORCE_DETECT_LANE_EN_SET(dst, src) (((dst) & ~0x10000) | (((UINT32) (src) << 16) & 0x10000))
+
+// CCIX_TP_CAP_TP_HDR2_OFF
+#define ESM_REACH_LENGTH_GET(val) ((val & 0x60000) >> 17)
+#define ESM_CALIBRATION_TIME_GET(val) ((val & 0x700000) >> 20)
+#define ESM_CALIBRATION_TIME_SET(dst, src) (((dst) & ~0x700000) | (((UINT32) (src) << 20) & 0x700000))
+
+// ESM_STAT_OFF
+#define ESM_CALIB_CMPLT_GET(val) ((val & 0x80) >> 7)
+#define ESM_CURNT_DATA_RATE_GET(val) ((val & 0x7F) >> 0)
+
+// ESM_CNTL_OFF
+#define QUICK_EQ_TIMEOUT_SET(dst, src) (((dst) & ~0x1C000000) | (((UINT32) (src) << 26) & 0x1C000000))
+#define LINK_REACH_TARGET_GET(val) ((val & 0x1000000) >> 24)
+#define LINK_REACH_TARGET_SET(dst, src) (((dst) & ~0x1000000) | (((UINT32) (src) << 24) & 0x1000000))
+#define ESM_EXT_EQ3_DSP_TIMEOUT_GET(val) ((val & 0x700000) >> 20)
+#define ESM_EXT_EQ3_DSP_TIMEOUT_SET(dst, src) (((dst) & ~0x700000) | (((UINT32) (src) << 20) & 0x700000))
+#define ESM_EXT_EQ2_USP_TIMEOUT_GET(val) ((val & 0x70000) >> 16)
+#define ESM_EXT_EQ2_USP_TIMEOUT_SET(dst, src) (((dst) & ~0x70000) | (((UINT32) (src) << 16) & 0x70000))
+#define ESM_ENABLE_SET(dst, src) (((dst) & ~0x8000) | (((UINT32) (src) << 15) & 0x8000))
+#define ESM_DATA_RATE1_SET(dst, src) (((dst) & ~0x7F00) | (((UINT32) (src) << 8) & 0x7F00))
+#define ESM_PERFORM_CAL_SET(dst, src) (((dst) & ~0x80) | (((UINT32) (src) << 7) & 0x80))
+#define ESM_DATA_RATE0_SET(dst, src) (((dst) & ~0x7F) | (((UINT32) (src)) & 0x7F))
+
+// PORT_LINK_CTRL_OFF
+#define LINK_CAPABLE_X1 0x1
+#define LINK_CAPABLE_X2 0x3
+#define LINK_CAPABLE_X4 0x7
+#define LINK_CAPABLE_X8 0xF
+#define LINK_CAPABLE_X16 0x1F
+#define LINK_CAPABLE_X32 0x3F
+#define LINK_CAPABLE_SET(dst, src) (((dst) & ~0x3F0000) | (((UINT32) (src) << 16) & 0x3F0000))
+#define FAST_LINK_MODE_SET(dst, src) (((dst) & ~0x80) | (((UINT32) (src) << 7) & 0x80))
+
+// FILTER_MASK_2_OFF
+#define CX_FLT_MASK_VENMSG0_DROP_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+#define CX_FLT_MASK_VENMSG1_DROP_SET(dst, src) (((dst) & ~0x2) | (((UINT32) (src) << 1) & 0x2))
+#define CX_FLT_MASK_DABORT_4UCPL_SET(dst, src) (((dst) & ~0x4) | (((UINT32) (src) << 2) & 0x4))
+
+// GEN2_CTRL_OFF
+#define NUM_OF_LANES_X2 0x2
+#define NUM_OF_LANES_X4 0x4
+#define NUM_OF_LANES_X8 0x8
+#define NUM_OF_LANES_X16 0x10
+#define NUM_OF_LANES_SET(dst, src) (((dst) & ~0x1F00) | (((UINT32) (src) << 8) & 0x1F00))
+
+// GEN3_RELATED_OFF
+#define RATE_SHADOW_SEL_SET(dst, src) (((dst) & ~0x3000000) | (((UINT32) (src) << 24) & 0x3000000))
+#define EQ_PHASE_2_3_SET(dst, src) (((dst) & ~0x200) | (((UINT32) (src) << 9) & 0x200))
+#define RXEQ_REGRDLESS_SET(dst, src) (((dst) & ~0x2000) | (((UINT32) (src) << 13) & 0x2000))
+
+// GEN3_EQ_CONTROL_OFF
+#define GEN3_EQ_FB_MODE(dst, src) (((dst) & ~0xF) | ((UINT32) (src) & 0xF))
+#define GEN3_EQ_PRESET_VEC(dst, src) (((dst) & 0xFF0000FF) | (((UINT32) (src) << 8) & 0xFFFF00))
+#define GEN3_EQ_INIT_EVAL(dst,src) (((dst) & ~0x1000000) | (((UINT32) (src) << 24) & 0x1000000))
+
+// MISC_CONTROL_1_OFF
+#define DBI_RO_WR_EN_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+// AMBA_ERROR_RESPONSE_DEFAULT_OFF
+#define AMBA_ERROR_RESPONSE_CRS_SET(dst, src) (((dst) & ~0x18) | (((UINT32) (src) << 3) & 0x18))
+#define AMBA_ERROR_RESPONSE_GLOBAL_SET(dst, src) (((dst) & ~0x1) | (((UINT32) (src)) & 0x1))
+
+// AMBA_LINK_TIMEOUT_OFF
+#define LINK_TIMEOUT_PERIOD_DEFAULT_SET(dst, src) (((dst) & ~0xFF) | (((UINT32) (src)) & 0xFF))
+
+// AMBA_ORDERING_CTRL_OFF
+#define AX_MSTR_ZEROLREAD_FW_SET(dst, src) (((dst) & ~0x80) | (((UINT32) (src) << 7) & 0x80))
+
+// DTIM_CTRL0_OFF
+#define DTIM_CTRL0_ROOT_PORT_ID_SET(dst, src) (((dst) & ~0xFFFF) | (((UINT32) (src)) & 0xFFFF))
+
+// AUX_CLK_FREQ_OFF
+#define AUX_CLK_500MHZ 500
+#define AUX_CLK_FREQ_SET(dst, src) (((dst) & ~0x1FF) | (((UINT32) (src)) & 0x1FF))
+
+#define EXT_CAP_OFFSET_START 0x100
+
+// EVENT_COUNTER_CONTROL_REG
+#define ECCR_GROUP_EVENT_SEL_SET(dst, src) (((dst) & ~0xFFF0000) | (((UINT32)(src) << 16) & 0xFFF0000))
+#define ECCR_GROUP_SEL_SET(dst, src) (((dst) & ~0xF000000) | (((UINT32)(src) << 24) & 0xF000000))
+#define ECCR_EVENT_SEL_SET(dst, src) (((dst) & ~0xFF0000) | (((UINT32)(src) << 16) & 0xFF0000))
+#define ECCR_LANE_SEL_SET(dst, src) (((dst) & ~0xF00) | (((UINT32)(src) << 8) & 0xF00))
+#define ECCR_EVENT_COUNTER_ENABLE_SET(dst, src) (((dst) & ~0x1C) | (((UINT32)(src) << 2) & 0x1C))
+#define ECCR_EVENT_COUNTER_CLEAR_SET(dst, src) (((dst) & ~0x3) | (((UINT32)(src)) & 0x3))
+
+// PFA Registers offests
+#define RAS_DES_CAP_ID 0xB
+#define EVENT_COUNTER_CONTROL_REG_OFF 0x8
+#define EVENT_COUNTER_DATA_REG_OFF 0xC
+
+#define MAX_NUM_ERROR_CODE 47
+#define DEFAULT_COMMON_LANE_NUM 15
+
+enum LTSSM_STATE {

See enum style rules mentioned for a previous file.

+ S_DETECT_QUIET = 0,
+ S_DETECT_ACT,
+ S_POLL_ACTIVE,
+ S_POLL_COMPLIANCE,
+ S_POLL_CONFIG,
+ S_PRE_DETECT_QUIET,
+ S_DETECT_WAIT,
+ S_CFG_LINKWD_START,
+ S_CFG_LINKWD_ACEPT,
+ S_CFG_LANENUM_WAI,
+ S_CFG_LANENUM_ACEPT,
+ S_CFG_COMPLETE,
+ S_CFG_IDLE,
+ S_RCVRY_LOCK,
+ S_RCVRY_SPEED,
+ S_RCVRY_RCVRCFG,
+ S_RCVRY_IDLE,
+ S_L0,
+ S_L0S,
+ S_L123_SEND_EIDLE,
+ S_L1_IDLE,
+ S_L2_IDLE,
+ S_L2_WAKE,
+ S_DISABLED_ENTRY,
+ S_DISABLED_IDLE,
+ S_DISABLED,
+ S_LPBK_ENTRY,
+ S_LPBK_ACTIVE,
+ S_LPBK_EXIT,
+ S_LPBK_EXIT_TIMEOUT,
+ S_HOT_RESET_ENTRY,
+ S_HOT_RESET,
+ S_RCVRY_EQ0,
+ S_RCVRY_EQ1,
+ S_RCVRY_EQ2,
+ S_RCVRY_EQ3,
+ MAX_LTSSM_STATE
+};
+
+INT32
+Ac01PcieCfgOut32 (
+ VOID *Addr,
+ UINT32 Val
+ );
+
+INT32
+Ac01PcieCfgOut16 (
+ VOID *Addr,
+ UINT16 Val
+ );
+
+INT32
+Ac01PcieCfgOut8 (
+ VOID *Addr,
+ UINT8 Val
+ );
+
+INT32
+Ac01PcieCfgIn32 (
+ VOID *Addr,
+ UINT32 *Val
+ );
+
+INT32
+Ac01PcieCfgIn16 (
+ VOID *Addr,
+ UINT16 *Val
+ );
+
+INT32
+Ac01PcieCfgIn8 (
+ VOID *Addr,
+ UINT8 *Val
+ );
+
+INT32
+Ac01PcieCoreSetup (
+ AC01_RC *RC
+ );
+
+VOID
+Ac01PcieCoreResetPort (
+ AC01_RC *RC
+ );
+
+VOID
+Ac01PcieClearConfigPort (
+ AC01_RC *RC
+ );
+
+AC01_RC *
+GetRCList (
+ UINT8 Idx
+ );
+
+VOID
+Ac01PcieCoreBuildRCStruct (
+ AC01_RC *RC,
+ UINT64 RegBase,
+ UINT64 MmioBase,
+ UINT64 MmioSize,
+ UINT64 Mmio32Base,
+ UINT64 Mmio32Size
+ );
+
+INT32
+Ac01PcieCoreSetupRC (
+ IN AC01_RC *RC,
+ IN UINT8 ReInit,
+ IN UINT8 ReInitPcieIndex
+ );
+
+VOID
+Ac01PcieCoreUpdateLink (
+ IN AC01_RC *RC,
+ OUT BOOLEAN *IsNextRoundNeeded,
+ OUT INT8 *FailedPciePtr,
+ OUT INT8 *FailedPcieCount
+ );
+
+VOID
+Ac01PcieCoreEndEnumeration (
+ AC01_RC *RC
+ );
+
+INT32
+Ac01PcieCoreQoSLinkCheckRecovery (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex
+ );
+
+#endif /* AC01_PCIE_CORE_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreCapCfg.h b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreCapCfg.h
new file mode 100644
index 000000000000..c3914673b852
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreCapCfg.h
@@ -0,0 +1,63 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AC01_PCIE_CAP_CFG_H_
+#define AC01_PCIE_CAP_CFG_H_
+
+/* PCIe config space capabilities offset */
+#define PM_CAP 0x40
+#define MSI_CAP 0x50
+#define PCIE_CAP 0x70
+#define MSIX_CAP 0xB0
+#define SLOT_CAP 0xC0
+#define VPD_CAP 0xD0
+#define SATA_CAP 0xE0
+#define CFG_NEXT_CAP 0x40
+#define PM_NEXT_CAP 0x70
+#define MSI_NEXT_CAP 0x00
+#define PCIE_NEXT_CAP 0x00
+#define MSIX_NEXT_CAP 0x00
+#define SLOT_NEXT_CAP 0x00
+#define VPD_NEXT_CAP 0x00
+#define SATA_NEXT_CAP 0x00
+#define BASE_CAP 0x100
+#define AER_CAP 0x100
+#define VC_CAP 0x148
+#define SN_CAP 0x178
+#define PB_CAP 0x178
+#define ARI_CAP 0x178
+#define SPCIE_CAP_x8 0x148
+#define SPCIE_CAP 0x178
+#define PL16G_CAP 0x1A8
+#define MARGIN_CAP 0x1D8
+#define PL32G_CAP 0x220
+#define SRIOV_CAP 0x220
+#define TPH_CAP 0x220
+#define ATS_CAP 0x220
+#define ACS_CAP 0x230
+#define PRS_CAP 0x238
+#define LTR_CAP 0x248
+#define L1SUB_CAP 0x248
+#define PASID_CAP 0x248
+#define DPA_CAP 0x248
+#define DPC_CAP 0x248
+#define MPCIE_CAP 0x248
+#define FRSQ_CAP 0x248
+#define RTR_CAP 0x248
+#define LN_CAP 0x248
+#define RAS_DES_CAP 0x248
+#define VSECRAS_CAP 0x348
+#define DLINK_CAP 0x380
+#define PTM_CAP 0x38C
+#define PTM_VSEC_CAP 0x38C
+#define CCIX_TP_CAP 0x38C
+#define CXS_CAP 0x3D0
+#define RBAR_CAP 0x3E8
+#define VF_RBAR_CAP 0x3E8
+
+#endif /* AC01_PCIE_CAP_CFG_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.h b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.h
new file mode 100644
index 000000000000..2dfefdf97159
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.h
@@ -0,0 +1,30 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef AC01_PCIE_PATCH_ACPI_H_
+#define AC01_PCIE_PATCH_ACPI_H_
+
+EFI_STATUS
+EFIAPI
+AcpiPatchPciMem32 (
+ INT8 *PciSegEnabled
+ );
+
+EFI_STATUS
+EFIAPI
+AcpiInstallMcfg (
+ INT8 *PciSegEnabled
+ );
+
+EFI_STATUS
+EFIAPI
+AcpiInstallIort (
+ INT8 *PciSegEnabled
+ );
+
+#endif /* AC01_PCIE_PATCH_ACPI_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
new file mode 100644
index 000000000000..684dd808a96a
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCore.c
@@ -0,0 +1,1659 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Guid/PlatformInfoHobGuid.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BoardPcieLib.h>
+#include <Library/HobLib.h>
+#include <Library/PciePhyLib.h>
+#include <Library/SystemFirmwareInterfaceLib.h>
+#include <PlatformInfoHob.h>
+
+#include "PcieCore.h"
+
+STATIC INT32
+Ac01PcieCsrOut32 (
+ VOID *Addr,
+ UINT32 Val
+ )
+{
+ MmioWrite32 ((UINT64)Addr, Val);
+ DEBUG_PCIE_CSR (
+ "PCIE CSR WR: 0x%p value: 0x%08X (0x%08X)\n",
+ Addr,
+ Val,
+ MmioRead32 ((UINT64)Addr)
+ );
+
+ return 0;
+}
+
+STATIC INT32
+Ac01PcieCsrOut32Serdes (
+ VOID *Addr,
+ UINT32 Val
+ )
+{
+ MmioWrite32 ((UINT64)Addr, Val);
+ DEBUG_PCIE_CSR (
+ "PCIE CSR WR: 0x%p value: 0x%08X (0x%08X)\n",
+ Addr,
+ Val,
+ MmioRead32 ((UINT64)Addr)
+ );
+
+ return 0;
+}
+
+STATIC INT32
+Ac01PcieCsrIn32 (
+ VOID *Addr,
+ UINT32 *Val
+ )
+{
+ *Val = MmioRead32 ((UINT64)Addr);
+ DEBUG_PCIE_CSR ("PCIE CSR RD: 0x%p value: 0x%08X\n", Addr, *Val);
+
+ return 0;
+}
+
+STATIC INT32
+Ac01PcieCsrIn32Serdes (
+ VOID *Addr,
+ UINT32 *Val
+ )
+{
+ *Val = MmioRead32 ((UINT64)Addr);
+ DEBUG_PCIE_CSR ("PCIE CSR RD: 0x%p value: 0x%08X\n", Addr, *Val);
+
+ return 0;
+}
+
+VOID
+Ac01PcieMmioRd (
+ UINT64 Addr,
+ UINT32 *Val
+ )
+{
+ Ac01PcieCsrIn32Serdes ((VOID *)Addr, (UINT32 *)Val);
+}
+
+VOID
+Ac01PcieMmioWr (
+ UINT64 Addr,
+ UINT32 Val
+ )
+{
+ Ac01PcieCsrOut32Serdes ((VOID *)Addr, (UINT32)Val);
+}
+
+VOID
+Ac01PciePuts (

Wait, what. We have *two* sets of output overlays in this patch?

+ CONST CHAR8 *Msg
+ )
+{
+ DEBUG_PCIE_PHY ("%a\n", __FUNCTION__);

Err, no, just a further level of abstraction, seemingly throwing away
the subsystem aspect when actually called here?

+}
+
+VOID
+Ac01PciePutInt (
+ UINT32 val
+ )
+{
+ DEBUG_PCIE_PHY ("%a\n", __FUNCTION__);
+}
+
+VOID
+Ac01PciePutHex (
+ UINT64 val
+ )
+{
+ DEBUG_PCIE_PHY ("%a\n", __FUNCTION__);
+}
+
+INT32
+Ac01PcieDebugPrint (
+ CONST CHAR8 *fmt,
+ ...
+ )
+{
+ DEBUG_PCIE_PHY ("%a\n", __FUNCTION__);
+ return 0;
+}
+
+VOID
+Ac01PcieDelay (
+ UINT32 Val
+ )
+{
+ MicroSecondDelay (Val);

No, use MicroSecondDelay directly.

+}
+
+/**
+ Write 32-bit value to config space address
+
+ @param Addr Address within config space
+ @param Val 32-bit value to write
+**/
+INT32
+Ac01PcieCfgOut32 (
+ IN VOID *Addr,
+ IN UINT32 Val
+ )
+{
+ MmioWrite32 ((UINT64)Addr, Val);
+ DEBUG_PCIE_CFG (
+ "PCIE CFG WR: 0x%p value: 0x%08X (0x%08X)\n",
+ Addr,
+ Val,
+ MmioRead32 ((UINT64)Addr)
+ );
+
+ return 0;
+}
+
+/**
+ Write 16-bit value to config space address
+
+ @param Addr Address within config space
+ @param Val 16-bit value to write
+**/
+INT32
+Ac01PcieCfgOut16 (
+ IN VOID *Addr,
+ IN UINT16 Val
+ )
+{
+ UINT64 AlignedAddr = (UINT64)Addr & ~0x3;
+ UINT32 Val32 = MmioRead32 (AlignedAddr);
+
+ switch ((UINT64)Addr & 0x3) {
+ case 2:
+ Val32 &= ~0xFFFF0000;
+ Val32 |= (UINT32)Val << 16;
+ break;
+
+ case 0:
+ default:
+ Val32 &= ~0xFFFF;
+ Val32 |= Val;
+ break;
+ }
+ MmioWrite32 (AlignedAddr, Val32);
+ DEBUG_PCIE_CFG (
+ "PCIE CFG WR16: 0x%p value: 0x%04X (0x%08llX 0x%08X)\n",
+ Addr,
+ Val,
+ AlignedAddr,
+ MmioRead32 ((UINT64)AlignedAddr)
+ );
+
+ return 0;
+}
+
+/**
+ Write 8-bit value to config space address
+
+ @param Addr Address within config space
+ @param Val 8-bit value to write
+**/
+INT32
+Ac01PcieCfgOut8 (
+ IN VOID *Addr,
+ IN UINT8 Val
+ )
+{
+ UINT64 AlignedAddr = (UINT64)Addr & ~0x3;
+ UINT32 Val32 = MmioRead32 (AlignedAddr);
+
+ switch ((UINT64)Addr & 0x3) {
+ case 0:
+ Val32 &= ~0xFF;
+ Val32 |= Val;
+ break;
+
+ case 1:
+ Val32 &= ~0xFF00;
+ Val32 |= (UINT32)Val << 8;
+ break;
+
+ case 2:
+ Val32 &= ~0xFF0000;
+ Val32 |= (UINT32)Val << 16;
+ break;
+
+ case 3:
+ default:
+ Val32 &= ~0xFF000000;
+ Val32 |= (UINT32)Val << 24;
+ break;
+ }
+ MmioWrite32 (AlignedAddr, Val32);
+ DEBUG_PCIE_CFG (
+ "PCIE CFG WR8: 0x%p value: 0x%04X (0x%08llX 0x%08X)\n",
+ Addr,
+ Val,
+ AlignedAddr,
+ MmioRead32 ((UINT64)AlignedAddr)
+ );
+
+ return 0;
+}
+
+/**
+ Read 32-bit value from config space address
+
+ @param Addr Address within config space
+ @param Val Point to address for read value
+**/
+INT32
+Ac01PcieCfgIn32 (
+ IN VOID *Addr,
+ OUT UINT32 *Val
+ )
+{
+ UINT32 RegC, Reg18;
+ UINT8 MfHt, Ht, Primary = 0, Sec = 0, Sub = 0;
+
+ if ((BUS_NUM (Addr) > 0) && (DEV_NUM (Addr) > 0) && (CFG_REG (Addr) == 0)) {
+ *Val = MmioRead32 ((UINT64)Addr);
+ DEBUG_PCIE_CFG (
+ "PCIE CFG RD: B%X|D%X 0x%p value: 0x%08X\n",
+ BUS_NUM (Addr),
+ DEV_NUM (Addr),
+ Addr,
+ *Val
+ );
+
+ if (*Val != 0xffffffff) {
+ RegC = MmioRead32 ((UINT64)Addr + 0xC);
+ DEBUG_PCIE_CFG ("Peek PCIE MfHt RD32: 0x%p value: 0x%08X\n", Addr + 0xc, RegC);
+ MfHt = RegC >> 16;
+ DEBUG_PCIE_CFG (" Peek RD8 MfHt=0x%02X\n", MfHt);
+
+ Ht = MfHt & 0x7F;
+ if (Ht != 0) { /* Type 1 header */
+ Reg18 = MmioRead32 ((UINT64)Addr + 0x18);
+ Primary = Reg18; Sec = Reg18 >> 8; Sub = Reg18 >> 16;
+ DEBUG_PCIE_CFG (
+ " Bus Peek PCIE Sub:%01X Sec:%01X Primary:%01X RD: 0x%p value: 0x%08X\n",
+ Sub,
+ Sec,
+ Primary,
+ Addr + 0x18,
+ Reg18
+ );
+ }
+ if ((Ht == 0) || (Primary != 0)) { /* Ampere Altra RPs Primary Bus is 0b */
+ *Val = 0xffffffff;
+ DEBUG_PCIE_CFG (
+ " Skip RD32 B%X|D%X PCIE CFG RD: 0x%p return 0xffffffff\n",
+ BUS_NUM (Addr),
+ DEV_NUM (Addr),
+ Addr
+ );
+ }
+ }
+ } else {
+ *Val = MmioRead32 ((UINT64)Addr);
+ }
+ DEBUG_PCIE_CFG ("PCIE CFG RD: 0x%p value: 0x%08X\n", Addr, *Val);
+
+ return 0;
+}
+
+/**
+ Read 16-bit value from config space address
+
+ @param Addr Address within config space
+ @param Val Point to address for read value
+**/
+INT32
+Ac01PcieCfgIn16 (
+ IN VOID *Addr,
+ OUT UINT16 *Val
+ )
+{
+ UINT64 AlignedAddr = (UINT64)Addr & ~0x3;
+ UINT32 RegC, Reg18;
+ UINT8 MfHt, Primary = 0, Sec = 0, Sub = 0;
+ UINT32 Val32;
+
+ if ((BUS_NUM (Addr) > 0) && (DEV_NUM (Addr) > 0) && (CFG_REG (Addr) == 0)) {
+ *Val = MmioRead32 ((UINT64)Addr);
+ DEBUG_PCIE_CFG (
+ "PCIE CFG16 RD: B%X|D%X 0x%p value: 0x%08X\n",
+ BUS_NUM (Addr),
+ DEV_NUM (Addr),
+ Addr,
+ *Val
+ );
+
+ if (*Val != 0xffff) {
+ RegC = MmioRead32 ((UINT64)Addr + 0xC);
+ DEBUG_PCIE_CFG (" Peek PCIE MfHt RD: 0x%p value: 0x%08X\n", Addr + 0xc, RegC);
+ MfHt = RegC >> 16;
+ DEBUG_PCIE_CFG (" Peek RD8 MfHt=0x%02X\n", MfHt);
+
+
+ if ((MfHt & 0x7F)!= 0) { /* Type 1 header */
+ Reg18 = MmioRead32 ((UINT64)Addr + 0x18);
+ Primary = Reg18; Sec = Reg18 >> 8; Sub = Reg18 >> 16;
+ DEBUG_PCIE_CFG (
+ " Bus Peek PCIE Sub:%01X Sec:%01X Primary:%01X RD: 0x%p value: 0x%08X\n",
+ Sub,
+ Sec,
+ Primary,
+ Addr + 0x18,
+ Reg18
+ );
+ }
+ if ((MfHt == 0) || (Primary != 0)) { /* QS RPs Primary Bus is 0b */
+ *Val = 0xffff;
+ DEBUG_PCIE_CFG (
+ " Skip RD16 B%X|D%X PCIE CFG RD: 0x%p return 0xffff\n",
+ BUS_NUM (Addr),
+ DEV_NUM (Addr),
+ Addr
+ );
+ return 0;
+ }
+ }
+ }
+
+ Val32 = MmioRead32 (AlignedAddr);
+ switch ((UINT64)Addr & 0x3) {
+ case 2:
+ *Val = Val32 >> 16;
+ break;
+
+ case 0:
+ default:
+ *Val = Val32;
+ break;
+ }
+ DEBUG_PCIE_CFG (
+ "PCIE CFG RD16: 0x%p value: 0x%04X (0x%08llX 0x%08X)\n",
+ Addr,
+ *Val,
+ AlignedAddr,
+ Val32
+ );
+
+ return 0;
+}
+
+/**
+ Read 8-bit value from config space address
+
+ @param Addr Address within config space
+ @param Val Point to address for read value
+**/
+INT32
+Ac01PcieCfgIn8 (
+ IN VOID *Addr,
+ OUT UINT8 *Val
+ )
+{
+ UINT64 AlignedAddr = (UINT64)Addr & ~0x3;
+ UINT32 Val32 = MmioRead32 (AlignedAddr);
+ switch ((UINT64)Addr & 0x3) {
+ case 3:
+ *Val = Val32 >> 24;
+ break;
+
+ case 2:
+ *Val = Val32 >> 16;
+ break;
+
+ case 1:
+ *Val = Val32 >> 8;
+ break;
+
+ case 0:
+ default:
+ *Val = Val32;
+ break;
+ }
+ DEBUG_PCIE_CFG (
+ "PCIE CFG RD8: 0x%p value: 0x%02X (0x%08llX 0x%08X)\n",
+ Addr,
+ *Val,
+ AlignedAddr,
+ Val32
+ );
+
+ return 0;
+}
+
+/**
+ Return the next extended capability address
+
+ @param RC Pointer to AC01_RC structure
+ @param PcieIndex PCIe index
+ @param IsRC 0x1: Checking RC configuration space
+ 0x0: Checking EP configuration space
+ @param ExtendedCapId
+**/
+UINT64
+PcieCheckCap (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex,
+ IN BOOLEAN IsRC,
+ IN UINT16 ExtendedCapId
+ )
+{
+ VOID *CfgAddr;
+ UINT32 Val = 0, NextCap = 0, CapId = 0, ExCap = 0;
+
+ if (IsRC) {
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+ } else {
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 20));
+ }
+
+ Ac01PcieCsrIn32 (CfgAddr + TYPE1_CAP_PTR_REG, &Val);
+ NextCap = Val & 0xFF;
+
+ // Loop untill desired capability is found else return 0
+ while (1) {
+ if ((NextCap & 0x3) != 0) {
+ /* Not alignment, just return */
+ return 0;
+ }
+ Ac01PcieCsrIn32 (CfgAddr + NextCap, &Val);
+ if (NextCap < EXT_CAP_OFFSET_START) {
+ CapId = Val & 0xFF;
+ } else {
+ CapId = Val & 0xFFFF;
+ }
+
+ if (CapId == ExtendedCapId) {
+ return (UINT64)(CfgAddr + NextCap);
+ }
+
+ if (NextCap < EXT_CAP_OFFSET_START) {
+ NextCap = (Val & 0xFFFF) >> 8;
+ } else {
+ NextCap = (Val & 0xFFFF0000) >> 20;
+ }
+
+ if ((NextCap == 0) && (ExCap == 0)) {
+ ExCap = 1;
+ NextCap = EXT_CAP_OFFSET_START;
+ }
+
+ if ((NextCap == 0) && (ExCap == 1)) {
+ return (UINT64)0;
+ }
+ }
+}
+
+/**
+ Read 8-bit value from config space address
+
+ @param RC Pointer to AC01_RC strucutre
+ @param RegBase Base address of CSR, TCU, Hostbridge, Msg, Serdes, and MMCFG register
+ @param MmioBase Base address of 64-bit MMIO
+ @param MmioSize Size of 64-bit MMIO space
+ @param Mmio32Base Base address of 32-bit MMIO
+ @param Mmio32Size Size of 32-bit MMIO space
+**/
+VOID
+Ac01PcieCoreBuildRCStruct (
+ AC01_RC *RC,

RootComplex.

+ UINT64 RegBase,
+ UINT64 MmioBase,
+ UINT64 MmioSize,
+ UINT64 Mmio32Base,
+ UINT64 Mmio32Size
+ )
+{
+ INTN PcieIndex;
+
+ RC->BaseAddr = RegBase;
+ RC->TcuAddr = RegBase + TCU_OFFSET;
+ RC->HBAddr = RegBase + HB_CSR_OFFSET;
+ RC->SerdesAddr = RegBase + SERDES_CSR_OFFSET;
+ RC->MmcfgAddr = RegBase + MMCONFIG_OFFSET;
+ RC->MmioAddr = MmioBase;
+ RC->MmioSize = MmioSize;
+ RC->Mmio32Addr = Mmio32Base;
+ RC->Mmio32Size = Mmio32Size;
+ RC->IoAddr = Mmio32Base + Mmio32Size - IO_SPACE;
+
+ RC->Type = (RC->ID < MAX_RCA) ? RCA : RCB;
+ RC->MaxPcieController = (RC->Type == RCB) ? MAX_PCIE_B : MAX_PCIE_A;
+
+ BoardPcieParseRCParams (RC);
+
+ for (PcieIndex = 0; PcieIndex < RC->MaxPcieController; PcieIndex++) {
+ RC->Pcie[PcieIndex].ID = PcieIndex;
+ RC->Pcie[PcieIndex].CsrAddr = RC->BaseAddr + PCIE0_CSR_OFFSET + PcieIndex*0x10000;
+ RC->Pcie[PcieIndex].SnpsRamAddr = RC->Pcie[PcieIndex].CsrAddr + SNPSRAM_OFFSET;
+ RC->Pcie[PcieIndex].DevNum = PcieIndex + 1;
+ }
+
+ DEBUG_PCIE_INFO (
+ " + S%d - RC%a%d, MMCfgAddr:0x%lx, MmioAddr:0x%lx, MmioSize:0x%lx, Mmio32Addr:0x%lx, Mmio32Size:0x%lx, Enabled:%a\n",
+ RC->Socket,
+ (RC->Type == RCA) ? "A" : "B",
+ RC->ID,
+ RC->MmcfgAddr,
+ RC->MmioAddr,
+ RC->MmioSize,
+ RC->Mmio32Addr,
+ RC->Mmio32Size,
+ (RC->Active) ? "Y" : "N"
+ );
+ DEBUG_PCIE_INFO (" + DevMapLow/High: 0x%x/0x%x\n", RC->DevMapLow, RC->DevMapHigh);
+ for (PcieIndex = 0; PcieIndex < RC->MaxPcieController; PcieIndex++) {
+ DEBUG_PCIE_INFO (
+ " + PCIE%d:0x%lx - Enabled:%a - DevNum:0x%x\n",
+ PcieIndex,
+ RC->Pcie[PcieIndex].CsrAddr,
+ (RC->Pcie[PcieIndex].Active) ? "Y" : "N",
+ RC->Pcie[PcieIndex].DevNum
+ );
+ }
+}
+
+/**
+ Configure equalization settings
+
+ @param RC Pointer to AC01_RC structure
+ @param PcieIndex PCIe index
+**/
+STATIC
+VOID
+Ac01PcieConfigureEqualization (
+ IN AC01_RC *RC,

RootComplex.

+ IN INTN PcieIndex
+ )
+{
+ VOID *CfgAddr;
+ UINT32 Val;
+
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Select the FoM method, need double-write to convey settings
+ Ac01PcieCfgIn32 (CfgAddr + GEN3_EQ_CONTROL_OFF, &Val);
+ Val = GEN3_EQ_FB_MODE (Val, 0x1);
+ Val = GEN3_EQ_PRESET_VEC (Val, 0x3FF);
+ Val = GEN3_EQ_INIT_EVAL (Val, 0x1);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_EQ_CONTROL_OFF, Val);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_EQ_CONTROL_OFF, Val);
+ Ac01PcieCfgIn32 (CfgAddr + GEN3_EQ_CONTROL_OFF, &Val);
+}
+
+/**
+ Configure presets for GEN3 equalization
+
+ @param RC Pointer to AC01_RC structure
+ @param PcieIndex PCIe index
+**/
+STATIC
+VOID
+Ac01PcieConfigurePresetGen3 (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex
+ )
+{
+ VOID *CfgAddr, *SpcieBaseAddr;
+ UINT32 Val, Idx;
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Bring to legacy mode
+ Ac01PcieCfgIn32 (CfgAddr + GEN3_RELATED_OFF, &Val);
+ Val = RATE_SHADOW_SEL_SET (Val, 0);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_RELATED_OFF, Val);
+ Val = EQ_PHASE_2_3_SET (Val, 0);
+ Val = RXEQ_REGRDLESS_SET (Val, 1);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_RELATED_OFF, Val);
+
+ // Generate SPCIE capability address
+ SpcieBaseAddr = (VOID *)PcieCheckCap (RC, PcieIndex, 0x1, SPCIE_CAP_ID);
+ if (SpcieBaseAddr == 0) {
+ DEBUG_PCIE_ERR (
+ "PCIE%d.%d: Cannot get SPCIE capability address\n",
+ RC->ID,
+ PcieIndex
+ );
+ return;
+ }
+
+ for (Idx=0; Idx < RC->Pcie[PcieIndex].MaxWidth/2; Idx++) {
+ // Program Preset to Gen3 EQ Lane Control
+ Ac01PcieCfgIn32 (SpcieBaseAddr + CAP_OFF_0C + Idx*4, &Val);
+ Val = DSP_TX_PRESET0_SET (Val, 0x7);
+ Val = DSP_TX_PRESET1_SET (Val, 0x7);
+ Ac01PcieCfgOut32 (SpcieBaseAddr + CAP_OFF_0C + Idx*4, Val);
+ }
+}
+
+/**
+ Configure presets for GEN4 equalization
+
+ @param RC Pointer to AC01_RC structure
+ @param PcieIndex PCIe index
+**/
+STATIC
+VOID
+Ac01PcieConfigurePresetGen4 (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex
+ )
+{
+ UINT32 Val;
+ VOID *CfgAddr, *SpcieBaseAddr, *Pl16BaseAddr;
+ UINT32 LinkWidth, i;
+ UINT8 Preset;
+
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Bring to legacy mode
+ Ac01PcieCfgIn32 (CfgAddr + GEN3_RELATED_OFF, &Val);
+ Val = RATE_SHADOW_SEL_SET (Val, 1);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_RELATED_OFF, Val);
+ Val = EQ_PHASE_2_3_SET (Val, 0);
+ Val = RXEQ_REGRDLESS_SET (Val, 1);
+ Ac01PcieCfgOut32 (CfgAddr + GEN3_RELATED_OFF, Val);
+
+ // Generate the PL16 capability address
+ Pl16BaseAddr = (VOID *)PcieCheckCap (RC, PcieIndex, 0x1, PL16_CAP_ID);
+ if (Pl16BaseAddr == 0) {
+ DEBUG_PCIE_ERR (
+ "PCIE%d.%d: Cannot get PL16 capability address\n",
+ RC->ID,
+ PcieIndex
+ );
+ return;
+ }
+
+ // Generate the SPCIE capability address
+ SpcieBaseAddr = (VOID *)PcieCheckCap (RC, PcieIndex, 0x1, SPCIE_CAP_ID);
+ if (SpcieBaseAddr == 0) {
+ DEBUG_PCIE_ERR (
+ "PCIE%d.%d: Cannot get SPICE capability address\n",
+ RC->ID,
+ PcieIndex
+ );
+ return;
+ }
+
+ // Configure downstream Gen4 Tx preset
+ if (RC->PresetGen4[PcieIndex] == PRESET_INVALID) {
+ Preset = 0x57; // Default Gen4 preset
+ } else {
+ Preset = RC->PresetGen4[PcieIndex];
+ }
+
+ LinkWidth = RC->Pcie[PcieIndex].MaxWidth;
+ if (LinkWidth == 0x2) {
+ Ac01PcieCfgIn32 (Pl16BaseAddr + PL16G_CAP_OFF_20H_REG_OFF, &Val);
+ Val = DSP_16G_RXTX_PRESET0_SET (Val, Preset);
+ Val = DSP_16G_RXTX_PRESET1_SET (Val, Preset);
+ Ac01PcieCfgOut32 (Pl16BaseAddr + PL16G_CAP_OFF_20H_REG_OFF, Val);
+ } else {
+ for (i = 0; i < LinkWidth/4; i++) {
+ Ac01PcieCfgIn32 (Pl16BaseAddr + PL16G_CAP_OFF_20H_REG_OFF + i*4, &Val);
+ Val = DSP_16G_RXTX_PRESET0_SET (Val, Preset);
+ Val = DSP_16G_RXTX_PRESET1_SET (Val, Preset);
+ Val = DSP_16G_RXTX_PRESET2_SET (Val, Preset);
+ Val = DSP_16G_RXTX_PRESET3_SET (Val, Preset);
+ Ac01PcieCfgOut32 (Pl16BaseAddr + PL16G_CAP_OFF_20H_REG_OFF + i*4, Val);
+ }
+ }
+
+ // Configure Gen3 preset
+ for (i = 0; i < LinkWidth/2; i++) {
+ Ac01PcieCfgIn32 (SpcieBaseAddr + CAP_OFF_0C + i*4, &Val);
+ Val = DSP_TX_PRESET0_SET (Val, 0x7);
+ Val = DSP_TX_PRESET1_SET (Val, 0x7);
+ Ac01PcieCfgOut32 (SpcieBaseAddr + CAP_OFF_0C + i*4, Val);
+ }
+}
+
+STATIC BOOLEAN
+RasdpMitigationCheck (
+ AC01_RC *RC,
+ INTN PcieIndex
+ )
+{
+ PLATFORM_INFO_HOB *PlatformHob;
+ VOID *Hob;
+
+ Hob = GetFirstGuidHob (&gPlatformHobGuid);
+ PlatformHob = (PLATFORM_INFO_HOB *)GET_GUID_HOB_DATA (Hob);
+ if ((PlatformHob->ScuProductId[0] & 0xff) == 0x01) {
+ if (AsciiStrCmp ((CONST CHAR8 *)PlatformHob->CpuVer, "A0") == 0) {
+ return ((RC->Type == RCB)||(PcieIndex > 0)) ? TRUE : FALSE;
+ }
+ }
+
+ return FALSE;
+}
+
+/**
+ Setup and initialize the AC01 PCIe Root Complex and underneath PCIe controllers
+

This is a lot of functionality, please break it down into smaller functions.

+ @param RC Pointer to Root Complex structure
+ @param ReInit Re-init status
+ @param ReInitPcieIndex PCIe index
+**/
+INT32
+Ac01PcieCoreSetupRC (
+ IN AC01_RC *RC,

RootComplex (global search/replace).

+ IN UINT8 ReInit,
+ IN UINT8 ReInitPcieIndex
+ )
+{
+ VOID *CsrAddr, *CfgAddr, *SnpsRamAddr, *DlinkBaseAddr;
+ INTN PcieIndex;
+ INTN TimeOut;
+ UINT32 Val;
+ PHY_CONTEXT PhyCtx = { 0 };
+ PHY_PLAT_RESOURCE PhyPlatResource = { 0 };
+ INTN Ret;
+ UINT16 NextExtendedCapabilityOff;
+ UINT32 VsecVal;
+
+ DEBUG_PCIE_INFO ("Initializing Socket%d RC%d\n", RC->Socket, RC->ID);
+
+ if (ReInit == 0) {
+ // Initialize SERDES
+ ZeroMem (&PhyCtx, sizeof (PhyCtx));
+ PhyCtx.SdsAddr = RC->SerdesAddr;
+ PhyCtx.PcieCtrlInfo |= ((RC->Socket & 0x1) << 2);
+ PhyCtx.PcieCtrlInfo |= ((RC->ID & 0x7) << 4);
+ PhyCtx.PcieCtrlInfo |= 0xF << 8;
+ PhyPlatResource.MmioRd = Ac01PcieMmioRd;
+ PhyPlatResource.MmioWr = Ac01PcieMmioWr;
+ PhyPlatResource.UsDelay = Ac01PcieDelay;
+ PhyPlatResource.Puts = Ac01PciePuts;

...added into a further abstraction layer?
And never used as far as I can tell.

+ PhyPlatResource.PutInt = Ac01PciePutInt;

Also unused.

+ PhyPlatResource.PutHex = Ac01PciePutInt;

With an unused alias. (Which would appear to be a copy/paste bug.)

+ PhyPlatResource.PutHex64 = Ac01PciePutHex;

Or maybe not, because Int is 32-bit but Hex is 64-bit?

I'm sorry, but this is a complete mess.
Please tear out *all* customised debug printout.

+ PhyPlatResource.DebugPrint = Ac01PcieDebugPrint;

Including that one.

+ PhyCtx.PhyPlatResource = &PhyPlatResource;
+
+ Ret = SerdesInitClkrst (&PhyCtx);
+ if (Ret != PHY_INIT_PASS) {
+ return -1;
+ }
+ }
+
+ // Setup each controller
+ for (PcieIndex = 0; PcieIndex < RC->MaxPcieController; PcieIndex++) {
+
+ if (ReInit == 1) {
+ PcieIndex = ReInitPcieIndex;
+ }
+
+ if (!RC->Pcie[PcieIndex].Active) {
+ continue;
+ }
+
+ DEBUG_PCIE_INFO ("Initializing Controller %d\n", PcieIndex);
+
+ CsrAddr = (VOID *)RC->Pcie[PcieIndex].CsrAddr;
+ SnpsRamAddr = (VOID *)RC->Pcie[PcieIndex].SnpsRamAddr;
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Put Controller into reset if not in reset already
+ Ac01PcieCsrIn32 (CsrAddr + RESET, &Val);
+ if (!(Val & RESET_MASK)) {
+ Val = DWCPCIE_SET (Val, 1);
+ Ac01PcieCsrOut32 (CsrAddr + RESET, Val);
+
+ // Delay 50ms to ensure controller finish its reset
+ MicroSecondDelay (50000);
+ }
+
+ // Clear memory shutdown
+ Ac01PcieCsrIn32 (CsrAddr + RAMSDR, &Val);
+ Val = SD_SET (Val, 0);
+ Ac01PcieCsrOut32 (CsrAddr + RAMSDR, Val);
+
+ // Poll till mem is ready
+ TimeOut = MEMRDY_TIMEOUT;
+ do {
+ Ac01PcieCsrIn32 (CsrAddr + MEMRDYR, &Val);
+ if (Val & 1) {
+ break;
+ }
+
+ TimeOut--;
+ MicroSecondDelay (1);
+ } while (TimeOut > 0);
+
+ if (TimeOut <= 0) {
+ DEBUG_PCIE_ERR ("- Pcie[%d] - Mem not ready\n", PcieIndex);
+ return -1;
+ }
+
+ // Hold link training
+ Ac01PcieCsrIn32 (CsrAddr + LINKCTRL, &Val);
+ Val = LTSSMENB_SET (Val, 0);
+ Ac01PcieCsrOut32 (CsrAddr + LINKCTRL, Val);
+
+ // Enable subsystem clock and release reset
+ Ac01PcieCsrIn32 (CsrAddr + CLOCK, &Val);
+ Val = AXIPIPE_SET (Val, 1);
+ Ac01PcieCsrOut32 (CsrAddr + CLOCK, Val);
+ Ac01PcieCsrIn32 (CsrAddr + RESET, &Val);
+ Val = DWCPCIE_SET (Val, 0);
+ Ac01PcieCsrOut32 (CsrAddr + RESET, Val);
+
+ //
+ // Controller does not provide any indicator for reset released.
+ // Must wait at least 1us as per EAS.
+ //
+ MicroSecondDelay (1);
+
+ // Poll till PIPE clock is stable
+ TimeOut = PIPE_CLOCK_TIMEOUT;
+ do {
+ Ac01PcieCsrIn32 (CsrAddr + LINKSTAT, &Val);
+ if (!(Val & PHY_STATUS_MASK)) {
+ break;
+ }
+
+ TimeOut--;
+ MicroSecondDelay (1);
+ } while (TimeOut > 0);
+
+ if (TimeOut <= 0) {
+ DEBUG_PCIE_ERR ("- Pcie[%d] - PIPE clock is not stable\n", PcieIndex);
+ return -1;
+ }
+
+ // Start PERST pulse
+ BoardPcieAssertPerst (RC, PcieIndex, 0, TRUE);
+
+ // Allow programming to config space
+ Ac01PcieCsrIn32 (CfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + MISC_CONTROL_1_OFF, Val);
+
+ // In order to detect the NVMe disk for booting without disk,
+ // need to set Hot-Plug Slot Capable during port initialization.
+ // It will help PCI Linux driver to initialize its slot iomem resource,
+ // the one is used for detecting the disk when it is inserted.
+ Ac01PcieCsrIn32 (CfgAddr + SLOT_CAPABILITIES_REG, &Val);
+ Val = SLOT_HPC_SET (Val, 1);
+ // Program the power limit
+ Val = SLOT_CAP_SLOT_POWER_LIMIT_VALUE_SET (Val, SLOT_POWER_LIMIT);
+ Ac01PcieCsrOut32 (CfgAddr + SLOT_CAPABILITIES_REG, Val);
+
+
+ // Apply RASDP error mitigation for all x8, x4, and x2 controllers
+ // This includes all RCB root ports, and every RCA root port controller
+ // except for index 0 (i.e. x16 controllers are exempted from this WA)
+ if (RasdpMitigationCheck (RC, PcieIndex)) {
+ // Change the Read Margin dual ported RAMs
+ Val = 0x10; // Margin to 0x0 (most conservative setting)
+ Ac01PcieCsrOut32 (SnpsRamAddr + TPSRAM_RMR, Val);
+
+ // Generate the DLINK capability address
+ DlinkBaseAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+ NextExtendedCapabilityOff = 0x100; // This is the 1st extended capability offset
+ do {
+ Ac01PcieCsrIn32 (DlinkBaseAddr + NextExtendedCapabilityOff, &Val);
+ if (Val == 0xFFFFFFFF) {
+ DlinkBaseAddr = 0x0;
+ break;
+ }
+ if ((Val & 0xFFFF) == DLINK_VENDOR_CAP_ID) {
+ Ac01PcieCsrIn32 (DlinkBaseAddr + NextExtendedCapabilityOff + 0x4, &VsecVal);
+ if (VsecVal == DLINK_VSEC) {
+ DlinkBaseAddr = DlinkBaseAddr + NextExtendedCapabilityOff;
+ break;
+ }
+ }
+ NextExtendedCapabilityOff = (Val >> 20);
+ } while (NextExtendedCapabilityOff != 0);
+
+ // Disable the scaled credit mode
+ if (DlinkBaseAddr != 0x0) {
+ Val = 1;
+ Ac01PcieCsrOut32 (DlinkBaseAddr + DATA_LINK_FEATURE_CAP_OFF, Val);
+ Ac01PcieCsrIn32 (DlinkBaseAddr + DATA_LINK_FEATURE_CAP_OFF, &Val);
+ if (Val != 1) {
+ DEBUG_PCIE_ERR ("- Pcie[%d] - Unable to disable scaled credit\n", PcieIndex);
+ return -1;
+ }
+ } else {
+ DEBUG_PCIE_ERR ("- Pcie[%d] - Unable to locate data link feature cap offset\n", PcieIndex);
+ return -1;
+ }
+
+ // Reduce Posted Credits to 1 packet header and data credit for all
+ // impacted controllers. Also zero credit scale values for both
+ // data and packet headers.
+ Val=0x40201020;
+ Ac01PcieCsrOut32 (CfgAddr + PORT_LOCIG_VC0_P_RX_Q_CTRL_OFF, Val);
+ }
+
+ // Program DTI for ATS support
+ Ac01PcieCsrIn32 (CfgAddr + DTIM_CTRL0_OFF, &Val);
+ Val = DTIM_CTRL0_ROOT_PORT_ID_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + DTIM_CTRL0_OFF, Val);
+
+ //
+ // Program number of lanes used
+ // - Reprogram LINK_CAPABLE of PORT_LINK_CTRL_OFF
+ // - Reprogram NUM_OF_LANES of GEN2_CTRL_OFF
+ // - Reprogram CAP_MAX_LINK_WIDTH of LINK_CAPABILITIES_REG
+ //
+ Ac01PcieCsrIn32 (CfgAddr + PORT_LINK_CTRL_OFF, &Val);
+ switch (RC->Pcie[PcieIndex].MaxWidth) {
+ case LNKW_X2:
+ Val = LINK_CAPABLE_SET (Val, LINK_CAPABLE_X2);
+ break;
+
+ case LNKW_X4:
+ Val = LINK_CAPABLE_SET (Val, LINK_CAPABLE_X4);
+ break;
+
+ case LNKW_X8:
+ Val = LINK_CAPABLE_SET (Val, LINK_CAPABLE_X8);
+ break;
+
+ case LNKW_X16:
+ default:
+ Val = LINK_CAPABLE_SET (Val, LINK_CAPABLE_X16);
+ break;
+ }
+ Ac01PcieCsrOut32 (CfgAddr + PORT_LINK_CTRL_OFF, Val);
+ Ac01PcieCsrIn32 (CfgAddr + GEN2_CTRL_OFF, &Val);
+ switch (RC->Pcie[PcieIndex].MaxWidth) {
+ case LNKW_X2:
+ Val = NUM_OF_LANES_SET (Val, NUM_OF_LANES_X2);
+ break;
+
+ case LNKW_X4:
+ Val = NUM_OF_LANES_SET (Val, NUM_OF_LANES_X4);
+ break;
+
+ case LNKW_X8:
+ Val = NUM_OF_LANES_SET (Val, NUM_OF_LANES_X8);
+ break;
+
+ case LNKW_X16:
+ default:
+ Val = NUM_OF_LANES_SET (Val, NUM_OF_LANES_X16);
+ break;
+ }
+ Ac01PcieCsrOut32 (CfgAddr + GEN2_CTRL_OFF, Val);
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CAPABILITIES_REG, &Val);
+ switch (RC->Pcie[PcieIndex].MaxWidth) {
+ case LNKW_X2:
+ Val = CAP_MAX_LINK_WIDTH_SET (Val, CAP_MAX_LINK_WIDTH_X2);
+ break;
+
+ case LNKW_X4:
+ Val = CAP_MAX_LINK_WIDTH_SET (Val, CAP_MAX_LINK_WIDTH_X4);
+ break;
+
+ case LNKW_X8:
+ Val = CAP_MAX_LINK_WIDTH_SET (Val, CAP_MAX_LINK_WIDTH_X8);
+ break;
+
+ case LNKW_X16:
+ default:
+ Val = CAP_MAX_LINK_WIDTH_SET (Val, CAP_MAX_LINK_WIDTH_X16);
+ break;
+ }
+
+ switch (RC->Pcie[PcieIndex].MaxGen) {
+ case SPEED_GEN1:
+ Val = CAP_MAX_LINK_SPEED_SET (Val, MAX_LINK_SPEED_25);
+ break;
+
+ case SPEED_GEN2:
+ Val = CAP_MAX_LINK_SPEED_SET (Val, MAX_LINK_SPEED_50);
+ break;
+
+ case SPEED_GEN3:
+ Val = CAP_MAX_LINK_SPEED_SET (Val, MAX_LINK_SPEED_80);
+ break;
+
+ default:
+ Val = CAP_MAX_LINK_SPEED_SET (Val, MAX_LINK_SPEED_160);
+ break;
+ }
+ /* Enable ASPM Capability */
+ Val = CAP_ACTIVE_STATE_LINK_PM_SUPPORT_SET (Val, L0S_L1_SUPPORTED);
+ Ac01PcieCsrOut32 (CfgAddr + LINK_CAPABILITIES_REG, Val);
+
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CONTROL2_LINK_STATUS2_REG, &Val);
+ switch (RC->Pcie[PcieIndex].MaxGen) {
+ case SPEED_GEN1:
+ Val = CAP_TARGET_LINK_SPEED_SET (Val, MAX_LINK_SPEED_25);
+ break;
+
+ case SPEED_GEN2:
+ Val = CAP_TARGET_LINK_SPEED_SET (Val, MAX_LINK_SPEED_50);
+ break;
+
+ case SPEED_GEN3:
+ Val = CAP_TARGET_LINK_SPEED_SET (Val, MAX_LINK_SPEED_80);
+ break;
+
+ default:
+ Val = CAP_TARGET_LINK_SPEED_SET (Val, MAX_LINK_SPEED_160);
+ break;
+ }
+ Ac01PcieCsrOut32 (CfgAddr + LINK_CONTROL2_LINK_STATUS2_REG, Val);
+
+ // Set Zero byte request handling
+ Ac01PcieCsrIn32 (CfgAddr + FILTER_MASK_2_OFF, &Val);
+ Val = CX_FLT_MASK_VENMSG0_DROP_SET (Val, 0);
+ Val = CX_FLT_MASK_VENMSG1_DROP_SET (Val, 0);
+ Val = CX_FLT_MASK_DABORT_4UCPL_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + FILTER_MASK_2_OFF, Val);
+ Ac01PcieCsrIn32 (CfgAddr + AMBA_ORDERING_CTRL_OFF, &Val);
+ Val = AX_MSTR_ZEROLREAD_FW_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + AMBA_ORDERING_CTRL_OFF, Val);
+
+ //
+ // Set Completion with CRS handling for CFG Request
+ // Set Completion with CA/UR handling non-CFG Request
+ //
+ Ac01PcieCsrIn32 (CfgAddr + AMBA_ERROR_RESPONSE_DEFAULT_OFF, &Val);
+ Val = AMBA_ERROR_RESPONSE_CRS_SET (Val, 0x2);
+ Ac01PcieCsrOut32 (CfgAddr + AMBA_ERROR_RESPONSE_DEFAULT_OFF, Val);
+
+ // Set Legacy PCIE interrupt map to INTA
+ Ac01PcieCsrIn32 (CfgAddr + BRIDGE_CTRL_INT_PIN_INT_LINE_REG, &Val);
+ Val = INT_PIN_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + BRIDGE_CTRL_INT_PIN_INT_LINE_REG, Val);
+ Ac01PcieCsrIn32 (CsrAddr + IRQSEL, &Val);
+ Val = INTPIN_SET (Val, 1);
+ Ac01PcieCsrOut32 (CsrAddr + IRQSEL, Val);
+
+ if (RC->Pcie[PcieIndex].MaxGen != SPEED_GEN1) {
+ Ac01PcieConfigureEqualization (RC, PcieIndex);
+ if (RC->Pcie[PcieIndex].MaxGen == SPEED_GEN3) {
+ Ac01PcieConfigurePresetGen3 (RC, PcieIndex);
+ } else if (RC->Pcie[PcieIndex].MaxGen == SPEED_GEN4) {
+ Ac01PcieConfigurePresetGen4 (RC, PcieIndex);
+ }
+ }
+
+ // Mask Completion Timeout
+ Ac01PcieCsrIn32 (CfgAddr + AMBA_LINK_TIMEOUT_OFF, &Val);
+ Val = LINK_TIMEOUT_PERIOD_DEFAULT_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + AMBA_LINK_TIMEOUT_OFF, Val);
+ Ac01PcieCsrIn32 (CfgAddr + UNCORR_ERR_MASK_OFF, &Val);
+ Val = CMPLT_TIMEOUT_ERR_MASK_SET (Val, 1);
+ // AER surprise link down error should be masked due to hotplug is enabled
+ // This event must be handled by Hotplug handler, instead of error handler
+ Val = SDES_ERR_MASK_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + UNCORR_ERR_MASK_OFF, Val);
+
+ // Program Class Code
+ Ac01PcieCsrIn32 (CfgAddr + TYPE1_CLASS_CODE_REV_ID_REG, &Val);
+ Val = REVISION_ID_SET (Val, 4);
+ Val = SUBCLASS_CODE_SET (Val, 4);
+ Val = BASE_CLASS_CODE_SET (Val, 6);
+ Ac01PcieCsrOut32 (CfgAddr + TYPE1_CLASS_CODE_REV_ID_REG, Val);
+
+ // Program VendorID and DeviceID
+ Ac01PcieCsrIn32 (CfgAddr + TYPE1_DEV_ID_VEND_ID_REG, &Val);
+ Val = VENDOR_ID_SET (Val, AMPERE_PCIE_VENDORID);
+ if (RCA == RC->Type) {
+ Val = DEVICE_ID_SET (Val, AC01_PCIE_BRIDGE_DEVICEID_RCA + PcieIndex);
+ } else {
+ Val = DEVICE_ID_SET (Val, AC01_PCIE_BRIDGE_DEVICEID_RCB + PcieIndex);
+ }
+ Ac01PcieCsrOut32 (CfgAddr + TYPE1_DEV_ID_VEND_ID_REG, Val);
+
+ // Enable common clock for downstream
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CONTROL_LINK_STATUS_REG, &Val);
+ Val = CAP_SLOT_CLK_CONFIG_SET (Val, 1);
+ Val = CAP_COMMON_CLK_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + LINK_CONTROL_LINK_STATUS_REG, Val);
+
+ // Match aux_clk to system
+ Ac01PcieCsrIn32 (CfgAddr + AUX_CLK_FREQ_OFF, &Val);
+ Val = AUX_CLK_FREQ_SET (Val, AUX_CLK_500MHZ);
+ Ac01PcieCsrOut32 (CfgAddr + AUX_CLK_FREQ_OFF, Val);
+
+ // Assert PERST low to reset endpoint
+ BoardPcieAssertPerst (RC, PcieIndex, 0, FALSE);
+
+ // Start link training
+ Ac01PcieCsrIn32 (CsrAddr + LINKCTRL, &Val);
+ Val = LTSSMENB_SET (Val, 1);
+ Ac01PcieCsrOut32 (CsrAddr + LINKCTRL, Val);
+
+ // Complete the PERST pulse
+ BoardPcieAssertPerst (RC, PcieIndex, 0, TRUE);
+
+ // Lock programming of config space
+ Ac01PcieCsrIn32 (CfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + MISC_CONTROL_1_OFF, Val);
+
+ if (ReInit == 1) {
+ return 0;
+ }
+ }
+
+ // Program VendorID and DeviceId
+ if (!EFI_ERROR (MailboxMsgRegisterRead (RC->Socket, RC->HBAddr + HBPDVIDR, &Val))) {
+ Val = PCIVENDID_SET (Val, AMPERE_PCIE_VENDORID);
+ if (RCA == RC->Type) {
+ Val = PCIDEVID_SET (Val, AC01_HOST_BRIDGE_DEVICEID_RCA);
+ } else {
+ Val = PCIDEVID_SET (Val, AC01_HOST_BRIDGE_DEVICEID_RCB);
+ }
+ MailboxMsgRegisterWrite (RC->Socket, RC->HBAddr + HBPDVIDR, Val);
+ }
+
+ return 0;
+}
+
+BOOLEAN
+PcieLinkUpCheck (
+ IN AC01_PCIE *Pcie
+ )
+{
+ VOID *CsrAddr;
+ UINT32 BlockEvent, LinkStat;
+
+ CsrAddr = (VOID *)Pcie->CsrAddr;
+
+ // Check if card present
+ // smlh_ltssm_state[13:8] = 0
+ // phy_status[2] = 0
+ // smlh_link_up[1] = 0
+ // rdlh_link_up[0] = 0
+ Ac01PcieCsrIn32 (CsrAddr + LINKSTAT, &LinkStat);
+ LinkStat = LinkStat & (SMLH_LTSSM_STATE_MASK | PHY_STATUS_MASK_BIT |
+ SMLH_LINK_UP_MASK_BIT | RDLH_LINK_UP_MASK_BIT);
+ if (LinkStat == 0x0000) {
+ return FALSE;
+ }
+
+ Ac01PcieCsrIn32 (CsrAddr + BLOCKEVENTSTAT, &BlockEvent);
+ Ac01PcieCsrIn32 (CsrAddr + LINKSTAT, &LinkStat);
+
+ if (((BlockEvent & LINKUP_MASK) != 0)
+ && (SMLH_LTSSM_STATE_GET(LinkStat) == S_L0))
+ {
+ DEBUG_PCIE_INFO ("%a Linkup\n", __FUNCTION__);
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+VOID
+Ac01PcieCoreEndEnumeration (
+ AC01_RC *RC
+ )
+{
+ VOID *Reg;
+ UINTN Index;
+ UINT32 Val;
+ VOID *CfgAddr;
+
+ if (RC == NULL || !RC->Active) {
+ return;
+ }
+
+ // Clear uncorrectable error during enumuration phase. Mainly completion timeout.
+ for (Index = 0; Index < RC->MaxPcieController; Index++) {
+ if (!RC->Pcie[Index].Active) {
+ continue;
+ }
+ if (!PcieLinkUpCheck(&RC->Pcie[Index])) {
+ // If link down/disabled after enumeration, disable completed time out
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[Index].DevNum << 15));
+ Ac01PcieCsrIn32 (CfgAddr + UNCORR_ERR_MASK_OFF, &Val);
+ Val = CMPLT_TIMEOUT_ERR_MASK_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + UNCORR_ERR_MASK_OFF, Val);
+ }
+ // Clear all errors
+ Reg = (VOID *)((UINT64)RC->MmcfgAddr + ((Index + 1) << 15) + UNCORR_ERR_STATUS_OFF);
+ Ac01PcieCfgIn32 (Reg, &Val);
+ if (Val != 0) {
+ // Clear error by writting
+ Ac01PcieCfgOut32 (Reg, Val);
+ }
+ }
+}
+
+/**
+ Comparing current link status with the max capabilities of the link
+
+ @param RC Pointer to AC01_RC structure
+ @param PcieIndex PCIe index
+ @param EpMaxWidth EP max link width
+ @param EpMaxGen EP max link speed
+ @retval -1: Link status do not match with link max capabilities
+ 1: Link capabilites are invalid
+ 0: Link status are correct
+**/
+INT32
+Ac01PcieCoreLinkCheck (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex,
+ IN UINT8 EpMaxWidth,
+ IN UINT8 EpMaxGen
+ )
+{
+ VOID *CsrAddr, *CfgAddr;
+ UINT32 Val, LinkStat;
+ UINT32 MaxWidth, MaxGen;
+
+ CsrAddr = (VOID *)RC->Pcie[PcieIndex].CsrAddr;
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CAPABILITIES_REG, &Val);
+ if ((CAP_MAX_LINK_WIDTH_GET (Val) == 0) ||
+ (CAP_MAX_LINK_SPEED_GET (Val) == 0)) {
+ DEBUG_PCIE_INFO ("\tPCIE%d.%d: Wrong RC capabilities\n", RC->ID, PcieIndex);
+ return LINK_CHECK_WRONG_PARAMETER;
+ }
+
+ if ((EpMaxWidth == 0) || (EpMaxGen == 0)) {
+ DEBUG_PCIE_INFO ("\tPCIE%d.%d: Wrong EP capabilities\n", RC->ID, PcieIndex);
+ return LINK_CHECK_FAILED;
+ }
+
+ // Compare RC and EP capabilities
+ if (CAP_MAX_LINK_WIDTH_GET (Val) > EpMaxWidth) {
+ MaxWidth = EpMaxWidth;
+ } else {
+ MaxWidth = CAP_MAX_LINK_WIDTH_GET (Val);
+ }
+
+ // Compare RC and EP capabilities
+ if (CAP_MAX_LINK_SPEED_GET (Val) > EpMaxGen) {
+ MaxGen = EpMaxGen;
+ } else {
+ MaxGen = CAP_MAX_LINK_SPEED_GET (Val);
+ }
+
+ Ac01PcieCsrIn32 (CsrAddr + LINKSTAT, &LinkStat);
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CONTROL_LINK_STATUS_REG, &Val);
+ DEBUG_PCIE_INFO (
+ "PCIE%d.%d: Link MaxWidth %d MaxGen %d, LINKSTAT 0x%x",
+ RC->ID,
+ PcieIndex,
+ MaxWidth,
+ MaxGen,
+ LinkStat
+ );
+
+ // Checking all conditions of the link
+ // If one of them is not sastified, return link up fail
+ if ((CAP_NEGO_LINK_WIDTH_GET (Val) != MaxWidth) ||
+ (CAP_LINK_SPEED_GET (Val) != MaxGen) ||
+ (RDLH_SMLH_LINKUP_STATUS_GET (LinkStat) != (SMLH_LINK_UP_MASK_BIT | RDLH_LINK_UP_MASK_BIT)))
+ {
+ DEBUG_PCIE_INFO ("\tLinkCheck FAILED\n");
+ return LINK_CHECK_FAILED;
+ }
+
+ DEBUG_PCIE_INFO ("\tLinkCheck SUCCESS\n");
+ return LINK_CHECK_SUCCESS;
+}
+
+INT32
+Ac01PFAEnableAll (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex,
+ IN INTN PFAMode
+ )
+{
+ VOID *RasDesVSecBase;
+ VOID *CfgAddr;
+ UINT8 ErrCode, ErrGrpNum;
+ UINT32 Val;
+ INT32 Ret = LINK_CHECK_SUCCESS;
+
+ UINT32 ErrCtrlCfg[MAX_NUM_ERROR_CODE] = {
+ 0x000, 0x001, 0x002, 0x003, 0x004, 0x005, 0x006, 0x007, 0x008, 0x009, 0x00A, // Per Lane
+ 0x105, 0x106, 0x107, 0x108, 0x109, 0x10A,
+ 0x200, 0x201, 0x202, 0x203, 0x204, 0x205, 0x206, 0x207,
+ 0x300, 0x301, 0x302, 0x303, 0x304, 0x305,
+ 0x400, 0x401, // Per Lane
+ 0x500, 0x501, 0x502, 0x503, 0x504, 0x505, 0x506, 0x507, 0x508, 0x509, 0x50A, 0x50B, 0x50C, 0x50D
+ };
+
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Allow programming to config space
+ Ac01PcieCsrIn32 (CfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 1);
+ Ac01PcieCsrOut32 (CfgAddr + MISC_CONTROL_1_OFF, Val);
+
+ // Generate the RAS DES capability address
+ // RAS_DES_CAP_ID = 0xB
+ RasDesVSecBase = (VOID *)PcieCheckCap (RC, PcieIndex, 0x1, RAS_DES_CAP_ID);
+ if (RasDesVSecBase == 0) {
+ DEBUG_PCIE_INFO ("PCIE%d.%d: Cannot get RAS DES capability address\n", RC->ID, PcieIndex);
+ return LINK_CHECK_WRONG_PARAMETER;
+ }
+
+ if (PFAMode == PFA_REG_ENABLE) {
+ // PFA Counters Enable
+ Ac01PcieCsrIn32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, &Val);
+ Val = ECCR_EVENT_COUNTER_ENABLE_SET (Val, 0x7);
+ Val = ECCR_EVENT_COUNTER_CLEAR_SET (Val, 0);
+ Ac01PcieCsrOut32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, Val);
+ } else if (PFAMode == PFA_REG_CLEAR) {
+ // PFA Counters Clear
+ Ac01PcieCsrIn32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, &Val);
+ Val = ECCR_EVENT_COUNTER_ENABLE_SET (Val, 0);
+ Val = ECCR_EVENT_COUNTER_CLEAR_SET (Val, 0x3);
+ Ac01PcieCsrOut32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, Val);
+ } else {

At least this else case needs to be a helper function.

+ // PFA Counters Read
+ for (ErrCode = 0; ErrCode < MAX_NUM_ERROR_CODE; ErrCode++) {
+ ErrGrpNum = (ErrCtrlCfg[ErrCode] & 0xF00) >> 8;
+ // Skipping per lane group
+ // Checking common lane group because AER error are included in common group only
+ if ((ErrGrpNum != 0) && (ErrGrpNum != 4)) {
+ Ac01PcieCsrIn32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, &Val);
+ if (RC->Type == RCA) { // RCA - 4 PCIe controller per port, 1 controller in charge of 4 lanes
+ Val = ECCR_LANE_SEL_SET (Val, PcieIndex*4);
+ } else { // RCB - 8 PCIe controller per port, 1 controller in charge of 2 lanes
+ Val = ECCR_LANE_SEL_SET (Val, PcieIndex*2);
+ }
+ Val = ECCR_GROUP_EVENT_SEL_SET (Val, ErrCtrlCfg[ErrCode]);
+ Ac01PcieCsrOut32 (RasDesVSecBase + EVENT_COUNTER_CONTROL_REG_OFF, Val);
+
+ // After setting Counter Control reg
+ // This delay just to make sure Counter Data reg is update with new value
+ MicroSecondDelay (1);
+ Ac01PcieCsrIn32 (RasDesVSecBase + EVENT_COUNTER_DATA_REG_OFF, &Val);
+ if (Val != 0) {
+ Ret = LINK_CHECK_FAILED;
+ DEBUG_PCIE_INFO (
+ "\tS%d RC%d RP%d \t%s: %d \tGROUP:%d-EVENT:%d\n",
+ RC->Socket,
+ RC->ID,
+ PcieIndex,
+ Val,
+ ((ErrCtrlCfg[ErrCode] & 0xF00) >> 8), // Group
+ (ErrCtrlCfg[ErrCode] & 0x0FF) // Event
+ );
+ }
+ }
+ }
+ }
+
+ // Disable programming to config space
+ Ac01PcieCsrIn32 (CfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + MISC_CONTROL_1_OFF, Val);
+
+ return Ret;
+}
+
+/**
+ Get link capabilities link width and speed of endpoint
+
+ @param RC[in] Pointer to AC01_RC structure
+ @param PcieIndex[in] PCIe controller index
+ @param EpMaxWidth[out] EP max link width
+ @param EpMaxGen[out] EP max link speed
+**/
+VOID
+Ac01PcieCoreGetEndpointInfo (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex,
+ OUT UINT8 *EpMaxWidth,
+ OUT UINT8 *EpMaxGen
+ )
+{
+ VOID *PcieCapBaseAddr, *EpCfgAddr;
+ VOID *RcCfgAddr;
+ UINT32 Val, RestoreVal;
+ UINTN TimeOut;
+
+ RcCfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ // Allow programming to config space
+ Ac01PcieCsrIn32 (RcCfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 1);
+ Ac01PcieCsrOut32 (RcCfgAddr + MISC_CONTROL_1_OFF, Val);
+
+ Ac01PcieCsrIn32 (RcCfgAddr + SEC_LAT_TIMER_SUB_BUS_SEC_BUS_PRI_BUS_REG, &Val);
+ RestoreVal = Val;
+ Val = SUB_BUS_SET (Val, DEFAULT_SUB_BUS); /* Set DEFAULT_SUB_BUS to Subordinate bus */
+ Val = SEC_BUS_SET (Val, RC->Pcie[PcieIndex].DevNum); /* Set RC->Pcie[PcieIndex].DevNum to Secondary bus */
+ Val = PRIM_BUS_SET (Val, 0x0); /* Set 0x0 to Primary bus */
+ Ac01PcieCsrOut32 (RcCfgAddr + SEC_LAT_TIMER_SUB_BUS_SEC_BUS_PRI_BUS_REG, Val);
+ EpCfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 20)); /* Bus 1, dev 0, func 0 */
+
+ // Loop read EpCfgAddr value until got valid value or
+ // reach to timeout EP_LINKUP_TIMEOUT (or more depend on card)

A helper function?

+ TimeOut = EP_LINKUP_TIMEOUT;
+ do {
+ Ac01PcieCsrIn32 (EpCfgAddr, &Val);
+ if (Val != 0xFFFF0001 && Val != 0xFFFFFFFF) {
+ break;
+ }
+ MicroSecondDelay (LINK_WAIT_INTERVAL_US);
+ TimeOut -= LINK_WAIT_INTERVAL_US;
+ } while (TimeOut > 0);
+
+ Ac01PcieCsrIn32 (EpCfgAddr, &Val);
+
+ // Check whether EP config space is accessible or not

This block, a helper function?

+ if (Val == 0xFFFFFFFF) {
+ *EpMaxWidth = 0; // Invalid Width
+ *EpMaxGen = 0; // Invalid Speed
+ DEBUG_PCIE_INFO ("PCIE%d.%d Cannot access EP config space!\n", RC->ID, PcieIndex);
+ } else {
+ PcieCapBaseAddr = (VOID *)PcieCheckCap (RC, PcieIndex, 0x0, CAP_ID);
+ if (PcieCapBaseAddr == 0) {
+ *EpMaxWidth = 0; // Invalid Width
+ *EpMaxGen = 0; // Invalid Speed
+ DEBUG_PCIE_INFO (
+ "PCIE%d.%d Cannot get PCIe capability extended address!\n",
+ RC->ID,
+ PcieIndex
+ );
+ } else {
+ Ac01PcieCsrIn32 (PcieCapBaseAddr + LINK_CAPABILITIES_REG_OFF, &Val);
+ *EpMaxWidth = (Val >> 4) & 0x3F;
+ *EpMaxGen = Val & 0xF;
+ DEBUG_PCIE_INFO (
+ "PCIE%d.%d EP MaxWidth %d EP MaxGen %d \n", RC->ID,
+ PcieIndex,
+ *EpMaxWidth,
+ *EpMaxGen
+ );
+
+ // From EP, enabling common clock for upstream
+ Ac01PcieCsrIn32 (PcieCapBaseAddr + LINK_CONTROL_LINK_STATUS_OFF, &Val);
+ Val = CAP_SLOT_CLK_CONFIG_SET (Val, 1);
+ Val = CAP_COMMON_CLK_SET (Val, 1);
+ Ac01PcieCsrOut32 (PcieCapBaseAddr + LINK_CONTROL_LINK_STATUS_OFF, Val);
+ }
+ }
+
+ // Restore value in order to not affect enumeration process
+ Ac01PcieCsrOut32 (RcCfgAddr + SEC_LAT_TIMER_SUB_BUS_SEC_BUS_PRI_BUS_REG, RestoreVal);
+
+ // Disable programming to config space
+ Ac01PcieCsrIn32 (RcCfgAddr + MISC_CONTROL_1_OFF, &Val);
+ Val = DBI_RO_WR_EN_SET (Val, 0);
+ Ac01PcieCsrOut32 (RcCfgAddr + MISC_CONTROL_1_OFF, Val);
+}
+
+/**
+ Check active PCIe controllers of RC, retrain or soft reset if needed
+
+ @param RC[in] Pointer to AC01_RC structure
+ @param PcieIndex[in] PCIe controller index
+
+ @retval -1: Link recovery had failed
+ 1: Link width and speed are not correct
+ 0: Link recovery succeed
+**/
+INT32
+Ac01PcieCoreQoSLinkCheckRecovery (
+ IN AC01_RC *RC,
+ IN INTN PcieIndex
+ )
+{
+ VOID *CsrAddr;
+ INTN TimeOut;
+ INT32 NumberOfReset = MAX_REINIT; // Number of soft reset retry
+ UINT8 EpMaxWidth, EpMaxGen;
+ INT32 LinkStatusCheck, RasdesChecking;
+
+ // PCIe controller is not active or Link is not up
+ // Nothing to be done
+ if ((!RC->Pcie[PcieIndex].Active) || (!RC->Pcie[PcieIndex].LinkUp)) {
+ return LINK_CHECK_WRONG_PARAMETER;
+ }
+
+ do {
+
+ if (RC->Pcie[PcieIndex].LinkUp) {
+ // Enable all of RASDES register to detect any training error
+ Ac01PFAEnableAll (RC, PcieIndex, PFA_REG_ENABLE);
+
+ // Accessing Endpoint and checking current link capabilities
+ Ac01PcieCoreGetEndpointInfo (RC, PcieIndex, &EpMaxWidth, &EpMaxGen);
+ LinkStatusCheck = Ac01PcieCoreLinkCheck (RC, PcieIndex, EpMaxWidth, EpMaxGen);
+
+ // Delay to allow the link to perform internal operation and generate
+ // any error status update. This allows detection of any error observed
+ // during initial link training. Possible evaluation time can be
+ // between 100ms to 200ms.
+ MicroSecondDelay (100000);
+
+ // Check for error
+ RasdesChecking = Ac01PFAEnableAll (RC, PcieIndex, PFA_REG_READ);
+
+ // Clear error counter
+ Ac01PFAEnableAll (RC, PcieIndex, PFA_REG_CLEAR);
+
+ // If link check functions return passed, then breaking out
+ // else go to soft reset
+ if (LinkStatusCheck != LINK_CHECK_FAILED &&
+ RasdesChecking != LINK_CHECK_FAILED &&
+ PcieLinkUpCheck (&RC->Pcie[PcieIndex]))
+ {
+ return LINK_CHECK_SUCCESS;
+ }
+
+ RC->Pcie[PcieIndex].LinkUp = FALSE;
+ }
+
+ // Trigger controller soft reset
+ DEBUG_PCIE_INFO ("PCIE%d.%d Start link re-initialization..\n", RC->ID, PcieIndex);
+ Ac01PcieCoreSetupRC (RC, 1, PcieIndex);
+
+ // Poll until link up
+ // This checking for linkup status and
+ // give LTSSM state the time to transit from DECTECT state to L0 state
+ // Total delay are 100ms, smaller number of delay cannot always make sure
+ // the state transition is completed
+ TimeOut = LTSSM_TRANSITION_TIMEOUT;
+ CsrAddr = (VOID *)RC->Pcie[PcieIndex].CsrAddr;
+ do {

This do/while inside a do/while loop - helper?

+ if (PcieLinkUpCheck (&RC->Pcie[PcieIndex])) {
+ DEBUG_PCIE_INFO (
+ "\tPCIE%d.%d LinkStat is correct after soft reset, transition time: %d\n",
+ RC->ID,
+ PcieIndex,
+ TimeOut
+ );
+ RC->Pcie[PcieIndex].LinkUp = TRUE;
+ break;
+ }
+
+ MicroSecondDelay (100);
+ TimeOut -= 100;
+ } while (TimeOut > 0);
+
+ if (TimeOut <= 0) {
+ DEBUG_PCIE_INFO ("\tPCIE%d.%d LinkStat TIMEOUT after re-init\n", RC->ID, PcieIndex);
+ } else {
+ DEBUG_PCIE_INFO ("PCIE%d.%d Link re-initialization passed!\n", RC->ID, PcieIndex);
+ }
+
+ NumberOfReset--;
+ } while (NumberOfReset > 0);
+
+ return LINK_CHECK_SUCCESS;
+}
+
+VOID
+Ac01PcieCoreUpdateLink (
+ IN AC01_RC *RC,
+ OUT BOOLEAN *IsNextRoundNeeded,
+ OUT INT8 *FailedPciePtr,
+ OUT INT8 *FailedPcieCount
+ )
+{
+ INTN PcieIndex;
+ AC01_PCIE *Pcie;
+ UINT32 i;
+ UINT32 Val;
+ VOID *CfgAddr;
+
+ *IsNextRoundNeeded = FALSE;
+ *FailedPcieCount = 0;
+ for (i = 0; i < MAX_PCIE_B; i++) {
+ FailedPciePtr[i] = -1;
+ }
+
+ if (!RC->Active) {
+ return;
+ }
+
+ // Loop for all controllers
+ for (PcieIndex = 0; PcieIndex < RC->MaxPcieController; PcieIndex++) {
+ Pcie = &RC->Pcie[PcieIndex];
+ CfgAddr = (VOID *)(RC->MmcfgAddr + (RC->Pcie[PcieIndex].DevNum << 15));
+
+ if (Pcie->Active && !Pcie->LinkUp) {
+ if (PcieLinkUpCheck (Pcie)) {
+ Pcie->LinkUp = TRUE;
+ Ac01PcieCsrIn32 (CfgAddr + LINK_CONTROL_LINK_STATUS_REG, &Val);
+ DEBUG_PCIE_INFO (
+ "%a S%d RC%d RP%d NEGO_LINK_WIDTH: 0x%x LINK_SPEED: 0x%x\n",
+ __FUNCTION__,
+ RC->Socket,
+ RC->ID,
+ PcieIndex,
+ CAP_NEGO_LINK_WIDTH_GET (Val),
+ CAP_LINK_SPEED_GET (Val)
+ );
+
+ // Doing link checking and recovery if needed
+ Ac01PcieCoreQoSLinkCheckRecovery (RC, PcieIndex);
+
+ // Un-mask Completion Timeout

Sounds like a helper function.

+ Ac01PcieCsrIn32 (CfgAddr + AMBA_LINK_TIMEOUT_OFF, &Val);
+ Val = LINK_TIMEOUT_PERIOD_DEFAULT_SET (Val, 32);
+ Ac01PcieCsrOut32 (CfgAddr + AMBA_LINK_TIMEOUT_OFF, Val);
+ Ac01PcieCsrIn32 (CfgAddr + UNCORR_ERR_MASK_OFF, &Val);
+ Val = CMPLT_TIMEOUT_ERR_MASK_SET (Val, 0);
+ Ac01PcieCsrOut32 (CfgAddr + UNCORR_ERR_MASK_OFF, Val);
+ } else {
+ *IsNextRoundNeeded = FALSE;
+ FailedPciePtr[*FailedPcieCount] = PcieIndex;
+ *FailedPcieCount += 1;
+ }
+ }
+ }
+}
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreLib.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreLib.c
new file mode 100644
index 000000000000..42a0f240763c
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PcieCoreLib.c
@@ -0,0 +1,556 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/AmpereCpuLib.h>
+#include <Library/ArmGenericTimerCounterLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BoardPcieLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PciHostBridgeLib.h>
+#include <Library/PrintLib.h>
+#include <Library/SerialPortLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Platform/Ac01.h>
+#include <Protocol/PciHostBridgeResourceAllocation.h>
+
+#include "PcieCore.h"
+#include "PciePatchAcpi.h"
+
+STATIC UINT64 RCRegBase[AC01_MAX_PCIE_ROOT_COMPLEX] = { AC01_PCIE_REGISTER_BASE };
+STATIC UINT64 RCMmioBase[AC01_MAX_PCIE_ROOT_COMPLEX] = { AC01_PCIE_MMIO_BASE };
+STATIC UINT64 RCMmioSize[AC01_MAX_PCIE_ROOT_COMPLEX] = {AC01_PCIE_MMIO_SIZE};
+STATIC UINT64 RCMmio32Base[AC01_MAX_PCIE_ROOT_COMPLEX] = { AC01_PCIE_MMIO32_BASE };
+STATIC UINT64 RCMmio32Base1P[AC01_MAX_PCIE_ROOT_COMPLEX] = { AC01_PCIE_MMIO32_BASE_1P };
+STATIC UINT64 RCMmio32Size[AC01_MAX_PCIE_ROOT_COMPLEX] = {AC01_PCIE_MMIO32_SIZE};
+STATIC UINT64 RCMmio32Size1P[AC01_MAX_PCIE_ROOT_COMPLEX] = {AC01_PCIE_MMIO32_SIZE_1P};
+STATIC AC01_RC RCList[AC01_MAX_PCIE_ROOT_COMPLEX];
+STATIC INT8 PciList[AC01_MAX_PCIE_ROOT_COMPLEX];

Global variables need m or g prefix.

+
+STATIC
+VOID
+SerialPrint (

Please, not yet another way to output debug info...
Nuke it.

+ IN CONST CHAR8 *FormatString,
+ ...
+ )
+{
+ UINT8 Buf[64];
+ VA_LIST Marker;
+ UINTN NumberOfPrinted;
+
+ VA_START (Marker, FormatString);
+ NumberOfPrinted = AsciiVSPrint ((CHAR8 *)Buf, sizeof (Buf), FormatString, Marker);
+ SerialPortWrite (Buf, NumberOfPrinted);
+ VA_END (Marker);
+}
+
+AC01_RC *
+GetRCList (
+ UINT8 Idx
+ )
+{
+ return &RCList[Idx];
+}
+
+/**
+ Map BusDxe Host bridge and Root bridge Indexes to PCIE core BSP driver Root Complex Index.
+
+ @param HBIndex Index to identify of PCIE Host bridge.
+ @param RBIndex Index to identify of PCIE Root bridge.

HB -> HostBridge
RB -> RootBridge
Search/replace.

+ @retval UINT8 Index to identify Root Complex instance from global RCList.
+**/
+STATIC
+UINT8
+GetRCIndex (
+ IN UINT8 HBIndex,
+ IN UINT8 RBIndex
+ )
+{
+ //
+ // BusDxe addresses resources based on Host bridge and Root bridge.
+ // Map those to Root Complex index/instance known for Pcie Core BSP driver
+ //
+ return HBIndex * AC01_MAX_PCIE_ROOT_BRIDGE + RBIndex;
+}
+
+/**
+ Prepare to start PCIE core BSP driver
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetup (
+ VOID
+ )
+{
+ AC01_RC *RC;
+ INTN RCIndex;
+
+ ZeroMem (&RCList, sizeof (AC01_RC) * AC01_MAX_PCIE_ROOT_COMPLEX);
+
+ // Adjust MMIO32 base address 1P vs 2P
+ if (!IsSlaveSocketPresent ()) {

Hmm. Another thing I missed in previous review.
TianoCore does not yet have a policy for inclusive language, but you
might want to give your set a renaming pass for good measure.

+ CopyMem ((VOID *)&RCMmio32Base, (VOID *)&RCMmio32Base1P, sizeof (RCMmio32Base1P));
+ CopyMem ((VOID *)&RCMmio32Size, (VOID *)&RCMmio32Size1P, sizeof (RCMmio32Size1P));
+ }
+
+ for (RCIndex = 0; RCIndex < AC01_MAX_PCIE_ROOT_COMPLEX; RCIndex++) {
+ RC = &RCList[RCIndex];
+ RC->Socket = RCIndex / AC01_MAX_RCS_PER_SOCKET;
+ RC->ID = RCIndex % AC01_MAX_RCS_PER_SOCKET;
+
+ Ac01PcieCoreBuildRCStruct (
+ RC,
+ RCRegBase[RCIndex],
+ RCMmioBase[RCIndex],
+ RCMmioSize[RCIndex],
+ RCMmio32Base[RCIndex],
+ RCMmio32Size[RCIndex]
+ );
+ }
+
+ // Build the UEFI menu
+ BoardPcieScreenInitialize (RCList);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Get Total HostBridge.
+
+ @retval UINTN Return Total HostBridge.
+**/
+UINT8
+Ac01PcieGetTotalHBs (
+ VOID
+ )
+{
+ return AC01_MAX_PCIE_ROOT_COMPLEX;
+}
+
+/**
+ Get Total RootBridge per HostBridge.
+
+ @param[in] RCIndex Index to identify of Root Complex.
+
+ @retval UINTN Return Total RootBridge per HostBridge.
+**/
+UINT8
+Ac01PcieGetTotalRBsPerHB (
+ IN UINTN RCIndex
+ )
+{
+ return AC01_MAX_PCIE_ROOT_BRIDGE;
+}
+
+/**
+ Get RootBridge Attribute.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval UINTN Return RootBridge Attribute.
+**/
+UINTN
+Ac01PcieGetRootBridgeAttribute (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ )
+{
+ return EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
+}
+
+/**
+ Get RootBridge Segment number
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval UINTN Return RootBridge Segment number.
+**/
+UINTN
+Ac01PcieGetRootBridgeSegmentNumber (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ )
+{
+ UINTN RCIndex;
+ AC01_RC *RC;
+ UINTN SegmentNumber;
+
+ RCIndex = GetRCIndex (HBIndex, RBIndex);
+ RC = &RCList[RCIndex];
+ SegmentNumber = RCIndex;
+
+ // Get board specific overrides
+ BoardPcieGetRCSegmentNumber (RC, &SegmentNumber);
+ RC->Logical = SegmentNumber;
+
+ return SegmentNumber;
+}
+
+STATIC
+VOID
+SortPciList (
+ INT8 *PciList
+ )
+{
+ INT8 Idx1, Idx2;
+
+ for (Idx2 = 0, Idx1 = 0; Idx2 < AC01_MAX_PCIE_ROOT_COMPLEX; Idx2++) {
+ if (PciList[Idx2] < 0) {
+ continue;
+ }
+ PciList[Idx1] = PciList[Idx2];
+ if (Idx1 != Idx2) {
+ PciList[Idx2] = -1;
+ }
+ Idx1++;
+ }
+
+ for (Idx2 = 0; Idx2 < Idx1; Idx2++) {
+ DEBUG_PCIE_INFO (
+ " %a: PciList[%d]=%d TcuAddr=0x%llx\n",
+ __FUNCTION__,
+ Idx2,
+ PciList[Idx2],
+ RCList[PciList[Idx2]].TcuAddr
+ );
+ }
+}
+
+/**
+ Get RootBridge disable status.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+
+ @retval BOOLEAN Return RootBridge disable status.
+**/
+BOOLEAN
+Ac01PcieCheckRootBridgeDisabled (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex
+ )
+{
+ UINTN RCIndex;
+ INT8 Ret;
+
+ RCIndex = HBIndex;
+ Ret = !RCList[RCIndex].Active;
+ if (Ret) {
+ PciList[HBIndex] = -1;
+ } else {
+ PciList[HBIndex] = HBIndex;
+ }
+ if (HBIndex == (AC01_MAX_PCIE_ROOT_COMPLEX -1)) {
+ SortPciList (PciList);
+ if (!IsSlaveSocketPresent ()) {
+ AcpiPatchPciMem32 (PciList);
+ }
+ AcpiInstallMcfg (PciList);
+ AcpiInstallIort (PciList);
+ }
+ return Ret;
+}
+
+/**
+ Initialize Host bridge
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetupHostBridge (
+ IN UINTN HBIndex
+ )
+{
+ return EFI_SUCCESS;
+}
+
+/**
+ Initialize Root bridge
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+ @param[in] RootBridgeInstance The pointer of instance of the Root bridge IO.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieSetupRootBridge (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex,
+ IN PCI_ROOT_BRIDGE *RootBridge
+ )
+{
+ UINTN RCIndex;
+ AC01_RC *RC;
+ UINT32 Result;
+
+ RCIndex = GetRCIndex (HBIndex, RBIndex);
+ RC = &RCList[RCIndex];
+ if (!RC->Active) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ RC->RootBridge = (VOID *)RootBridge;
+
+ // Initialize Root Complex and underneath controllers
+ Result = Ac01PcieCoreSetupRC (RC, 0, 0);
+ if (Result) {
+ DEBUG_PCIE_ERR ("RootComplex[%d]: Init Failed\n", RCIndex);
+
+ goto Error;
+ }
+
+ // Populate resource aperture
+ RootBridge->Bus.Base = 0x0;
+ RootBridge->Bus.Limit = 0xFF;
+ RootBridge->Io.Base = RC->IoAddr;
+ RootBridge->Io.Limit = RC->IoAddr + IO_SPACE - 1;
+ RootBridge->Mem.Base = RC->Mmio32Addr;
+ RootBridge->Mem.Limit = (RootBridge->Mem.Base) ? (RootBridge->Mem.Base + RC->Mmio32Size - 1) : 0;
+ RootBridge->PMem.Base = RootBridge->Mem.Base;
+ RootBridge->PMem.Limit = RootBridge->Mem.Limit;
+ RootBridge->MemAbove4G.Base = MAX_UINT64;
+ RootBridge->MemAbove4G.Limit = 0x0;
+ RootBridge->PMemAbove4G.Base = RC->MmioAddr;
+ RootBridge->PMemAbove4G.Limit = (RootBridge->PMemAbove4G.Base) ? (RootBridge->PMemAbove4G.Base + RC->MmioSize - 1) : 0;
+
+ DEBUG_PCIE_INFO (" + Bus: 0x%lx - 0x%lx\n", RootBridge->Bus.Base, RootBridge->Bus.Limit);
+ DEBUG_PCIE_INFO (" + Io: 0x%lx - 0x%lx\n", RootBridge->Io.Base, RootBridge->Io.Limit);
+ DEBUG_PCIE_INFO (" + Mem: 0x%lx - 0x%lx\n", RootBridge->Mem.Base, RootBridge->Mem.Limit);
+ DEBUG_PCIE_INFO (" + PMem: 0x%lx - 0x%lx\n", RootBridge->PMem.Base, RootBridge->PMem.Limit);
+ DEBUG_PCIE_INFO (" + 4GMem: 0x%lx - 0x%lx\n", RootBridge->MemAbove4G.Base, RootBridge->MemAbove4G.Limit);
+ DEBUG_PCIE_INFO (" + 4GPMem: 0x%lx - 0x%lx\n", RootBridge->PMemAbove4G.Base, RootBridge->PMemAbove4G.Limit);
+
+ return EFI_SUCCESS;
+
+Error:
+ RC->Active = FALSE;
+ return EFI_DEVICE_ERROR;
+}
+
+/**
+ Reads/Writes an PCI configuration register.
+
+ @param[in] RootInstance Pointer to RootInstance structure.
+ @param[in] Address Address which want to read or write to.
+ @param[in] Write Indicate that this is a read or write command.
+ @param[in] Width Specify the width of the data.
+ @param[in, out] Data The buffer to hold the data.
+
+ @retval EFI_SUCCESS The operation is successful.
+ @retval Others An error occurred.
+**/
+EFI_STATUS
+Ac01PcieConfigRW (
+ IN VOID *RootInstance,
+ IN UINT64 Address,
+ IN BOOLEAN Write,
+ IN UINTN Width,
+ IN OUT VOID *Data
+ )
+{
+ AC01_RC *RC = NULL;
+ VOID *CfgBase = NULL;
+ UINTN RCIndex;
+ UINT32 Reg;
+ UINT32 Segment;
+
+ Segment = RShiftU64 (Address, 32) & 0xFFFF;
+
+ for (RCIndex = 0; RCIndex < AC01_MAX_PCIE_ROOT_COMPLEX; RCIndex++) {
+ RC = &RCList[RCIndex];
+ if (RC->RootBridge != NULL) {
+ if (RC->RootBridge == RootInstance
+ || (RootInstance == NULL
+ && ((PCI_ROOT_BRIDGE *)RC->RootBridge)->Segment == Segment)) {
+ break;
+ }
+ }
+ }
+
+ if ((RCIndex == AC01_MAX_PCIE_ROOT_COMPLEX) || (RC == NULL)) {
+ DEBUG_PCIE_ERR ("Can't find Root Bridge instance:%p\n", RootInstance);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Reg = Address & 0xFFF;
+
+ CfgBase = (VOID *)((UINT64)RC->MmcfgAddr + (Address & 0x0FFFF000));
+ if (Write) {
+ switch (Width) {
+ case 1:
+ Ac01PcieCfgOut8 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), *((UINT8 *)Data));
+ break;
+
+ case 2:
+ Ac01PcieCfgOut16 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), *((UINT16 *)Data));
+ break;
+
+ case 4:
+ Ac01PcieCfgOut32 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), *((UINT32 *)Data));
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ } else {
+ switch (Width) {
+ case 1:
+ Ac01PcieCfgIn8 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), (UINT8 *)Data);
+ break;
+
+ case 2:
+ Ac01PcieCfgIn16 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), (UINT16 *)Data);
+ if (Reg == 0xAE && (*((UINT16 *)Data)) == 0xFFFF) {
+ SerialPrint ("PANIC due to PCIE RC:%d link issue\n", RC->ID);
+ // Loop forever waiting for failsafe/watch dog time out
+ do {
+ } while (1);
+ }
+ break;
+
+ case 4:
+ Ac01PcieCfgIn32 ((VOID *)(CfgBase + (Reg & (~(Width - 1)))), (UINT32 *)Data);
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+VOID
+Ac01PcieCorePollLinkUp (
+ VOID
+ )
+{
+ INTN RCIndex, PcieIndex, i;
+ BOOLEAN IsNextRoundNeeded = FALSE, NextRoundNeeded;
+ UINT64 PrevTick, CurrTick, ElapsedCycle;
+ UINT64 TimerTicks64;
+ UINT8 ReInit;
+ INT8 FailedPciePtr[MAX_PCIE_B];
+ INT8 FailedPcieCount;
+
+ ReInit = 0;
+
+_link_polling:
+ NextRoundNeeded = 0;
+ //
+ // It is not guaranteed the timer service is ready prior to PCI Dxe.
+ // Calculate system ticks for link training.
+ //
+ TimerTicks64 = ArmGenericTimerGetTimerFreq (); /* 1 Second */
+ PrevTick = ArmGenericTimerGetSystemCount ();
+ ElapsedCycle = 0;
+
+ do {
+ // Update timer
+ CurrTick = ArmGenericTimerGetSystemCount ();
+ if (CurrTick < PrevTick) {
+ ElapsedCycle += (UINT64)(~0x0ULL) - PrevTick;
+ PrevTick = 0;
+ }
+ ElapsedCycle += (CurrTick - PrevTick);
+ PrevTick = CurrTick;
+ } while (ElapsedCycle < TimerTicks64);
+
+ for (RCIndex = 0; RCIndex < AC01_MAX_PCIE_ROOT_COMPLEX; RCIndex++) {
+ Ac01PcieCoreUpdateLink (&RCList[RCIndex], &IsNextRoundNeeded, FailedPciePtr, &FailedPcieCount);
+ if (IsNextRoundNeeded) {
+ NextRoundNeeded = 1;
+ }
+ }
+
+ if (NextRoundNeeded && ReInit < MAX_REINIT) {
+ // Timer is up. Give another chance to re-program controller
+ ReInit++;
+ for (RCIndex = 0; RCIndex < AC01_MAX_PCIE_ROOT_COMPLEX; RCIndex++) {
+ Ac01PcieCoreUpdateLink (&RCList[RCIndex], &IsNextRoundNeeded, FailedPciePtr, &FailedPcieCount);
+ if (IsNextRoundNeeded) {
+ for (i = 0; i < FailedPcieCount; i++) {
+ PcieIndex = FailedPciePtr[i];
+ if (PcieIndex == -1) {
+ continue;
+ }
+
+ // Some controller still observes link-down. Re-init controller
+ Ac01PcieCoreSetupRC (&RCList[RCIndex], 1, PcieIndex);
+ }
+ }
+ }
+
+ goto _link_polling;
+ }
+}
+
+/**
+ Prepare to end PCIE core BSP driver
+**/
+VOID
+Ac01PcieEnd (
+ VOID
+ )
+{
+ Ac01PcieCorePollLinkUp ();
+}
+
+/**
+ Callback funciton for EndEnumeration notification from PCI stack.
+
+ @param[in] HBIndex Index to identify of PCIE Host bridge.
+ @param[in] RBIndex Index to identify of underneath PCIE Root bridge.
+ @param[in] Phase The phase of enumeration as informed from PCI stack.
+**/
+VOID
+Ac01PcieHostBridgeNotifyPhase (
+ IN UINTN HBIndex,
+ IN UINTN RBIndex,
+ IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PHASE Phase
+ )
+{
+ AC01_RC *RC;
+ UINTN RCIndex;
+
+ RCIndex = GetRCIndex (HBIndex, RBIndex);
+ RC = &RCList[RCIndex];
+
+ switch (Phase) {
+ case EfiPciHostBridgeEndEnumeration:
+ Ac01PcieCoreEndEnumeration (RC);
+ break;
+
+ case EfiPciHostBridgeBeginEnumeration:
+ /* 100ms that help fixing completion timeout issue */
+ MicroSecondDelay (100000);
+ break;
+
+ case EfiPciHostBridgeBeginBusAllocation:
+ case EfiPciHostBridgeEndBusAllocation:
+ case EfiPciHostBridgeBeginResourceAllocation:
+ case EfiPciHostBridgeAllocateResources:
+ case EfiPciHostBridgeSetResources:
+ case EfiPciHostBridgeFreeResources:
+ case EfiPciHostBridgeEndResourceAllocation:
+ case EfiMaxPciHostBridgeEnumerationPhase:
+ break;
+ }
+}
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.c b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.c
new file mode 100644
index 000000000000..b15dea459a07
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/Ac01PcieLib/PciePatchAcpi.c
@@ -0,0 +1,646 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <AcpiHeader.h>
+#include <IndustryStandard/Acpi30.h>
+#include <IndustryStandard/IoRemappingTable.h>
+#include <Library/AcpiLib.h>
+#include <Library/AmpereCpuLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BoardPcieLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PrintLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Platform/Ac01.h>
+#include <Protocol/AcpiTable.h>
+
+#include "PcieCore.h"
+
+#define ACPI_RESOURCE_NAME_ADDRESS16 0x88
+#define ACPI_RESOURCE_NAME_ADDRESS64 0x8A
+
+#define RCA_NUM_TBU_PMU 6
+#define RCB_NUM_TBU_PMU 10
+
+// Required to be 1 to match the kernel quirk for ECAM
+#define EFI_ACPI_MCFG_OEM_REVISION 1
+
+STATIC UINT32 gTbuPmuIrqArray[] = { AC01_SMMU_TBU_PMU_IRQS };
+STATIC UINT32 gTcuPmuIrqArray[] = { AC01_SMMU_TCU_PMU_IRQS };
+
+#pragma pack(1)
+typedef struct
+{
+ UINT64 ullBaseAddress;

Absolutely no Hungarian notation.

+ UINT16 usSegGroupNum;
+ UINT8 ucStartBusNum;
+ UINT8 ucEndBusNum;

Those three too.

+ UINT32 Reserved2;
+} EFI_MCFG_CONFIG_STRUCTURE;
+
+typedef struct
+{
+ EFI_ACPI_DESCRIPTION_HEADER Header;
+ UINT64 Reserved1;
+} EFI_MCFG_TABLE_CONFIG;
+
+typedef struct {
+ UINT64 AddressGranularity;
+ UINT64 AddressMin;
+ UINT64 AddressMax;
+ UINT64 AddressTranslation;
+ UINT64 RangeLength;
+} QWordMemory;
+
+typedef struct ResourceEntry {
+ UINT8 ResourceType;
+ UINT16 ResourceSize;
+ UINT8 Attribute;
+ UINT8 Byte0;
+ UINT8 Byte1;
+ VOID *ResourcePtr;
+} RESOURCE;
+
+STATIC QWordMemory Qmem[] = {
+ { AC01_PCIE_RCA2_QMEM },
+ { AC01_PCIE_RCA3_QMEM },
+ { AC01_PCIE_RCB0_QMEM },
+ { AC01_PCIE_RCB1_QMEM },
+ { AC01_PCIE_RCB2_QMEM },
+ { AC01_PCIE_RCB3_QMEM }
+};
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_NODE Node;
+ UINT64 Base;
+ UINT32 Flags;
+ UINT32 Reserved;
+ UINT64 VatosAddress;
+ UINT32 Model;
+ UINT32 Event;
+ UINT32 Pri;
+ UINT32 Gerr;
+ UINT32 Sync;
+ UINT32 ProximityDomain;
+ UINT32 DeviceIdMapping;
+} EFI_ACPI_6_2_IO_REMAPPING_SMMU3_NODE;
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE Node;
+ UINT32 ItsIdentifier;
+} AC01_ITS_NODE;
+
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_RC_NODE Node;
+ EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE RcIdMapping;
+} AC01_RC_NODE;
+
+typedef struct {
+ EFI_ACPI_6_2_IO_REMAPPING_SMMU3_NODE Node;
+ EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE InterruptMsiMapping;
+ EFI_ACPI_6_0_IO_REMAPPING_ID_TABLE InterruptMsiMappingSingle;
+} AC01_SMMU_NODE;
+
+typedef struct {
+ EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort;
+ AC01_ITS_NODE ItsNode[2];
+ AC01_RC_NODE RcNode[2];
+ AC01_SMMU_NODE SmmuNode[2];
+} AC01_IO_REMAPPING_STRUCTURE;
+
+#define FIELD_OFFSET(type, name) __builtin_offsetof (type, name)

Use OFFSET_OF from Base.h.
If we ever need toolchain-specific macros, they should explicitly be
guarded by macros so other toolchains don't try to invoke them. The
Base.h version does this.

+#define __AC01_ID_MAPPING(In, Num, Out, Ref, Flags) \
+ { \
+ In, \
+ Num, \
+ Out, \
+ FIELD_OFFSET (AC01_IO_REMAPPING_STRUCTURE, Ref), \
+ Flags \
+ }
+
+EFI_STATUS
+EFIAPI
+AcpiPatchPciMem32 (
+ INT8 *PciSegEnabled
+ )
+{
+ EFI_ACPI_SDT_PROTOCOL *AcpiSdtProtocol;
+ EFI_STATUS Status;
+ UINTN Idx, Ix;

This borders very closely on code obfuscation.
I, J are fine and accepted.
The code has used Idx1, Idx2 in other places, and I won't kick back on
that.

+ EFI_ACPI_DESCRIPTION_HEADER *Table;
+ UINTN TableKey;
+ UINTN TableIndex;
+ EFI_ACPI_HANDLE TableHandle, SegHandle;
+ CHAR8 Buffer[256];
+ CHAR8 *KB, *B;

Use proper names.

+ EFI_ACPI_DATA_TYPE DataType;
+ UINTN DataSize, Mem32;
+ RESOURCE *Rs;
+ QWordMemory *Qm;

Use proper names.

+ UINT8 Segment;

This list of local variables is in itself a good hint that this
function should be split up into some helpers.

+
+ Status = gBS->LocateProtocol (&gEfiAcpiSdtProtocolGuid, NULL, (VOID **)&AcpiSdtProtocol);
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("Unable to locate ACPI table protocol Guid\n");
+ return Status;
+ }
+
+ TableIndex = 0;
+ Status = AcpiLocateTableBySignature (
+ AcpiSdtProtocol,
+ EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ &TableIndex,
+ &Table,
+ &TableKey
+ );
+ Status = AcpiSdtProtocol->OpenSdt (TableKey, &TableHandle);
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("Unable to open DSDT table\n");
+ return Status;
+ }
+
+ for (Idx = 0; PciSegEnabled[Idx] != -1; Idx++) {
+ if (PciSegEnabled[Idx] > SOCKET0_LAST_RC) { /* Physical segment */
+ break;
+ }
+ if (PciSegEnabled[Idx] < SOCKET0_FIRST_RC) {
+ continue;
+ }
+
+ /* DSDT PCI devices to use Physical segment */
+ AsciiSPrint (Buffer, sizeof (Buffer), "\\_SB.PCI%x.RBUF", PciSegEnabled[Idx]);
+ Status = AcpiSdtProtocol->FindPath (TableHandle, (VOID *)Buffer, &SegHandle);
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ for (Ix = 0; Ix < 3; Ix++) {
+ Status = AcpiSdtProtocol->GetOption (
+ SegHandle,
+ Ix,
+ &DataType,
+ (VOID *)&B,
+ &DataSize
+ );
+ KB = B;

OK, that reads KiloByte = Byte to me.
Please use proper variable names.

+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ if (Ix == 0) { /* B[0] == AML_NAME_OP */
+ if (!((DataSize == 1) && (DataType == EFI_ACPI_DATA_TYPE_OPCODE))) {
+ break;
+ }
+ } else if (Ix == 1) { /* *B == "RBUF" */
+ if (!((DataSize == 4) && (DataType == EFI_ACPI_DATA_TYPE_NAME_STRING))) {
+ break;
+ }
+ } else { /* Ix:2 11 42 07 0A 6E 88 ... */
+ if (DataType != EFI_ACPI_DATA_TYPE_CHILD) {
+ break;
+ }
+
+ KB += 5; /* Point to Resource type */
+ Rs = (RESOURCE *)KB;
+ Mem32 = 0;
+ while ((Mem32 == 0) && ((KB - B) < DataSize)) {
+ if (Rs->ResourceType == ACPI_RESOURCE_NAME_ADDRESS16) {
+ KB += (Rs->ResourceSize + 3); /* Type + Size */
+ Rs = (RESOURCE *)KB;
+ } else if (Rs->ResourceType == ACPI_RESOURCE_NAME_ADDRESS64) {
+
+ if (Rs->Attribute == 0x00) { /* The first QWordMemory */
+ Mem32 = 1;
+ Segment = PciSegEnabled[Idx] - 2;
+ Qm = (QWordMemory *)&(Rs->ResourcePtr);
+ *Qm = Qmem[Segment]; /* Physical segment */
+ }
+ KB += (Rs->ResourceSize + 3); /* Type + Size */
+ Rs = (RESOURCE *)KB;
+ }
+ }
+ if (Mem32 != 0) {
+ Status = AcpiSdtProtocol->SetOption (
+ SegHandle,
+ Ix,
+ (VOID *)B,
+ DataSize
+ );
+ }
+ }
+ }
+
+ AcpiSdtProtocol->Close (SegHandle);
+ }
+
+ AcpiSdtProtocol->Close (TableHandle);
+ AcpiUpdateChecksum ((UINT8 *)Table, Table->Length);
+
+ return Status;
+}
+
+VOID
+ConstructMcfg (
+ VOID *McfgPtr,

It's a pointer, it doesn't have to be named Ptr.

+ INT8 *PciSegEnabled,
+ UINT32 McfgCount
+ )
+{
+ EFI_MCFG_TABLE_CONFIG McfgHeader = {
+ {
+ EFI_ACPI_6_1_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
+ McfgCount,
+ 1,
+ 0x00, // Checksum will be updated at runtime
+ EFI_ACPI_OEM_ID,
+ EFI_ACPI_OEM_TABLE_ID,
+ EFI_ACPI_MCFG_OEM_REVISION,
+ EFI_ACPI_CREATOR_ID,
+ EFI_ACPI_CREATOR_REVISION
+ },
+ 0x0000000000000000, // Reserved
+ };
+ EFI_MCFG_CONFIG_STRUCTURE TMcfg = {
+ .ullBaseAddress = 0,
+ .usSegGroupNum = 0,
+ .ucStartBusNum = 0,
+ .ucEndBusNum = 255,
+ .Reserved2 = 0,
+ };
+ UINT32 Idx;
+ VOID *TMcfgPtr = McfgPtr;

What does T mean? CamelCase please.

+ AC01_RC *Rc;
+
+ CopyMem (TMcfgPtr, &McfgHeader, sizeof (EFI_MCFG_TABLE_CONFIG));
+ TMcfgPtr += sizeof (EFI_MCFG_TABLE_CONFIG);
+ for (Idx = 0; PciSegEnabled[Idx] != -1; Idx++) {
+ Rc = GetRCList (PciSegEnabled[Idx]); /* Logical */
+ TMcfg.ullBaseAddress = Rc->MmcfgAddr;
+ TMcfg.usSegGroupNum = Rc->Logical;
+ CopyMem (TMcfgPtr, &TMcfg, sizeof (EFI_MCFG_CONFIG_STRUCTURE));
+ TMcfgPtr += sizeof (EFI_MCFG_CONFIG_STRUCTURE);
+ }
+}
+
+EFI_STATUS
+EFIAPI
+AcpiInstallMcfg (
+ INT8 *PciSegEnabled
+ )
+{
+ UINT32 RcCount, McfgCount;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiSdtProtocol;
+ UINTN TableKey;
+ EFI_STATUS Status;
+ VOID *McfgPtr;

It's a pointer, it doesn't need to be called pointer.

+
+ Status = gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID **)&AcpiSdtProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("MCFG: Unable to locate ACPI table entry\n");
+ return Status;
+ }
+ for (RcCount = 0; PciSegEnabled[RcCount] != -1; RcCount++) {
+ }
+ McfgCount = sizeof (EFI_MCFG_TABLE_CONFIG) + sizeof (EFI_MCFG_CONFIG_STRUCTURE) * RcCount;
+ McfgPtr = AllocateZeroPool (McfgCount);
+ if (McfgPtr == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ ConstructMcfg (McfgPtr, PciSegEnabled, McfgCount);
+ Status = AcpiSdtProtocol->InstallAcpiTable (
+ AcpiSdtProtocol,
+ McfgPtr,
+ McfgCount,
+ &TableKey
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("MCFG: Unable to install MCFG table entry\n");
+ }
+ FreePool (McfgPtr);
+ return Status;
+}
+
+STATIC
+VOID
+ConstructIort (
+ VOID *IortPtr,
+ UINT32 RcCount,
+ UINT32 SmmuPmuAgentCount,
+ UINT32 HeaderCount,
+ INT8 *PciSegEnabled

Segment

+ )
+{
+ EFI_ACPI_6_0_IO_REMAPPING_TABLE TIort = {
+ .Header = __ACPI_HEADER (
+ EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,
+ AC01_IO_REMAPPING_STRUCTURE,
+ EFI_ACPI_IO_REMAPPING_TABLE_REVISION
+ ),
+ .NumNodes = (3 * RcCount) + SmmuPmuAgentCount,
+ .NodeOffset = sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE),
+ 0
+ };
+
+ AC01_ITS_NODE TItsNode = {

What's T? CamelCase - write it out.
(Please address throughout.)

+ .Node = {
+ {
+ EFI_ACPI_IORT_TYPE_ITS_GROUP,
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_ITS_NODE) + 4,
+ 0x0,
+ 0x0,
+ 0x0,
+ 0x0,
+ },
+ .NumItsIdentifiers = 1,
+ },
+ .ItsIdentifier = 1,
+ };
+
+ AC01_RC_NODE TRcNode = {
+ {
+ {
+ EFI_ACPI_IORT_TYPE_ROOT_COMPLEX,
+ sizeof (AC01_RC_NODE),
+ 0x1,
+ 0x0,
+ 0x1,
+ FIELD_OFFSET (AC01_RC_NODE, RcIdMapping),
+ },
+ EFI_ACPI_IORT_MEM_ACCESS_PROP_CCA,
+ 0x0,
+ 0x0,
+ EFI_ACPI_IORT_MEM_ACCESS_FLAGS_CPM |
+ EFI_ACPI_IORT_MEM_ACCESS_FLAGS_DACS,
+ EFI_ACPI_IORT_ROOT_COMPLEX_ATS_UNSUPPORTED,
+ .PciSegmentNumber = 0,
+ .MemoryAddressSize = 64,
+ },
+ __AC01_ID_MAPPING (0x0, 0xffff, 0x0, SmmuNode, 0),
+ };
+
+ AC01_SMMU_NODE TSmmuNode = {
+ {
+ {
+ EFI_ACPI_IORT_TYPE_SMMUv3,
+ sizeof (AC01_SMMU_NODE),
+ 0x2, /* Revision */
+ 0x0,
+ 0x2, /* Mapping Count */
+ FIELD_OFFSET (AC01_SMMU_NODE, InterruptMsiMapping),
+ },
+ .Base = 0,
+ EFI_ACPI_IORT_SMMUv3_FLAG_COHAC_OVERRIDE,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0,
+ 0x0,
+ 0x0,
+ 0, // Proximity domain - need fill in
+ .DeviceIdMapping = 1,
+ },
+ __AC01_ID_MAPPING (0x0, 0xffff, 0, SmmuNode, 0),
+ __AC01_ID_MAPPING (0x0, 0x1, 0, SmmuNode, 1),
+ };
+
+ EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE TPmcgNode = {
+ {
+ EFI_ACPI_IORT_TYPE_PMCG,
+ sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE),
+ 0x1,
+ 0x0,
+ 0x0,
+ 0x0,
+ },
+ 0, /* Page 0 Base. Need to be filled */
+ 0, /* GSIV. Need to be filled */
+ 0, /* Node reference. Need to be filled */
+ 0, /* Page 1 Base. Need to be filled. */
+ };
+
+ UINT32 Idx, Idx1, SmmuNodeOffset[AC01_MAX_PCIE_ROOT_COMPLEX];
+ VOID *TIortPtr = IortPtr, *SmmuPtr, *PmcgPtr;
+ UINT32 ItsOffset[AC01_MAX_PCIE_ROOT_COMPLEX];
+ AC01_RC *Rc;
+ UINTN NumTbuPmu;

This splat serves as a good pointer that this should probably be more
than one function.

+
+ TIort.Header.Length = HeaderCount;
+ CopyMem (TIortPtr, &TIort, sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE));
+ TIortPtr += sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE);
+ for (Idx = 0; Idx < RcCount; Idx++) {
+ ItsOffset[Idx] = TIortPtr - IortPtr;
+ TItsNode.ItsIdentifier = PciSegEnabled[Idx]; /* Physical */
+ CopyMem (TIortPtr, &TItsNode, sizeof (AC01_ITS_NODE));
+ TIortPtr += sizeof (AC01_ITS_NODE);
+ }
+
+ SmmuPtr = TIortPtr + RcCount * sizeof (AC01_RC_NODE);
+ PmcgPtr = SmmuPtr + RcCount * sizeof (AC01_SMMU_NODE);
+ for (Idx = 0; Idx < RcCount; Idx++) {
+ SmmuNodeOffset[Idx] = SmmuPtr - IortPtr;
+ Rc = GetRCList (PciSegEnabled[Idx]); /* Physical RC */
+ TSmmuNode.Node.Base = Rc->TcuAddr;
+ TSmmuNode.InterruptMsiMapping.OutputBase = PciSegEnabled[Idx] << 16;
+ TSmmuNode.InterruptMsiMapping.OutputReference = ItsOffset[Idx];
+ TSmmuNode.InterruptMsiMappingSingle.OutputBase = PciSegEnabled[Idx] << 16;
+ TSmmuNode.InterruptMsiMappingSingle.OutputReference = ItsOffset[Idx];
+ /* All RCs on master be assigned to node 0, while remote RCs be assigned to first remote node */
+ TSmmuNode.Node.Flags = EFI_ACPI_IORT_SMMUv3_FLAG_PROXIMITY_DOMAIN;
+ TSmmuNode.Node.ProximityDomain = 0;
+ if ((Rc->TcuAddr & SLAVE_SOCKET_BASE_ADDRESS_OFFSET) != 0) {
+ /* RC on remote socket */
+ TSmmuNode.Node.Flags = EFI_ACPI_IORT_SMMUv3_FLAG_PROXIMITY_DOMAIN;
+ switch (CpuGetSubNumaMode ()) {
+ case SUBNUMA_MODE_MONOLITHIC:
+ TSmmuNode.Node.ProximityDomain += MONOLITIC_NUM_OF_REGION;
+ break;
+ case SUBNUMA_MODE_HEMISPHERE:
+ TSmmuNode.Node.ProximityDomain += HEMISPHERE_NUM_OF_REGION;
+ break;
+ case SUBNUMA_MODE_QUADRANT:
+ TSmmuNode.Node.ProximityDomain += QUADRANT_NUM_OF_REGION;
+ break;
+ }
+ }
+ CopyMem (SmmuPtr, &TSmmuNode, sizeof (AC01_SMMU_NODE));
+ SmmuPtr += sizeof (AC01_SMMU_NODE);
+
+ if (!SmmuPmuAgentCount) {
+ continue;
+ }
+
+ /* Add PMCG nodes */
+ if (Rc->Type == RCA) {
+ NumTbuPmu = RCA_NUM_TBU_PMU;
+ } else {
+ NumTbuPmu = RCB_NUM_TBU_PMU;
+ }
+ for (Idx1 = 0; Idx1 < NumTbuPmu; Idx1++) {

This for loop should be a function, and quite possibly each of the
case statements should be too.

+ TPmcgNode.Base = Rc->TcuAddr;
+ if (NumTbuPmu == RCA_NUM_TBU_PMU) {
+ switch (Idx1) {
+ case 0:
+ TPmcgNode.Base += 0x40000;
+ break;
+
+ case 1:
+ TPmcgNode.Base += 0x60000;
+ break;
+
+ case 2:
+ TPmcgNode.Base += 0xA0000;
+ break;
+
+ case 3:
+ TPmcgNode.Base += 0xE0000;
+ break;
+
+ case 4:
+ TPmcgNode.Base += 0x100000;
+ break;
+
+ case 5:
+ TPmcgNode.Base += 0x140000;
+ break;
+ }
+ } else {
+ switch (Idx1) {
+ case 0:
+ TPmcgNode.Base += 0x40000;
+ break;
+
+ case 1:
+ TPmcgNode.Base += 0x60000;
+ break;
+
+ case 2:
+ TPmcgNode.Base += 0xA0000;
+ break;
+
+ case 3:
+ TPmcgNode.Base += 0xE0000;
+ break;
+
+ case 4:
+ TPmcgNode.Base += 0x120000;
+ break;
+
+ case 5:
+ TPmcgNode.Base += 0x160000;
+ break;
+
+ case 6:
+ TPmcgNode.Base += 0x180000;
+ break;
+
+ case 7:
+ TPmcgNode.Base += 0x1C0000;
+ break;
+
+ case 8:
+ TPmcgNode.Base += 0x200000;
+ break;
+
+ case 9:
+ TPmcgNode.Base += 0x240000;
+ break;
+ }
+ }
+ TPmcgNode.Page1Base = TPmcgNode.Base + 0x12000;
+ TPmcgNode.Base += 0x2000;

For the case statements there might be an argument made that the
live-coded values would not be more readable by turning them into
#defines. (I wouldn't agree, but the case could be made.)
No such argument can be made here. Please provide some helpfully named
#defines for those values.

+ TPmcgNode.NodeReference = SmmuNodeOffset[Idx];
+ TPmcgNode.OverflowInterruptGsiv = gTbuPmuIrqArray[PciSegEnabled[Idx]] + Idx1;
+ CopyMem (PmcgPtr, &TPmcgNode, sizeof (TPmcgNode));
+ PmcgPtr += sizeof (TPmcgNode);
+ }
+
+ /* TCU PMCG */

A comment should be instantly distinguishable from a cat having walked
on the keyboard.

+ TPmcgNode.Base = Rc->TcuAddr;
+ TPmcgNode.Base += 0x2000;
+ TPmcgNode.Page1Base = Rc->TcuAddr + 0x12000;

And those ones, with interestingly an identical pair of live-coded values.

+ TPmcgNode.NodeReference = SmmuNodeOffset[Idx];
+ TPmcgNode.OverflowInterruptGsiv = gTcuPmuIrqArray[PciSegEnabled[Idx]];
+ CopyMem (PmcgPtr, &TPmcgNode, sizeof (TPmcgNode));
+ PmcgPtr += sizeof (TPmcgNode);
+ }
+
+ for (Idx = 0; Idx < RcCount; Idx++) {
+ TRcNode.Node.PciSegmentNumber = GetRCList (PciSegEnabled[Idx])->Logical; /* Logical */

That comment serves no purpose. Improve it or delete it.

/
Leif

+ TRcNode.RcIdMapping.OutputReference = SmmuNodeOffset[Idx];
+ CopyMem (TIortPtr, &TRcNode, sizeof (AC01_RC_NODE));
+ TIortPtr += sizeof (AC01_RC_NODE);
+ }
+}
+
+EFI_STATUS
+EFIAPI
+AcpiInstallIort (
+ INT8 *PciSegEnabled
+ )
+{
+ EFI_STATUS Status;
+ EFI_ACPI_TABLE_PROTOCOL *AcpiSdtProtocol;
+ UINTN TableKey;
+ VOID *IortPtr;
+ BOOLEAN IsSmmuPmuEnabled;
+ UINT32 RcCount, SmmuPmuAgentCount, TotalCount;
+
+ Status = gBS->LocateProtocol (
+ &gEfiAcpiTableProtocolGuid,
+ NULL,
+ (VOID **)&AcpiSdtProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("IORT: Unable to locate ACPI table entry\n");
+ return Status;
+ }
+
+ for (RcCount = 0, SmmuPmuAgentCount = 0; PciSegEnabled[RcCount] != -1; RcCount++) {
+ if ((GetRCList (PciSegEnabled[RcCount]))->Type == RCA) {
+ SmmuPmuAgentCount += RCA_NUM_TBU_PMU;
+ } else {
+ SmmuPmuAgentCount += RCB_NUM_TBU_PMU;
+ }
+ SmmuPmuAgentCount += 1; /* Only 1 TCU */
+ }
+
+ BoardPcieCheckSmmuPmuEnabled (&IsSmmuPmuEnabled);
+ if (!IsSmmuPmuEnabled) {
+ SmmuPmuAgentCount = 0;
+ }
+
+ TotalCount = sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE) +
+ RcCount * (sizeof (AC01_ITS_NODE) + sizeof (AC01_RC_NODE) + sizeof (AC01_SMMU_NODE)) +
+ SmmuPmuAgentCount * sizeof (EFI_ACPI_6_0_IO_REMAPPING_PMCG_NODE);
+
+ IortPtr = AllocateZeroPool (TotalCount);
+ if (IortPtr == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ ConstructIort (IortPtr, RcCount, SmmuPmuAgentCount, TotalCount, PciSegEnabled);
+
+ Status = AcpiSdtProtocol->InstallAcpiTable (
+ AcpiSdtProtocol,
+ IortPtr,
+ TotalCount,
+ &TableKey
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG_PCIE_ERR ("IORT: Unable to install IORT table entry\n");
+ }
+ FreePool (IortPtr);
+ return Status;
+}
--
2.17.1

Good point, Min.

If https://github.com/AMDESE/ovmf/blob/snp-v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm is the proposal, then I have more comment:

Type: OVMF_SECTION_TYPE_CODE, OVMF_SECTION_TYPE_VARS are NOT used for SEV. I am not sure why they are there.

Type: OVMF_SECTION_TYPE_CPUID should be SEV specific. TDX does not need CPUID page.

Type: OVMF_SECTION_TYPE_SEC_MEM also seems for SEV. TDX does not need this special memory, such as Page table. It is already covered by code.

Type: OVMF_SECTION_TYPE_SNP_SECRETS / OVMF_SECTION_TYPE_SNP_SEC_MEM is SEV specific.

The SEV table is totally different with TDX metadata table. I really cannot see the benefit to merge into one table.

Thank you
Yao Jiewen

Please see my comments below inline.

Thanks,
Chasel

I suggest SEV and TDX keep their own metadata in separate files. This is because SEV and TDX has different item structure.

From the OvmfMetadata definition in SEV (https://github.com/AMDESE/ovmf/blob/snp-v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm) there are 3 fields in the item. (Base/Size/Type).

But for TDX, there are 6 fields (DataOffset/RawDataSize/MemoryAddress/MemorySize/Type/Attribute) in one item.
That is because TDX-QEMU not only initialize the memory region, but also does more tasks (measurement) if the Attribute indicates.
DataOffset/RawDataSize is used by the TDX-QEMU to do the measurement if the Attribute field is MR.EXTEND.
MemoryAddress/MemorySize indicates the TDX-QEMU how to initialize the memory region.

We can add more fields in the item to make it workable for both SEV and TDX, (for example, add DataOffset/RawDataSize/Attribute), but it also restrict the changes in the future if more fields is needed (TDX's change will impact the existing SEV-QEMU).

The metadata table definition for TDX is at https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf, Chapter 11.2 TDVF descriptor. And we will add more entry there.
May I get a proposed SEV or OVMF meta-table definition somewhere?
Before we get a clear definition for SEV meta-table, I feel it is too early to say one-table.

Another thing I would like to claim *TDVF metadata design principle*: We treat metadata is the *interface* between TDVF and VMM.
The metadata table should contain and only contain the information that is consumed by VMM.
If it is something VMM does not need to know, then it should NOT be in metadata table.



For https://github.com/AMDESE/ovmf/blob/snp-v8/OvmfPkg/ResetVector/X64/OvmfMetadata.asm, I am not sure why we put below structure there.
ExtractHandlerTable:
DD GUID_EXTRACT_HANDLER_TABLE_BASE
DD GUID_EXTRACT_HANDLER_TABLE_SIZE
DD OVMF_SECTION_TYPE_SEC_MEM

Does VMM need to know the Extraction Handler Table???

Thank you
Yao Jiewen

Like Gerd I would prefer to have one metadata table in the reset GUID.
The metadata table will contain multiple entries; lot of entries are
common between SNP and TDX. Some entries will have specific meaning for
the platform. Those special entries should be marked using the
OVMF_SECTION_TYPE_{TDX,SNP}_XXXX. It is perfectly fine to have a more
than one entry for the same region with different type, e.g

GhcbBookkeepingSnp:

  GHCB_BOOKKEPING_BASE_ADDRESS

  GHCB_BOOKKEEPING_SIZE

  OVMF_SECTION_TYPE_SNP_MEM

TdxMailBoxExt:

  GHCB_BOOKKEPING_BASE_ADDRESS

  GHCB_BOOKKEEPING_SIZE

  OVMF_SECTION_TYPE_TDX_MAILBOX

If we want all the OVMF_SECTION_TYPE_SNP_xxx should be defined in a
separate file then that is also doable. I put everything in one place
because I was trying to keep entry order similar to what is present in
MEMFD.

thanks

On 9/23/21 4:30 AM, Leif Lindholm wrote:

Not just an improvement, but an actual bugfix.

I would propose a subject line change though:
Qemu: SbsaQemu: Set the DSDT revision value to 2 to use 64-bit math
->
Platform/Qemu: fix SbsaQemu DSDT format version

If you're OK with the change, I can fold that in.
Reviewed-by: Leif Lindholm <leif@...>

Yes, please do.


--
Rebecca Cran

On Wed, Sep 15, 2021 at 22:55:08 +0700, Nhi Pham wrote:

From: Vu Nguyen <vunguyen@...>

This supports storing the UEFI non-volatile varibles on flash through
the MM communication interface instead of emulating. Below are modules
added for this support:
* FlashLib provides APIs to access flash through MM service.
* FlashPei driver helps to restore the saved variables from
flash on each boot.
* FlashFvbDxe driver provides the implementation for the
gEfiFirmwareVolumeBlock protocol

Cc: Thang Nguyen <thang@...>
Cc: Chuong Tran <chuong@...>
Cc: Phong Vo <phong@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>

Signed-off-by: Vu Nguyen <vunguyen@...>
---
Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec | 3 +
Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec | 8 +
Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc | 10 +-
Platform/Ampere/JadePkg/Jade.dsc | 5 +
Platform/Ampere/JadePkg/Jade.fdf | 62 ++-
Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf | 54 ++
Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf | 52 ++
Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf | 36 ++
Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h | 42 ++
Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c | 525 ++++++++++++++++++++
Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.c | 273 ++++++++++
Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.c | 358 +++++++++++++
12 files changed, 1424 insertions(+), 4 deletions(-)

diff --git a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
index be827dd19a96..d5b12a81e9bf 100644
--- a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+++ b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
@@ -37,6 +37,9 @@ [LibraryClasses]
## @libraryclass Defines a set of methods to communicate with secure parition over MM interface.
MmCommunicationLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/MmCommunicationLib.h

+ ## @libraryclass Defines a set of methods to access flash memory.
+ FlashLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h
+
## @libraryclass Defines a set of methods to generate random numbers by using Hardware RNG.
TrngLib|Silicon/Ampere/AmpereAltraPkg/Include/Library/TrngLib.h

diff --git a/Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec b/Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
index 6ebdf7db0a57..646a53fc031c 100644
--- a/Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
+++ b/Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
@@ -31,6 +31,11 @@ [Guids]
[Ppis]

[PcdsFixedAtBuild]
+ #
+ # NVRAM
+ #
+ gAmpereTokenSpaceGuid.PcdNvramUuid|"C416535D-970B-41B9-859A-3CAF0FAF198C"|VOID*|0x00000010

This still makes no semantic sense. The UUID does not identify the
NVRAM, it identifies "something somehow related to the NVRAM".
Please eename the Pcd something that describes *what* it identifies.

+
#
# SMpro PMpro Pcds
#
@@ -44,3 +49,6 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx]
# Firmware Volume Pcds
#
gAmpereTokenSpaceGuid.PcdFvBlockSize|0|UINT32|0xB0000001
+
+ # NVRam Pcds
+ gAmpereTokenSpaceGuid.PcdNvramErased|FALSE|BOOLEAN|0xB0000009
diff --git a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
index bb144183164d..7f84ac2df50a 100644
--- a/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
+++ b/Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dsc.inc
@@ -226,6 +226,7 @@ [LibraryClasses.common.DXE_DRIVER]
SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
+ FlashLib|Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf

[LibraryClasses.common.UEFI_APPLICATION]
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiTianoCustomDecompressLib.inf
@@ -256,6 +257,7 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]

EfiResetSystemLib|ArmPkg/Library/ArmPsciResetSystemLib/ArmPsciResetSystemLib.inf
ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+ FlashLib|Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf
AmpereCpuLib|Silicon/Ampere/AmpereAltraPkg/Library/AmpereCpuLib/RuntimeAmpereCpuLib.inf

[LibraryClasses.ARM,LibraryClasses.AARCH64]
@@ -508,6 +510,8 @@ [PcdsDynamicDefault.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|0x0
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|0x0

+ gAmpereTokenSpaceGuid.PcdNvramErased|FALSE
+
################################################################################
#
# Component Section - list of all EDK II Component Entries defined by this Platform
@@ -528,8 +532,10 @@ [Components.common]
Silicon/Ampere/AmpereAltraPkg/Drivers/ATFHobPei/ATFHobPeim.inf
Silicon/Ampere/AmpereAltraPkg/Drivers/MemoryInitPeim/MemoryInitPeim.inf
Silicon/Ampere/AmpereAltraPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
+ Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf
ArmPkg/Drivers/CpuPei/CpuPei.inf
UefiCpuPkg/CpuIoPei/CpuIoPei.inf
+ MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
<LibraryClasses>
@@ -583,9 +589,9 @@ [Components.common]
#
# Environment Variables Protocol
#
+ Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
+ MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
- <PcdsFixedAtBuild>
- gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
<LibraryClasses>
BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
diff --git a/Platform/Ampere/JadePkg/Jade.dsc b/Platform/Ampere/JadePkg/Jade.dsc
index 332cd8ac0c8e..c670fd0f34b6 100644
--- a/Platform/Ampere/JadePkg/Jade.dsc
+++ b/Platform/Ampere/JadePkg/Jade.dsc
@@ -52,6 +52,7 @@ [Defines]
DEFINE EDK2_SKIP_PEICORE = TRUE
DEFINE SECURE_BOOT_ENABLE = FALSE
DEFINE INCLUDE_TFTP_COMMAND = TRUE
+ DEFINE NVRAM_UUID = 84BC921F-9D4A-4D1D-A1A1-1AE13EDD07E5

And rename this define similarly.

#
# Network definition
@@ -89,6 +90,10 @@ [LibraryClasses]
################################################################################
[PcdsFeatureFlag.common]
[PcdsFixedAtBuild.common]
+ #
+ # NVRAM
+ #
+ gAmpereTokenSpaceGuid.PcdNvramUuid|"$(NVRAM_UUID)"

!if $(SECURE_BOOT_ENABLE) == TRUE
# Override the default values from SecurityPkg to ensure images
diff --git a/Platform/Ampere/JadePkg/Jade.fdf b/Platform/Ampere/JadePkg/Jade.fdf
index 1857296a8ea5..375455086d0b 100644
--- a/Platform/Ampere/JadePkg/Jade.fdf
+++ b/Platform/Ampere/JadePkg/Jade.fdf
@@ -26,7 +26,7 @@ [FD.BL33_JADE_UEFI]
ErasePolarity = 1

# This one is tricky, it must be: BlockSize * NumBlocks = Size
-BlockSize = 0x10000
+BlockSize = 0x10000|gAmpereTokenSpaceGuid.PcdFvBlockSize
NumBlocks = 0x7C

################################################################################
@@ -56,8 +56,61 @@ [FD.BL33_JADE_UEFI]

#
# NV Variables
-# T.B.D

As requested for v2, please reword "T.B.D" as "Placeholder" in the
patch where it is introduced.

+# Offset: 0x00740000
+# Size: 0x00080000
#
+0x00740000|0x00030000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+DATA = {
+ ## This is the EFI_FIRMWARE_VOLUME_HEADER
+ # ZeroVector []
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ # FileSystemGuid: gEfiSystemNvDataFvGuid =
+ # { 0xFFF12B8D, 0x7696, 0x4C8B,
+ # { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
+ 0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
+ 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
+ # FvLength: 0x80000
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00,
+ # Signature "_FVH" # Attributes
+ 0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
+ # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
+ 0x48, 0x00, 0x2D, 0x09, 0x00, 0x00, 0x00, 0x02,
+ # Blockmap[0]: 0x2 Blocks * 0x40000 Bytes / Block
+ 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
+ # Blockmap[1]: End
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ ## This is the VARIABLE_STORE_HEADER
+ # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
+ # Signature: gEfiAuthenticatedVariableGuid =
+ # { 0xaaf32c78, 0x947b, 0x439a,
+ # { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
+ 0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
+ 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
+ # Size: 0x30000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
+ # 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x2FFB8
+ # This can speed up the Variable Dispatch a bit.
+ 0xB8, 0xFF, 0x02, 0x00,
+ # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
+ 0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x00770000|0x00010000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+DATA = {
+ # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid =
+ # { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
+ 0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
+ 0xa0, 0xce, 0x65, 0x0, 0xfd, 0x9f, 0x1b, 0x95,
+ # Crc:UINT32 #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
+ 0x2c, 0xaf, 0x2c, 0x64, 0xFE, 0xFF, 0xFF, 0xFF,
+ # WriteQueueSize: UINT64 Size: 0x10000 - 0x20 (FTW_WORKING_HEADER) = 0xFFE0
+ 0xE0, 0xFF, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x00780000|0x00040000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize

################################################################################
#
@@ -102,9 +155,11 @@ [FV.FVMAIN_COMPACT]
INF Silicon/Ampere/AmpereAltraPkg/Drivers/ATFHobPei/ATFHobPeim.inf
INF Silicon/Ampere/AmpereAltraPkg/Drivers/MemoryInitPeim/MemoryInitPeim.inf
INF Silicon/Ampere/AmpereAltraPkg/Drivers/MmCommunicationPei/MmCommunicationPei.inf
+ INF Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf
INF Silicon/Ampere/AmpereAltraPkg/Drivers/BootProgress/BootProgressPeim/BootProgressPeim.inf
INF ArmPkg/Drivers/CpuPei/CpuPei.inf
INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+ INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
INF MdeModulePkg/Universal/ReportStatusCodeRouter/Pei/ReportStatusCodeRouterPei.inf
INF MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf
@@ -144,6 +199,7 @@ [FV.FvMain]
INF MdeModulePkg/Universal/ReportStatusCodeRouter/RuntimeDxe/ReportStatusCodeRouterRuntimeDxe.inf
INF MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf
INF Silicon/Ampere/AmpereAltraPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+ INF Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
}

INF MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -173,6 +229,8 @@ [FV.FvMain]
# Environment Variables Protocol
#
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+ INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+ INF Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf

#
# Multiple Console IO support
diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
new file mode 100644
index 000000000000..782278615094
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.inf
@@ -0,0 +1,54 @@
+## @file
+#
+# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = FlashFvbDxe
+ FILE_GUID = 9E6EA240-DF80-11EA-8B6E-0800200C9A66
+ MODULE_TYPE = DXE_RUNTIME_DRIVER
+ VERSION_STRING = 0.1
+ ENTRY_POINT = FlashFvbDxeInitialize
+
+[Sources]
+ FlashFvbDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+ Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
+
+[LibraryClasses]
+ DebugLib
+ FlashLib
+ PcdLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ UefiRuntimeLib
+
+[FixedPcd]
+ gAmpereTokenSpaceGuid.PcdFvBlockSize
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
+
+[Guids]
+ gEfiEventVirtualAddressChangeGuid
+ gSpiNorMmGuid
+
+[Protocols]
+ gEfiFirmwareVolumeBlockProtocolGuid ## PRODUCES
+ gEfiMmCommunicationProtocolGuid ## CONSUMES
+
+[Depex]
+ gEfiMmCommunicationProtocolGuid
diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf
new file mode 100644
index 000000000000..9b640adb6421
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.inf
@@ -0,0 +1,52 @@
+## @file
+#
+# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = FlashPei
+ FILE_GUID = 967CFBD0-DF81-11EA-8B6E-0800200C9A66
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ ENTRY_POINT = FlashPeiEntryPoint
+
+[Sources]
+ FlashPei.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+ Silicon/Ampere/AmpereSiliconPkg/AmpereSiliconPkg.dec
+
+[LibraryClasses]
+ ArmSmcLib
+ BaseMemoryLib
+ DebugLib
+ MmCommunicationLib
+ PcdLib
+ PeimEntryPoint
+
+[Guids]
+ gSpiNorMmGuid
+
+[FixedPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
+
+ gAmpereTokenSpaceGuid.PcdNvramErased
+ gAmpereTokenSpaceGuid.PcdNvramUuid
+
+[Depex]
+ TRUE
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf b/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf
new file mode 100644
index 000000000000..2d5003d1af17
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.inf
@@ -0,0 +1,36 @@
+## @file
+#
+# Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = FlashLib
+ FILE_GUID = 9E9D093D-6484-45AE-BA49-0745AA0BB481
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 0.1
+ LIBRARY_CLASS = FlashLib
+ CONSTRUCTOR = FlashLibConstructor
+
+[Sources.common]
+ FlashLib.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdePkg/MdePkg.dec
+ Silicon/Ampere/AmpereAltraPkg/AmpereAltraPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+
+[Guids]
+ gSpiNorMmGuid
+
+[Protocols]
+ gEfiMmCommunicationProtocolGuid
diff --git a/Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h b/Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h
new file mode 100644
index 000000000000..9207dee643a5
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Include/Library/FlashLib.h
@@ -0,0 +1,42 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef FLASH_LIB_H_
+#define FLASH_LIB_H_
+
+EFI_STATUS
+EFIAPI
+FlashGetNvRamInfo (
+ OUT UINT64 *NvRamBase,
+ OUT UINT32 *NvRamSize
+ );
+
+EFI_STATUS
+EFIAPI
+FlashEraseCommand (
+ IN UINT8 *pBlockAddress,
+ IN UINT32 Length
+ );
+
+EFI_STATUS
+EFIAPI
+FlashProgramCommand (
+ IN UINT8 *pByteAddress,
+ IN UINT8 *Byte,
+ IN OUT UINTN *Length
+ );
+
+EFI_STATUS
+EFIAPI
+FlashReadCommand (
+ IN UINT8 *pByteAddress,
+ OUT UINT8 *Byte,
+ IN OUT UINTN *Length
+ );
+
+#endif /* FLASH_LIB_H_ */
diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
new file mode 100644
index 000000000000..dcd92151b7d2
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashFvbDxe/FlashFvbDxe.c
@@ -0,0 +1,525 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/FlashLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeLib.h>
+#include <Protocol/FirmwareVolumeBlock.h>
+
+//
+// These temporary buffers are used to calculate and convert linear virtual
+// to physical address
+//
+STATIC UINT64 mNvFlashBase;
+STATIC UINT32 mNvFlashSize;
+STATIC UINT32 mFlashBlockSize;
+STATIC UINT64 mNvStorageBase;
+STATIC UINT64 mNvStorageSize;
+
+/**
+ Fixup internal data so that EFI can be call in virtual mode.
+ Call the passed in Child Notify event and convert any pointers in
+ lib to virtual mode.
+
+ @param[in] Event The Event that is being processed
+ @param[in] Context Event Context
+**/
+VOID
+EFIAPI
+FlashFvbAddressChangeEvent (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EfiConvertPointer (0x0, (VOID **)&mNvStorageBase);
+}
+
+/**
+ The GetAttributes() function retrieves the attributes and
+ current settings of the block.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Attributes Pointer to EFI_FVB_ATTRIBUTES_2 in which the
+ attributes and current settings are
+ returned. Type EFI_FVB_ATTRIBUTES_2 is defined
+ in EFI_FIRMWARE_VOLUME_HEADER.
+
+ @retval EFI_SUCCESS The firmware volume attributes were
+ returned.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeGetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ )
+{
+ ASSERT (Attributes != NULL);
+
+ *Attributes = EFI_FVB2_READ_ENABLED_CAP | // Reads may be enabled
+ EFI_FVB2_READ_STATUS | // Reads are currently enabled
+ EFI_FVB2_WRITE_STATUS | // Writes are currently enabled
+ EFI_FVB2_WRITE_ENABLED_CAP | // Writes may be enabled
+ EFI_FVB2_STICKY_WRITE | // A block erase is required to flip bits into EFI_FVB2_ERASE_POLARITY
+ EFI_FVB2_MEMORY_MAPPED | // It is memory mapped
+ EFI_FVB2_ALIGNMENT |
+ EFI_FVB2_ERASE_POLARITY; // After erasure all bits take this value (i.e. '1')
+
+ return EFI_SUCCESS;
+}
+
+/**
+ The SetAttributes() function sets configurable firmware volume
+ attributes and returns the new settings of the firmware volume.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Attributes On input, Attributes is a pointer to
+ EFI_FVB_ATTRIBUTES_2 that contains the
+ desired firmware volume settings. On
+ successful return, it contains the new
+ settings of the firmware volume. Type
+ EFI_FVB_ATTRIBUTES_2 is defined in
+ EFI_FIRMWARE_VOLUME_HEADER.
+
+ @retval EFI_SUCCESS The firmware volume attributes were returned.
+
+ @retval EFI_INVALID_PARAMETER The attributes requested are in
+ conflict with the capabilities
+ as declared in the firmware
+ volume header.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeSetAttributes (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ IN OUT EFI_FVB_ATTRIBUTES_2 *Attributes
+ )
+{
+ return EFI_SUCCESS; // ignore for now
+}
+
+/**
+ The GetPhysicalAddress() function retrieves the base address of
+ a memory-mapped firmware volume. This function should be called
+ only for memory-mapped firmware volumes.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Address Pointer to a caller-allocated
+ EFI_PHYSICAL_ADDRESS that, on successful
+ return from GetPhysicalAddress(), contains the
+ base address of the firmware volume.
+
+ @retval EFI_SUCCESS The firmware volume base address was returned.
+
+ @retval EFI_UNSUPPORTED The firmware volume is not memory mapped.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeGetPhysicalAddress (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ OUT EFI_PHYSICAL_ADDRESS *Address
+ )
+{
+ ASSERT (Address != NULL);
+
+ *Address = (EFI_PHYSICAL_ADDRESS)mNvStorageBase;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ The GetBlockSize() function retrieves the size of the requested
+ block. It also returns the number of additional blocks with
+ the identical size. The GetBlockSize() function is used to
+ retrieve the block map (see EFI_FIRMWARE_VOLUME_HEADER).
+
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Lba Indicates the block for which to return the size.
+
+ @param BlockSize Pointer to a caller-allocated UINTN in which
+ the size of the block is returned.
+
+ @param NumberOfBlocks Pointer to a caller-allocated UINTN in
+ which the number of consecutive blocks,
+ starting with Lba, is returned. All
+ blocks in this range have a size of
+ BlockSize.
+
+
+ @retval EFI_SUCCESS The firmware volume base address was returned.
+
+ @retval EFI_INVALID_PARAMETER The requested LBA is out of range.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeGetBlockSize (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ OUT UINTN *BlockSize,
+ OUT UINTN *NumberOfBlocks
+ )
+{
+ UINTN TotalNvStorageBlocks;
+
+ ASSERT (BlockSize != NULL);
+ ASSERT (NumberOfBlocks != NULL);
+
+ TotalNvStorageBlocks = mNvStorageSize / mFlashBlockSize;
+
+ if (TotalNvStorageBlocks <= (UINTN)Lba) {
+ DEBUG ((DEBUG_ERROR, "The requested LBA is out of range\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *NumberOfBlocks = TotalNvStorageBlocks - (UINTN)Lba;
+ *BlockSize = mFlashBlockSize;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Reads the specified number of bytes into a buffer from the specified block.
+
+ The Read() function reads the requested number of bytes from the
+ requested block and stores them in the provided buffer.
+ Implementations should be mindful that the firmware volume
+ might be in the ReadDisabled state. If it is in this state,
+ the Read() function must return the status code
+ EFI_ACCESS_DENIED without modifying the contents of the
+ buffer. The Read() function must also prevent spanning block
+ boundaries. If a read is requested that would span a block
+ boundary, the read must read up to the boundary but not
+ beyond. The output parameter NumBytes must be set to correctly
+ indicate the number of bytes actually read. The caller must be
+ aware that a read may be partially completed.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Lba The starting logical block index
+ from which to read.
+
+ @param Offset Offset into the block at which to begin reading.
+
+ @param NumBytes Pointer to a UINTN. At entry, *NumBytes
+ contains the total size of the buffer. At
+ exit, *NumBytes contains the total number of
+ bytes read.
+
+ @param Buffer Pointer to a caller-allocated buffer that will
+ be used to hold the data that is read.
+
+ @retval EFI_SUCCESS The firmware volume was read successfully,
+ and contents are in Buffer.
+
+ @retval EFI_BAD_BUFFER_SIZE Read attempted across an LBA
+ boundary. On output, NumBytes
+ contains the total number of bytes
+ returned in Buffer.
+
+ @retval EFI_ACCESS_DENIED The firmware volume is in the
+ ReadDisabled state.
+
+ @retval EFI_DEVICE_ERROR The block device is not
+ functioning correctly and could
+ not be read.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeRead (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ IN OUT UINT8 *Buffer
+ )
+{
+ EFI_STATUS Status;
+
+ ASSERT (NumBytes != NULL);
+ ASSERT (Buffer != NULL);
+
+ if (Offset + *NumBytes > mFlashBlockSize) {
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ Status = FlashReadCommand (
+ (UINT8 *)(mNvFlashBase + Lba * mFlashBlockSize + Offset),
+ Buffer,
+ NumBytes
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to do flash read\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Writes the specified number of bytes from the input buffer to the block.
+
+ The Write() function writes the specified number of bytes from
+ the provided buffer to the specified block and offset. If the
+ firmware volume is sticky write, the caller must ensure that
+ all the bits of the specified range to write are in the
+ EFI_FVB_ERASE_POLARITY state before calling the Write()
+ function, or else the result will be unpredictable. This
+ unpredictability arises because, for a sticky-write firmware
+ volume, a write may negate a bit in the EFI_FVB_ERASE_POLARITY
+ state but cannot flip it back again. Before calling the
+ Write() function, it is recommended for the caller to first call
+ the EraseBlocks() function to erase the specified block to
+ write. A block erase cycle will transition bits from the
+ (NOT)EFI_FVB_ERASE_POLARITY state back to the
+ EFI_FVB_ERASE_POLARITY state. Implementations should be
+ mindful that the firmware volume might be in the WriteDisabled
+ state. If it is in this state, the Write() function must
+ return the status code EFI_ACCESS_DENIED without modifying the
+ contents of the firmware volume. The Write() function must
+ also prevent spanning block boundaries. If a write is
+ requested that spans a block boundary, the write must store up
+ to the boundary but not beyond. The output parameter NumBytes
+ must be set to correctly indicate the number of bytes actually
+ written. The caller must be aware that a write may be
+ partially completed. All writes, partial or otherwise, must be
+ fully flushed to the hardware before the Write() service
+ returns.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL instance.
+
+ @param Lba The starting logical block index to write to.
+
+ @param Offset Offset into the block at which to begin writing.
+
+ @param NumBytes The pointer to a UINTN. At entry, *NumBytes
+ contains the total size of the buffer. At
+ exit, *NumBytes contains the total number of
+ bytes actually written.
+
+ @param Buffer The pointer to a caller-allocated buffer that
+ contains the source for the write.
+
+ @retval EFI_SUCCESS The firmware volume was written successfully.
+
+ @retval EFI_BAD_BUFFER_SIZE The write was attempted across an
+ LBA boundary. On output, NumBytes
+ contains the total number of bytes
+ actually written.
+
+ @retval EFI_ACCESS_DENIED The firmware volume is in the
+ WriteDisabled state.
+
+ @retval EFI_DEVICE_ERROR The block device is malfunctioning
+ and could not be written.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeWrite (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ IN EFI_LBA Lba,
+ IN UINTN Offset,
+ IN OUT UINTN *NumBytes,
+ IN UINT8 *Buffer
+ )
+{
+ EFI_STATUS Status;
+
+ ASSERT (NumBytes != NULL);
+ ASSERT (Buffer != NULL);
+
+ if (Offset + *NumBytes > mFlashBlockSize) {
+ return EFI_BAD_BUFFER_SIZE;
+ }
+
+ Status = FlashProgramCommand (
+ (UINT8 *)(mNvFlashBase + Lba * mFlashBlockSize + Offset),
+ Buffer,
+ NumBytes
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to do flash program\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return Status;
+}
+
+/**
+ Erases and initializes a firmware volume block.
+
+ The EraseBlocks() function erases one or more blocks as denoted
+ by the variable argument list. The entire parameter list of
+ blocks must be verified before erasing any blocks. If a block is
+ requested that does not exist within the associated firmware
+ volume (it has a larger index than the last block of the
+ firmware volume), the EraseBlocks() function must return the
+ status code EFI_INVALID_PARAMETER without modifying the contents
+ of the firmware volume. Implementations should be mindful that
+ the firmware volume might be in the WriteDisabled state. If it
+ is in this state, the EraseBlocks() function must return the
+ status code EFI_ACCESS_DENIED without modifying the contents of
+ the firmware volume. All calls to EraseBlocks() must be fully
+ flushed to the hardware before the EraseBlocks() service
+ returns.
+
+ @param This Indicates the EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL
+ instance.
+
+ @param ... The variable argument list is a list of tuples.
+ Each tuple describes a range of LBAs to erase
+ and consists of the following:
+ - An EFI_LBA that indicates the starting LBA
+ - A UINTN that indicates the number of blocks to
+ erase.
+
+ The list is terminated with an
+ EFI_LBA_LIST_TERMINATOR. For example, the
+ following indicates that two ranges of blocks
+ (5-7 and 10-11) are to be erased: EraseBlocks
+ (This, 5, 3, 10, 2, EFI_LBA_LIST_TERMINATOR);
+
+ @retval EFI_SUCCESS The erase request successfully
+ completed.
+
+ @retval EFI_ACCESS_DENIED The firmware volume is in the
+ WriteDisabled state.
+ @retval EFI_DEVICE_ERROR The block device is not functioning
+ correctly and could not be written.
+ The firmware device may have been
+ partially erased.
+ @retval EFI_INVALID_PARAMETER One or more of the LBAs listed
+ in the variable argument list do
+ not exist in the firmware volume.
+
+**/
+EFI_STATUS
+EFIAPI
+FlashFvbDxeErase (
+ IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+ ...
+ )
+{
+ VA_LIST Args;
+ EFI_LBA Start;
+ UINTN Length;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+
+ VA_START (Args, This);
+
+ for (Start = VA_ARG (Args, EFI_LBA);
+ Start != EFI_LBA_LIST_TERMINATOR;
+ Start = VA_ARG (Args, EFI_LBA))
+ {
+ Length = VA_ARG (Args, UINTN);
+ Status = FlashEraseCommand (
+ (UINT8 *)(mNvFlashBase + Start * mFlashBlockSize),
+ Length * mFlashBlockSize
+ );
+ }
+
+ VA_END (Args);
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to do flash erase\n"));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFlashFvbProtocol = {
+ FlashFvbDxeGetAttributes,
+ FlashFvbDxeSetAttributes,
+ FlashFvbDxeGetPhysicalAddress,
+ FlashFvbDxeGetBlockSize,
+ FlashFvbDxeRead,
+ FlashFvbDxeWrite,
+ FlashFvbDxeErase
+};
+
+EFI_STATUS
+EFIAPI
+FlashFvbDxeInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE FvbHandle = NULL;
+ EFI_EVENT VirtualAddressChangeEvent;
+
+ // Get NV store FV info
+ mFlashBlockSize = FixedPcdGet32 (PcdFvBlockSize);
+ mNvStorageBase = PcdGet64 (PcdFlashNvStorageVariableBase64);
+ mNvStorageSize = FixedPcdGet32 (PcdFlashNvStorageVariableSize) +
+ FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) +
+ FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize);
+
+ DEBUG ((
+ DEBUG_INFO,
+ "%a: Using NV store FV in-memory copy at 0x%lx with size 0x%x\n",
+ __FUNCTION__,
+ mNvStorageBase,
+ mNvStorageSize
+ ));
+
+ // Get NV Flash information
+ Status = FlashGetNvRamInfo ((UINT64 *)&mNvFlashBase, (UINT32 *)&mNvFlashSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to get Flash info\n", __FUNCTION__));
+ return EFI_DEVICE_ERROR;
+ }
+
+ if (mNvFlashSize >= (mNvStorageSize * 2)) {
+ DEBUG ((DEBUG_INFO, "%a: NV store on Flash is valid\n", __FUNCTION__));
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: NV store on Flash is invalid\n", __FUNCTION__));
+ return EFI_DEVICE_ERROR;
+ }
+
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ FlashFvbAddressChangeEvent,
+ NULL,
+ &gEfiEventVirtualAddressChangeGuid,
+ &VirtualAddressChangeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &FvbHandle,
+ &gEfiFirmwareVolumeBlockProtocolGuid,
+ &mFlashFvbProtocol,
+ NULL
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to install Firmware Volume Block protocol\n"));
+ return Status;
+ }
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.c b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.c
new file mode 100644
index 000000000000..0939f1b70869
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Drivers/FlashPei/FlashPei.c
@@ -0,0 +1,273 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+#include <Uefi.h>
+
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MmCommunicationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeimEntryPoint.h>
+#include <MmLib.h>
+
+EFI_MM_COMM_REQUEST mEfiMmSpiNorReq;
+
+STATIC
+EFI_STATUS
+UefiMmCreateSpiNorReq (
+ VOID *Data,
+ UINT64 Size
+ )
+{
+ CopyGuid (&mEfiMmSpiNorReq.EfiMmHdr.HeaderGuid, &gSpiNorMmGuid);
+ mEfiMmSpiNorReq.EfiMmHdr.MsgLength = Size;
+
+ if (Size != 0) {
+ ASSERT (Data);
+ ASSERT (Size <= EFI_MM_MAX_PAYLOAD_SIZE);
+
+ CopyMem (mEfiMmSpiNorReq.PayLoad.Data, Data, Size);
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Entry point function for the PEIM
+
+ @param FileHandle Handle of the file being invoked.
+ @param PeiServices Describes the list of possible PEI Services.
+
+ @return EFI_SUCCESS If we installed our PPI
+
+**/
+EFI_STATUS
+EFIAPI
+FlashPeiEntryPoint (

This function is very long, and would really benefit from breaking up
into some smaller helper functions. Those could then be given helpful
names which would help clarify what's going on.

+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ CHAR8 BuildUuid[PcdGetSize (PcdNvramUuid)];
+ CHAR8 StoredUuid[PcdGetSize (PcdNvramUuid)];
+ EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *MmSpiNorNVInfoRes;
+ EFI_MM_COMMUNICATE_SPINOR_RES *MmSpiNorRes;
+ EFI_STATUS Status;
+ UINT64 FWNvRamStartOffset;
+ UINT64 MmData[5];
+ UINTN NvRamSize;
+ UINTN Size;
+ VOID *NvRamAddress;
+
+#if defined(RAM_BLOCKIO_START_ADDRESS) && defined(RAM_BLOCKIO_SIZE)
+ EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *MmSpiNorNV2InfoRes;
+ UINT64 NV2Base, NV2Size;
+#endif
+
+ NvRamAddress = (VOID *)PcdGet64 (PcdFlashNvStorageVariableBase64);
+ NvRamSize = FixedPcdGet32 (PcdFlashNvStorageVariableSize) +
+ FixedPcdGet32 (PcdFlashNvStorageFtwWorkingSize) +
+ FixedPcdGet32 (PcdFlashNvStorageFtwSpareSize);
+
+ /* Find out about the start offset of NVRAM to be passed to SMC */
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_GET_NVRAM_INFO;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorNVInfoRes = (EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorNVInfoRes->Status != MM_SPINOR_RES_SUCCESS) {
+ /* Old FW so just exit */
+ return EFI_SUCCESS;
+ }
+ FWNvRamStartOffset = MmSpiNorNVInfoRes->NVBase;
+
+ CopyMem ((VOID *)BuildUuid, PcdGetPtr (PcdNvramUuid), sizeof (BuildUuid));
+ if (MmSpiNorNVInfoRes->NVSize < (NvRamSize * 2 + sizeof (BuildUuid))) {
+ /* NVRAM size provided by FW is not enough */
+ return EFI_INVALID_PARAMETER;
+ }
+
+ /* We stored BIOS UUID build at the offset NVRAM_SIZE * 2 */
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_READ;
+ MmData[1] = (UINT64)(FWNvRamStartOffset + NvRamSize * 2);
+ MmData[2] = (UINT64)sizeof (StoredUuid);
+ MmData[3] = (UINT64)StoredUuid;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+
+ if (CompareMem ((VOID *)StoredUuid, (VOID *)BuildUuid, sizeof (BuildUuid))) {

Definitely helper function for this scope.

+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_ERASE;
+ MmData[1] = (UINT64)FWNvRamStartOffset;
+ MmData[2] = (UINT64)(NvRamSize * 2);
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_WRITE;
+ MmData[1] = (UINT64)FWNvRamStartOffset;
+ MmData[2] = (UINT64)(NvRamSize * 2);
+ MmData[3] = (UINT64)NvRamAddress;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+
+ /* Update UUID */

Which UUID? Describe the content, not the storage format.
But this also seems like a clear logic block to move to a helper, at
which point the function name will be as useful as a comment.

+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_ERASE;
+ MmData[1] = (UINT64)(FWNvRamStartOffset + NvRamSize * 2);
+ MmData[2] = (UINT64)sizeof (BuildUuid);
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_WRITE;
+ MmData[1] = (UINT64)(FWNvRamStartOffset + NvRamSize * 2);
+ MmData[2] = (UINT64)sizeof (BuildUuid);
+ MmData[3] = (UINT64)BuildUuid;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+ DEBUG ((DEBUG_INFO, "UUID Changed, Update Storage with FV NVRAM\n"));
+
+ /* Indicate that NVRAM was cleared */
+ PcdSetBoolS (PcdNvramErased, TRUE);
+ } else {

Another helper function for the other path.

+ /* Copy the stored NVRAM to RAM */
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_READ;
+ MmData[1] = (UINT64)FWNvRamStartOffset;
+ MmData[2] = (UINT64)(NvRamSize * 2);
+ MmData[3] = (UINT64)NvRamAddress;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ return Status;
+ }
+ DEBUG ((DEBUG_INFO, "Identical UUID, copy stored NVRAM to RAM\n"));
+ }
+
+#if defined(RAM_BLOCKIO_START_ADDRESS) && defined(RAM_BLOCKIO_SIZE)

Can the whole contents of the #ifdef block be moved into a helper
function, only built if these conditions match?

/
Leif

+ /* Find out about the start offset of NVRAM2 to be passed to SMC */
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_GET_NVRAM2_INFO;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorNV2InfoRes = (EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *)&mEfiMmSpiNorReq.PayLoad;
+ if (MmSpiNorNV2InfoRes->Status == MM_SPINOR_RES_SUCCESS) {
+ NV2Base = MmSpiNorNV2InfoRes->NVBase;
+ NV2Size = MmSpiNorNV2InfoRes->NVSize;
+ /* Make sure the requested size is smaller than allocated */
+ if (RAM_BLOCKIO_SIZE <= NV2Size) {
+ /* Copy the ramdisk image to RAM */
+ ZeroMem ((VOID *)MmData, sizeof (MmData));
+ MmData[0] = MM_SPINOR_FUNC_READ;
+ MmData[1] = (UINT64)NV2Base; /* Start virtual address */
+ MmData[2] = (UINT64)RAM_BLOCKIO_SIZE;
+ MmData[3] = (UINT64)RAM_BLOCKIO_START_ADDRESS;
+ UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = MmCommunicationCommunicate (
+ (VOID *)&mEfiMmSpiNorReq,
+ &Size
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mEfiMmSpiNorReq.PayLoad;
+ ASSERT (MmSpiNorRes->Status == MM_SPINOR_RES_SUCCESS);
+ }
+
+ BuildMemoryAllocationHob (
+ (EFI_PHYSICAL_ADDRESS)RAM_BLOCKIO_START_ADDRESS,
+ EFI_SIZE_TO_PAGES (RAM_BLOCKIO_SIZE) * EFI_PAGE_SIZE,
+ EfiLoaderData
+ );
+ }
+#endif
+
+ return EFI_SUCCESS;
+}
diff --git a/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.c b/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.c
new file mode 100644
index 000000000000..cd77aed3cfe1
--- /dev/null
+++ b/Silicon/Ampere/AmpereAltraPkg/Library/FlashLib/FlashLib.c
@@ -0,0 +1,358 @@
+/** @file
+
+ Copyright (c) 2020 - 2021, Ampere Computing LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/FlashLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <MmLib.h>
+#include <Protocol/MmCommunication.h>
+
+STATIC EFI_MM_COMMUNICATION_PROTOCOL *mMmCommunicationProtocol = NULL;
+STATIC EFI_MM_COMM_REQUEST *mCommBuffer = NULL;
+
+BOOLEAN mIsEfiRuntime;
+UINT8 *mTmpBufVirt;
+UINT8 *mTmpBufPhy;
+
+/**
+ This is a notification function registered on EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE
+ event. It converts a pointer to a new virtual address.
+
+ @param Event Event whose notification function is being invoked.
+ @param Context Pointer to the notification function's context
+
+**/
+VOID
+EFIAPI
+FlashLibAddressChangeEvent (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ gRT->ConvertPointer (0x0, (VOID **)&mTmpBufVirt);
+ gRT->ConvertPointer (0x0, (VOID **)&mCommBuffer);
+ gRT->ConvertPointer (0x0, (VOID **)&mMmCommunicationProtocol);
+
+ mIsEfiRuntime = TRUE;
+}
+
+EFI_STATUS
+EFIAPI
+FlashLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_EVENT VirtualAddressChangeEvent = NULL;
+ EFI_STATUS Status = EFI_SUCCESS;
+
+ mCommBuffer = AllocateRuntimeZeroPool (sizeof (EFI_MM_COMM_REQUEST));
+ ASSERT (mCommBuffer != NULL);
+
+ mTmpBufPhy = AllocateRuntimeZeroPool (EFI_MM_MAX_TMP_BUF_SIZE);
+ mTmpBufVirt = mTmpBufPhy;
+ ASSERT (mTmpBufPhy != NULL);
+
+ Status = gBS->LocateProtocol (
+ &gEfiMmCommunicationProtocolGuid,
+ NULL,
+ (VOID **)&mMmCommunicationProtocol
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->CreateEvent (
+ EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE,
+ TPL_CALLBACK,
+ FlashLibAddressChangeEvent,
+ NULL,
+ &VirtualAddressChangeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+FlashMmCommunicate (
+ IN OUT VOID *CommBuffer,
+ IN OUT UINTN *CommSize
+ )
+{
+ if (mMmCommunicationProtocol == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ return mMmCommunicationProtocol->Communicate (
+ mMmCommunicationProtocol,
+ CommBuffer,
+ CommSize
+ );
+}
+
+STATIC
+EFI_STATUS
+UefiMmCreateSpiNorReq (
+ IN VOID *Data,
+ IN UINT64 Size
+ )
+{
+ if (mCommBuffer == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ CopyGuid (&mCommBuffer->EfiMmHdr.HeaderGuid, &gSpiNorMmGuid);
+ mCommBuffer->EfiMmHdr.MsgLength = Size;
+
+ if (Size != 0) {
+ ASSERT (Data);
+ ASSERT (Size <= EFI_MM_MAX_PAYLOAD_SIZE);
+
+ CopyMem (mCommBuffer->PayLoad.Data, Data, Size);
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Convert Virtual Address to Physical Address at Runtime Services
+
+ @param VirtualPtr Virtual Address Pointer
+ @param Size Total bytes of the buffer
+
+ @retval Ptr Return the pointer of the converted address
+
+**/
+STATIC
+UINT8 *
+ConvertVirtualToPhysical (
+ IN UINT8 *VirtualPtr,
+ IN UINTN Size
+ )
+{
+ if (mIsEfiRuntime) {
+ ASSERT (VirtualPtr != NULL);
+ CopyMem ((VOID *)mTmpBufVirt, (VOID *)VirtualPtr, Size);
+ return (UINT8 *)mTmpBufPhy;
+ }
+
+ return (UINT8 *)VirtualPtr;
+}
+
+/**
+ Convert Physical Address to Virtual Address at Runtime Services
+
+ @param VirtualPtr Physical Address Pointer
+ @param Size Total bytes of the buffer
+
+**/
+STATIC
+VOID
+ConvertPhysicaltoVirtual (
+ IN UINT8 *PhysicalPtr,
+ IN UINTN Size
+ )
+{
+ if (mIsEfiRuntime) {
+ ASSERT (PhysicalPtr != NULL);
+ CopyMem ((VOID *)PhysicalPtr, (VOID *)mTmpBufVirt, Size);
+ }
+}
+
+EFI_STATUS
+EFIAPI
+FlashGetNvRamInfo (
+ OUT UINT64 *NvRamBase,
+ OUT UINT32 *NvRamSize
+ )
+{
+ EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *MmSpiNorNVInfoRes;
+ EFI_STATUS Status;
+ UINT64 MmData[5];
+ UINTN Size;
+
+ MmData[0] = MM_SPINOR_FUNC_GET_NVRAM_INFO;
+
+ Status = UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = FlashMmCommunicate (
+ mCommBuffer,
+ &Size
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ MmSpiNorNVInfoRes = (EFI_MM_COMMUNICATE_SPINOR_NVINFO_RES *)&mCommBuffer->PayLoad;
+ if (MmSpiNorNVInfoRes->Status == MM_SPINOR_RES_SUCCESS) {
+ *NvRamBase = MmSpiNorNVInfoRes->NVBase;
+ *NvRamSize = MmSpiNorNVInfoRes->NVSize;
+ DEBUG ((DEBUG_INFO, "NVInfo Base 0x%llx, Size 0x%lx\n", *NvRamBase, *NvRamSize));
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FlashEraseCommand (
+ IN UINT8 *pBlockAddress,
+ IN UINT32 Length
+ )
+{
+ EFI_MM_COMMUNICATE_SPINOR_RES *MmSpiNorRes;
+ EFI_STATUS Status;
+ UINT64 MmData[5];
+ UINTN Size;
+
+ ASSERT (pBlockAddress != NULL);
+
+ MmData[0] = MM_SPINOR_FUNC_ERASE;
+ MmData[1] = (UINT64)pBlockAddress;
+ MmData[2] = Length;
+
+ Status = UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = FlashMmCommunicate (
+ mCommBuffer,
+ &Size
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mCommBuffer->PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "Flash Erase: Device error %llx\n", MmSpiNorRes->Status));
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FlashProgramCommand (
+ IN UINT8 *pByteAddress,
+ IN UINT8 *Byte,
+ IN OUT UINTN *Length
+ )
+{
+ EFI_MM_COMMUNICATE_SPINOR_RES *MmSpiNorRes;
+ EFI_STATUS Status;
+ UINT64 MmData[5];
+ UINTN Remain, Size, NumWrite;
+ UINTN Count = 0;
+
+ ASSERT (pByteAddress != NULL);
+ ASSERT (Byte != NULL);
+ ASSERT (Length != NULL);
+
+ Remain = *Length;
+ while (Remain > 0) {
+ NumWrite = (Remain > EFI_MM_MAX_TMP_BUF_SIZE) ? EFI_MM_MAX_TMP_BUF_SIZE : Remain;
+
+ MmData[0] = MM_SPINOR_FUNC_WRITE;
+ MmData[1] = (UINT64)pByteAddress;
+ MmData[2] = NumWrite;
+ MmData[3] = (UINT64)ConvertVirtualToPhysical (Byte + Count, NumWrite);
+
+ Status = UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = FlashMmCommunicate (
+ mCommBuffer,
+ &Size
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mCommBuffer->PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "Flash program: Device error 0x%llx\n", MmSpiNorRes->Status));
+ return EFI_DEVICE_ERROR;
+ }
+
+ Remain -= NumWrite;
+ Count += NumWrite;
+ }
+
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FlashReadCommand (
+ IN UINT8 *pByteAddress,
+ OUT UINT8 *Byte,
+ IN OUT UINTN *Length
+ )
+{
+ EFI_MM_COMMUNICATE_SPINOR_RES *MmSpiNorRes;
+ EFI_STATUS Status;
+ UINT64 MmData[5];
+ UINTN Remain, Size, NumRead;
+ UINTN Count = 0;
+
+ ASSERT (pByteAddress != NULL);
+ ASSERT (Byte != NULL);
+ ASSERT (Length != NULL);
+
+ Remain = *Length;
+ while (Remain > 0) {
+ NumRead = (Remain > EFI_MM_MAX_TMP_BUF_SIZE) ? EFI_MM_MAX_TMP_BUF_SIZE : Remain;
+
+ MmData[0] = MM_SPINOR_FUNC_READ;
+ MmData[1] = (UINT64)pByteAddress;
+ MmData[2] = NumRead;
+ MmData[3] = (UINT64)ConvertVirtualToPhysical (Byte + Count, NumRead);
+
+ Status = UefiMmCreateSpiNorReq ((VOID *)&MmData, sizeof (MmData));
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Size = sizeof (EFI_MM_COMM_HEADER_NOPAYLOAD) + sizeof (MmData);
+ Status = FlashMmCommunicate (
+ mCommBuffer,
+ &Size
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ MmSpiNorRes = (EFI_MM_COMMUNICATE_SPINOR_RES *)&mCommBuffer->PayLoad;
+ if (MmSpiNorRes->Status != MM_SPINOR_RES_SUCCESS) {
+ DEBUG ((DEBUG_ERROR, "Flash Read: Device error %llx\n", MmSpiNorRes->Status));
+ return EFI_DEVICE_ERROR;
+ }
+
+ ConvertPhysicaltoVirtual (Byte + Count, NumRead);
+ Remain -= NumRead;
+ Count += NumRead;
+ }
+
+ return EFI_SUCCESS;
+}
--
2.17.1