1. Patchew
  2. linux
  3. View series

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

Kuppuswamy Sathyanarayanan posted 1 patch 1 year, 7 months ago 
drivers/pci/pcie/edr.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
[PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec
Posted by Kuppuswamy Sathyanarayanan 1 year, 7 months ago 
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@linux.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
Re: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec
Posted by Bjorn Helgaas 1 year, 7 months ago 
On Wed, May 01, 2024 at 02:25:43AM +0000, Kuppuswamy Sathyanarayanan wrote:
> 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@linux.intel.com>

I split this into two patches and applied them to pci/edr for v6.10,
thanks!

Take a look here:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=edr
and make sure I didn't mess it up (only differences are comments and
commit logs).

> ---
>  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
>
Re: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec
Posted by Kuppuswamy Sathyanarayanan 1 year, 7 months ago 
On 5/8/24 1:14 PM, Bjorn Helgaas wrote:
> On Wed, May 01, 2024 at 02:25:43AM +0000, Kuppuswamy Sathyanarayanan wrote:
>> 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@linux.intel.com>
> I split this into two patches and applied them to pci/edr for v6.10,
> thanks!
>
> Take a look here:
> https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=edr
> and make sure I didn't mess it up (only differences are comments and
> commit logs).
>

Thanks for doing it. It looks good to me.

>> ---
>>  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
>>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Re: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec
Posted by Bjorn Helgaas 1 year, 7 months ago 
On Wed, May 01, 2024 at 02:25:43AM +0000, Kuppuswamy Sathyanarayanan wrote:
> 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@linux.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))

How confident are we that this won't break any existing platforms?
Any idea how many platforms implement EDR_PORT_DPC_ENABLE_DSM and what
Revision IDs they support?

>  		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;
> +	}

This changes two _DSMs, and I think it should be two patches.

Same question here: we now depend on functionality we didn't depend on
before.  How confident are we in this?

>  	/*
>  	 * Firmware returns DPC port BDF details in following format:
>  	 *	15:8 = bus
> -- 
> 2.25.1
>
Re: [PATCH v1] PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec
Posted by Kuppuswamy Sathyanarayanan 1 year, 7 months ago 
Hi,

On 5/1/24 2:50 PM, Bjorn Helgaas wrote:
> On Wed, May 01, 2024 at 02:25:43AM +0000, Kuppuswamy Sathyanarayanan wrote:
>> 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@linux.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))
> How confident are we that this won't break any existing platforms?

Since we are already using arg4 as package, it wont work with
platforms that implement version 5.  So I think we won't be
breaking any existing users of version 5.


> Any idea how many platforms implement EDR_PORT_DPC_ENABLE_DSM and what
> Revision IDs they support?

I am not very sure about it. I think it is being used in some Dell
and Nvidia platforms.

@Satish from Dell, tested this fix in some Dell server platforms
that implements this support and found it working.

@Tushar Dave, since you previously submitted some error report
related to EDR, I assume you have some platforms that uses these
_DSMs. Can you please take a look at this patch and let us know
whether it works for you?

>
>>  		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;
>> +	}
> This changes two _DSMs, and I think it should be two patches.

Ok. I can split it into two patches.

> Same question here: we now depend on functionality we didn't depend on
> before.  How confident are we in this?

In some platforms I have tested so far, it seems to work. But I am
not sure whether there are other platforms that does not implement
this support.

IMO, since this change aligns with the spec, it is best to fix it.

>
>>  	/*
>>  	 * Firmware returns DPC port BDF details in following format:
>>  	 *	15:8 = bus
>> -- 
>> 2.25.1
>>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.