[+cc iommu folks]
On Thu, May 23, 2024 at 12:05:28PM +0530, Vidya Sagar wrote:
> For iommu_groups to form correctly, the ACS settings in the PCIe fabric
> need to be setup early in the boot process, either via the BIOS or via
> the kernel disable_acs_redir parameter.
Can you point to the iommu code that is involved here? It sounds like
the iommu_groups are built at boot time and are immutable after that?
If we need per-device ACS config that depends on the workload, it
seems kind of problematic to only be able to specify this at boot
time. I guess we would need to reboot if we want to run a workload
that needs a different config?
Is this the iommu usage model we want in the long term?
> disable_acs_redir allows clearing the RR|CR|EC ACS flags, but the PCIe
> spec Rev3.0 already defines 7 different ACS related flags with many more
> useful combinations depending on the fabric design.
If we need a spec citation, I'd rather use r6.x since r3.0 is from
2010.
> For backward compatibility, leave the 'disable_acs_redir' as is and add
> a new parameter 'config_acs'so that the user can directly specify the ACS
> flags to set on a per-device basis. Use a similar syntax to the existing
> 'resource_alignment' parameter by using the @ character and have the user
> specify the ACS flags using a bit encoding. If both 'disable_acs_redir' and
> 'config_acs' are specified for a particular device, configuration specified
> through 'config_acs' takes precedence over the other.
>
> Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> ---
> v3:
> * Fixed a documentation issue reported by kernel test bot
>
> v2:
> * Refactored the code as per Jason's suggestion
>
> .../admin-guide/kernel-parameters.txt | 22 +++
> drivers/pci/pci.c | 148 +++++++++++-------
> 2 files changed, 112 insertions(+), 58 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 41644336e..b4a8207eb 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4456,6 +4456,28 @@
> bridges without forcing it upstream. Note:
> this removes isolation between devices and
> may put more devices in an IOMMU group.
> + config_acs=
> + Format:
> + =<ACS flags>@<pci_dev>[; ...]
> + Specify one or more PCI devices (in the format
> + specified above) optionally prepended with flags
> + and separated by semicolons. The respective
> + capabilities will be enabled, disabled or unchanged
> + based on what is specified in flags.
> + ACS Flags is defined as follows
> + bit-0 : ACS Source Validation
> + bit-1 : ACS Translation Blocking
> + bit-2 : ACS P2P Request Redirect
> + bit-3 : ACS P2P Completion Redirect
> + bit-4 : ACS Upstream Forwarding
> + bit-5 : ACS P2P Egress Control
> + bit-6 : ACS Direct Translated P2P
> + Each bit can be marked as
> + ‘0‘ – force disabled
> + ‘1’ – force enabled
> + ‘x’ – unchanged.
> + Note: this may remove isolation between devices
> + and may put more devices in an IOMMU group.
> force_floating [S390] Force usage of floating interrupts.
> nomio [S390] Do not use MIO instructions.
> norid [S390] ignore the RID field and force use of
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index a607f277c..a46264f83 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -887,30 +887,67 @@ void pci_request_acs(void)
> }
>
> static const char *disable_acs_redir_param;
> +static const char *config_acs_param;
>
> -/**
> - * pci_disable_acs_redir - disable ACS redirect capabilities
> - * @dev: the PCI device
> - *
> - * For only devices specified in the disable_acs_redir parameter.
> - */
> -static void pci_disable_acs_redir(struct pci_dev *dev)
> +struct pci_acs {
> + u16 cap;
> + u16 ctrl;
> + u16 fw_ctrl;
> +};
> +
> +static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
> + const char *p, u16 mask, u16 flags)
> {
> + char *delimit;
> int ret = 0;
> - const char *p;
> - int pos;
> - u16 ctrl;
>
> - if (!disable_acs_redir_param)
> + if (!p)
> return;
>
> - p = disable_acs_redir_param;
> while (*p) {
> + if (!mask) {
> + /* Check for ACS flags */
> + delimit = strstr(p, "@");
> + if (delimit) {
> + int end;
> + u32 shift = 0;
> +
> + end = delimit - p - 1;
> +
> + while (end > -1) {
> + if (*(p + end) == '0') {
> + mask |= 1 << shift;
> + shift++;
> + end--;
> + } else if (*(p + end) == '1') {
> + mask |= 1 << shift;
> + flags |= 1 << shift;
> + shift++;
> + end--;
> + } else if ((*(p + end) == 'x') || (*(p + end) == 'X')) {
> + shift++;
> + end--;
> + } else {
> + pci_err(dev, "Invalid ACS flags... Ignoring\n");
> + return;
> + }
> + }
> + p = delimit + 1;
> + } else {
> + pci_err(dev, "ACS Flags missing\n");
> + return;
> + }
> + }
> +
> + if (mask & ~(PCI_ACS_SV | PCI_ACS_TB | PCI_ACS_RR | PCI_ACS_CR |
> + PCI_ACS_UF | PCI_ACS_EC | PCI_ACS_DT)) {
> + pci_err(dev, "Invalid ACS flags specified\n");
> + return;
> + }
> +
> ret = pci_dev_str_match(dev, p, &p);
> if (ret < 0) {
> - pr_info_once("PCI: Can't parse disable_acs_redir parameter: %s\n",
> - disable_acs_redir_param);
> -
> + pr_info_once("PCI: Can't parse acs command line parameter\n");
> break;
> } else if (ret == 1) {
> /* Found a match */
> @@ -930,56 +967,38 @@ static void pci_disable_acs_redir(struct pci_dev *dev)
> if (!pci_dev_specific_disable_acs_redir(dev))
> return;
>
> - pos = dev->acs_cap;
> - if (!pos) {
> - pci_warn(dev, "cannot disable ACS redirect for this hardware as it does not have ACS capabilities\n");
> - return;
> - }
> -
> - pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> + pci_dbg(dev, "ACS mask = 0x%X\n", mask);
> + pci_dbg(dev, "ACS flags = 0x%X\n", flags);
>
> - /* P2P Request & Completion Redirect */
> - ctrl &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC);
> + /* If mask is 0 then we copy the bit from the firmware setting. */
> + caps->ctrl = (caps->ctrl & ~mask) | (caps->fw_ctrl & mask);
> + caps->ctrl |= flags;
>
> - pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> -
> - pci_info(dev, "disabled ACS redirect\n");
> + pci_info(dev, "Configured ACS to 0x%x\n", caps->ctrl);
> }
>
> /**
> * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
> * @dev: the PCI device
> + * @caps: default ACS controls
> */
> -static void pci_std_enable_acs(struct pci_dev *dev)
> +static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
> {
> - int pos;
> - u16 cap;
> - u16 ctrl;
> -
> - pos = dev->acs_cap;
> - if (!pos)
> - return;
> -
> - pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
> - pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> -
> /* Source Validation */
> - ctrl |= (cap & PCI_ACS_SV);
> + caps->ctrl |= (caps->cap & PCI_ACS_SV);
>
> /* P2P Request Redirect */
> - ctrl |= (cap & PCI_ACS_RR);
> + caps->ctrl |= (caps->cap & PCI_ACS_RR);
>
> /* P2P Completion Redirect */
> - ctrl |= (cap & PCI_ACS_CR);
> + caps->ctrl |= (caps->cap & PCI_ACS_CR);
>
> /* Upstream Forwarding */
> - ctrl |= (cap & PCI_ACS_UF);
> + caps->ctrl |= (caps->cap & PCI_ACS_UF);
>
> /* Enable Translation Blocking for external devices and noats */
> if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
> - ctrl |= (cap & PCI_ACS_TB);
> -
> - pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> + caps->ctrl |= (caps->cap & PCI_ACS_TB);
> }
>
> /**
> @@ -988,23 +1007,33 @@ static void pci_std_enable_acs(struct pci_dev *dev)
> */
> static void pci_enable_acs(struct pci_dev *dev)
> {
> - if (!pci_acs_enable)
> - goto disable_acs_redir;
> + struct pci_acs caps;
> + int pos;
> +
> + pos = dev->acs_cap;
> + if (!pos)
> + return;
>
> - if (!pci_dev_specific_enable_acs(dev))
> - goto disable_acs_redir;
> + pci_read_config_word(dev, pos + PCI_ACS_CAP, &caps.cap);
> + pci_read_config_word(dev, pos + PCI_ACS_CTRL, &caps.ctrl);
> + caps.fw_ctrl = caps.ctrl;
>
> - pci_std_enable_acs(dev);
> + /* If an iommu is present we start with kernel default caps */
> + if (pci_acs_enable) {
> + if (pci_dev_specific_enable_acs(dev))
> + pci_std_enable_acs(dev, &caps);
> + }
>
> -disable_acs_redir:
> /*
> - * Note: pci_disable_acs_redir() must be called even if ACS was not
> - * enabled by the kernel because it may have been enabled by
> - * platform firmware. So if we are told to disable it, we should
> - * always disable it after setting the kernel's default
> - * preferences.
> + * Always apply caps from the command line, even if there is no iommu.
> + * Trust that the admin has a reason to change the ACS settings.
> */
> - pci_disable_acs_redir(dev);
> + __pci_config_acs(dev, &caps, disable_acs_redir_param,
> + PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC,
> + ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC));
> + __pci_config_acs(dev, &caps, config_acs_param, 0, 0);
> +
> + pci_write_config_word(dev, pos + PCI_ACS_CTRL, caps.ctrl);
> }
>
> /**
> @@ -7023,6 +7052,8 @@ static int __init pci_setup(char *str)
> pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS);
> } else if (!strncmp(str, "disable_acs_redir=", 18)) {
> disable_acs_redir_param = str + 18;
> + } else if (!strncmp(str, "config_acs=", 11)) {
> + config_acs_param = str + 11;
> } else {
> pr_err("PCI: Unknown option `%s'\n", str);
> }
> @@ -7047,6 +7078,7 @@ static int __init pci_realloc_setup_params(void)
> resource_alignment_param = kstrdup(resource_alignment_param,
> GFP_KERNEL);
> disable_acs_redir_param = kstrdup(disable_acs_redir_param, GFP_KERNEL);
> + config_acs_param = kstrdup(config_acs_param, GFP_KERNEL);
>
> return 0;
> }
> --
> 2.25.1
>
