>-----Original Message----- >From: Joao Martins <joao.m.mart...@oracle.com> >Sent: Tuesday, June 27, 2023 6:57 PM >To: Duan, Zhenzhong <zhenzhong.d...@intel.com>; Avihai Horon ><avih...@nvidia.com> >Cc: alex.william...@redhat.com; c...@redhat.com; Peng, Chao P ><chao.p.p...@intel.com>; qemu-devel@nongnu.org >Subject: Re: [PATCH v3 3/3] vfio/migration: vfio/migration: Refactor and fix >print of "Migration disabled" > >On 27/06/2023 03:55, Duan, Zhenzhong wrote: >>> I guess it makes sense -- the thing that was tieing him was the >>> global migration blocker, which is now consolidated into the main >migration blocker. >>> >>> My vIOMMU series will relax this condition yes (still same per-device >>> scope). >>> And I will need to check the maximum IOVA in the vIOMMU. But it's new >>> code I can adjust and Zhenzhong shouldn't worry about the vIOMMU >>> future stuff >> A bit confused, you agreed to move vfio_block_giommu_migration into >> migration.c >> >>> but I don't expect to influence location, say if it gets moved into >>> migration.c >> and final decision is no move for influencing location reason? Do I >misunderstand? > >Sorry for the confusion. > >The thing is that I will need 'similar code' to test if a vIOMMU is enabled or >not. >The reason being that dirty tracking will need this to understand what to track >meaning to decide whether we track the whole address space or just the >linear map[0]. And all that code is in common, not migration.c and where I >use it will have to look at all address spaces (because dirty tracking is >started >for all devices, so there's no vbasedev to look at). > >Eventually after the vIOMMU stuff, the migration blocker condition will look >more or less like this: > > return (!vfio_viommu_preset(vbasedev) || > (vfio_viommu_preset(vbasedev) && > !vfio_viommu_get_max_iova(&max))) > >Whereby vfio_viommu_preset(vbasedev) is what you currently call >vfio_block_giommu_migration(vbasedev) in current patch. So perhaps I would >say to leave it in common.c and rename it to vfio_viommu_preset(vbasedev) >with a comment on top for /* Block migration with a vIOMMU */ > >Then when the time comes I can introduce in my vIOMMU series a >vfio_viommu_possible() [or some other name] when starting dirty tracking >which looks all VFIOAddressSpaces and reuses your >vfio_viommu_preset(vbasedev). This should cover current and future needs, >without going to great extents beyond the purpose of this patch?
Thanks for detailed explanation, clear, I'll update based on above suggestions. Zhenzhong
