public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment
@ 2017-01-26  6:09 Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 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.


Ruiyu Ni (5):
  MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  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 -1 to match any IDs

 .../IncompatiblePciDeviceSupport.c                 | 97 +++++++++++-----------
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c       | 17 ++--
 MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c      |  2 +-
 MdePkg/Include/IndustryStandard/Pci22.h            | 16 ++--
 4 files changed, 67 insertions(+), 65 deletions(-)

-- 
2.9.0.windows.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
@ 2017-01-26  6:09 ` Ruiyu Ni
  2017-02-03  3:38   ` Gao, Liming
  2017-01-26  6:09 ` [PATCH 2/5] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, 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>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jeff Fan <jeff.fan@intel.com>
---
 MdePkg/Include/IndustryStandard/Pci22.h | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Pci22.h b/MdePkg/Include/IndustryStandard/Pci22.h
index 4cf8389..8f87b04 100644
--- a/MdePkg/Include/IndustryStandard/Pci22.h
+++ b/MdePkg/Include/IndustryStandard/Pci22.h
@@ -780,14 +780,18 @@ typedef struct {
   ///
 } EFI_PCI_CAPABILITY_HOTPLUG;
 
-#define DEVICE_ID_NOCARE    0xFFFF
+///
+/// Below macros (till PCI_BAR_ALL) were used by EfiIncompatiblePciDeviceSupport Protocol.
+/// Some of them don't match Spec or are not defined in Spec. Those are marked as deprecated.
+///
+#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't match Spec.
 
 #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_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_IDX0        0x00
 #define PCI_BAR_IDX1        0x01
@@ -795,7 +799,7 @@ typedef struct {
 #define PCI_BAR_IDX3        0x03
 #define PCI_BAR_IDX4        0x04
 #define PCI_BAR_IDX5        0x05
-#define PCI_BAR_ALL         0xFF
+#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match Spec.
 
 ///
 /// EFI PCI Option ROM definitions
-- 
2.9.0.windows.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL
  2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni
@ 2017-01-26  6:09 ` Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 3/5] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 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] 11+ messages in thread

* [PATCH 3/5] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment
  2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 2/5] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni
@ 2017-01-26  6:09 ` Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 4/5] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 5/5] MdeModulePkg/IncompatiblePci: Use -1 to match any IDs Ruiyu Ni
  4 siblings, 0 replies; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jeff Fan, Feng Tian

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>
Cc: Feng Tian <feng.tian@intel.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index ac4d323..4ddec1f 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
@@ -1335,8 +1335,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 +1390,19 @@ UpdatePciInfo (
       break;
     }
 
-    BarIndex    = (UINTN) Ptr->AddrTranslationOffset;
+    BarIndex    = Ptr->AddrTranslationOffset;
     BarEndIndex = BarIndex;
 
     //
     // Update all the bars in the device
+    // Compare against PCI_BAR_ALL is to keep backward compatibility.
     //
-    if (BarIndex == PCI_BAR_ALL) {
+    if ((BarIndex == (UINT64)-1) || (BarIndex == PCI_BAR_ALL)) {
       BarIndex    = 0;
       BarEndIndex = PCI_MAX_BAR - 1;
     }
 
-    if (BarIndex > PCI_MAX_BAR) {
+    if (BarIndex >= PCI_MAX_BAR) {
       Ptr++;
       continue;
     }
@@ -1472,7 +1473,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;
         }
       }
@@ -1506,7 +1507,7 @@ SetNewAlign (
   // The new alignment is the same as the original,
   // so skip it
   //
-  if (NewAlignment == PCI_BAR_OLD_ALIGN) {
+  if ((NewAlignment == 0) || (NewAlignment == PCI_BAR_OLD_ALIGN)) {
     return ;
   }
   //
-- 
2.9.0.windows.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros
  2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
                   ` (2 preceding siblings ...)
  2017-01-26  6:09 ` [PATCH 3/5] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni
@ 2017-01-26  6:09 ` Ruiyu Ni
  2017-01-26  6:09 ` [PATCH 5/5] MdeModulePkg/IncompatiblePci: Use -1 to match any IDs Ruiyu Ni
  4 siblings, 0 replies; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jeff Fan, Feng Tian

The patch replaces the following macros:
DEVICE_ID_NOCARE (0xFF) --> (UINT64)-1
PCI_ACPI_UNUSED (0) --> 0
PCI_BAR_ALL (0xFF) --> (UINT64)-1
PCI_BAR_NOCHANGE (0) --> 0

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>
Cc: Feng Tian <feng.tian@intel.com>
---
 .../IncompatiblePciDeviceSupport.c                 | 97 +++++++++++-----------
 1 file changed, 47 insertions(+), 50 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
index 3d581b6..dfb7a59 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,9 +48,6 @@ 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
@@ -114,72 +111,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, (UINT64)-1, (UINT64)-1, (UINT64)-1, (UINT64)-1),
   DEVICE_RES_TAG,
-  PCI_BAR_TYPE_IO,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
+  ACPI_ADDRESS_SPACE_TYPE_IO,
+  0,
+  0,
+  0,
+  0,
   PCI_BAR_EVEN_ALIGN,
-  PCI_BAR_ALL,
-  PCI_BAR_NOCHANGE,
+  (UINT64)-1,
+  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, (UINT64)-1, (UINT64)-1, (UINT64)-1, (UINT64)-1),
   DEVICE_RES_TAG,
-  PCI_BAR_TYPE_IO,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
+  ACPI_ADDRESS_SPACE_TYPE_IO,
+  0,
+  0,
+  0,
+  0,
   PCI_BAR_EVEN_ALIGN,
-  PCI_BAR_ALL,
-  PCI_BAR_NOCHANGE,
+  (UINT64)-1,
+  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, (UINT64)-1, (UINT64)-1, (UINT64)-1, (UINT64)-1),
   DEVICE_RES_TAG,
-  PCI_BAR_TYPE_IO,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
+  ACPI_ADDRESS_SPACE_TYPE_IO,
+  0,
+  0,
+  0,
+  0,
   PCI_BAR_EVEN_ALIGN,
-  PCI_BAR_ALL,
-  PCI_BAR_NOCHANGE,
+  (UINT64)-1,
+  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, (UINT64)-1, (UINT64)-1, (UINT64)-1, (UINT64)-1),
   DEVICE_RES_TAG,
-  PCI_BAR_TYPE_IO,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
+  ACPI_ADDRESS_SPACE_TYPE_IO,
+  0,
+  0,
+  0,
+  0,
   PCI_BAR_EVEN_ALIGN,
-  PCI_BAR_ALL,
-  PCI_BAR_NOCHANGE,
+  (UINT64)-1,
+  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, (UINT64)-1, (UINT64)-1, (UINT64)-1, (UINT64)-1),
   DEVICE_RES_TAG,
-  PCI_BAR_TYPE_IO,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
-  PCI_ACPI_UNUSED,
+  ACPI_ADDRESS_SPACE_TYPE_IO,
+  0,
+  0,
+  0,
+  0,
   PCI_BAR_EVEN_ALIGN,
-  PCI_BAR_ALL,
-  PCI_BAR_NOCHANGE,
+  (UINT64)-1,
+  0,
   //
   // The end of the list
   //
@@ -285,31 +282,31 @@ PCheckDevice (
       //
       // See if the Header matches the parameters passed in
       //
-      if (Header->VendorId != DEVICE_ID_NOCARE) {
+      if (Header->VendorId != (UINT64)-1) {
         if (Header->VendorId != VendorId) {
           continue;
         }
       }
 
-      if (Header->DeviceId != DEVICE_ID_NOCARE) {
+      if (Header->DeviceId != (UINT64)-1) {
         if (DeviceId != Header->DeviceId) {
           continue;
         }
       }
 
-      if (Header->RevisionId != DEVICE_ID_NOCARE) {
+      if (Header->RevisionId != (UINT64)-1) {
         if (RevisionId != Header->RevisionId) {
           continue;
         }
       }
 
-      if (Header->SubsystemVendorId != DEVICE_ID_NOCARE) {
+      if (Header->SubsystemVendorId != (UINT64)-1) {
         if (SubsystemVendorId != Header->SubsystemVendorId) {
           continue;
         }
       }
 
-      if (Header->SubsystemDeviceId != DEVICE_ID_NOCARE) {
+      if (Header->SubsystemDeviceId != (UINT64)-1) {
         if (SubsystemDeviceId != Header->SubsystemDeviceId) {
           continue;
         }
-- 
2.9.0.windows.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] MdeModulePkg/IncompatiblePci: Use -1 to match any IDs
  2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
                   ` (3 preceding siblings ...)
  2017-01-26  6:09 ` [PATCH 4/5] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni
@ 2017-01-26  6:09 ` Ruiyu Ni
  4 siblings, 0 replies; 11+ messages in thread
From: Ruiyu Ni @ 2017-01-26  6:09 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jeff Fan, Feng Tian

When the VendorId/DeviceId/RevisionId/SubsystemVendorId
/SubsystemDeviceId is (UINT64)-1, 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, (UINT64)-1 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>
Cc: 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 dfb7a59..333ed56 100644
--- a/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.c
@@ -282,31 +282,31 @@ PCheckDevice (
       //
       // See if the Header matches the parameters passed in
       //
-      if (Header->VendorId != (UINT64)-1) {
+      if ((Header->VendorId != (UINT64)-1) && (VendorId != (UINT64)-1)) {
         if (Header->VendorId != VendorId) {
           continue;
         }
       }
 
-      if (Header->DeviceId != (UINT64)-1) {
+      if ((Header->DeviceId != (UINT64)-1) && (DeviceId != (UINT64) -1)) {
         if (DeviceId != Header->DeviceId) {
           continue;
         }
       }
 
-      if (Header->RevisionId != (UINT64)-1) {
+      if ((Header->RevisionId != (UINT64)-1) && (RevisionId != (UINT64) -1)) {
         if (RevisionId != Header->RevisionId) {
           continue;
         }
       }
 
-      if (Header->SubsystemVendorId != (UINT64)-1) {
+      if ((Header->SubsystemVendorId != (UINT64)-1) && (SubsystemVendorId != (UINT64) -1)) {
         if (SubsystemVendorId != Header->SubsystemVendorId) {
           continue;
         }
       }
 
-      if (Header->SubsystemDeviceId != (UINT64)-1) {
+      if ((Header->SubsystemDeviceId != (UINT64)-1) && (SubsystemDeviceId != (UINT64) -1)) {
         if (SubsystemDeviceId != Header->SubsystemDeviceId) {
           continue;
         }
-- 
2.9.0.windows.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-01-26  6:09 ` [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni
@ 2017-02-03  3:38   ` Gao, Liming
  2017-02-03  8:13     ` Laszlo Ersek
  0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2017-02-03  3:38 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Fan, Jeff

Ray:
  How about wrap them by macro DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?

Thanks
Liming
>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Thursday, January 26, 2017 2:09 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>; Fan, Jeff <jeff.fan@intel.com>
>Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>IncompatiblePciDevice macros
>
>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>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Jeff Fan <jeff.fan@intel.com>
>---
> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>b/MdePkg/Include/IndustryStandard/Pci22.h
>index 4cf8389..8f87b04 100644
>--- a/MdePkg/Include/IndustryStandard/Pci22.h
>+++ b/MdePkg/Include/IndustryStandard/Pci22.h
>@@ -780,14 +780,18 @@ typedef struct {
>   ///
> } EFI_PCI_CAPABILITY_HOTPLUG;
>
>-#define DEVICE_ID_NOCARE    0xFFFF
>+///
>+/// Below macros (till PCI_BAR_ALL) were used by
>EfiIncompatiblePciDeviceSupport Protocol.
>+/// Some of them don't match Spec or are not defined in Spec. Those are
>marked as deprecated.
>+///
>+#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't
>match Spec.
>
> #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_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_IDX0        0x00
> #define PCI_BAR_IDX1        0x01
>@@ -795,7 +799,7 @@ typedef struct {
> #define PCI_BAR_IDX3        0x03
> #define PCI_BAR_IDX4        0x04
> #define PCI_BAR_IDX5        0x05
>-#define PCI_BAR_ALL         0xFF
>+#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match Spec.
>
> ///
> /// EFI PCI Option ROM definitions
>--
>2.9.0.windows.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-02-03  3:38   ` Gao, Liming
@ 2017-02-03  8:13     ` Laszlo Ersek
  2017-02-03  8:28       ` Ni, Ruiyu
  0 siblings, 1 reply; 11+ messages in thread
From: Laszlo Ersek @ 2017-02-03  8:13 UTC (permalink / raw)
  To: Gao, Liming, Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Fan, Jeff

On 02/03/17 04:38, Gao, Liming wrote:
> Ray:
>   How about wrap them by macro DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?

OvmfPkg uses some of these macros, and also defines
DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
followed, then a conversion patch for OvmfPkg becomes necessary as first
step, to the new values.

Thanks
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: Ni, Ruiyu
>> Sent: Thursday, January 26, 2017 2:09 PM
>> To: edk2-devel@lists.01.org
>> Cc: Gao, Liming <liming.gao@intel.com>; Fan, Jeff <jeff.fan@intel.com>
>> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> 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>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> ---
>> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> b/MdePkg/Include/IndustryStandard/Pci22.h
>> index 4cf8389..8f87b04 100644
>> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> @@ -780,14 +780,18 @@ typedef struct {
>>   ///
>> } EFI_PCI_CAPABILITY_HOTPLUG;
>>
>> -#define DEVICE_ID_NOCARE    0xFFFF
>> +///
>> +/// Below macros (till PCI_BAR_ALL) were used by
>> EfiIncompatiblePciDeviceSupport Protocol.
>> +/// Some of them don't match Spec or are not defined in Spec. Those are
>> marked as deprecated.
>> +///
>> +#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't
>> match Spec.
>>
>> #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_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_IDX0        0x00
>> #define PCI_BAR_IDX1        0x01
>> @@ -795,7 +799,7 @@ typedef struct {
>> #define PCI_BAR_IDX3        0x03
>> #define PCI_BAR_IDX4        0x04
>> #define PCI_BAR_IDX5        0x05
>> -#define PCI_BAR_ALL         0xFF
>> +#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match Spec.
>>
>> ///
>> /// EFI PCI Option ROM definitions
>> --
>> 2.9.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-02-03  8:13     ` Laszlo Ersek
@ 2017-02-03  8:28       ` Ni, Ruiyu
  2017-02-03  8:36         ` Gao, Liming
  0 siblings, 1 reply; 11+ messages in thread
From: Ni, Ruiyu @ 2017-02-03  8:28 UTC (permalink / raw)
  To: Laszlo Ersek, Gao, Liming, edk2-devel@lists.01.org; +Cc: Fan, Jeff

Laszlo,
Sure I will make sure OVMF build is fine with this change.

Liming,
The three deprecated macros PCI_BAR_EVEN_ALIGN,
PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have replacement.
If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
What's your opinion about this driver?


Thanks/Ray

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 3, 2017 4:13 PM
> To: Gao, Liming <liming.gao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
> Cc: Fan, Jeff <jeff.fan@intel.com>
> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
> IncompatiblePciDevice macros
> 
> On 02/03/17 04:38, Gao, Liming wrote:
> > Ray:
> >   How about wrap them by macro
> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
> 
> OvmfPkg uses some of these macros, and also defines
> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
> followed, then a conversion patch for OvmfPkg becomes necessary as first
> step, to the new values.
> 
> Thanks
> Laszlo
> 
> >
> > Thanks
> > Liming
> >> -----Original Message-----
> >> From: Ni, Ruiyu
> >> Sent: Thursday, January 26, 2017 2:09 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Gao, Liming <liming.gao@intel.com>; Fan, Jeff
> >> <jeff.fan@intel.com>
> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
> >> IncompatiblePciDevice macros
> >>
> >> 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>
> >> Cc: Liming Gao <liming.gao@intel.com>
> >> Cc: Jeff Fan <jeff.fan@intel.com>
> >> ---
> >> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++++++++++------
> >> 1 file changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
> >> b/MdePkg/Include/IndustryStandard/Pci22.h
> >> index 4cf8389..8f87b04 100644
> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
> >> @@ -780,14 +780,18 @@ typedef struct {
> >>   ///
> >> } EFI_PCI_CAPABILITY_HOTPLUG;
> >>
> >> -#define DEVICE_ID_NOCARE    0xFFFF
> >> +///
> >> +/// Below macros (till PCI_BAR_ALL) were used by
> >> EfiIncompatiblePciDeviceSupport Protocol.
> >> +/// Some of them don't match Spec or are not defined in Spec. Those
> >> +are
> >> marked as deprecated.
> >> +///
> >> +#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't
> >> match Spec.
> >>
> >> #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_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_IDX0        0x00
> >> #define PCI_BAR_IDX1        0x01
> >> @@ -795,7 +799,7 @@ typedef struct {
> >> #define PCI_BAR_IDX3        0x03
> >> #define PCI_BAR_IDX4        0x04
> >> #define PCI_BAR_IDX5        0x05
> >> -#define PCI_BAR_ALL         0xFF
> >> +#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match
> Spec.
> >>
> >> ///
> >> /// EFI PCI Option ROM definitions
> >> --
> >> 2.9.0.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-02-03  8:28       ` Ni, Ruiyu
@ 2017-02-03  8:36         ` Gao, Liming
  2017-02-03  9:03           ` Ni, Ruiyu
  0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2017-02-03  8:36 UTC (permalink / raw)
  To: Ni, Ruiyu, Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Fan, Jeff

Ray:
  If they are not recommended to be used any longer, I suggest to update IncompatiblePciDeviceSupportDxe driver not use them. The driver can have its incompatible definition. 

Thanks
Liming
>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Friday, February 03, 2017 4:29 PM
>To: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>;
>edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
>Cc: Fan, Jeff <jeff.fan@intel.com>
>Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>IncompatiblePciDevice macros
>
>Laszlo,
>Sure I will make sure OVMF build is fine with this change.
>
>Liming,
>The three deprecated macros PCI_BAR_EVEN_ALIGN,
>PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have
>replacement.
>If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
>MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
>What's your opinion about this driver?
>
>
>Thanks/Ray
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, February 3, 2017 4:13 PM
>> To: Gao, Liming <liming.gao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
>> edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
>> Cc: Fan, Jeff <jeff.fan@intel.com>
>> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> On 02/03/17 04:38, Gao, Liming wrote:
>> > Ray:
>> >   How about wrap them by macro
>> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
>>
>> OvmfPkg uses some of these macros, and also defines
>> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
>> followed, then a conversion patch for OvmfPkg becomes necessary as first
>> step, to the new values.
>>
>> Thanks
>> Laszlo
>>
>> >
>> > Thanks
>> > Liming
>> >> -----Original Message-----
>> >> From: Ni, Ruiyu
>> >> Sent: Thursday, January 26, 2017 2:09 PM
>> >> To: edk2-devel@lists.01.org
>> >> Cc: Gao, Liming <liming.gao@intel.com>; Fan, Jeff
>> >> <jeff.fan@intel.com>
>> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> >> IncompatiblePciDevice macros
>> >>
>> >> 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>
>> >> Cc: Liming Gao <liming.gao@intel.com>
>> >> Cc: Jeff Fan <jeff.fan@intel.com>
>> >> ---
>> >> MdePkg/Include/IndustryStandard/Pci22.h | 16 ++++++++++------
>> >> 1 file changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> index 4cf8389..8f87b04 100644
>> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> @@ -780,14 +780,18 @@ typedef struct {
>> >>   ///
>> >> } EFI_PCI_CAPABILITY_HOTPLUG;
>> >>
>> >> -#define DEVICE_ID_NOCARE    0xFFFF
>> >> +///
>> >> +/// Below macros (till PCI_BAR_ALL) were used by
>> >> EfiIncompatiblePciDeviceSupport Protocol.
>> >> +/// Some of them don't match Spec or are not defined in Spec. Those
>> >> +are
>> >> marked as deprecated.
>> >> +///
>> >> +#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't
>> >> match Spec.
>> >>
>> >> #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_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_IDX0        0x00
>> >> #define PCI_BAR_IDX1        0x01
>> >> @@ -795,7 +799,7 @@ typedef struct {
>> >> #define PCI_BAR_IDX3        0x03
>> >> #define PCI_BAR_IDX4        0x04
>> >> #define PCI_BAR_IDX5        0x05
>> >> -#define PCI_BAR_ALL         0xFF
>> >> +#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match
>> Spec.
>> >>
>> >> ///
>> >> /// EFI PCI Option ROM definitions
>> >> --
>> >> 2.9.0.windows.1
>> >
>> > _______________________________________________
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>> >



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros
  2017-02-03  8:36         ` Gao, Liming
@ 2017-02-03  9:03           ` Ni, Ruiyu
  0 siblings, 0 replies; 11+ messages in thread
From: Ni, Ruiyu @ 2017-02-03  9:03 UTC (permalink / raw)
  To: Gao, Liming, Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Fan, Jeff

yes. That's a good idea to move the definitions to local.

Regards,
Ray

From: Gao, Liming
Sent: Friday, February 3, 2017 4:36 PM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>
Cc: Fan, Jeff <jeff.fan@intel.com>
Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros

Ray:
  If they are not recommended to be used any longer, I suggest to update IncompatiblePciDeviceSupportDxe driver not use them. The driver can have its incompatible definition.

Thanks
Liming
>-----Original Message-----
>From: Ni, Ruiyu
>Sent: Friday, February 03, 2017 4:29 PM
>To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>;
>edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> <edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>>
>Cc: Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
>Subject: RE: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>IncompatiblePciDevice macros
>
>Laszlo,
>Sure I will make sure OVMF build is fine with this change.
>
>Liming,
>The three deprecated macros PCI_BAR_EVEN_ALIGN,
>PCI_BAR_SQUAD_ALIGN and PCI_BAR_DQUAD_ALIGN don't have
>replacement.
>If wrapping them with DISABLE_NEW_DEPRECATED_INTERFACES,
>MdeModulePkg/Bus/Pci/IncompatiblePciDeviceSupportDxe build will break.
>What's your opinion about this driver?
>
>
>Thanks/Ray
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, February 3, 2017 4:13 PM
>> To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>;
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> <edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>>
>> Cc: Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
>> Subject: Re: [edk2] [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> IncompatiblePciDevice macros
>>
>> On 02/03/17 04:38, Gao, Liming wrote:
>> > Ray:
>> >   How about wrap them by macro
>> DISABLE_NEW_DEPRECATED_INTERFACES to avoid them be used any more?
>>
>> OvmfPkg uses some of these macros, and also defines
>> DISABLE_NEW_DEPRECATED_INTERFACES. So if the above suggestion is
>> followed, then a conversion patch for OvmfPkg becomes necessary as first
>> step, to the new values.
>>
>> Thanks
>> Laszlo
>>
>> >
>> > Thanks
>> > Liming
>> >> -----Original Message-----
>> >> From: Ni, Ruiyu
>> >> Sent: Thursday, January 26, 2017 2:09 PM
>> >> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> >> Cc: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Fan, Jeff
>> >> <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>
>> >> Subject: [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec
>> >> IncompatiblePciDevice macros
>> >>
>> >> 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>>
>> >> Cc: 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 | 16 ++++++++++------
>> >> 1 file changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> index 4cf8389..8f87b04 100644
>> >> --- a/MdePkg/Include/IndustryStandard/Pci22.h
>> >> +++ b/MdePkg/Include/IndustryStandard/Pci22.h
>> >> @@ -780,14 +780,18 @@ typedef struct {
>> >>   ///
>> >> } EFI_PCI_CAPABILITY_HOTPLUG;
>> >>
>> >> -#define DEVICE_ID_NOCARE    0xFFFF
>> >> +///
>> >> +/// Below macros (till PCI_BAR_ALL) were used by
>> >> EfiIncompatiblePciDeviceSupport Protocol.
>> >> +/// Some of them don't match Spec or are not defined in Spec. Those
>> >> +are
>> >> marked as deprecated.
>> >> +///
>> >> +#define DEVICE_ID_NOCARE    0xFFFF  ///< Deprecated. Value doesn't
>> >> match Spec.
>> >>
>> >> #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_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_IDX0        0x00
>> >> #define PCI_BAR_IDX1        0x01
>> >> @@ -795,7 +799,7 @@ typedef struct {
>> >> #define PCI_BAR_IDX3        0x03
>> >> #define PCI_BAR_IDX4        0x04
>> >> #define PCI_BAR_IDX5        0x05
>> >> -#define PCI_BAR_ALL         0xFF
>> >> +#define PCI_BAR_ALL         0xFF    ///< Deprecated. Value doesn't match
>> Spec.
>> >>
>> >> ///
>> >> /// EFI PCI Option ROM definitions
>> >> --
>> >> 2.9.0.windows.1
>> >
>> > _______________________________________________
>> > 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] 11+ messages in thread

end of thread, other threads:[~2017-02-03  9:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-26  6:09 [PATCH 0/5] Fix PciBus to accept Spec values as BarIndex and Alignment Ruiyu Ni
2017-01-26  6:09 ` [PATCH 1/5] MdePkg/Pci22.h: Deprecate out-of-Spec IncompatiblePciDevice macros Ruiyu Ni
2017-02-03  3:38   ` Gao, Liming
2017-02-03  8:13     ` Laszlo Ersek
2017-02-03  8:28       ` Ni, Ruiyu
2017-02-03  8:36         ` Gao, Liming
2017-02-03  9:03           ` Ni, Ruiyu
2017-01-26  6:09 ` [PATCH 2/5] MdeModulePkg/PciSioSerialDxe: Use MAX_UINT8 instead of PCI_BAR_ALL Ruiyu Ni
2017-01-26  6:09 ` [PATCH 3/5] MdeModulePkg/PciBus: Accept Spec values as BarIndex and Alignment Ruiyu Ni
2017-01-26  6:09 ` [PATCH 4/5] MdeModulePkg/IncompatiblePciDevice: Do not use deprecated macros Ruiyu Ni
2017-01-26  6:09 ` [PATCH 5/5] MdeModulePkg/IncompatiblePci: Use -1 to match any IDs Ruiyu Ni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox