Re: [PATCH v3 12/28] AmpereAltraPkg: Add Ac01PcieLib library instance

["Leif Lindholm" <leif@nuviainc.com>]

Apart from the request to break out Ac01PcieConfigRW and
Ac01PcieCfgIn/Out# I noticed a further thing.

On Wed, Sep 15, 2021 at 22:55:11 +0700, Nhi Pham wrote:
> +/**
> +  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);

Should a function named "Check if RootBridge is Disabled" really have
the undocumented side effect of going off and installing ACPI tables?

Please move that logic over to PciHostBridgeReadyToBootEvent.

With that, I think this revision is fully reviewed.

/
    Leif

> +  }
> +  return Ret;
> +}