[v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[Joshua Peraza]

This patchset rebases two previously posted patches supporting
recognition of Microsoft's DmaProperty.

v8: Joshua renames untrusted_dma to requires_dma_protection and updates
some comments, reducing use of the word "trust" to refer to PCI devices
and matching the word choice in Microsoft's documentation.

v7: Rajat updates a comment with Robin's suggestion. Joshua re-sends and
Greg requests clarity and documentation on why untrusted_dma is the
right name.

v6: Rajat renames pci_dev_has_dma_property and links to Microsoft's
documentation in the commit message. Robin suggests clarifying a
comment.

v5: Rajat changes the name to untrusted_dma. Bjorn suggesting changing
another function's name pci_acpi_check_for_dma_protection to
pci_dev_has_dma_property and seeks clarified documentation.

v4: Rajat changes the name to poses_dma_risk. Christoph suggests this
name doesn't capture the intent as well as untrusted_dma and Rafael
agrees.

v1,v2,v3: Greg suggests that (un)trusted is the wrong word for referring
to PCI devices, recommending a name something like "platform wants to
protect dma access for this device."


Rajat Jain (2):
  PCI/ACPI: Support Microsoft's "DmaProperty"
  PCI: Rename pci_dev->untrusted to pci_dev->requires_dma_protection

 drivers/acpi/property.c     |  3 +++
 drivers/iommu/amd/iommu.c   |  3 +--
 drivers/iommu/dma-iommu.c   | 16 ++++++++--------
 drivers/iommu/intel/iommu.c | 10 +++++-----
 drivers/iommu/iommu.c       |  5 ++---
 drivers/pci/ats.c           |  2 +-
 drivers/pci/pci-acpi.c      | 22 ++++++++++++++++++++++
 drivers/pci/pci.c           |  2 +-
 drivers/pci/probe.c         |  8 ++++----
 drivers/pci/quirks.c        |  4 ++--
 include/linux/pci.h         |  7 ++++---
 11 files changed, 53 insertions(+), 29 deletions(-)


base-commit: 2d5404caa8c7bb5c4e0435f94b28834ae5456623
-- 
2.47.0.338.g60cca15819-goog

[PATCH 1/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[Joshua Peraza]

From: Rajat Jain <rajatja@google.com>

The "DmaProperty" is supported and currently documented and used by
Microsoft [link 1 below], to flag internal PCIe root ports that need
DMA protection [link 2 below]. We have discussed with them and reached
a common understanding that they shall change their MSDN documentation
to say that the same property can be used to protect any PCI device,
and not just internal PCIe root ports (since there is no point
introducing yet another property for arbitrary PCI devices). This helps
with security from internal devices that offer an attack surface for
DMA attacks (e.g. internal network devices).

Support DmaProperty to mark DMA from a PCI device as untrusted.

Link: [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-internal-pcie-ports-accessible-to-users-and-requiring-dma-protection
Link: [2] https://docs.microsoft.com/en-us/windows/security/information-protection/kernel-dma-protection-for-thunderbolt
Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Joshua Peraza <jperaza@google.com>
---
 drivers/acpi/property.c |  3 +++
 drivers/pci/pci-acpi.c  | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 80a52a4e66dd..139e042ad2cb 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -56,6 +56,9 @@ static const guid_t prp_guids[] = {
 	/* Storage device needs D3 GUID: 5025030f-842f-4ab4-a561-99a5189762d0 */
 	GUID_INIT(0x5025030f, 0x842f, 0x4ab4,
 		  0xa5, 0x61, 0x99, 0xa5, 0x18, 0x97, 0x62, 0xd0),
+	/* DmaProperty for PCI devices GUID: 70d24161-6dd5-4c9e-8070-705531292865 */
+	GUID_INIT(0x70d24161, 0x6dd5, 0x4c9e,
+		  0x80, 0x70, 0x70, 0x55, 0x31, 0x29, 0x28, 0x65),
 };
 
 /* ACPI _DSD data subnodes GUID [1]: dbb8e3e6-5886-4ba6-8795-1319f52a966b */
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index af370628e583..a457ae3e811a 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1438,12 +1438,34 @@ static void pci_acpi_set_external_facing(struct pci_dev *dev)
 		dev->external_facing = 1;
 }
 
+static int pci_dev_has_dma_property(struct pci_dev *dev)
+{
+	struct acpi_device *adev;
+	const union acpi_object *obj;
+
+	adev = ACPI_COMPANION(&dev->dev);
+	if (!adev)
+		return 0;
+
+	/*
+	 * Property used by Microsoft Windows to enforce IOMMU DMA
+	 * protection from any device, that the system may not fully trust;
+	 * we'll honour it the same way.
+	 */
+	if (!acpi_dev_get_property(adev, "DmaProperty", ACPI_TYPE_INTEGER,
+				   &obj) && obj->integer.value == 1)
+		return 1;
+
+	return 0;
+}
+
 void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 
 	pci_acpi_optimize_delay(pci_dev, adev->handle);
 	pci_acpi_set_external_facing(pci_dev);
+	pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev);
 	pci_acpi_add_edr_notifier(pci_dev);
 
 	pci_acpi_add_pm_notifier(adev, pci_dev);
-- 
2.47.0.338.g60cca15819-goog

[PATCH 2/2] PCI: Rename pci_dev->untrusted to pci_dev->requires_dma_protection

[Joshua Peraza]

From: Rajat Jain <rajatja@google.com>

Rename the field to make it more clear, that the device can execute DMA
attacks on the system, and thus the system may need protection from
such attacks from this device.

No functional change intended.

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Joshua Peraza <jperaza@google.com>
---
 drivers/iommu/amd/iommu.c   |  3 +--
 drivers/iommu/dma-iommu.c   | 16 ++++++++--------
 drivers/iommu/intel/iommu.c | 10 +++++-----
 drivers/iommu/iommu.c       |  5 ++---
 drivers/pci/ats.c           |  2 +-
 drivers/pci/pci-acpi.c      |  2 +-
 drivers/pci/pci.c           |  2 +-
 drivers/pci/probe.c         |  8 ++++----
 drivers/pci/quirks.c        |  4 ++--
 include/linux/pci.h         |  7 ++++---
 10 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 8364cd6fa47d..6bf4944834b2 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2776,8 +2776,7 @@ static int amd_iommu_def_domain_type(struct device *dev)
 	if (!dev_data)
 		return 0;
 
-	/* Always use DMA domain for untrusted device */
-	if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
+	if (dev_is_pci(dev) && to_pci_dev(dev)->requires_dma_protection)
 		return IOMMU_DOMAIN_DMA;
 
 	/*
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 2a9fa0c8cc00..1358f98691ab 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -598,16 +598,16 @@ static int iova_reserve_iommu_regions(struct device *dev,
 	return ret;
 }
 
-static bool dev_is_untrusted(struct device *dev)
+static bool dev_requires_dma_protection(struct device *dev)
 {
-	return dev_is_pci(dev) && to_pci_dev(dev)->untrusted;
+	return dev_is_pci(dev) && to_pci_dev(dev)->requires_dma_protection;
 }
 
 static bool dev_use_swiotlb(struct device *dev, size_t size,
 			    enum dma_data_direction dir)
 {
 	return IS_ENABLED(CONFIG_SWIOTLB) &&
-		(dev_is_untrusted(dev) ||
+		(dev_requires_dma_protection(dev) ||
 		 dma_kmalloc_needs_bounce(dev, size, dir));
 }
 
@@ -620,7 +620,7 @@ static bool dev_use_sg_swiotlb(struct device *dev, struct scatterlist *sg,
 	if (!IS_ENABLED(CONFIG_SWIOTLB))
 		return false;
 
-	if (dev_is_untrusted(dev))
+	if (dev_requires_dma_protection(dev))
 		return true;
 
 	/*
@@ -1192,12 +1192,12 @@ dma_addr_t iommu_dma_map_page(struct device *dev, struct page *page,
 			return DMA_MAPPING_ERROR;
 
 		/*
-		 * Untrusted devices should not see padding areas with random
-		 * leftover kernel data, so zero the pre- and post-padding.
+		 * Zero the pre- and post-padding to prevent exposing kernel data to devices
+		 * requiring DMA protection.
 		 * swiotlb_tbl_map_single() has initialized the bounce buffer
 		 * proper to the contents of the original memory buffer.
 		 */
-		if (dev_is_untrusted(dev)) {
+		if (dev_requires_dma_protection(dev)) {
 			size_t start, virt = (size_t)phys_to_virt(phys);
 
 			/* Pre-padding */
@@ -1738,7 +1738,7 @@ size_t iommu_dma_opt_mapping_size(void)
 
 size_t iommu_dma_max_mapping_size(struct device *dev)
 {
-	if (dev_is_untrusted(dev))
+	if (dev_requires_dma_protection(dev))
 		return swiotlb_max_mapping_size(dev);
 
 	return SIZE_MAX;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e860bc9439a2..bcf907604c28 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3134,7 +3134,7 @@ static int __init platform_optin_force_iommu(void)
 
 	/*
 	 * If Intel-IOMMU is disabled by default, we will apply identity
-	 * map for all devices except those marked as being untrusted.
+	 * map for all devices except those marked as requiring DMA protection.
 	 */
 	if (dmar_disabled)
 		iommu_set_default_passthrough(false);
@@ -4228,13 +4228,13 @@ static bool intel_iommu_is_attach_deferred(struct device *dev)
 }
 
 /*
- * Check that the device does not live on an external facing PCI port that is
- * marked as untrusted. Such devices should not be able to apply quirks and
- * thus not be able to bypass the IOMMU restrictions.
+ * Check that the device does not require DMA protection. Such devices should
+ * not be able to apply quirks and thus not be able to bypass the IOMMU
+ * restrictions.
  */
 static bool risky_device(struct pci_dev *pdev)
 {
-	if (pdev->untrusted) {
+	if (pdev->requires_dma_protection) {
 		pci_info(pdev,
 			 "Skipping IOMMU quirk for dev [%04X:%04X] on untrusted PCI link\n",
 			 pdev->vendor, pdev->device);
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 83c8e617a2c5..0165f1d232b9 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1745,10 +1745,9 @@ static int iommu_get_default_domain_type(struct iommu_group *group,
 		driver_type = iommu_get_def_domain_type(group, gdev->dev,
 							driver_type);
 
-		if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->untrusted) {
+		if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->requires_dma_protection) {
 			/*
-			 * No ARM32 using systems will set untrusted, it cannot
-			 * work.
+			 * ARM32 systems don't support DMA protection.
 			 */
 			if (WARN_ON(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)))
 				return -1;
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 6afff1f1b143..05aab7c74491 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -43,7 +43,7 @@ bool pci_ats_supported(struct pci_dev *dev)
 	if (!dev->ats_cap)
 		return false;
 
-	return (dev->untrusted == 0);
+	return (dev->requires_dma_protection == 0);
 }
 EXPORT_SYMBOL_GPL(pci_ats_supported);
 
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index a457ae3e811a..1713e2856a88 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1465,7 +1465,7 @@ void pci_acpi_setup(struct device *dev, struct acpi_device *adev)
 
 	pci_acpi_optimize_delay(pci_dev, adev->handle);
 	pci_acpi_set_external_facing(pci_dev);
-	pci_dev->untrusted |= pci_dev_has_dma_property(pci_dev);
+	pci_dev->requires_dma_protection |= pci_dev_has_dma_property(pci_dev);
 	pci_acpi_add_edr_notifier(pci_dev);
 
 	pci_acpi_add_pm_notifier(adev, pci_dev);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 225a6cd2e9ca..761bc845a7f2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1056,7 +1056,7 @@ static void pci_std_enable_acs(struct pci_dev *dev, struct pci_acs *caps)
 	caps->ctrl |= (caps->cap & PCI_ACS_UF);
 
 	/* Enable Translation Blocking for external devices and noats */
-	if (pci_ats_disabled() || dev->external_facing || dev->untrusted)
+	if (pci_ats_disabled() || dev->external_facing || dev->requires_dma_protection)
 		caps->ctrl |= (caps->cap & PCI_ACS_TB);
 }
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f1615805f5b0..7589a524c9b8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1631,7 +1631,7 @@ static void set_pcie_thunderbolt(struct pci_dev *dev)
 		dev->is_thunderbolt = 1;
 }
 
-static void set_pcie_untrusted(struct pci_dev *dev)
+static void pci_set_requires_dma_protection(struct pci_dev *dev)
 {
 	struct pci_dev *parent;
 
@@ -1640,8 +1640,8 @@ static void set_pcie_untrusted(struct pci_dev *dev)
 	 * untrusted as well.
 	 */
 	parent = pci_upstream_bridge(dev);
-	if (parent && (parent->untrusted || parent->external_facing))
-		dev->untrusted = true;
+	if (parent && (parent->requires_dma_protection || parent->external_facing))
+		dev->requires_dma_protection = true;
 }
 
 static void pci_set_removable(struct pci_dev *dev)
@@ -1945,7 +1945,7 @@ int pci_setup_device(struct pci_dev *dev)
 	/* Need to have dev->cfg_size ready */
 	set_pcie_thunderbolt(dev);
 
-	set_pcie_untrusted(dev);
+	pci_set_requires_dma_protection(dev);
 
 	/* "Unknown power state" */
 	dev->current_state = PCI_UNKNOWN;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index dccb60c1d9cc..cc2f4792e03b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5296,7 +5296,7 @@ static void pci_quirk_enable_intel_rp_mpc_acs(struct pci_dev *dev)
  * PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF
  *
  * TODO: This quirk also needs to do equivalent of PCI_ACS_TB,
- * if dev->external_facing || dev->untrusted
+ * if dev->external_facing || dev->requires_dma_protection
  */
 static int pci_quirk_enable_intel_pch_acs(struct pci_dev *dev)
 {
@@ -5337,7 +5337,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 (pci_ats_disabled() || dev->external_facing || dev->untrusted)
+	if (pci_ats_disabled() || dev->external_facing || dev->requires_dma_protection)
 		ctrl |= (cap & PCI_ACS_TB);
 
 	pci_write_config_dword(dev, pos + INTEL_SPT_ACS_CTRL, ctrl);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 573b4c4c2be6..b883539cfd88 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -444,13 +444,14 @@ struct pci_dev {
 	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
+	 * Devices marked with requires_dma_protection 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;
+	unsigned int	requires_dma_protection:1;
+
 	/*
 	 * Info from the platform, e.g., ACPI or device tree, may mark a
 	 * device as "external-facing".  An external-facing device is
-- 
2.47.0.338.g60cca15819-goog

Re: [v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[Greg KH]

On Mon, Nov 18, 2024 at 07:30:22PM +0000, Joshua Peraza wrote:
> This patchset rebases two previously posted patches supporting
> recognition of Microsoft's DmaProperty.
> 
> v8: Joshua renames untrusted_dma to requires_dma_protection and updates
> some comments, reducing use of the word "trust" to refer to PCI devices
> and matching the word choice in Microsoft's documentation.

So this is the "clarity"?  I'm not sold, sorry.  Again, did you look at
the previous discussions we had about this name?  We don't have to use
Microsoft's term here as it is used differently by Linux today, right?
If you really want to support the DmaProperty, why not just support that
with a new bit as that's something different here, right?

Again, look at what this is supposed to be conveying.  They ability to
DMA to anywhere isn't really the root issue here, or is it?  What is the
threat model you are trying to mitigate?

> v7: Rajat updates a comment with Robin's suggestion. Joshua re-sends and
> Greg requests clarity and documentation on why untrusted_dma is the
> right name.
> 
> v6: Rajat renames pci_dev_has_dma_property and links to Microsoft's
> documentation in the commit message. Robin suggests clarifying a
> comment.
> 
> v5: Rajat changes the name to untrusted_dma. Bjorn suggesting changing
> another function's name pci_acpi_check_for_dma_protection to
> pci_dev_has_dma_property and seeks clarified documentation.
> 
> v4: Rajat changes the name to poses_dma_risk. Christoph suggests this
> name doesn't capture the intent as well as untrusted_dma and Rafael
> agrees.
> 
> v1,v2,v3: Greg suggests that (un)trusted is the wrong word for referring
> to PCI devices, recommending a name something like "platform wants to
> protect dma access for this device."

Or is it?  I said this when?  Just how old is this patch series?

confused,

greg k-h

Re: [v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[Joshua Peraza]

On Mon, Nov 18, 2024 at 11:43 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Nov 18, 2024 at 07:30:22PM +0000, Joshua Peraza wrote:
> > This patchset rebases two previously posted patches supporting
> > recognition of Microsoft's DmaProperty.
> >
> > v8: Joshua renames untrusted_dma to requires_dma_protection and updates
> > some comments, reducing use of the word "trust" to refer to PCI devices
> > and matching the word choice in Microsoft's documentation.
>
> So this is the "clarity"?  I'm not sold, sorry.  Again, did you look at
> the previous discussions we had about this name?  We don't have to use
> Microsoft's term here as it is used differently by Linux today, right?
> If you really want to support the DmaProperty, why not just support that
> with a new bit as that's something different here, right?
>
> Again, look at what this is supposed to be conveying.  They ability to
> DMA to anywhere isn't really the root issue here, or is it?  What is the
> threat model you are trying to mitigate?
>
> > v7: Rajat updates a comment with Robin's suggestion. Joshua re-sends and
> > Greg requests clarity and documentation on why untrusted_dma is the
> > right name.
> >
> > v6: Rajat renames pci_dev_has_dma_property and links to Microsoft's
> > documentation in the commit message. Robin suggests clarifying a
> > comment.
> >
> > v5: Rajat changes the name to untrusted_dma. Bjorn suggesting changing
> > another function's name pci_acpi_check_for_dma_protection to
> > pci_dev_has_dma_property and seeks clarified documentation.
> >
> > v4: Rajat changes the name to poses_dma_risk. Christoph suggests this
> > name doesn't capture the intent as well as untrusted_dma and Rafael
> > agrees.
> >
> > v1,v2,v3: Greg suggests that (un)trusted is the wrong word for referring
> > to PCI devices, recommending a name something like "platform wants to
> > protect dma access for this device."
>
> Or is it?  I said this when?  Just how old is this patch series?
>
> confused,
>
> greg k-h

(sorry if you're getting this again; re-sending as plain text)

Sorry for the confusion! What do you think about the following for a
new cover letter?

Threat model: An overview of the security implications of strict vs
non-strict IOMMU is presented at [1]. This change is motivated by
“Case 1” where a DMA-capable device is processing untrusted inputs,
e.g. network devices.

This patchset proposes using “DMA protection” to mitigate this risk.
This has the following effects, currently controlled by the
“pci_dev::untrusted” flag.

- Separate IOMMU DMA domains
- Use of SWIOTLB if CONFIG_SWIOTLB
- Disables “quirks” in intel IOMMU
- Disables Address Translation Services

The “untrusted” flag was introduced in 2018 in [2]. The motivation for
that change was to enable using IOMMU to protect against DMA attacks
from externally facing devices such as thunderbolt ports. The patchset
introduces the “untrusted” flag which “is supposed to cover various
PCIe devices that may be used to conduct DMA attacks.” The patchset
originally proposes naming the flag “is_external” but is renamed to
“is_untrusted” and then “untrusted” supposing that it could apply to
more than just externally facing thunderbolt devices. The fact that
“ExternalFacingPort” is not part of any standard is called out during
review but also that Windows expecting firmware to identify external
facing ports makes it “as good as a formal standard in the Windows
world.”

This current patch series was first proposed in January 2022 [3]. It
originally proposed a new property “UntrustedDevice” which would cause
the untrusted flag to be set. In V1 Greg questions whether the new
property is part of the ACPI standard, asks who is making this policy
decision, and states “This notion of ‘trust’ for PCI devices is
crazy….” Mika links to Microsoft's documentation of “DmaProperty” and
suggests that property is adopted instead. Greg objects that Linux
does not have “dma protection” but Mika says that this is the IOMMU.
The term “DMA protection” is also used in thunderbolt driver code for
the same purpose and in an Intel white paper [4] describing the
technique. Mika also observes that Linux has recognized several
properties documented by Microsoft but not part of the ACPI standard.
There is discussion between Mika, Rafael, and Rajat about seeking to
align with Microsoft on the semantics of “DmaProperty” for
compatibility with firmware produced for Windows.

V2 of this patch series [5] again proposed an “UntrustedDevice”
property which Greg objects to because it is not sufficiently
descriptive, not sufficiently documented, and policies about trust
don’t belong in the kernel. Rajat describes the “untrusted” flag’s
current use, controlling IOMMU and Greg suggests naming the flag
“use_iommu” or “able to do DMA”

V3 of this patch series [6] proposes recognizing “DmaProperty” with
slightly altered semantics from Microsoft’s documentation. Greg
suggests adhering to Microsoft’s semantics for “DmaProperty” and to
introduce a new property with new semantics instead. Greg again states
that the flag being named “untrusted” (not changed in this patch) is
confusing.

V4 renames “untrusted” to “poses_dma_risk”. Christoph suggests
“untrusted_dma” and Rafael agrees.

V5 renames the flag to “untrusted_dma”. Bjorn asks for clarification
about whether the semantics of this flag will match Microsoft’s
documentation. Rajat responds that Microsoft has agreed to update
their documentation to have aligned semantics, in particular “the
property is not restricted to identify ‘internal PCIe hierarchies’
(starting at root port), but to "any PCI device". As of today,
Microsoft’s documentation does not appear to have been updated.

In V6 Rajat updates a link to Microsoft’s documentation, renames a
function to pci_dev_has_dma_property() and uses
acpi_dev_get_property() to read “DmaProperty”.

In V7-V8 Joshua re-sends and Greg requests a summary of the history of
debate about the name for the “untrusted” flag as well as what is the
threat model, what does this property convey, and why we should use
Microsoft’s DmaProperty and its semantics instead of inventing
something new.

Links:
[1] https://lore.kernel.org/linux-arm-msm/20210624101557.v2.3.Icde6be7601a5939960caf802056c88cd5132eb4e@changeid/
[2] https://lore.kernel.org/lkml/20181129155153.35840-1-mika.westerberg@linux.intel.com/
[3] https://lore.kernel.org/all/20220120000409.2706549-1-rajatja@google.com/
[4] https://www.intel.com/content/dam/develop/external/us/en/documents/intel-whitepaper-using-iommu-for-dma-protection-in-uefi-820238.pdf
[5] https://lore.kernel.org/all/20220202020103.2149130-1-rajatja@google.com/
[6] https://lore.kernel.org/all/20220216220541.1635665-1-rajatja@google.com/

Re: [v8 PATCH 0/2] PCI/ACPI: Support Microsoft's "DmaProperty"

[Greg KH]

On Wed, Feb 19, 2025 at 01:53:48PM -0800, Joshua Peraza wrote:
> On Mon, Nov 18, 2024 at 11:43 AM Greg KH <gregkh@linuxfoundation.org> wrote:

Wow this is a slow discussion :)

> > On Mon, Nov 18, 2024 at 07:30:22PM +0000, Joshua Peraza wrote:
> > > This patchset rebases two previously posted patches supporting
> > > recognition of Microsoft's DmaProperty.
> > >
> > > v8: Joshua renames untrusted_dma to requires_dma_protection and updates
> > > some comments, reducing use of the word "trust" to refer to PCI devices
> > > and matching the word choice in Microsoft's documentation.
> >
> > So this is the "clarity"?  I'm not sold, sorry.  Again, did you look at
> > the previous discussions we had about this name?  We don't have to use
> > Microsoft's term here as it is used differently by Linux today, right?
> > If you really want to support the DmaProperty, why not just support that
> > with a new bit as that's something different here, right?
> >
> > Again, look at what this is supposed to be conveying.  They ability to
> > DMA to anywhere isn't really the root issue here, or is it?  What is the
> > threat model you are trying to mitigate?
> >
> > > v7: Rajat updates a comment with Robin's suggestion. Joshua re-sends and
> > > Greg requests clarity and documentation on why untrusted_dma is the
> > > right name.
> > >
> > > v6: Rajat renames pci_dev_has_dma_property and links to Microsoft's
> > > documentation in the commit message. Robin suggests clarifying a
> > > comment.
> > >
> > > v5: Rajat changes the name to untrusted_dma. Bjorn suggesting changing
> > > another function's name pci_acpi_check_for_dma_protection to
> > > pci_dev_has_dma_property and seeks clarified documentation.
> > >
> > > v4: Rajat changes the name to poses_dma_risk. Christoph suggests this
> > > name doesn't capture the intent as well as untrusted_dma and Rafael
> > > agrees.
> > >
> > > v1,v2,v3: Greg suggests that (un)trusted is the wrong word for referring
> > > to PCI devices, recommending a name something like "platform wants to
> > > protect dma access for this device."
> >
> > Or is it?  I said this when?  Just how old is this patch series?
> >
> > confused,
> >
> > greg k-h
> 
> (sorry if you're getting this again; re-sending as plain text)
> 
> Sorry for the confusion! What do you think about the following for a
> new cover letter?

I really don't remember anymore, sorry.  Try submitting the whole series
again as I don't know what you wrote the first time here.

thanks,

greg "I get 1000 emails a day" k-h