From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Ming Huang <ming.huang@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>,
Leif Lindholm <leif.lindholm@linaro.org>,
linaro-uefi <linaro-uefi@lists.linaro.org>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
Graeme Gregory <graeme.gregory@linaro.org>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
guoheyi@huawei.com, wanghuiqiang <wanghuiqiang@huawei.com>,
huangming <huangming23@huawei.com>,
Jason Zhang <zhangjinsong2@huawei.com>,
huangdaode@hisilicon.com, John Garry <john.garry@huawei.com>,
Xinliang Liu <xinliang.liu@linaro.org>,
zhangfeng56@huawei.com
Subject: Re: [PATCH edk2-platforms v5 21/28] Platform/Hisilicon/D06: Add PciHostBridgeLib
Date: Mon, 5 Nov 2018 12:23:32 +0100 [thread overview]
Message-ID: <CAKv+Gu-EnSznp-x0765J_a0uxkGKTLEhghuLJ663Zudy8MJqPg@mail.gmail.com> (raw)
In-Reply-To: <84939887-b3d3-f1c8-48fc-a1902133ead9@linaro.org>
On 26 October 2018 at 10:18, Ming Huang <ming.huang@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.huang@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.huang@linaro.org>
>>>> Reviewed-by: Leif Lindholm <leif.lindholm@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
>>
next prev parent reply other threads:[~2018-11-05 11:23 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-31 13:26 [PATCH edk2-platforms v5 00/28] Upload for D06 platform Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 01/28] Hisilicon/D0x: Modify PcdBootManagerMenuFile for build Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 02/28] Silicon/Hisilicon/D0x: Move RAS macro to PlatformArch.h Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 03/28] Silicon/Hisilicon/Acpi: Unify HisiAcpiPlatformDxe Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 04/28] Hisilicon/D06: Add several base file for D06 Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 05/28] Platform/Hisilicon/D06: Add M41T83RealTimeClockLib Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 06/28] Hisilicon/D06: Add OemMiscLibD06 Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 07/28] Platform/Hisilicon/D06: Add edk2-non-osi components for D06 Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 08/28] Hisilicon/D06: Add some modules Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 09/28] Silicon/Hisilicon/D06: Wait for all disk ready Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 10/28] Hisilicon/D06: Add Debug Serial Port Init Driver Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 11/28] Hisilicon/D06: Add ACPI Tables for D06 Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 12/28] Hisilicon/D06: Add Hi1620OemConfigUiLib Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 13/28] Silicon/Hisilicon/D06: Stop watchdog Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 14/28] Silicon/Hisilicon/Hi1620/Setup: Add Setup Item "EnableGOP" Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 15/28] Hisilicon/Hi1620: Add ACPI PPTT table Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 16/28] Platform/Hisilicon/D06: Enable ACPI PPTT Ming Huang
2018-08-31 13:26 ` [PATCH edk2-platforms v5 17/28] Platform/Hisilicon/D06: Add OemNicLib Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 18/28] Platform/Hisilicon/D06: Add OemNicConfig2P Driver Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 19/28] Hisilicon/D0x: Update SMBIOS type9 info Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 20/28] Platform/Hisilicon/D06: Add EarlyConfigPeim peim Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 21/28] Platform/Hisilicon/D06: Add PciHostBridgeLib Ming Huang
2018-10-12 7:29 ` Ard Biesheuvel
2018-10-12 8:08 ` Laszlo Ersek
2018-10-26 8:18 ` Ming Huang
2018-11-05 11:23 ` Ard Biesheuvel [this message]
2018-08-31 13:27 ` [PATCH edk2-platforms v5 22/28] Hisilicon/D06: add apei driver Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 23/28] Platform/Hisilicon/D06: Add capsule upgrade support Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 24/28] Silicon/Hisilicon: Modify for disable slave core clock Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 25/28] Silicon/Hisilicon: Add I2C Bus Exception handle function Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 26/28] Silicon/Hisilicon/Setup: Support SPCR table switch Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 27/28] Silicon/Hisilicon/setup: Enable/disable SMMU Ming Huang
2018-08-31 13:27 ` [PATCH edk2-platforms v5 28/28] Platform/Hisilicon/D0x: Update version string to 18.08 Ming Huang
2018-08-31 22:57 ` [PATCH edk2-platforms v5 00/28] Upload for D06 platform Leif Lindholm
2018-09-03 15:43 ` Leif Lindholm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKv+Gu-EnSznp-x0765J_a0uxkGKTLEhghuLJ663Zudy8MJqPg@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox