From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web08.27162.1611146780424388411 for ; Wed, 20 Jan 2021 04:46:20 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=AuHvOSZX; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611146779; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=CK3KLbkt4xvcnt+gpGRpeqZVXg24s9EIdGzOB7fbjgw=; b=AuHvOSZXnMNJVmhMweIUXvGtr4Q1SodVkImDzE0RuCC/H/gxr10uRZHwo0kQ+0rE1Ahk2T N1ZNSR9o/HmIiJs2zEubAs3cwkyKaqwtQ2H3J2ill8jb/cwLX6zIvBDYy0t3AYF0BWWLtj UmpAYYAxSCc6Cifky7j9dGiCCxfrRYE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-416-5dNdE7CcPmO5XQkbMdkCvQ-1; Wed, 20 Jan 2021 07:46:15 -0500 X-MC-Unique: 5dNdE7CcPmO5XQkbMdkCvQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6C5EE107AD34; Wed, 20 Jan 2021 12:46:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-96.ams2.redhat.com [10.36.115.96]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8EF0F10023B2; Wed, 20 Jan 2021 12:46:10 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v6 06/11] ArmVirtPkg/FdtPciHostBridgeLib: Refactor init/uninit of root bridge To: devel@edk2.groups.io, cenjiahui@huawei.com Cc: Jordan Justen , Ard Biesheuvel , Rebecca Cran , Peter Grehan , Anthony Perard , Julien Grall , Leif Lindholm , Sami Mujawar , xieyingtai@huawei.com, wu.wubin@huawei.com, Yubo Miao References: <20210119011302.10908-1-cenjiahui@huawei.com> <20210119011302.10908-7-cenjiahui@huawei.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 20 Jan 2021 13:46:09 +0100 MIME-Version: 1.0 In-Reply-To: <20210119011302.10908-7-cenjiahui@huawei.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/19/21 02:12, Jiahui Cen via groups.io wrote: > Rebase ArmVirtPkg/FdtPciHostBridgeLib to the new > PciHostBridgeUtilityInitRootBridge()/PciHostBridgeUtilityUninitRootBridge() > utility functions. > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059 > > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Signed-off-by: Jiahui Cen > Signed-off-by: Yubo Miao > --- > ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf | 1 + > ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 124 ++++++++++---------- > 2 files changed, 61 insertions(+), 64 deletions(-) Reviewed-by: Laszlo Ersek Thanks Laszlo > > diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf > index 01d39626d14c..b813a0851d2a 100644 > --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf > +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf > @@ -34,6 +34,7 @@ [Packages] > OvmfPkg/OvmfPkg.dec > > [LibraryClasses] > + BaseMemoryLib > DebugLib > DevicePathLib > DxeServicesTableLib > diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c > index d554479bf0de..3ec7992b6331 100644 > --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c > +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c > @@ -7,6 +7,7 @@ > > **/ > #include > +#include > #include > #include > #include > @@ -20,37 +21,6 @@ > #include > #include > > -#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 = { > - { > - { > - ACPI_DEVICE_PATH, > - ACPI_DP, > - { > - (UINT8) (sizeof(ACPI_HID_DEVICE_PATH)), > - (UINT8) ((sizeof(ACPI_HID_DEVICE_PATH)) >> 8) > - } > - }, > - EISA_PNP_ID(0x0A03), > - 0 > - }, > - > - { > - END_DEVICE_PATH_TYPE, > - END_ENTIRE_DEVICE_PATH_SUBTYPE, > - { > - END_DEVICE_PATH_LENGTH, > - 0 > - } > - } > -}; > - > // > // We expect the "ranges" property of "pci-host-ecam-generic" to consist of > // records like this. > @@ -319,11 +289,18 @@ PciHostBridgeGetRootBridges ( > UINTN *Count > ) > { > - UINT64 IoBase, IoSize; > - UINT64 Mmio32Base, Mmio32Size; > - UINT64 Mmio64Base, Mmio64Size; > - UINT32 BusMin, BusMax; > - EFI_STATUS Status; > + UINT64 IoBase, IoSize; > + UINT64 Mmio32Base, Mmio32Size; > + UINT64 Mmio64Base, Mmio64Size; > + UINT32 BusMin, BusMax; > + EFI_STATUS Status; > + UINT64 Attributes; > + UINT64 AllocationAttributes; > + PCI_ROOT_BRIDGE_APERTURE Io; > + PCI_ROOT_BRIDGE_APERTURE Mem; > + PCI_ROOT_BRIDGE_APERTURE MemAbove4G; > + PCI_ROOT_BRIDGE_APERTURE PMem; > + PCI_ROOT_BRIDGE_APERTURE PMemAbove4G; > > if (PcdGet64 (PcdPciExpressBaseAddress) == 0) { > DEBUG ((EFI_D_INFO, "%a: PCI host bridge not present\n", __FUNCTION__)); > @@ -341,33 +318,29 @@ PciHostBridgeGetRootBridges ( > return NULL; > } > > - *Count = 1; > + ZeroMem (&Io, sizeof (Io)); > + ZeroMem (&Mem, sizeof (Mem)); > + ZeroMem (&MemAbove4G, sizeof (MemAbove4G)); > + ZeroMem (&PMem, sizeof (PMem)); > + ZeroMem (&PMemAbove4G, sizeof (PMemAbove4G)); > > - mRootBridge.Segment = 0; > - mRootBridge.Supports = EFI_PCI_ATTRIBUTE_ISA_IO_16 | > - EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | > - EFI_PCI_ATTRIBUTE_VGA_IO_16 | > - EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16; > - mRootBridge.Attributes = mRootBridge.Supports; > + Attributes = EFI_PCI_ATTRIBUTE_ISA_IO_16 | > + EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | > + EFI_PCI_ATTRIBUTE_VGA_IO_16 | > + EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16; > > - mRootBridge.DmaAbove4G = TRUE; > - mRootBridge.NoExtendedConfigSpace = FALSE; > - mRootBridge.ResourceAssigned = FALSE; > + AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM; > > - mRootBridge.AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM; > - > - mRootBridge.Bus.Base = BusMin; > - mRootBridge.Bus.Limit = BusMax; > - mRootBridge.Io.Base = IoBase; > - mRootBridge.Io.Limit = IoBase + IoSize - 1; > - mRootBridge.Mem.Base = Mmio32Base; > - mRootBridge.Mem.Limit = Mmio32Base + Mmio32Size - 1; > + Io.Base = IoBase; > + Io.Limit = IoBase + IoSize - 1; > + Mem.Base = Mmio32Base; > + Mem.Limit = Mmio32Base + Mmio32Size - 1; > > if (sizeof (UINTN) == sizeof (UINT64)) { > - mRootBridge.MemAbove4G.Base = Mmio64Base; > - mRootBridge.MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1; > + MemAbove4G.Base = Mmio64Base; > + MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1; > if (Mmio64Size > 0) { > - mRootBridge.AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE; > + AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE; > } > } else { > // > @@ -376,19 +349,41 @@ PciHostBridgeGetRootBridges ( > // BARs unless they are allocated below 4 GB. So ignore the range above > // 4 GB in this case. > // > - mRootBridge.MemAbove4G.Base = MAX_UINT64; > - mRootBridge.MemAbove4G.Limit = 0; > + MemAbove4G.Base = MAX_UINT64; > + MemAbove4G.Limit = 0; > } > > // > // No separate ranges for prefetchable and non-prefetchable BARs > // > - mRootBridge.PMem.Base = MAX_UINT64; > - mRootBridge.PMem.Limit = 0; > - mRootBridge.PMemAbove4G.Base = MAX_UINT64; > - mRootBridge.PMemAbove4G.Limit = 0; > + PMem.Base = MAX_UINT64; > + PMem.Limit = 0; > + PMemAbove4G.Base = MAX_UINT64; > + PMemAbove4G.Limit = 0; > > - mRootBridge.DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)&mEfiPciRootBridgeDevicePath; > + Status = PciHostBridgeUtilityInitRootBridge ( > + Attributes, > + Attributes, > + AllocationAttributes, > + TRUE, > + FALSE, > + BusMin, > + BusMax, > + &Io, > + &Mem, > + &MemAbove4G, > + &PMem, > + &PMemAbove4G, > + &mRootBridge > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: failed to initialize PCI host bridge: %r\n", > + __FUNCTION__, Status)); > + *Count = 0; > + return NULL; > + } > + > + *Count = 1; > > return &mRootBridge; > } > @@ -408,6 +403,7 @@ PciHostBridgeFreeRootBridges ( > ) > { > ASSERT (Count == 1); > + PciHostBridgeUtilityUninitRootBridge (Bridges); > } > > /** >