Re: [bugzilla-dae...@bugzilla.kernel.org: [Bug 209149] New: "iommu/vt-d: Enable PCI ACS for platform opt in hint" makes NVMe config space not accessible after S3]

2020-09-23 Thread Rajat Jain via iommu
On Wed, Sep 23, 2020 at 9:19 AM Raj, Ashok wrote: > > Hi Bjorn > > > On Wed, Sep 23, 2020 at 11:03:27AM -0500, Bjorn Helgaas wrote: > > [+cc IOMMU and NVMe folks] > > > > Sorry, I forgot to forward this to linux-pci when it was first > > reported. > > > > Apparently this happens with v5.9-rc3,

Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-09-15 Thread Rajat Jain via iommu
Hello Bjorn, On Tue, Jul 14, 2020 at 1:19 PM Rajat Jain wrote: > > On Sat, Jul 11, 2020 at 12:53 PM Bjorn Helgaas wrote: > > > > On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote: > > > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > > > On Fri, Jul 10, 2020 at 03:29:22PM

Re: [PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS

2020-09-10 Thread Rajat Jain via iommu
Hello Bjorn, On Mon, Aug 17, 2020 at 3:50 PM Rajat Jain wrote: > > Hello Bjorn, > > > On Sat, Aug 1, 2020 at 5:30 PM Rajat Jain wrote: > > > > Hi Bjorn, > > > > > > On Tue, Jul 14, 2020 at 1:24 PM Rajat Jain wrote: > > > > > > On Tue, Jul 14, 2020 at 1:15 PM Rajat Jain wrote: > > > > > > > >

Re: [PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS

2020-08-01 Thread Rajat Jain via iommu
Hi Bjorn, On Tue, Jul 14, 2020 at 1:24 PM Rajat Jain wrote: > > On Tue, Jul 14, 2020 at 1:15 PM Rajat Jain wrote: > > > > The ACS "Translation Blocking" bit blocks the translated addresses from > > the devices. We don't expect such traffic from devices unless ATS is > > enabled on them. A

Re: [PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS

2020-08-01 Thread Rajat Jain via iommu
Hi Bjorn, On Tue, Jul 14, 2020 at 1:15 PM Rajat Jain wrote: > The ACS "Translation Blocking" bit blocks the translated addresses from > the devices. We don't expect such traffic from devices unless ATS is > enabled on them. A device sending such traffic without ATS enabled, > indicates

[PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS

2020-07-14 Thread Rajat Jain via iommu
The ACS "Translation Blocking" bit blocks the translated addresses from the devices. We don't expect such traffic from devices unless ATS is enabled on them. A device sending such traffic without ATS enabled, indicates malicious intent, and thus should be blocked. Enable PCI_ACS_TB by default for

Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-14 Thread Rajat Jain via iommu
On Sat, Jul 11, 2020 at 12:53 PM Bjorn Helgaas wrote: > > On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote: > > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote: > > > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat

Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-10 Thread Rajat Jain via iommu
Hello, On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote: > > Hi Bjorn > > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote: > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat Jain wrote: > > > When enabling ACS, enable translation blocking for external facing ports > > > and

[PATCH v4 2/4] PCI: Keep the ACS capability offset in device

2020-07-07 Thread Rajat Jain via iommu
Currently ACS capabiity is being looked up at a number of places. Read and store it once at bootup so that it can be used by all later. Signed-off-by: Rajat Jain --- v4: No change v3: fix commit log, remove forward declation of static function v2: Commit log cosmetic changes

[PATCH v4 1/4] PCI: Move pci_enable_acs() and its dependencies up in pci.c

2020-07-07 Thread Rajat Jain via iommu
Move pci_enable_acs() and the functions it depends on, further up in the source code to avoid having to forward declare it when we make it static in near future (next patch). No functional changes intended. Signed-off-by: Rajat Jain --- v4: Same as v3 v3: Initial version of the patch, created

[PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-07 Thread Rajat Jain via iommu
When enabling ACS, enable translation blocking for external facing ports and untrusted devices. Signed-off-by: Rajat Jain --- v4: Add braces to avoid warning from kernel robot print warning for only external-facing devices. v3: print warning if ACS_TB not supported on

[PATCH v4 3/4] PCI: Treat "external-facing" devices as internal

2020-07-07 Thread Rajat Jain via iommu
The "ExternalFacingPort" devices (root ports) are internal devices that sit on the internal system fabric. Ref: https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports Currently they were treated (marked as untrusted) at par with other external devices downstream

[PATCH v3 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-07 Thread Rajat Jain via iommu
When enabling ACS, enable translation blocking for external facing ports and untrusted devices. Signed-off-by: Rajat Jain --- v3: print warning if ACS_TB not supported on external-facing/untrusted ports. Minor code comments fixes. v2: Commit log change drivers/pci/pci.c| 7 +++

Re: [PATCH v2 5/7] driver core: Add device location to "struct device" and expose it in sysfs

2020-07-07 Thread Rajat Jain via iommu
On Wed, Jul 1, 2020 at 10:23 PM Oliver O'Halloran wrote: > > On Thu, Jul 2, 2020 at 4:07 AM Rajat Jain wrote: > > > > *snip* > > > > > > I guess it would make sense to have an attribute for user space to > > > > write to in order to make the kernel reject device plug-in events > > > > coming

[PATCH v3 3/4] PCI: Treat "external-facing" devices as internal

2020-07-06 Thread Rajat Jain via iommu
The "ExternalFacingPort" devices (root ports) are internal devices that sit on the internal system fabric. Ref: https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports Currently they were treated (marked as untrusted) at par with other external devices downstream

[PATCH v3 1/4] PCI: Move pci_enable_acs() and its dependencies up in pci.c

2020-07-06 Thread Rajat Jain via iommu
Move pci_enable_acs() and the functions it depends on, further up in the source code to avoid having to forward declare it when we make it static in near future (next patch). No functional changes intended. Signed-off-by: Rajat Jain --- v3: Initial version of the patch, created per Bjorn's

[PATCH v3 2/4] PCI: Keep the ACS capability offset in device

2020-07-06 Thread Rajat Jain via iommu
Currently ACS capabiity is being looked up at a number of places. Read and store it once at bootup so that it can be used by all later. Signed-off-by: Rajat Jain --- v3: fix commit log, remove forward declation of static function v2: Commit log cosmetic changes drivers/pci/p2pdma.c | 2 +-

Re: [PATCH v2 2/7] PCI: Set "untrusted" flag for truly external devices only

2020-07-06 Thread Rajat Jain via iommu
Hello Bjorn, On Mon, Jul 6, 2020 at 4:30 PM Bjorn Helgaas wrote: > > On Mon, Jul 06, 2020 at 03:31:47PM -0700, Rajat Jain wrote: > > On Mon, Jul 6, 2020 at 9:38 AM Bjorn Helgaas wrote: > > > On Mon, Jun 29, 2020 at 09:49:38PM -0700, Rajat Jain wrote: > > > > > -static void

Re: [PATCH v2 4/7] PCI: Add device even if driver attach failed

2020-07-06 Thread Rajat Jain via iommu
On Tue, Jun 30, 2020 at 1:02 AM Greg Kroah-Hartman wrote: > > On Mon, Jun 29, 2020 at 09:49:40PM -0700, Rajat Jain wrote: > > device_attach() returning failure indicates a driver error while trying to > > probe the device. In such a scenario, the PCI device should still be added > > in the system

[PATCH RESEND v2] PCI: Add device even if driver attach failed

2020-07-06 Thread Rajat Jain via iommu
device_attach() returning failure indicates a driver error while trying to probe the device. In such a scenario, the PCI device should still be added in the system and be visible to the user. This patch partially reverts: commit ab1a187bba5c ("PCI: Check device_attach() return value always")

Re: [PATCH v2 3/7] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-06 Thread Rajat Jain via iommu
On Mon, Jul 6, 2020 at 10:07 AM Bjorn Helgaas wrote: > > On Mon, Jun 29, 2020 at 09:49:39PM -0700, Rajat Jain wrote: > > When enabling ACS, enable translation blocking for external facing ports > > and untrusted devices. > > > > Signed-off-by: Rajat Jain > > --- > > v2: Commit log change > > > >

Re: [PATCH v2 3/7] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-07-06 Thread Rajat Jain via iommu
On Mon, Jul 6, 2020 at 9:45 AM Bjorn Helgaas wrote: > > On Mon, Jun 29, 2020 at 09:49:39PM -0700, Rajat Jain wrote: > > When enabling ACS, enable translation blocking for external facing ports > > and untrusted devices. > > > > Signed-off-by: Rajat Jain > > --- > > v2: Commit log change > > > >

Re: [PATCH v2 2/7] PCI: Set "untrusted" flag for truly external devices only

2020-07-06 Thread Rajat Jain via iommu
Hello, On Mon, Jul 6, 2020 at 9:38 AM Bjorn Helgaas wrote: > > On Mon, Jun 29, 2020 at 09:49:38PM -0700, Rajat Jain wrote: > > The "ExternalFacing" devices (root ports) are still internal devices that > > sit on the internal system fabric and thus trusted. Currently they were > > being marked

Re: [PATCH v2 0/7] Tighten PCI security, expose dev location in sysfs

2020-07-06 Thread Rajat Jain via iommu
On Sat, Jul 4, 2020 at 4:44 AM Pavel Machek wrote: > > Hi! > > > * The first 3 patches tighten the PCI security using ACS, and take care > > of a border case. > > * The 4th patch takes care of PCI bug. > > * 5th and 6th patches expose a device's location into the sysfs to allow > > admin to

Re: [PATCH v2 1/7] PCI: Keep the ACS capability offset in device

2020-07-06 Thread Rajat Jain via iommu
Hi Bjorn, Thanks for taking a look. On Mon, Jul 6, 2020 at 8:58 AM Bjorn Helgaas wrote: > > On Mon, Jun 29, 2020 at 09:49:37PM -0700, Rajat Jain wrote: > > Currently this is being looked up at a number of places. Read and store it > > once at bootup so that it can be used by all later. > >

Re: [PATCH v2 5/7] driver core: Add device location to "struct device" and expose it in sysfs

2020-07-01 Thread Rajat Jain via iommu
Hello, On Tue, Jun 30, 2020 at 10:00 AM Greg Kroah-Hartman wrote: > > On Tue, Jun 30, 2020 at 06:08:31PM +0200, Rafael J. Wysocki wrote: > > On Tue, Jun 30, 2020 at 5:38 PM Greg Kroah-Hartman > > wrote: > > > > > > On Tue, Jun 30, 2020 at 03:00:34PM +0200, Rafael J. Wysocki wrote: > > > > On

[PATCH v2 5/7] driver core: Add device location to "struct device" and expose it in sysfs

2020-06-29 Thread Rajat Jain via iommu
Add a new (optional) field to denote the physical location of a device in the system, and expose it in sysfs. This was discussed here: https://lore.kernel.org/linux-acpi/20200618184621.ga446...@kroah.com/ (The primary choice for attribute name i.e. "location" is already exposed as an ABI

[PATCH v2 7/7] PCI: Add parameter to disable attaching external devices

2020-06-29 Thread Rajat Jain via iommu
Introduce a PCI parameter that disables the automatic attachment of external devices to their drivers. This is needed to allow an admin to control which drivers he wants to allow on external ports. For more context, see threads at:

[PATCH v2 3/7] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-06-29 Thread Rajat Jain via iommu
When enabling ACS, enable translation blocking for external facing ports and untrusted devices. Signed-off-by: Rajat Jain --- v2: Commit log change drivers/pci/pci.c| 4 drivers/pci/quirks.c | 11 +++ 2 files changed, 15 insertions(+) diff --git a/drivers/pci/pci.c

[PATCH v2 4/7] PCI: Add device even if driver attach failed

2020-06-29 Thread Rajat Jain via iommu
device_attach() returning failure indicates a driver error while trying to probe the device. In such a scenario, the PCI device should still be added in the system and be visible to the user. This patch partially reverts: commit ab1a187bba5c ("PCI: Check device_attach() return value always")

[PATCH v2 6/7] PCI: Move pci_dev->untrusted logic to use device location instead

2020-06-29 Thread Rajat Jain via iommu
The firmware was provinding "ExternalFacing" attribute on PCI root ports, to allow the kernel to mark devices behind it as external. Note that the firmware provides an immutable, read-only property, i.e. the location of the device. The use of (external) device location as hint for (dis)trust, is

[PATCH v2 2/7] PCI: Set "untrusted" flag for truly external devices only

2020-06-29 Thread Rajat Jain via iommu
The "ExternalFacing" devices (root ports) are still internal devices that sit on the internal system fabric and thus trusted. Currently they were being marked untrusted. This patch uses the platform flag to identify the external facing devices and then use it to mark any downstream devices as

[PATCH v2 1/7] PCI: Keep the ACS capability offset in device

2020-06-29 Thread Rajat Jain via iommu
Currently this is being looked up at a number of places. Read and store it once at bootup so that it can be used by all later. Signed-off-by: Rajat Jain --- v2: Commit log cosmetic changes drivers/pci/p2pdma.c | 2 +- drivers/pci/pci.c| 21 + drivers/pci/pci.h| 2

[PATCH v2 0/7] Tighten PCI security, expose dev location in sysfs

2020-06-29 Thread Rajat Jain via iommu
This is a set of loosely related patches most of whom emerged out of discussion in the following threads. In a nutshell the goal was to allow an administrator to specify which driver he wants to allow on external ports, and a strategy was chalked out:

Re: [PATCH 1/2] pci: Add pci device even if the driver failed to attach

2020-06-26 Thread Rajat Jain via iommu
On Fri, Jun 26, 2020 at 8:39 AM Bjorn Helgaas wrote: > > Nit: when you update these patches, can you run "git log --oneline > drivers/pci/bus.c" and make your subject lines match the convention? Sorry, will do. > E.g., > > PCI: Add device even if driver attach failed > > On Thu, Jun 25, 2020

Re: [PATCH 2/2] pci: Add parameter to disable attaching untrusted devices

2020-06-26 Thread Rajat Jain via iommu
Hello, Thanks for taking a look. On Fri, Jun 26, 2020 at 7:18 AM Greg Kroah-Hartman wrote: > > On Thu, Jun 25, 2020 at 05:27:10PM -0700, Rajat Jain wrote: > > Introduce a PCI parameter that disables the automatic attachment of > > untrusted devices to their drivers. > > > > Signed-off-by: Rajat

[PATCH 2/2] pci: Add parameter to disable attaching untrusted devices

2020-06-25 Thread Rajat Jain via iommu
Introduce a PCI parameter that disables the automatic attachment of untrusted devices to their drivers. Signed-off-by: Rajat Jain --- Context: I set out to implement the approach outlined in https://lkml.org/lkml/2020/6/9/1331 https://lkml.org/lkml/2020/6/15/1453 But to my

[PATCH 1/2] pci: Add pci device even if the driver failed to attach

2020-06-25 Thread Rajat Jain via iommu
device_attach() returning failure indicates a driver error while trying to probe the device. In such a scenario, the PCI device should still be added in the system and be visible to the user. This patch partially reverts: commit ab1a187bba5c ("PCI: Check device_attach() return value always")

Re: [PATCH 3/4] pci: acs: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-06-22 Thread Rajat Jain via iommu
Hi Ashok, On Fri, Jun 19, 2020 at 9:10 AM Raj, Ashok wrote: > > Hi Rajat > > > On Mon, Jun 15, 2020 at 06:17:41PM -0700, Rajat Jain wrote: > > When enabling ACS, currently the bit "translation blocking" was > > not getting changed at all. Set it to disable translation blocking > > Maybe you

Re: [PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-18 Thread Rajat Jain via iommu
Hi Bjorn, All, Thank you so much for your helpful review and inputs. On Mon, Jun 15, 2020 at 6:17 PM Rajat Jain wrote: > > This is needed to allow the userspace to determine when an untrusted > device has been added, and thus allowing it to bind the driver manually > to it, if it so wishes.

Re: [PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-18 Thread Rajat Jain via iommu
Thanks Greg and Andy for your continued inputs, and thanks Ashok for chiming in. On Thu, Jun 18, 2020 at 9:23 AM Raj, Ashok wrote: > > Hi Greg, > > > On Thu, Jun 18, 2020 at 06:02:12PM +0200, Greg Kroah-Hartman wrote: > > On Thu, Jun 18, 2020 at 08:03:49AM -0700, Rajat Jain wrote: > > > Hello, >

Re: [PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-18 Thread Rajat Jain via iommu
Hello, On Thu, Jun 18, 2020 at 2:14 AM Andy Shevchenko wrote: > > On Thu, Jun 18, 2020 at 11:36 AM Greg Kroah-Hartman > wrote: > > > > On Thu, Jun 18, 2020 at 11:12:56AM +0300, Andy Shevchenko wrote: > > > On Wed, Jun 17, 2020 at 10:56 PM Rajat Jain wrote: > > > > On Wed, Jun 17, 2020 at 12:31

Re: [PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-17 Thread Rajat Jain via iommu
Hi Greg, Christoph, On Wed, Jun 17, 2020 at 12:31 AM Christoph Hellwig wrote: > > On Tue, Jun 16, 2020 at 12:27:35PM -0700, Rajat Jain wrote: > > Need clarification. The flag "untrusted" is currently a part of > > pci_dev struct, and is populated within the PCI subsystem. > > Yes, and that is

Re: [PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-16 Thread Rajat Jain via iommu
On Tue, Jun 16, 2020 at 12:32 AM Christoph Hellwig wrote: > > On Mon, Jun 15, 2020 at 06:17:42PM -0700, Rajat Jain via iommu wrote: > > This is needed to allow the userspace to determine when an untrusted > > device has been added, and thus allowing it to bind t

[PATCH 2/4] pci: set "untrusted" flag for truly external devices only

2020-06-15 Thread Rajat Jain via iommu
The "ExternalFacing" devices (root ports) are still internal devices that sit on the internal system fabric and thus trusted. Currently they were being marked untrusted - likely as an unintended border case. This patch uses the platform flag to identify the external facing devices and then use it

[PATCH 4/4] pci: export untrusted attribute in sysfs

2020-06-15 Thread Rajat Jain via iommu
This is needed to allow the userspace to determine when an untrusted device has been added, and thus allowing it to bind the driver manually to it, if it so wishes. This is being done as part of the approach discussed at https://lkml.org/lkml/2020/6/9/1331 Signed-off-by: Rajat Jain ---

[PATCH 1/4] pci: Keep the ACS capability offset in device

2020-06-15 Thread Rajat Jain via iommu
Currently this is being looked up at a number of places. Read and store it once at bootup so that it can be used by all later. Signed-off-by: Rajat Jain --- drivers/pci/p2pdma.c | 2 +- drivers/pci/pci.c| 21 + drivers/pci/pci.h| 2 +- drivers/pci/probe.c | 2 +-

[PATCH 3/4] pci: acs: Enable PCI_ACS_TB for untrusted/external-facing devices

2020-06-15 Thread Rajat Jain via iommu
When enabling ACS, currently the bit "translation blocking" was not getting changed at all. Set it to disable translation blocking too for all external facing or untrusted devices. This is OK because ATS is only allowed on internal devces. Signed-off-by: Rajat Jain --- drivers/pci/pci.c| 4

[PATCH v4] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-03 Thread Rajat Jain via iommu
Currently, an external malicious PCI device can masquerade the VID:PID of faulty gfx devices, and thus apply iommu quirks to effectively disable the IOMMU restrictions for itself. Thus we need to ensure that the device we are applying quirks to, is indeed an internal trusted device.

Re: [PATCH v3] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-03 Thread Rajat Jain via iommu
On Tue, Jun 2, 2020 at 10:30 PM Mika Westerberg wrote: > > On Tue, Jun 02, 2020 at 04:26:02PM -0700, Rajat Jain wrote: > > +static bool risky_device(struct pci_dev *pdev) > > +{ > > + if (pdev->untrusted) { > > + pci_warn(pdev, > > + "Skipping IOMMU quirk for

[PATCH v3] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-02 Thread Rajat Jain via iommu
Currently, an external malicious PCI device can masquerade the VID:PID of faulty gfx devices, and thus apply iommu quirks to effectively disable the IOMMU restrictions for itself. Thus we need to ensure that the device we are applying quirks to, is indeed an internal trusted device.

Re: [PATCH] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-02 Thread Rajat Jain via iommu
Hi MIka, Thanks for taking a look. On Tue, Jun 2, 2020 at 2:50 AM Mika Westerberg wrote: > > On Mon, Jun 01, 2020 at 10:45:17PM -0700, Rajat Jain wrote: > > Currently, an external malicious PCI device can masquerade the VID:PID > > of faulty gfx devices, and thus apply iommu quirks to

[PATCH v2] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-02 Thread Rajat Jain via iommu
Currently, an external malicious PCI device can masquerade the VID:PID of faulty gfx devices, and thus apply iommu quirks to effectively disable the IOMMU restrictions for itself. Thus we need to ensure that the device we are applying quirks to, is indeed an internal trusted device.

[PATCH] iommu/vt-d: Don't apply gfx quirks to untrusted devices

2020-06-02 Thread Rajat Jain via iommu
Currently, an external malicious PCI device can masquerade the VID:PID of faulty gfx devices, and thus apply iommu quirks to effectively disable the IOMMU restrictions for itself. Thus we need to ensure that the device we are applying quirks to, is indeed an internal trusted device.