mbox series

[v3,0/4] virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG

Message ID 20220214124356.872985-1-jean-philippe@linaro.org
Headers show
Series virtio-iommu: Support VIRTIO_IOMMU_F_BYPASS_CONFIG | expand

Message

Jean-Philippe Brucker Feb. 14, 2022, 12:43 p.m. UTC
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/

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

Comments

Cornelia Huck Feb. 14, 2022, 5:34 p.m. UTC | #1
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.
Eric Auger Feb. 15, 2022, 9:16 a.m. UTC | #2
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.
>
Eric Auger Feb. 15, 2022, 9:25 a.m. UTC | #3
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
>
Jean-Philippe Brucker Feb. 15, 2022, 9:48 a.m. UTC | #4
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
Jean-Philippe Brucker Feb. 15, 2022, 9:49 a.m. UTC | #5
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
Cornelia Huck Feb. 15, 2022, 11:58 a.m. UTC | #6
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)
Jean-Philippe Brucker March 3, 2022, 11:41 a.m. UTC | #7
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
>