linux-kernel - RE: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: 
 <CY8PR19MB72753D2514337FCE78FFBAB1FC182@CY8PR19MB7275.namprd19.prod.outlook.com>
Date: Thu, 2 May 2024 22:01:02 +0000
From: "Thatchanamurthy, Satish" <Satish.Thatchanamurt@...l.com>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
CC: "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Bolen,
 Austin" <Austin.Bolen@...l.com>,
        "Arzola, Christopher"
	<Christopher.Arzola@...l.com>
Subject: RE: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware
 r3.3 spec

Hi Sathya/Bjorn,

Right, from Dell we did perform limited testing on one selected platform with this EDR patch and it did work.
But if need to be tested with multiple platforms then we need to test few other configs. Please advise

Thanks,
Satish


Internal Use - Confidential
-----Original Message-----
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
Sent: Tuesday, April 30, 2024 9:26 PM
To: Bjorn Helgaas <bhelgaas@...gle.com>
Cc: linux-pci@...r.kernel.org; linux-kernel@...r.kernel.org; Thatchanamurthy, Satish <Satish.Thatchanamurt@...l.com>
Subject: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec


[EXTERNAL EMAIL]

During the Error Disconnect Recover (EDR) spec transition from r3.2 ECN to PCI firmware spec r3.3, improvements were made to definitions of EDR_PORT_DPC_ENABLE_DSM (0x0C) and EDR_PORT_LOCATE_DSM(0x0D) _DSMs.

Specifically,

* EDR_PORT_DPC_ENABLE_DSM _DSM version changed from 5 to 6, and
  arg4 is now a package type instead of an integer in version 5.
* EDR_PORT_LOCATE_DSM _DSM uses BIT(31) to return the status of the
  operation.

Ensure _DSM implementation aligns with PCI firmware r3.3 spec recommendation. More details about the EDR_PORT_DPC_ENABLE_DSM and EDR_PORT_LOCATE_DSM _DSMs can be found in PCI firmware specification, r3.3, sec 4.6.12 and sec 4.6.13.

While at it, fix a typo in EDR_PORT_LOCATE_DSM comments.

Fixes: ac1c8e35a326 ("PCI/DPC: Add Error Disconnect Recover (EDR) support")
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
---
 drivers/pci/pcie/edr.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c index 5f4914d313a1..fea098e22e3e 100644
--- a/drivers/pci/pcie/edr.c
+++ b/drivers/pci/pcie/edr.c
@@ -35,7 +35,7 @@ static int acpi_enable_dpc(struct pci_dev *pdev)
         * Behavior when calling unsupported _DSM functions is undefined,
         * so check whether EDR_PORT_DPC_ENABLE_DSM is supported.
         */
-       if (!acpi_check_dsm(adev->handle, &pci_acpi_dsm_guid, 5,
+       if (!acpi_check_dsm(adev->handle, &pci_acpi_dsm_guid, 6,
                            1ULL << EDR_PORT_DPC_ENABLE_DSM))
                return 0;

@@ -47,11 +47,11 @@ static int acpi_enable_dpc(struct pci_dev *pdev)
        argv4.package.elements = &req;

        /*
-        * Per Downstream Port Containment Related Enhancements ECN to PCI
-        * Firmware Specification r3.2, sec 4.6.12, EDR_PORT_DPC_ENABLE_DSM is
-        * optional.  Return success if it's not implemented.
+        * Per PCI Firmware Specification r3.3, sec 4.6.12,
+        * EDR_PORT_DPC_ENABLE_DSM is optional. Return success if it's not
+        * implemented.
         */
-       obj = acpi_evaluate_dsm(adev->handle, &pci_acpi_dsm_guid, 5,
+       obj = acpi_evaluate_dsm(adev->handle, &pci_acpi_dsm_guid, 6,
                                EDR_PORT_DPC_ENABLE_DSM, &argv4);
        if (!obj)
                return 0;
@@ -86,7 +86,7 @@ static struct pci_dev *acpi_dpc_port_get(struct pci_dev *pdev)

        /*
         * Behavior when calling unsupported _DSM functions is undefined,
-        * so check whether EDR_PORT_DPC_ENABLE_DSM is supported.
+        * so check whether EDR_PORT_LOCATE_DSM is supported.
         */
        if (!acpi_check_dsm(adev->handle, &pci_acpi_dsm_guid, 5,
                            1ULL << EDR_PORT_LOCATE_DSM))
@@ -103,6 +103,17 @@ static struct pci_dev *acpi_dpc_port_get(struct pci_dev *pdev)
                return NULL;
        }

+       /*
+        * Per PCI Firmware Specification r3.3, sec 4.6.13, bit 31 represents
+        * the success/failure of the operation. If bit 31 is set, the operation
+        * is failed.
+        */
+       if (obj->integer.value & BIT(31)) {
+               ACPI_FREE(obj);
+               pci_err(pdev, "Locate Port _DSM failed\n");
+               return NULL;
+       }
+
        /*
         * Firmware returns DPC port BDF details in following format:
         *      15:8 = bus
--
2.25.1