public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
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
>>


  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