From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by mx.groups.io with SMTP id smtpd.web12.17391.1605151291456581972 for ; Wed, 11 Nov 2020 19:21:31 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: huawei.com, ip: 45.249.212.191, mailfrom: cenjiahui@huawei.com) Received: from DGGEMS402-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4CWn1p6cFSzhjhT; Thu, 12 Nov 2020 11:21:18 +0800 (CST) Received: from [10.174.184.155] (10.174.184.155) by DGGEMS402-HUB.china.huawei.com (10.3.19.202) with Microsoft SMTP Server id 14.3.487.0; Thu, 12 Nov 2020 11:21:17 +0800 Subject: Re: [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib To: Laszlo Ersek , CC: , , , , References: <20201109130511.5946-1-cenjiahui@huawei.com> <20201109130511.5946-2-cenjiahui@huawei.com> From: "Jiahui Cen" Message-ID: <0ea2b503-56ef-0c3e-4544-05fd083894c4@huawei.com> Date: Thu, 12 Nov 2020 11:21:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.174.184.155] X-CFilter-Loop: Reflected Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Laszlo, Thanks for your detailed review. Will fix them in v3. Jiahui On 2020/11/12 0:45, Laszlo Ersek wrote: > On 11/09/20 14:05, Jiahui Cen wrote: >> From: Yubo Miao >> >> Introduce a new PciHostBridgeUtilityLib class to share duplicate code >> between OvmfPkg and ArmVirtPkg. >> >> Extract function PciHostBridgeResourceConflict from >> OvmfPkg/PciHostBridgeLib. >> >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Signed-off-by: Yubo Miao >> Signed-off-by: Jiahui Cen >> --- >> OvmfPkg/OvmfPkgIa32.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> OvmfPkg/OvmfXen.dsc | 1 + >> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 + >> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 49 +++++++++++++ >> OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 33 +++++++++ >> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 64 +---------------- >> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 74 ++++++++++++++++++++ >> 9 files changed, 162 insertions(+), 63 deletions(-) > > (1) There is a typo in the subject: s/form/from/ > > > (2) "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf" is consumed > in the following platform DSC file as well -- please update it too: > > OvmfPkg/Bhyve/BhyveX64.dsc > > (This will affect the list of necessary CC's on this patch.) > > > (3) In my v1 review at > (msgid <57fd0043-d63a-55b0-9c55-4ca079331885@redhat.com>), I mentioned > "OvmfPkg/OvmfPkg.dec". > > You forgot to extend that file, with the new library class. Please add > the following hunk to this patch: > >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index 118c968fda4e..b89252eeab2a 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -49,6 +49,10 @@ [LibraryClasses] >> # access. >> PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h >> >> + ## @libraryclass Provide common utility functions to PciHostBridgeLib >> + # instances in ArmVirtPkg and OvmfPkg. >> + PciHostBridgeUtilityLib|Include/Library/PciHostBridgeUtilityLib.h >> + >> ## @libraryclass Register a status code handler for printing the Boot >> # Manager's LoadImage() and StartImage() preparations, and >> # return codes, to the UEFI console. > > > (4) Issue (3) above would have been caught by CI, namely by the > "LibraryClassCheck" plugin: > > https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#library-declaration-test---libraryclasscheck > > In order to put your patches through CI before posting, I suggest > pushing them to a branch in your edk2 clone on github.com, and > submitting a pull request against the central repository. The > pull request will be auto-closed in the end, unconditionally, but it > will give you CI results. > > (You can also run CI locally, but setting that up is laborious.) > > Back to your patch: > > > On 11/09/20 14:05, Jiahui Cen wrote: >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >> index 58d9f292f9ac..0c2bf0b13c34 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -738,6 +738,7 @@ [Components] >> >> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf >> + PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> } >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf { >> >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 3551f9710a6c..baf36a4f7a54 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -752,6 +752,7 @@ [Components.X64] >> >> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf >> + PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> } >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf { >> >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index 7a8bdb8a8697..219b5f559b53 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -748,6 +748,7 @@ [Components] >> >> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf >> + PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> } >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf { >> >> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc >> index 34c9de19dfba..442c0730ef32 100644 >> --- a/OvmfPkg/OvmfXen.dsc >> +++ b/OvmfPkg/OvmfXen.dsc >> @@ -544,6 +544,7 @@ [Components] >> >> PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf >> + PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> } >> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf { >> > > (5) Please keep the PciHostBridgeUtilityLib class resolution right next > to the PciHostBridgeLib one. > > >> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> index 6ec9ec751478..7d01528c94f1 100644 >> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> @@ -41,6 +41,7 @@ [LibraryClasses] >> MemoryAllocationLib >> PciLib >> QemuFwCfgLib >> + PciHostBridgeUtilityLib >> >> [Pcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase > > (6) Please keep the entries under [LibraryClasses] alphabetically > sorted. > > >> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> new file mode 100644 >> index 000000000000..c88ab8e4155d >> --- /dev/null >> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> @@ -0,0 +1,49 @@ >> +## @file >> +# OVMF and Arm's instance of the PCI Host Bridge Utility Library. > > (7) I suggest the following description: > > Provide common utility functions to PciHostBridgeLib instances in > ArmVirtPkg and OvmfPkg. > > >> +# >> +# Copyright (C) 2016, Red Hat, Inc. >> +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
> > (8) It makes sense to duplicate the (C) notices from the original file > (namely "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf"), but > this is still a new file due to your contribution. > > Therefore, please add a proper Huawei copyright notice. > > >> +# >> +# SPDX-License-Identifier: BSD-2-Clause-Patent >> +# >> +# >> +## >> + >> +[Defines] >> + INF_VERSION = 0x00010005 > > (9) The latest specified INF version is 1.29, according to > ; > please write > > INF_VERSION = 1.29 > > >> + BASE_NAME = PciHostBridgeUtilityLib >> + FILE_GUID = e3aa5932-527a-42e7-86f5-81b144c7e5f1 >> + MODULE_TYPE = DXE_DRIVER >> + VERSION_STRING = 1.0 >> + LIBRARY_CLASS = PciHostBridgeUtilityLib >> + >> +# >> +# The following information is for reference only and not required by the build >> +# tools. >> +# >> +# VALID_ARCHITECTURES = IA32 X64 EBC >> +# >> + > > (10) EBC is gone, plus we'll want to plug this lib into > "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf", which > itself names AARCH64 and ARM for VALID_ARCHITECTURES. > > Therefore, please state "IA32 X64 AARCH64 ARM" here. > > >> +[Sources] >> + PciHostBridgeUtilityLib.c >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + OvmfPkg/OvmfPkg.dec >> + >> +[LibraryClasses] >> + BaseMemoryLib >> + DebugLib >> + DevicePathLib >> + MemoryAllocationLib >> + PciLib >> + QemuFwCfgLib > > (11) The list of required (consumed) library classes, along with the > library class header files included in "PciHostBridgeUtilityLib.c", > should be as minimal as possible (and alphabetically sorted). > > Specifically, you only need to keep "DebugLib"; everything else should > be removed, as far as I can tell. > > (We can extend the [LibraryClasses] section in later patches, if > necessary.) > > >> + >> +[Pcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId > > (12) The [Pcd] section should be dropped altogether. > > >> diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h >> new file mode 100644 >> index 000000000000..d2622fd907e6 >> --- /dev/null >> +++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h >> @@ -0,0 +1,33 @@ >> +/** @file >> + PCI Host Bridge Library consumed by PciHostBridgeDxe driver returning >> + the platform specific information about the PCI Host Bridge. > > (13) This file-top comment should say: > > Provide common utility functions to PciHostBridgeLib instances in > ArmVirtPkg and OvmfPkg. > > >> + >> + Copyright (c) 2016, Intel Corporation. All rights reserved.
> > (14) It's fine to keep the (C) notices from the file that you are using > as template, but you should please add a Huawei (C) notice as well. > > >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> +#ifndef __PCI_HOST_BRIDGE_UTILITY_LIB_H__ >> +#define __PCI_HOST_BRIDGE_UTILITY_LIB_H__ >> + >> +/** >> + Inform the platform that the resource conflict happens. >> + >> + @param HostBridgeHandle Handle of the Host Bridge. >> + @param Configuration Pointer to PCI I/O and PCI memory resource >> + descriptors. The Configuration contains the resources >> + for all the root bridges. The resource for each root >> + bridge is terminated with END descriptor and an >> + additional END is appended indicating the end of the >> + entire resources. The resource descriptor field >> + values follow the description in >> + EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL >> + .SubmitResources(). >> +**/ >> +VOID >> +EFIAPI >> +PciHostBridgeResourceConflict ( >> + EFI_HANDLE HostBridgeHandle, >> + VOID *Configuration >> + ); >> + > > (15) Please rename this function to > "PciHostBridgeUtilityResourceConflict". > > The idea is that no PciHostBridgeUtilityLib instance should directly > implement a PciHostBridgeLib API. PciHostBridgeUtilityLib can offer APIs > that are as "fat" as necessary, taking over as much work as possible, > but we should keep the namespaces isolated. > > > (16) In our helper library, the "HostBridgeHandle" parameter is not > used; please remove it from both the leading comment on the function, > and also from the function declaration. > > >> +#endif > > (17) Please append a comment to this line: > > // __PCI_HOST_BRIDGE_UTILITY_LIB_H__ > > >> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> index e850f7d183ee..1c8f465834f3 100644 >> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> @@ -22,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> #include "PciHostBridge.h" >> >> > > (18) Please keep the #include list of the Library class headers > alphabetically sorted. > > >> @@ -33,12 +34,6 @@ typedef struct { >> #pragma pack () >> >> >> -GLOBAL_REMOVE_IF_UNREFERENCED >> -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { >> - L"Mem", L"I/O", L"Bus" >> -}; >> - >> - >> STATIC >> CONST >> OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH mRootBridgeDevicePathTemplate = { >> @@ -384,60 +379,3 @@ PciHostBridgeFreeRootBridges ( >> >> FreePool (Bridges); >> } >> - >> - >> -/** >> - Inform the platform that the resource conflict happens. >> - >> - @param HostBridgeHandle Handle of the Host Bridge. >> - @param Configuration Pointer to PCI I/O and PCI memory resource >> - descriptors. The Configuration contains the resources >> - for all the root bridges. The resource for each root >> - bridge is terminated with END descriptor and an >> - additional END is appended indicating the end of the >> - entire resources. The resource descriptor field >> - values follow the description in >> - EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL >> - .SubmitResources(). >> -**/ >> -VOID >> -EFIAPI >> -PciHostBridgeResourceConflict ( >> - EFI_HANDLE HostBridgeHandle, >> - VOID *Configuration >> - ) >> -{ >> - EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor; >> - UINTN RootBridgeIndex; >> - DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n")); >> - >> - RootBridgeIndex = 0; >> - Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration; >> - while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) { >> - DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++)); >> - for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) { >> - ASSERT (Descriptor->ResType < >> - ARRAY_SIZE (mPciHostBridgeLibAcpiAddressSpaceTypeStr) >> - ); >> - DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n", >> - mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType], >> - Descriptor->AddrLen, Descriptor->AddrRangeMax >> - )); >> - if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) { >> - DEBUG ((DEBUG_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n", >> - Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag, >> - ((Descriptor->SpecificFlag & >> - EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE >> - ) != 0) ? L" (Prefetchable)" : L"" >> - )); >> - } >> - } >> - // >> - // Skip the END descriptor for root bridge >> - // >> - ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR); >> - Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)( >> - (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1 >> - ); >> - } >> -} > > (19) Please do not remove the entire definition of this function. > According to my point (15), only replace the body of this function with > the following function call: > > PciHostBridgeUtilityResourceConflict (Configuration); > > >> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c >> new file mode 100644 >> index 000000000000..7e9512dc08f1 >> --- /dev/null >> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c >> @@ -0,0 +1,74 @@ >> +/** @file >> + OVMF's instance of the PCI Host Bridge Library. >> + >> + Copyright (c) 2020, Huawei Corporation. All rights reserved.
> > (20) Please preserve the (C) notices from the original file (from where > the code is coming); namely > "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c". > > >> + >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> +#include >> +#include > > (21) Is this #include list really sufficient to compile the tree when > only this patch is applied? > > I would think the following directive is needed, additionally: > > #include > > (I suggest placing it above the Library #includes.) > > >> + >> + >> +GLOBAL_REMOVE_IF_UNREFERENCED >> +CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { >> + L"Mem", L"I/O", L"Bus" >> +}; >> + >> + > > (22) Please rename this object to > "mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr" (you'll have to modify > the reference to it below as well). > > >> +/** >> + Inform the platform that the resource conflict happens. >> + >> + @param HostBridgeHandle Handle of the Host Bridge. >> + @param Configuration Pointer to PCI I/O and PCI memory resource >> + descriptors. The Configuration contains the resources >> + for all the root bridges. The resource for each root >> + bridge is terminated with END descriptor and an >> + additional END is appended indicating the end of the >> + entire resources. The resource descriptor field >> + values follow the description in >> + EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL >> + .SubmitResources(). >> +**/ >> +VOID >> +EFIAPI >> +PciHostBridgeResourceConflict ( > > (23) Please rename to "PciHostBridgeUtilityResourceConflict". > > (24) Please drop "HostBridgeHandle", corresponding to point (16). > > >> + EFI_HANDLE HostBridgeHandle, >> + VOID *Configuration >> + ) >> +{ >> + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor; >> + UINTN RootBridgeIndex; >> + DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n")); >> + >> + RootBridgeIndex = 0; >> + Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration; >> + while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) { >> + DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++)); >> + for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) { >> + ASSERT (Descriptor->ResType < >> + ARRAY_SIZE (mPciHostBridgeLibAcpiAddressSpaceTypeStr) >> + ); >> + DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n", >> + mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType], >> + Descriptor->AddrLen, Descriptor->AddrRangeMax >> + )); >> + if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) { >> + DEBUG ((DEBUG_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n", >> + Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag, >> + ((Descriptor->SpecificFlag & >> + EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE >> + ) != 0) ? L" (Prefetchable)" : L"" >> + )); >> + } >> + } >> + // >> + // Skip the END descriptor for root bridge >> + // >> + ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR); >> + Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)( >> + (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1 >> + ); >> + } >> +} >> + >> > > Thanks! > Laszlo > > . >