[PATCH v4 11/17] intel_iommu: Add an internal API to find an address space with PASID

Zhenzhong Duan posted 17 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 11/17] intel_iommu: Add an internal API to find an address space with PASID
Posted by Zhenzhong Duan 1 month, 2 weeks ago
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

This will be used to implement the device IOTLB invalidation

Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 289278ce30..a1596ba47d 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -70,6 +70,11 @@ struct vtd_hiod_key {
     uint8_t devfn;
 };
 
+struct vtd_as_raw_key {
+    uint16_t sid;
+    uint32_t pasid;
+};
+
 struct vtd_iotlb_key {
     uint64_t gfn;
     uint32_t pasid;
@@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr)
     return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <= VTD_INTERRUPT_ADDR_LAST;
 }
 
-static gboolean vtd_find_as_by_sid(gpointer key, gpointer value,
-                                   gpointer user_data)
+static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value,
+                                             gpointer user_data)
 {
     struct vtd_as_key *as_key = (struct vtd_as_key *)key;
-    uint16_t target_sid = *(uint16_t *)user_data;
+    struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data;
     uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn);
-    return sid == target_sid;
+
+    return (as_key->pasid == target.pasid) &&
+           (sid == target.sid);
 }
 
-static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
+static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,
+                                                    uint16_t sid,
+                                                    uint32_t pasid)
 {
-    uint8_t bus_num = PCI_BUS_NUM(sid);
-    VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num];
-
-    if (vtd_as &&
-        (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) {
-        return vtd_as;
-    }
+    struct vtd_as_raw_key key = {
+        .sid = sid,
+        .pasid = pasid
+    };
 
-    vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid, &sid);
-    s->vtd_as_cache[bus_num] = vtd_as;
+    return g_hash_table_find(s->vtd_address_spaces,
+                             vtd_find_as_by_sid_and_pasid, &key);
+}
 
-    return vtd_as;
+static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
+{
+    return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
 }
 
 static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)
-- 
2.34.1


Re: [PATCH v4 11/17] intel_iommu: Add an internal API to find an address space with PASID
Posted by Yi Liu 1 week, 3 days ago
On 2024/9/30 17:26, Zhenzhong Duan wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
> This will be used to implement the device IOTLB invalidation
> 
> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>   hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++---------------
>   1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 289278ce30..a1596ba47d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -70,6 +70,11 @@ struct vtd_hiod_key {
>       uint8_t devfn;
>   };
>   
> +struct vtd_as_raw_key {
> +    uint16_t sid;
> +    uint32_t pasid;
> +};
> +
>   struct vtd_iotlb_key {
>       uint64_t gfn;
>       uint32_t pasid;
> @@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr)
>       return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <= VTD_INTERRUPT_ADDR_LAST;
>   }
>   
> -static gboolean vtd_find_as_by_sid(gpointer key, gpointer value,
> -                                   gpointer user_data)
> +static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value,
> +                                             gpointer user_data)
>   {
>       struct vtd_as_key *as_key = (struct vtd_as_key *)key;
> -    uint16_t target_sid = *(uint16_t *)user_data;
> +    struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data;

why not just define target as a pointer?

>       uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn);
> -    return sid == target_sid;
> +
> +    return (as_key->pasid == target.pasid) &&
> +           (sid == target.sid);

hence using target->pasid and target->sid here. Otherwise, looks good to me.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

>   }
>   
> -static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
> +static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,
> +                                                    uint16_t sid,
> +                                                    uint32_t pasid)
>   {
> -    uint8_t bus_num = PCI_BUS_NUM(sid);
> -    VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num];
> -
> -    if (vtd_as &&
> -        (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) {
> -        return vtd_as;
> -    }
> +    struct vtd_as_raw_key key = {
> +        .sid = sid,
> +        .pasid = pasid
> +    };
>   
> -    vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid, &sid);
> -    s->vtd_as_cache[bus_num] = vtd_as;
> +    return g_hash_table_find(s->vtd_address_spaces,
> +                             vtd_find_as_by_sid_and_pasid, &key);
> +}
>   
> -    return vtd_as;
> +static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
> +{
> +    return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
>   }
>   
>   static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)

-- 
Regards,
Yi Liu

RE: [PATCH v4 11/17] intel_iommu: Add an internal API to find an address space with PASID
Posted by Duan, Zhenzhong 1 week, 2 days ago

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Sent: Monday, November 4, 2024 10:51 AM
>Subject: Re: [PATCH v4 11/17] intel_iommu: Add an internal API to find an address
>space with PASID
>
>On 2024/9/30 17:26, Zhenzhong Duan wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> This will be used to implement the device IOTLB invalidation
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++---------------
>>   1 file changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 289278ce30..a1596ba47d 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -70,6 +70,11 @@ struct vtd_hiod_key {
>>       uint8_t devfn;
>>   };
>>
>> +struct vtd_as_raw_key {
>> +    uint16_t sid;
>> +    uint32_t pasid;
>> +};
>> +
>>   struct vtd_iotlb_key {
>>       uint64_t gfn;
>>       uint32_t pasid;
>> @@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr
>addr)
>>       return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <=
>VTD_INTERRUPT_ADDR_LAST;
>>   }
>>
>> -static gboolean vtd_find_as_by_sid(gpointer key, gpointer value,
>> -                                   gpointer user_data)
>> +static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value,
>> +                                             gpointer user_data)
>>   {
>>       struct vtd_as_key *as_key = (struct vtd_as_key *)key;
>> -    uint16_t target_sid = *(uint16_t *)user_data;
>> +    struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data;
>
>why not just define target as a pointer?
>
>>       uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn);
>> -    return sid == target_sid;
>> +
>> +    return (as_key->pasid == target.pasid) &&
>> +           (sid == target.sid);
>
>hence using target->pasid and target->sid here.

Sure, will do.

Thanks
Zhenzhong

> Otherwise, looks good to me.
>
>Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>
>>   }
>>
>> -static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
>> +static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s,
>> +                                                    uint16_t sid,
>> +                                                    uint32_t pasid)
>>   {
>> -    uint8_t bus_num = PCI_BUS_NUM(sid);
>> -    VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num];
>> -
>> -    if (vtd_as &&
>> -        (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) {
>> -        return vtd_as;
>> -    }
>> +    struct vtd_as_raw_key key = {
>> +        .sid = sid,
>> +        .pasid = pasid
>> +    };
>>
>> -    vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid,
>&sid);
>> -    s->vtd_as_cache[bus_num] = vtd_as;
>> +    return g_hash_table_find(s->vtd_address_spaces,
>> +                             vtd_find_as_by_sid_and_pasid, &key);
>> +}
>>
>> -    return vtd_as;
>> +static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid)
>> +{
>> +    return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID);
>>   }
>>
>>   static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)
>
>--
>Regards,
>Yi Liu