[+cc Lukas, Alex for pciehp and ACS comments]
On Thu, Jan 12, 2023 at 12:35:33AM +0530, Vidya Sagar wrote:
> PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
> send upstream messages before they have been assigned a bus number and
> such messages have a Requester ID with Bus number set to 00h.
> If the Downstrem port has ACS Source Validation enabled, these messages
> will be detected as ACS violation error.
> Hence, disable ACS Source Validation in the bridge device during
> hot-remove operation and re-enable it after enumeration of the
> downstream hierarchy but before binding the respective device drivers.
s/Downstrem/Downstream/
Format as paragraphs (blank line between).
> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> ---
> v2:
> * Fixed build issues
>
> drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
> drivers/pci/pci.c | 22 ++++++++++++++++++++++
> include/linux/pci.h | 6 ++++++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index d17f3bf36f70..ad90bcf3f621 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
>
> pci_assign_unassigned_bridge_resources(bridge);
> pcie_bus_configure_settings(parent);
> + pci_configure_acs_sv(bridge, true);
> pci_bus_add_devices(parent);
>
> out:
> @@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
> }
> pci_dev_put(dev);
> }
> -
> + /*
> + * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
> + * to send upstream messages before they have been assigned a bus
> + * number and such messages have a Requester ID with Bus number
> + * set to 00h. If the Downstrem port has ACS Source Validation enabled,
s/Downstrem/Downstream/
> + * these messages will be detected as ACS violation error.
> + * Hence, disable ACS Source Validation here and re-enable it after
> + * enumeration of the downstream hierarchy and before binding the
> + * respective device drivers in pciehp_configure_device().
> + */
> + pci_configure_acs_sv(ctrl->pcie->port, false);
What if we have a slot that's empty at boot and we add a device later?
It looks like we still might see ACS errors there because the add
happens before a remove?
> pci_unlock_rescan_remove();
> }
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 95bc329e74c0..9cefaf814f49 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -991,6 +991,28 @@ static void pci_enable_acs(struct pci_dev *dev)
> pci_disable_acs_redir(dev);
> }
>
> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
> +{
> + u16 cap;
> + u16 ctrl;
> +
> + if (!pci_acs_enable || !dev->acs_cap)
> + return;
> +
> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
> +
> + if (flag)
> + ctrl |= (cap & PCI_ACS_SV);
> + else
> + ctrl &= ~(cap & PCI_ACS_SV);
> +
> + pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
I guess we don't have a way to do this for the non-standard ACS-like
devices, i.e., pci_dev_specific_enable_acs(). Not the end of the
world, just unfortunate that we'll have different behavior there.
> +}
> +EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
Doesn't seem like this needs to be exported or exposed via
linux/pci.h. pciehp cannot be built as a module.
> +#endif
> +
> /**
> * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
> * @dev: PCI device to have its BARs restored
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 060af91bafcd..edf516e39764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
> void pci_hp_remove_module_link(struct pci_slot *pci_slot);
> #endif
>
> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
> +#else
> +static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { }
> +#endif
> +
> /**
> * pci_pcie_cap - get the saved PCIe capability offset
> * @dev: PCI device
> --
> 2.17.1
>
