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

["Cédric Le Goater" <clg@redhat.com>]

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


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


diff --git a/meson.build b/meson.build
index 72a57288a026325d5ff753131c037e99f6f35c1a..c946cbef5b29e23475dc4cf345655e0466cbfade 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.