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