[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <44aa28c7-69fe-4073-bd61-6aca57203c64@amd.com>
Date: Wed, 30 Apr 2025 17:22:14 +0530
From: Vasant Hegde <vasant.hegde@....com>
To: Joao Martins <joao.m.martins@...cle.com>, Ankit Soni
<Ankit.Soni@....com>, iommu@...ts.linux.dev,
Baolu Lu <baolu.lu@...ux.intel.com>
Cc: suravee.suthikulpanit@....com, joro@...tes.org, will@...nel.org,
robin.murphy@....com, linux-kernel@...r.kernel.org,
Alejandro Jimenez <alejandro.j.jimenez@...cle.com>,
David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 1/2] iommu/amd: Add HATDis feature support
Hi Joao,
[+Baolu to get clarification on intel driver behaviour.]
On 4/24/2025 5:49 PM, Joao Martins wrote:
> On 23/04/2025 07:50, Ankit Soni wrote:
>> Current AMD IOMMU assumes Host Address Translation (HAT) is always
>> supported, and Linux kernel enables this capability by default. However,
>> in case of emulated and virtualized IOMMU, this might not be the case.
>
> +Alejandro as he is filling that gap
>
>> For example,current QEMU-emulated AMD vIOMMU does not support host
>> translation for VFIO pass-through device, but the interrupt remapping
>> support is required for x2APIC (i.e. kvm-msi-ext-dest-id is also not
>> supported by the guest OS). This would require the guest kernel to boot
>> with guest kernel option iommu=pt to by-pass the initialization of
>> host (v1) table.
>>
>> The AMD I/O Virtualization Technology (IOMMU) Specification Rev 3.10 [1]>
> introduces a new flag 'HATDis' in the IVHD 11h IOMMU attributes to indicate
>> that HAT is not supported on a particular IOMMU instance.
>>
>> Therefore, modifies the AMD IOMMU driver to detect the new HATDis
>> attributes, and disable host translation and switch to use guest
>> translation if it is available. Otherwise, the driver will disable DMA
>> translation.
>>
>> [1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
>>
>> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>
>> Signed-off-by: Ankit Soni <Ankit.Soni@....com>
>> ---
>> drivers/iommu/amd/amd_iommu.h | 1 +
>> drivers/iommu/amd/amd_iommu_types.h | 6 +++++-
>> drivers/iommu/amd/init.c | 23 +++++++++++++++++++++--
>> drivers/iommu/amd/iommu.c | 13 +++++++++++++
>> 4 files changed, 40 insertions(+), 3 deletions(-)
>>
.../...
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index be8761bbef0f..0ebc264726da 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2393,6 +2393,13 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>> pci_max_pasids(to_pci_dev(dev)));
>> }
>>
>> + if (amd_iommu_pgtable == PD_MODE_NONE) {
>> + pr_warn_once("%s: DMA translation not supported by iommu.\n",
>> + __func__);
>> + iommu_dev = ERR_PTR(-ENODEV);
>> + goto out_err;
>> + }
>> +
>> out_err:
>>
>> iommu_completion_wait(iommu);
>> @@ -2480,6 +2487,9 @@ static int pdom_setup_pgtable(struct protection_domain *domain,
>> case PD_MODE_V2:
>> fmt = AMD_IOMMU_V2;
>> break;
>> + case PD_MODE_NONE:
>> + WARN_ON_ONCE(1);
>> + return -EPERM;
>> }
>>
>> domain->iop.pgtbl.cfg.amd.nid = dev_to_node(dev);
>> @@ -2501,6 +2511,9 @@ static inline u64 dma_max_address(enum protection_domain_mode pgtable)
>>
>> static bool amd_iommu_hd_support(struct amd_iommu *iommu)
>> {
>> + if (amd_iommu_hatdis)
>> + return false;
>> +
>> return iommu && (iommu->features & FEATURE_HDSUP);
>> }
>>
>
> It's strange we seem to somehow have host translation disabled, while it
> advertises other translation-related features like the normal case.
I think even if qemu advertises those features we shouldn't hit this code path
as probe() will fail and core shouldn't try to allocate domain, etc.
I was extra cautious and suggested to add this check!
>
> In any case we should probably follow Intel's example (which does similar thing
> to HATSDis) where we only call invoke IOMMU groups
> iommu_device_register()/iommu_device_sysfs_add() with DMA translation enabled?
> That should simplify most of the patch as those codepaths are not reachable via
> kernel/userspace? Unless I am missing something ofc
>
> See also commit c40aaaac10 ("iommu/vt-d: Gracefully handle DMAR units with no
> supported address widths"). I am not sure what else is the closest example here
> besides intel-iommu equivalent.
@Baolu,
Looking into intel driver, my understanding is if DMA is not supported, intel
driver will set `drhd->ignored` to 1 (alloc_iommu()). So iommu_is_dummy() check
will return `true` and hence intel_iommu_probe_device() will fail. But it will
continue to support interrupt remapping. Is that the correct understanding?
-Vasant
Powered by blists - more mailing lists