[v2,13/17] intel_iommu: piotlb invalidation should notify unmap

This is used by some emulated devices which caches address
translation result. When piotlb invalidation issued in guest,
those caches should be refreshed.

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(-)

On 2024/8/5 14:27, 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.

Perhaps I have asked it in the before. :) To me, such emulated devices
should implement an ATS-capability. You may mention the devices that
does not implement ATS-capability, but caches the translation result,
and note that it is better to implement ATS cap if there is need to
cache the translation request.

> 
> 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 fa00f85fd7..317e630e08 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2907,7 +2907,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);
>               }
>           }
> @@ -2919,6 +2919,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;
> @@ -2929,6 +2932,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;
> +            }
> +
> +            /*
> +             * 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,

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 13/17] intel_iommu: piotlb invalidation should
>notify unmap
>
>On 2024/8/5 14:27, 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.
>
>Perhaps I have asked it in the before. :) To me, such emulated devices
>should implement an ATS-capability. You may mention the devices that
>does not implement ATS-capability, but caches the translation result,
>and note that it is better to implement ATS cap if there is need to
>cache the translation request.

OK, will do. Will be like:

"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 request."

Thanks
Zhenzhong

>
>>
>> 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 fa00f85fd7..317e630e08 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2907,7 +2907,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);
>>               }
>>           }
>> @@ -2919,6 +2919,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;
>> @@ -2929,6 +2932,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;
>> +            }
>> +
>> +            /*
>> +             * 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

On 2024/8/19 17:57, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Subject: Re: [PATCH v2 13/17] intel_iommu: piotlb invalidation should
>> notify unmap
>>
>> On 2024/8/5 14:27, 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.
>>
>> Perhaps I have asked it in the before. :) To me, such emulated devices
>> should implement an ATS-capability. You may mention the devices that
>> does not implement ATS-capability, but caches the translation result,
>> and note that it is better to implement ATS cap if there is need to
>> cache the translation request.
> 
> OK, will do. Will be like:
> 
> "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 request."

sorry for a typo. s/request/result/

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 13/17] intel_iommu: piotlb invalidation should
>notify unmap
>
>On 2024/8/19 17:57, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>> Subject: Re: [PATCH v2 13/17] intel_iommu: piotlb invalidation should
>>> notify unmap
>>>
>>> On 2024/8/5 14:27, 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.
>>>
>>> Perhaps I have asked it in the before. :) To me, such emulated devices
>>> should implement an ATS-capability. You may mention the devices that
>>> does not implement ATS-capability, but caches the translation result,
>>> and note that it is better to implement ATS cap if there is need to
>>> cache the translation request.
>>
>> OK, will do. Will be like:
>>
>> "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 request."
>
>sorry for a typo. s/request/result/

Applied.

Thanks
Zhenzhong