lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <01dfc5ba-43cb-fbea-93e6-4963c411c750@deltatee.com>
Date:   Mon, 9 Jul 2018 16:27:40 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-doc@...r.kernel.org, Stephen Bates <sbates@...thlin.com>,
        Christoph Hellwig <hch@....de>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Frederic Weisbecker <frederic@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Jérôme Glisse <jglisse@...hat.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Christian König <christian.koenig@....com>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v5 3/3] PCI: Introduce the disable_acs_redir parameter

Hey Alex,

On 06/07/18 04:56 PM, Alex Williamson wrote:
> Maybe we track if we enabled ACS via a device specific quirk and
> minimally print an incompatibility error if it's also specified for
> disable_acs_redir?  Thanks,

Ok, I dug into this a bit and I think tracking if we enabled via a
device specific quirk does not work. If 'pci_acs_enable' is not set, we
wouldn't know if we used a device specific quirk and still try to
disable a quirked device the standard way.

Instead, I've looked at adding a device specific disable_acs_redir
function next to enable_acs. In this way, the device specific quirks can
override the operation. See the diff below.

Implementing the Intel SPT PCH version is pretty trivial, so I've done
that. The Intel PCH variant I've just left as unsupported and printed a
warning.

If this sounds good to you, I'll fold it into my patchset and resubmit a v6.

Thanks,

Logan

--

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 079f7c911e09..54001b307496 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3028,6 +3028,9 @@ static void pci_disable_acs_redir(struct pci_dev *dev)
 	if (!pos)
 		return;

+	if (!pci_dev_specific_disable_acs_redir(dev))
+		return;
+
 	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);

 	/* P2P Request & Completion Redirect */
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index f439de848658..c976a025ae92 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4526,6 +4526,16 @@ static int pci_quirk_enable_intel_pch_acs(struct
pci_dev *dev)
 	return 0;
 }

+static int pci_quirk_disable_intel_pch_acs_redir(struct pci_dev *dev)
+{
+	if (!pci_quirk_intel_pch_acs_match(dev))
+		return -ENOTTY;
+
+	pci_warn(dev, "Disabling ACS redirect on the Intel PCH root port is
not supported\n");
+
+	return 0;
+}
+
 static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
 {
 	int pos;
@@ -4553,22 +4563,53 @@ static int
pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
 	return 0;
 }

-static const struct pci_dev_enable_acs {
+static int pci_quirk_disable_intel_spt_pch_acs_redir(struct pci_dev *dev)
+{
+	int pos;
+	u32 cap, ctrl;
+
+	if (!pci_quirk_intel_spt_pch_acs_match(dev))
+		return -ENOTTY;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+	if (!pos)
+		return -ENOTTY;
+
+	pci_read_config_dword(dev, pos + PCI_ACS_CAP, &cap);
+	pci_read_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, &ctrl);
+
+	ctrl &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC);
+
+	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
+
+	pci_info(dev, "Intel SPT PCH root port workaround: disabled ACS
redirect\n");
+
+	return 0;
+}
+
+static const struct pci_dev_acs_ops {
 	u16 vendor;
 	u16 device;
 	int (*enable_acs)(struct pci_dev *dev);
-} pci_dev_enable_acs[] = {
-	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_enable_intel_pch_acs },
-	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_enable_intel_spt_pch_acs },
+	int (*disable_acs_redir)(struct pci_dev *dev);
+} pci_dev_acs_ops[] = {
+	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
+	  .enable_acs = pci_quirk_enable_intel_pch_acs,
+	  .disable_acs_redir = pci_quirk_disable_intel_pch_acs_redir,
+	},
+	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
+	  .enable_acs = pci_quirk_enable_intel_spt_pch_acs,
+	  .disable_acs_redir = pci_quirk_disable_intel_spt_pch_acs_redir
+	},
 	{ 0 }
 };

 int pci_dev_specific_enable_acs(struct pci_dev *dev)
 {
-	const struct pci_dev_enable_acs *i;
+	const struct pci_dev_acs_ops *i;
 	int ret;

-	for (i = pci_dev_enable_acs; i->enable_acs; i++) {
+	for (i = pci_dev_acs_ops; i->enable_acs; i++) {
 		if ((i->vendor == dev->vendor ||
 		     i->vendor == (u16)PCI_ANY_ID) &&
 		    (i->device == dev->device ||
@@ -4582,6 +4623,25 @@ int pci_dev_specific_enable_acs(struct pci_dev *dev)
 	return -ENOTTY;
 }

+int pci_dev_specific_disable_acs_redir(struct pci_dev *dev)
+{
+	const struct pci_dev_acs_ops *i;
+	int ret;
+
+	for (i = pci_dev_acs_ops; i->enable_acs; i++) {
+		if ((i->vendor == dev->vendor ||
+		     i->vendor == (u16)PCI_ANY_ID) &&
+		    (i->device == dev->device ||
+		     i->device == (u16)PCI_ANY_ID)) {
+			ret = i->disable_acs_redir(dev);
+			if (ret >= 0)
+				return ret;
+		}
+	}
+
+	return -ENOTTY;
+}
+
 /*
  * The PCI capabilities list for Intel DH895xCC VFs (device ID 0x0443) with
  * QuickAssist Technology (QAT) is prematurely terminated in hardware.  The
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 340029b2fb38..7ee208aa1a31 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1878,6 +1878,7 @@ enum pci_fixup_pass {
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
 int pci_dev_specific_enable_acs(struct pci_dev *dev);
+int pci_dev_specific_disable_acs_redir(struct pci_dev *dev);
 #else
 static inline void pci_fixup_device(enum pci_fixup_pass pass,
 				    struct pci_dev *dev) { }

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ