1. Patchew
  2. QEMU
  3. View series

RE: [PATCH v3 00/37] vfio: Adopt iommufd

Duan, Zhenzhong posted 37 patches 1 year, 1 month ago
Only 0 patches received!
RE: [PATCH v3 00/37] vfio: Adopt iommufd
Posted by Duan, Zhenzhong 1 year, 1 month ago 
Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Friday, October 27, 2023 3:46 PM
>Subject: Re: [PATCH v3 00/37] vfio: Adopt iommufd
>
>On 10/27/23 08:17, Duan, Zhenzhong wrote:
>> Hi Markus,
>>
>>> -----Original Message-----
>>> From: Markus Armbruster <armbru@redhat.com>
>>> Sent: Thursday, October 26, 2023 9:27 PM
>>> Subject: Re: [PATCH v3 00/37] vfio: Adopt iommufd
>>>
>>> Zhenzhong Duan <zhenzhong.duan@intel.com> writes:
>>>
>>>> Hi,
>>>>
>>>> Thanks all for giving guides and comments on previous series, here is
>>>> the v3 of pure iommufd support part.
>>>>
>>>> Based on Cédric's suggestion, this series includes an effort to remove
>>>> spapr code from container.c, now all spapr functions are moved to spapr.c
>>>> or spapr_pci_vfio.c, but there are still a few trival check on
>>>> VFIO_SPAPR_TCE_*_IOMMU which I am not sure if deserved to introduce
>many
>>>> callbacks and duplicate code just to remove them. Some functions are moved
>>>> to spapr.c instead of spapr_pci_vfio.c to avoid compile issue because
>>>> spapr_pci_vfio.c is arch specific, or else we need to introduce stub
>>>> functions to those spapr functions moved.
>>>
>>> [...]
>>>
>>>> qemu code:
>>> https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v3
>>>> Based on vfio-next, commit id: fd0e1c8bc1
>>>
>>> I fetched this, and get several compile errors with Clang (but not with
>>> GCC):
>>>
>>>     FAILED: libqemu-x86_64-softmmu.fa.p/hw_vfio_common.c.o
>>>     clang -m64 -mcx16 -Ilibqemu-x86_64-softmmu.fa.p -I. -I.. -Itarget/i386 -
>>> I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -
>>> I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -
>>> I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -
>>> I/usr/include/sysprof-4 -I/usr/include/nss3 -I/usr/include/nspr4 -
>>> I/usr/include/PCSC -fcolor-diagnostics -Wall -Winvalid-pch -Werror -
>std=gnu11 -
>>> O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-
>prototypes -
>>> Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -
>>> Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-
>body
>>> -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-
>>> attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-
>>> negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-
>>> tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-
>not-
>>> at-end -Wthread-safety -isystem /work/armbru/qemu/linux-headers -isystem
>>> linux-headers -iquote . -iquote /work/armbru/qemu -iquote
>>> /work/armbru/qemu/include -iquote
>/work/armbru/qemu/host/include/x86_64 -
>>> iquote /work/armbru/qemu/host/include/generic -iquote
>>> /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -
>>> D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-
>>> common -fwrapv -fsanitize-coverage-allowlist=instrumentation-filter -
>>> fsanitize=fuzzer-no-link -fPIE -isystem../linux-headers -isystemlinux-headers -
>>> DNEED_CPU_H '-DCONFIG_TARGET="x86_64-softmmu-config-target.h"' '-
>>> DCONFIG_DEVICES="x86_64-softmmu-config-devices.h"' -MD -MQ libqemu-
>>> x86_64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-x86_64-
>>> softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-x86_64-
>>> softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>>     ../hw/vfio/common.c:682:40: error: variable 'hostwin' is uninitialized when
>>> used here [-Werror,-Wuninitialized]
>>>             hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>>                                            ^~~~~~~
>>>     ../hw/vfio/common.c:578:31: note: initialize the variable 'hostwin' to
>silence
>>> this warning
>>>         VFIOHostDMAWindow *hostwin;
>>>                                   ^
>>>                                    = NULL
>>>     ../hw/vfio/common.c:785:33: error: variable 'hostwin' is uninitialized when
>>> used here [-Werror,-Wuninitialized]
>>>             pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>>                                     ^~~~~~~
>>>     ../hw/vfio/common.c:783:35: note: initialize the variable 'hostwin' to
>silence
>>> this warning
>>>             VFIOHostDMAWindow *hostwin;
>>>                                       ^
>>>                                        = NULL
>>>     2 errors generated.
>>>     FAILED: tests/unit/test-resv-mem.p/test-resv-mem.c.o
>>>     clang -m64 -mcx16 -Itests/unit/test-resv-mem.p -Itests/unit -I../tests/unit -I.
>-
>>> Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
>-
>>> I/usr/include/sysprof-4 -fcolor-diagnostics -Wall -Winvalid-pch -Werror -
>>> std=gnu11 -O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -
>Wmissing-
>>> prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -
>>> Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-
>qualifiers -
>>> Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -
>>> Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-
>dirs -
>>> Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -
>Wno-
>>> tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-
>not-
>>> at-end -Wthread-safety -isystem /work/armbru/qemu/linux-headers -isystem
>>> linux-headers -iquote . -iquote /work/armbru/qemu -iquote
>>> /work/armbru/qemu/include -iquote
>/work/armbru/qemu/host/include/x86_64 -
>>> iquote /work/armbru/qemu/host/include/generic -iquote
>>> /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -
>>> D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-
>>> common -fwrapv -fsanitize-coverage-allowlist=instrumentation-filter -
>>> fsanitize=fuzzer-no-link -fPIE -MD -MQ tests/unit/test-resv-mem.p/test-resv-
>>> mem.c.o -MF tests/unit/test-resv-mem.p/test-resv-mem.c.o.d -o
>tests/unit/test-
>>> resv-mem.p/test-resv-mem.c.o -c ../tests/unit/test-resv-mem.c
>>>     ../tests/unit/test-resv-mem.c:42:9: error: variable 'i' set but not used [-
>>> Werror,-Wunused-but-set-variable]
>>>         int i = 0;
>>>             ^
>>>     1 error generated.
>>>
>>> Delete @i, please.
>>
>> Thanks for your report, I'll fix it and build with both compilers next time.
>
>a compile on a fedora rawhide also reports :
>
>../hw/vfio/iommufd.c: In function ‘vfio_get_info_iova_range’:
>../hw/vfio/iommufd.c:370:27: error: cast from pointer to integer of different size
>[-Werror=pointer-to-int-cast]
>   370 |     info->allowed_iovas = (uint64_t)(info + 1);
>       |                           ^
>../hw/vfio/iommufd.c:377:19: error: cast to pointer from integer of different size
>[-Werror=int-to-pointer-cast]
>   377 |     iova_ranges = (struct iommu_iova_range *)info->allowed_iovas;
>       |                   ^
>cc1: all warnings being treated as errors

I failed to understand this warning, is your env a 32bit fedora?

>
>
>> About second error, I'll send a separate patch as it's unrelated to this series.
>
>
>Since this is a big series, I suggest that you take this patch in your build
>environment,

Got it, will do, thanks for your suggestions.

BRs.
Zhenzhong

>
>
>diff --git a/meson.build b/meson.build
>index
>72a57288a026325d5ff753131c037e99f6f35c1a..c946cbef5b29e23475dc4cf3456
>55e0466cbfade 100644
>--- a/meson.build
>+++ b/meson.build
>@@ -462,6 +462,9 @@ warn_flags = [
>    '-Wno-tautological-type-limit-compare',
>    '-Wno-psabi',
>    '-Wno-gnu-variable-sized-type-not-at-end',
>+  '-Wshadow=local',
>+  '-Wno-error=shadow=local',
>+  '-Wno-error=shadow=compatible-local',
>  ]
>
>  if targetos != 'darwin'
>
>
>Thanks,
>
>C.
Re: [PATCH v3 00/37] vfio: Adopt iommufd
Posted by Cédric Le Goater 1 year, 1 month ago 
On 10/27/23 10:16, Duan, Zhenzhong wrote:
> Hi Cédric,
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: Friday, October 27, 2023 3:46 PM
>> Subject: Re: [PATCH v3 00/37] vfio: Adopt iommufd
>>
>> On 10/27/23 08:17, Duan, Zhenzhong wrote:
>>> Hi Markus,
>>>
>>>> -----Original Message-----
>>>> From: Markus Armbruster <armbru@redhat.com>
>>>> Sent: Thursday, October 26, 2023 9:27 PM
>>>> Subject: Re: [PATCH v3 00/37] vfio: Adopt iommufd
>>>>
>>>> Zhenzhong Duan <zhenzhong.duan@intel.com> writes:
>>>>
>>>>> Hi,
>>>>>
>>>>> Thanks all for giving guides and comments on previous series, here is
>>>>> the v3 of pure iommufd support part.
>>>>>
>>>>> Based on Cédric's suggestion, this series includes an effort to remove
>>>>> spapr code from container.c, now all spapr functions are moved to spapr.c
>>>>> or spapr_pci_vfio.c, but there are still a few trival check on
>>>>> VFIO_SPAPR_TCE_*_IOMMU which I am not sure if deserved to introduce
>> many
>>>>> callbacks and duplicate code just to remove them. Some functions are moved
>>>>> to spapr.c instead of spapr_pci_vfio.c to avoid compile issue because
>>>>> spapr_pci_vfio.c is arch specific, or else we need to introduce stub
>>>>> functions to those spapr functions moved.
>>>>
>>>> [...]
>>>>
>>>>> qemu code:
>>>> https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v3
>>>>> Based on vfio-next, commit id: fd0e1c8bc1
>>>>
>>>> I fetched this, and get several compile errors with Clang (but not with
>>>> GCC):
>>>>
>>>>      FAILED: libqemu-x86_64-softmmu.fa.p/hw_vfio_common.c.o
>>>>      clang -m64 -mcx16 -Ilibqemu-x86_64-softmmu.fa.p -I. -I.. -Itarget/i386 -
>>>> I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -
>>>> I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -
>>>> I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -
>>>> I/usr/include/sysprof-4 -I/usr/include/nss3 -I/usr/include/nspr4 -
>>>> I/usr/include/PCSC -fcolor-diagnostics -Wall -Winvalid-pch -Werror -
>> std=gnu11 -
>>>> O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-
>> prototypes -
>>>> Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -
>>>> Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-
>> body
>>>> -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-
>>>> attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-
>>>> negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-
>>>> tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-
>> not-
>>>> at-end -Wthread-safety -isystem /work/armbru/qemu/linux-headers -isystem
>>>> linux-headers -iquote . -iquote /work/armbru/qemu -iquote
>>>> /work/armbru/qemu/include -iquote
>> /work/armbru/qemu/host/include/x86_64 -
>>>> iquote /work/armbru/qemu/host/include/generic -iquote
>>>> /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -
>>>> D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-
>>>> common -fwrapv -fsanitize-coverage-allowlist=instrumentation-filter -
>>>> fsanitize=fuzzer-no-link -fPIE -isystem../linux-headers -isystemlinux-headers -
>>>> DNEED_CPU_H '-DCONFIG_TARGET="x86_64-softmmu-config-target.h"' '-
>>>> DCONFIG_DEVICES="x86_64-softmmu-config-devices.h"' -MD -MQ libqemu-
>>>> x86_64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-x86_64-
>>>> softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-x86_64-
>>>> softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>>>      ../hw/vfio/common.c:682:40: error: variable 'hostwin' is uninitialized when
>>>> used here [-Werror,-Wuninitialized]
>>>>              hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>>>                                             ^~~~~~~
>>>>      ../hw/vfio/common.c:578:31: note: initialize the variable 'hostwin' to
>> silence
>>>> this warning
>>>>          VFIOHostDMAWindow *hostwin;
>>>>                                    ^
>>>>                                     = NULL
>>>>      ../hw/vfio/common.c:785:33: error: variable 'hostwin' is uninitialized when
>>>> used here [-Werror,-Wuninitialized]
>>>>              pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1;
>>>>                                      ^~~~~~~
>>>>      ../hw/vfio/common.c:783:35: note: initialize the variable 'hostwin' to
>> silence
>>>> this warning
>>>>              VFIOHostDMAWindow *hostwin;
>>>>                                        ^
>>>>                                         = NULL
>>>>      2 errors generated.
>>>>      FAILED: tests/unit/test-resv-mem.p/test-resv-mem.c.o
>>>>      clang -m64 -mcx16 -Itests/unit/test-resv-mem.p -Itests/unit -I../tests/unit -I.
>> -
>>>> Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
>> -
>>>> I/usr/include/sysprof-4 -fcolor-diagnostics -Wall -Winvalid-pch -Werror -
>>>> std=gnu11 -O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -
>> Wmissing-
>>>> prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -
>>>> Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-
>> qualifiers -
>>>> Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -
>>>> Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-
>> dirs -
>>>> Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -
>> Wno-
>>>> tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-
>> not-
>>>> at-end -Wthread-safety -isystem /work/armbru/qemu/linux-headers -isystem
>>>> linux-headers -iquote . -iquote /work/armbru/qemu -iquote
>>>> /work/armbru/qemu/include -iquote
>> /work/armbru/qemu/host/include/x86_64 -
>>>> iquote /work/armbru/qemu/host/include/generic -iquote
>>>> /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -
>>>> D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-
>>>> common -fwrapv -fsanitize-coverage-allowlist=instrumentation-filter -
>>>> fsanitize=fuzzer-no-link -fPIE -MD -MQ tests/unit/test-resv-mem.p/test-resv-
>>>> mem.c.o -MF tests/unit/test-resv-mem.p/test-resv-mem.c.o.d -o
>> tests/unit/test-
>>>> resv-mem.p/test-resv-mem.c.o -c ../tests/unit/test-resv-mem.c
>>>>      ../tests/unit/test-resv-mem.c:42:9: error: variable 'i' set but not used [-
>>>> Werror,-Wunused-but-set-variable]
>>>>          int i = 0;
>>>>              ^
>>>>      1 error generated.
>>>>
>>>> Delete @i, please.
>>>
>>> Thanks for your report, I'll fix it and build with both compilers next time.
>>
>> a compile on a fedora rawhide also reports :
>>
>> ../hw/vfio/iommufd.c: In function ‘vfio_get_info_iova_range’:
>> ../hw/vfio/iommufd.c:370:27: error: cast from pointer to integer of different size
>> [-Werror=pointer-to-int-cast]
>>    370 |     info->allowed_iovas = (uint64_t)(info + 1);
>>        |                           ^
>> ../hw/vfio/iommufd.c:377:19: error: cast to pointer from integer of different size
>> [-Werror=int-to-pointer-cast]
>>    377 |     iova_ranges = (struct iommu_iova_range *)info->allowed_iovas;
>>        |                   ^
>> cc1: all warnings being treated as errors
> 
> I failed to understand this warning, is your env a 32bit fedora?

it's not a rawhide but a 32bit indeed.

Thanks,

C.


> 
>>
>>
>>> About second error, I'll send a separate patch as it's unrelated to this series.
>>
>>
>> Since this is a big series, I suggest that you take this patch in your build
>> environment,
> 
> Got it, will do, thanks for your suggestions.
> 
> BRs.
> Zhenzhong
> 
>>
>>
>> diff --git a/meson.build b/meson.build
>> index
>> 72a57288a026325d5ff753131c037e99f6f35c1a..c946cbef5b29e23475dc4cf3456
>> 55e0466cbfade 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -462,6 +462,9 @@ warn_flags = [
>>     '-Wno-tautological-type-limit-compare',
>>     '-Wno-psabi',
>>     '-Wno-gnu-variable-sized-type-not-at-end',
>> +  '-Wshadow=local',
>> +  '-Wno-error=shadow=local',
>> +  '-Wno-error=shadow=compatible-local',
>>   ]
>>
>>   if targetos != 'darwin'
>>
>>
>> Thanks,
>>
>> C.
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.