From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d43; helo=mail-io1-xd43.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A7ED421168211 for ; Fri, 12 Oct 2018 00:29:26 -0700 (PDT) Received: by mail-io1-xd43.google.com with SMTP id w2-v6so8570671ioc.1 for ; Fri, 12 Oct 2018 00:29:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=eWgyEJp0NiuQIFJ2B/8wo0Dnr/KGb4dM1Q3x+FJchhc=; b=bDcdtem/BC+nmrF2g3Y4+rqbFE5poMdx4MZ3r7csklXoKlPIvG6FeY7+5yLRhrxuWM sEyyuLasHL81wUmPN8k1Mb18iXo6lNA90d8IO6w01KIM5xVv4J/mcyHB3LserIl9rRW/ HCd/7Wl2ya74vE4izFi8aPDi7/4BUiyf655u0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=eWgyEJp0NiuQIFJ2B/8wo0Dnr/KGb4dM1Q3x+FJchhc=; b=Mzeg07KIId2j+bEDjoiffeMuOalTLsAL5B4fmGFGqeOezj0+o1oxg6ZnaEpL4KkSns rPAMZDcq3Afp9wHRAPcXQNhB1uwka/2qw8k+6ohYJlQrLj8pAJtZtsIzZtFAi4uuAgD/ xHYfRc3q0y1Bk+V/4Ehp1nByaj1yE50lxgDSR0eD4844PT2sJIuj8oQZp9i73ouqgwN+ 3wDepaneYCjkNTBLsmEuRtONPrhrldkjVMNfAlebcQ+fWsBFeszRccoaHYzfTceEfVXX RK+fDPvRc3W601mfG33Czomzz+ddI/318uGbQPuhb8AHruTlye/6Dmmvc77j0IjP5HW+ EjxA== X-Gm-Message-State: ABuFfog/MMzJia5BcD3P2MopoWD7EKM/X3qSHfvKje+rgUh/LzmVfpTf 3TqD7pmIn0Z65urwMl3WKn2Q0C3p7N89NTfzOhV3PQ== X-Google-Smtp-Source: ACcGV63OFFIaow5ZZ8Lg6Vp4FrGUzN1vEXblfhKouGwcU5uivkS8952Tf7/BEukStqzZ0jEp9zdmu7J5Gl25gQpqmZg= X-Received: by 2002:a6b:3787:: with SMTP id e129-v6mr3557576ioa.60.1539329365726; Fri, 12 Oct 2018 00:29:25 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:5910:0:0:0:0:0 with HTTP; Fri, 12 Oct 2018 00:29:25 -0700 (PDT) In-Reply-To: <20180831132710.23055-22-ming.huang@linaro.org> References: <20180831132710.23055-1-ming.huang@linaro.org> <20180831132710.23055-22-ming.huang@linaro.org> From: Ard Biesheuvel Date: Fri, 12 Oct 2018 09:29:25 +0200 Message-ID: To: Ming Huang Cc: Leif Lindholm , linaro-uefi , "edk2-devel@lists.01.org" , Graeme Gregory , "Kinney, Michael D" , Laszlo Ersek , guoheyi@huawei.com, wanghuiqiang , huangming , Jason Zhang , huangdaode@hisilicon.com, John Garry , Xinliang Liu , zhangfeng56@huawei.com Subject: Re: [PATCH edk2-platforms v5 21/28] Platform/Hisilicon/D06: Add PciHostBridgeLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Oct 2018 07:29:26 -0000 Content-Type: text/plain; charset="UTF-8" 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 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 > Reviewed-by: Leif Lindholm > --- > 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 @@ > > 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.
> +# Copyright (c) 2018, Linaro Limited. All rights reserved.
> +# > +# 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.
> + Copyright (c) 2018, Linaro Limited. All rights reserved.
> + > + 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 > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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. 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.