RE: [PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap

[Duan, Zhenzhong]

>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>Subject: Re: [PATCH v1 14/17] intel_iommu: piotlb invalidation should
>notify unmap
>
>
>
>On 24/07/2024 07:45, CLEMENT MATHIEU--DRIF wrote:
>> Maybe I'm missing something but why do we invalidate device IOTLB
>> upon piotlb receipt of a regular IOTLB inv desc?
>> I don't get why we don't wait for a device IOTLB inv desc?
>I thought you were planning to remove that after the last rfc version

Look at vtd_iotlb_page_invalidate(), it has same operation.
Reason is even if we don't enable device IOTLB, devices such as vhost may still 
caches IOTLB entries. So we need to flush those stale IOTLB entries in this 
case.

Thanks
Zhenzhong

>>
>> On 18/07/2024 10:16, Zhenzhong Duan wrote:
>>> Caution: External email. Do not open attachments or click links, unless
>this email comes from a known sender and you know the content is safe.
>>>
>>>
>>> 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>
>>> ---
>>>    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 8b66d6cfa5..c0116497b1 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -2910,7 +2910,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);
>>>                }
>>>            }
>>> @@ -2922,6 +2922,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;
>>> @@ -2932,6 +2935,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,
>>> --
>>> 2.34.1
>>>