Hello,

This series is based on the previous one that added the basic VFIO
migration protocol v2 implementation [1].

The series starts by adding pre-copy support for VFIO migration protocol
v2. Pre-copy support allows the VFIO device data to be transferred while
the VM is running. This can improve performance and reduce migration
downtime. Full description of it can be found here [2].

It then moves to implement device dirty page tracking. Device dirty page
tracking allows the VFIO device to record its DMAs and report them back
when needed. This is part of VFIO migration and is used during pre-copy
phase of migration to track the RAM pages that the device has written to
and mark those pages dirty, so they can later be re-sent to target.

Device dirty page tracking uses the DMA logging uAPI to discover device
capabilities, to start and stop tracking, and to get dirty page bitmap
report. Extra details and uAPI definition can be found here [3].

Device dirty page tracking operates in VFIOContainer scope. I.e., When
dirty tracking is started, stopped or dirty page report is queried, all
devices within a VFIOContainer are iterated and for each of them device
dirty page tracking is started, stopped or dirty page report is queried,
respectively.

Device dirty page tracking is used only if all devices within a
VFIOContainer support it. Otherwise, VFIO IOMMU dirty page tracking is
used, and if that is not supported as well, memory is perpetually marked
dirty by QEMU. Note that since VFIO IOMMU dirty page tracking has no HW
support, the last two usually have the same effect of perpetually
marking all pages dirty.

Normally, when asked to start dirty tracking, all the currently DMA
mapped ranges are tracked by device dirty page tracking. However, when
vIOMMU is enabled IOVA ranges are DMA mapped/unmapped on the fly as the
vIOMMU maps/unmaps them. These IOVA ranges can potentially be mapped
anywhere in the vIOMMU IOVA space. Due to this dynamic nature of vIOMMU
mapping/unmapping, tracking only the currently DMA mapped IOVA ranges
doesn't work very well.

Thus, when vIOMMU is enabled, we try to track the entire vIOMMU IOVA
space. If that fails (IOVA space can be rather big and we might hit HW
limitation), we try to track smaller range while marking untracked
ranges dirty.

Patch breakdown:
- Patches 1-3 add VFIO migration pre-copy support.
- Patches 4-10 fix bugs and do some preparatory work required prior to
  adding device dirty page tracking.
- Patches 11-13 implement device dirty page tracking.
- Patches 14-18 add vIOMMU support to device dirty page tracking.
- Patches 19-20 enable device dirty page tracking and document it.



Changes from v1 [4]:
- Rebased on latest master branch. As part of it, made some changes in
  pre-copy to adjust it to Juan's new patches:
  1. Added a new patch that passes threshold_size parameter to
     .state_pending_{estimate,exact}() handlers.
  2. Added a new patch that refactors vfio_save_block().
  3. Changed the pre-copy patch to cache and report pending pre-copy
     size in the .state_pending_estimate() handler.
- Removed unnecessary P2P code. This should be added later on when P2P
  support is added. (Alex)
- Moved the dirty sync to be after the DMA unmap in vfio_dma_unmap()
  (patch #11). (Alex)
- Stored vfio_devices_all_device_dirty_tracking()'s value in a local
  variable in vfio_get_dirty_bitmap() so it can be re-used (patch #11).
- Refactored the viommu device dirty tracking ranges creation code to
  make it clearer (patch #15).
- Changed overflow check in vfio_iommu_range_is_device_tracked() to
  emphasize that we specifically check for 2^64 wrap around (patch #15).
- Added R-bs / Acks.

Thanks.

[1]
https://lore.kernel.org/qemu-devel/167658846945.932837.1420176491103357684.stgit@omen/

[2]
https://lore.kernel.org/kvm/20221206083438.37807-3-yishaih@nvidia.com/

[3]
https://lore.kernel.org/netdev/20220908183448.195262-4-yishaih@nvidia.com/

Avihai Horon (14):
  migration: Pass threshold_size to .state_pending_{estimate,exact}()
  vfio/migration: Refactor vfio_save_block() to return saved data size
  vfio/migration: Add VFIO migration pre-copy support
  vfio/common: Fix error reporting in vfio_get_dirty_bitmap()
  vfio/common: Fix wrong %m usages
  vfio/common: Abort migration if dirty log start/stop/sync fails
  vfio/common: Add VFIOBitmap and (de)alloc functions
  vfio/common: Extract code from vfio_get_dirty_bitmap() to new function
  vfio/common: Extract vIOMMU code from vfio_sync_dirty_bitmap()
  memory/iommu: Add IOMMU_ATTR_MAX_IOVA attribute
  intel-iommu: Implement get_attr() method
  vfio/common: Support device dirty page tracking with vIOMMU
  vfio/common: Optimize device dirty page tracking with vIOMMU
  docs/devel: Document VFIO device dirty page tracking

Joao Martins (6):
  util: Add iova_tree_nnodes()
  util: Extend iova_tree_foreach() to take data argument
  vfio/common: Record DMA mapped IOVA ranges
  vfio/common: Add device dirty page tracking start/stop
  vfio/common: Add device dirty page bitmap sync
  vfio/migration: Query device dirty page tracking support

 docs/devel/vfio-migration.rst  |  85 ++-
 include/exec/memory.h          |   3 +-
 include/hw/vfio/vfio-common.h  |  10 +
 include/migration/register.h   |   7 +-
 include/qemu/iova-tree.h       |  19 +-
 migration/savevm.h             |   6 +-
 hw/i386/intel_iommu.c          |  18 +
 hw/s390x/s390-stattrib.c       |   4 +-
 hw/vfio/common.c               | 911 +++++++++++++++++++++++++++++----
 hw/vfio/migration.c            | 210 +++++++-
 migration/block-dirty-bitmap.c |   2 +-
 migration/block.c              |   4 +-
 migration/migration.c          |  12 +-
 migration/ram.c                |   6 +-
 migration/savevm.c             |  12 +-
 util/iova-tree.c               |  23 +-
 hw/vfio/trace-events           |   4 +-
 migration/trace-events         |   4 +-
 18 files changed, 1161 insertions(+), 179 deletions(-)

-- 
2.26.3

There are various errors running this through the CI on gitlab.

This one seems bogus but needs to be resolved regardless:

https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o 
2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
2789 1772 |     if (ret) {
2790      |        ^

32-bit builds have some actual errors though:

https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o 
2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
2602../hw/vfio/common.c: In function 'vfio_device_feature_dma_logging_start_create':
2603../hw/vfio/common.c:1572:27: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
2604 1572 |         control->ranges = (uint64_t)ranges;
2605      |                           ^
2606../hw/vfio/common.c:1596:23: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
2607 1596 |     control->ranges = (uint64_t)ranges;
2608      |                       ^
2609../hw/vfio/common.c: In function 'vfio_device_feature_dma_logging_start_destroy':
2610../hw/vfio/common.c:1620:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
2611 1620 |         (struct vfio_device_feature_dma_logging_range *)control->ranges;
2612      |         ^
2613../hw/vfio/common.c: In function 'vfio_device_dma_logging_report':
2614../hw/vfio/common.c:1810:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
2615 1810 |     report->bitmap = (uint64_t)bitmap;
2616      |                      ^

Thanks,
Alex

On 22/02/2023 22:55, Alex Williamson wrote:
> External email: Use caution opening links or attachments
>
>
> There are various errors running this through the CI on gitlab.
>
> This one seems bogus but needs to be resolved regardless:
>
> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 2789 1772 |     if (ret) {
> 2790      |        ^
>
> 32-bit builds have some actual errors though:
>
> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
> 2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
> 2602../hw/vfio/common.c: In function 'vfio_device_feature_dma_logging_start_create':
> 2603../hw/vfio/common.c:1572:27: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 2604 1572 |         control->ranges = (uint64_t)ranges;
> 2605      |                           ^
> 2606../hw/vfio/common.c:1596:23: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 2607 1596 |     control->ranges = (uint64_t)ranges;
> 2608      |                       ^
> 2609../hw/vfio/common.c: In function 'vfio_device_feature_dma_logging_start_destroy':
> 2610../hw/vfio/common.c:1620:9: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
> 2611 1620 |         (struct vfio_device_feature_dma_logging_range *)control->ranges;
> 2612      |         ^
> 2613../hw/vfio/common.c: In function 'vfio_device_dma_logging_report':
> 2614../hw/vfio/common.c:1810:22: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 2615 1810 |     report->bitmap = (uint64_t)bitmap;
> 2616      |                      ^

Sure, I will fix these errors.

Thanks.

On 23/02/2023 14:56, Avihai Horon wrote:
> On 22/02/2023 22:55, Alex Williamson wrote:
>> There are various errors running this through the CI on gitlab.
>>
>> This one seems bogus but needs to be resolved regardless:
>>
>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I..
>> -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader
>> -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0
>> -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall
>> -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
>> /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote .
>> -iquote /builds/alex.williamson/qemu -iquote
>> /builds/alex.williamson/qemu/include -iquote
>> /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE
>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>> -isystemlinux-headers -DNEED_CPU_H
>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this
>> function [-Werror=maybe-uninitialized]
>> 2789 1772 |     if (ret) {
>> 2790      |        ^
>>
>> 32-bit builds have some actual errors though:
>>
>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>> 2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm
>> -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1
>> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4
>> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
>> -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers
>> -iquote . -iquote /builds/alex.williamson/qemu -iquote
>> /builds/alex.williamson/qemu/include -iquote
>> /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>> -isystemlinux-headers -DNEED_CPU_H
>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>> 2602../hw/vfio/common.c: In function
>> 'vfio_device_feature_dma_logging_start_create':
>> 2603../hw/vfio/common.c:1572:27: error: cast from pointer to integer of
>> different size [-Werror=pointer-to-int-cast]
>> 2604 1572 |         control->ranges = (uint64_t)ranges;
>> 2605      |                           ^
>> 2606../hw/vfio/common.c:1596:23: error: cast from pointer to integer of
>> different size [-Werror=pointer-to-int-cast]
>> 2607 1596 |     control->ranges = (uint64_t)ranges;
>> 2608      |                       ^
>> 2609../hw/vfio/common.c: In function
>> 'vfio_device_feature_dma_logging_start_destroy':
>> 2610../hw/vfio/common.c:1620:9: error: cast to pointer from integer of
>> different size [-Werror=int-to-pointer-cast]
>> 2611 1620 |         (struct vfio_device_feature_dma_logging_range
>> *)control->ranges;
>> 2612      |         ^
>> 2613../hw/vfio/common.c: In function 'vfio_device_dma_logging_report':
>> 2614../hw/vfio/common.c:1810:22: error: cast from pointer to integer of
>> different size [-Werror=pointer-to-int-cast]
>> 2615 1810 |     report->bitmap = (uint64_t)bitmap;
>> 2616      |                      ^
> 
> Sure, I will fix these errors.

Just a thought: should the pre-copy series be moved towards the end of this
series, given that it's more of an improvement of downtime than a must-have like
dirty tracking?

	Joao

On 24/02/2023 21:26, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> On 23/02/2023 14:56, Avihai Horon wrote:
>> On 22/02/2023 22:55, Alex Williamson wrote:
>>> There are various errors running this through the CI on gitlab.
>>>
>>> This one seems bogus but needs to be resolved regardless:
>>>
>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I..
>>> -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader
>>> -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0
>>> -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall
>>> -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
>>> /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote .
>>> -iquote /builds/alex.williamson/qemu -iquote
>>> /builds/alex.williamson/qemu/include -iquote
>>> /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE
>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>> -isystemlinux-headers -DNEED_CPU_H
>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
>>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this
>>> function [-Werror=maybe-uninitialized]
>>> 2789 1772 |     if (ret) {
>>> 2790      |        ^
>>>
>>> 32-bit builds have some actual errors though:
>>>
>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>> 2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm
>>> -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1
>>> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4
>>> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
>>> -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers
>>> -iquote . -iquote /builds/alex.williamson/qemu -iquote
>>> /builds/alex.williamson/qemu/include -iquote
>>> /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>> -isystemlinux-headers -DNEED_CPU_H
>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>> 2602../hw/vfio/common.c: In function
>>> 'vfio_device_feature_dma_logging_start_create':
>>> 2603../hw/vfio/common.c:1572:27: error: cast from pointer to integer of
>>> different size [-Werror=pointer-to-int-cast]
>>> 2604 1572 |         control->ranges = (uint64_t)ranges;
>>> 2605      |                           ^
>>> 2606../hw/vfio/common.c:1596:23: error: cast from pointer to integer of
>>> different size [-Werror=pointer-to-int-cast]
>>> 2607 1596 |     control->ranges = (uint64_t)ranges;
>>> 2608      |                       ^
>>> 2609../hw/vfio/common.c: In function
>>> 'vfio_device_feature_dma_logging_start_destroy':
>>> 2610../hw/vfio/common.c:1620:9: error: cast to pointer from integer of
>>> different size [-Werror=int-to-pointer-cast]
>>> 2611 1620 |         (struct vfio_device_feature_dma_logging_range
>>> *)control->ranges;
>>> 2612      |         ^
>>> 2613../hw/vfio/common.c: In function 'vfio_device_dma_logging_report':
>>> 2614../hw/vfio/common.c:1810:22: error: cast from pointer to integer of
>>> different size [-Werror=pointer-to-int-cast]
>>> 2615 1810 |     report->bitmap = (uint64_t)bitmap;
>>> 2616      |                      ^
>> Sure, I will fix these errors.
> Just a thought: should the pre-copy series be moved towards the end of this
> series, given that it's more of an improvement of downtime than a must-have like
> dirty tracking?

Given recent discussion, maybe it would be better to split this series 
and go one step at a time:
Start with basic support for device dirty tracking (without vIOMMU 
support), then add pre-copy and then add vIOMMU support to device dirty 
tracking.

Thanks.

On 2/26/23 18:00, Avihai Horon wrote:
> 
> On 24/02/2023 21:26, Joao Martins wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 23/02/2023 14:56, Avihai Horon wrote:
>>> On 22/02/2023 22:55, Alex Williamson wrote:
>>>> There are various errors running this through the CI on gitlab.
>>>>
>>>> This one seems bogus but needs to be resolved regardless:
>>>>
>>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I..
>>>> -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader
>>>> -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0
>>>> -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall
>>>> -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
>>>> /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote .
>>>> -iquote /builds/alex.williamson/qemu -iquote
>>>> /builds/alex.williamson/qemu/include -iquote
>>>> /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE
>>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>>> -isystemlinux-headers -DNEED_CPU_H
>>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>>> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
>>>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this
>>>> function [-Werror=maybe-uninitialized]
>>>> 2789 1772 |     if (ret) {
>>>> 2790      |        ^
>>>>
>>>> 32-bit builds have some actual errors though:
>>>>
>>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
>>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>>> 2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm
>>>> -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1
>>>> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4
>>>> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
>>>> -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers
>>>> -iquote . -iquote /builds/alex.williamson/qemu -iquote
>>>> /builds/alex.williamson/qemu/include -iquote
>>>> /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
>>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
>>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security
>>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
>>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value
>>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>>> -isystemlinux-headers -DNEED_CPU_H
>>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>>> 2602../hw/vfio/common.c: In function
>>>> 'vfio_device_feature_dma_logging_start_create':
>>>> 2603../hw/vfio/common.c:1572:27: error: cast from pointer to integer of
>>>> different size [-Werror=pointer-to-int-cast]
>>>> 2604 1572 |         control->ranges = (uint64_t)ranges;
>>>> 2605      |                           ^
>>>> 2606../hw/vfio/common.c:1596:23: error: cast from pointer to integer of
>>>> different size [-Werror=pointer-to-int-cast]
>>>> 2607 1596 |     control->ranges = (uint64_t)ranges;
>>>> 2608      |                       ^
>>>> 2609../hw/vfio/common.c: In function
>>>> 'vfio_device_feature_dma_logging_start_destroy':
>>>> 2610../hw/vfio/common.c:1620:9: error: cast to pointer from integer of
>>>> different size [-Werror=int-to-pointer-cast]
>>>> 2611 1620 |         (struct vfio_device_feature_dma_logging_range
>>>> *)control->ranges;
>>>> 2612      |         ^
>>>> 2613../hw/vfio/common.c: In function 'vfio_device_dma_logging_report':
>>>> 2614../hw/vfio/common.c:1810:22: error: cast from pointer to integer of
>>>> different size [-Werror=pointer-to-int-cast]
>>>> 2615 1810 |     report->bitmap = (uint64_t)bitmap;
>>>> 2616      |                      ^
>>> Sure, I will fix these errors.
>> Just a thought: should the pre-copy series be moved towards the end of this
>> series, given that it's more of an improvement of downtime than a must-have like
>> dirty tracking?
> 
> Given recent discussion, maybe it would be better to split this series and go one step at a time:
> Start with basic support for device dirty tracking (without vIOMMU support), then add pre-copy and then add vIOMMU support to device dirty tracking.

and add the fixes first in the series. They could be merged quickly.

Thanks,

C.

On 27/02/2023 15:50, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 2/26/23 18:00, Avihai Horon wrote:
>>
>> On 24/02/2023 21:26, Joao Martins wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 23/02/2023 14:56, Avihai Horon wrote:
>>>> On 22/02/2023 22:55, Alex Williamson wrote:
>>>>> There are various errors running this through the CI on gitlab.
>>>>>
>>>>> This one seems bogus but needs to be resolved regardless:
>>>>>
>>>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>>>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>>>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I..
>>>>> -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader
>>>>> -I/usr/include/pixman-1 -I/usr/include/capstone 
>>>>> -I/usr/include/glib-2.0
>>>>> -I/usr/lib/s390x-linux-gnu/glib-2.0/include 
>>>>> -fdiagnostics-color=auto -Wall
>>>>> -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem
>>>>> /builds/alex.williamson/qemu/linux-headers -isystem linux-headers 
>>>>> -iquote .
>>>>> -iquote /builds/alex.williamson/qemu -iquote
>>>>> /builds/alex.williamson/qemu/include -iquote
>>>>> /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE
>>>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
>>>>> -D_LARGEFILE_SOURCE
>>>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits 
>>>>> -Wformat-security
>>>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body 
>>>>> -Wnested-externs
>>>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>>>> -Wmissing-format-attribute -Wno-missing-include-dirs 
>>>>> -Wno-shift-negative-value
>>>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>>>> -isystemlinux-headers -DNEED_CPU_H
>>>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c 
>>>>> ../hw/vfio/common.c
>>>>> 2787../hw/vfio/common.c: In function 
>>>>> ‘vfio_listener_log_global_start’:
>>>>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used 
>>>>> uninitialized in this
>>>>> function [-Werror=maybe-uninitialized]
>>>>> 2789 1772 |     if (ret) {
>>>>> 2790      |        ^
>>>>>
>>>>> 32-bit builds have some actual errors though:
>>>>>
>>>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940719
>>>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>>>> 2601cc -m32 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm
>>>>> -I../target/arm -Iqapi -Itrace -Iui -Iui/shader 
>>>>> -I/usr/include/pixman-1
>>>>> -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include 
>>>>> -I/usr/include/sysprof-4
>>>>> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 
>>>>> -O2 -g
>>>>> -isystem /builds/alex.williamson/qemu/linux-headers -isystem 
>>>>> linux-headers
>>>>> -iquote . -iquote /builds/alex.williamson/qemu -iquote
>>>>> /builds/alex.williamson/qemu/include -iquote
>>>>> /builds/alex.williamson/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
>>>>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
>>>>> -D_LARGEFILE_SOURCE
>>>>> -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings
>>>>> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls
>>>>> -Wold-style-declaration -Wold-style-definition -Wtype-limits 
>>>>> -Wformat-security
>>>>> -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body 
>>>>> -Wnested-externs
>>>>> -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
>>>>> -Wmissing-format-attribute -Wno-missing-include-dirs 
>>>>> -Wno-shift-negative-value
>>>>> -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers
>>>>> -isystemlinux-headers -DNEED_CPU_H
>>>>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"'
>>>>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o
>>>>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c 
>>>>> ../hw/vfio/common.c
>>>>> 2602../hw/vfio/common.c: In function
>>>>> 'vfio_device_feature_dma_logging_start_create':
>>>>> 2603../hw/vfio/common.c:1572:27: error: cast from pointer to 
>>>>> integer of
>>>>> different size [-Werror=pointer-to-int-cast]
>>>>> 2604 1572 |         control->ranges = (uint64_t)ranges;
>>>>> 2605      |                           ^
>>>>> 2606../hw/vfio/common.c:1596:23: error: cast from pointer to 
>>>>> integer of
>>>>> different size [-Werror=pointer-to-int-cast]
>>>>> 2607 1596 |     control->ranges = (uint64_t)ranges;
>>>>> 2608      |                       ^
>>>>> 2609../hw/vfio/common.c: In function
>>>>> 'vfio_device_feature_dma_logging_start_destroy':
>>>>> 2610../hw/vfio/common.c:1620:9: error: cast to pointer from 
>>>>> integer of
>>>>> different size [-Werror=int-to-pointer-cast]
>>>>> 2611 1620 |         (struct vfio_device_feature_dma_logging_range
>>>>> *)control->ranges;
>>>>> 2612      |         ^
>>>>> 2613../hw/vfio/common.c: In function 
>>>>> 'vfio_device_dma_logging_report':
>>>>> 2614../hw/vfio/common.c:1810:22: error: cast from pointer to 
>>>>> integer of
>>>>> different size [-Werror=pointer-to-int-cast]
>>>>> 2615 1810 |     report->bitmap = (uint64_t)bitmap;
>>>>> 2616      |                      ^
>>>> Sure, I will fix these errors.
>>> Just a thought: should the pre-copy series be moved towards the end 
>>> of this
>>> series, given that it's more of an improvement of downtime than a 
>>> must-have like
>>> dirty tracking?
>>
>> Given recent discussion, maybe it would be better to split this 
>> series and go one step at a time:
>> Start with basic support for device dirty tracking (without vIOMMU 
>> support), then add pre-copy and then add vIOMMU support to device 
>> dirty tracking.
>
> and add the fixes first in the series. They could be merged quickly.

Yes, of course. I will add them.

Thanks.

On 2/22/23 21:55, Alex Williamson wrote:
> 
> There are various errors running this through the CI on gitlab.
> 
> This one seems bogus but needs to be resolved regardless:
> 
> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 2789 1772 |     if (ret) {
> 2790      |        ^


The routine to fix is vfio_devices_start_dirty_page_tracking(). The compiler
is doing some inlining.

Thanks,
C.

On 23/02/2023 12:05, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
>
>
> On 2/22/23 21:55, Alex Williamson wrote:
>>
>> There are various errors running this through the CI on gitlab.
>>
>> This one seems bogus but needs to be resolved regardless:
>>
>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. 
>> -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader 
>> -I/usr/include/pixman-1 -I/usr/include/capstone 
>> -I/usr/include/glib-2.0 -I/usr/lib/s390x-linux-gnu/glib-2.0/include 
>> -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 
>> -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem 
>> linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote 
>> /builds/alex.williamson/qemu/include -iquote 
>> /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE 
>> -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
>> -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef 
>> -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes 
>> -Wredundant-decls -Wold-style-declaration -Wold-style-definition 
>> -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self 
>> -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels 
>> -Wexpansion-to-defined -Wimplicit-fallthrough=2 
>> -Wmissing-format-attribute -Wno-missing-include-dirs 
>> -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE 
>> -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H 
>> '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' 
>> '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ 
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF 
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o 
>> libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used 
>> uninitialized in this function [-Werror=maybe-uninitialized]
>> 2789 1772 |     if (ret) {
>> 2790      |        ^
>
>
> The routine to fix is vfio_devices_start_dirty_page_tracking(). The 
> compiler
> is doing some inlining.
>
I don't think I understand how inlining could cause it.
Could you elaborate on this?

I thought that the compiler just missed the initialization of ret 
because it happens in the if else statement, and that simply doing "int 
ret = 0;" would solve it.

Thanks.

On 2/23/23 16:07, Avihai Horon wrote:
> 
> On 23/02/2023 12:05, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2/22/23 21:55, Alex Williamson wrote:
>>>
>>> There are various errors running this through the CI on gitlab.
>>>
>>> This one seems bogus but needs to be resolved regardless:
>>>
>>> https://gitlab.com/alex.williamson/qemu/-/jobs/3817940731
>>> FAILED: libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o
>>> 2786s390x-linux-gnu-gcc -m64 -Ilibqemu-aarch64-softmmu.fa.p -I. -I.. -Itarget/arm -I../target/arm -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/s390x-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /builds/alex.williamson/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/alex.williamson/qemu -iquote /builds/alex.williamson/qemu/include -iquote /builds/alex.williamson/qemu/tcg/s390x -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
>>> -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="aarch64-softmmu-config-target.h"' '-DCONFIG_DEVICES="aarch64-softmmu-config-devices.h"' -MD -MQ libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -MF libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o.d -o libqemu-aarch64-softmmu.fa.p/hw_vfio_common.c.o -c ../hw/vfio/common.c
>>> 2787../hw/vfio/common.c: In function ‘vfio_listener_log_global_start’:
>>> 2788../hw/vfio/common.c:1772:8: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>> 2789 1772 |     if (ret) {
>>> 2790      |        ^
>>
>>
>> The routine to fix is vfio_devices_start_dirty_page_tracking(). The compiler
>> is doing some inlining.
>>
> I don't think I understand how inlining could cause it.
> Could you elaborate on this?

The compiler reports an error in routine 'vfio_listener_log_global_start'
but the fix should be in 'vfio_devices_start_dirty_page_tracking'. Surely
because the compiler optimization inlines the latter.

> 
> I thought that the compiler just missed the initialization of ret because it happens in the if else statement, and that simply doing "int ret = 0;" would solve it.

Yes. This will work.

Thanks,

C.

On 22/02/2023 19:48, Avihai Horon wrote:
> Changes from v1 [4]:
> - Rebased on latest master branch. As part of it, made some changes in
>    pre-copy to adjust it to Juan's new patches:
>    1. Added a new patch that passes threshold_size parameter to
>       .state_pending_{estimate,exact}() handlers.
>    2. Added a new patch that refactors vfio_save_block().
>    3. Changed the pre-copy patch to cache and report pending pre-copy
>       size in the .state_pending_estimate() handler.
> - Removed unnecessary P2P code. This should be added later on when P2P
>    support is added. (Alex)
> - Moved the dirty sync to be after the DMA unmap in vfio_dma_unmap()
>    (patch #11). (Alex)
> - Stored vfio_devices_all_device_dirty_tracking()'s value in a local
>    variable in vfio_get_dirty_bitmap() so it can be re-used (patch #11).
> - Refactored the viommu device dirty tracking ranges creation code to
>    make it clearer (patch #15).
> - Changed overflow check in vfio_iommu_range_is_device_tracked() to
>    emphasize that we specifically check for 2^64 wrap around (patch #15).
> - Added R-bs / Acks.
>
> Thanks.
>
> [1]
> https://lore.kernel.org/qemu-devel/167658846945.932837.1420176491103357684.stgit@omen/
>
> [2]
> https://lore.kernel.org/kvm/20221206083438.37807-3-yishaih@nvidia.com/
>
> [3]
> https://lore.kernel.org/netdev/20220908183448.195262-4-yishaih@nvidia.com/

and here is v1 link:
[4]
https://lore.kernel.org/qemu-devel/20230126184948.10478-1-avihaih@nvidia.com/

Thanks.