| Message ID | 20220214124356.872985-1-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG | expand |
On Mon, Feb 14 2022, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > Replace the VIRTIO_IOMMU_F_BYPASS feature with > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch > global bypass on and off. > > Add a boot-bypass option, which defaults to 'on' to be in line with > other vIOMMUs and to allow running firmware/bootloader that are unaware > of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu > anymore. > > Since v2 [1]: > * Added the new bypass bits to the migration stream. > As discussed on the v2 thread, we assume that cross-version > compatibility is not required for live migration at the moment, so we > only increase the version number. Patch 2 says: "We add the bypass > field to the migration stream without introducing subsections, based > on the assumption that this virtio-iommu device isn't being used in > production enough to require cross-version migration at the moment > (all previous version required workarounds since they didn't support > ACPI and boot-bypass)." > > [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ One thing that we could do to avoid surprises in the unlikely case that somebody has a virtio-iommu device and wants to migrate to an older machine version is to add a migration blocker for the virtio-iommu device for all compat machines for versions 6.2 or older (i.e. only 7.0 or newer machine types can have a migratable virtio-iommu device starting with QEMU 7.0.) Not too complicated to implement, but I'm not sure whether we'd add too much code to prevent something very unlikely to happen anyway. I would not insist on it :) Otherwise, I didn't spot problems in this series.
Hi Connie, On 2/14/22 6:34 PM, Cornelia Huck wrote: > On Mon, Feb 14 2022, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > >> Replace the VIRTIO_IOMMU_F_BYPASS feature with >> VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch >> global bypass on and off. >> >> Add a boot-bypass option, which defaults to 'on' to be in line with >> other vIOMMUs and to allow running firmware/bootloader that are unaware >> of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu >> anymore. >> >> Since v2 [1]: >> * Added the new bypass bits to the migration stream. >> As discussed on the v2 thread, we assume that cross-version >> compatibility is not required for live migration at the moment, so we >> only increase the version number. Patch 2 says: "We add the bypass >> field to the migration stream without introducing subsections, based >> on the assumption that this virtio-iommu device isn't being used in >> production enough to require cross-version migration at the moment >> (all previous version required workarounds since they didn't support >> ACPI and boot-bypass)." >> >> [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ > One thing that we could do to avoid surprises in the unlikely case that > somebody has a virtio-iommu device and wants to migrate to an older > machine version is to add a migration blocker for the virtio-iommu > device for all compat machines for versions 6.2 or older (i.e. only 7.0 > or newer machine types can have a migratable virtio-iommu device > starting with QEMU 7.0.) Not too complicated to implement, but I'm not > sure whether we'd add too much code to prevent something very unlikely > to happen anyway. I would not insist on it :) As nobody has shout and we are not aware of anybody using the device in production mode yet due to the missing boot bypass feature this series brings, I would be personally in favour of leaving things as is. Now, up to Jean if he wants to go and implement your suggestion. Thanks Eric > > Otherwise, I didn't spot problems in this series. >
Hi Jean, On 2/14/22 1:43 PM, Jean-Philippe Brucker wrote: > Replace the VIRTIO_IOMMU_F_BYPASS feature with > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch > global bypass on and off. > > Add a boot-bypass option, which defaults to 'on' to be in line with > other vIOMMUs and to allow running firmware/bootloader that are unaware > of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu > anymore. > > Since v2 [1]: > * Added the new bypass bits to the migration stream. > As discussed on the v2 thread, we assume that cross-version > compatibility is not required for live migration at the moment, so we > only increase the version number. Patch 2 says: "We add the bypass > field to the migration stream without introducing subsections, based > on the assumption that this virtio-iommu device isn't being used in > production enough to require cross-version migration at the moment > (all previous version required workarounds since they didn't support > ACPI and boot-bypass)." > > [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ Reviewed-by: Eric Auger <eric.auger@redhat.com> Tested-by: Eric Auger <eric.auger@redhat.com> I tested both the boot bypass feature with virtio-blk-pci, virtio-net-pci and virtio-gpu-pci and migration. Thanks Eric > > Jean-Philippe Brucker (4): > linux-headers: update to v5.17-rc1 > virtio-iommu: Default to bypass during boot > virtio-iommu: Support bypass domain > tests/qtest/virtio-iommu-test: Check bypass config > > include/hw/virtio/virtio-iommu.h | 1 + > include/standard-headers/asm-x86/kvm_para.h | 1 + > include/standard-headers/drm/drm_fourcc.h | 11 ++ > include/standard-headers/linux/ethtool.h | 1 + > include/standard-headers/linux/fuse.h | 60 +++++++- > include/standard-headers/linux/pci_regs.h | 142 +++++++++--------- > include/standard-headers/linux/virtio_gpio.h | 72 +++++++++ > include/standard-headers/linux/virtio_i2c.h | 47 ++++++ > include/standard-headers/linux/virtio_iommu.h | 8 +- > .../standard-headers/linux/virtio_pcidev.h | 65 ++++++++ > include/standard-headers/linux/virtio_scmi.h | 24 +++ > linux-headers/asm-generic/unistd.h | 5 +- > linux-headers/asm-mips/unistd_n32.h | 2 + > linux-headers/asm-mips/unistd_n64.h | 2 + > linux-headers/asm-mips/unistd_o32.h | 2 + > linux-headers/asm-powerpc/unistd_32.h | 2 + > linux-headers/asm-powerpc/unistd_64.h | 2 + > linux-headers/asm-riscv/bitsperlong.h | 14 ++ > linux-headers/asm-riscv/mman.h | 1 + > linux-headers/asm-riscv/unistd.h | 44 ++++++ > linux-headers/asm-s390/unistd_32.h | 2 + > linux-headers/asm-s390/unistd_64.h | 2 + > linux-headers/asm-x86/kvm.h | 16 +- > linux-headers/asm-x86/unistd_32.h | 1 + > linux-headers/asm-x86/unistd_64.h | 1 + > linux-headers/asm-x86/unistd_x32.h | 1 + > linux-headers/linux/kvm.h | 17 +++ > hw/virtio/virtio-iommu.c | 99 ++++++++++-- > tests/qtest/virtio-iommu-test.c | 2 + > hw/virtio/trace-events | 4 +- > 30 files changed, 561 insertions(+), 90 deletions(-) > create mode 100644 include/standard-headers/linux/virtio_gpio.h > create mode 100644 include/standard-headers/linux/virtio_i2c.h > create mode 100644 include/standard-headers/linux/virtio_pcidev.h > create mode 100644 include/standard-headers/linux/virtio_scmi.h > create mode 100644 linux-headers/asm-riscv/bitsperlong.h > create mode 100644 linux-headers/asm-riscv/mman.h > create mode 100644 linux-headers/asm-riscv/unistd.h >
On Tue, Feb 15, 2022 at 10:16:40AM +0100, Eric Auger wrote: > Hi Connie, > > On 2/14/22 6:34 PM, Cornelia Huck wrote: > > On Mon, Feb 14 2022, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > > >> Replace the VIRTIO_IOMMU_F_BYPASS feature with > >> VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch > >> global bypass on and off. > >> > >> Add a boot-bypass option, which defaults to 'on' to be in line with > >> other vIOMMUs and to allow running firmware/bootloader that are unaware > >> of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu > >> anymore. > >> > >> Since v2 [1]: > >> * Added the new bypass bits to the migration stream. > >> As discussed on the v2 thread, we assume that cross-version > >> compatibility is not required for live migration at the moment, so we > >> only increase the version number. Patch 2 says: "We add the bypass > >> field to the migration stream without introducing subsections, based > >> on the assumption that this virtio-iommu device isn't being used in > >> production enough to require cross-version migration at the moment > >> (all previous version required workarounds since they didn't support > >> ACPI and boot-bypass)." > >> > >> [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ > > One thing that we could do to avoid surprises in the unlikely case that > > somebody has a virtio-iommu device and wants to migrate to an older > > machine version is to add a migration blocker for the virtio-iommu > > device for all compat machines for versions 6.2 or older (i.e. only 7.0 > > or newer machine types can have a migratable virtio-iommu device > > starting with QEMU 7.0.) Not too complicated to implement, but I'm not > > sure whether we'd add too much code to prevent something very unlikely > > to happen anyway. I would not insist on it :) > As nobody has shout and we are not aware of anybody using the device in > production mode yet due to the missing boot bypass feature this series > brings, I would be personally in favour of leaving things as is. Now, up > to Jean if he wants to go and implement your suggestion. I agree, it seems too unlikely that someone would want to migrate it back to 6.2 where it wasn't really useable except for experiments Thanks, Jean
On Tue, Feb 15, 2022 at 10:25:21AM +0100, Eric Auger wrote: > Hi Jean, > > On 2/14/22 1:43 PM, Jean-Philippe Brucker wrote: > > Replace the VIRTIO_IOMMU_F_BYPASS feature with > > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch > > global bypass on and off. > > > > Add a boot-bypass option, which defaults to 'on' to be in line with > > other vIOMMUs and to allow running firmware/bootloader that are unaware > > of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu > > anymore. > > > > Since v2 [1]: > > * Added the new bypass bits to the migration stream. > > As discussed on the v2 thread, we assume that cross-version > > compatibility is not required for live migration at the moment, so we > > only increase the version number. Patch 2 says: "We add the bypass > > field to the migration stream without introducing subsections, based > > on the assumption that this virtio-iommu device isn't being used in > > production enough to require cross-version migration at the moment > > (all previous version required workarounds since they didn't support > > ACPI and boot-bypass)." > > > > [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Tested-by: Eric Auger <eric.auger@redhat.com> > > I tested both the boot bypass feature with virtio-blk-pci, > virtio-net-pci and virtio-gpu-pci and migration. Thanks! Jean
On Tue, Feb 15 2022, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > On Tue, Feb 15, 2022 at 10:16:40AM +0100, Eric Auger wrote: >> Hi Connie, >> >> On 2/14/22 6:34 PM, Cornelia Huck wrote: >> > On Mon, Feb 14 2022, Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: >> > >> >> Replace the VIRTIO_IOMMU_F_BYPASS feature with >> >> VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch >> >> global bypass on and off. >> >> >> >> Add a boot-bypass option, which defaults to 'on' to be in line with >> >> other vIOMMUs and to allow running firmware/bootloader that are unaware >> >> of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu >> >> anymore. >> >> >> >> Since v2 [1]: >> >> * Added the new bypass bits to the migration stream. >> >> As discussed on the v2 thread, we assume that cross-version >> >> compatibility is not required for live migration at the moment, so we >> >> only increase the version number. Patch 2 says: "We add the bypass >> >> field to the migration stream without introducing subsections, based >> >> on the assumption that this virtio-iommu device isn't being used in >> >> production enough to require cross-version migration at the moment >> >> (all previous version required workarounds since they didn't support >> >> ACPI and boot-bypass)." >> >> >> >> [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ >> > One thing that we could do to avoid surprises in the unlikely case that >> > somebody has a virtio-iommu device and wants to migrate to an older >> > machine version is to add a migration blocker for the virtio-iommu >> > device for all compat machines for versions 6.2 or older (i.e. only 7.0 >> > or newer machine types can have a migratable virtio-iommu device >> > starting with QEMU 7.0.) Not too complicated to implement, but I'm not >> > sure whether we'd add too much code to prevent something very unlikely >> > to happen anyway. I would not insist on it :) >> As nobody has shout and we are not aware of anybody using the device in >> production mode yet due to the missing boot bypass feature this series >> brings, I would be personally in favour of leaving things as is. Now, up >> to Jean if he wants to go and implement your suggestion. > > I agree, it seems too unlikely that someone would want to migrate it back > to 6.2 where it wasn't really useable except for experiments Fair enough. Let's make this an Acked-by: Cornelia Huck <cohuck@redhat.com> (for the series)
Hello, On Mon, Feb 14, 2022 at 12:43:52PM +0000, Jean-Philippe Brucker wrote: > Replace the VIRTIO_IOMMU_F_BYPASS feature with > VIRTIO_IOMMU_F_BYPASS_CONFIG, which enables a config space bit to switch > global bypass on and off. > > Add a boot-bypass option, which defaults to 'on' to be in line with > other vIOMMUs and to allow running firmware/bootloader that are unaware > of the IOMMU. x86 doesn't need a workaround to boot with virtio-iommu > anymore. I just wanted to make sure this doesn't fall through, since it is ready for v7.0 Thanks, Jean > > Since v2 [1]: > * Added the new bypass bits to the migration stream. > As discussed on the v2 thread, we assume that cross-version > compatibility is not required for live migration at the moment, so we > only increase the version number. Patch 2 says: "We add the bypass > field to the migration stream without introducing subsections, based > on the assumption that this virtio-iommu device isn't being used in > production enough to require cross-version migration at the moment > (all previous version required workarounds since they didn't support > ACPI and boot-bypass)." > > [1] https://lore.kernel.org/qemu-devel/20220127142940.671333-1-jean-philippe@linaro.org/ > > Jean-Philippe Brucker (4): > linux-headers: update to v5.17-rc1 > virtio-iommu: Default to bypass during boot > virtio-iommu: Support bypass domain > tests/qtest/virtio-iommu-test: Check bypass config > > include/hw/virtio/virtio-iommu.h | 1 + > include/standard-headers/asm-x86/kvm_para.h | 1 + > include/standard-headers/drm/drm_fourcc.h | 11 ++ > include/standard-headers/linux/ethtool.h | 1 + > include/standard-headers/linux/fuse.h | 60 +++++++- > include/standard-headers/linux/pci_regs.h | 142 +++++++++--------- > include/standard-headers/linux/virtio_gpio.h | 72 +++++++++ > include/standard-headers/linux/virtio_i2c.h | 47 ++++++ > include/standard-headers/linux/virtio_iommu.h | 8 +- > .../standard-headers/linux/virtio_pcidev.h | 65 ++++++++ > include/standard-headers/linux/virtio_scmi.h | 24 +++ > linux-headers/asm-generic/unistd.h | 5 +- > linux-headers/asm-mips/unistd_n32.h | 2 + > linux-headers/asm-mips/unistd_n64.h | 2 + > linux-headers/asm-mips/unistd_o32.h | 2 + > linux-headers/asm-powerpc/unistd_32.h | 2 + > linux-headers/asm-powerpc/unistd_64.h | 2 + > linux-headers/asm-riscv/bitsperlong.h | 14 ++ > linux-headers/asm-riscv/mman.h | 1 + > linux-headers/asm-riscv/unistd.h | 44 ++++++ > linux-headers/asm-s390/unistd_32.h | 2 + > linux-headers/asm-s390/unistd_64.h | 2 + > linux-headers/asm-x86/kvm.h | 16 +- > linux-headers/asm-x86/unistd_32.h | 1 + > linux-headers/asm-x86/unistd_64.h | 1 + > linux-headers/asm-x86/unistd_x32.h | 1 + > linux-headers/linux/kvm.h | 17 +++ > hw/virtio/virtio-iommu.c | 99 ++++++++++-- > tests/qtest/virtio-iommu-test.c | 2 + > hw/virtio/trace-events | 4 +- > 30 files changed, 561 insertions(+), 90 deletions(-) > create mode 100644 include/standard-headers/linux/virtio_gpio.h > create mode 100644 include/standard-headers/linux/virtio_i2c.h > create mode 100644 include/standard-headers/linux/virtio_pcidev.h > create mode 100644 include/standard-headers/linux/virtio_scmi.h > create mode 100644 linux-headers/asm-riscv/bitsperlong.h > create mode 100644 linux-headers/asm-riscv/mman.h > create mode 100644 linux-headers/asm-riscv/unistd.h > > -- > 2.35.1 >