linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled
@ 2022-03-15 16:24 Mario Limonciello
  2022-03-15 16:24 ` [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems Mario Limonciello
  2022-03-16  0:51 ` [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled kernel test robot
  0 siblings, 2 replies; 11+ messages in thread
From: Mario Limonciello @ 2022-03-15 16:24 UTC (permalink / raw)
  To: Joerg Roedel, Suravee Suthikulpanit, Mika Westerberg
  Cc: Will Deacon, Andreas Noever, Michael Jamet, Yehezkel Bernat,
	open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER, Mario Limonciello

Bit 1 of the IVFS IVInfo field indicates that IOMMU has been used for
pre-boot DMA protection.  Export this information to the kernel to
allow other drivers to use it.

Link: https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/iommu/amd/amd_iommu_types.h |  1 +
 drivers/iommu/amd/init.c            | 18 ++++++++++++++++++
 include/linux/amd-iommu.h           |  5 +++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 47108ed44fbb..68e51213f119 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -407,6 +407,7 @@
 /* IOMMU IVINFO */
 #define IOMMU_IVINFO_OFFSET     36
 #define IOMMU_IVINFO_EFRSUP     BIT(0)
+#define IOMMU_IVINFO_DMA_REMAP  BIT(1)
 
 /* IOMMU Feature Reporting Field (for IVHD type 10h */
 #define IOMMU_FEAT_GASUP_SHIFT	6
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 7bfe37e52e21..e9b669592dfc 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -182,6 +182,7 @@ u32 amd_iommu_max_pasid __read_mostly = ~0;
 
 bool amd_iommu_v2_present __read_mostly;
 static bool amd_iommu_pc_present __read_mostly;
+static bool amdr_ivrs_remap_support __read_mostly;
 
 bool amd_iommu_force_isolation __read_mostly;
 
@@ -326,6 +327,8 @@ static void __init early_iommu_features_init(struct amd_iommu *iommu,
 {
 	if (amd_iommu_ivinfo & IOMMU_IVINFO_EFRSUP)
 		iommu->features = h->efr_reg;
+	if (amd_iommu_ivinfo & IOMMU_IVINFO_DMA_REMAP)
+		amdr_ivrs_remap_support = true;
 }
 
 /* Access to l1 and l2 indexed register spaces */
@@ -3269,6 +3272,21 @@ struct amd_iommu *get_amd_iommu(unsigned int idx)
 	return NULL;
 }
 
+/*
+ * ivrs_remap_support - Is %IOMMU_IVINFO_DMA_REMAP set in IVRS table
+ *
+ * Returns true if the platform has %IOMMU_IVINFO_DMA_REMAP% set in the IOMMU
+ * IVRS IVInfo field.
+ * Presence of this flag indicates to the OS/HV that the IOMMU is used for
+ * Preboot DMA protection and device accessed memory should be remapped after
+ * the OS has loaded.
+ */
+bool amd_ivrs_remap_support(void)
+{
+	return amdr_ivrs_remap_support;
+}
+EXPORT_SYMBOL_GPL(amd_ivrs_remap_support);
+
 /****************************************************************************
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 58e6c3806c09..d07b9fed6474 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -170,6 +170,7 @@ amd_iommu_update_ga(int cpu, bool is_run, void *data);
 
 extern int amd_iommu_activate_guest_mode(void *data);
 extern int amd_iommu_deactivate_guest_mode(void *data);
+extern bool amd_ivrs_remap_support(void);
 
 #else /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
 
@@ -194,6 +195,10 @@ static inline int amd_iommu_deactivate_guest_mode(void *data)
 {
 	return 0;
 }
+static inline bool amd_ivrs_remap_support(void)
+{
+	return false;
+}
 #endif /* defined(CONFIG_AMD_IOMMU) && defined(CONFIG_IRQ_REMAP) */
 
 int amd_iommu_get_num_iommus(void);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 16:24 [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled Mario Limonciello
@ 2022-03-15 16:24 ` Mario Limonciello
  2022-03-15 16:48   ` Christoph Hellwig
  2022-03-16  1:32   ` kernel test robot
  2022-03-16  0:51 ` [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled kernel test robot
  1 sibling, 2 replies; 11+ messages in thread
From: Mario Limonciello @ 2022-03-15 16:24 UTC (permalink / raw)
  To: Joerg Roedel, Suravee Suthikulpanit, Mika Westerberg
  Cc: Will Deacon, Andreas Noever, Michael Jamet, Yehezkel Bernat,
	open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER, Mario Limonciello

The information is exported from the IOMMU driver whether or not
pre-boot DMA protection has been enabled on AMD systems.  Use this
information to properly set iomma_dma_protection.

Link: https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-kernel-dma-protection
Link: https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/thunderbolt/domain.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 7018d959f775..e03790735c12 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -6,6 +6,7 @@
  * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
  */
 
+#include <linux/amd-iommu.h>
 #include <linux/device.h>
 #include <linux/dmar.h>
 #include <linux/idr.h>
@@ -259,11 +260,15 @@ static ssize_t iommu_dma_protection_show(struct device *dev,
 {
 	/*
 	 * Kernel DMA protection is a feature where Thunderbolt security is
-	 * handled natively using IOMMU. It is enabled when IOMMU is
-	 * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
+	 * handled natively using IOMMU. It is enabled when the IOMMU is
+	 * enabled and either:
+	 * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
+	 * or
+	 * ACPI IVRS table has DMA_REMAP bitset
 	 */
 	return sprintf(buf, "%d\n",
-		       iommu_present(&pci_bus_type) && dmar_platform_optin());
+		       iommu_present(&pci_bus_type) &&
+		       (dmar_platform_optin() || amd_ivrs_remap_support()));
 }
 static DEVICE_ATTR_RO(iommu_dma_protection);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 16:24 ` [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems Mario Limonciello
@ 2022-03-15 16:48   ` Christoph Hellwig
  2022-03-15 16:54     ` Limonciello, Mario
  2022-03-16  1:32   ` kernel test robot
  1 sibling, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2022-03-15 16:48 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Joerg Roedel, Suravee Suthikulpanit, Mika Westerberg,
	Will Deacon, Andreas Noever, Michael Jamet, Yehezkel Bernat,
	open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER

On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
> -	 * handled natively using IOMMU. It is enabled when IOMMU is
> -	 * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
> +	 * handled natively using IOMMU. It is enabled when the IOMMU is
> +	 * enabled and either:
> +	 * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
> +	 * or
> +	 * ACPI IVRS table has DMA_REMAP bitset
>  	 */
>  	return sprintf(buf, "%d\n",
> -		       iommu_present(&pci_bus_type) && dmar_platform_optin());
> +		       iommu_present(&pci_bus_type) &&
> +		       (dmar_platform_optin() || amd_ivrs_remap_support()));

Yikes.  No, the thunderbot code does not have any business poking into
either dmar_platform_optin or amd_ivrs_remap_support.  This needs
a proper abstration from the IOMMU code.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 16:48   ` Christoph Hellwig
@ 2022-03-15 16:54     ` Limonciello, Mario
  2022-03-15 18:07       ` Robin Murphy
  0 siblings, 1 reply; 11+ messages in thread
From: Limonciello, Mario @ 2022-03-15 16:54 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Joerg Roedel, Suthikulpanit, Suravee, Mika Westerberg,
	Will Deacon, Andreas Noever, Michael Jamet, Yehezkel Bernat,
	open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER

[Public]


> On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
> > -	 * handled natively using IOMMU. It is enabled when IOMMU is
> > -	 * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
> > +	 * handled natively using IOMMU. It is enabled when the IOMMU is
> > +	 * enabled and either:
> > +	 * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
> > +	 * or
> > +	 * ACPI IVRS table has DMA_REMAP bitset
> >  	 */
> >  	return sprintf(buf, "%d\n",
> > -		       iommu_present(&pci_bus_type) &&
> dmar_platform_optin());
> > +		       iommu_present(&pci_bus_type) &&
> > +		       (dmar_platform_optin() || amd_ivrs_remap_support()));
> 
> Yikes.  No, the thunderbot code does not have any business poking into
> either dmar_platform_optin or amd_ivrs_remap_support.  This needs
> a proper abstration from the IOMMU code.

To make sure I follow your ask - it's to make a new generic iommu function
That would check dmar/ivrs, and switch out thunderbolt domain.c to use the
symbol?

I'm happy to rework that if that is what you want.
Do you have a preferred proposed function name for that?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 16:54     ` Limonciello, Mario
@ 2022-03-15 18:07       ` Robin Murphy
  2022-03-15 18:36         ` Limonciello, Mario
  0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2022-03-15 18:07 UTC (permalink / raw)
  To: Limonciello, Mario, Christoph Hellwig
  Cc: Michael Jamet, Mika Westerberg, open list:THUNDERBOLT DRIVER,
	open list, Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	Andreas Noever, Will Deacon

On 2022-03-15 16:54, Limonciello, Mario via iommu wrote:
> [Public]
> 
> 
>> On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
>>> -	 * handled natively using IOMMU. It is enabled when IOMMU is
>>> -	 * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
>>> +	 * handled natively using IOMMU. It is enabled when the IOMMU is
>>> +	 * enabled and either:
>>> +	 * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
>>> +	 * or
>>> +	 * ACPI IVRS table has DMA_REMAP bitset
>>>   	 */
>>>   	return sprintf(buf, "%d\n",
>>> -		       iommu_present(&pci_bus_type) &&
>> dmar_platform_optin());
>>> +		       iommu_present(&pci_bus_type) &&
>>> +		       (dmar_platform_optin() || amd_ivrs_remap_support()));
>>
>> Yikes.  No, the thunderbot code does not have any business poking into
>> either dmar_platform_optin or amd_ivrs_remap_support.  This needs
>> a proper abstration from the IOMMU code.
> 
> To make sure I follow your ask - it's to make a new generic iommu function
> That would check dmar/ivrs, and switch out thunderbolt domain.c to use the
> symbol?
> 
> I'm happy to rework that if that is what you want.
> Do you have a preferred proposed function name for that?

But why? Either IOMMU translation is enabled or it isn't, and if it is, 
what's to gain from guessing at *why* it might have been? And even if 
the IOMMU's firmware table did tell the IOMMU driver to enable the 
IOMMU, why should that be Thunderbolt's business?

Furthermore, looking at patch #1 I can only conclude that this is 
entirely meaningless anyway. AFAICS it's literally reporting whether the 
firmware flag was set or not. Not whether it's actually been honoured 
and the IOMMU is enforcing any kind of DMA protection at all. Even on 
Intel where the flag does at least have some effect on the IOMMU driver, 
that can still be overridden.

I already have a patch refactoring this to get rid of iommu_present(), 
but at the time I wasn't looking to closely at what it's trying to *do* 
with the information. If it's supposed to accurately reflect whether the 
Thunderbolt device is subject to IOMMU translation and not bypassed, I 
can fix that too (and unexport dmar_platform_optin() in the process...)

Robin.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 18:07       ` Robin Murphy
@ 2022-03-15 18:36         ` Limonciello, Mario
  2022-03-15 21:34           ` Limonciello, Mario
                             ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Limonciello, Mario @ 2022-03-15 18:36 UTC (permalink / raw)
  To: Robin Murphy, Christoph Hellwig, christian, Mika Westerberg
  Cc: Michael Jamet, open list:THUNDERBOLT DRIVER, open list,
	Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	Andreas Noever, Will Deacon

+ Christian Kellner (Bolt userspace maintainer)

On 3/15/2022 13:07, Robin Murphy wrote:
> On 2022-03-15 16:54, Limonciello, Mario via iommu wrote:
>> [Public]
>>
>>
>>> On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
>>>> -     * handled natively using IOMMU. It is enabled when IOMMU is
>>>> -     * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
>>>> +     * handled natively using IOMMU. It is enabled when the IOMMU is
>>>> +     * enabled and either:
>>>> +     * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
>>>> +     * or
>>>> +     * ACPI IVRS table has DMA_REMAP bitset
>>>>        */
>>>>       return sprintf(buf, "%d\n",
>>>> -               iommu_present(&pci_bus_type) &&
>>> dmar_platform_optin());
>>>> +               iommu_present(&pci_bus_type) &&
>>>> +               (dmar_platform_optin() || amd_ivrs_remap_support()));
>>>
>>> Yikes.  No, the thunderbot code does not have any business poking into
>>> either dmar_platform_optin or amd_ivrs_remap_support.  This needs
>>> a proper abstration from the IOMMU code.
>>
>> To make sure I follow your ask - it's to make a new generic iommu 
>> function
>> That would check dmar/ivrs, and switch out thunderbolt domain.c to use 
>> the
>> symbol?
>>
>> I'm happy to rework that if that is what you want.
>> Do you have a preferred proposed function name for that?
> 
> But why? Either IOMMU translation is enabled or it isn't, and if it is, 
> what's to gain from guessing at *why* it might have been? And even if 
> the IOMMU's firmware table did tell the IOMMU driver to enable the 
> IOMMU, why should that be Thunderbolt's business?
A lot of this comes from baggage from early Thunderbolt 3 implementation 
on systems with ICM (Intel's FW CM). On those systems there was a 
concept of "Security Levels".  This meant that downstream PCIe devices 
were not automatically authorized when a TBT3 device was plugged in.  In 
those cases there was no guarantee that the IOMMU was in use and so the 
security was passed on to the user to make a decision.

In Linux this was accomplished using the 'authorized' attribute in 
/sys/bus/thunderbolt/devices/$NUM/authorized.  When this was set to 1 
then the TBT3 device and PCIe topology behind it would be enumerated.

Further documentation explaining how this works is available here:
https://www.kernel.org/doc/html/latest/admin-guide/thunderbolt.html#security-levels-and-how-to-use-them

(Intel based) Platforms from 2018+ w/ TBT3 started to use the IOMMU 
consistently at runtime but had this existing implementation of security 
levels to worry about.  Furthermore tunnels could be created pre-boot, 
and so the thunderbolt driver may or may not re-create them based on policy.

So a new attribute was created "iommu_dma_protection" that userspace 
could use as part of a policy decision to automatically authorize 
devices.  Exporting this attribute is very similar to what Microsoft 
does to let the user see the security of the system.

https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-kernel-dma-protection

In Linux today some userspace software "bolt" has a policy included by
default that will automatically authorize TBT3 and USB4 (w/ PCIe) 
devices when iommu_dma_protection is set to 1.

> 
> Furthermore, looking at patch #1 I can only conclude that this is 
> entirely meaningless anyway. AFAICS it's literally reporting whether the 
> firmware flag was set or not. Not whether it's actually been honoured 
> and the IOMMU is enforcing any kind of DMA protection at all. Even on 
> Intel where the flag does at least have some effect on the IOMMU driver, 
> that can still be overridden.

Take a look at the Microsoft link I shared above.  They also make policy
decisions based on the information in these tables.

> 
> I already have a patch refactoring this to get rid of iommu_present(), 
> but at the time I wasn't looking to closely at what it's trying to *do* 
> with the information. If it's supposed to accurately reflect whether the 
> Thunderbolt device is subject to IOMMU translation and not bypassed, I 
> can fix that too (and unexport dmar_platform_optin() in the process...)
> 
> Robin.

This patch series stems from that history.  To give the best experience 
to end users you want hotplugged devices to be automatically authorized 
when software says it's safe to do so.

To summarize the flow:
* User plugs in device
* USB4 CM will query supported tunnels
* USB4 CM will create devices in /sys/bus/thunderbolt/devices for new 
plugged in TBT3/USB4 device
* "authorized" attribute will default to "0" and PCIe tunnels are not 
created
* Userspace gets a uevent that the device was added
* Userspace (bolt) reacts by reading 
/sys/bus/thunderbolt/devices/domainX/iommu_dma_protection
* If that is set to "1", bolt will write "1" to "authorized"  and USB4 
CM will create PCIe tunnels
* If that is set to "0", bolt will send an event to GUI to show a popup 
asking to authorize the device
* After user acks the authorization then it will write "1" to 
"authorized" and USB4 CM will create PCIe tunnels


Mika,

I wonder if maybe what we really want is to only use that flow for the 
authorized attribute when using TBT3 + ICM (or IOMMU disabled at 
runtime).  If we're using a USB4 host, check IOMMU translation layer 
active like Robin suggested and then automatically authorize from the CM.

Thoughts?




^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 18:36         ` Limonciello, Mario
@ 2022-03-15 21:34           ` Limonciello, Mario
  2022-03-15 22:04           ` Robin Murphy
  2022-03-16  9:50           ` Mika Westerberg
  2 siblings, 0 replies; 11+ messages in thread
From: Limonciello, Mario @ 2022-03-15 21:34 UTC (permalink / raw)
  To: Robin Murphy, Christoph Hellwig, christian, Mika Westerberg
  Cc: Michael Jamet, open list:THUNDERBOLT DRIVER, open list,
	Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	Andreas Noever, Will Deacon

[Public]



> -----Original Message-----
> From: Limonciello, Mario
> Sent: Tuesday, March 15, 2022 13:36
> To: Robin Murphy <robin.murphy@arm.com>; Christoph Hellwig
> <hch@infradead.org>; christian@kellner.me; Mika Westerberg
> <mika.westerberg@linux.intel.com>
> Cc: Michael Jamet <michael.jamet@intel.com>; open list:THUNDERBOLT
> DRIVER <linux-usb@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>; Yehezkel Bernat <YehezkelShB@gmail.com>;
> open list:AMD IOMMU (AMD-VI) <iommu@lists.linux-foundation.org>;
> Andreas Noever <andreas.noever@gmail.com>; Will Deacon
> <will@kernel.org>
> Subject: Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD
> systems
> 
> + Christian Kellner (Bolt userspace maintainer)
> 
> On 3/15/2022 13:07, Robin Murphy wrote:
> > On 2022-03-15 16:54, Limonciello, Mario via iommu wrote:
> >> [Public]
> >>
> >>
> >>> On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
> >>>> -     * handled natively using IOMMU. It is enabled when IOMMU is
> >>>> -     * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN
> set.
> >>>> +     * handled natively using IOMMU. It is enabled when the IOMMU is
> >>>> +     * enabled and either:
> >>>> +     * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
> >>>> +     * or
> >>>> +     * ACPI IVRS table has DMA_REMAP bitset
> >>>>        */
> >>>>       return sprintf(buf, "%d\n",
> >>>> -               iommu_present(&pci_bus_type) &&
> >>> dmar_platform_optin());
> >>>> +               iommu_present(&pci_bus_type) &&
> >>>> +               (dmar_platform_optin() || amd_ivrs_remap_support()));
> >>>
> >>> Yikes.  No, the thunderbot code does not have any business poking into
> >>> either dmar_platform_optin or amd_ivrs_remap_support.  This needs
> >>> a proper abstration from the IOMMU code.
> >>
> >> To make sure I follow your ask - it's to make a new generic iommu
> >> function
> >> That would check dmar/ivrs, and switch out thunderbolt domain.c to use
> >> the
> >> symbol?
> >>
> >> I'm happy to rework that if that is what you want.
> >> Do you have a preferred proposed function name for that?
> >
> > But why? Either IOMMU translation is enabled or it isn't, and if it is,
> > what's to gain from guessing at *why* it might have been? And even if
> > the IOMMU's firmware table did tell the IOMMU driver to enable the
> > IOMMU, why should that be Thunderbolt's business?
> A lot of this comes from baggage from early Thunderbolt 3 implementation
> on systems with ICM (Intel's FW CM). On those systems there was a
> concept of "Security Levels".  This meant that downstream PCIe devices
> were not automatically authorized when a TBT3 device was plugged in.  In
> those cases there was no guarantee that the IOMMU was in use and so the
> security was passed on to the user to make a decision.
> 
> In Linux this was accomplished using the 'authorized' attribute in
> /sys/bus/thunderbolt/devices/$NUM/authorized.  When this was set to 1
> then the TBT3 device and PCIe topology behind it would be enumerated.
> 
> Further documentation explaining how this works is available here:
> https://www.kernel.org/doc/html/latest/admin-
> guide/thunderbolt.html#security-levels-and-how-to-use-them
> 
> (Intel based) Platforms from 2018+ w/ TBT3 started to use the IOMMU
> consistently at runtime but had this existing implementation of security
> levels to worry about.  Furthermore tunnels could be created pre-boot,
> and so the thunderbolt driver may or may not re-create them based on
> policy.
> 
> So a new attribute was created "iommu_dma_protection" that userspace
> could use as part of a policy decision to automatically authorize
> devices.  Exporting this attribute is very similar to what Microsoft
> does to let the user see the security of the system.
> 
> https://docs.microsoft.com/en-us/windows-hardware/design/device-
> experiences/oem-kernel-dma-protection
> 
> In Linux today some userspace software "bolt" has a policy included by
> default that will automatically authorize TBT3 and USB4 (w/ PCIe)
> devices when iommu_dma_protection is set to 1.
> 
> >
> > Furthermore, looking at patch #1 I can only conclude that this is
> > entirely meaningless anyway. AFAICS it's literally reporting whether the
> > firmware flag was set or not. Not whether it's actually been honoured
> > and the IOMMU is enforcing any kind of DMA protection at all. Even on
> > Intel where the flag does at least have some effect on the IOMMU driver,
> > that can still be overridden.
> 
> Take a look at the Microsoft link I shared above.  They also make policy
> decisions based on the information in these tables.
> 
> >
> > I already have a patch refactoring this to get rid of iommu_present(),
> > but at the time I wasn't looking to closely at what it's trying to *do*
> > with the information. If it's supposed to accurately reflect whether the
> > Thunderbolt device is subject to IOMMU translation and not bypassed, I
> > can fix that too (and unexport dmar_platform_optin() in the process...)
> >
> > Robin.
> 
> This patch series stems from that history.  To give the best experience
> to end users you want hotplugged devices to be automatically authorized
> when software says it's safe to do so.
> 
> To summarize the flow:
> * User plugs in device
> * USB4 CM will query supported tunnels
> * USB4 CM will create devices in /sys/bus/thunderbolt/devices for new
> plugged in TBT3/USB4 device
> * "authorized" attribute will default to "0" and PCIe tunnels are not
> created
> * Userspace gets a uevent that the device was added
> * Userspace (bolt) reacts by reading
> /sys/bus/thunderbolt/devices/domainX/iommu_dma_protection
> * If that is set to "1", bolt will write "1" to "authorized"  and USB4
> CM will create PCIe tunnels
> * If that is set to "0", bolt will send an event to GUI to show a popup
> asking to authorize the device
> * After user acks the authorization then it will write "1" to
> "authorized" and USB4 CM will create PCIe tunnels
> 
> 
> Mika,
> 
> I wonder if maybe what we really want is to only use that flow for the
> authorized attribute when using TBT3 + ICM (or IOMMU disabled at
> runtime).  If we're using a USB4 host, check IOMMU translation layer
> active like Robin suggested and then automatically authorize from the CM.
> 
> Thoughts?
> 
>
 
I put an RFC together with what this idea looks like, comments welcome.
https://lore.kernel.org/linux-usb/20220315213008.5357-1-mario.limonciello@amd.com/T/#u

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 18:36         ` Limonciello, Mario
  2022-03-15 21:34           ` Limonciello, Mario
@ 2022-03-15 22:04           ` Robin Murphy
  2022-03-16  9:50           ` Mika Westerberg
  2 siblings, 0 replies; 11+ messages in thread
From: Robin Murphy @ 2022-03-15 22:04 UTC (permalink / raw)
  To: Limonciello, Mario, Christoph Hellwig, christian, Mika Westerberg
  Cc: Michael Jamet, open list:THUNDERBOLT DRIVER, open list,
	Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	Andreas Noever, Will Deacon

On 2022-03-15 18:36, Limonciello, Mario wrote:
> + Christian Kellner (Bolt userspace maintainer)
> 
> On 3/15/2022 13:07, Robin Murphy wrote:
>> On 2022-03-15 16:54, Limonciello, Mario via iommu wrote:
>>> [Public]
>>>
>>>
>>>> On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
>>>>> -     * handled natively using IOMMU. It is enabled when IOMMU is
>>>>> -     * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
>>>>> +     * handled natively using IOMMU. It is enabled when the IOMMU is
>>>>> +     * enabled and either:
>>>>> +     * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
>>>>> +     * or
>>>>> +     * ACPI IVRS table has DMA_REMAP bitset
>>>>>        */
>>>>>       return sprintf(buf, "%d\n",
>>>>> -               iommu_present(&pci_bus_type) &&
>>>> dmar_platform_optin());
>>>>> +               iommu_present(&pci_bus_type) &&
>>>>> +               (dmar_platform_optin() || amd_ivrs_remap_support()));
>>>>
>>>> Yikes.  No, the thunderbot code does not have any business poking into
>>>> either dmar_platform_optin or amd_ivrs_remap_support.  This needs
>>>> a proper abstration from the IOMMU code.
>>>
>>> To make sure I follow your ask - it's to make a new generic iommu 
>>> function
>>> That would check dmar/ivrs, and switch out thunderbolt domain.c to 
>>> use the
>>> symbol?
>>>
>>> I'm happy to rework that if that is what you want.
>>> Do you have a preferred proposed function name for that?
>>
>> But why? Either IOMMU translation is enabled or it isn't, and if it 
>> is, what's to gain from guessing at *why* it might have been? And even 
>> if the IOMMU's firmware table did tell the IOMMU driver to enable the 
>> IOMMU, why should that be Thunderbolt's business?
> A lot of this comes from baggage from early Thunderbolt 3 implementation 
> on systems with ICM (Intel's FW CM). On those systems there was a 
> concept of "Security Levels".  This meant that downstream PCIe devices 
> were not automatically authorized when a TBT3 device was plugged in.  In 
> those cases there was no guarantee that the IOMMU was in use and so the 
> security was passed on to the user to make a decision.
> 
> In Linux this was accomplished using the 'authorized' attribute in 
> /sys/bus/thunderbolt/devices/$NUM/authorized.  When this was set to 1 
> then the TBT3 device and PCIe topology behind it would be enumerated.
> 
> Further documentation explaining how this works is available here:
> https://www.kernel.org/doc/html/latest/admin-guide/thunderbolt.html#security-levels-and-how-to-use-them 
> 
> 
> (Intel based) Platforms from 2018+ w/ TBT3 started to use the IOMMU 
> consistently at runtime but had this existing implementation of security 
> levels to worry about.  Furthermore tunnels could be created pre-boot, 
> and so the thunderbolt driver may or may not re-create them based on 
> policy.
> 
> So a new attribute was created "iommu_dma_protection" that userspace 
> could use as part of a policy decision to automatically authorize 
> devices.  Exporting this attribute is very similar to what Microsoft 
> does to let the user see the security of the system.
> 
> https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-kernel-dma-protection 
> 
> 
> In Linux today some userspace software "bolt" has a policy included by
> default that will automatically authorize TBT3 and USB4 (w/ PCIe) 
> devices when iommu_dma_protection is set to 1.
> 
>>
>> Furthermore, looking at patch #1 I can only conclude that this is 
>> entirely meaningless anyway. AFAICS it's literally reporting whether 
>> the firmware flag was set or not. Not whether it's actually been 
>> honoured and the IOMMU is enforcing any kind of DMA protection at all. 
>> Even on Intel where the flag does at least have some effect on the 
>> IOMMU driver, that can still be overridden.
> 
> Take a look at the Microsoft link I shared above.  They also make policy
> decisions based on the information in these tables.
> 
>>
>> I already have a patch refactoring this to get rid of iommu_present(), 
>> but at the time I wasn't looking to closely at what it's trying to 
>> *do* with the information. If it's supposed to accurately reflect 
>> whether the Thunderbolt device is subject to IOMMU translation and not 
>> bypassed, I can fix that too (and unexport dmar_platform_optin() in 
>> the process...)
>>
>> Robin.
> 
> This patch series stems from that history.  To give the best experience 
> to end users you want hotplugged devices to be automatically authorized 
> when software says it's safe to do so.
> 
> To summarize the flow:
> * User plugs in device
> * USB4 CM will query supported tunnels
> * USB4 CM will create devices in /sys/bus/thunderbolt/devices for new 
> plugged in TBT3/USB4 device
> * "authorized" attribute will default to "0" and PCIe tunnels are not 
> created
> * Userspace gets a uevent that the device was added
> * Userspace (bolt) reacts by reading 
> /sys/bus/thunderbolt/devices/domainX/iommu_dma_protection
> * If that is set to "1", bolt will write "1" to "authorized"  and USB4 
> CM will create PCIe tunnels
> * If that is set to "0", bolt will send an event to GUI to show a popup 
> asking to authorize the device
> * After user acks the authorization then it will write "1" to 
> "authorized" and USB4 CM will create PCIe tunnels
> 
> 
> Mika,
> 
> I wonder if maybe what we really want is to only use that flow for the 
> authorized attribute when using TBT3 + ICM (or IOMMU disabled at 
> runtime).  If we're using a USB4 host, check IOMMU translation layer 
> active like Robin suggested and then automatically authorize from the CM.

Thanks for the explanation. I don't think there's anything wrong with 
that flow per se - fundamentally, whether it's relayed through userspace 
or done automagically inside the kernel doesn't change the end result - 
but it does seem to confirm my suspicion that even now it's not actually 
working as intended and may end up letting devices be authorised in 
circumstances that they probably shouldn't be.

It's absolutely fine for Thunderbolt to care about whether a device 
currently has IOMMU translation enabled (and to expose that to userspace 
in its own way if it wants to), but that's generic IOMMU API stuff, no 
firmware-poking required :)

Tomorrow I'll rework the patch out of my iommu_present() cleanup stack 
to do the right thing, and share it.

Cheers,
Robin.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled
  2022-03-15 16:24 [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled Mario Limonciello
  2022-03-15 16:24 ` [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems Mario Limonciello
@ 2022-03-16  0:51 ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2022-03-16  0:51 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Suravee Suthikulpanit, Mika Westerberg
  Cc: kbuild-all, Will Deacon, Andreas Noever, Michael Jamet,
	Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER, Mario Limonciello

Hi Mario,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on joro-iommu/next]
[also build test ERROR on arm-perf/for-next/perf linus/master v5.17-rc8 next-20220315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mario-Limonciello/iommu-amd-Add-support-to-indicate-whether-DMA-remap-support-is-enabled/20220316-002821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20220316/202203160844.lKviWR1Q-lkp@intel.com/config)
compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/fa63035401902e438c5ef3213112901a1054c621
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mario-Limonciello/iommu-amd-Add-support-to-indicate-whether-DMA-remap-support-is-enabled/20220316-002821
        git checkout fa63035401902e438c5ef3213112901a1054c621
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iommu/amd/init.c:3294:6: error: redefinition of 'amd_ivrs_remap_support'
    3294 | bool amd_ivrs_remap_support(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/iommu/amd/init.c:20:
   include/linux/amd-iommu.h:198:20: note: previous definition of 'amd_ivrs_remap_support' was here
     198 | static inline bool amd_ivrs_remap_support(void)
         |                    ^~~~~~~~~~~~~~~~~~~~~~


vim +/amd_ivrs_remap_support +3294 drivers/iommu/amd/init.c

  3284	
  3285	/*
  3286	 * ivrs_remap_support - Is %IOMMU_IVINFO_DMA_REMAP set in IVRS table
  3287	 *
  3288	 * Returns true if the platform has %IOMMU_IVINFO_DMA_REMAP% set in the IOMMU
  3289	 * IVRS IVInfo field.
  3290	 * Presence of this flag indicates to the OS/HV that the IOMMU is used for
  3291	 * Preboot DMA protection and device accessed memory should be remapped after
  3292	 * the OS has loaded.
  3293	 */
> 3294	bool amd_ivrs_remap_support(void)
  3295	{
  3296		return amdr_ivrs_remap_support;
  3297	}
  3298	EXPORT_SYMBOL_GPL(amd_ivrs_remap_support);
  3299	

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 16:24 ` [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems Mario Limonciello
  2022-03-15 16:48   ` Christoph Hellwig
@ 2022-03-16  1:32   ` kernel test robot
  1 sibling, 0 replies; 11+ messages in thread
From: kernel test robot @ 2022-03-16  1:32 UTC (permalink / raw)
  To: Mario Limonciello, Joerg Roedel, Suravee Suthikulpanit, Mika Westerberg
  Cc: llvm, kbuild-all, Will Deacon, Andreas Noever, Michael Jamet,
	Yehezkel Bernat, open list:AMD IOMMU (AMD-VI),
	open list, open list:THUNDERBOLT DRIVER, Mario Limonciello

Hi Mario,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on joro-iommu/next]
[also build test ERROR on arm-perf/for-next/perf linus/master v5.17-rc8 next-20220315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mario-Limonciello/iommu-amd-Add-support-to-indicate-whether-DMA-remap-support-is-enabled/20220316-002821
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220316/202203160904.VB4alCdg-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a6b2f50fb47da3baeee10b1906da6e30ac5d26ec)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9b0b7079d348c607cba7af4c87eaae1a79e52d91
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mario-Limonciello/iommu-amd-Add-support-to-indicate-whether-DMA-remap-support-is-enabled/20220316-002821
        git checkout 9b0b7079d348c607cba7af4c87eaae1a79e52d91
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/thunderbolt/domain.c:9:
>> include/linux/amd-iommu.h:159:52: error: use of undeclared identifier 'ENODEV'
   static inline int amd_iommu_detect(void) { return -ENODEV; }
                                                      ^
   1 error generated.


vim +/ENODEV +159 include/linux/amd-iommu.h

6a9401a7ac13e6 arch/x86/include/asm/amd_iommu.h Joerg Roedel          2009-11-20  158  
480125ba49ba62 arch/x86/include/asm/amd_iommu.h Konrad Rzeszutek Wilk 2010-08-26 @159  static inline int amd_iommu_detect(void) { return -ENODEV; }
6a9401a7ac13e6 arch/x86/include/asm/amd_iommu.h Joerg Roedel          2009-11-20  160  

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems
  2022-03-15 18:36         ` Limonciello, Mario
  2022-03-15 21:34           ` Limonciello, Mario
  2022-03-15 22:04           ` Robin Murphy
@ 2022-03-16  9:50           ` Mika Westerberg
  2 siblings, 0 replies; 11+ messages in thread
From: Mika Westerberg @ 2022-03-16  9:50 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Robin Murphy, Christoph Hellwig, christian, Michael Jamet,
	open list:THUNDERBOLT DRIVER, open list, Yehezkel Bernat,
	open list:AMD IOMMU (AMD-VI),
	Andreas Noever, Will Deacon

Hi,

On Tue, Mar 15, 2022 at 01:36:11PM -0500, Limonciello, Mario wrote:
> + Christian Kellner (Bolt userspace maintainer)
> 
> On 3/15/2022 13:07, Robin Murphy wrote:
> > On 2022-03-15 16:54, Limonciello, Mario via iommu wrote:
> > > [Public]
> > > 
> > > 
> > > > On Tue, Mar 15, 2022 at 11:24:55AM -0500, Mario Limonciello wrote:
> > > > > -     * handled natively using IOMMU. It is enabled when IOMMU is
> > > > > -     * enabled and ACPI DMAR table has DMAR_PLATFORM_OPT_IN set.
> > > > > +     * handled natively using IOMMU. It is enabled when the IOMMU is
> > > > > +     * enabled and either:
> > > > > +     * ACPI DMAR table has DMAR_PLATFORM_OPT_IN set
> > > > > +     * or
> > > > > +     * ACPI IVRS table has DMA_REMAP bitset
> > > > >        */
> > > > >       return sprintf(buf, "%d\n",
> > > > > -               iommu_present(&pci_bus_type) &&
> > > > dmar_platform_optin());
> > > > > +               iommu_present(&pci_bus_type) &&
> > > > > +               (dmar_platform_optin() || amd_ivrs_remap_support()));
> > > > 
> > > > Yikes.  No, the thunderbot code does not have any business poking into
> > > > either dmar_platform_optin or amd_ivrs_remap_support.  This needs
> > > > a proper abstration from the IOMMU code.

I agree. When it was originally added it was only the DMAR (Intel) based
platforms that provided this hint so adding an abstraction for that did
not make much sense. Now, since we are seeing more and more USB4 host
controllers and many of them support PCIe tunneling (and IOMMU) adding
an API makes more sense.

> > > 
> > > To make sure I follow your ask - it's to make a new generic iommu
> > > function
> > > That would check dmar/ivrs, and switch out thunderbolt domain.c to
> > > use the
> > > symbol?
> > > 
> > > I'm happy to rework that if that is what you want.
> > > Do you have a preferred proposed function name for that?
> > 
> > But why? Either IOMMU translation is enabled or it isn't, and if it is,
> > what's to gain from guessing at *why* it might have been? And even if
> > the IOMMU's firmware table did tell the IOMMU driver to enable the
> > IOMMU, why should that be Thunderbolt's business?
> A lot of this comes from baggage from early Thunderbolt 3 implementation on
> systems with ICM (Intel's FW CM). On those systems there was a concept of
> "Security Levels".  This meant that downstream PCIe devices were not
> automatically authorized when a TBT3 device was plugged in.  In those cases
> there was no guarantee that the IOMMU was in use and so the security was
> passed on to the user to make a decision.
> 
> In Linux this was accomplished using the 'authorized' attribute in
> /sys/bus/thunderbolt/devices/$NUM/authorized.  When this was set to 1 then
> the TBT3 device and PCIe topology behind it would be enumerated.
> 
> Further documentation explaining how this works is available here:
> https://www.kernel.org/doc/html/latest/admin-guide/thunderbolt.html#security-levels-and-how-to-use-them
> 
> (Intel based) Platforms from 2018+ w/ TBT3 started to use the IOMMU
> consistently at runtime but had this existing implementation of security
> levels to worry about.  Furthermore tunnels could be created pre-boot, and
> so the thunderbolt driver may or may not re-create them based on policy.
> 
> So a new attribute was created "iommu_dma_protection" that userspace could
> use as part of a policy decision to automatically authorize devices.
> Exporting this attribute is very similar to what Microsoft does to let the
> user see the security of the system.
> 
> https://docs.microsoft.com/en-us/windows-hardware/design/device-experiences/oem-kernel-dma-protection
> 
> In Linux today some userspace software "bolt" has a policy included by
> default that will automatically authorize TBT3 and USB4 (w/ PCIe) devices
> when iommu_dma_protection is set to 1.
> 
> > 
> > Furthermore, looking at patch #1 I can only conclude that this is
> > entirely meaningless anyway. AFAICS it's literally reporting whether the
> > firmware flag was set or not. Not whether it's actually been honoured
> > and the IOMMU is enforcing any kind of DMA protection at all. Even on
> > Intel where the flag does at least have some effect on the IOMMU driver,
> > that can still be overridden.
> 
> Take a look at the Microsoft link I shared above.  They also make policy
> decisions based on the information in these tables.
> 
> > 
> > I already have a patch refactoring this to get rid of iommu_present(),
> > but at the time I wasn't looking to closely at what it's trying to *do*
> > with the information. If it's supposed to accurately reflect whether the
> > Thunderbolt device is subject to IOMMU translation and not bypassed, I
> > can fix that too (and unexport dmar_platform_optin() in the process...)
> > 
> > Robin.
> 
> This patch series stems from that history.  To give the best experience to
> end users you want hotplugged devices to be automatically authorized when
> software says it's safe to do so.
> 
> To summarize the flow:
> * User plugs in device
> * USB4 CM will query supported tunnels
> * USB4 CM will create devices in /sys/bus/thunderbolt/devices for new
> plugged in TBT3/USB4 device
> * "authorized" attribute will default to "0" and PCIe tunnels are not
> created
> * Userspace gets a uevent that the device was added
> * Userspace (bolt) reacts by reading
> /sys/bus/thunderbolt/devices/domainX/iommu_dma_protection
> * If that is set to "1", bolt will write "1" to "authorized"  and USB4 CM
> will create PCIe tunnels
> * If that is set to "0", bolt will send an event to GUI to show a popup
> asking to authorize the device
> * After user acks the authorization then it will write "1" to "authorized"
> and USB4 CM will create PCIe tunnels
> 
> 
> Mika,
> 
> I wonder if maybe what we really want is to only use that flow for the
> authorized attribute when using TBT3 + ICM (or IOMMU disabled at runtime).
> If we're using a USB4 host, check IOMMU translation layer active like Robin
> suggested and then automatically authorize from the CM.

I would still leave that policy to userspace to decide.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-03-16  9:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-15 16:24 [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled Mario Limonciello
2022-03-15 16:24 ` [PATCH 2/2] thunderbolt: Use pre-boot DMA protection on AMD systems Mario Limonciello
2022-03-15 16:48   ` Christoph Hellwig
2022-03-15 16:54     ` Limonciello, Mario
2022-03-15 18:07       ` Robin Murphy
2022-03-15 18:36         ` Limonciello, Mario
2022-03-15 21:34           ` Limonciello, Mario
2022-03-15 22:04           ` Robin Murphy
2022-03-16  9:50           ` Mika Westerberg
2022-03-16  1:32   ` kernel test robot
2022-03-16  0:51 ` [PATCH 1/2] iommu/amd: Add support to indicate whether DMA remap support is enabled kernel test robot

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).