RE: [PATCH v2 05/10] vfio: Implement get_host_iommu_info() callback

2024-04-15 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>Subject: Re: [PATCH v2 05/10] vfio: Implement get_host_iommu_info()
>callback
>
>On 4/8/24 10:12, Zhenzhong Duan wrote:
>> Utilize iova_ranges to calculate host IOMMU address width and
>> package it in HIOD_LEGACY_INFO for vIOMMU usage.
>>
>> HIOD_LEGACY_INFO will be used by both VFIO and VDPA so declare
>> it in host_iommu_device.h.
>>
>> Signed-off-by: Zhenzhong Duan 
>> ---
>>   include/sysemu/host_iommu_device.h | 10 ++
>>   hw/vfio/container.c| 24 
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/include/sysemu/host_iommu_device.h
>b/include/sysemu/host_iommu_device.h
>> index 22ccbe3a5d..beb8be8231 100644
>> --- a/include/sysemu/host_iommu_device.h
>> +++ b/include/sysemu/host_iommu_device.h
>> @@ -16,4 +16,14 @@ struct HostIOMMUDeviceClass {
>>   int (*get_host_iommu_info)(HostIOMMUDevice *hiod, void *data,
>uint32_t len,
>>  Error **errp);
>>   };
>> +
>> +/*
>> + * Define the format of host IOMMU related info that current VFIO
>> + * or VDPA can privode to vIOMMU.
>> + *
>> + * @aw_bits: Host IOMMU address width. 0xff if no limitation.
>> + */
>> +typedef struct HIOD_LEGACY_INFO {
>
>Please use CamelCase names.

Sure.

>
>> +uint8_t aw_bits;
>> +} HIOD_LEGACY_INFO;
>>   #endif
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 44018ef085..ba0ad4a41b 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -1143,8 +1143,32 @@ static void
>vfio_iommu_legacy_class_init(ObjectClass *klass, void *data)
>>   vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
>>   };
>>
>> +static int hiod_legacy_vfio_get_host_iommu_info(HostIOMMUDevice
>*hiod,
>> +void *data, uint32_t len,
>> +Error **errp)
>> +{
>> +VFIODevice *vbasedev = HIOD_LEGACY_VFIO(hiod)->vdev;
>> +/* iova_ranges is a sorted list */
>> +GList *l = g_list_last(vbasedev->bcontainer->iova_ranges);
>> +HIOD_LEGACY_INFO *info = data;
>> +
>> +assert(sizeof(HIOD_LEGACY_INFO) <= len);
>> +
>> +if (l) {
>> +Range *range = l->data;
>> +info->aw_bits = find_last_bit(>upb, BITS_PER_LONG) + 1;
>
>There is a comment in range.h saying:
>
> /*
>  * Do not access members directly, use the functions!
>
>Please introduce a new helper.

Sure, thanks for point out.

BRs.
Zhenzhong

>
>
>Thanks,
>
>C.
>
>
>
>> +} else {
>> +info->aw_bits = 0xff;
>> +}
>> +
>> +return 0;
>> +}
>> +
>>   static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data)
>>   {
>> +HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc);
>> +
>> +hioc->get_host_iommu_info =
>hiod_legacy_vfio_get_host_iommu_info;
>>   };
>>
>>   static const TypeInfo types[] = {



Re: [PATCH v2 05/10] vfio: Implement get_host_iommu_info() callback

2024-04-15 Thread Cédric Le Goater

On 4/8/24 10:12, Zhenzhong Duan wrote:

Utilize iova_ranges to calculate host IOMMU address width and
package it in HIOD_LEGACY_INFO for vIOMMU usage.

HIOD_LEGACY_INFO will be used by both VFIO and VDPA so declare
it in host_iommu_device.h.

Signed-off-by: Zhenzhong Duan 
---
  include/sysemu/host_iommu_device.h | 10 ++
  hw/vfio/container.c| 24 
  2 files changed, 34 insertions(+)

diff --git a/include/sysemu/host_iommu_device.h 
b/include/sysemu/host_iommu_device.h
index 22ccbe3a5d..beb8be8231 100644
--- a/include/sysemu/host_iommu_device.h
+++ b/include/sysemu/host_iommu_device.h
@@ -16,4 +16,14 @@ struct HostIOMMUDeviceClass {
  int (*get_host_iommu_info)(HostIOMMUDevice *hiod, void *data, uint32_t 
len,
 Error **errp);
  };
+
+/*
+ * Define the format of host IOMMU related info that current VFIO
+ * or VDPA can privode to vIOMMU.
+ *
+ * @aw_bits: Host IOMMU address width. 0xff if no limitation.
+ */
+typedef struct HIOD_LEGACY_INFO {


Please use CamelCase names.


+uint8_t aw_bits;
+} HIOD_LEGACY_INFO;
  #endif
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 44018ef085..ba0ad4a41b 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -1143,8 +1143,32 @@ static void vfio_iommu_legacy_class_init(ObjectClass 
*klass, void *data)
  vioc->pci_hot_reset = vfio_legacy_pci_hot_reset;
  };
  
+static int hiod_legacy_vfio_get_host_iommu_info(HostIOMMUDevice *hiod,

+void *data, uint32_t len,
+Error **errp)
+{
+VFIODevice *vbasedev = HIOD_LEGACY_VFIO(hiod)->vdev;
+/* iova_ranges is a sorted list */
+GList *l = g_list_last(vbasedev->bcontainer->iova_ranges);
+HIOD_LEGACY_INFO *info = data;
+
+assert(sizeof(HIOD_LEGACY_INFO) <= len);
+
+if (l) {
+Range *range = l->data;
+info->aw_bits = find_last_bit(>upb, BITS_PER_LONG) + 1;


There is a comment in range.h saying:

/*
 * Do not access members directly, use the functions!

Please introduce a new helper.


Thanks,

C.




+} else {
+info->aw_bits = 0xff;
+}
+
+return 0;
+}
+
  static void hiod_legacy_vfio_class_init(ObjectClass *oc, void *data)
  {
+HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc);
+
+hioc->get_host_iommu_info = hiod_legacy_vfio_get_host_iommu_info;
  };
  
  static const TypeInfo types[] = {