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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>
>





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux