...

...

...

Currently, yes, i40e has build dependency on vfio-pci. It's like this, if i40e decides to support SRIOV and compiles in vf related code who depends on vfio-pci, it will also have build dependency on vfio-pci. isn't it natural?

No, this is not natural. There are certainly i40e VF use cases that have no interest in vfio and having dependencies between the two modules is unacceptable. I think you probably want to modularize the i40e vfio support code and then perhaps register a table in vfio-pci that the vfio-pci code can perform a module request when using a compatible device. Just and idea, there might be better options. I will not accept a solution that requires unloading the i40e driver in order to unload the vfio-pci driver. It's inconvenient with just one NIC driver, imagine how poorly that scales.

what about this way: mediate driver registers a module notifier and every time when vfio_pci is loaded, register to vfio_pci its mediate ops? (Just like in below sample code) This way vfio-pci is free to unload and this registering only gives vfio-pci a name of what module to request. After that, in vfio_pci_open(), vfio-pci requests the mediate driver. (or puts the mediate driver when mediate driver does not support mediating the device) in vfio_pci_release(), vfio-pci puts the mediate driver.

static void register_mediate_ops(void) { int (*func)(struct vfio_pci_mediate_ops *ops) = NULL;

func = symbol_get(vfio_pci_register_mediate_ops);

if (func) { func(&igd_dt_ops); symbol_put(vfio_pci_register_mediate_ops); } }

static int igd_module_notify(struct notifier_block *self, unsigned long val, void *data) { struct module *mod = data; int ret = 0;

switch (val) { case MODULE_STATE_LIVE: if (!strcmp(mod->name, "vfio_pci")) register_mediate_ops(); break; case MODULE_STATE_GOING: break; default: break; } return ret; }

static struct notifier_block igd_module_nb = { .notifier_call = igd_module_notify, .priority = 0, };

static int __init igd_dt_init(void) { ... register_mediate_ops(); register_module_notifier(&igd_module_nb); ... return 0; }

No, this is bad. Please look at MODULE_ALIAS() and request_module() as used in the vfio-platform for loading reset driver modules. I think the correct approach is that vfio-pci should perform a request_module() based on the device being probed. Having the mediation provider listening for vfio-pci and registering itself regardless of whether we intend to use it assumes that we will want to use it and assumes that the mediation provider module is already loaded. We should be able to support demand loading of modules that may serve no other purpose than providing this mediation. Thanks, hi Alex Thanks for this message. So is it good to create a separate module as mediation provider driver, and alias its module name to "vfio-pci-mediate-vid-did". Then when vfio-pci probes the device, it requests module of that name ?

Thanks Yan

On Wed, Dec 11, 2019 at 12:58:24AM +0800, Alex Williamson wrote:

On Mon, 9 Dec 2019 21:44:23 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

...

...

...

...

Currently, yes, i40e has build dependency on vfio-pci. It's like this, if i40e decides to support SRIOV and compiles in vf related code who depends on vfio-pci, it will also have build dependency on vfio-pci. isn't it natural?

No, this is not natural. There are certainly i40e VF use cases that have no interest in vfio and having dependencies between the two modules is unacceptable. I think you probably want to modularize the i40e vfio support code and then perhaps register a table in vfio-pci that the vfio-pci code can perform a module request when using a compatible device. Just and idea, there might be better options. I will not accept a solution that requires unloading the i40e driver in order to unload the vfio-pci driver. It's inconvenient with just one NIC driver, imagine how poorly that scales.

what about this way: mediate driver registers a module notifier and every time when vfio_pci is loaded, register to vfio_pci its mediate ops? (Just like in below sample code) This way vfio-pci is free to unload and this registering only gives vfio-pci a name of what module to request. After that, in vfio_pci_open(), vfio-pci requests the mediate driver. (or puts the mediate driver when mediate driver does not support mediating the device) in vfio_pci_release(), vfio-pci puts the mediate driver.

static void register_mediate_ops(void) { int (*func)(struct vfio_pci_mediate_ops *ops) = NULL;

func = symbol_get(vfio_pci_register_mediate_ops);

if (func) { func(&igd_dt_ops); symbol_put(vfio_pci_register_mediate_ops); } }

static int igd_module_notify(struct notifier_block *self, unsigned long val, void *data) { struct module *mod = data; int ret = 0;

switch (val) { case MODULE_STATE_LIVE: if (!strcmp(mod->name, "vfio_pci")) register_mediate_ops(); break; case MODULE_STATE_GOING: break; default: break; } return ret; }

static struct notifier_block igd_module_nb = { .notifier_call = igd_module_notify, .priority = 0, };

static int __init igd_dt_init(void) { ... register_mediate_ops(); register_module_notifier(&igd_module_nb); ... return 0; }

No, this is bad. Please look at MODULE_ALIAS() and request_module() as used in the vfio-platform for loading reset driver modules. I think the correct approach is that vfio-pci should perform a request_module() based on the device being probed. Having the mediation provider listening for vfio-pci and registering itself regardless of whether we intend to use it assumes that we will want to use it and assumes that the mediation provider module is already loaded. We should be able to support demand loading of modules that may serve no other purpose than providing this mediation. Thanks, hi Alex Thanks for this message. So is it good to create a separate module as mediation provider driver, and alias its module name to "vfio-pci-mediate-vid-did". Then when vfio-pci probes the device, it requests module of that name ?

I think this would give us an option to have the mediator as a separate module, but not require it. Maybe rather than a request_module(), where if we follow the platform reset example we'd then expect the init code for the module to register into a list, we could do a symbol_request(). AIUI, this would give us a reference to the symbol if the module providing it is already loaded, and request a module (perhaps via an alias) if it's not already load. Thanks,

ok. got it! Thank you :) Yan

Sorry about that. I'll pay attention to them next time and thank you for pointing them out :) On Sat, Dec 07, 2019 at 07:13:30AM +0800, Eric Blake wrote:

On 12/4/19 9:25 PM, Yan Zhao wrote:

...

when vfio-pci is bound to a physical device, almost all the hardware resources are passthroughed.

The intent is obvious, but it sounds awkward to a native speaker. s/passthroughed/passed through/

...

Sometimes, vendor driver of this physcial device may want to mediate some

physical

...

hardware resource access for a short period of time, e.g. dirty page tracking during live migration.

Here we introduce mediate ops in vfio-pci for this purpose.

Vendor driver can register a mediate ops to vfio-pci. But rather than directly bind to the passthroughed device, the

passed-through

-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On Fri, 6 Dec 2019 01:04:07 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote:

...

On Wed, 4 Dec 2019 22:26:50 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

Dynamic trap bar info region is a channel for QEMU and vendor driver to communicate dynamic trap info. It is of type VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO.

This region has two fields: dt_fd and trap. When QEMU detects a device regions of this type, it will create an eventfd and write its eventfd id to dt_fd field. When vendor drivre signals this eventfd, QEMU reads trap field of this info region. - If trap is true, QEMU would search the device's PCI BAR regions and disable all the sparse mmaped subregions (if the sparse mmaped subregion is disablable). - If trap is false, QEMU would re-enable those subregions.

A typical usage is 1. vendor driver first cuts its bar 0 into several sections, all in a sparse mmap array. So initally, all its bar 0 are passthroughed. 2. vendor driver specifys part of bar 0 sections to be disablable. 3. on migration starts, vendor driver signals dt_fd and set trap to true to notify QEMU disabling the bar 0 sections of disablable flags on. 4. QEMU disables those bar 0 section and hence let vendor driver be able to trap access of bar 0 registers and make dirty page tracking possible. 5. on migration failure, vendor driver signals dt_fd to QEMU again. QEMU reads trap field of this info region which is false and QEMU re-passthrough the whole bar 0 region.

Vendor driver specifies whether it supports dynamic-trap-bar-info region through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in vfio_pci_mediate_ops->open().

If vfio-pci detects this cap, it will create a default dynamic_trap_bar_info region on behalf of vendor driver with region len=0 and region->ops=null. Vvendor driver should override this region's len, flags, rw, mmap in its vfio_pci_mediate_ops.

TBH, I don't like this interface at all. Userspace doesn't pass data to the kernel via INFO ioctls. We have a SET_IRQS ioctl for configuring user signaling with eventfds. I think we only need to define an IRQ type that tells the user to re-evaluate the sparse mmap information for a region. The user would enumerate the device IRQs via GET_IRQ_INFO, find one of this type where the IRQ info would also indicate which region(s) should be re-evaluated on signaling. The user would enable that signaling via SET_IRQS and simply re-evaluate the ok. I'll try to switch to this way. Thanks for this suggestion.

...

sparse mmap capability for the associated regions when signaled.

Do you like the "disablable" flag of sparse mmap ? I think it's a lightweight way for user to switch mmap state of a whole region, otherwise going through a complete flow of GET_REGION_INFO and re-setup region might be too heavy.

No, I don't like the disable-able flag. At what frequency do we expect regions to change? It seems like we'd only change when switching into and out of the _SAVING state, which is rare. It seems easy for userspace, at least QEMU, to drop the entire mmap configuration and re-read it. Another concern here is how do we synchronize the event? Are we assuming that this event would occur when a user switch to _SAVING mode on the device? That operation is synchronous, the device must be in saving mode after the write to device state completes, but it seems like this might be trying to add an asynchronous dependency. Will the write to device_state only complete once the user handles the eventfd? How would the kernel know when the mmap re-evaluation is complete. It seems like there are gaps here that the vendor driver could miss traps required for migration because the user hasn't completed the mmap transition yet. Thanks, Alex

On Mon, 9 Dec 2019 01:22:12 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote:

...

On Fri, 6 Dec 2019 01:04:07 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote:

...

On Wed, 4 Dec 2019 22:26:50 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

Dynamic trap bar info region is a channel for QEMU and vendor driver to communicate dynamic trap info. It is of type VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO.

This region has two fields: dt_fd and trap. When QEMU detects a device regions of this type, it will create an eventfd and write its eventfd id to dt_fd field. When vendor drivre signals this eventfd, QEMU reads trap field of this info region. - If trap is true, QEMU would search the device's PCI BAR regions and disable all the sparse mmaped subregions (if the sparse mmaped subregion is disablable). - If trap is false, QEMU would re-enable those subregions.

A typical usage is 1. vendor driver first cuts its bar 0 into several sections, all in a sparse mmap array. So initally, all its bar 0 are passthroughed. 2. vendor driver specifys part of bar 0 sections to be disablable. 3. on migration starts, vendor driver signals dt_fd and set trap to true to notify QEMU disabling the bar 0 sections of disablable flags on. 4. QEMU disables those bar 0 section and hence let vendor driver be able to trap access of bar 0 registers and make dirty page tracking possible. 5. on migration failure, vendor driver signals dt_fd to QEMU again. QEMU reads trap field of this info region which is false and QEMU re-passthrough the whole bar 0 region.

Vendor driver specifies whether it supports dynamic-trap-bar-info region through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in vfio_pci_mediate_ops->open().

If vfio-pci detects this cap, it will create a default dynamic_trap_bar_info region on behalf of vendor driver with region len=0 and region->ops=null. Vvendor driver should override this region's len, flags, rw, mmap in its vfio_pci_mediate_ops.

TBH, I don't like this interface at all. Userspace doesn't pass data to the kernel via INFO ioctls. We have a SET_IRQS ioctl for configuring user signaling with eventfds. I think we only need to define an IRQ type that tells the user to re-evaluate the sparse mmap information for a region. The user would enumerate the device IRQs via GET_IRQ_INFO, find one of this type where the IRQ info would also indicate which region(s) should be re-evaluated on signaling. The user would enable that signaling via SET_IRQS and simply re-evaluate the ok. I'll try to switch to this way. Thanks for this suggestion.

...

sparse mmap capability for the associated regions when signaled.

Do you like the "disablable" flag of sparse mmap ? I think it's a lightweight way for user to switch mmap state of a whole region, otherwise going through a complete flow of GET_REGION_INFO and re-setup region might be too heavy.

No, I don't like the disable-able flag. At what frequency do we expect regions to change? It seems like we'd only change when switching into and out of the _SAVING state, which is rare. It seems easy for userspace, at least QEMU, to drop the entire mmap configuration and ok. I'll try this way.

...

re-read it. Another concern here is how do we synchronize the event? Are we assuming that this event would occur when a user switch to _SAVING mode on the device? That operation is synchronous, the device must be in saving mode after the write to device state completes, but it seems like this might be trying to add an asynchronous dependency. Will the write to device_state only complete once the user handles the eventfd? How would the kernel know when the mmap re-evaluation is complete. It seems like there are gaps here that the vendor driver could miss traps required for migration because the user hasn't completed the mmap transition yet. Thanks,

Alex

yes, this asynchronous event notification will cause vendor driver miss traps. But it's supposed to be of very short period time. That's also a reason for us to wish the re-evaluation to be lightweight. E.g. if it's able to be finished before the first iterate, it's still safe.

Making the re-evaluation lightweight cannot solve the race, it only masks it.

But I agree, the timing is not guaranteed, and so it's best for kernel to wait for mmap re-evaluation to complete.

migration_thread |->qemu_savevm_state_setup | |->ram_save_setup | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | | | |->vfio_save_setup | |->set_device_state(_SAVING) | |->qemu_savevm_state_pending | |->ram_save_pending | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | |->vfio_save_pending | |->qemu_savevm_state_iterate | |->ram_save_iterate //send pages | |->vfio_save_iterate ...

Actually, we previously let qemu trigger the re-evaluation when migration starts. And now the reason for we to wish kernel to trigger the mmap re-evaluation is that there're other two possible use cases: (1) keep passing through devices when migration starts and track dirty pages using hardware IOMMU. Then when migration is about to complete, stop the device and start trap PCI BARs for software emulation. (we made some changes to let device stop ahead of vcpu )

How is that possible? I/O devices need to continue to work until the vCPU stops otherwise the vCPU can get blocked on the device. Maybe QEMU should assume all mmaps should be dropped on vfio device after we pass some point of the migration process. If there are a fixed set of mmap settings for a region and discrete conditions under which they become active (ex. switch device to SAVING mode) then QEMU could choose the right mapping itself and we wouldn't need to worry about this asynchronous signaling problem, it would just be defined as part of the protocol userspace needs to use.

(2) performance optimization. There's an example in GVT (mdev case): PCI BARs are passed through on vGPU initialization and are mmaped to a host dummy buffer. Then after initialization done, start trap of PCI BARs of vGPUs and start normal host mediation. The initial pass-through can save 1000000 times of mmio trap.

Much of this discussion has me worried that many assumptions are being made about the user and device interaction. Backwards compatible behavior is required. If a mdev device presents an initial sparse mmap capability for this acceleration, how do you support an existing userspace that doesn't understand the new dynamic mmap semantics and continues to try to operate with the initial sparse mmap? Doesn't this introduce another example of the raciness of the device trying to switch mmaps? Seems that if QEMU doesn't handle the eventfd with sufficient timeliness the switch back to trap behavior could miss an important transaction. This also seems like an optimization targeted at VMs running for only a short time, where it's not obvious to me that GVT-g overlaps those sorts of use cases. How much initialization time is actually being saved with such a hack? Thanks, Alex

On Tue, 10 Dec 2019 02:44:44 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Mon, 9 Dec 2019 01:22:12 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote:

...

On Fri, 6 Dec 2019 01:04:07 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote:

...

On Wed, 4 Dec 2019 22:26:50 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

> Dynamic trap bar info region is a channel for QEMU and vendor driver to > communicate dynamic trap info. It is of type > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > This region has two fields: dt_fd and trap. > When QEMU detects a device regions of this type, it will create an > eventfd and write its eventfd id to dt_fd field. > When vendor drivre signals this eventfd, QEMU reads trap field of this > info region. > - If trap is true, QEMU would search the device's PCI BAR > regions and disable all the sparse mmaped subregions (if the sparse > mmaped subregion is disablable). > - If trap is false, QEMU would re-enable those subregions. > > A typical usage is > 1. vendor driver first cuts its bar 0 into several sections, all in a > sparse mmap array. So initally, all its bar 0 are passthroughed. > 2. vendor driver specifys part of bar 0 sections to be disablable. > 3. on migration starts, vendor driver signals dt_fd and set trap to true > to notify QEMU disabling the bar 0 sections of disablable flags on. > 4. QEMU disables those bar 0 section and hence let vendor driver be able > to trap access of bar 0 registers and make dirty page tracking possible. > 5. on migration failure, vendor driver signals dt_fd to QEMU again. > QEMU reads trap field of this info region which is false and QEMU > re-passthrough the whole bar 0 region. > > Vendor driver specifies whether it supports dynamic-trap-bar-info region > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > vfio_pci_mediate_ops->open(). > > If vfio-pci detects this cap, it will create a default > dynamic_trap_bar_info region on behalf of vendor driver with region len=0 > and region->ops=null. > Vvendor driver should override this region's len, flags, rw, mmap in its > vfio_pci_mediate_ops.

TBH, I don't like this interface at all. Userspace doesn't pass data to the kernel via INFO ioctls. We have a SET_IRQS ioctl for configuring user signaling with eventfds. I think we only need to define an IRQ type that tells the user to re-evaluate the sparse mmap information for a region. The user would enumerate the device IRQs via GET_IRQ_INFO, find one of this type where the IRQ info would also indicate which region(s) should be re-evaluated on signaling. The user would enable that signaling via SET_IRQS and simply re-evaluate the ok. I'll try to switch to this way. Thanks for this suggestion.

...

sparse mmap capability for the associated regions when signaled.

Do you like the "disablable" flag of sparse mmap ? I think it's a lightweight way for user to switch mmap state of a whole region, otherwise going through a complete flow of GET_REGION_INFO and re-setup region might be too heavy.

No, I don't like the disable-able flag. At what frequency do we expect regions to change? It seems like we'd only change when switching into and out of the _SAVING state, which is rare. It seems easy for userspace, at least QEMU, to drop the entire mmap configuration and ok. I'll try this way.

...

re-read it. Another concern here is how do we synchronize the event? Are we assuming that this event would occur when a user switch to _SAVING mode on the device? That operation is synchronous, the device must be in saving mode after the write to device state completes, but it seems like this might be trying to add an asynchronous dependency. Will the write to device_state only complete once the user handles the eventfd? How would the kernel know when the mmap re-evaluation is complete. It seems like there are gaps here that the vendor driver could miss traps required for migration because the user hasn't completed the mmap transition yet. Thanks,

Alex

yes, this asynchronous event notification will cause vendor driver miss traps. But it's supposed to be of very short period time. That's also a reason for us to wish the re-evaluation to be lightweight. E.g. if it's able to be finished before the first iterate, it's still safe.

Making the re-evaluation lightweight cannot solve the race, it only masks it.

...

But I agree, the timing is not guaranteed, and so it's best for kernel to wait for mmap re-evaluation to complete.

migration_thread |->qemu_savevm_state_setup | |->ram_save_setup | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | | | |->vfio_save_setup | |->set_device_state(_SAVING) | |->qemu_savevm_state_pending | |->ram_save_pending | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | |->vfio_save_pending | |->qemu_savevm_state_iterate | |->ram_save_iterate //send pages | |->vfio_save_iterate ...

Actually, we previously let qemu trigger the re-evaluation when migration starts. And now the reason for we to wish kernel to trigger the mmap re-evaluation is that there're other two possible use cases: (1) keep passing through devices when migration starts and track dirty pages using hardware IOMMU. Then when migration is about to complete, stop the device and start trap PCI BARs for software emulation. (we made some changes to let device stop ahead of vcpu )

How is that possible? I/O devices need to continue to work until the vCPU stops otherwise the vCPU can get blocked on the device. Maybe QEMU hi Alex For devices like DSA [1], it can support SVM mode. In this mode, when a

On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: page fault happens, the Intel DSA device blocks until the page fault is resolved, if PRS is enabled; otherwise it is reported as an error.

Therefore, to pass through DSA into guest and do live migration with it, it is desired to stop DSA before stopping vCPU, as there may be an outstanding page fault to be resolved.

During the period when DSA is stopped and vCPUs are still running, all the pass-through resources are trapped and emulated by host mediation driver until vCPUs stop.

If the DSA is stopped and resources are trapped and emulated, then is the device really stopped from a QEMU perspective or has it simply switched modes underneath QEMU? If the device is truly stopped, then I'd like to understand how a vCPU doing a PIO read from the device wouldn't wedge the VM.

[1] https://software.intel.com/sites/default/files/341204-intel-data-streaming-a...

...

should assume all mmaps should be dropped on vfio device after we pass some point of the migration process.

yes, it should be workable for the use case of DSA.

...

If there are a fixed set of mmap settings for a region and discrete conditions under which they become active (ex. switch device to SAVING mode) then QEMU could choose the right mapping itself and we wouldn't need to worry about this asynchronous signaling problem, it would just be defined as part of the protocol userspace needs to use.

It's ok to let QEMU trigger dynamic trap on certain condition (like switching device to SAVING mode), but it seems that there's no fixed set of mmap settings for a region. For example, some devices may want to trap the whole BARs, but some devices only requires to trap a range of pages in a BAR for performance consideration.

If the "disable-able" flag is not preferable, maybe re-evaluation way is the only choice? But it is a burden to ask for re-evaluation if they are not required.

What about introducing a "region_bitmask" in ctl header of the migration region? when QEMU writes a region index to the "region_bitmask", it can read back from this field a bitmask to know which mmap to disable.

If a vendor driver wanted to have a migration sparse mmap that's different from its runtime sparse mmap, we could simply add a new capability in the region_info. Userspace would only need to switch to a different mapping for regions which advertise a new migration sparse mmap capability. Doesn't that serve the same purpose as the proposed bitmap?

...

...

(2) performance optimization. There's an example in GVT (mdev case): PCI BARs are passed through on vGPU initialization and are mmaped to a host dummy buffer. Then after initialization done, start trap of PCI BARs of vGPUs and start normal host mediation. The initial pass-through can save 1000000 times of mmio trap.

Much of this discussion has me worried that many assumptions are being made about the user and device interaction. Backwards compatible behavior is required. If a mdev device presents an initial sparse mmap capability for this acceleration, how do you support an existing userspace that doesn't understand the new dynamic mmap semantics and continues to try to operate with the initial sparse mmap? Doesn't this introduce another example of the raciness of the device trying to switch mmaps? Seems that if QEMU doesn't handle the eventfd with sufficient timeliness the switch back to trap behavior could miss an important transaction. This also seems like an optimization targeted at VMs running for only a short time, where it's not obvious to me that GVT-g overlaps those sorts of use cases. How much initialization time is actually being saved with such a hack? Thanks,

It can save about 4s initialization time with such a hack. But you are right, the backward compatibility is a problem and we are not going to upstream that. Just an example to show the usage. It's fine if we drop the way of asynchronous kernel notification.

I think to handle such a situation we'd need a mechanism to revoke the user's mmap. We can make use of an asynchronous mechanism to improve performance of a device, but we need a synchronous mechanism to maintain correctness. For this example, the sparse mmap capability could advertise the section of the BAR as mmap'able and revoke that user mapping after the device finishes the initialization phase. Potentially the user re-evaluating region_info after the initialization phase would see a different sparse mmap capability excluding these sections, but then we might need to think whether we want to suggest that the user always re-read the region_info after device reset. AFAIK, we currently have no mechanism to revoke user mmaps. Thanks, Alex

On Wed, 11 Dec 2019 01:25:55 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote:

...

On Tue, 10 Dec 2019 02:44:44 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

...

On Mon, 9 Dec 2019 01:22:12 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote:

...

On Fri, 6 Dec 2019 01:04:07 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

> On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote: > > On Wed, 4 Dec 2019 22:26:50 -0500 > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > Dynamic trap bar info region is a channel for QEMU and vendor driver to > > > communicate dynamic trap info. It is of type > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > This region has two fields: dt_fd and trap. > > > When QEMU detects a device regions of this type, it will create an > > > eventfd and write its eventfd id to dt_fd field. > > > When vendor drivre signals this eventfd, QEMU reads trap field of this > > > info region. > > > - If trap is true, QEMU would search the device's PCI BAR > > > regions and disable all the sparse mmaped subregions (if the sparse > > > mmaped subregion is disablable). > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > A typical usage is > > > 1. vendor driver first cuts its bar 0 into several sections, all in a > > > sparse mmap array. So initally, all its bar 0 are passthroughed. > > > 2. vendor driver specifys part of bar 0 sections to be disablable. > > > 3. on migration starts, vendor driver signals dt_fd and set trap to true > > > to notify QEMU disabling the bar 0 sections of disablable flags on. > > > 4. QEMU disables those bar 0 section and hence let vendor driver be able > > > to trap access of bar 0 registers and make dirty page tracking possible. > > > 5. on migration failure, vendor driver signals dt_fd to QEMU again. > > > QEMU reads trap field of this info region which is false and QEMU > > > re-passthrough the whole bar 0 region. > > > > > > Vendor driver specifies whether it supports dynamic-trap-bar-info region > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > vfio_pci_mediate_ops->open(). > > > > > > If vfio-pci detects this cap, it will create a default > > > dynamic_trap_bar_info region on behalf of vendor driver with region len=0 > > > and region->ops=null. > > > Vvendor driver should override this region's len, flags, rw, mmap in its > > > vfio_pci_mediate_ops. > > > > TBH, I don't like this interface at all. Userspace doesn't pass data > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > configuring user signaling with eventfds. I think we only need to > > define an IRQ type that tells the user to re-evaluate the sparse mmap > > information for a region. The user would enumerate the device IRQs via > > GET_IRQ_INFO, find one of this type where the IRQ info would also > > indicate which region(s) should be re-evaluated on signaling. The user > > would enable that signaling via SET_IRQS and simply re-evaluate the > ok. I'll try to switch to this way. Thanks for this suggestion. > > > sparse mmap capability for the associated regions when signaled. > > Do you like the "disablable" flag of sparse mmap ? > I think it's a lightweight way for user to switch mmap state of a whole region, > otherwise going through a complete flow of GET_REGION_INFO and re-setup > region might be too heavy.

No, I don't like the disable-able flag. At what frequency do we expect regions to change? It seems like we'd only change when switching into and out of the _SAVING state, which is rare. It seems easy for userspace, at least QEMU, to drop the entire mmap configuration and ok. I'll try this way.

...

re-read it. Another concern here is how do we synchronize the event? Are we assuming that this event would occur when a user switch to _SAVING mode on the device? That operation is synchronous, the device must be in saving mode after the write to device state completes, but it seems like this might be trying to add an asynchronous dependency. Will the write to device_state only complete once the user handles the eventfd? How would the kernel know when the mmap re-evaluation is complete. It seems like there are gaps here that the vendor driver could miss traps required for migration because the user hasn't completed the mmap transition yet. Thanks,

Alex

yes, this asynchronous event notification will cause vendor driver miss traps. But it's supposed to be of very short period time. That's also a reason for us to wish the re-evaluation to be lightweight. E.g. if it's able to be finished before the first iterate, it's still safe.

Making the re-evaluation lightweight cannot solve the race, it only masks it.

...

But I agree, the timing is not guaranteed, and so it's best for kernel to wait for mmap re-evaluation to complete.

migration_thread |->qemu_savevm_state_setup | |->ram_save_setup | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | | | |->vfio_save_setup | |->set_device_state(_SAVING) | |->qemu_savevm_state_pending | |->ram_save_pending | | |->migration_bitmap_sync | | |->kvm_log_sync | | |->vfio_log_sync | |->vfio_save_pending | |->qemu_savevm_state_iterate | |->ram_save_iterate //send pages | |->vfio_save_iterate ...

Actually, we previously let qemu trigger the re-evaluation when migration starts. And now the reason for we to wish kernel to trigger the mmap re-evaluation is that there're other two possible use cases: (1) keep passing through devices when migration starts and track dirty pages using hardware IOMMU. Then when migration is about to complete, stop the device and start trap PCI BARs for software emulation. (we made some changes to let device stop ahead of vcpu )

How is that possible? I/O devices need to continue to work until the vCPU stops otherwise the vCPU can get blocked on the device. Maybe QEMU hi Alex For devices like DSA [1], it can support SVM mode. In this mode, when a

On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: page fault happens, the Intel DSA device blocks until the page fault is resolved, if PRS is enabled; otherwise it is reported as an error.

Therefore, to pass through DSA into guest and do live migration with it, it is desired to stop DSA before stopping vCPU, as there may be an outstanding page fault to be resolved.

During the period when DSA is stopped and vCPUs are still running, all the pass-through resources are trapped and emulated by host mediation driver until vCPUs stop.

If the DSA is stopped and resources are trapped and emulated, then is the device really stopped from a QEMU perspective or has it simply switched modes underneath QEMU? If the device is truly stopped, then I'd like to understand how a vCPU doing a PIO read from the device wouldn't wedge the VM.

It doesn't matter if the device is truly stopped or not (although from my point of view, just draining commands and keeping device running is better as it handles live migration failure better). PIOs also need to be trapped and emulated if a vCPU accesses them.

We seem to be talking around each other here. If PIOs are trapped and emulated then the device is not "stopped" as far as QEMU is concerned, right? "Stopping" a device suggests to me that a running vCPU doing a PIO read from the device would block and cause problems in the still running VM. So I think you're suggesting some sort of mode switch in the device where direct access is disabled an emulation takes over until the vCPUs are stopped.

...

...

[1] https://software.intel.com/sites/default/files/341204-intel-data-streaming-a...

...

should assume all mmaps should be dropped on vfio device after we pass some point of the migration process.

yes, it should be workable for the use case of DSA.

...

If there are a fixed set of mmap settings for a region and discrete conditions under which they become active (ex. switch device to SAVING mode) then QEMU could choose the right mapping itself and we wouldn't need to worry about this asynchronous signaling problem, it would just be defined as part of the protocol userspace needs to use.

It's ok to let QEMU trigger dynamic trap on certain condition (like switching device to SAVING mode), but it seems that there's no fixed set of mmap settings for a region. For example, some devices may want to trap the whole BARs, but some devices only requires to trap a range of pages in a BAR for performance consideration.

If the "disable-able" flag is not preferable, maybe re-evaluation way is the only choice? But it is a burden to ask for re-evaluation if they are not required.

What about introducing a "region_bitmask" in ctl header of the migration region? when QEMU writes a region index to the "region_bitmask", it can read back from this field a bitmask to know which mmap to disable.

If a vendor driver wanted to have a migration sparse mmap that's different from its runtime sparse mmap, we could simply add a new capability in the region_info. Userspace would only need to switch to a different mapping for regions which advertise a new migration sparse mmap capability. Doesn't that serve the same purpose as the proposed bitmap?

yes, it does. I will try this way in next version.

...

...

...

...

(2) performance optimization. There's an example in GVT (mdev case): PCI BARs are passed through on vGPU initialization and are mmaped to a host dummy buffer. Then after initialization done, start trap of PCI BARs of vGPUs and start normal host mediation. The initial pass-through can save 1000000 times of mmio trap.

Much of this discussion has me worried that many assumptions are being made about the user and device interaction. Backwards compatible behavior is required. If a mdev device presents an initial sparse mmap capability for this acceleration, how do you support an existing userspace that doesn't understand the new dynamic mmap semantics and continues to try to operate with the initial sparse mmap? Doesn't this introduce another example of the raciness of the device trying to switch mmaps? Seems that if QEMU doesn't handle the eventfd with sufficient timeliness the switch back to trap behavior could miss an important transaction. This also seems like an optimization targeted at VMs running for only a short time, where it's not obvious to me that GVT-g overlaps those sorts of use cases. How much initialization time is actually being saved with such a hack? Thanks,

It can save about 4s initialization time with such a hack. But you are right, the backward compatibility is a problem and we are not going to upstream that. Just an example to show the usage. It's fine if we drop the way of asynchronous kernel notification.

I think to handle such a situation we'd need a mechanism to revoke the user's mmap. We can make use of an asynchronous mechanism to improve performance of a device, but we need a synchronous mechanism to maintain correctness. For this example, the sparse mmap capability could advertise the section of the BAR as mmap'able and revoke that user mapping after the device finishes the initialization phase. Potentially the user re-evaluating region_info after the initialization phase would see a different sparse mmap capability excluding these sections, but then we might need to think whether we want to suggest that the user always re-read the region_info after device reset. AFAIK, we currently have no mechanism to revoke user mmaps. Thanks,

Actually I think the "disable-able" flag is good except for its backward compatibility :)

Setting a flag on a section of a region doesn't solve the asynchronous problem. Thanks, Alex

On Wed, 11 Dec 2019 21:02:40 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

On Thu, Dec 12, 2019 at 02:56:55AM +0800, Alex Williamson wrote:

...

On Wed, 11 Dec 2019 01:25:55 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote:

...

On Tue, 10 Dec 2019 02:44:44 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

...

...

On Mon, 9 Dec 2019 01:22:12 -0500 Yan Zhao <yan.y.zhao@intel.com> wrote:

> On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > On Fri, 6 Dec 2019 01:04:07 -0500 > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote: > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > Yan Zhao <yan.y.zhao@intel.com> wrote: > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and vendor driver to > > > > > communicate dynamic trap info. It is of type > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > When QEMU detects a device regions of this type, it will create an > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > When vendor drivre signals this eventfd, QEMU reads trap field of this > > > > > info region. > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > regions and disable all the sparse mmaped subregions (if the sparse > > > > > mmaped subregion is disablable). > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > A typical usage is > > > > > 1. vendor driver first cuts its bar 0 into several sections, all in a > > > > > sparse mmap array. So initally, all its bar 0 are passthroughed. > > > > > 2. vendor driver specifys part of bar 0 sections to be disablable. > > > > > 3. on migration starts, vendor driver signals dt_fd and set trap to true > > > > > to notify QEMU disabling the bar 0 sections of disablable flags on. > > > > > 4. QEMU disables those bar 0 section and hence let vendor driver be able > > > > > to trap access of bar 0 registers and make dirty page tracking possible. > > > > > 5. on migration failure, vendor driver signals dt_fd to QEMU again. > > > > > QEMU reads trap field of this info region which is false and QEMU > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > Vendor driver specifies whether it supports dynamic-trap-bar-info region > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > dynamic_trap_bar_info region on behalf of vendor driver with region len=0 > > > > > and region->ops=null. > > > > > Vvendor driver should override this region's len, flags, rw, mmap in its > > > > > vfio_pci_mediate_ops. > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't pass data > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > configuring user signaling with eventfds. I think we only need to > > > > define an IRQ type that tells the user to re-evaluate the sparse mmap > > > > information for a region. The user would enumerate the device IRQs via > > > > GET_IRQ_INFO, find one of this type where the IRQ info would also > > > > indicate which region(s) should be re-evaluated on signaling. The user > > > > would enable that signaling via SET_IRQS and simply re-evaluate the > > > ok. I'll try to switch to this way. Thanks for this suggestion. > > > > > > > sparse mmap capability for the associated regions when signaled. > > > > > > Do you like the "disablable" flag of sparse mmap ? > > > I think it's a lightweight way for user to switch mmap state of a whole region, > > > otherwise going through a complete flow of GET_REGION_INFO and re-setup > > > region might be too heavy. > > > > No, I don't like the disable-able flag. At what frequency do we expect > > regions to change? It seems like we'd only change when switching into > > and out of the _SAVING state, which is rare. It seems easy for > > userspace, at least QEMU, to drop the entire mmap configuration and > ok. I'll try this way. > > > re-read it. Another concern here is how do we synchronize the event? > > Are we assuming that this event would occur when a user switch to > > _SAVING mode on the device? That operation is synchronous, the device > > must be in saving mode after the write to device state completes, but > > it seems like this might be trying to add an asynchronous dependency. > > Will the write to device_state only complete once the user handles the > > eventfd? How would the kernel know when the mmap re-evaluation is > > complete. It seems like there are gaps here that the vendor driver > > could miss traps required for migration because the user hasn't > > completed the mmap transition yet. Thanks, > > > > Alex > > yes, this asynchronous event notification will cause vendor driver miss > traps. But it's supposed to be of very short period time. That's also a > reason for us to wish the re-evaluation to be lightweight. E.g. if it's > able to be finished before the first iterate, it's still safe.

Making the re-evaluation lightweight cannot solve the race, it only masks it.

> But I agree, the timing is not guaranteed, and so it's best for kernel > to wait for mmap re-evaluation to complete. > > migration_thread > |->qemu_savevm_state_setup > | |->ram_save_setup > | | |->migration_bitmap_sync > | | |->kvm_log_sync > | | |->vfio_log_sync > | | > | |->vfio_save_setup > | |->set_device_state(_SAVING) > | > |->qemu_savevm_state_pending > | |->ram_save_pending > | | |->migration_bitmap_sync > | | |->kvm_log_sync > | | |->vfio_log_sync > | |->vfio_save_pending > | > |->qemu_savevm_state_iterate > | |->ram_save_iterate //send pages > | |->vfio_save_iterate > ... > > > Actually, we previously let qemu trigger the re-evaluation when migration starts. > And now the reason for we to wish kernel to trigger the mmap re-evaluation is that > there're other two possible use cases: > (1) keep passing through devices when migration starts and track dirty pages > using hardware IOMMU. Then when migration is about to complete, stop the > device and start trap PCI BARs for software emulation. (we made some > changes to let device stop ahead of vcpu )

How is that possible? I/O devices need to continue to work until the vCPU stops otherwise the vCPU can get blocked on the device. Maybe QEMU hi Alex For devices like DSA [1], it can support SVM mode. In this mode, when a

On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: page fault happens, the Intel DSA device blocks until the page fault is resolved, if PRS is enabled; otherwise it is reported as an error.

Therefore, to pass through DSA into guest and do live migration with it, it is desired to stop DSA before stopping vCPU, as there may be an outstanding page fault to be resolved.

During the period when DSA is stopped and vCPUs are still running, all the pass-through resources are trapped and emulated by host mediation driver until vCPUs stop.

If the DSA is stopped and resources are trapped and emulated, then is the device really stopped from a QEMU perspective or has it simply switched modes underneath QEMU? If the device is truly stopped, then I'd like to understand how a vCPU doing a PIO read from the device wouldn't wedge the VM.

It doesn't matter if the device is truly stopped or not (although from my point of view, just draining commands and keeping device running is better as it handles live migration failure better). PIOs also need to be trapped and emulated if a vCPU accesses them.

We seem to be talking around each other here. If PIOs are trapped and emulated then the device is not "stopped" as far as QEMU is concerned, right? "Stopping" a device suggests to me that a running vCPU doing a PIO read from the device would block and cause problems in the still running VM. So I think you're suggesting some sort of mode switch in the device where direct access is disabled an emulation takes over until the vCPUs are stopped.

sorry for this confusion. yes, it's a kind of mode switch from a QEMU perspective. Currently, its implementation in our local branch is like that: 1. before migration thread stopping vCPUs, a migration state (COMPLETING) notification is sent to vfio migration state notifier, and this notifier would put device state to !RUNNING, and put all BARs to trap state. 2. in the kernel, when device state is set to !RUNNING, draining all pending device requests, and starts emulation.

This implementation has two issues: 1. it requires hardcode in QEMU to put all BARs trapped and the time spending on revoking mmaps is not necessary for devices that do not need it. 2. !RUNNING state here is not accurate and it will confuse vendor drivers who stop devices after vCPUs stop.

For the 2nd issue, I think we can propose a new device state like PRE-STOPPING.

Yes, this is absolutely abusing the !RUNNING state, if the device is still processing accesses by the vCPU, it's still running.

But for the 1st issue, not sure how to fix it right now. Maybe we can still add an asynchronous kernel notification and wait until QEMU have switched the region mmap state?

It seems like you're preemptively trying to optimize the SAVING state before we even have migration working. Shouldn't SAVING be the point at which you switch to trapping the device in order to track it? Thanks, Alex

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

Hi:

On 2019/12/5 上午11:24, Yan Zhao wrote:

...

For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and

A silly question, what's the reason for doing this, is this a must for dirty page tracking?

For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need.

...

(3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver.

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __________ register mediate ops| ___________ ___________ | | |<-----------------------| VF | | | | vfio-pci | | | mediate | | PF driver | | |__________|----------------------->| driver | |___________| | open(pdev) | ----------- | | | | | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ ----------- ------------ | VF | | PF | ----------- ------------

VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle.

Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw.

when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter.

The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id.

When VF mediate driver exits, it unregisters its mediate ops from vfio-pci.

In this patchset, we enable vfio-pci to provide 3 things: (1) calling mediate ops to allow vendor driver customizing default region info/rw/mmap of a region. (2) provide a migration region to support migration

What's the benefit of introducing a region? It looks to me we don't expect the region to be accessed directly from guest. Could we simply extend device fd ioctl for doing such things?

You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/ generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way. The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs.

...

(3) provide a dynamic trap bar info region to allow vendor driver control trap/untrap of device pci bars

This vfio-pci + mediate ops way differs from mdev way in that (1) medv way needs to create a 1:1 mdev device on top of one VF, device specific mdev parent driver is bound to VF directly. (2) vfio-pci + mediate ops way does not create mdev devices and VF mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs.

The reason why we don't choose the way of writing mdev parent driver is that (1) VFs are almost all the time directly passthroughed. Directly binding to vfio-pci can make most of the code shared/reused.

Can we split out the common parts from vfio-pci?

That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :)

...

If we write a vendor specific mdev parent driver, most of the code (like passthrough style of rw/mmap) still needs to be copied from vfio-pci driver, which is actually a duplicated and tedious work.

The mediate ops looks quite similar to what vfio-mdev did. And it looks to me we need to consider live migration for mdev as well. In that case, do we still expect mediate ops through VFIO directly?

...

(2) For features like dynamically trap/untrap pci bars, if they are in vfio-pci, they can be available to most people without repeated code copying and re-testing. (3) with a 1:1 mdev driver which passthrough VFs most of the time, people have to decide whether to bind VFs to vfio-pci or mdev parent driver before it runs into a real migration need. However, if vfio-pci is bound initially, they have no chance to do live migration when there's a need later.

We can teach management layer to do this.

No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now. Thanks Yn

Thanks

...

In this patchset, - patches 1-4 enable vfio-pci to call mediate ops registered by vendor driver to mediate/customize region info/rw/mmap.

- patches 5-6 provide a standalone sample driver to register a mediate ops for Intel Graphics Devices. It does not bind to IGDs directly but decides what devices it supports via its pciidlist. It also demonstrates how to dynamic trap a device's PCI bars. (by adding more pciids in its pciidlist, this sample driver actually is not necessarily limited to support IGDs)

- patch 7-9 provide a sample on i40e driver that supports Intel(R) Ethernet Controller XL710 Family of devices. It supports VF precopy live migration on Intel's 710 SRIOV. (but we commented out the real implementation of dirty page tracking and device state retrieving part to focus on demonstrating framework part. Will send out them in future versions) patch 7 registers/unregisters VF mediate ops when PF driver probes/removes. It specifies its supporting VFs via vfio_pci_mediate_ops->open(pdev)

patch 8 reports device cap of VFIO_PCI_DEVICE_CAP_MIGRATION and provides a sample implementation of migration region. The QEMU part of vfio migration is based on v8 https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html. We do not based on recent v9 because we think there are still opens in dirty page track part in that series.

patch 9 reports device cap of VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR and provides an example on how to trap part of bar0 when migration starts and passthrough this part of bar0 again when migration fails.

Yan Zhao (9): vfio/pci: introduce mediate ops to intercept vfio-pci ops vfio/pci: test existence before calling region->ops vfio/pci: register a default migration region vfio-pci: register default dynamic-trap-bar-info region samples/vfio-pci/igd_dt: sample driver to mediate a passthrough IGD sample/vfio-pci/igd_dt: dynamically trap/untrap subregion of IGD bar0 i40e/vf_migration: register mediate_ops to vfio-pci i40e/vf_migration: mediate migration region i40e/vf_migration: support dynamic trap of bar0

drivers/net/ethernet/intel/Kconfig | 2 +- drivers/net/ethernet/intel/i40e/Makefile | 3 +- drivers/net/ethernet/intel/i40e/i40e.h | 2 + drivers/net/ethernet/intel/i40e/i40e_main.c | 3 + .../ethernet/intel/i40e/i40e_vf_migration.c | 626 ++++++++++++++++++ .../ethernet/intel/i40e/i40e_vf_migration.h | 78 +++ drivers/vfio/pci/vfio_pci.c | 189 +++++- drivers/vfio/pci/vfio_pci_private.h | 2 + include/linux/vfio.h | 18 + include/uapi/linux/vfio.h | 160 +++++ samples/Kconfig | 6 + samples/Makefile | 1 + samples/vfio-pci/Makefile | 2 + samples/vfio-pci/igd_dt.c | 367 ++++++++++ 14 files changed, 1455 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.c create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.h create mode 100644 samples/vfio-pci/Makefile create mode 100644 samples/vfio-pci/igd_dt.c

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

On 2019/12/5 下午4:51, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

...

Hi:

On 2019/12/5 上午11:24, Yan Zhao wrote:

...

For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and

A silly question, what's the reason for doing this, is this a must for dirty page tracking?

For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need.

Right, but how does this matter for the case of dirty page tracking?

Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0. There's still no IOMMU Dirty bit available.

...

...

...

(3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver.

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __________ register mediate ops| ___________ ___________ | | |<-----------------------| VF | | | | vfio-pci | | | mediate | | PF driver | | |__________|----------------------->| driver | |___________| | open(pdev) | ----------- | | | | | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ ----------- ------------ | VF | | PF | ----------- ------------

VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle.

Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw.

when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter.

The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id.

When VF mediate driver exits, it unregisters its mediate ops from vfio-pci.

In this patchset, we enable vfio-pci to provide 3 things: (1) calling mediate ops to allow vendor driver customizing default region info/rw/mmap of a region. (2) provide a migration region to support migration

What's the benefit of introducing a region? It looks to me we don't expect the region to be accessed directly from guest. Could we simply extend device fd ioctl for doing such things?

You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/

generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way.

So in mdev, do you still have a mediate driver? Or you expect the parent to implement the region?

No, currently it's only for vfio-pci. mdev parent driver is free to customize its regions and hence does not requires this mediate ops hooks.

...

The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs.

Well, at least you introduce new type of region in uapi. So this does not answer why region is better than ioctl. If the region will only be used by qemu, using ioctl is much more easier and straightforward.

It's not introduced by me :) mdev live migration is actually using this way, I'm just keeping compatible to the uapi.

From my own perspective, my answer is that a region is more flexible compared to ioctl. vendor driver can freely define the size, mmap cap of its data subregion. Also, there're already too many ioctls in vfio.

...

...

...

(3) provide a dynamic trap bar info region to allow vendor driver control trap/untrap of device pci bars

This vfio-pci + mediate ops way differs from mdev way in that (1) medv way needs to create a 1:1 mdev device on top of one VF, device specific mdev parent driver is bound to VF directly. (2) vfio-pci + mediate ops way does not create mdev devices and VF mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs.

The reason why we don't choose the way of writing mdev parent driver is that (1) VFs are almost all the time directly passthroughed. Directly binding to vfio-pci can make most of the code shared/reused.

Can we split out the common parts from vfio-pci?

That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :)

Well, I think there should be not hard to do that. E..g you can route it back to like:

vfio -> vfio_mdev -> parent -> vfio_pci

it's desired for us to have mediate driver binding to PF device. so once a VF device is created, only PF driver and vfio-pci are required. Just the same as what needs to be done for a normal VF passthrough. otherwise, a separate parent driver binding to VF is required. Also, this parent driver has many drawbacks as I mentions in this cover-letter.

...

...

...

If we write a vendor specific mdev parent driver, most of the code (like passthrough style of rw/mmap) still needs to be copied from vfio-pci driver, which is actually a duplicated and tedious work.

The mediate ops looks quite similar to what vfio-mdev did. And it looks to me we need to consider live migration for mdev as well. In that case, do we still expect mediate ops through VFIO directly?

...

(2) For features like dynamically trap/untrap pci bars, if they are in vfio-pci, they can be available to most people without repeated code copying and re-testing. (3) with a 1:1 mdev driver which passthrough VFs most of the time, people have to decide whether to bind VFs to vfio-pci or mdev parent driver before it runs into a real migration need. However, if vfio-pci is bound initially, they have no chance to do live migration when there's a need later.

We can teach management layer to do this.

No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now.

I'm not quite sure I get here but in this case, just tech them to use the driver that has migration support?

That's a way, but as more and more passthrough devices have demands and caps to do migration, will vfio-pci be used in future any more ? Thanks Yan

Thanks

...

Thanks Yn

...

Thanks

...

In this patchset, - patches 1-4 enable vfio-pci to call mediate ops registered by vendor driver to mediate/customize region info/rw/mmap.

- patches 5-6 provide a standalone sample driver to register a mediate ops for Intel Graphics Devices. It does not bind to IGDs directly but decides what devices it supports via its pciidlist. It also demonstrates how to dynamic trap a device's PCI bars. (by adding more pciids in its pciidlist, this sample driver actually is not necessarily limited to support IGDs)

- patch 7-9 provide a sample on i40e driver that supports Intel(R) Ethernet Controller XL710 Family of devices. It supports VF precopy live migration on Intel's 710 SRIOV. (but we commented out the real implementation of dirty page tracking and device state retrieving part to focus on demonstrating framework part. Will send out them in future versions) patch 7 registers/unregisters VF mediate ops when PF driver probes/removes. It specifies its supporting VFs via vfio_pci_mediate_ops->open(pdev)

patch 8 reports device cap of VFIO_PCI_DEVICE_CAP_MIGRATION and provides a sample implementation of migration region. The QEMU part of vfio migration is based on v8 https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html. We do not based on recent v9 because we think there are still opens in dirty page track part in that series.

patch 9 reports device cap of VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR and provides an example on how to trap part of bar0 when migration starts and passthrough this part of bar0 again when migration fails.

Yan Zhao (9): vfio/pci: introduce mediate ops to intercept vfio-pci ops vfio/pci: test existence before calling region->ops vfio/pci: register a default migration region vfio-pci: register default dynamic-trap-bar-info region samples/vfio-pci/igd_dt: sample driver to mediate a passthrough IGD sample/vfio-pci/igd_dt: dynamically trap/untrap subregion of IGD bar0 i40e/vf_migration: register mediate_ops to vfio-pci i40e/vf_migration: mediate migration region i40e/vf_migration: support dynamic trap of bar0

drivers/net/ethernet/intel/Kconfig | 2 +- drivers/net/ethernet/intel/i40e/Makefile | 3 +- drivers/net/ethernet/intel/i40e/i40e.h | 2 + drivers/net/ethernet/intel/i40e/i40e_main.c | 3 + .../ethernet/intel/i40e/i40e_vf_migration.c | 626 ++++++++++++++++++ .../ethernet/intel/i40e/i40e_vf_migration.h | 78 +++ drivers/vfio/pci/vfio_pci.c | 189 +++++- drivers/vfio/pci/vfio_pci_private.h | 2 + include/linux/vfio.h | 18 + include/uapi/linux/vfio.h | 160 +++++ samples/Kconfig | 6 + samples/Makefile | 1 + samples/vfio-pci/Makefile | 2 + samples/vfio-pci/igd_dt.c | 367 ++++++++++ 14 files changed, 1455 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.c create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.h create mode 100644 samples/vfio-pci/Makefile create mode 100644 samples/vfio-pci/igd_dt.c

On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote:

On 2019/12/6 下午4:22, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

...

On 2019/12/5 下午4:51, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

...

Hi:

...

For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and A silly question, what's the reason for doing this, is this a must for dirty

On 2019/12/5 上午11:24, Yan Zhao wrote: page tracking?

For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need.

Right, but how does this matter for the case of dirty page tracking?

Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0.

Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case.

hi Jason not familiar with the way you mentioned. could you elaborate more?

...

There's still no IOMMU Dirty bit available.

...

...

...

...

(3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver.

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __________ register mediate ops| ___________ ___________ | | |<-----------------------| VF | | | | vfio-pci | | | mediate | | PF driver | | |__________|----------------------->| driver | |___________| | open(pdev) | ----------- | | | | | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ ----------- ------------ | VF | | PF | ----------- ------------

VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle.

Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw.

when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter.

The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id.

When VF mediate driver exits, it unregisters its mediate ops from vfio-pci.

In this patchset, we enable vfio-pci to provide 3 things: (1) calling mediate ops to allow vendor driver customizing default region info/rw/mmap of a region. (2) provide a migration region to support migration

What's the benefit of introducing a region? It looks to me we don't expect the region to be accessed directly from guest. Could we simply extend device fd ioctl for doing such things?

You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/

generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way.

So in mdev, do you still have a mediate driver? Or you expect the parent to implement the region?

No, currently it's only for vfio-pci.

And specific to PCI.

...

mdev parent driver is free to customize its regions and hence does not requires this mediate ops hooks.

...

...

The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs.

Well, at least you introduce new type of region in uapi. So this does not answer why region is better than ioctl. If the region will only be used by qemu, using ioctl is much more easier and straightforward.

It's not introduced by me :) mdev live migration is actually using this way, I'm just keeping compatible to the uapi.

I meant e.g VFIO_REGION_TYPE_MIGRATION.

here's the history of vfio live migration: https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05564.html https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html If you have any concern of this region way, feel free to comment to the latest v9 patchset: https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html The patchset here will always keep compatible to there.

...

From my own perspective, my answer is that a region is more flexible compared to ioctl. vendor driver can freely define the size,

Probably not since it's an ABI I think.

that's why I need to define VFIO_REGION_TYPE_MIGRATION here in this patchset, as it's not upstreamed yet. maybe I should make it into a prerequisite patch, indicating it is not introduced by this patchset

...

mmap cap of its data subregion.

It doesn't help much unless it can be mapped into guest (which I don't think it was the case here).

it's access by host qemu, the same as how linux app access an mmaped memory. the mmap here is to reduce memory copy from kernel to user. No need to get mapped into guest.

...

Also, there're already too many ioctls in vfio.

Probably not :) We had a brunch of  subsystems that have much more ioctls than VFIO. (e.g DRM)

...

...

...

...

...

(3) provide a dynamic trap bar info region to allow vendor driver control trap/untrap of device pci bars

This vfio-pci + mediate ops way differs from mdev way in that (1) medv way needs to create a 1:1 mdev device on top of one VF, device specific mdev parent driver is bound to VF directly. (2) vfio-pci + mediate ops way does not create mdev devices and VF mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs.

The reason why we don't choose the way of writing mdev parent driver is that (1) VFs are almost all the time directly passthroughed. Directly binding to vfio-pci can make most of the code shared/reused. Can we split out the common parts from vfio-pci?

That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :)

Well, I think there should be not hard to do that. E..g you can route it back to like:

vfio -> vfio_mdev -> parent -> vfio_pci

it's desired for us to have mediate driver binding to PF device. so once a VF device is created, only PF driver and vfio-pci are required. Just the same as what needs to be done for a normal VF passthrough. otherwise, a separate parent driver binding to VF is required. Also, this parent driver has many drawbacks as I mentions in this cover-letter.

Well, as discussed, no need to duplicate the code, bar trick should still work. The main issues I saw with this proposal is:

1) PCI specific, other bus may need something similar vfio-pci is only for PCI of course.

2) Function duplicated with mdev and mdev can do even more

could you elaborate how mdev can do solve the above saying problem ?

...

...

...

...

...

If we write a vendor specific mdev parent driver, most of the code (like passthrough style of rw/mmap) still needs to be copied from vfio-pci driver, which is actually a duplicated and tedious work.

The mediate ops looks quite similar to what vfio-mdev did. And it looks to me we need to consider live migration for mdev as well. In that case, do we still expect mediate ops through VFIO directly?

...

(2) For features like dynamically trap/untrap pci bars, if they are in vfio-pci, they can be available to most people without repeated code copying and re-testing. (3) with a 1:1 mdev driver which passthrough VFs most of the time, people have to decide whether to bind VFs to vfio-pci or mdev parent driver before it runs into a real migration need. However, if vfio-pci is bound initially, they have no chance to do live migration when there's a need later. We can teach management layer to do this.

No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now.

I'm not quite sure I get here but in this case, just tech them to use the driver that has migration support?

That's a way, but as more and more passthrough devices have demands and caps to do migration, will vfio-pci be used in future any more ?

This should not be a problem: - If we introduce a common mdev for vfio-pci, we can just bind that driver always

what is common mdev for vfio-pci? a common mdev parent driver that have the same implementation as vfio-pci? There's actually already a solution of creating only one mdev on top of each passthrough device, and make mdev share the same iommu group with it. We've also made an implementation on it already. here's a sample one made by Yi at https://patchwork.kernel.org/cover/11134695/. But, as I said, it's desired to re-use vfio-pci directly for SRIOV, which is straghtforward :)

- The most straightforward way to support dirty page tracking is done by IOMMU instead of device specific operations.

No such IOMMU yet. And all kinds of platforms should be cared, right? Thanks Yan

Thanks

...

Thanks Yan

...

Thanks

...

Thanks Yn

...

Thanks

...

In this patchset, - patches 1-4 enable vfio-pci to call mediate ops registered by vendor driver to mediate/customize region info/rw/mmap.

- patches 5-6 provide a standalone sample driver to register a mediate ops for Intel Graphics Devices. It does not bind to IGDs directly but decides what devices it supports via its pciidlist. It also demonstrates how to dynamic trap a device's PCI bars. (by adding more pciids in its pciidlist, this sample driver actually is not necessarily limited to support IGDs)

- patch 7-9 provide a sample on i40e driver that supports Intel(R) Ethernet Controller XL710 Family of devices. It supports VF precopy live migration on Intel's 710 SRIOV. (but we commented out the real implementation of dirty page tracking and device state retrieving part to focus on demonstrating framework part. Will send out them in future versions) patch 7 registers/unregisters VF mediate ops when PF driver probes/removes. It specifies its supporting VFs via vfio_pci_mediate_ops->open(pdev)

patch 8 reports device cap of VFIO_PCI_DEVICE_CAP_MIGRATION and provides a sample implementation of migration region. The QEMU part of vfio migration is based on v8 https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html. We do not based on recent v9 because we think there are still opens in dirty page track part in that series.

patch 9 reports device cap of VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR and provides an example on how to trap part of bar0 when migration starts and passthrough this part of bar0 again when migration fails.

Yan Zhao (9): vfio/pci: introduce mediate ops to intercept vfio-pci ops vfio/pci: test existence before calling region->ops vfio/pci: register a default migration region vfio-pci: register default dynamic-trap-bar-info region samples/vfio-pci/igd_dt: sample driver to mediate a passthrough IGD sample/vfio-pci/igd_dt: dynamically trap/untrap subregion of IGD bar0 i40e/vf_migration: register mediate_ops to vfio-pci i40e/vf_migration: mediate migration region i40e/vf_migration: support dynamic trap of bar0

drivers/net/ethernet/intel/Kconfig | 2 +- drivers/net/ethernet/intel/i40e/Makefile | 3 +- drivers/net/ethernet/intel/i40e/i40e.h | 2 + drivers/net/ethernet/intel/i40e/i40e_main.c | 3 + .../ethernet/intel/i40e/i40e_vf_migration.c | 626 ++++++++++++++++++ .../ethernet/intel/i40e/i40e_vf_migration.h | 78 +++ drivers/vfio/pci/vfio_pci.c | 189 +++++- drivers/vfio/pci/vfio_pci_private.h | 2 + include/linux/vfio.h | 18 + include/uapi/linux/vfio.h | 160 +++++ samples/Kconfig | 6 + samples/Makefile | 1 + samples/vfio-pci/Makefile | 2 + samples/vfio-pci/igd_dt.c | 367 ++++++++++ 14 files changed, 1455 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.c create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.h create mode 100644 samples/vfio-pci/Makefile create mode 100644 samples/vfio-pci/igd_dt.c

On Thu, Dec 12, 2019 at 11:48:25AM +0800, Jason Wang wrote:

On 2019/12/6 下午8:49, Yan Zhao wrote:

...

On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote:

...

On 2019/12/6 下午4:22, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

...

On 2019/12/5 下午4:51, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: > Hi: > > On 2019/12/5 上午11:24, Yan Zhao wrote: >> For SRIOV devices, VFs are passthroughed into guest directly without host >> driver mediation. However, when VMs migrating with passthroughed VFs, >> dynamic host mediation is required to (1) get device states, (2) get >> dirty pages. Since device states as well as other critical information >> required for dirty page tracking for VFs are usually retrieved from PFs, >> it is handy to provide an extension in PF driver to centralizingly control >> VFs' migration. >> >> Therefore, in order to realize (1) passthrough VFs at normal time, (2) >> dynamically trap VFs' bars for dirty page tracking and > A silly question, what's the reason for doing this, is this a must for dirty > page tracking? > For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need. Right, but how does this matter for the case of dirty page tracking?

Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0.

Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case.

hi Jason

not familiar with the way you mentioned. could you elaborate more?

It looks to me that you want to intercept the bar that contains the head. Then you can figure out the buffers submitted from driver and you still need to decide a proper time to mark them as dirty.

Not need to be accurate, right? just a superset of real dirty bitmap is enough.

What I meant is, intercept the interrupt, then you can figure still figure out the buffers which has been modified by the device and make them as dirty.

Then there's no need to trap BAR and do decoding/emulation etc.

But it will still be tricky to be correct...

intercept the interrupt is a little hard if post interrupt is enabled.. I think what you worried about here is the timing to mark dirty pages, right? upon interrupt receiving, you regard DMAs are finished and safe to make them dirty. But with BAR trap way, we at least can keep those dirtied pages as dirty until device stop. Of course we have other methods to optimize it.

...

...

...

There's still no IOMMU Dirty bit available.

...

...

>> (3) centralizing >> VF critical states retrieving and VF controls into one driver, we propose >> to introduce mediate ops on top of current vfio-pci device driver. >> >> >> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ >> __________ register mediate ops| ___________ ___________ | >> | |<-----------------------| VF | | | >> | vfio-pci | | | mediate | | PF driver | | >> |__________|----------------------->| driver | |___________| >> | open(pdev) | ----------- | | >> | | >> | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| >> \|/ \|/ >> ----------- ------------ >> | VF | | PF | >> ----------- ------------ >> >> >> VF mediate driver could be a standalone driver that does not bind to >> any devices (as in demo code in patches 5-6) or it could be a built-in >> extension of PF driver (as in patches 7-9) . >> >> Rather than directly bind to VF, VF mediate driver register a mediate >> ops into vfio-pci in driver init. vfio-pci maintains a list of such >> mediate ops. >> (Note that: VF mediate driver can register mediate ops into vfio-pci >> before vfio-pci binding to any devices. And VF mediate driver can >> support mediating multiple devices.) >> >> When opening a device (e.g. a VF), vfio-pci goes through the mediate ops >> list and calls each vfio_pci_mediate_ops->open() with pdev of the opening >> device as a parameter. >> VF mediate driver should return success or failure depending on it >> supports the pdev or not. >> E.g. VF mediate driver would compare its supported VF devfn with the >> devfn of the passed-in pdev. >> Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will >> stop querying other mediate ops and bind the opening device with this >> mediate ops using the returned mediate handle. >> >> Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the >> VF will be intercepted into VF mediate driver as >> vfio_pci_mediate_ops->get_region_info(), >> vfio_pci_mediate_ops->rw, >> vfio_pci_mediate_ops->mmap, and get customized. >> For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will >> further return 'pt' to indicate whether vfio-pci should further >> passthrough data to hw. >> >> when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() >> with a mediate handle as parameter. >> >> The mediate handle returned from vfio_pci_mediate_ops->open() lets VF >> mediate driver be able to differentiate two opening VFs of the same device >> id and vendor id. >> >> When VF mediate driver exits, it unregisters its mediate ops from >> vfio-pci. >> >> >> In this patchset, we enable vfio-pci to provide 3 things: >> (1) calling mediate ops to allow vendor driver customizing default >> region info/rw/mmap of a region. >> (2) provide a migration region to support migration > What's the benefit of introducing a region? It looks to me we don't expect > the region to be accessed directly from guest. Could we simply extend device > fd ioctl for doing such things? > You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/

generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way. So in mdev, do you still have a mediate driver? Or you expect the parent to implement the region?

No, currently it's only for vfio-pci. And specific to PCI.

...

mdev parent driver is free to customize its regions and hence does not requires this mediate ops hooks.

...

...

The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs. Well, at least you introduce new type of region in uapi. So this does not answer why region is better than ioctl. If the region will only be used by qemu, using ioctl is much more easier and straightforward.

It's not introduced by me :) mdev live migration is actually using this way, I'm just keeping compatible to the uapi.

I meant e.g VFIO_REGION_TYPE_MIGRATION.

here's the history of vfio live migration: https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05564.html https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

If you have any concern of this region way, feel free to comment to the latest v9 patchset: https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

The patchset here will always keep compatible to there.

Sure.

...

...

...

From my own perspective, my answer is that a region is more flexible compared to ioctl. vendor driver can freely define the size,

Probably not since it's an ABI I think.

that's why I need to define VFIO_REGION_TYPE_MIGRATION here in this patchset, as it's not upstreamed yet. maybe I should make it into a prerequisite patch, indicating it is not introduced by this patchset

Yes.

...

...

...

mmap cap of its data subregion.

It doesn't help much unless it can be mapped into guest (which I don't think it was the case here).

it's access by host qemu, the same as how linux app access an mmaped memory. the mmap here is to reduce memory copy from kernel to user. No need to get mapped into guest.

But copy_to_user() is not a bad choice. If I read the code correctly only the dirty bitmap was mmaped. This means you probably need to deal with dcache carefully on some archs. [1]

Note KVM doesn't use shared dirty bitmap, it uses copy_to_user().

[1] https://lkml.org/lkml/2019/4/9/5

on those platforms, mmap can be safely disabled by vendor driver at will. Also, when mmap is disabled, copy_to_user() is also used in region way. Any way, please raise you concern in kirti's thread for this common part.

...

...

...

Also, there're already too many ioctls in vfio. Probably not :) We had a brunch of  subsystems that have much more ioctls than VFIO. (e.g DRM)

...

...

...

>> (3) provide a dynamic trap bar info region to allow vendor driver >> control trap/untrap of device pci bars >> >> This vfio-pci + mediate ops way differs from mdev way in that >> (1) medv way needs to create a 1:1 mdev device on top of one VF, device >> specific mdev parent driver is bound to VF directly. >> (2) vfio-pci + mediate ops way does not create mdev devices and VF >> mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs. >> >> The reason why we don't choose the way of writing mdev parent driver is >> that >> (1) VFs are almost all the time directly passthroughed. Directly binding >> to vfio-pci can make most of the code shared/reused. > Can we split out the common parts from vfio-pci? > That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :) Well, I think there should be not hard to do that. E..g you can route it back to like:

vfio -> vfio_mdev -> parent -> vfio_pci

it's desired for us to have mediate driver binding to PF device. so once a VF device is created, only PF driver and vfio-pci are required. Just the same as what needs to be done for a normal VF passthrough. otherwise, a separate parent driver binding to VF is required. Also, this parent driver has many drawbacks as I mentions in this cover-letter. Well, as discussed, no need to duplicate the code, bar trick should still work. The main issues I saw with this proposal is:

1) PCI specific, other bus may need something similar vfio-pci is only for PCI of course.

I meant if what propose here makes sense, other bus driver like vfio-platform may want something similar.

sure they can follow.

...

...

2) Function duplicated with mdev and mdev can do even more

could you elaborate how mdev can do solve the above saying problem ?

Well, I think both of us agree the mdev can do what mediate ops did, mdev device implementation just need to add the direct PCI access part.

...

...

...

...

...

>> If we write a >> vendor specific mdev parent driver, most of the code (like passthrough >> style of rw/mmap) still needs to be copied from vfio-pci driver, which is >> actually a duplicated and tedious work. > The mediate ops looks quite similar to what vfio-mdev did. And it looks to > me we need to consider live migration for mdev as well. In that case, do we > still expect mediate ops through VFIO directly? > > >> (2) For features like dynamically trap/untrap pci bars, if they are in >> vfio-pci, they can be available to most people without repeated code >> copying and re-testing. >> (3) with a 1:1 mdev driver which passthrough VFs most of the time, people >> have to decide whether to bind VFs to vfio-pci or mdev parent driver before >> it runs into a real migration need. However, if vfio-pci is bound >> initially, they have no chance to do live migration when there's a need >> later. > We can teach management layer to do this. > No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now. I'm not quite sure I get here but in this case, just tech them to use the driver that has migration support?

That's a way, but as more and more passthrough devices have demands and caps to do migration, will vfio-pci be used in future any more ?

This should not be a problem: - If we introduce a common mdev for vfio-pci, we can just bind that driver always what is common mdev for vfio-pci? a common mdev parent driver that have the same implementation as vfio-pci?

The common part is not PCI of course. The common part is the both mdev and mediate ops want to do some kind of mediation. Mdev is bus agnostic, but what you propose here is PCI specific but should be bus agnostic as well. Assume we implement a bug agnostic mediate ops, mdev could be even built on top.

I believe Alex has already replied the above better than me.

...

There's actually already a solution of creating only one mdev on top of each passthrough device, and make mdev share the same iommu group with it. We've also made an implementation on it already. here's a sample one made by Yi at https://patchwork.kernel.org/cover/11134695/.

But, as I said, it's desired to re-use vfio-pci directly for SRIOV, which is straghtforward :)

Can we have a device that is capable of both SRIOV and function slicing? If yes, does it mean you need to provides two drivers? One for mdev, another for mediate ops?

what do you mean by "function slicing"? SIOV? For vendor driver, in SRIOV - with mdev approach, two drivers required: one for mdev parent driver on VF, one for PF driver. - with mediate ops + vfio-pci: one driver on PF. in SIOV, only one driver on PF in both case.

...

...

- The most straightforward way to support dirty page tracking is done by IOMMU instead of device specific operations.

No such IOMMU yet. And all kinds of platforms should be cared, right?

Or the device can track dirty pages by itself, otherwise it would be very hard to implement dirty page tracking correctly without the help of switching to software datapath (or maybe you can post the part of BAR0

I think you mixed "correct" and "accurate". DMA pre-inspection is a long existing term and we have implemented and verified it in NIC for both precopy and postcopy case. Though I can't promise there's 100% no bug, the method is right. Also, whether to trap BARs for dirty page is vendor specific and is not what should be cared about from this interface part.

mediation and dirty page tracking which is missed in this series?)

Currently, that part of code is owned by shaopeng's team. The code I posted is only for demonstrating how to use the interface. Shaopeng's team is responsible for upsteam of their part at their timing. Thanks Yan

...

...

...

...

...

> >> In this patchset, >> - patches 1-4 enable vfio-pci to call mediate ops registered by vendor >> driver to mediate/customize region info/rw/mmap. >> >> - patches 5-6 provide a standalone sample driver to register a mediate ops >> for Intel Graphics Devices. It does not bind to IGDs directly but decides >> what devices it supports via its pciidlist. It also demonstrates how to >> dynamic trap a device's PCI bars. (by adding more pciids in its >> pciidlist, this sample driver actually is not necessarily limited to >> support IGDs) >> >> - patch 7-9 provide a sample on i40e driver that supports Intel(R) >> Ethernet Controller XL710 Family of devices. It supports VF precopy live >> migration on Intel's 710 SRIOV. (but we commented out the real >> implementation of dirty page tracking and device state retrieving part >> to focus on demonstrating framework part. Will send out them in future >> versions) >> patch 7 registers/unregisters VF mediate ops when PF driver >> probes/removes. It specifies its supporting VFs via >> vfio_pci_mediate_ops->open(pdev) >> >> patch 8 reports device cap of VFIO_PCI_DEVICE_CAP_MIGRATION and >> provides a sample implementation of migration region. >> The QEMU part of vfio migration is based on v8 >> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg05542.html. >> We do not based on recent v9 because we think there are still opens in >> dirty page track part in that series. >> >> patch 9 reports device cap of VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR and >> provides an example on how to trap part of bar0 when migration starts >> and passthrough this part of bar0 again when migration fails. >> >> Yan Zhao (9): >> vfio/pci: introduce mediate ops to intercept vfio-pci ops >> vfio/pci: test existence before calling region->ops >> vfio/pci: register a default migration region >> vfio-pci: register default dynamic-trap-bar-info region >> samples/vfio-pci/igd_dt: sample driver to mediate a passthrough IGD >> sample/vfio-pci/igd_dt: dynamically trap/untrap subregion of IGD bar0 >> i40e/vf_migration: register mediate_ops to vfio-pci >> i40e/vf_migration: mediate migration region >> i40e/vf_migration: support dynamic trap of bar0 >> >> drivers/net/ethernet/intel/Kconfig | 2 +- >> drivers/net/ethernet/intel/i40e/Makefile | 3 +- >> drivers/net/ethernet/intel/i40e/i40e.h | 2 + >> drivers/net/ethernet/intel/i40e/i40e_main.c | 3 + >> .../ethernet/intel/i40e/i40e_vf_migration.c | 626 ++++++++++++++++++ >> .../ethernet/intel/i40e/i40e_vf_migration.h | 78 +++ >> drivers/vfio/pci/vfio_pci.c | 189 +++++- >> drivers/vfio/pci/vfio_pci_private.h | 2 + >> include/linux/vfio.h | 18 + >> include/uapi/linux/vfio.h | 160 +++++ >> samples/Kconfig | 6 + >> samples/Makefile | 1 + >> samples/vfio-pci/Makefile | 2 + >> samples/vfio-pci/igd_dt.c | 367 ++++++++++ >> 14 files changed, 1455 insertions(+), 4 deletions(-) >> create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.c >> create mode 100644 drivers/net/ethernet/intel/i40e/i40e_vf_migration.h >> create mode 100644 samples/vfio-pci/Makefile >> create mode 100644 samples/vfio-pci/igd_dt.c >>

On Fri, 6 Dec 2019 17:40:02 +0800 Jason Wang <jasowang@redhat.com> wrote:

On 2019/12/6 下午4:22, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

...

On 2019/12/5 下午4:51, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

...

Hi:

...

For SRIOV devices, VFs are passthroughed into guest directly without host driver mediation. However, when VMs migrating with passthroughed VFs, dynamic host mediation is required to (1) get device states, (2) get dirty pages. Since device states as well as other critical information required for dirty page tracking for VFs are usually retrieved from PFs, it is handy to provide an extension in PF driver to centralizingly control VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2) dynamically trap VFs' bars for dirty page tracking and A silly question, what's the reason for doing this, is this a must for dirty

On 2019/12/5 上午11:24, Yan Zhao wrote: page tracking?

For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need.

Right, but how does this matter for the case of dirty page tracking?

Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0.

Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case.

This sounds like a NIC specific solution, I believe the goal here is to allow any device type to implement a partial mediation solution, in this case to sufficiently track the device while in the migration saving state.

...

There's still no IOMMU Dirty bit available.

...

...

...

...

(3) centralizing VF critical states retrieving and VF controls into one driver, we propose to introduce mediate ops on top of current vfio-pci device driver.

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ __________ register mediate ops| ___________ ___________ | | |<-----------------------| VF | | | | vfio-pci | | | mediate | | PF driver | | |__________|----------------------->| driver | |___________| | open(pdev) | ----------- | | | | | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| \|/ \|/ ----------- ------------ | VF | | PF | ----------- ------------

VF mediate driver could be a standalone driver that does not bind to any devices (as in demo code in patches 5-6) or it could be a built-in extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate ops into vfio-pci in driver init. vfio-pci maintains a list of such mediate ops. (Note that: VF mediate driver can register mediate ops into vfio-pci before vfio-pci binding to any devices. And VF mediate driver can support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops list and calls each vfio_pci_mediate_ops->open() with pdev of the opening device as a parameter. VF mediate driver should return success or failure depending on it supports the pdev or not. E.g. VF mediate driver would compare its supported VF devfn with the devfn of the passed-in pdev. Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will stop querying other mediate ops and bind the opening device with this mediate ops using the returned mediate handle.

Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the VF will be intercepted into VF mediate driver as vfio_pci_mediate_ops->get_region_info(), vfio_pci_mediate_ops->rw, vfio_pci_mediate_ops->mmap, and get customized. For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will further return 'pt' to indicate whether vfio-pci should further passthrough data to hw.

when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() with a mediate handle as parameter.

The mediate handle returned from vfio_pci_mediate_ops->open() lets VF mediate driver be able to differentiate two opening VFs of the same device id and vendor id.

When VF mediate driver exits, it unregisters its mediate ops from vfio-pci.

In this patchset, we enable vfio-pci to provide 3 things: (1) calling mediate ops to allow vendor driver customizing default region info/rw/mmap of a region. (2) provide a migration region to support migration

What's the benefit of introducing a region? It looks to me we don't expect the region to be accessed directly from guest. Could we simply extend device fd ioctl for doing such things?

You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/

generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way.

So in mdev, do you still have a mediate driver? Or you expect the parent to implement the region?

No, currently it's only for vfio-pci.

And specific to PCI.

What's PCI specific? The implementation, yes, it's done in the bus vfio bus driver here but all device access is performed by the bus driver. I'm not sure how we could introduce the intercept at the vfio-core level, but I'm open to suggestions.

...

mdev parent driver is free to customize its regions and hence does not requires this mediate ops hooks.

...

...

The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs.

Well, at least you introduce new type of region in uapi. So this does not answer why region is better than ioctl. If the region will only be used by qemu, using ioctl is much more easier and straightforward.

It's not introduced by me :) mdev live migration is actually using this way, I'm just keeping compatible to the uapi.

I meant e.g VFIO_REGION_TYPE_MIGRATION.

...

From my own perspective, my answer is that a region is more flexible compared to ioctl. vendor driver can freely define the size,

Probably not since it's an ABI I think.

I think Kirti's thread proposing the migration interface is a better place for this discussion, I believe Yan has already linked to it. In general we prefer to be frugal in our introduction of new ioctls, especially when we have existing mechanisms via regions to support the interactions. The interface is designed to be flexible to the vendor driver needs, partially thanks to it being a region.

...

mmap cap of its data subregion.

It doesn't help much unless it can be mapped into guest (which I don't think it was the case here).

...

Also, there're already too many ioctls in vfio.

Probably not :) We had a brunch of  subsystems that have much more ioctls than VFIO. (e.g DRM)

And this is a good thing? We can more easily deprecate and revise region support than we can take back ioctls that have been previously used. I generally don't like the "let's create a new ioctl for that" approach versus trying to fit something within the existing architecture and convention.

...

...

...

...

...

(3) provide a dynamic trap bar info region to allow vendor driver control trap/untrap of device pci bars

This vfio-pci + mediate ops way differs from mdev way in that (1) medv way needs to create a 1:1 mdev device on top of one VF, device specific mdev parent driver is bound to VF directly. (2) vfio-pci + mediate ops way does not create mdev devices and VF mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs.

The reason why we don't choose the way of writing mdev parent driver is that (1) VFs are almost all the time directly passthroughed. Directly binding to vfio-pci can make most of the code shared/reused. Can we split out the common parts from vfio-pci?

That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :)

Well, I think there should be not hard to do that. E..g you can route it back to like:

vfio -> vfio_mdev -> parent -> vfio_pci

it's desired for us to have mediate driver binding to PF device. so once a VF device is created, only PF driver and vfio-pci are required. Just the same as what needs to be done for a normal VF passthrough. otherwise, a separate parent driver binding to VF is required. Also, this parent driver has many drawbacks as I mentions in this cover-letter.

Well, as discussed, no need to duplicate the code, bar trick should still work. The main issues I saw with this proposal is:

1) PCI specific, other bus may need something similar

Propose how it could be implemented higher in the vfio stack to make it device agnostic.

2) Function duplicated with mdev and mdev can do even more

mdev also comes with a device lifecycle interface that doesn't really make sense when a driver is only trying to partially mediate a single physical device rather than multiplex a physical device into virtual devices. mdev would also require vendor drivers to re-implement much of vfio-pci for the direct access mechanisms. Also, do we really want users or management tools to decide between binding a device to vfio-pci or a separate mdev driver to get this functionality. We've already been burnt trying to use mdev beyond its scope.

...

...

...

...

...

If we write a vendor specific mdev parent driver, most of the code (like passthrough style of rw/mmap) still needs to be copied from vfio-pci driver, which is actually a duplicated and tedious work.

The mediate ops looks quite similar to what vfio-mdev did. And it looks to me we need to consider live migration for mdev as well. In that case, do we still expect mediate ops through VFIO directly?

...

(2) For features like dynamically trap/untrap pci bars, if they are in vfio-pci, they can be available to most people without repeated code copying and re-testing. (3) with a 1:1 mdev driver which passthrough VFs most of the time, people have to decide whether to bind VFs to vfio-pci or mdev parent driver before it runs into a real migration need. However, if vfio-pci is bound initially, they have no chance to do live migration when there's a need later. We can teach management layer to do this.

No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now.

I'm not quite sure I get here but in this case, just tech them to use the driver that has migration support?

That's a way, but as more and more passthrough devices have demands and caps to do migration, will vfio-pci be used in future any more ?

This should not be a problem: - If we introduce a common mdev for vfio-pci, we can just bind that driver always

There's too much of mdev that doesn't make sense for this usage model, this is why Yi's proposed generic mdev PCI wrapper is only a sample driver. I think we do not want to introduce user confusion regarding which driver to use and there are outstanding non-singleton group issues with mdev that don't seem worthwhile to resolve.

- The most straightforward way to support dirty page tracking is done by IOMMU instead of device specific operations.

Of course, but it doesn't exist yet. We're attempting to design the dirty page tracking in a way that's mostly transparent for current mdev drivers, would provide generic support for IOMMU-based dirty tracking, and extensible to the inevitability of vendor driver tracking. Thanks, Alex

On Thu, 12 Dec 2019 12:09:48 +0800 Jason Wang <jasowang@redhat.com> wrote:

On 2019/12/7 上午1:42, Alex Williamson wrote:

...

On Fri, 6 Dec 2019 17:40:02 +0800 Jason Wang <jasowang@redhat.com> wrote:

...

On 2019/12/6 下午4:22, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

...

On 2019/12/5 下午4:51, Yan Zhao wrote:

...

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote: > Hi: > > On 2019/12/5 上午11:24, Yan Zhao wrote: >> For SRIOV devices, VFs are passthroughed into guest directly without host >> driver mediation. However, when VMs migrating with passthroughed VFs, >> dynamic host mediation is required to (1) get device states, (2) get >> dirty pages. Since device states as well as other critical information >> required for dirty page tracking for VFs are usually retrieved from PFs, >> it is handy to provide an extension in PF driver to centralizingly control >> VFs' migration. >> >> Therefore, in order to realize (1) passthrough VFs at normal time, (2) >> dynamically trap VFs' bars for dirty page tracking and > A silly question, what's the reason for doing this, is this a must for dirty > page tracking? > For performance consideration. VFs' bars should be passthoughed at normal time and only enter into trap state on need. Right, but how does this matter for the case of dirty page tracking?

Take NIC as an example, to trap its VF dirty pages, software way is required to trap every write of ring tail that resides in BAR0.

Interesting, but it looks like we need: - decode the instruction - mediate all access to BAR0 All of which seems a great burden for the VF driver. I wonder whether or not doing interrupt relay and tracking head is better in this case. This sounds like a NIC specific solution, I believe the goal here is to allow any device type to implement a partial mediation solution, in this case to sufficiently track the device while in the migration saving state.

I suspect there's a solution that can work for any device type. E.g for virtio, avail index (head) doesn't belongs to any BAR and device may decide to disable doorbell from guest. So did interrupt relay since driver may choose to disable interrupt from device. In this case, the only way to track dirty pages correctly is to switch to software datapath.

...

...

...

There's still no IOMMU Dirty bit available.

...

...

>> (3) centralizing >> VF critical states retrieving and VF controls into one driver, we propose >> to introduce mediate ops on top of current vfio-pci device driver. >> >> >> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ >> __________ register mediate ops| ___________ ___________ | >> | |<-----------------------| VF | | | >> | vfio-pci | | | mediate | | PF driver | | >> |__________|----------------------->| driver | |___________| >> | open(pdev) | ----------- | | >> | | >> | |_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _| >> \|/ \|/ >> ----------- ------------ >> | VF | | PF | >> ----------- ------------ >> >> >> VF mediate driver could be a standalone driver that does not bind to >> any devices (as in demo code in patches 5-6) or it could be a built-in >> extension of PF driver (as in patches 7-9) . >> >> Rather than directly bind to VF, VF mediate driver register a mediate >> ops into vfio-pci in driver init. vfio-pci maintains a list of such >> mediate ops. >> (Note that: VF mediate driver can register mediate ops into vfio-pci >> before vfio-pci binding to any devices. And VF mediate driver can >> support mediating multiple devices.) >> >> When opening a device (e.g. a VF), vfio-pci goes through the mediate ops >> list and calls each vfio_pci_mediate_ops->open() with pdev of the opening >> device as a parameter. >> VF mediate driver should return success or failure depending on it >> supports the pdev or not. >> E.g. VF mediate driver would compare its supported VF devfn with the >> devfn of the passed-in pdev. >> Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will >> stop querying other mediate ops and bind the opening device with this >> mediate ops using the returned mediate handle. >> >> Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the >> VF will be intercepted into VF mediate driver as >> vfio_pci_mediate_ops->get_region_info(), >> vfio_pci_mediate_ops->rw, >> vfio_pci_mediate_ops->mmap, and get customized. >> For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will >> further return 'pt' to indicate whether vfio-pci should further >> passthrough data to hw. >> >> when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release() >> with a mediate handle as parameter. >> >> The mediate handle returned from vfio_pci_mediate_ops->open() lets VF >> mediate driver be able to differentiate two opening VFs of the same device >> id and vendor id. >> >> When VF mediate driver exits, it unregisters its mediate ops from >> vfio-pci. >> >> >> In this patchset, we enable vfio-pci to provide 3 things: >> (1) calling mediate ops to allow vendor driver customizing default >> region info/rw/mmap of a region. >> (2) provide a migration region to support migration > What's the benefit of introducing a region? It looks to me we don't expect > the region to be accessed directly from guest. Could we simply extend device > fd ioctl for doing such things? > You may take a look on mdev live migration discussions in https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html, which has kernel side implemetation https://patchwork.freedesktop.org/series/56876/

generaly speaking, qemu part of live migration is consistent for vfio-pci + mediate ops way or mdev way. So in mdev, do you still have a mediate driver? Or you expect the parent to implement the region?

No, currently it's only for vfio-pci. And specific to PCI. What's PCI specific? The implementation, yes, it's done in the bus vfio bus driver here but all device access is performed by the bus driver. I'm not sure how we could introduce the intercept at the vfio-core level, but I'm open to suggestions.

I haven't thought this too much, but if we can intercept at core level, it basically can do what mdev can do right now.

An intercept at the core level is essentially a new vfio bus driver.

...

...

...

mdev parent driver is free to customize its regions and hence does not requires this mediate ops hooks.

...

...

The region is only a channel for QEMU and kernel to communicate information without introducing IOCTLs. Well, at least you introduce new type of region in uapi. So this does not answer why region is better than ioctl. If the region will only be used by qemu, using ioctl is much more easier and straightforward.

It's not introduced by me :) mdev live migration is actually using this way, I'm just keeping compatible to the uapi.

I meant e.g VFIO_REGION_TYPE_MIGRATION.

...

From my own perspective, my answer is that a region is more flexible compared to ioctl. vendor driver can freely define the size,

Probably not since it's an ABI I think. I think Kirti's thread proposing the migration interface is a better place for this discussion, I believe Yan has already linked to it. In general we prefer to be frugal in our introduction of new ioctls, especially when we have existing mechanisms via regions to support the interactions. The interface is designed to be flexible to the vendor driver needs, partially thanks to it being a region.

...

...

mmap cap of its data subregion.

It doesn't help much unless it can be mapped into guest (which I don't think it was the case here). /

...

Also, there're already too many ioctls in vfio. Probably not :) We had a brunch of  subsystems that have much more ioctls than VFIO. (e.g DRM) And this is a good thing?

Well, I just meant that "having too much ioctls already" is not a good reason for not introducing new ones.

Avoiding ioctl proliferation is a reason to require a high bar for any new ioctl though. Push back on every ioctl and maybe we won't get to that point.

...

We can more easily deprecate and revise region support than we can take back ioctls that have been previously used.

It belongs to uapi, how easily can we deprecate that?

I'm not saying there shouldn't be a deprecation process, but the core uapi for vfio remains (relatively) unchanged. The user has a protocol for discovering the features of a device and if we decide we've screwed up the implementation of the migration_region-v1 we can simply add a migration_region-v2 and both can be exposed via the same core ioctls until we decide to no longer expose v1. Our address space of region types is defined within vfio, not shared with every driver in the kernel. The "add an ioctl for that" approach is not the model I advocate.

...

I generally don't like the "let's create a new ioctl for that" approach versus trying to fit something within the existing architecture and convention.

...

...

...

...

>> (3) provide a dynamic trap bar info region to allow vendor driver >> control trap/untrap of device pci bars >> >> This vfio-pci + mediate ops way differs from mdev way in that >> (1) medv way needs to create a 1:1 mdev device on top of one VF, device >> specific mdev parent driver is bound to VF directly. >> (2) vfio-pci + mediate ops way does not create mdev devices and VF >> mediate driver does not bind to VFs. Instead, vfio-pci binds to VFs. >> >> The reason why we don't choose the way of writing mdev parent driver is >> that >> (1) VFs are almost all the time directly passthroughed. Directly binding >> to vfio-pci can make most of the code shared/reused. > Can we split out the common parts from vfio-pci? > That's very attractive. but one cannot implement a vfio-pci except export everything in it as common part :) Well, I think there should be not hard to do that. E..g you can route it back to like:

vfio -> vfio_mdev -> parent -> vfio_pci

it's desired for us to have mediate driver binding to PF device. so once a VF device is created, only PF driver and vfio-pci are required. Just the same as what needs to be done for a normal VF passthrough. otherwise, a separate parent driver binding to VF is required. Also, this parent driver has many drawbacks as I mentions in this cover-letter. Well, as discussed, no need to duplicate the code, bar trick should still work. The main issues I saw with this proposal is:

1) PCI specific, other bus may need something similar Propose how it could be implemented higher in the vfio stack to make it device agnostic.

E.g doing it in vfio_device_fops instead of vfio_pci_ops?

Which is essentially a new vfio bus driver. This is something vfio has supported since day one. Issues with doing that here are that it puts the burden on the mediation/vendor driver to re-implement or re-use a lot of existing code in vfio-pci, and I think it creates user confusion around which driver to use for what feature set when using a device through vfio. You're complaining this series is PCI specific, when re-using the vfio-pci code is exactly what we're trying to achieve. Other bus types can do something similar and injecting vendor specific drivers a layer above the bus driver is already a fundamental part of the infrastructure.

...

...

2) Function duplicated with mdev and mdev can do even more mdev also comes with a device lifecycle interface that doesn't really make sense when a driver is only trying to partially mediate a single physical device rather than multiplex a physical device into virtual devices.

Yes, but that part could be decoupled out of mdev.

There would be nothing left. vfio-mdev is essentially nothing more than a vfio bus driver that forwards through to mdev to provide that lifecycle interface to the vendor driver. Without that, it's just another vfio bus driver.

...

mdev would also require vendor drivers to re-implement much of vfio-pci for the direct access mechanisms. Also, do we really want users or management tools to decide between binding a device to vfio-pci or a separate mdev driver to get this functionality. We've already been burnt trying to use mdev beyond its scope.

The problem is, if we had a device that support both SRIOV and mdev. Does this mean we need prepare two set of drivers?

We have this situation today, modulo SR-IOV, but that's a red herring anyway, VF vs PF is irrelevant. For example we can either directly assign IGD graphics to a VM with vfio-pci or we can enable GVT-g support in the i915 driver, which registers vGPU support via mdev. These are different use cases, expose different features, and have different support models. NVIDIA is the same way, assigning a GPU via vfio-pci or a vGPU via vfio-mdev are entirely separate usage models. Once we use mdev, it's at the vendor driver's discretion how the device resources are backed, they might make use of the resource isolation of SR-IOV or they might divide a single function. If your question is whether there's a concern around proliferation of vfio bus drivers and user confusion over which to use for what features, yes, absolutely. I think this is why we're starting with seeing what it looks like to add mediation to vfio-pci rather than modularize vfio-pci and ask Intel to develop a new vfio-pci-intel-dsa driver. I'm not yet convinced we won't eventually come back to that latter approach though if this initial draft is what we can expect of a mediated vfio-pci.

...

...

...

...

...

>> If we write a >> vendor specific mdev parent driver, most of the code (like passthrough >> style of rw/mmap) still needs to be copied from vfio-pci driver, which is >> actually a duplicated and tedious work. > The mediate ops looks quite similar to what vfio-mdev did. And it looks to > me we need to consider live migration for mdev as well. In that case, do we > still expect mediate ops through VFIO directly? > > >> (2) For features like dynamically trap/untrap pci bars, if they are in >> vfio-pci, they can be available to most people without repeated code >> copying and re-testing. >> (3) with a 1:1 mdev driver which passthrough VFs most of the time, people >> have to decide whether to bind VFs to vfio-pci or mdev parent driver before >> it runs into a real migration need. However, if vfio-pci is bound >> initially, they have no chance to do live migration when there's a need >> later. > We can teach management layer to do this. > No. not possible as vfio-pci by default has no migration region and dirty page tracking needs vendor's mediation at least for most passthrough devices now. I'm not quite sure I get here but in this case, just tech them to use the driver that has migration support?

That's a way, but as more and more passthrough devices have demands and caps to do migration, will vfio-pci be used in future any more ?

This should not be a problem: - If we introduce a common mdev for vfio-pci, we can just bind that driver always There's too much of mdev that doesn't make sense for this usage model, this is why Yi's proposed generic mdev PCI wrapper is only a sample driver. I think we do not want to introduce user confusion regarding which driver to use and there are outstanding non-singleton group issues with mdev that don't seem worthwhile to resolve.

I agree, but I think what user want is a unified driver that works for both SRIOV and mdev. That's why trying to have a common way for doing mediation may make sense.

I don't think we can get to one driver, nor is it clear to me that we should. Direct assignment and mdev currently provide different usage models. Both are valid, both are useful. That said, I don't necessarily want a user to need to choose whether to bind a device to vfio-pci for base functionality or vfio-pci-vendor-foo for extended functionality either. I think that's why we're exploring this mediation approach and there's already precedent in vfio-pci for some extent of device specific support. It's still a question though whether it can be done clean enough to make it worthwhile. Thanks, Alex