Ard,
At first glance of this patch, I immediately checked the PI spec SubResources()
description,
it says when a root bridge doesn't require any resource, "a zero-length
resource request
must explicitly be submitted."
"zero-length resource" can be interpreted in two ways:
1. only END descriptor
2. MEM resource with 0-length, plus IO resource with 0-length, plus END
descriptor
I believe that both ways should be accepted by PciHostBridge driver.
But without your patch, the zero descriptor cannot be accepted by PciHostBridge
driver.
I think it fails in the check:
case ACPI_ADDRESS_SPACE_TYPE_MEM /* 0 */:
if (Descriptor->AddrSpaceGranularity != 32 &&
Descriptor->AddrSpaceGranularity != 64) {
return EFI_INVALID_PARAMETER;
}
I agree with your patch.
Reviewed-by: Ruiyu Ni <ruiyu...@intel.com>
Regards,
Ray
>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
>Biesheuvel
>Sent: Thursday, April 28, 2016 11:51 PM
>To: edk2-devel@lists.01.org; Tian, Feng <feng.t...@intel.com>; Zeng, Star
><star.z...@intel.com>
>Cc: leif.lindh...@linaro.org; Ard Biesheuvel <ard.biesheu...@linaro.org>
>Subject: [edk2] [PATCH] MdeModulePkg/PciBusDxe: don't create bogus descriptor
>if no resources needed
>
>If the current PCI configuration requires no resources to be allocated at
>all (i.e., unpopulated bus), the PCI enumeration code creates a single
>ACPI_ADDRESS_SPACE_DESCRIPTOR memory descriptor with all fields cleared.
>This is rejected by the SubmitResources() implementation of the generic
>PciHostBridgeDxe in the following way:
>
> PciHostBridge: SubmitResources for PcieRoot(0x0)
> Mem: Granularity/SpecificFlag = 0 / 00
> Length/Alignment = 0x0 / 0x0
> PciBus: HostBridge->SubmitResources() - Invalid Parameter
>
> ASSERT_EFI_ERROR (Status = Invalid Parameter)
> ASSERT [PciBusDxe] .../PciBusDxe/PciLib.c(561): !EFI_ERROR (Status)
>
>So instead, create the empty configuration as a single entry of type
>EFI_ACPI_END_TAG_DESCRIPTOR.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>---
> MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
>b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
>index 597c0834e0f5..469a2ddb8ac0 100644
>--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
>+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
>@@ -1307,15 +1307,12 @@ ConstructAcpiResourceRequestor (
> //
> // If there is no resource request
> //
>- Configuration = AllocateZeroPool (sizeof
>(EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof
>(EFI_ACPI_END_TAG_DESCRIPTOR));
>+ Configuration = AllocateZeroPool (sizeof (EFI_ACPI_END_TAG_DESCRIPTOR));
> if (Configuration == NULL) {
> return EFI_OUT_OF_RESOURCES;
> }
>
>- Ptr = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) (Configuration);
>- Ptr->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
>-
>- PtrEnd = (EFI_ACPI_END_TAG_DESCRIPTOR *) (Ptr + 1);
>+ PtrEnd = (EFI_ACPI_END_TAG_DESCRIPTOR *) (Configuration);
> PtrEnd->Desc = ACPI_END_TAG_DESCRIPTOR;
> PtrEnd->Checksum = 0;
> }
>--
>2.7.4
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
