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

[CLEMENT MATHIEU--DRIF]

On 24/07/2024 08:07, Duan, Zhenzhong 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.
>
>
>> -----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
Ok fine,

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>

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