Re: [PATCH v4 13/17] intel_iommu: piotlb invalidation should notify unmap

[Yi Liu <yi.l.liu@intel.com>]

On 2024/9/30 17:26, Zhenzhong Duan wrote:
> This is used by some emulated devices which caches address
> translation result. When piotlb invalidation issued in guest,
> those caches should be refreshed.
> 
> For device that does not implement ATS capability or disable
> it but still caches the translation result, it is better to
> implement ATS cap or enable it if there is need to cache the
> translation result.

Is there a list of such devices? Though I don't object this patch,
but it may be helpful to list such devices. One day we may remove
this when the list becomes empty.

> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
> ---
>   hw/i386/intel_iommu.c | 35 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 5ea59167b3..91d7b1abfa 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2908,7 +2908,7 @@ static void vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
>                   continue;
>               }
>   
> -            if (!s->scalable_modern) {
> +            if (!s->scalable_modern || !vtd_as_has_map_notifier(vtd_as)) {
>                   vtd_address_space_sync(vtd_as);
>               }
>           }
> @@ -2920,6 +2920,9 @@ static void vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>                                          bool ih)
>   {
>       VTDIOTLBPageInvInfo info;
> +    VTDAddressSpace *vtd_as;
> +    VTDContextEntry ce;
> +    hwaddr size = (1 << am) * VTD_PAGE_SIZE;
>   
>       info.domain_id = domain_id;
>       info.pasid = pasid;
> @@ -2930,6 +2933,36 @@ static void vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>       g_hash_table_foreach_remove(s->iotlb,
>                                   vtd_hash_remove_by_page_piotlb, &info);
>       vtd_iommu_unlock(s);
> +
> +    QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
> +        if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
> +                                      vtd_as->devfn, &ce) &&
> +            domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid)) {
> +            uint32_t rid2pasid = VTD_CE_GET_RID2PASID(&ce);
> +            IOMMUTLBEvent event;
> +
> +            if ((vtd_as->pasid != PCI_NO_PASID || pasid != rid2pasid) &&
> +                vtd_as->pasid != pasid) {
> +                continue;

not quite get the logic here. patch 4 has a similar logic.

> +            }
> +
> +            /*
> +             * Page-Selective-within-PASID PASID-based-IOTLB Invalidation
> +             * does not flush stage-2 entries. See spec section 6.5.2.4
> +             */
> +            if (!s->scalable_modern) {
> +                continue;
> +            }
> +
> +            event.type = IOMMU_NOTIFIER_UNMAP;
> +            event.entry.target_as = &address_space_memory;
> +            event.entry.iova = addr;
> +            event.entry.perm = IOMMU_NONE;
> +            event.entry.addr_mask = size - 1;
> +            event.entry.translated_addr = 0;
> +            memory_region_notify_iommu(&vtd_as->iommu, 0, event);
> +        }
> +    }
>   }
>   
>   static bool vtd_process_piotlb_desc(IntelIOMMUState *s,

-- 
Regards,
Yi Liu