linux-kernel - Re: [PATCH 1/2] iommu/amd: Add HATDis feature support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4f04d596-685e-4e48-9038-8aee6c927ebc@oracle.com>
Date: Wed, 30 Apr 2025 12:41:04 +0100
From: Joao Martins <joao.m.martins@...cle.com>
To: Ankit Soni <Ankit.Soni@....com>
Cc: iommu@...ts.linux.dev, 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

On 25/04/2025 17:25, Ankit Soni wrote:
> Hi Joao,
> Thank you for review this patch.
> 
> On Thu, Apr 24, 2025 at 01:19:43PM +0100, Joao Martins wrote:
>>>  {
>>> +	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.
> 
> If HATDis bit is enabled driver will ignore host(v1) page table related field,
> this includes nested page table.
> 

OK, I thought it was both actually

>>
>> 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.
> 
> With intel patch you mentioned above, it seems that it is mostly handling 
> "second stage translation support" disable, which will eventually disable dma 
> translation. And in AMD case, HATDis bit indicates host(v1) translation is not
> available, then attempt to use guest(v2) translation, and if both page
> table modes are not available then disable dma tranlation.

OK, I guess it makes sense if HATDis is v1 only.

My other call out was that when we disable dma-translation all together (aka
both modes), then we shouldn't advertise the IOMMU groups (internally and to
userspace) by not calling iommu_device_register()/iommu_device_sysfs_add().

	Joao