* [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment @ 2017-02-07 3:32 Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 1/6] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni ` (5 more replies) 0 siblings, 6 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:32 UTC (permalink / raw) To: edk2-devel If a platform developer follows the PI spec to write an IncompatiblePciDeviceSupport driver, due to a spec complaince bug in PciBus driver, the IncompatiblePciDeviceSupport driver may not work as expected. The patches fix PciBus to follow Spec to accept Spec defined values. v2: Use DISABLE_NEW_DEPRECATED_INTERFACES to deprecate macros. v3: Use MAX_UINT64/MAX_UINTN to replace (UINT64)-1 and (UINTN)-1. Ruiyu Ni (6): MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs OvmfPkg/IncompatiblePci: Do not use deprecated macros MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros .../IncompatiblePciDeviceSupport.c | 108 ++++++++++----------- .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 36 ++++--- MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 2 +- MdePkg/Include/IndustryStandard/Pci22.h | 24 +++-- .../IncompatiblePciDeviceSupport.c | 13 +-- 5 files changed, 100 insertions(+), 83 deletions(-) -- 2.9.0.windows.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/6] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni ` (4 subsequent siblings) 5 siblings, 0 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Feng Tian When BarIndex equals to 0xFF, default value 0 is used as the BAR index. Though PCI_BAR_ALL and MAX_UINT8 shares the same value, using PCI_BAR_ALL is like to match any BAR not BAR 0, it's more proper to use MAX_UINT8 here. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Feng Tian <feng.tian@intel.com> --- MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c index 65ddf5d..a9dc827 100644 --- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c +++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c @@ -473,7 +473,7 @@ CreateSerialDevice ( // For PCI serial device, use the information from PCD // if (PciSerialParameter != NULL) { - BarIndex = (PciSerialParameter->BarIndex == PCI_BAR_ALL) ? 0 : PciSerialParameter->BarIndex; + BarIndex = (PciSerialParameter->BarIndex == MAX_UINT8) ? 0 : PciSerialParameter->BarIndex; Offset = PciSerialParameter->Offset; if (PciSerialParameter->RegisterStride != 0) { SerialDevice->RegisterStride = PciSerialParameter->RegisterStride; -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 1/6] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 3/6] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni ` (3 subsequent siblings) 5 siblings, 0 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Jeff Fan PI spec IncompatiblePciSupport part defines (UINT64)-1 as all BARs and 0 to use existing alignment. PciBus driver didn't accept these values. It treated 0xFF as all BARs and 0xFFFFFFFFFFFFFFFFULL to use existing alignment. The patch changes the code to still accept old values while also accept values defined in PI spec. So that the driver can provide backward compatibility and follow spec. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jeff Fan <jeff.fan@intel.com> Reviewed-by: Feng Tian <feng.tian@intel.com> --- .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 36 +++++++++++++--------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c index ac4d323..ecda088 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c @@ -1,7 +1,7 @@ /** @file PCI emumeration support functions implementation for PCI Bus module. -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -17,6 +17,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. extern CHAR16 *mBarTypeStr[]; +#define OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL +#define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL +#define SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL +#define DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL + /** This routine is used to check whether the pci device is present. @@ -1335,8 +1340,8 @@ UpdatePciInfo ( ) { EFI_STATUS Status; - UINTN BarIndex; - UINTN BarEndIndex; + UINT64 BarIndex; + UINT64 BarEndIndex; BOOLEAN SetFlag; VOID *Configuration; EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Ptr; @@ -1390,18 +1395,19 @@ UpdatePciInfo ( break; } - BarIndex = (UINTN) Ptr->AddrTranslationOffset; + BarIndex = Ptr->AddrTranslationOffset; BarEndIndex = BarIndex; // // Update all the bars in the device + // Compare against 0xFF is to keep backward compatibility. // - if (BarIndex == PCI_BAR_ALL) { + if ((BarIndex == MAX_UINT64) || (BarIndex == 0xFF)) { BarIndex = 0; BarEndIndex = PCI_MAX_BAR - 1; } - if (BarIndex > PCI_MAX_BAR) { + if (BarIndex >= PCI_MAX_BAR) { Ptr++; continue; } @@ -1472,7 +1478,7 @@ UpdatePciInfo ( // // Update the new length for the device // - if (Ptr->AddrLen != PCI_BAR_NOCHANGE) { + if (Ptr->AddrLen != 0) { PciIoDevice->PciBar[BarIndex].Length = Ptr->AddrLen; } } @@ -1488,6 +1494,8 @@ UpdatePciInfo ( /** This routine will update the alignment with the new alignment. + Compare with OLD_ALIGN/EVEN_ALIGN/SQUAD_ALIGN/DQUAD_ALIGN is to keep + backward compatibility. @param Alignment Input Old alignment. Output updated alignment. @param NewAlignment New alignment. @@ -1506,15 +1514,15 @@ SetNewAlign ( // The new alignment is the same as the original, // so skip it // - if (NewAlignment == PCI_BAR_OLD_ALIGN) { + if ((NewAlignment == 0) || (NewAlignment == OLD_ALIGN)) { return ; } // // Check the validity of the parameter // - if (NewAlignment != PCI_BAR_EVEN_ALIGN && - NewAlignment != PCI_BAR_SQUAD_ALIGN && - NewAlignment != PCI_BAR_DQUAD_ALIGN ) { + if (NewAlignment != EVEN_ALIGN && + NewAlignment != SQUAD_ALIGN && + NewAlignment != DQUAD_ALIGN ) { *Alignment = NewAlignment; return ; } @@ -1533,15 +1541,15 @@ SetNewAlign ( // // Adjust the alignment to even, quad or double quad boundary // - if (NewAlignment == PCI_BAR_EVEN_ALIGN) { + if (NewAlignment == EVEN_ALIGN) { if ((OldAlignment & 0x01) != 0) { OldAlignment = OldAlignment + 2 - (OldAlignment & 0x01); } - } else if (NewAlignment == PCI_BAR_SQUAD_ALIGN) { + } else if (NewAlignment == SQUAD_ALIGN) { if ((OldAlignment & 0x03) != 0) { OldAlignment = OldAlignment + 4 - (OldAlignment & 0x03); } - } else if (NewAlignment == PCI_BAR_DQUAD_ALIGN) { + } else if (NewAlignment == DQUAD_ALIGN) { if ((OldAlignment & 0x07) != 0) { OldAlignment = OldAlignment + 8 - (OldAlignment & 0x07); } -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 3/6] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 1/6] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 4/6] MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs Ruiyu Ni ` (2 subsequent siblings) 5 siblings, 0 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Jeff Fan The patch replaces the following macros: DEVICE_ID_NOCARE (0xFF) --> MAX_UINT64 PCI_ACPI_UNUSED (0) --> 0 PCI_BAR_ALL (0xFF) --> MAX_UINT64 PCI_BAR_NOCHANGE (0) --> 0 PCI_BAR_EVEN_ALIGN --> EVEN_ALIGN (local definition) Since the PciBus driver was updated to accept Spec defined values in previous commit, the above replacements don't impact functionality. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jeff Fan <jeff.fan@intel.com> Reviewed-by: Feng Tian <feng.tian@intel.com> --- .../IncompatiblePciDeviceSupport.c | 108 ++++++++++----------- 1 file changed, 53 insertions(+), 55 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c index 3d581b6..bf0d47a 100644 --- a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c +++ b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c @@ -1,11 +1,11 @@ /** @file This module is one template module for Incompatible PCI Device Support protocol. - It includes one incompatile pci devices list template. + It includes one incompatible pci devices list template. Incompatible PCI Device Support protocol allows the PCI bus driver to support resource allocation for some PCI devices that do not comply with the PCI Specification. -Copyright (c) 2009, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2009 - 2017, Intel Corporation. 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 @@ -48,13 +48,11 @@ typedef struct { #define PCI_DEVICE_ID(VendorId, DeviceId, Revision, SubVendorId, SubDeviceId) \ VendorId, DeviceId, Revision, SubVendorId, SubDeviceId -#define PCI_BAR_TYPE_IO ACPI_ADDRESS_SPACE_TYPE_IO -#define PCI_BAR_TYPE_MEM ACPI_ADDRESS_SPACE_TYPE_MEM - #define DEVICE_INF_TAG 0xFFF2 #define DEVICE_RES_TAG 0xFFF1 #define LIST_END_TAG 0x0000 +#define EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL /** Returns a list of ACPI resource descriptors that detail the special @@ -114,72 +112,72 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINT64 mIncompatiblePciDeviceList[] = { // Device Adaptec 9004 // DEVICE_INF_TAG, - PCI_DEVICE_ID(0x9004, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), + PCI_DEVICE_ID(0x9004, MAX_UINT64, MAX_UINT64, MAX_UINT64, MAX_UINT64), DEVICE_RES_TAG, - PCI_BAR_TYPE_IO, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_BAR_EVEN_ALIGN, - PCI_BAR_ALL, - PCI_BAR_NOCHANGE, + ACPI_ADDRESS_SPACE_TYPE_IO, + 0, + 0, + 0, + 0, + EVEN_ALIGN, + MAX_UINT64, + 0, // // Device Adaptec 9005 // DEVICE_INF_TAG, - PCI_DEVICE_ID(0x9005, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), + PCI_DEVICE_ID(0x9005, MAX_UINT64, MAX_UINT64, MAX_UINT64, MAX_UINT64), DEVICE_RES_TAG, - PCI_BAR_TYPE_IO, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_BAR_EVEN_ALIGN, - PCI_BAR_ALL, - PCI_BAR_NOCHANGE, + ACPI_ADDRESS_SPACE_TYPE_IO, + 0, + 0, + 0, + 0, + EVEN_ALIGN, + MAX_UINT64, + 0, // // Device QLogic 1007 // DEVICE_INF_TAG, - PCI_DEVICE_ID(0x1077, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), + PCI_DEVICE_ID(0x1077, MAX_UINT64, MAX_UINT64, MAX_UINT64, MAX_UINT64), DEVICE_RES_TAG, - PCI_BAR_TYPE_IO, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_BAR_EVEN_ALIGN, - PCI_BAR_ALL, - PCI_BAR_NOCHANGE, + ACPI_ADDRESS_SPACE_TYPE_IO, + 0, + 0, + 0, + 0, + EVEN_ALIGN, + MAX_UINT64, + 0, // // Device Agilent 103C // DEVICE_INF_TAG, - PCI_DEVICE_ID(0x103C, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), + PCI_DEVICE_ID(0x103C, MAX_UINT64, MAX_UINT64, MAX_UINT64, MAX_UINT64), DEVICE_RES_TAG, - PCI_BAR_TYPE_IO, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_BAR_EVEN_ALIGN, - PCI_BAR_ALL, - PCI_BAR_NOCHANGE, + ACPI_ADDRESS_SPACE_TYPE_IO, + 0, + 0, + 0, + 0, + EVEN_ALIGN, + MAX_UINT64, + 0, // // Device Agilent 15BC // DEVICE_INF_TAG, - PCI_DEVICE_ID(0x15BC, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE, DEVICE_ID_NOCARE), + PCI_DEVICE_ID(0x15BC, MAX_UINT64, MAX_UINT64, MAX_UINT64, MAX_UINT64), DEVICE_RES_TAG, - PCI_BAR_TYPE_IO, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_ACPI_UNUSED, - PCI_BAR_EVEN_ALIGN, - PCI_BAR_ALL, - PCI_BAR_NOCHANGE, + ACPI_ADDRESS_SPACE_TYPE_IO, + 0, + 0, + 0, + 0, + EVEN_ALIGN, + MAX_UINT64, + 0, // // The end of the list // @@ -285,31 +283,31 @@ PCheckDevice ( // // See if the Header matches the parameters passed in // - if (Header->VendorId != DEVICE_ID_NOCARE) { + if (Header->VendorId != MAX_UINT64) { if (Header->VendorId != VendorId) { continue; } } - if (Header->DeviceId != DEVICE_ID_NOCARE) { + if (Header->DeviceId != MAX_UINT64) { if (DeviceId != Header->DeviceId) { continue; } } - if (Header->RevisionId != DEVICE_ID_NOCARE) { + if (Header->RevisionId != MAX_UINT64) { if (RevisionId != Header->RevisionId) { continue; } } - if (Header->SubsystemVendorId != DEVICE_ID_NOCARE) { + if (Header->SubsystemVendorId != MAX_UINT64) { if (SubsystemVendorId != Header->SubsystemVendorId) { continue; } } - if (Header->SubsystemDeviceId != DEVICE_ID_NOCARE) { + if (Header->SubsystemDeviceId != MAX_UINT64) { if (SubsystemDeviceId != Header->SubsystemDeviceId) { continue; } -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 4/6] MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni ` (2 preceding siblings ...) 2017-02-07 3:33 ` [PATCH v3 3/6] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni 5 siblings, 0 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Jeff Fan When the VendorId/DeviceId/RevisionId/SubsystemVendorId /SubsystemDeviceId is MAX_UINTN, IncompatiblePciDeviceSupport driver doesn't use it to match any IDs. The patch fixes this bug. Since PciBus driver always calls IncompatiblePciDeviceSupport using IDs read from HW, MAX_UINTN is never passed to this driver. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jeff Fan <jeff.fan@intel.com> Reviewed-by: Feng Tian <feng.tian@intel.com> --- .../IncompatiblePciDeviceSupport.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c index bf0d47a..149662d 100644 --- a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c +++ b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c @@ -283,31 +283,31 @@ PCheckDevice ( // // See if the Header matches the parameters passed in // - if (Header->VendorId != MAX_UINT64) { + if ((Header->VendorId != MAX_UINT64) && (VendorId != MAX_UINTN)) { if (Header->VendorId != VendorId) { continue; } } - if (Header->DeviceId != MAX_UINT64) { + if ((Header->DeviceId != MAX_UINT64) && (DeviceId != MAX_UINTN)) { if (DeviceId != Header->DeviceId) { continue; } } - if (Header->RevisionId != MAX_UINT64) { + if ((Header->RevisionId != MAX_UINT64) && (RevisionId != MAX_UINTN)) { if (RevisionId != Header->RevisionId) { continue; } } - if (Header->SubsystemVendorId != MAX_UINT64) { + if ((Header->SubsystemVendorId != MAX_UINT64) && (SubsystemVendorId != MAX_UINTN)) { if (SubsystemVendorId != Header->SubsystemVendorId) { continue; } } - if (Header->SubsystemDeviceId != MAX_UINT64) { + if ((Header->SubsystemDeviceId != MAX_UINT64) && (SubsystemDeviceId != MAX_UINTN)) { if (SubsystemDeviceId != Header->SubsystemDeviceId) { continue; } -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni ` (3 preceding siblings ...) 2017-02-07 3:33 ` [PATCH v3 4/6] MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 17:40 ` Laszlo Ersek 2017-02-07 3:33 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni 5 siblings, 1 reply; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> --- .../IncompatiblePciDeviceSupport.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c index b6ff128..df9eb60 100644 --- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c +++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c @@ -4,6 +4,7 @@ is not present), conserving 32-bit MMIO aperture for 32-bit BARs. Copyright (C) 2016, Red Hat, Inc. + Copyright (c) 2017, Intel Corporation. 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 @@ -66,19 +67,19 @@ STATIC CONST MMIO64_PREFERENCE mConfiguration = { ) ), ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType - PCI_ACPI_UNUSED, // GenFlag - PCI_ACPI_UNUSED, // SpecificFlag + 0, // GenFlag + 0, // SpecificFlag 64, // AddrSpaceGranularity: // aperture selection hint // for BAR allocation - PCI_ACPI_UNUSED, // AddrRangeMin - PCI_BAR_OLD_ALIGN, // AddrRangeMax: + 0, // AddrRangeMin + 0, // AddrRangeMax: // no special alignment // for affected BARs - PCI_BAR_ALL, // AddrTranslationOffset: + MAX_UINT64, // AddrTranslationOffset: // hint covers all // eligible BARs - PCI_BAR_NOCHANGE // AddrLen: + 0 // AddrLen: // use probed BAR size }, // -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros 2017-02-07 3:33 ` [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros Ruiyu Ni @ 2017-02-07 17:40 ` Laszlo Ersek 0 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2017-02-07 17:40 UTC (permalink / raw) To: Ruiyu Ni, edk2-devel On 02/07/17 04:33, Ruiyu Ni wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > --- > .../IncompatiblePciDeviceSupport.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c > index b6ff128..df9eb60 100644 > --- a/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c > +++ b/OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c > @@ -4,6 +4,7 @@ > is not present), conserving 32-bit MMIO aperture for 32-bit BARs. > > Copyright (C) 2016, Red Hat, Inc. > + Copyright (c) 2017, Intel Corporation. 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 > @@ -66,19 +67,19 @@ STATIC CONST MMIO64_PREFERENCE mConfiguration = { > ) > ), > ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType > - PCI_ACPI_UNUSED, // GenFlag > - PCI_ACPI_UNUSED, // SpecificFlag > + 0, // GenFlag > + 0, // SpecificFlag > 64, // AddrSpaceGranularity: > // aperture selection hint > // for BAR allocation PCI_ACPI_UNUSED expands to 0, so this is fine. > - PCI_ACPI_UNUSED, // AddrRangeMin > - PCI_BAR_OLD_ALIGN, // AddrRangeMax: > + 0, // AddrRangeMin > + 0, // AddrRangeMax: > // no special alignment > // for affected BARs PCI_BAR_OLD_ALIGN used to expand to 0xFFFFFFFFFFFFFFFFULL. In patch #2, the condtion that checks PCI_BAR_OLD_ALIGN is extended to cover both 0 (employed here) and the new macro OLD_ALIGN (which is being introduced in the same patch #2, for the old compat value 0xFFFFFFFFFFFFFFFFULL). So this looks good too. > - PCI_BAR_ALL, // AddrTranslationOffset: > + MAX_UINT64, // AddrTranslationOffset: > // hint covers all > // eligible BARs PCI_BAR_ALL expands to 0xFF. Patch #2 updates PciBusDxe to cover both 0xFF and MAX_UINT64. Okay. However, I notice whitespace corruption here: the "//" to the right of MAX_UINT64 no longer lines up with the rest of the comments. > - PCI_BAR_NOCHANGE // AddrLen: > + 0 // AddrLen: > // use probed BAR size > }, > // > PCI_BAR_NOCHANGE expands to 0, so this is fine. Ray, please fix up the whitespace corruption on the MAX_UINT64 line, before committing the patch. With that fixed: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni ` (4 preceding siblings ...) 2017-02-07 3:33 ` [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros Ruiyu Ni @ 2017-02-07 3:33 ` Ruiyu Ni 2017-02-07 5:54 ` Bug 367 - Version Check for capsule update logic error wang xiaofeng 2017-02-07 17:43 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Laszlo Ersek 5 siblings, 2 replies; 12+ messages in thread From: Ruiyu Ni @ 2017-02-07 3:33 UTC (permalink / raw) To: edk2-devel; +Cc: Jeff Fan DEVICE_ID_NOCARE is defined as 0xFFFF but Spec says (UINT64) -1 should be used to match any VendorId/DeviceId/RevisionId/ SubsystemVendorId/SubsystemDeviceId. PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/ PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such definitions. PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be used to match all BARs. All of the above macros are marked as deprecated. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com> Cc: Jeff Fan <jeff.fan@intel.com> --- MdePkg/Include/IndustryStandard/Pci22.h | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/MdePkg/Include/IndustryStandard/Pci22.h b/MdePkg/Include/IndustryStandard/Pci22.h index 4cf8389..5dec65d 100644 --- a/MdePkg/Include/IndustryStandard/Pci22.h +++ b/MdePkg/Include/IndustryStandard/Pci22.h @@ -7,7 +7,7 @@ PC Card Standard, 8.0 PCI Power Management Interface Specifiction, Revision 1.2 - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR> + Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> Copyright (c) 2014 - 2015, Hewlett-Packard Development Company, L.P.<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -780,14 +780,25 @@ typedef struct { /// } EFI_PCI_CAPABILITY_HOTPLUG; -#define DEVICE_ID_NOCARE 0xFFFF +/// +/// Below macros (till PCI_BAR_NOCHANGE) were used by EfiIncompatiblePciDeviceSupport Protocol. +/// +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES + +/// +/// [ATTENTION] These macros are deprecated because they don't match Spec or not defined in Spec. +/// +#define DEVICE_ID_NOCARE 0xFFFF ///< Deprecated. Value doesn't match Spec. +#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL ///< Deprecated. Value isn't defined in Spec. +#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL ///< Deprecated. Value isn't defined in Spec. +#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL ///< Deprecated. Value isn't defined in Spec. +#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL ///< Deprecated. Value isn't defined in Spec. +#define PCI_BAR_ALL 0xFF ///< Deprecated. Value doesn't match Spec. + +#endif #define PCI_ACPI_UNUSED 0 #define PCI_BAR_NOCHANGE 0 -#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL -#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL -#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL -#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL #define PCI_BAR_IDX0 0x00 #define PCI_BAR_IDX1 0x01 @@ -795,7 +806,6 @@ typedef struct { #define PCI_BAR_IDX3 0x03 #define PCI_BAR_IDX4 0x04 #define PCI_BAR_IDX5 0x05 -#define PCI_BAR_ALL 0xFF /// /// EFI PCI Option ROM definitions -- 2.9.0.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Bug 367 - Version Check for capsule update logic error 2017-02-07 3:33 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni @ 2017-02-07 5:54 ` wang xiaofeng 2017-02-07 17:43 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Laszlo Ersek 1 sibling, 0 replies; 12+ messages in thread From: wang xiaofeng @ 2017-02-07 5:54 UTC (permalink / raw) To: edk2-devel, jiewen yao Hi Jiewen, Bug 367 is submitted for Version Check for capsule update logic error . In CapsuleAuthenticateSystemFirmware of SignedCapsulePkg\Library\EdkiiSystemCapsuleLib\EdkiiSystemCapsuleLib.c, the following logic seems error if (ForceVersionMatch) { if (CurrentImageFmpInfo->Version != ImageFmpInfo->Version) { *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION; DEBUG((DEBUG_INFO, "ForceVersionMatch check - fail\n")); return EFI_SECURITY_VIOLATION; } } else { if (CurrentImageFmpInfo->Version < ImageFmpInfo->LowestSupportedImageVersion) { *LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_INCORRECT_VERSION; DEBUG((DEBUG_INFO, "LowestSupportedImageVersion check - fail\n")); return EFI_SECURITY_VIOLATION; } } I think it should be CurrentImageFmpInfo->LowestSupportedImageVersion > ImageFmpInfo->Version \x16�& ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros 2017-02-07 3:33 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni 2017-02-07 5:54 ` Bug 367 - Version Check for capsule update logic error wang xiaofeng @ 2017-02-07 17:43 ` Laszlo Ersek 2017-02-08 0:58 ` Ni, Ruiyu 1 sibling, 1 reply; 12+ messages in thread From: Laszlo Ersek @ 2017-02-07 17:43 UTC (permalink / raw) To: Ruiyu Ni, edk2-devel; +Cc: Jeff Fan On 02/07/17 04:33, Ruiyu Ni wrote: > DEVICE_ID_NOCARE is defined as 0xFFFF but Spec says (UINT64) -1 > should be used to match any VendorId/DeviceId/RevisionId/ > SubsystemVendorId/SubsystemDeviceId. > > PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/ > PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such > definitions. > > PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be > used to match all BARs. > > All of the above macros are marked as deprecated. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> > Reviewed-by: Liming Gao <liming.gao@intel.com> > Cc: Jeff Fan <jeff.fan@intel.com> > --- > MdePkg/Include/IndustryStandard/Pci22.h | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/Pci22.h b/MdePkg/Include/IndustryStandard/Pci22.h > index 4cf8389..5dec65d 100644 > --- a/MdePkg/Include/IndustryStandard/Pci22.h > +++ b/MdePkg/Include/IndustryStandard/Pci22.h > @@ -7,7 +7,7 @@ > PC Card Standard, 8.0 > PCI Power Management Interface Specifiction, Revision 1.2 > > - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2014 - 2015, Hewlett-Packard Development Company, L.P.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -780,14 +780,25 @@ typedef struct { > /// > } EFI_PCI_CAPABILITY_HOTPLUG; > > -#define DEVICE_ID_NOCARE 0xFFFF > +/// > +/// Below macros (till PCI_BAR_NOCHANGE) were used by EfiIncompatiblePciDeviceSupport Protocol. > +/// > +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES > + > +/// > +/// [ATTENTION] These macros are deprecated because they don't match Spec or not defined in Spec. > +/// > +#define DEVICE_ID_NOCARE 0xFFFF ///< Deprecated. Value doesn't match Spec. > +#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_ALL 0xFF ///< Deprecated. Value doesn't match Spec. > + > +#endif > > #define PCI_ACPI_UNUSED 0 > #define PCI_BAR_NOCHANGE 0 Any particular reason for not deprecating these two macros? Is it an oversight, or is it intentional to keep them? (If they comply with the spec, then replacing them in patch #5 seems unwarranted.) Thanks, Laszlo > -#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL > -#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL > -#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL > -#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL > > #define PCI_BAR_IDX0 0x00 > #define PCI_BAR_IDX1 0x01 > @@ -795,7 +806,6 @@ typedef struct { > #define PCI_BAR_IDX3 0x03 > #define PCI_BAR_IDX4 0x04 > #define PCI_BAR_IDX5 0x05 > -#define PCI_BAR_ALL 0xFF > > /// > /// EFI PCI Option ROM definitions > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros 2017-02-07 17:43 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Laszlo Ersek @ 2017-02-08 0:58 ` Ni, Ruiyu 2017-02-08 1:09 ` Laszlo Ersek 0 siblings, 1 reply; 12+ messages in thread From: Ni, Ruiyu @ 2017-02-08 0:58 UTC (permalink / raw) To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Fan, Jeff These macros comply with the spec so I had no reason to deprecate them. But I did want to avoid using them in drivers. Maybe I could just deprecate the two macros PCI_ACPI_UNUSED and PCI_BAR_NOCHANGE. What do you think? Regards, Ray From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek Sent: Wednesday, February 8, 2017 1:43 AM To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@ml01.01.org Cc: Fan, Jeff <jeff.fan@intel.com> Subject: Re: [edk2] [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros On 02/07/17 04:33, Ruiyu Ni wrote: > DEVICE_ID_NOCARE is defined as 0xFFFF but Spec says (UINT64) -1 > should be used to match any VendorId/DeviceId/RevisionId/ > SubsystemVendorId/SubsystemDeviceId. > > PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/ > PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such > definitions. > > PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be > used to match all BARs. > > All of the above macros are marked as deprecated. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>> > Reviewed-by: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com>> > Cc: Jeff Fan <jeff.fan@intel.com<mailto:jeff.fan@intel.com>> > --- > MdePkg/Include/IndustryStandard/Pci22.h | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/Pci22.h b/MdePkg/Include/IndustryStandard/Pci22.h > index 4cf8389..5dec65d 100644 > --- a/MdePkg/Include/IndustryStandard/Pci22.h > +++ b/MdePkg/Include/IndustryStandard/Pci22.h > @@ -7,7 +7,7 @@ > PC Card Standard, 8.0 > PCI Power Management Interface Specifiction, Revision 1.2 > > - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > Copyright (c) 2014 - 2015, Hewlett-Packard Development Company, L.P.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -780,14 +780,25 @@ typedef struct { > /// > } EFI_PCI_CAPABILITY_HOTPLUG; > > -#define DEVICE_ID_NOCARE 0xFFFF > +/// > +/// Below macros (till PCI_BAR_NOCHANGE) were used by EfiIncompatiblePciDeviceSupport Protocol. > +/// > +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES > + > +/// > +/// [ATTENTION] These macros are deprecated because they don't match Spec or not defined in Spec. > +/// > +#define DEVICE_ID_NOCARE 0xFFFF ///< Deprecated. Value doesn't match Spec. > +#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL ///< Deprecated. Value isn't defined in Spec. > +#define PCI_BAR_ALL 0xFF ///< Deprecated. Value doesn't match Spec. > + > +#endif > > #define PCI_ACPI_UNUSED 0 > #define PCI_BAR_NOCHANGE 0 Any particular reason for not deprecating these two macros? Is it an oversight, or is it intentional to keep them? (If they comply with the spec, then replacing them in patch #5 seems unwarranted.) Thanks, Laszlo > -#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL > -#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL > -#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL > -#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL > > #define PCI_BAR_IDX0 0x00 > #define PCI_BAR_IDX1 0x01 > @@ -795,7 +806,6 @@ typedef struct { > #define PCI_BAR_IDX3 0x03 > #define PCI_BAR_IDX4 0x04 > #define PCI_BAR_IDX5 0x05 > -#define PCI_BAR_ALL 0xFF > > /// > /// EFI PCI Option ROM definitions > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros 2017-02-08 0:58 ` Ni, Ruiyu @ 2017-02-08 1:09 ` Laszlo Ersek 0 siblings, 0 replies; 12+ messages in thread From: Laszlo Ersek @ 2017-02-08 1:09 UTC (permalink / raw) To: Ni, Ruiyu, edk2-devel@ml01.01.org; +Cc: Fan, Jeff On 02/08/17 01:58, Ni, Ruiyu wrote: > These macros comply with the spec so I had no reason to deprecate them. > > But I did want to avoid using them in drivers. > > Maybe I could just deprecate the two macros PCI_ACPI_UNUSED and > PCI_BAR_NOCHANGE. > > What do you think? I'm fine either way: - deprecating these two macros (and keeping patch #5 as-is, modulo the whitespace fix) sounds good, - not deprecating these two macros (and then preserving them in OVMF too) sounds good as well. What I don't really understand is, what speaks against using the two macros if they are not deprecated / against the spec? But, again, if you deprecate these two as well, that's totally fine by me. Thanks! Laszlo > > > > Regards, > > Ray > > > > *From:*edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of > *Laszlo Ersek > *Sent:* Wednesday, February 8, 2017 1:43 AM > *To:* Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@ml01.01.org > *Cc:* Fan, Jeff <jeff.fan@intel.com> > *Subject:* Re: [edk2] [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate > out-of-Spec IncompatiblePciDevice macros > > > > On 02/07/17 04:33, Ruiyu Ni wrote: >> DEVICE_ID_NOCARE is defined as 0xFFFF but Spec says (UINT64) -1 >> should be used to match any VendorId/DeviceId/RevisionId/ >> SubsystemVendorId/SubsystemDeviceId. >> >> PCI_BAR_OLD_ALIGN/PCI_BAR_EVEN_ALIGN/PCI_BAR_SQUAD_ALIGN/ >> PCI_BAR_DQUAD_ALIGN are defined but Spec doesn't have such >> definitions. >> >> PCI_BAR_ALL is defined as 0xFF but Spec says (UINT64) -1 should be >> used to match all BARs. >> >> All of the above macros are marked as deprecated. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>> >> Reviewed-by: Liming Gao <liming.gao@intel.com <mailto:liming.gao@intel.com>> >> Cc: Jeff Fan <jeff.fan@intel.com <mailto:jeff.fan@intel.com>> >> --- >> MdePkg/Include/IndustryStandard/Pci22.h | 24 +++++++++++++++++------- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h b/MdePkg/Include/IndustryStandard/Pci22.h >> index 4cf8389..5dec65d 100644 >> --- a/MdePkg/Include/IndustryStandard/Pci22.h >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h >> @@ -7,7 +7,7 @@ >> PC Card Standard, 8.0 >> PCI Power Management Interface Specifiction, Revision 1.2 >> >> - Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR> >> + Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> >> Copyright (c) 2014 - 2015, Hewlett-Packard Development Company, L.P.<BR> >> This program and the accompanying materials >> are licensed and made available under the terms and conditions of the BSD License >> @@ -780,14 +780,25 @@ typedef struct { >> /// >> } EFI_PCI_CAPABILITY_HOTPLUG; >> >> -#define DEVICE_ID_NOCARE 0xFFFF >> +/// >> +/// Below macros (till PCI_BAR_NOCHANGE) were used by EfiIncompatiblePciDeviceSupport Protocol. >> +/// >> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES >> + >> +/// >> +/// [ATTENTION] These macros are deprecated because they don't match Spec or not defined in Spec. >> +/// >> +#define DEVICE_ID_NOCARE 0xFFFF ///< Deprecated. Value doesn't match Spec. >> +#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL ///< Deprecated. Value isn't defined in Spec. >> +#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL ///< Deprecated. Value isn't defined in Spec. >> +#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL ///< Deprecated. Value isn't defined in Spec. >> +#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL ///< Deprecated. Value isn't defined in Spec. >> +#define PCI_BAR_ALL 0xFF ///< Deprecated. Value doesn't match Spec. >> + >> +#endif >> >> #define PCI_ACPI_UNUSED 0 >> #define PCI_BAR_NOCHANGE 0 > > Any particular reason for not deprecating these two macros? Is it an > oversight, or is it intentional to keep them? > > (If they comply with the spec, then replacing them in patch #5 seems > unwarranted.) > > Thanks, > Laszlo > >> -#define PCI_BAR_OLD_ALIGN 0xFFFFFFFFFFFFFFFFULL >> -#define PCI_BAR_EVEN_ALIGN 0xFFFFFFFFFFFFFFFEULL >> -#define PCI_BAR_SQUAD_ALIGN 0xFFFFFFFFFFFFFFFDULL >> -#define PCI_BAR_DQUAD_ALIGN 0xFFFFFFFFFFFFFFFCULL >> >> #define PCI_BAR_IDX0 0x00 >> #define PCI_BAR_IDX1 0x01 >> @@ -795,7 +806,6 @@ typedef struct { >> #define PCI_BAR_IDX3 0x03 >> #define PCI_BAR_IDX4 0x04 >> #define PCI_BAR_IDX5 0x05 >> -#define PCI_BAR_ALL 0xFF >> >> /// >> /// EFI PCI Option ROM definitions >> > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org> > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-08 1:09 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-07 3:32 [PATCH v3 0/6] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 1/6] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 2/6] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 3/6] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 4/6] MdeModulePkg/IncompatiblePci: Use MAX_UINTN to match any IDs Ruiyu Ni 2017-02-07 3:33 ` [PATCH v3 5/6] OvmfPkg/IncompatiblePci: Do not use deprecated macros Ruiyu Ni 2017-02-07 17:40 ` Laszlo Ersek 2017-02-07 3:33 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni 2017-02-07 5:54 ` Bug 367 - Version Check for capsule update logic error wang xiaofeng 2017-02-07 17:43 ` [PATCH v3 6/6] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Laszlo Ersek 2017-02-08 0:58 ` Ni, Ruiyu 2017-02-08 1:09 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox