From: "Kubacki, Michael A" <michael.a.kubacki@intel.com>
To: "Chen, Marc W" <marc.w.chen@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>
Subject: Re: [PATCH] MinPlatformPkg: Add multiple segment support for PciHostBridgeLib
Date: Mon, 13 May 2019 21:24:20 +0000 [thread overview]
Message-ID: <49AB4ACB9627B8468F29D589A27B74558893B564@ORSMSX121.amr.corp.intel.com> (raw)
In-Reply-To: 20190513094655.90868-1-marc.w.chen@intel.com
Actually run PatchCheck.py against the patch there's a few errors that need to be fixed.
> -----Original Message-----
> From: Kubacki, Michael A
> Sent: Monday, May 13, 2019 2:22 PM
> To: Chen, Marc W <marc.w.chen@intel.com>; devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Subject: RE: [PATCH] MinPlatformPkg: Add multiple segment support for
> PciHostBridgeLib
>
> Please update the copyright years. The other changes look fine.
>
> Thanks,
> Michael
>
> > -----Original Message-----
> > From: Chen, Marc W
> > Sent: Monday, May 13, 2019 2:47 AM
> > To: devel@edk2.groups.io
> > Cc: Chen, Marc W <marc.w.chen@intel.com>; Kubacki, Michael A
> > <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>
> > Subject: [PATCH] MinPlatformPkg: Add multiple segment support for
> > PciHostBridgeLib
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1799
> >
> > Add PcdPciSegmentCount PCD in MinPlatformPkg.dec and set default to 1,
> > then base on PciHostBridge related PCDs to Initialize RootBridges.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marc Chen <marc.w.chen@intel.com>
> > Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> > Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> > ---
> > Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 1 +
> > .../PciHostBridgeLibSimple.c | 71 +++++++++++++++-------
> > .../PciHostBridgeLibSimple.inf | 2 +-
> > 3 files changed, 51 insertions(+), 23 deletions(-)
> >
> > diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > index 3185776ac3..09701bd004 100644
> > --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> > @@ -223,6 +223,7 @@
> >
> gMinPlatformPkgTokenSpaceGuid.PcdPcIoApicEnable|0x0|UINT32|0x90000
> > 019
> > gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
> > |FALSE|BOOLEAN|0x4001004B
> > gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
> > |FALSE|BOOLEAN|0x4001004C
> > gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> > |FALSE|BOOLEAN|0x4001004D
> > + gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount |0x1
> > |UINT8|0x4001004E
> >
> >
> >
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1AEventBlockAddress|0x1800|
> > UINT16|0x00010035
> >
> >
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1BEventBlockAddress|0x0000|
> > UINT16|0x00010036
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.c
> > b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.c
> > index 557ac2a5b3..aafadc598b 100644
> > ---
> > a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.c
> > +++ b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple
> > +++ /P
> > +++ ciHostBridgeLibSimple.c
> > @@ -15,6 +15,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND,
> > EITHER EXPRESS OR IMPLIED.
> > #include <IndustryStandard/Pci.h>
> > #include <Protocol/PciHostBridgeResourceAllocation.h>
> > #include <Library/BaseLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > #include <Library/DevicePathLib.h>
> > #include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h>
> > @@
> > -28,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND,
> > EITHER EXPRESS OR IMPLIED.
> > GLOBAL_REMOVE_IF_UNREFERENCED CHAR16
> > *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
> > L"Mem", L"I/O", L"Bus"
> > };
> > -ACPI_HID_DEVICE_PATH mRootBridgeDeviceNode = {
> > +ACPI_HID_DEVICE_PATH mRootBridgeDeviceNodeTemplate = {
> > {
> > ACPI_DEVICE_PATH,
> > ACPI_DP,
> > @@ -41,7 +42,7 @@ ACPI_HID_DEVICE_PATH mRootBridgeDeviceNode =
> {
> > 0
> > };
> >
> > -PCI_ROOT_BRIDGE mRootBridge = {
> > +PCI_ROOT_BRIDGE mRootBridgeTemplate = {
> > 0,
> > EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
> > EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO |
> > @@ -66,41 +67,67 @@ PCI_ROOT_BRIDGE mRootBridge = {
> > NULL // DevicePath;
> > };
> >
> > +/**
> > + Return all the root bridge instances.
> > +
> > + @param Count Return the count of root bridge instances.
> > +
> > + @return All the root bridge instances, it will be NULL if system
> > + has
> > insufficient memory
> > + resources available and count will be zero.
> > +**/
> > +
> > PCI_ROOT_BRIDGE *
> > EFIAPI
> > PciHostBridgeGetRootBridges (
> > UINTN *Count
> > )
> > {
> > - mRootBridge.Mem.Base = PcdGet32 (PcdPciReservedMemBase);
> > + UINT8 Index;
> > + PCI_ROOT_BRIDGE *RootBridge;
> > +
> > + RootBridge = AllocateZeroPool (sizeof (PCI_ROOT_BRIDGE) * PcdGet8
> > + (PcdPciSegmentCount)); if (RootBridge == NULL) {
> > + DEBUG ((EFI_D_ERROR, "PciHostBridge: Out of resource\n"));
> > + *Count = 0;
> > + return RootBridge;
> > + }
> > +
> > + mRootBridgeTemplate.Mem.Base = PcdGet32
> (PcdPciReservedMemBase);
> > if (PcdGet32(PcdPciReservedMemLimit) != 0) {
> > - mRootBridge.Mem.Limit = PcdGet32 (PcdPciReservedMemLimit);
> > + mRootBridgeTemplate.Mem.Limit = PcdGet32
> > (PcdPciReservedMemLimit);
> > } else {
> > - mRootBridge.Mem.Limit = (UINT32)PcdGet64
> > (PcdPciExpressBaseAddress);
> > + mRootBridgeTemplate.Mem.Limit = (UINT32) PcdGet64
> > + (PcdPciExpressBaseAddress);
> > }
> >
> > - mRootBridge.MemAbove4G.Base = PcdGet64
> > (PcdPciReservedMemAbove4GBBase);
> > - mRootBridge.MemAbove4G.Limit = PcdGet64
> > (PcdPciReservedMemAbove4GBLimit);
> > + mRootBridgeTemplate.MemAbove4G.Base = PcdGet64
> > + (PcdPciReservedMemAbove4GBBase);
> > mRootBridgeTemplate.MemAbove4G.Limit
> > + = PcdGet64 (PcdPciReservedMemAbove4GBLimit);
> >
> > - mRootBridge.PMem.Base = PcdGet32 (PcdPciReservedPMemBase);
> > - mRootBridge.PMem.Limit = PcdGet32 (PcdPciReservedPMemLimit);
> > - mRootBridge.PMemAbove4G.Base = PcdGet64
> > (PcdPciReservedPMemAbove4GBBase);
> > - mRootBridge.PMemAbove4G.Limit = PcdGet64
> > (PcdPciReservedPMemAbove4GBLimit);
> > + mRootBridgeTemplate.PMem.Base = PcdGet32
> > (PcdPciReservedPMemBase);
> > + mRootBridgeTemplate.PMem.Limit = PcdGet32
> > (PcdPciReservedPMemLimit);
> > + mRootBridgeTemplate.PMemAbove4G.Base = PcdGet64
> > + (PcdPciReservedPMemAbove4GBBase);
> > + mRootBridgeTemplate.PMemAbove4G.Limit = PcdGet64
> > + (PcdPciReservedPMemAbove4GBLimit);
> >
> > - if (mRootBridge.MemAbove4G.Base < mRootBridge.MemAbove4G.Limit)
> {
> > - mRootBridge.AllocationAttributes |=
> > EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
> > + if (mRootBridgeTemplate.MemAbove4G.Base <
> > mRootBridgeTemplate.MemAbove4G.Limit) {
> > + mRootBridgeTemplate.AllocationAttributes |=
> > + EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
> > }
> >
> > - mRootBridge.Io.Base = PcdGet16 (PcdPciReservedIobase);
> > - mRootBridge.Io.Limit = PcdGet16 (PcdPciReservedIoLimit);
> > + mRootBridgeTemplate.Io.Base = PcdGet16 (PcdPciReservedIobase);
> > + mRootBridgeTemplate.Io.Limit = PcdGet16 (PcdPciReservedIoLimit);
> >
> > - mRootBridge.DmaAbove4G = PcdGetBool (PcdPciDmaAbove4G);
> > - mRootBridge.NoExtendedConfigSpace = PcdGetBool
> > (PcdPciNoExtendedConfigSpace);
> > - mRootBridge.ResourceAssigned = PcdGetBool (PcdPciResourceAssigned);
> > + mRootBridgeTemplate.DmaAbove4G = PcdGetBool
> (PcdPciDmaAbove4G);
> > + mRootBridgeTemplate.NoExtendedConfigSpace = PcdGetBool
> > + (PcdPciNoExtendedConfigSpace);
> > mRootBridgeTemplate.ResourceAssigned =
> > + PcdGetBool (PcdPciResourceAssigned);
> > +
> > + for (Index = 0; Index < PcdGet8 (PcdPciSegmentCount); Index ++) {
> > + mRootBridgeDeviceNodeTemplate.UID = Index;
> > + mRootBridgeTemplate.Segment = Index;
> > + mRootBridgeTemplate.DevicePath = NULL;
> > + mRootBridgeTemplate.DevicePath = AppendDevicePathNode (NULL,
> > &mRootBridgeDeviceNodeTemplate.Header);
> > + CopyMem (RootBridge + Index, &mRootBridgeTemplate, sizeof
> > + (PCI_ROOT_BRIDGE)); }
> >
> > - mRootBridge.DevicePath = AppendDevicePathNode (NULL,
> > &mRootBridgeDeviceNode.Header);
> > - *Count = 1;
> > - return &mRootBridge;
> > + *Count = PcdGet8 (PcdPciSegmentCount); return RootBridge;
> > }
> >
> > VOID
> > @@ -110,7 +137,7 @@ PciHostBridgeFreeRootBridges (
> > UINTN Count
> > )
> > {
> > - ASSERT (Count == 1);
> > + ASSERT (Count <= PcdGet8 (PcdPciSegmentCount));
> > FreePool (Bridges->DevicePath);
> > }
> >
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.inf
> > b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.inf
> > index f9a769155b..e5c0ca2774 100644
> > ---
> > a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.inf
> > +++
> > b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/Pci
> > Ho
> > stBridgeLibSimple.inf
> > @@ -56,4 +56,4 @@
> > gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
> > gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
> > gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> > -
> > + gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount
> > --
> > 2.16.2.windows.1
next prev parent reply other threads:[~2019-05-13 21:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-13 9:46 [PATCH] MinPlatformPkg: Add multiple segment support for PciHostBridgeLib Marc W Chen
2019-05-13 21:22 ` Kubacki, Michael A
2019-05-13 21:24 ` Kubacki, Michael A [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-05-14 3:17 Marc W Chen
2019-05-13 7:20 Marc W Chen
2019-05-13 6:18 marc.w.chen
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=49AB4ACB9627B8468F29D589A27B74558893B564@ORSMSX121.amr.corp.intel.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