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

Zhenzhong Duan posted 17 patches 2 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap
Posted by Zhenzhong Duan 2 months ago
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
Re: [PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap
Posted by CLEMENT MATHIEU--DRIF 1 month, 3 weeks ago
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?

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
>
Re: [PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap
Posted by CLEMENT MATHIEU--DRIF 1 month, 3 weeks ago

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
>
> 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
>>
RE: [PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap
Posted by Duan, Zhenzhong 1 month, 3 weeks ago

>-----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
>>>
Re: [PATCH v1 14/17] intel_iommu: piotlb invalidation should notify unmap
Posted by CLEMENT MATHIEU--DRIF 1 month, 3 weeks ago

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