Re: [PATCH 1/3] PCI: Call PCI ACPI _DSM with consistent revision argument

From: Rafael J. Wysocki
Date: Thu Nov 30 2023 - 08:34:34 EST

On Fri, Nov 10, 2023 at 9:32 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
>
> The PCI ACPI _DSM is called across multiple places in the PCI core
> with different arguments for the revision.
>
> The PCI firmware specification specifies that this is an incorrect
> behavior.
> "OSPM must invoke all Functions other than Function 0 with the
> same Revision ID value"
>
> Link: https://members.pcisig.com/wg/PCI-SIG/document/15350
> PCI Firmware specification 3.3, section 4.6
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

and I haven't seen much activity related to this series, so I'm not
sure what's happening to it.

Regardless, I think that the remaining two patches are better sent
along with the first users of the new APIs.

> ---
> drivers/acpi/pci_root.c | 3 ++-
> drivers/pci/pci-acpi.c | 6 ++++--
> drivers/pci/pci-label.c | 4 ++--
> drivers/pci/pcie/edr.c | 13 +++++++------
> include/linux/pci-acpi.h | 1 +
> 5 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 58b89b8d950e..bca2270a93d4 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -1055,7 +1055,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> * exists and returns 0, we must preserve any PCI resource
> * assignments made by firmware for this host bridge.
> */
> - obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 1,
> + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(bus->bridge),
> + &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> DSM_PCI_PRESERVE_BOOT_CONFIG, NULL, ACPI_TYPE_INTEGER);
> if (obj && obj->integer.value == 0)
> host_bridge->preserve_config = 1;
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 004575091596..bea72e807817 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -28,6 +28,7 @@
> const guid_t pci_acpi_dsm_guid =
> GUID_INIT(0xe5c937d0, 0x3553, 0x4d7a,
> 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d);
> +const int pci_acpi_dsm_rev = 5;
>
> #if defined(CONFIG_PCI_QUIRKS) && defined(CONFIG_ARM64)
> static int acpi_get_rc_addr(struct acpi_device *adev, struct resource *res)
> @@ -1215,7 +1216,8 @@ void acpi_pci_add_bus(struct pci_bus *bus)
> if (!pci_is_root_bus(bus))
> return;
>
> - obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(bus->bridge), &pci_acpi_dsm_guid, 3,
> + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(bus->bridge),
> + &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> DSM_PCI_POWER_ON_RESET_DELAY, NULL, ACPI_TYPE_INTEGER);
> if (!obj)
> return;
> @@ -1376,7 +1378,7 @@ static void pci_acpi_optimize_delay(struct pci_dev *pdev,
> if (bridge->ignore_reset_delay)
> pdev->d3cold_delay = 0;
>
> - obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, 3,
> + obj = acpi_evaluate_dsm_typed(handle, &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> DSM_PCI_DEVICE_READINESS_DURATIONS, NULL,
> ACPI_TYPE_PACKAGE);
> if (!obj)
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> index 0c6446519640..91bdd04029f0 100644
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -41,7 +41,7 @@ static bool device_has_acpi_name(struct device *dev)
> if (!handle)
> return false;
>
> - return acpi_check_dsm(handle, &pci_acpi_dsm_guid, 0x2,
> + return acpi_check_dsm(handle, &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> 1 << DSM_PCI_DEVICE_NAME);
> #else
> return false;
> @@ -162,7 +162,7 @@ static int dsm_get_label(struct device *dev, char *buf,
> if (!handle)
> return -1;
>
> - obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, 0x2,
> + obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> DSM_PCI_DEVICE_NAME, NULL);
> if (!obj)
> return -1;
> diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
> index 5f4914d313a1..ab6a50201124 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, pci_acpi_dsm_rev,
> 1ULL << EDR_PORT_DPC_ENABLE_DSM))
> return 0;
>
> @@ -51,8 +51,9 @@ static int acpi_enable_dpc(struct pci_dev *pdev)
> * Firmware Specification r3.2, 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,
> - EDR_PORT_DPC_ENABLE_DSM, &argv4);
> + obj = acpi_evaluate_dsm(adev->handle, &pci_acpi_dsm_guid,
> + pci_acpi_dsm_rev, EDR_PORT_DPC_ENABLE_DSM,
> + &argv4);
> if (!obj)
> return 0;
>
> @@ -88,12 +89,12 @@ 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.
> */
> - if (!acpi_check_dsm(adev->handle, &pci_acpi_dsm_guid, 5,
> + if (!acpi_check_dsm(adev->handle, &pci_acpi_dsm_guid, pci_acpi_dsm_rev,
> 1ULL << EDR_PORT_LOCATE_DSM))
> return pci_dev_get(pdev);
>
> - obj = acpi_evaluate_dsm(adev->handle, &pci_acpi_dsm_guid, 5,
> - EDR_PORT_LOCATE_DSM, NULL);
> + obj = acpi_evaluate_dsm(adev->handle, &pci_acpi_dsm_guid,
> + pci_acpi_dsm_rev, EDR_PORT_LOCATE_DSM, NULL);
> if (!obj)
> return pci_dev_get(pdev);
>
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 078225b514d4..7966ef8f14b3 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -115,6 +115,7 @@ static inline void acpiphp_check_host_bridge(struct acpi_device *adev) { }
> #endif
>
> extern const guid_t pci_acpi_dsm_guid;
> +extern const int pci_acpi_dsm_rev;
>
> /* _DSM Definitions for PCI */
> #define DSM_PCI_PRESERVE_BOOT_CONFIG 0x05
> --
> 2.34.1
>
>