From: Lu Baolu <baolu.lu@linux.intel.com>
To: Rajat Jain <rajatja@google.com>,
David Woodhouse <dwmw2@infradead.org>,
Joerg Roedel <joro@8bytes.org>,
Bjorn Helgaas <bhelgaas@google.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Len Brown <lenb@kernel.org>,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
Raj Ashok <ashok.raj@intel.com>,
lalithambika.krishnakumar@intel.com,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Jean-Philippe Brucker <jean-philippe@linaro.org>,
Prashant Malani <pmalani@google.com>,
Benson Leung <bleung@google.com>, Todd Broch <tbroch@google.com>,
Alex Levin <levinale@google.com>,
Mattias Nissler <mnissler@google.com>,
Rajat Jain <rajatxjain@gmail.com>,
Bernie Keany <bernie.keany@intel.com>,
Aaron Durbin <adurbin@google.com>,
Diego Rivas <diegorivas@google.com>,
Duncan Laurie <dlaurie@google.com>,
Furquan Shaikh <furquan@google.com>,
Jesse Barnes <jsbarnes@google.com>,
Christian Kellner <christian@kellner.me>,
Alex Williamson <alex.williamson@redhat.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
oohall@gmail.com, Saravana Kannan <saravanak@google.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Arnd Bergmann <arnd@arndb.de>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>
Subject: Re: [PATCH v2 6/7] PCI: Move pci_dev->untrusted logic to use device location instead
Date: Tue, 30 Jun 2020 15:39:41 +0800 [thread overview]
Message-ID: <74181d77-ea1a-2289-1a42-5257ea820364@linux.intel.com> (raw)
In-Reply-To: <20200630044943.3425049-7-rajatja@google.com>
On 2020/6/30 12:49, Rajat Jain wrote:
> 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 a
> decision that IOMMU drivers have taken, so we should call it out
> explicitly.
>
> This patch removes the pci_dev->untrusted and changes the users of it to
> use device core provided device location instead. This location is
> populated by PCI using the same "ExternalFacing" firmware info. Any
> device not behind the "ExternalFacing" bridges are marked internal and
> the ones behind such bridges are markes external.
>
> Signed-off-by: Rajat Jain <rajatja@google.com>
For changes in Intel VT-d driver,
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Best regards,
baolu
> ---
> v2: (Initial version)
>
> drivers/iommu/intel/iommu.c | 31 +++++++++++++++++++++----------
> drivers/pci/ats.c | 2 +-
> drivers/pci/pci-driver.c | 1 +
> drivers/pci/pci.c | 2 +-
> drivers/pci/probe.c | 18 ++++++++++++------
> drivers/pci/quirks.c | 2 +-
> include/linux/pci.h | 10 +---------
> 7 files changed, 38 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 1ccb224f82496..ca66a196f5e97 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -168,6 +168,22 @@ static inline unsigned long virt_to_dma_pfn(void *p)
> return page_to_dma_pfn(virt_to_page(p));
> }
>
> +static inline bool untrusted_dev(struct device *dev)
> +{
> + /*
> + * Treat all external PCI devices as untrusted devices. These are the
> + * devices behing marked behind external-facing bridges as marked by
> + * the firmware. The untrusted devices are the ones that can potentially
> + * execute DMA attacks and similar. They are typically connected through
> + * external thunderbolt ports. When an IOMMU is enabled they should be
> + * getting full mappings to ensure they cannot access arbitrary memory.
> + */
> + if (dev_is_pci(dev) && dev_is_external(dev))
> + return true;
> +
> + return false;
> +}
> +
> /* global iommu list, set NULL for ignored DMAR units */
> static struct intel_iommu **g_iommus;
>
> @@ -383,8 +399,7 @@ struct device_domain_info *get_domain_info(struct device *dev)
> DEFINE_SPINLOCK(device_domain_lock);
> static LIST_HEAD(device_domain_list);
>
> -#define device_needs_bounce(d) (!intel_no_bounce && dev_is_pci(d) && \
> - to_pci_dev(d)->untrusted)
> +#define device_needs_bounce(d) (!intel_no_bounce && untrusted_dev(d))
>
> /*
> * Iterate over elements in device_domain_list and call the specified
> @@ -2830,7 +2845,7 @@ static int device_def_domain_type(struct device *dev)
> * Prevent any device marked as untrusted from getting
> * placed into the statically identity mapping domain.
> */
> - if (pdev->untrusted)
> + if (untrusted_dev(dev))
> return IOMMU_DOMAIN_DMA;
>
> if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev))
> @@ -3464,7 +3479,6 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
> unsigned long iova_pfn;
> struct intel_iommu *iommu;
> struct page *freelist;
> - struct pci_dev *pdev = NULL;
>
> domain = find_domain(dev);
> BUG_ON(!domain);
> @@ -3477,11 +3491,8 @@ static void intel_unmap(struct device *dev, dma_addr_t dev_addr, size_t size)
> start_pfn = mm_to_dma_pfn(iova_pfn);
> last_pfn = start_pfn + nrpages - 1;
>
> - if (dev_is_pci(dev))
> - pdev = to_pci_dev(dev);
> -
> freelist = domain_unmap(domain, start_pfn, last_pfn);
> - if (intel_iommu_strict || (pdev && pdev->untrusted) ||
> + if (intel_iommu_strict || untrusted_dev(dev) ||
> !has_iova_flush_queue(&domain->iovad)) {
> iommu_flush_iotlb_psi(iommu, domain, start_pfn,
> nrpages, !freelist, 0);
> @@ -4743,7 +4754,7 @@ static inline bool has_untrusted_dev(void)
> struct pci_dev *pdev = NULL;
>
> for_each_pci_dev(pdev)
> - if (pdev->untrusted || pdev->external_facing)
> + if (pdev->external_facing || untrusted_dev(&pdev->dev))
> return true;
>
> return false;
> @@ -6036,7 +6047,7 @@ intel_iommu_domain_set_attr(struct iommu_domain *domain,
> */
> static bool risky_device(struct pci_dev *pdev)
> {
> - if (pdev->untrusted) {
> + if (untrusted_dev(&pdev->dev)) {
> pci_info(pdev,
> "Skipping IOMMU quirk for dev [%04X:%04X] on untrusted PCI link\n",
> pdev->vendor, pdev->device);
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index b761c1f72f672..ebd370f4d5b06 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -42,7 +42,7 @@ bool pci_ats_supported(struct pci_dev *dev)
> if (!dev->ats_cap)
> return false;
>
> - return (dev->untrusted == 0);
> + return (!dev_is_external(&dev->dev));
> }
> EXPORT_SYMBOL_GPL(pci_ats_supported);
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index da6510af12214..9608053a8a62c 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1630,6 +1630,7 @@ struct bus_type pci_bus_type = {
> .pm = PCI_PM_OPS_PTR,
> .num_vf = pci_bus_num_vf,
> .dma_configure = pci_dma_configure,
> + .supports_site = true,
> };
> EXPORT_SYMBOL(pci_bus_type);
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 79853b52658a2..35f25ac39167b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3330,7 +3330,7 @@ static void pci_std_enable_acs(struct pci_dev *dev)
> /* Upstream Forwarding */
> ctrl |= (cap & PCI_ACS_UF);
>
> - if (dev->external_facing || dev->untrusted)
> + if (dev->external_facing || dev_is_external(&dev->dev))
> /* Translation Blocking */
> ctrl |= (cap & PCI_ACS_TB);
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8c40c00413e74..1609329cc5b4e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1543,17 +1543,23 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
> }
> }
>
> -static void set_pcie_untrusted(struct pci_dev *dev)
> +static void set_pcie_dev_site(struct pci_dev *dev)
> {
> struct pci_dev *parent;
>
> /*
> - * If the upstream bridge is untrusted we treat this device
> - * untrusted as well.
> + * All devices are considered internal by default, unless behind an
> + * external-facing bridge, as marked by the firmware.
> + */
> + dev_set_site(&dev->dev, SITE_INTERNAL);
> +
> + /*
> + * If the upstream bridge is external or external-facing, this device
> + * is also external.
> */
> parent = pci_upstream_bridge(dev);
> - if (parent && (parent->untrusted || parent->external_facing))
> - dev->untrusted = true;
> + if (parent && (parent->external_facing || dev_is_external(&parent->dev)))
> + dev_set_site(&dev->dev, SITE_EXTERNAL);
> }
>
> /**
> @@ -1814,7 +1820,7 @@ int pci_setup_device(struct pci_dev *dev)
> /* Need to have dev->cfg_size ready */
> set_pcie_thunderbolt(dev);
>
> - set_pcie_untrusted(dev);
> + set_pcie_dev_site(dev);
>
> /* "Unknown power state" */
> dev->current_state = PCI_UNKNOWN;
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6294adeac4049..65d0b8745c915 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -4980,7 +4980,7 @@ static int pci_quirk_enable_intel_spt_pch_acs(struct pci_dev *dev)
> ctrl |= (cap & PCI_ACS_CR);
> ctrl |= (cap & PCI_ACS_UF);
>
> - if (dev->external_facing || dev->untrusted)
> + if (dev->external_facing || dev_is_external(&dev->dev))
> /* Translation Blocking */
> ctrl |= (cap & PCI_ACS_TB);
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fe1bc603fda40..8bb5065e5aed2 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -424,20 +424,12 @@ struct pci_dev {
> unsigned int is_hotplug_bridge:1;
> unsigned int shpc_managed:1; /* SHPC owned by shpchp */
> unsigned int is_thunderbolt:1; /* Thunderbolt controller */
> - /*
> - * Devices marked being untrusted are the ones that can potentially
> - * execute DMA attacks and similar. They are typically connected
> - * through external ports such as Thunderbolt but not limited to
> - * that. When an IOMMU is enabled they should be getting full
> - * mappings to make sure they cannot access arbitrary memory.
> - */
> - unsigned int untrusted:1;
> /*
> * Devices are marked as external-facing using info from platform
> * (ACPI / devicetree). An external-facing device is still an internal
> * trusted device, but it faces external untrusted devices. Thus any
> * devices enumerated downstream an external-facing device is marked
> - * as untrusted.
> + * as external device.
> */
> unsigned int external_facing:1;
> unsigned int broken_intx_masking:1; /* INTx masking can't be used */
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-06-30 7:40 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-30 4:49 [PATCH v2 0/7] Tighten PCI security, expose dev location in sysfs Rajat Jain via iommu
2020-06-30 4:49 ` [PATCH v2 1/7] PCI: Keep the ACS capability offset in device Rajat Jain via iommu
2020-07-06 15:58 ` Bjorn Helgaas
2020-07-06 22:16 ` Rajat Jain via iommu
2020-07-06 23:18 ` Bjorn Helgaas
2020-06-30 4:49 ` [PATCH v2 2/7] PCI: Set "untrusted" flag for truly external devices only Rajat Jain via iommu
2020-06-30 7:38 ` Lu Baolu
2020-06-30 7:55 ` Greg Kroah-Hartman
2020-07-06 16:41 ` Bjorn Helgaas
2020-07-06 18:48 ` Greg Kroah-Hartman
2020-07-06 16:38 ` Bjorn Helgaas
2020-07-06 22:31 ` Rajat Jain via iommu
2020-07-06 23:30 ` Bjorn Helgaas
2020-07-06 23:40 ` Rajat Jain via iommu
2020-06-30 4:49 ` [PATCH v2 3/7] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices Rajat Jain via iommu
2020-07-06 16:45 ` Bjorn Helgaas
2020-07-06 23:12 ` Rajat Jain via iommu
2020-07-06 17:07 ` Bjorn Helgaas
2020-07-06 23:19 ` Rajat Jain via iommu
2020-06-30 4:49 ` [PATCH v2 4/7] PCI: Add device even if driver attach failed Rajat Jain via iommu
2020-06-30 8:02 ` Greg Kroah-Hartman
2020-07-06 23:35 ` Rajat Jain via iommu
2020-06-30 4:49 ` [PATCH v2 5/7] driver core: Add device location to "struct device" and expose it in sysfs Rajat Jain via iommu
2020-06-30 8:01 ` Greg Kroah-Hartman
2020-06-30 10:49 ` Heikki Krogerus
2020-06-30 12:52 ` Greg Kroah-Hartman
2020-06-30 13:00 ` Rafael J. Wysocki
2020-06-30 15:38 ` Greg Kroah-Hartman
2020-06-30 16:08 ` Rafael J. Wysocki
2020-06-30 17:00 ` Greg Kroah-Hartman
2020-07-01 18:06 ` Rajat Jain via iommu
2020-07-02 5:23 ` Oliver O'Halloran
2020-07-02 7:32 ` Greg Kroah-Hartman
2020-07-02 8:40 ` Oliver O'Halloran
2020-07-02 8:52 ` Greg Kroah-Hartman
2020-07-02 8:53 ` Greg Kroah-Hartman
2020-07-07 6:03 ` Rajat Jain via iommu
2020-06-30 17:43 ` Saravana Kannan via iommu
2020-06-30 4:49 ` [PATCH v2 6/7] PCI: Move pci_dev->untrusted logic to use device location instead Rajat Jain via iommu
2020-06-30 7:39 ` Lu Baolu [this message]
2020-06-30 4:49 ` [PATCH v2 7/7] PCI: Add parameter to disable attaching external devices Rajat Jain via iommu
2020-07-04 11:44 ` [PATCH v2 0/7] Tighten PCI security, expose dev location in sysfs Pavel Machek
2020-07-06 22:18 ` Rajat Jain via iommu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74181d77-ea1a-2289-1a42-5257ea820364@linux.intel.com \
--to=baolu.lu@linux.intel.com \
--cc=adurbin@google.com \
--cc=alex.williamson@redhat.com \
--cc=arnd@arndb.de \
--cc=ashok.raj@intel.com \
--cc=bernie.keany@intel.com \
--cc=bhelgaas@google.com \
--cc=bleung@google.com \
--cc=christian@kellner.me \
--cc=diegorivas@google.com \
--cc=dlaurie@google.com \
--cc=dwmw2@infradead.org \
--cc=furquan@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=joro@8bytes.org \
--cc=jsbarnes@google.com \
--cc=lalithambika.krishnakumar@intel.com \
--cc=lenb@kernel.org \
--cc=levinale@google.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=mnissler@google.com \
--cc=oohall@gmail.com \
--cc=pmalani@google.com \
--cc=rajatja@google.com \
--cc=rajatxjain@gmail.com \
--cc=rjw@rjwysocki.net \
--cc=saravanak@google.com \
--cc=suzuki.poulose@arm.com \
--cc=tbroch@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).