Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Vasant Hegde <vasant.hegde@amd.com>]

On 8/26/2026 5:51 PM, Jason Gunthorpe wrote:
> On Wed, Aug 26, 2026 at 04:58:15PM +0530, Vasant Hegde wrote:
>> Pranjal,
>>
>>
>> On 8/26/2026 12:38 AM, Pranjal Shrivastava wrote:
>>> On Tue, Aug 25, 2026 at 02:53:15PM -0300, Jason Gunthorpe wrote:
>>>> On Tue, Aug 25, 2026 at 05:29:59PM +0000, Pranjal Shrivastava wrote:
>>>>
>>>>> I agree, but I wonder why the existing code used memset here 
>>>>> (in ignore_device):
>>>>>
>>>>> memset(&dev_table[devid], 0, sizeof(struct dev_table_entry));
>>>>>
>>>>> I was thinking it might've been done for probe failures in a kdump
>>>>> kernel (normal kexec would've called shutdown for clearing all DTEs).
>>>>> (I see this was added long time back and existed when PCI segments were
>>>>> added [1]).
>>>>
>>>> For kdump you'd want to keep the original translation running in this
>>>> case.
>>
>> If probe is failed then we can't do much. Why keep original translation running?
> 
> It might cause the kdump to fail if you abruptly change the DTE. The
> kdump semantics are to leave the DTE unchanged until a defered attach
> event. An error flow should not defeat that.

Since probe is failed we are not going to attach device again.


> 
> It was already running when probe fails, it can keep going.
> 
> The memset doesn't even work since it doesn't flush the DTE cache, it
> isn't going to actually change any active transfer with a cache hit
> DTE anyhow.

Yeah. We can keep it as is and it can keep going.

> 
>>> Even I'm not sure why we had this memset here, I'll just dig into
>>> the history once if there's anything. Otherwise, I'll simply drop this.
>>>
>>> Vasant, please let us know if there was a different context to it?
>>
>> Looking into git history, it looks like, during boot init_device_table_dma()
>> sets dte.v bit for all devices. So probe fails then clear everything in DTE.
> 
> That isn't how a secure'd iommu driver should boot..
> 
> In a secure boot flow (eg DRTM or something with untrusted PCI) the FW
> will leave the iommu setup to block dma when booting the OS. The OS
> should then ensure that it never permits an identity mapping as it
> boots up the iommu.
> 
> Having the driver boot up with all DTEs programmed to identity (eg
> 0'd) and then try to fix them to blocking after the iommu probes
> devices is security backwards.


During boot, it only sets dte.v bit. For DMA to work we have to set dte.tv bit
that's done in set_dte_entry. So it doesn't break the security.


> 
> Look at how ARM sequences it, the stream table (aka DTEs) are fully
> configured before programming to HW. First it loads force blocking
> then it does a pass to switch those with IOMMU_RESV_DIRECT to identity
> (see arm_smmu_rmr_install_bypass_ste), then it tells the HW to
> hitlessly switch from the FW configuration to the table. Ensuring no
> device that shouldn't has even a moment of identity access.
> 
> Since these secure boots have become very trendy now, I saw AMD PR
> about their version, this should probably be fixed! :)

Yep! We have secure vIOMMU prototype. We want to start discussion once hw-viommu
series settles.

-Vasant