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::144; helo=mail-it1-x144.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x144.google.com (mail-it1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) (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 C7D7321184E9F for ; Mon, 5 Nov 2018 03:23:33 -0800 (PST) Received: by mail-it1-x144.google.com with SMTP id j79-v6so70764itb.2 for ; Mon, 05 Nov 2018 03:23:33 -0800 (PST) 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=TmhCMrUPwv833nbMdJXkS61s/bIL6mP1tU5r7/I5mPM=; b=UbEyhThvaIiELZkN5DkEpyjrNFgMDO8khwjsE78R1s11VYQveiQ9Ze9fQGFaHLrUgb Y1NtotYQjnM1pdZUSOSofmV5wqJK8Npy52YVzABTi8x7hohQsH5FDKIL2aOKhwOuV7F3 yXI9AbNqu6bG2Htct3rDVkK+ZrHV8u/WN0sNY= 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=TmhCMrUPwv833nbMdJXkS61s/bIL6mP1tU5r7/I5mPM=; b=CONfY3ok/aB3SKWEGTCCB3wa951mfG4bLd8eg8doNxA3plM4d8Zq45nXbeLpU4Hpon yvc7z/T1PdcuC5p6t5gH8nSQ+zSrbrp5HYRXDixu1f4xJbjelOUdC5VaooSzM/YypEN/ ded0OBAhUulqrJvA94jX+vvJFlImIWL7vFKa+V2NGtM6J/1l6ovurhr6h50B/ezlXFhj 7jvpGdIXpgvGT5Br7fnOvRvSXjChjywm68BWP+T7sZjMT0a65rQAUX/K7h+mdFZ90dEs RNVV62ahCGRlM8bfEG2KZ7agnPQGscp7uWfmGrvuCRR2q/XfzA5yEQTizSGdwtqSjbxO 1iLA== X-Gm-Message-State: AGRZ1gLnNUhB1DyE9klARsL0+BrWUDGPvZOxcay+51GTRmIs6m6L22Ip Z5lVcQPMXZ8It50bbXspToxX1VaLf+5u8YIJI25ibA== X-Google-Smtp-Source: AJdET5ecFjFB8KLGC+VrQcpNnskT74Ra9UuRlbWEAW3i1HUA8UYx9qu2l2PGzFrUIT/oaA9M74hyWvFIAXETcWX3tm8= X-Received: by 2002:a24:8347:: with SMTP id d68-v6mr6887100ite.158.1541417012962; Mon, 05 Nov 2018 03:23:32 -0800 (PST) MIME-Version: 1.0 Received: by 2002:a6b:4f16:0:0:0:0:0 with HTTP; Mon, 5 Nov 2018 03:23:32 -0800 (PST) In-Reply-To: <84939887-b3d3-f1c8-48fc-a1902133ead9@linaro.org> References: <20180831132710.23055-1-ming.huang@linaro.org> <20180831132710.23055-22-ming.huang@linaro.org> <84939887-b3d3-f1c8-48fc-a1902133ead9@linaro.org> From: Ard Biesheuvel Date: Mon, 5 Nov 2018 12:23:32 +0100 Message-ID: To: Ming Huang Cc: Laszlo Ersek , Leif Lindholm , linaro-uefi , "edk2-devel@lists.01.org" , Graeme Gregory , "Kinney, Michael D" , 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: Mon, 05 Nov 2018 11:23:34 -0000 Content-Type: text/plain; charset="UTF-8" On 26 October 2018 at 10:18, Ming Huang 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 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. >> >> 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 >>