Re: [PATCH v2 15/21] hw/vfio/pci: Check CONFIG_IOMMUFD at runtime using iommufd_builtin()

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 11/3/25 02:54, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Subject: Re: [PATCH v2 15/21] hw/vfio/pci: Check CONFIG_IOMMUFD at runtime
>> using iommufd_builtin()
>>
>> On 10/3/25 05:11, Duan, Zhenzhong wrote:
>>> Hi Philippe,
>>>
>>>> -----Original Message-----
>>>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Subject: [PATCH v2 15/21] hw/vfio/pci: Check CONFIG_IOMMUFD at runtime
>>>> using iommufd_builtin()
>>>>
>>>> Convert the compile time check on the CONFIG_IOMMUFD definition
>>>> by a runtime one by calling iommufd_builtin().
>>>>
>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> hw/vfio/pci.c | 38 ++++++++++++++++++--------------------
>>>> 1 file changed, 18 insertions(+), 20 deletions(-)
>>
>>
>>>> static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
>>>> {
>>>> @@ -3433,9 +3430,10 @@ static void vfio_pci_dev_class_init(ObjectClass
>> *klass,
>>>> void *data)
>>>>
>>>>       device_class_set_legacy_reset(dc, vfio_pci_reset);
>>>>       device_class_set_props(dc, vfio_pci_dev_properties);
>>>> -#ifdef CONFIG_IOMMUFD
>>>> -    object_class_property_add_str(klass, "fd", NULL, vfio_pci_set_fd);
>>>> -#endif
>>>> +    if (iommufd_builtin()) {
>>>> +        device_class_set_props(dc, vfio_pci_dev_iommufd_properties);
>>>
>>> device_class_set_props() is called twice. Won't it break qdev_print_props() and
>> qdev_prop_walk()?
>>
>> device_class_set_props() is misnamed, as it doesn't SET an array of
>> properties, but ADD them (or 'register') to the class.
>>
>> See device_class_set_props_n() in hw/core/qdev-properties.c.
> 
> But it set dc->props_ and dc->props_count_, first to vfio_pci_dev_properties
> and then vfio_pci_dev_iommufd_properties, this will make qdev_prop_walk()
> find only iommufd property and miss others. Do I misunderstand?

You are right! And I thought I was understanding what this code does...