Since SEV guest depends on the unencrypted swiotlb bounce buffer
to support DMA, the guest AMD IOMMU driver must be force to setup to
pass-through mode.

Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 drivers/iommu/amd/init.c  | 15 +++++++++++++++
 drivers/iommu/amd/iommu.c |  6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index b3ff89952c7f..1dccf030f674 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3179,6 +3179,20 @@ static bool __init detect_ivrs(void)
 	return true;
 }
 
+static void iommu_sev_guest_enable(void)
+{
+	/*
+	 * Force IOMMU default domain to pass-through for
+	 * SEV guest since we cannot support DMA-remapping.
+	 * Note: This check must be done after IOMMU_ENABLED state.
+	 */
+	if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+		return;
+
+	pr_info("Force pass-through for SEV guest\n");
+	iommu_set_default_passthrough(false);
+}
+
 static void iommu_snp_enable(void)
 {
 #ifdef CONFIG_KVM_AMD_SEV
@@ -3247,6 +3261,7 @@ static int __init state_next(void)
 		break;
 	case IOMMU_ENABLED:
 		register_syscore_ops(&amd_iommu_syscore_ops);
+		iommu_sev_guest_enable();
 		iommu_snp_enable();
 		ret = amd_iommu_init_pci();
 		init_state = ret ? IOMMU_INIT_ERROR : IOMMU_PCI_INIT;
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index f98a10b7925b..c985d23c8528 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2876,6 +2876,12 @@ static int amd_iommu_def_domain_type(struct device *dev)
 		return IOMMU_DOMAIN_IDENTITY;
 	}
 
+	/*
+	 * Force identity map for SEV guest.
+	 */
+	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
+		return IOMMU_DOMAIN_IDENTITY;
+
 	return 0;
 }
 
-- 
2.34.1

On Tue, Apr 30, 2024 at 03:24:30PM +0000, Suravee Suthikulpanit wrote:
> Since SEV guest depends on the unencrypted swiotlb bounce buffer
> to support DMA, the guest AMD IOMMU driver must be force to setup to
> pass-through mode.

You should block the creation of paging domains as well if the HW
can't support them.

But, is there actually a functional problem here? Doesn't swiotlb work
OK with iommu even with the encrypted memory cases? What is missing if
not?

Jason

Jason,

On 5/1/2024 9:17 PM, Jason Gunthorpe wrote:
> On Tue, Apr 30, 2024 at 03:24:30PM +0000, Suravee Suthikulpanit wrote:
>> Since SEV guest depends on the unencrypted swiotlb bounce buffer
>> to support DMA, the guest AMD IOMMU driver must be force to setup to
>> pass-through mode.
> 
> You should block the creation of paging domains as well if the HW
> can't support them.

Sure, I'll add a logic to check and block domain creation.

> But, is there actually a functional problem here? Doesn't swiotlb work
> OK with iommu even with the encrypted memory cases? What is missing if
> not?

Currently, SEV guest is default to use SWIOTLB. This does not have any 
issues.

However, in order to support vcpus w/ x2APIC ID (> 255) in a guest, it 
requires guest interrupt remapping support. This is achieved by adding 
QEMU-emulated AMD or Intel vIOMMU models.

In case of AMD IOMMU, depending on the CONFIG_IOMMU_DEFAULT_PASSTHROUGH 
kernel config, it would default to setup the v1 table for DMA remapping, 
which is not supported in the SEV guest (since it requires to use SWIOTLB).

This patch is needed to avoid the need to have 
CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y, or specifying kernel command-line 
option iommu=pt in the guest.

Thanks,
Suravee

> Jason

On Mon, May 13, 2024 at 07:17:49PM +0700, Suthikulpanit, Suravee wrote:
> Jason,
> 
> On 5/1/2024 9:17 PM, Jason Gunthorpe wrote:
> > On Tue, Apr 30, 2024 at 03:24:30PM +0000, Suravee Suthikulpanit wrote:
> > > Since SEV guest depends on the unencrypted swiotlb bounce buffer
> > > to support DMA, the guest AMD IOMMU driver must be force to setup to
> > > pass-through mode.
> > 
> > You should block the creation of paging domains as well if the HW
> > can't support them.
> 
> Sure, I'll add a logic to check and block domain creation.
> 
> > But, is there actually a functional problem here? Doesn't swiotlb work
> > OK with iommu even with the encrypted memory cases? What is missing if
> > not?
> 
> Currently, SEV guest is default to use SWIOTLB. This does not have any
> issues.
> 
> However, in order to support vcpus w/ x2APIC ID (> 255) in a guest, it
> requires guest interrupt remapping support. This is achieved by adding
> QEMU-emulated AMD or Intel vIOMMU models.
> 
> In case of AMD IOMMU, depending on the CONFIG_IOMMU_DEFAULT_PASSTHROUGH
> kernel config, it would default to setup the v1 table for DMA remapping,
> which is not supported in the SEV guest (since it requires to use SWIOTLB).

But this just means you are inserting an iommu hw that is totally
non-working. I'd expect that the iommu continues to work correctly but
cannot access any encrypted pages..

If it is unusable do you even need to allow it to probe to any
drivers? Nothing works so there isn't much point to binding devices to
the iommu..?

Jason