Re: [PATCH v2 19/20] amd_iommu: Do not assume passthrough translation when DTE[TV]=0

[Alejandro Jimenez <alejandro.j.jimenez@oracle.com>]

On 5/12/25 3:00 AM, Sairaj Kodilkar wrote:
> 
> 
> On 5/2/2025 7:46 AM, Alejandro Jimenez wrote:

>> -    if (pte & AMDVI_DEV_TRANSLATION_VALID) {
>> -        level = get_pte_translation_mode(pte);
>> -        if (level >= 7) {
>> -            trace_amdvi_mode_invalid(level, addr);
>> +    if (!(pte & AMDVI_DEV_TRANSLATION_VALID)) {
>> +        /*
>> +         * A DTE with V=1, TV=0 does not have a valid Page Table Root 
>> Pointer.
>> +         * An IOMMU processing a request that requires a table walk 
>> terminates
>> +         * the walk when it encounters this condition. Do the same 
>> and return
>> +         * instead of assuming that the address is forwarded without 
>> translation
>> +         * i.e. the passthrough case, as it is done for the case 
>> where DTE[V]=0.
>> +         */
>> +        return;
>> +    }
> 
> Above change seems redundant since caller of the amdvi_page_walk(),
> amdvi_do_translate() checks the return value of amdvi_as_to_dte().
> amdvi_do_translate() returns when it encounters -AMDVI_FR_DTE_TV and
> does not call amdvi_page_walk().
> 

It is perhaps redundant at this point for the reason you mention, as we 
are already checking the DTE for all sorts of conditions earlier.

But I would like to leave it in since it serves as a good validation 
that any future callers of amdvi_page_walk(), which might not check or 
care about the return of amdvi_as_to_dte(), are passing a DTE that 
specifically supports a page walk, which is exactly what this function 
needs to proceed. Plus it provides a nice place to place the comment 
detailing the IOMMU behavior in such cases.

If you are concerned about efficiency, this is already not a very fast 
path, and the 5 assembly instructions this check takes (with a non-debug 
build I think is ~2 instructions) will certainly not be what brings the 
performance down :).

Alejandro

> Regards
> Sairaj Kodilkar
> 
> 
>