diff mbox

enabling ACS P2P upstream forwarding

Message ID 57C9024A16AD2D4C97DC78E552063EA3E05DC218@orsmsx505.amr.corp.intel.com
State Superseded, archived
Headers show

Commit Message

Kay, Allen M Sept. 15, 2009, 11:44 p.m. UTC
This patch enables P2P upstream forwarding in ACS capable PCIe switches.  This solves two potential problems in virtualization environment where a PCIe device is assigned to a guest domain using a HW iommu such as VT-d:

1) Unintentional failure caused by guest physical address programmed into the device's DMA that happens to match the memory address range of other downstream ports in the same PCIe switch.  This causes the PCI transaction to go to the matching downstream port instead of go to the root complex to get translated by VT-d as it should be.

2) Malicious guest software intentionally attacks another downstream PCIe device by programming the DMA address into the assigned device that matches memory address range of the downstream PCIe port.

We are in process of implementing device filtering software in KVM/XEN management software to allow device assignment of PCIe devices behind a PCIe switch only if it has ACS capability and with the P2P upstream forwarding bits enabled.  This patch is intended to work for both KVM and Xen environments.

Signed-off-by: Allen Kay allen.m.kay@intel.com

---

 drivers/pci/pci.c        |   37 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h        |    1 +
 drivers/pci/probe.c      |    3 +++
 include/linux/pci_regs.h |   14 ++++++++++++++
 4 files changed, 55 insertions(+)

Comments

Daniel Walker Sept. 16, 2009, 12:12 a.m. UTC | #1
On Tue, 2009-09-15 at 16:44 -0700, Kay, Allen M wrote:
> This patch enables P2P upstream forwarding in ACS capable PCIe switches.  This solves two potential problems in virtualization environment where a PCIe device is assigned to a guest domain using a HW iommu such as VT-d:
> 
> 1) Unintentional failure caused by guest physical address programmed into the device's DMA that happens to match the memory address range of other downstream ports in the same PCIe switch.  This causes the PCI transaction to go to the matching downstream port instead of go to the root complex to get translated by VT-d as it should be.
> 
> 2) Malicious guest software intentionally attacks another downstream PCIe device by programming the DMA address into the assigned device that matches memory address range of the downstream PCIe port.
> 
> We are in process of implementing device filtering software in KVM/XEN management software to allow device assignment of PCIe devices behind a PCIe switch only if it has ACS capability and with the P2P upstream forwarding bits enabled.  This patch is intended to work for both KVM and Xen environments.
> 
> Signed-off-by: Allen Kay allen.m.kay@intel.com


Both times that you've submitted this the patch has been corrupt.. Your
mailer, or something is adding little "=20" or "=3D" all over. For
instance,

"Signed-off-by: Allen Kay allen.m.kay@intel.com

---

 drivers/pci/pci.c        |   37 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h        |    1 +
 drivers/pci/probe.c      |    3 +++
 include/linux/pci_regs.h |   14 ++++++++++++++
 4 files changed, 55 insertions(+)

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7b70312..e70abde 100644
..."


So there's no way to test or apply your patch .. Most reviewers are just
going to walk away if they see stuff like this..

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kay, Allen M Sept. 16, 2009, 12:13 a.m. UTC | #2
Are you able to look at the file in the attachment?

-----Original Message-----
From: Daniel Walker [mailto:dwalker@fifo99.com] 
Sent: Tuesday, September 15, 2009 5:12 PM
To: Kay, Allen M
Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; jbarnes@virtuousgeek.org
Subject: Re: [PATCH] enabling ACS P2P upstream forwarding

On Tue, 2009-09-15 at 16:44 -0700, Kay, Allen M wrote:
> This patch enables P2P upstream forwarding in ACS capable PCIe switches.  This solves two potential problems in virtualization environment where a PCIe device is assigned to a guest domain using a HW iommu such as VT-d:
> 
> 1) Unintentional failure caused by guest physical address programmed into the device's DMA that happens to match the memory address range of other downstream ports in the same PCIe switch.  This causes the PCI transaction to go to the matching downstream port instead of go to the root complex to get translated by VT-d as it should be.
> 
> 2) Malicious guest software intentionally attacks another downstream PCIe device by programming the DMA address into the assigned device that matches memory address range of the downstream PCIe port.
> 
> We are in process of implementing device filtering software in KVM/XEN management software to allow device assignment of PCIe devices behind a PCIe switch only if it has ACS capability and with the P2P upstream forwarding bits enabled.  This patch is intended to work for both KVM and Xen environments.
> 
> Signed-off-by: Allen Kay allen.m.kay@intel.com


Both times that you've submitted this the patch has been corrupt.. Your
mailer, or something is adding little "=20" or "=3D" all over. For
instance,

"Signed-off-by: Allen Kay allen.m.kay@intel.com

---

 drivers/pci/pci.c        |   37 +++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h        |    1 +
 drivers/pci/probe.c      |    3 +++
 include/linux/pci_regs.h |   14 ++++++++++++++
 4 files changed, 55 insertions(+)

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7b70312..e70abde 100644
..."


So there's no way to test or apply your patch .. Most reviewers are just
going to walk away if they see stuff like this..

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Walker Sept. 16, 2009, 12:24 a.m. UTC | #3
On Tue, 2009-09-15 at 17:13 -0700, Kay, Allen M wrote:
> Are you able to look at the file in the attachment?
> 

Yeah I can, but attachments aren't the proper way to submit patches to
this list.. There are some easy way to do it. For instance, if your
using git, git includes programs called "git-format-patch" and
"git-send-email" which you can use to submit your changes un-corrupted.
They aren't very difficult to use..

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Grundler Sept. 16, 2009, 5:46 a.m. UTC | #4
On Tue, Sep 15, 2009 at 05:12:17PM -0700, Daniel Walker wrote:
...
> Both times that you've submitted this the patch has been corrupt.. Your
> mailer, or something is adding little "=20" or "=3D" all over. For
> instance,
> 
> "Signed-off-by: Allen Kay allen.m.kay@intel.com
> 
> ---
> 
>  drivers/pci/pci.c        |   37 +++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h        |    1 +
>  drivers/pci/probe.c      |    3 +++
>  include/linux/pci_regs.h |   14 ++++++++++++++
>  4 files changed, 55 insertions(+)
> 
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

I did not see any "=3D" in the posting - using "mutt" to read the email.

And AFAIK, many maintainers will accept attachments if that's what works.
Sure, it's not preferred, but pragmatic folks manage to get over that.

hth
grant
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Walker Sept. 16, 2009, 9:05 a.m. UTC | #5
On Tue, 2009-09-15 at 23:46 -0600, Grant Grundler wrote:
> On Tue, Sep 15, 2009 at 05:12:17PM -0700, Daniel Walker wrote:
> ...
> > Both times that you've submitted this the patch has been corrupt.. Your
> > mailer, or something is adding little "=20" or "=3D" all over. For
> > instance,
> > 
> > "Signed-off-by: Allen Kay allen.m.kay@intel.com
> > 
> > ---
> > 
> >  drivers/pci/pci.c        |   37 +++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h        |    1 +
> >  drivers/pci/probe.c      |    3 +++
> >  include/linux/pci_regs.h |   14 ++++++++++++++
> >  4 files changed, 55 insertions(+)
> > 
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> 
> I did not see any "=3D" in the posting - using "mutt" to read the email.

I've seen them in two different mailing systems, so I'm pretty sure they
are there.

> And AFAIK, many maintainers will accept attachments if that's what works.
> Sure, it's not preferred, but pragmatic folks manage to get over that.

Sure in a pinch, but learning how to do it a better way is good too.

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Wilcox Sept. 16, 2009, 9:57 a.m. UTC | #6
On Tue, Sep 15, 2009 at 04:44:00PM -0700, Kay, Allen M wrote:
> This patch enables P2P upstream forwarding in ACS capable PCIe switches.
> This solves two potential problems in virtualization environment where
> a PCIe device is assigned to a guest domain using a HW iommu such as VT-d:

Seems like a good idea.

> @@ -1513,6 +1513,49 @@ void pci_enable_ari(struct pci_dev *dev)
>  }
>  
>  /**
> + * pci_acs_enable - enable ACS if hardware support it
> + * @dev: the PCI device
> + */
> +#define ACS_ENABLED (PCI_EXP_ACS_V|PCI_EXP_ACS_R|PCI_EXP_ACS_C|PCI_EXP_ACS_U)

This is a little hard to read.  I find it easier with spaces, ie:

#define ACS_ENABLED (PCI_EXP_ACS_V | PCI_EXP_ACS_R | PCI_EXP_ACS_C | \
			PCI_EXP_ACS_U)

Now it doesn't fit on one line, but see below.

> +void pci_acs_init(struct pci_dev *dev)
> +{
> +	int pos;
> +	u16 cap;
> +	u16 ctrl;
> +
> +	if (!dev->is_pcie)
> +		return;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
> +	if ((cap & ACS_ENABLED) != ACS_ENABLED)
> +		dev_info(&dev->dev, "ACS P2P upstream not supported\n");

As I read the ACS spec, the Source Validation and Upstream Forwarding
bits must not be implemented for multifunction devices, so you'll produce
this warning for all non-downstream ports that implement ACS.  Not that
I have any of those.

> +	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);

Couldn't this:

> +	/* Source Validation */
> +	if (cap & PCI_EXP_ACS_V)
> +		ctrl |= PCI_EXP_ACS_V;
> +
> +	/* P2P Request Redirect */
> +	if (cap & PCI_EXP_ACS_R)
> +		ctrl |= PCI_EXP_ACS_R;
> +
> +	/* P2P Completion Redirect */
> +	if (cap & PCI_EXP_ACS_C)
> +		ctrl |= PCI_EXP_ACS_C;
> +
> +	/* Upstream Forwarding */
> +	if (cap & PCI_EXP_ACS_U)
> +		ctrl |= PCI_EXP_ACS_U;

be written more pithily as:

	ctrl |= (cap & ACS_ENABLED);
?

> +	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> +}
> +
> +/**
>   * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
>   * @dev: the PCI device
>   * @pin: the INTx pin (1=INTA, 2=INTB, 3=INTD, 4=INTD)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 5ff4d25..1d8976d 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -202,6 +202,7 @@ static inline int pci_ari_enabled(struct pci_bus *bus)
>  {
>  	return bus->self && bus->self->ari_enabled;
>  }
> +extern void pci_acs_init(struct pci_dev *dev);
>  
>  #ifdef CONFIG_PCI_QUIRKS
>  extern int pci_is_reassigndev(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 40e75f6..4a5ec9e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -991,6 +991,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  
>  	/* Single Root I/O Virtualization */
>  	pci_iov_init(dev);
> +
> +	/* Access Control Service */
> +	pci_acs_init(dev);
>  }
>  
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index fcaee42..90014ad 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -492,6 +492,14 @@
>  #define PCI_EXP_LNKCTL2		48	/* Link Control 2 */
>  #define PCI_EXP_SLTCTL2		56	/* Slot Control 2 */
>  
> +#define  PCI_EXP_ACS_V		0x01	/* Source Validation */
> +#define  PCI_EXP_ACS_B		0x02	/* Translation Blocking */
> +#define  PCI_EXP_ACS_R		0x04	/* P2P Request Redirect */
> +#define  PCI_EXP_ACS_C		0x08	/* P2P Completion Redirect */
> +#define  PCI_EXP_ACS_U		0x10	/* Upstream Forwarding */
> +#define  PCI_EXP_ACS_E		0x20	/* P2P Egress Control */
> +#define  PCI_EXP_ACS_T		0x40	/* Direct Translated P2P */

These definitions are in the wrong place and have the wrong name ;-)

You've put them in the part of the file used for the PCI Express capability,
and named them as if they're part of the PCI Express capability.

>  /* Extended Capabilities (PCI-X 2.0 and Express) */
>  #define PCI_EXT_CAP_ID(header)		(header & 0x0000ffff)
>  #define PCI_EXT_CAP_VER(header)		((header >> 16) & 0xf)
> @@ -501,6 +509,7 @@
>  #define PCI_EXT_CAP_ID_VC	2
>  #define PCI_EXT_CAP_ID_DSN	3
>  #define PCI_EXT_CAP_ID_PWR	4
> +#define PCI_EXT_CAP_ID_ACS	13
>  #define PCI_EXT_CAP_ID_ARI	14
>  #define PCI_EXT_CAP_ID_ATS	15
>  #define PCI_EXT_CAP_ID_SRIOV	16
> @@ -661,4 +670,9 @@
>  #define  PCI_SRIOV_VFM_MO	0x2	/* Active.MigrateOut */
>  #define  PCI_SRIOV_VFM_AV	0x3	/* Active.Available */
>  
> +/* Access Control Service */
> +#define PCI_ACS_CAP		0x04	/* ACS Capability Register */

They should be down here instead and named like this:

+#define  PCI_ACS_SV		0x01	/* Source Validation */
+#define  PCI_ACS_TB		0x02	/* Translation Blocking */
+#define  PCI_ACS_RR		0x04	/* P2P Request Redirect */
+#define  PCI_ACS_CR		0x08	/* P2P Completion Redirect */
+#define  PCI_ACS_UF		0x10	/* Upstream Forwarding */
+#define  PCI_ACS_EC		0x20	/* P2P Egress Control */
+#define  PCI_ACS_DT		0x40	/* Direct Translated P2P */

Now ACS_ENABLED fits on one line again ;-)

#define ACS_ENABLED (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)

> +#define PCI_ACS_CTRL		0x06	/* ACS Control Register */
> +#define PCI_ACS_EGRESS_CTL_V	0x08	/* ACS Egress Control Vector */
> +
>  #endif /* LINUX_PCI_REGS_H */
Jonathan Corbet Sept. 16, 2009, 3:34 p.m. UTC | #7
On Tue, 15 Sep 2009 17:12:17 -0700
Daniel Walker <dwalker@fifo99.com> wrote:

> Both times that you've submitted this the patch has been corrupt.. Your
> mailer, or something is adding little "=20" or "=3D" all over.

That looks a lot like *your* mail client not understanding
quoted-printable encoding.  QP can be a pain, but it is a legitimate
encoding; the message in question looks fine in claws.

jon
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Walker Sept. 16, 2009, 4:39 p.m. UTC | #8
On Wed, 2009-09-16 at 09:34 -0600, Jonathan Corbet wrote:
> On Tue, 15 Sep 2009 17:12:17 -0700
> Daniel Walker <dwalker@fifo99.com> wrote:
> 
> > Both times that you've submitted this the patch has been corrupt.. Your
> > mailer, or something is adding little "=20" or "=3D" all over.
> 
> That looks a lot like *your* mail client not understanding
> quoted-printable encoding.  QP can be a pain, but it is a legitimate
> encoding; the message in question looks fine in claws.

Shouldn't all patches be sent as plain text untouched? I don't think I
should necessarily need a specific email client to strip off some
characters..

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Cox Sept. 16, 2009, 4:41 p.m. UTC | #9
> Shouldn't all patches be sent as plain text untouched? I don't think I
> should necessarily need a specific email client to strip off some
> characters..

Plain text is 7bit (netascii). People have unicode names.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Walker Sept. 16, 2009, 4:58 p.m. UTC | #10
On Wed, 2009-09-16 at 17:41 +0100, Alan Cox wrote:
> > Shouldn't all patches be sent as plain text untouched? I don't think I
> > should necessarily need a specific email client to strip off some
> > characters..
> 
> Plain text is 7bit (netascii). People have unicode names.

Is "=20" or "=3D" unicode encodings? Seems like we do allow UTF-8 for
patches.. For this patch I don't see why you would need unicode in the
actual patch content or for names tho.

Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kay, Allen M Sept. 17, 2009, 2:14 a.m. UTC | #11
Thank you for your review and comments.  I've sent out a new version of the patch that incorporates your feedbacks.

Allen

-----Original Message-----
From: Matthew Wilcox [mailto:matthew@wil.cx] 
Sent: Wednesday, September 16, 2009 2:58 AM
To: Kay, Allen M
Cc: linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org; jbarnes@virtuousgeek.org
Subject: Re: [PATCH] enabling ACS P2P upstream forwarding

On Tue, Sep 15, 2009 at 04:44:00PM -0700, Kay, Allen M wrote:
> This patch enables P2P upstream forwarding in ACS capable PCIe switches.
> This solves two potential problems in virtualization environment where
> a PCIe device is assigned to a guest domain using a HW iommu such as VT-d:

Seems like a good idea.

> @@ -1513,6 +1513,49 @@ void pci_enable_ari(struct pci_dev *dev)
>  }
>  
>  /**
> + * pci_acs_enable - enable ACS if hardware support it
> + * @dev: the PCI device
> + */
> +#define ACS_ENABLED (PCI_EXP_ACS_V|PCI_EXP_ACS_R|PCI_EXP_ACS_C|PCI_EXP_ACS_U)

This is a little hard to read.  I find it easier with spaces, ie:

#define ACS_ENABLED (PCI_EXP_ACS_V | PCI_EXP_ACS_R | PCI_EXP_ACS_C | \
			PCI_EXP_ACS_U)

Now it doesn't fit on one line, but see below.

> +void pci_acs_init(struct pci_dev *dev)
> +{
> +	int pos;
> +	u16 cap;
> +	u16 ctrl;
> +
> +	if (!dev->is_pcie)
> +		return;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
> +	if ((cap & ACS_ENABLED) != ACS_ENABLED)
> +		dev_info(&dev->dev, "ACS P2P upstream not supported\n");

As I read the ACS spec, the Source Validation and Upstream Forwarding
bits must not be implemented for multifunction devices, so you'll produce
this warning for all non-downstream ports that implement ACS.  Not that
I have any of those.

> +	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);

Couldn't this:

> +	/* Source Validation */
> +	if (cap & PCI_EXP_ACS_V)
> +		ctrl |= PCI_EXP_ACS_V;
> +
> +	/* P2P Request Redirect */
> +	if (cap & PCI_EXP_ACS_R)
> +		ctrl |= PCI_EXP_ACS_R;
> +
> +	/* P2P Completion Redirect */
> +	if (cap & PCI_EXP_ACS_C)
> +		ctrl |= PCI_EXP_ACS_C;
> +
> +	/* Upstream Forwarding */
> +	if (cap & PCI_EXP_ACS_U)
> +		ctrl |= PCI_EXP_ACS_U;

be written more pithily as:

	ctrl |= (cap & ACS_ENABLED);
?

> +	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> +}
> +
> +/**
>   * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
>   * @dev: the PCI device
>   * @pin: the INTx pin (1=INTA, 2=INTB, 3=INTD, 4=INTD)
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 5ff4d25..1d8976d 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -202,6 +202,7 @@ static inline int pci_ari_enabled(struct pci_bus *bus)
>  {
>  	return bus->self && bus->self->ari_enabled;
>  }
> +extern void pci_acs_init(struct pci_dev *dev);
>  
>  #ifdef CONFIG_PCI_QUIRKS
>  extern int pci_is_reassigndev(struct pci_dev *dev);
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 40e75f6..4a5ec9e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -991,6 +991,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
>  
>  	/* Single Root I/O Virtualization */
>  	pci_iov_init(dev);
> +
> +	/* Access Control Service */
> +	pci_acs_init(dev);
>  }
>  
>  void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
> index fcaee42..90014ad 100644
> --- a/include/linux/pci_regs.h
> +++ b/include/linux/pci_regs.h
> @@ -492,6 +492,14 @@
>  #define PCI_EXP_LNKCTL2		48	/* Link Control 2 */
>  #define PCI_EXP_SLTCTL2		56	/* Slot Control 2 */
>  
> +#define  PCI_EXP_ACS_V		0x01	/* Source Validation */
> +#define  PCI_EXP_ACS_B		0x02	/* Translation Blocking */
> +#define  PCI_EXP_ACS_R		0x04	/* P2P Request Redirect */
> +#define  PCI_EXP_ACS_C		0x08	/* P2P Completion Redirect */
> +#define  PCI_EXP_ACS_U		0x10	/* Upstream Forwarding */
> +#define  PCI_EXP_ACS_E		0x20	/* P2P Egress Control */
> +#define  PCI_EXP_ACS_T		0x40	/* Direct Translated P2P */

These definitions are in the wrong place and have the wrong name ;-)

You've put them in the part of the file used for the PCI Express capability,
and named them as if they're part of the PCI Express capability.

>  /* Extended Capabilities (PCI-X 2.0 and Express) */
>  #define PCI_EXT_CAP_ID(header)		(header & 0x0000ffff)
>  #define PCI_EXT_CAP_VER(header)		((header >> 16) & 0xf)
> @@ -501,6 +509,7 @@
>  #define PCI_EXT_CAP_ID_VC	2
>  #define PCI_EXT_CAP_ID_DSN	3
>  #define PCI_EXT_CAP_ID_PWR	4
> +#define PCI_EXT_CAP_ID_ACS	13
>  #define PCI_EXT_CAP_ID_ARI	14
>  #define PCI_EXT_CAP_ID_ATS	15
>  #define PCI_EXT_CAP_ID_SRIOV	16
> @@ -661,4 +670,9 @@
>  #define  PCI_SRIOV_VFM_MO	0x2	/* Active.MigrateOut */
>  #define  PCI_SRIOV_VFM_AV	0x3	/* Active.Available */
>  
> +/* Access Control Service */
> +#define PCI_ACS_CAP		0x04	/* ACS Capability Register */

They should be down here instead and named like this:

+#define  PCI_ACS_SV		0x01	/* Source Validation */
+#define  PCI_ACS_TB		0x02	/* Translation Blocking */
+#define  PCI_ACS_RR		0x04	/* P2P Request Redirect */
+#define  PCI_ACS_CR		0x08	/* P2P Completion Redirect */
+#define  PCI_ACS_UF		0x10	/* Upstream Forwarding */
+#define  PCI_ACS_EC		0x20	/* P2P Egress Control */
+#define  PCI_ACS_DT		0x40	/* Direct Translated P2P */

Now ACS_ENABLED fits on one line again ;-)

#define ACS_ENABLED (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)

> +#define PCI_ACS_CTRL		0x06	/* ACS Control Register */
> +#define PCI_ACS_EGRESS_CTL_V	0x08	/* ACS Egress Control Vector */
> +
>  #endif /* LINUX_PCI_REGS_H */
diff mbox

Patch

=================================================================

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7b70312..e70abde 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1513,6 +1513,49 @@  void pci_enable_ari(struct pci_dev *dev)
 }
 
 /**
+ * pci_acs_enable - enable ACS if hardware support it
+ * @dev: the PCI device
+ */
+#define ACS_ENABLED (PCI_EXP_ACS_V|PCI_EXP_ACS_R|PCI_EXP_ACS_C|PCI_EXP_ACS_U)
+void pci_acs_init(struct pci_dev *dev)
+{
+	int pos;
+	u16 cap;
+	u16 ctrl;
+
+	if (!dev->is_pcie)
+		return;
+
+	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
+	if (!pos)
+		return;
+
+	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
+	if ((cap & ACS_ENABLED) != ACS_ENABLED)
+		dev_info(&dev->dev, "ACS P2P upstream not supported\n");
+
+	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
+
+	/* Source Validation */
+	if (cap & PCI_EXP_ACS_V)
+		ctrl |= PCI_EXP_ACS_V;
+
+	/* P2P Request Redirect */
+	if (cap & PCI_EXP_ACS_R)
+		ctrl |= PCI_EXP_ACS_R;
+
+	/* P2P Completion Redirect */
+	if (cap & PCI_EXP_ACS_C)
+		ctrl |= PCI_EXP_ACS_C;
+
+	/* Upstream Forwarding */
+	if (cap & PCI_EXP_ACS_U)
+		ctrl |= PCI_EXP_ACS_U;
+
+	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+}
+
+/**
  * pci_swizzle_interrupt_pin - swizzle INTx for device behind bridge
  * @dev: the PCI device
  * @pin: the INTx pin (1=INTA, 2=INTB, 3=INTD, 4=INTD)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 5ff4d25..1d8976d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -202,6 +202,7 @@  static inline int pci_ari_enabled(struct pci_bus *bus)
 {
 	return bus->self && bus->self->ari_enabled;
 }
+extern void pci_acs_init(struct pci_dev *dev);
 
 #ifdef CONFIG_PCI_QUIRKS
 extern int pci_is_reassigndev(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 40e75f6..4a5ec9e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -991,6 +991,9 @@  static void pci_init_capabilities(struct pci_dev *dev)
 
 	/* Single Root I/O Virtualization */
 	pci_iov_init(dev);
+
+	/* Access Control Service */
+	pci_acs_init(dev);
 }
 
 void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index fcaee42..90014ad 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -492,6 +492,14 @@ 
 #define PCI_EXP_LNKCTL2		48	/* Link Control 2 */
 #define PCI_EXP_SLTCTL2		56	/* Slot Control 2 */
 
+#define  PCI_EXP_ACS_V		0x01	/* Source Validation */
+#define  PCI_EXP_ACS_B		0x02	/* Translation Blocking */
+#define  PCI_EXP_ACS_R		0x04	/* P2P Request Redirect */
+#define  PCI_EXP_ACS_C		0x08	/* P2P Completion Redirect */
+#define  PCI_EXP_ACS_U		0x10	/* Upstream Forwarding */
+#define  PCI_EXP_ACS_E		0x20	/* P2P Egress Control */
+#define  PCI_EXP_ACS_T		0x40	/* Direct Translated P2P */
+
 /* Extended Capabilities (PCI-X 2.0 and Express) */
 #define PCI_EXT_CAP_ID(header)		(header & 0x0000ffff)
 #define PCI_EXT_CAP_VER(header)		((header >> 16) & 0xf)
@@ -501,6 +509,7 @@ 
 #define PCI_EXT_CAP_ID_VC	2
 #define PCI_EXT_CAP_ID_DSN	3
 #define PCI_EXT_CAP_ID_PWR	4
+#define PCI_EXT_CAP_ID_ACS	13
 #define PCI_EXT_CAP_ID_ARI	14
 #define PCI_EXT_CAP_ID_ATS	15
 #define PCI_EXT_CAP_ID_SRIOV	16
@@ -661,4 +670,9 @@ 
 #define  PCI_SRIOV_VFM_MO	0x2	/* Active.MigrateOut */
 #define  PCI_SRIOV_VFM_AV	0x3	/* Active.Available */
 
+/* Access Control Service */
+#define PCI_ACS_CAP		0x04	/* ACS Capability Register */
+#define PCI_ACS_CTRL		0x06	/* ACS Control Register */
+#define PCI_ACS_EGRESS_CTL_V	0x08	/* ACS Egress Control Vector */
+
 #endif /* LINUX_PCI_REGS_H */