On 26 October 2018 at 10:18, Ming Huang <ming.hu...@linaro.org> wrote: > Hi Ard & Laszlo, > > Sorry for delay reply. > Should all host bridges use EISA_PNP_ID(0x0A03)? >
Yes. > On 10/12/2018 4:08 PM, Laszlo Ersek wrote: >> On 10/12/18 09:29, Ard Biesheuvel wrote: >>> Hello all, >>> >>> While grepping through the code in edk2-platforms, I noticed an issue >>> with this commit. Apologies for not spotting it earlier. >>> >>> On 31 August 2018 at 15:27, Ming Huang <ming.hu...@linaro.org> wrote: >>>> PciHostBridgeLib which is need by PciHostBridgeDxe, provide >>>> root bridges and deal with resource conflict. >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.1 >>>> Signed-off-by: Ming Huang <ming.hu...@linaro.org> >>>> Reviewed-by: Leif Lindholm <leif.lindh...@linaro.org> >>>> --- >>>> Platform/Hisilicon/D06/D06.dsc | >>>> 2 +- >>>> Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf | >>>> 36 ++ >>>> Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c | >>>> 635 ++++++++++++++++++++ >>>> 3 files changed, 672 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Platform/Hisilicon/D06/D06.dsc >>>> b/Platform/Hisilicon/D06/D06.dsc >>>> index 2659cb7e37..83dcbab6c4 100644 >>>> --- a/Platform/Hisilicon/D06/D06.dsc >>>> +++ b/Platform/Hisilicon/D06/D06.dsc >>>> @@ -419,7 +419,7 @@ >>>> <LibraryClasses> >>>> >>>> PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf >>>> PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf >>>> - >>>> PciHostBridgeLib|MdeModulePkg/Library/PciHostBridgeLibNull/PciHostBridgeLibNull.inf >>>> + >>>> PciHostBridgeLib|Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>>> } >>>> >>>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf >>>> diff --git >>>> a/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>>> b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>>> new file mode 100644 >>>> index 0000000000..8a998681a3 >>>> --- /dev/null >>>> +++ b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.inf >>>> @@ -0,0 +1,36 @@ >>>> +## @file >>>> +# >>>> +# Copyright (c) 2018, Hisilicon Limited. All rights reserved.<BR> >>>> +# Copyright (c) 2018, Linaro Limited. All rights reserved.<BR> >>>> +# >>>> +# This program and the accompanying materials >>>> +# are licensed and made available under the terms and conditions of the >>>> BSD License >>>> +# which accompanies this distribution. The full text of the license may >>>> be found at >>>> +# http://opensource.org/licenses/bsd-license.php >>>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>>> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>>> IMPLIED. >>>> +# >>>> +# >>>> +## >>>> + >>>> +[Defines] >>>> + INF_VERSION = 0x0001001A >>>> + BASE_NAME = PciHostBridgeLib >>>> + FILE_GUID = 61b7276a-fc67-11e5-82fd-47ea9896dd5d >>>> + MODULE_TYPE = DXE_DRIVER >>>> + VERSION_STRING = 1.0 >>>> + LIBRARY_CLASS = PciHostBridgeLib|DXE_DRIVER >>>> + >>>> +[Sources] >>>> + PciHostBridgeLib.c >>>> + >>>> +[Packages] >>>> + MdeModulePkg/MdeModulePkg.dec >>>> + MdePkg/MdePkg.dec >>>> + >>>> +[LibraryClasses] >>>> + BaseLib >>>> + DebugLib >>>> + DevicePathLib >>>> + MemoryAllocationLib >>>> + UefiBootServicesTableLib >>>> diff --git >>>> a/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c >>>> b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c >>>> new file mode 100644 >>>> index 0000000000..d1a436d9bc >>>> --- /dev/null >>>> +++ b/Platform/Hisilicon/D06/Library/PciHostBridgeLib/PciHostBridgeLib.c >>>> @@ -0,0 +1,635 @@ >>>> +/** @file >>>> + >>>> + Copyright (c) 2018, Hisilicon Limited. All rights reserved.<BR> >>>> + Copyright (c) 2018, Linaro Limited. All rights reserved.<BR> >>>> + >>>> + This program and the accompanying materials >>>> + are licensed and made available under the terms and conditions of the >>>> BSD License >>>> + which accompanies this distribution. The full text of the license may >>>> be found at >>>> + http://opensource.org/licenses/bsd-license.php >>>> + >>>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >>>> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>>> IMPLIED. >>>> + >>>> +**/ >>>> +#include <PiDxe.h> >>>> +#include <Library/DebugLib.h> >>>> +#include <Library/DevicePathLib.h> >>>> +#include <Library/MemoryAllocationLib.h> >>>> +#include <Library/PciHostBridgeLib.h> >>>> +#include <Protocol/PciHostBridgeResourceAllocation.h> >>>> +#include <Protocol/PciRootBridgeIo.h> >>>> + >>>> +#define ENUM_HB_NUM 8 >>>> + >>>> +#define EFI_PCI_SUPPORT (EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | \ >>>> + EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO | \ >>>> + EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | \ >>>> + EFI_PCI_ATTRIBUTE_ISA_IO_16 | \ >>>> + EFI_PCI_ATTRIBUTE_VGA_MEMORY | \ >>>> + EFI_PCI_ATTRIBUTE_VGA_IO_16 | \ >>>> + EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16) >>>> + >>>> +#define EFI_PCI_ATTRIBUTE EFI_PCI_SUPPORT >>>> + >>>> +#pragma pack(1) >>>> +typedef struct { >>>> + ACPI_HID_DEVICE_PATH AcpiDevicePath; >>>> + EFI_DEVICE_PATH_PROTOCOL EndDevicePath; >>>> +} EFI_PCI_ROOT_BRIDGE_DEVICE_PATH; >>>> +#pragma pack () >>>> + >>>> +STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mEfiPciRootBridgeDevicePath >>>> [ENUM_HB_NUM] = { >>>> +//Host Bridge 0 >>>> + { >>>> + { >>>> + { >>>> + ACPI_DEVICE_PATH, >>>> + ACPI_DP, >>>> + { >>>> + (UINT8)sizeof (ACPI_HID_DEVICE_PATH), >>>> + (UINT8)(sizeof (ACPI_HID_DEVICE_PATH) >> 8) >>>> + } >>>> + }, >>>> + EISA_PNP_ID(0x0A03), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 2 >>> ... >>>> + EISA_PNP_ID(0x0A04), // PCI >>>> + 0 >>> .. >>>> +//Host Bridge 4 >>> ... >>>> + EISA_PNP_ID(0x0A05), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 5 >>> ... >>>> + EISA_PNP_ID(0x0A06), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 6 >>> ... >>>> + EISA_PNP_ID(0x0A07), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 8 >>> ... >>>> + EISA_PNP_ID(0x0A08), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 10 >>> ... >>>> + EISA_PNP_ID(0x0A09), // PCI >>>> + 0 >>> ... >>>> +//Host Bridge 11 >>> ... >>>> + EISA_PNP_ID(0x0A0A), // PCI >>>> + 0 >>> >>> This is *not* how it works. You cannot invent your own ACPI HIDs like >>> that. If you have multiple instances of a device, you increment the >>> UID. >> >> I agree. >> >>> Please synchronize these definitions with the HIDs/UIDs used in the >>> DSDT/SSDT for PCIe. And please make sure all host bridges are >>> accounted for. >> >> I agree again; this is the cleanest approach, by the book. >> >> ( >> >> In order to be completely honest, I have to point out that we're >> currently cutting some corners on the PciRoot() / PcieRoot() difference, >> in ArmVirtQemu. (Which translates to a PNPID difference, between 0x0A03 >> / 0x0A08.) Please refer to the following discussions (search them for >> "EISA_PNP_ID"): >> >> - http://mid.mail-archive.com/8ba58ec8-9360-4805-c9a5-b9d5c193fdb0@redhat.com >> - http://mid.mail-archive.com/b87c8a0e-e4c9-f9d3-3d53-f7bb1e27cc1c@redhat.com >> - >> http://mid.mail-archive.com/1472840159-28957-4-git-send-email-ard.biesheuvel@linaro.org >> (and this was pushed ultimately) >> >> ) >> >> Thanks >> Laszlo >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
