| Message ID | 20241216094638.26406-1-avihaih@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | migration: Drop/unexport migration_is_device() and migration_is_active() | expand |
On 16/12/2024 09:46, Avihai Horon wrote: > Hello, > > This follows up on Peter's series [1] to simplify migration status API > to a single migration_is_running() function. > > Peter's series tried to drop migration_is_device() and > migration_is_active(), however VFIO used them to check if dirty page > tracking has been started in order to avoid errors in log sync, so they > couldn't simply be dropped without some preliminary cleanups. > > This series handles these preliminary cleanups and eventually drops > migration_is_device() and unexports migration_is_active(). > > The series has been migration tested with the following: > - VFIO device dirty tracking. > - Legacy VFIO iommu dirty tracking. > - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is > currently blocked, so I used a patched QEMU to allow it). > vIOMMU on IOMMU HW doesn't suffer from the same problems of VF dirty tracking where there's a aggregate limit into how much VFs can track in terms of IOVA space. So we can lift some of those restrictions for IOMMU even right now provided we implement the last remaining pre-requisite. I also have a much smaller series for that sort of unblockage that I can give you a pointer. Though, eventually the optimizations we will do for VF dirty tracking for vIOMMU will apply to IOMMU HW too just so we minimize the amount of calls to get dirty bits. > I didn't test it with iommu DPT as I don't have access to such HW. > Cedric, I remember you said that you have such HW, it would be very > helpful if you could test it. > I am starting to prep the unblocking vIOMMU for Qemu 10, so I can validate if this series works as well -- but from what I have looked so far it should be all OK. If it helps I have some pending series that lets you test emulated x86 IOMMU DPT support (either on intel-iommu or amd-iommu) that can help you when you don't have the hardware to test. Regarding this series, since you are looking at the the dirty tracking 'status' I'll comment here too as one of your original patches introduced it as it's related to the use of migration_is_running(). And since you're looking at this exact part of the code, might as well cover that too.
On 16/12/2024 14:00, Joao Martins wrote: > External email: Use caution opening links or attachments > > > On 16/12/2024 09:46, Avihai Horon wrote: >> Hello, >> >> This follows up on Peter's series [1] to simplify migration status API >> to a single migration_is_running() function. >> >> Peter's series tried to drop migration_is_device() and >> migration_is_active(), however VFIO used them to check if dirty page >> tracking has been started in order to avoid errors in log sync, so they >> couldn't simply be dropped without some preliminary cleanups. >> >> This series handles these preliminary cleanups and eventually drops >> migration_is_device() and unexports migration_is_active(). >> >> The series has been migration tested with the following: >> - VFIO device dirty tracking. >> - Legacy VFIO iommu dirty tracking. >> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >> currently blocked, so I used a patched QEMU to allow it). >> > vIOMMU on IOMMU HW doesn't suffer from the same problems of VF dirty tracking > where there's a aggregate limit into how much VFs can track in terms of IOVA > space. So we can lift some of those restrictions for IOMMU even right now > provided we implement the last remaining pre-requisite. That would be helpful if there is a user who needs migration (with iommu DPT) + vIOMMU support right now. Otherwise, we don't have to rush and we can do it along with the optimizations or whatever we see fit. I only needed migration + vIOMMU to test the vIOMMU dma unmap flow. > I also have a much > smaller series for that sort of unblockage that I can give you a pointer. Yes, if you have it at hand, that could be useful for testing next versions. > Though, eventually the optimizations we will do for VF dirty tracking for vIOMMU > will apply to IOMMU HW too just so we minimize the amount of calls to get dirty > bits. > >> I didn't test it with iommu DPT as I don't have access to such HW. >> Cedric, I remember you said that you have such HW, it would be very >> helpful if you could test it. >> > I am starting to prep the unblocking vIOMMU for Qemu 10, so I can validate if > this series works as well -- but from what I have looked so far it should be all > OK. Thanks, that wouldn't hurt :) > If it helps I have some pending series that lets you test emulated x86 IOMMU > DPT support (either on intel-iommu or amd-iommu) that can help you when you > don't have the hardware to test. That would be great, I didn't know such thing existed. Thanks! > > Regarding this series, since you are looking at the the dirty tracking 'status' > I'll comment here too as one of your original patches introduced it as it's > related to the use of migration_is_running(). And since you're looking at this > exact part of the code, might as well cover that too.
On 16/12/2024 14:45, Avihai Horon wrote: > On 16/12/2024 14:00, Joao Martins wrote: >> On 16/12/2024 09:46, Avihai Horon wrote: >> I also have a much >> smaller series for that sort of unblockage that I can give you a pointer. > > Yes, if you have it at hand, that could be useful for testing next versions. > >> Though, eventually the optimizations we will do for VF dirty tracking for vIOMMU >> will apply to IOMMU HW too just so we minimize the amount of calls to get dirty >> bits. >> >>> I didn't test it with iommu DPT as I don't have access to such HW. >>> Cedric, I remember you said that you have such HW, it would be very >>> helpful if you could test it. >>> >> I am starting to prep the unblocking vIOMMU for Qemu 10, so I can validate if >> this series works as well -- but from what I have looked so far it should be all >> OK. > > Thanks, that wouldn't hurt :) > >> If it helps I have some pending series that lets you test emulated x86 IOMMU >> DPT support (either on intel-iommu or amd-iommu) that can help you when you >> don't have the hardware to test. > > That would be great, I didn't know such thing existed. > I did post it, but it was some time ago (2y). While I got some comments, but failed to follow-up: https://lore.kernel.org/qemu-devel/20220428211351.3897-1-joao.m.martins@oracle.com/#t The link above has bugs, but let me conjure the version alongside vIOMMU and the other stuff I mentioned earlier.
Hello Joao, On 12/16/24 16:37, Joao Martins wrote: > On 16/12/2024 14:45, Avihai Horon wrote: >> On 16/12/2024 14:00, Joao Martins wrote: >>> On 16/12/2024 09:46, Avihai Horon wrote: >>> I also have a much >>> smaller series for that sort of unblockage that I can give you a pointer. >> >> Yes, if you have it at hand, that could be useful for testing next versions. >> >>> Though, eventually the optimizations we will do for VF dirty tracking for vIOMMU >>> will apply to IOMMU HW too just so we minimize the amount of calls to get dirty >>> bits. >>> >>>> I didn't test it with iommu DPT as I don't have access to such HW. >>>> Cedric, I remember you said that you have such HW, it would be very >>>> helpful if you could test it. >>>> >>> I am starting to prep the unblocking vIOMMU for Qemu 10, so I can validate if >>> this series works as well -- but from what I have looked so far it should be all >>> OK. >> >> Thanks, that wouldn't hurt :) >> >>> If it helps I have some pending series that lets you test emulated x86 IOMMU >>> DPT support (either on intel-iommu or amd-iommu) that can help you when you >>> don't have the hardware to test. >> >> That would be great, I didn't know such thing existed. >> > > I did post it, but it was some time ago (2y). While I got some comments, but > failed to follow-up: > > https://lore.kernel.org/qemu-devel/20220428211351.3897-1-joao.m.martins@oracle.com/#t > > The link above has bugs, but let me conjure the version alongside vIOMMU and the > other stuff I mentioned earlier. Please Cc: me next time. Thanks, C.
On 16/12/2024 17:33, Cédric Le Goater wrote: > On 12/16/24 16:37, Joao Martins wrote: >> On 16/12/2024 14:45, Avihai Horon wrote: >>> On 16/12/2024 14:00, Joao Martins wrote: >>>> On 16/12/2024 09:46, Avihai Horon wrote: >>>> I also have a much >>>> smaller series for that sort of unblockage that I can give you a pointer. >>> >>> Yes, if you have it at hand, that could be useful for testing next versions. >>> >>>> Though, eventually the optimizations we will do for VF dirty tracking for >>>> vIOMMU >>>> will apply to IOMMU HW too just so we minimize the amount of calls to get dirty >>>> bits. >>>> >>>>> I didn't test it with iommu DPT as I don't have access to such HW. >>>>> Cedric, I remember you said that you have such HW, it would be very >>>>> helpful if you could test it. >>>>> >>>> I am starting to prep the unblocking vIOMMU for Qemu 10, so I can validate if >>>> this series works as well -- but from what I have looked so far it should be >>>> all >>>> OK. >>> >>> Thanks, that wouldn't hurt :) >>> >>>> If it helps I have some pending series that lets you test emulated x86 IOMMU >>>> DPT support (either on intel-iommu or amd-iommu) that can help you when you >>>> don't have the hardware to test. >>> >>> That would be great, I didn't know such thing existed. >>> >> >> I did post it, but it was some time ago (2y). While I got some comments, but >> failed to follow-up: >> >> https://lore.kernel.org/qemu-devel/20220428211351.3897-1- >> joao.m.martins@oracle.com/#t >> >> The link above has bugs, but let me conjure the version alongside vIOMMU and the >> other stuff I mentioned earlier. > > Please Cc: me next time. > OK
On 12/16/24 10:46, Avihai Horon wrote: > Hello, > > This follows up on Peter's series [1] to simplify migration status API > to a single migration_is_running() function. > > Peter's series tried to drop migration_is_device() and > migration_is_active(), however VFIO used them to check if dirty page > tracking has been started in order to avoid errors in log sync, so they > couldn't simply be dropped without some preliminary cleanups. > > This series handles these preliminary cleanups and eventually drops > migration_is_device() and unexports migration_is_active(). > > The series has been migration tested with the following: > - VFIO device dirty tracking. > - Legacy VFIO iommu dirty tracking. > - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is > currently blocked, so I used a patched QEMU to allow it). > > I didn't test it with iommu DPT as I don't have access to such HW. > Cedric, I remember you said that you have such HW, it would be very > helpful if you could test it. Yes. I can. I am bit busy now and I only looked quickly at the comments. I understand from Joao that it shouldn't be too complex to change the VFIO dirty tracking engines to use calc-dirty-rate, which I agree would useful. How far are we from that ? We are early in the QEMU cycle and this series looks ok, may be needs one respin. Should we merge it first ? I feel this would be good to have before the migration cleanups and the next version of Maciej's multifd series. Thanks, C. > > Patch structure: > 1-6: Refactor and clean up VFIO dirty page tracking helpers. > 7: Refactor dirty limit code. > 8-9: Drop/unexport migration_is_device() and migration_is_running(). > > Thanks. > > [1] > https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/ > > Avihai Horon (9): > vfio/container: Add dirty tracking started flag > vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic > vfio/migration: Refactor vfio_devices_all_running_and_mig_active() > logic > vfio/migration: Add vfio_devices_all_dirty_tracking_started() helper > vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed() > vfio/migration: Rename vfio_devices_all_dirty_tracking() > system/dirtylimit: Don't use migration_is_active() > migration: Drop migration_is_device() > migration: Unexport migration_is_active() > > include/hw/vfio/vfio-common.h | 4 +- > include/hw/vfio/vfio-container-base.h | 1 + > include/migration/misc.h | 2 - > hw/vfio/common.c | 61 ++++++++++++--------------- > hw/vfio/container-base.c | 8 +++- > hw/vfio/container.c | 2 +- > migration/migration.c | 23 ++++------ > system/dirtylimit.c | 3 +- > 8 files changed, 48 insertions(+), 56 deletions(-) >
On 16/12/2024 23:43, Cédric Le Goater wrote: > External email: Use caution opening links or attachments > > > On 12/16/24 10:46, Avihai Horon wrote: >> Hello, >> >> This follows up on Peter's series [1] to simplify migration status API >> to a single migration_is_running() function. >> >> Peter's series tried to drop migration_is_device() and >> migration_is_active(), however VFIO used them to check if dirty page >> tracking has been started in order to avoid errors in log sync, so they >> couldn't simply be dropped without some preliminary cleanups. >> >> This series handles these preliminary cleanups and eventually drops >> migration_is_device() and unexports migration_is_active(). >> >> The series has been migration tested with the following: >> - VFIO device dirty tracking. >> - Legacy VFIO iommu dirty tracking. >> - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is >> currently blocked, so I used a patched QEMU to allow it). >> >> I didn't test it with iommu DPT as I don't have access to such HW. >> Cedric, I remember you said that you have such HW, it would be very >> helpful if you could test it. > > Yes. I can. Thanks. > > I am bit busy now and I only looked quickly at the comments. I understand > from Joao that it shouldn't be too complex to change the VFIO dirty > tracking engines to use calc-dirty-rate, which I agree would useful. Sure, will do. > > How far are we from that ? Not far, I hope I can send v2 later this week. > We are early in the QEMU cycle and this series > looks ok, may be needs one respin. Should we merge it first ? I feel this > would be good to have before the migration cleanups and the next version > of Maciej's multifd series. > Agreed. Thanks.
Hello, This follows up on Peter's series [1] to simplify migration status API to a single migration_is_running() function. Peter's series tried to drop migration_is_device() and migration_is_active(), however VFIO used them to check if dirty page tracking has been started in order to avoid errors in log sync, so they couldn't simply be dropped without some preliminary cleanups. This series handles these preliminary cleanups and eventually drops migration_is_device() and unexports migration_is_active(). The series has been migration tested with the following: - VFIO device dirty tracking. - Legacy VFIO iommu dirty tracking. - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is currently blocked, so I used a patched QEMU to allow it). I didn't test it with iommu DPT as I don't have access to such HW. Cedric, I remember you said that you have such HW, it would be very helpful if you could test it. Patch structure: 1-6: Refactor and clean up VFIO dirty page tracking helpers. 7: Refactor dirty limit code. 8-9: Drop/unexport migration_is_device() and migration_is_running(). Thanks. [1] https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/ Avihai Horon (9): vfio/container: Add dirty tracking started flag vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic vfio/migration: Add vfio_devices_all_dirty_tracking_started() helper vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed() vfio/migration: Rename vfio_devices_all_dirty_tracking() system/dirtylimit: Don't use migration_is_active() migration: Drop migration_is_device() migration: Unexport migration_is_active() include/hw/vfio/vfio-common.h | 4 +- include/hw/vfio/vfio-container-base.h | 1 + include/migration/misc.h | 2 - hw/vfio/common.c | 61 ++++++++++++--------------- hw/vfio/container-base.c | 8 +++- hw/vfio/container.c | 2 +- migration/migration.c | 23 ++++------ system/dirtylimit.c | 3 +- 8 files changed, 48 insertions(+), 56 deletions(-)