Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-18 Thread Baolu Lu

On 2022/5/17 19:13, Jason Gunthorpe wrote:

On Tue, May 17, 2022 at 10:05:43AM +0800, Baolu Lu wrote:

Hi Jason,

On 2022/5/17 02:06, Jason Gunthorpe wrote:

+static __init int tboot_force_iommu(void)
+{
+   if (!tboot_enabled())
+   return 0;
+
+   if (no_iommu || dmar_disabled)
+   pr_warn("Forcing Intel-IOMMU to enabled\n");

Unrelated, but when we are in the special secure IOMMU modes, do we
force ATS off? Specifically does the IOMMU reject TLPs that are marked
as translated?


Good question. From IOMMU point of view, I don't see a point to force
ATS off, but trust boot involves lots of other things that I am not
familiar with. Anybody else could help to answer?


ATS is inherently not secure, if a rouge device can issue a TLP with
the translated bit set then it has unlimited access to host memory.


Agreed. The current logic is that the platform lets the OS know such
devices through firmware (ACPI/DT) and OS sets the untrusted flag in
their device structures. The IOMMU subsystem will disable ATS on devices
with the untrusted flag set.

There is some discussion about allowing the supervisor users to set the
trust policy through the sysfs ABI, but I don't think this has happened
in upstream kernel.


Many of these trusted iommu scenarios rely on the idea that a rouge
device cannot DMA to arbitary system memory.


I am not sure whether tboot has the same requirement.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-17 Thread Jason Gunthorpe via iommu
On Tue, May 17, 2022 at 10:05:43AM +0800, Baolu Lu wrote:
> Hi Jason,
> 
> On 2022/5/17 02:06, Jason Gunthorpe wrote:
> > > +static __init int tboot_force_iommu(void)
> > > +{
> > > + if (!tboot_enabled())
> > > + return 0;
> > > +
> > > + if (no_iommu || dmar_disabled)
> > > + pr_warn("Forcing Intel-IOMMU to enabled\n");
> > Unrelated, but when we are in the special secure IOMMU modes, do we
> > force ATS off? Specifically does the IOMMU reject TLPs that are marked
> > as translated?
> 
> Good question. From IOMMU point of view, I don't see a point to force
> ATS off, but trust boot involves lots of other things that I am not
> familiar with. Anybody else could help to answer?

ATS is inherently not secure, if a rouge device can issue a TLP with
the translated bit set then it has unlimited access to host memory.

Many of these trusted iommu scenarios rely on the idea that a rouge
device cannot DMA to arbitary system memory.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-16 Thread Baolu Lu

Hi Jason,

On 2022/5/17 02:06, Jason Gunthorpe wrote:

+static __init int tboot_force_iommu(void)
+{
+   if (!tboot_enabled())
+   return 0;
+
+   if (no_iommu || dmar_disabled)
+   pr_warn("Forcing Intel-IOMMU to enabled\n");

Unrelated, but when we are in the special secure IOMMU modes, do we
force ATS off? Specifically does the IOMMU reject TLPs that are marked
as translated?


Good question. From IOMMU point of view, I don't see a point to force
ATS off, but trust boot involves lots of other things that I am not
familiar with. Anybody else could help to answer?

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-16 Thread Jacob Pan
Hi Jason,

On Mon, 16 May 2022 15:06:28 -0300, Jason Gunthorpe  wrote:

> Unrelated, but when we are in the special secure IOMMU modes, do we
> force ATS off? Specifically does the IOMMU reject TLPs that are marked
> as translated?
Yes, VT-d context entry has a Device TLB Enable bit, if 0, it means
"Translation Requests (with or without PASID) and Translated Requests
received and processed through this scalable-mode context-entry are
blocked."

Thanks,

Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-16 Thread Jason Gunthorpe via iommu
On Sat, May 14, 2022 at 09:43:21AM +0800, Lu Baolu wrote:
> tboot_force_iommu() is only called by the Intel IOMMU driver. Move the
> helper into that driver. No functional change intended.
> 
> Signed-off-by: Lu Baolu 
> ---
>  include/linux/tboot.h   |  2 --
>  arch/x86/kernel/tboot.c | 15 ---
>  drivers/iommu/intel/iommu.c | 14 ++
>  3 files changed, 14 insertions(+), 17 deletions(-)

Reviewed-by: Jason Gunthorpe 

> +static __init int tboot_force_iommu(void)
> +{
> + if (!tboot_enabled())
> + return 0;
> +
> + if (no_iommu || dmar_disabled)
> + pr_warn("Forcing Intel-IOMMU to enabled\n");

Unrelated, but when we are in the special secure IOMMU modes, do we
force ATS off? Specifically does the IOMMU reject TLPs that are marked
as translated?

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

2022-05-13 Thread Lu Baolu
tboot_force_iommu() is only called by the Intel IOMMU driver. Move the
helper into that driver. No functional change intended.

Signed-off-by: Lu Baolu 
---
 include/linux/tboot.h   |  2 --
 arch/x86/kernel/tboot.c | 15 ---
 drivers/iommu/intel/iommu.c | 14 ++
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/include/linux/tboot.h b/include/linux/tboot.h
index 5146d2574e85..d2279160ef39 100644
--- a/include/linux/tboot.h
+++ b/include/linux/tboot.h
@@ -126,7 +126,6 @@ extern void tboot_probe(void);
 extern void tboot_shutdown(u32 shutdown_type);
 extern struct acpi_table_header *tboot_get_dmar_table(
  struct acpi_table_header *dmar_tbl);
-extern int tboot_force_iommu(void);
 
 #else
 
@@ -136,7 +135,6 @@ extern int tboot_force_iommu(void);
 #define tboot_sleep(sleep_state, pm1a_control, pm1b_control)   \
do { } while (0)
 #define tboot_get_dmar_table(dmar_tbl) (dmar_tbl)
-#define tboot_force_iommu()0
 
 #endif /* !CONFIG_INTEL_TXT */
 
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index f9af561c3cd4..6eb9c4146f17 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -6,7 +6,6 @@
  * Copyright (c) 2006-2009, Intel Corporation
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -517,17 +516,3 @@ struct acpi_table_header *tboot_get_dmar_table(struct 
acpi_table_header *dmar_tb
 
return dmar_tbl;
 }
-
-int tboot_force_iommu(void)
-{
-   if (!tboot_enabled())
-   return 0;
-
-   if (no_iommu || dmar_disabled)
-   pr_warn("Forcing Intel-IOMMU to enabled\n");
-
-   dmar_disabled = 0;
-   no_iommu = 0;
-
-   return 1;
-}
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 744af407d0da..ea1c3bcd38d5 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4043,6 +4043,20 @@ static int __init probe_acpi_namespace_devices(void)
return 0;
 }
 
+static __init int tboot_force_iommu(void)
+{
+   if (!tboot_enabled())
+   return 0;
+
+   if (no_iommu || dmar_disabled)
+   pr_warn("Forcing Intel-IOMMU to enabled\n");
+
+   dmar_disabled = 0;
+   no_iommu = 0;
+
+   return 1;
+}
+
 int __init intel_iommu_init(void)
 {
int ret = -ENODEV;
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu