| Message ID | 1461858688-31517-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | b59e2427c2d92cfee0238d9bde7372691c2af17c |
| Headers | show |
On 04/28/16 17:51, Ard Biesheuvel wrote: > 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.biesheuvel@linaro.org> > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Is this error related to the other thread? http://thread.gmane.org/gmane.comp.bios.edk2.devel/11135 In particular, to PcdPciDisableBusEnumeration? Thanks Laszlo > 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; > } > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 28 April 2016 at 18:06, Laszlo Ersek <lersek@redhat.com> wrote: > On 04/28/16 17:51, Ard Biesheuvel wrote: >> 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.biesheuvel@linaro.org> >> --- >> MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 7 ++----- >> 1 file changed, 2 insertions(+), 5 deletions(-) > > Is this error related to the other thread? > > http://thread.gmane.org/gmane.comp.bios.edk2.devel/11135 > > In particular, to PcdPciDisableBusEnumeration? > No, it is completely separate, as far as I can tell. It simply creates an incorrect descriptor list if none of the PCI devices behind a root bridge require any resources at all, and this is probably a code path that you are very unlikely to hit on x86. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/28/16 18:09, Ard Biesheuvel wrote: > On 28 April 2016 at 18:06, Laszlo Ersek <lersek@redhat.com> wrote: >> On 04/28/16 17:51, Ard Biesheuvel wrote: >>> 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.biesheuvel@linaro.org> >>> --- >>> MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 7 ++----- >>> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> Is this error related to the other thread? >> >> http://thread.gmane.org/gmane.comp.bios.edk2.devel/11135 >> >> In particular, to PcdPciDisableBusEnumeration? >> > > No, it is completely separate, as far as I can tell. It simply creates > an incorrect descriptor list if none of the PCI devices behind a root > bridge require any resources at all, and this is probably a code path > that you are very unlikely to hit on x86. Ah, very interesting. So only devices with PCI config space, right? How curious. :) Thank you for the explanation! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 29 April 2016 at 09:19, Ni, Ruiyu <ruiyu.ni@intel.com> wrote: > 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 analysis. The NULL entry is misidentified as a ACPI_ADDRESS_SPACE_TYPE_MEM resource descriptor because the constant is defined as 0 > I agree with your patch. > > Reviewed-by: Ruiyu Ni <Ruiyu.ni@intel.com> > Thank you. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
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; }
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.biesheuvel@linaro.org> --- MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel