Re: [PATCH 14/18] amd_iommu: Toggle address translation on device table entry invalidation

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

On 4/22/25 8:48 AM, Sairaj Kodilkar wrote:
> 
> 
> On 4/14/2025 7:32 AM, Alejandro Jimenez wrote:


>> +    if ((ret < 0) || (!ret && !dte_mode)) {
>> +        /*
>> +         * The DTE could not be retrieved, it is not valid, or it is 
>> not setup
>> +         * for paging. In either case, ensure that if paging was 
>> previously in
>> +         * use then switch to use the no_dma memory region, and 
>> invalidate all
>> +         * existing mappings.
>> +         */
>> +        if (as->addr_translation) {
>> +            as->addr_translation = false;
>> +
>> +            amdvi_switch_address_space(as);
>> +
>> +            IOMMU_NOTIFIER_FOREACH(n, &as->iommu) {
>> +                amdvi_address_space_unmap(as, n);
>> +            }
> 
> Hi,
> I think amdvi_switch_address_space() should come after
> amdvi_address_space_unmap(). amdvi_switch_address_space() unregister the
> VFIO notifier, hence mr->iommu_notify list is empty and we do not unmap
> the shadow page table.

Good point, fixed it.

Alejandro

> 
> Code works fine because eventually vfio_iommu_map_notify maps
> entire the address space, but we should keep the right ordering.
> 
> Regards
> Sairaj Kodilkar
> 
>> +        }
>> +    } else if (!as->addr_translation) {
>> +        /*
>> +         * Installing a DTE that enables translation where it wasn't 
>> previously
>> +         * active. Activate the DMA memory region.
>> +         */
>> +        as->addr_translation = true;
>> +        amdvi_switch_address_space(as);
>> +        amdvi_address_space_sync(as);
>> +    }
>> +
>>       trace_amdvi_devtab_inval(PCI_BUS_NUM(devid), PCI_SLOT(devid),
>>                                PCI_FUNC(devid));
>>   }
>