From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-5.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id AC0257D072 for ; Mon, 16 Jul 2018 17:15:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727866AbeGPRoI convert rfc822-to-8bit (ORCPT ); Mon, 16 Jul 2018 13:44:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42810 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727514AbeGPRoI (ORCPT ); Mon, 16 Jul 2018 13:44:08 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 22064821C3; Mon, 16 Jul 2018 17:15:46 +0000 (UTC) Received: from t450s.home (ovpn-116-29.phx2.redhat.com [10.3.116.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id 95F7F5D9C8; Mon, 16 Jul 2018 17:15:43 +0000 (UTC) Date: Mon, 16 Jul 2018 11:15:43 -0600 From: Alex Williamson To: Logan Gunthorpe Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, linux-doc@vger.kernel.org, Stephen Bates , Christoph Hellwig , Bjorn Helgaas , Jonathan Corbet , Ingo Molnar , Thomas Gleixner , "Paul E. McKenney" , Marc Zyngier , Kai-Heng Feng , Frederic Weisbecker , Dan Williams , =?UTF-8?B?SsOpcsO0bWU=?= Glisse , Benjamin Herrenschmidt , Christian =?UTF-8?B?S8O2bmln?= , Matthew Wilcox Subject: Re: [PATCH v6 4/4] PCI: Introduce the disable_acs_redir parameter Message-ID: <20180716111543.6ddf584c@t450s.home> In-Reply-To: <20180713233158.19528-5-logang@deltatee.com> References: <20180713233158.19528-1-logang@deltatee.com> <20180713233158.19528-5-logang@deltatee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 16 Jul 2018 17:15:47 +0000 (UTC) Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Fri, 13 Jul 2018 17:31:58 -0600 Logan Gunthorpe wrote: > In order to support P2P traffic on a segment of the PCI hierarchy, > we must be able to disable the ACS redirect bits for select > PCI bridges. The bridges must be selected before the devices are > discovered by the kernel and the IOMMU groups created. Therefore, > a kernel command line parameter is created to specify devices > which must have their ACS bits disabled. > > The new parameter takes a list of devices separated by a semicolon. > Each device specified will have it's ACS redirect bits disabled. > This is similar to the existing 'resource_alignment' parameter. > > The ACS Request P2P Request Redirect, P2P Completion Redirect and P2P > Egress Control bits are disabled which is sufficient to always allow > passing P2P traffic uninterrupted. The bits are set after the kernel > (optionally) enables the ACS bits itself. It is also done regardless of > whether the kernel sets the bits or not seeing some BIOS firmware is known > to set the bits on boot. > > If the user tries to disable the ACS redirct for a device without the > ACS capability, a warning is printed to dmesg. > > Signed-off-by: Logan Gunthorpe > Reviewed-by: Stephen Bates > Acked-by: Christian König > --- > Documentation/admin-guide/kernel-parameters.txt | 9 +++ > drivers/pci/pci.c | 76 ++++++++++++++++++++++++- > 2 files changed, 83 insertions(+), 2 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index a69947d9e14e..02634c4ab181 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3192,6 +3192,15 @@ > Adding the window is slightly risky (it may > conflict with unreported devices), so this > taints the kernel. > + disable_acs_redir=[; ...] > + Specify one or more PCI devices (in the format > + specified above) separated by semicolons. > + Each device specified will have the PCI ACS > + redirect capabilities forced off which will > + allow P2P traffic between devices through > + bridges without forcing it upstream. Note: > + this removes isolation between devices and > + will make the IOMMU groups less granular. > > pcie_aspm= [PCIE] Forcibly enable or disable PCIe Active State Power > Management. > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 59638075b4df..195b93824e6c 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2983,6 +2983,66 @@ void pci_request_acs(void) > pci_acs_enable = 1; > } > > +static const char *disable_acs_redir_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) > +{ > + int ret = 0; > + const char *p; > + int pos; > + u16 ctrl; > + > + if (!disable_acs_redir_param) > + return; > + > + p = disable_acs_redir_param; > + while (*p) { > + 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); > + > + break; > + } else if (ret == 1) { > + /* Found a match */ > + break; > + } > + > + if (*p != ';' && *p != ',') { > + /* End of param or invalid format */ > + break; > + } > + p++; > + } > + > + if (ret != 1) > + return; > + > + pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS); > + if (!pos) { > + pci_warn(dev, "cannot disable ACS redirect for this hardware as it does not have ACS capabilities\n"); > + return; > + } > + > + if (!pci_dev_specific_disable_acs_redir(dev)) > + return; Shouldn't this come before the above ACS capability test? The PCH quirk without a disable_acs_redir callback will fall out with the pci_warn() above either way, but this ordering precludes that a quirk could be written for that device since it doesn't have an ACS capability. Otherwise the series looks ok to me. Thanks, Alex > + > + pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl); > + > + /* P2P Request & Completion Redirect */ > + ctrl &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC); > + > + pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl); > + > + pci_info(dev, "disabled ACS redirect\n"); > +} > + > /** > * pci_std_enable_acs - enable ACS on devices using standard ACS capabilites > * @dev: the PCI device > @@ -3022,12 +3082,22 @@ static void pci_std_enable_acs(struct pci_dev *dev) > void pci_enable_acs(struct pci_dev *dev) > { > if (!pci_acs_enable) > - return; > + goto disable_acs_redir; > > if (!pci_dev_specific_enable_acs(dev)) > - return; > + goto disable_acs_redir; > > pci_std_enable_acs(dev); > + > +disable_acs_redir: > + /* > + * Note: pci_disable_acs_redir() must be called even if > + * ACS is not enabled by the kernel because the firmware > + * may have unexpectedly set the flags. So if we are told > + * to disable it, we should always disable it after setting > + * the kernel's default preferences. > + */ > + pci_disable_acs_redir(dev); > } > > static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags) > @@ -5967,6 +6037,8 @@ static int __init pci_setup(char *str) > pcie_bus_config = PCIE_BUS_PEER2PEER; > } else if (!strncmp(str, "pcie_scan_all", 13)) { > pci_add_flags(PCI_SCAN_ALL_PCIE_DEVS); > + } else if (!strncmp(str, "disable_acs_redir=", 18)) { > + disable_acs_redir_param = str + 18; > } else { > printk(KERN_ERR "PCI: Unknown option `%s'\n", > str); -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html