mbox series

[v4,00/11] virtio-iommu: Add ACPI support

Message ID 20211001173358.863017-1-jean-philippe@linaro.org (mailing list archive)
Headers show
Series virtio-iommu: Add ACPI support | expand

Message

Jean-Philippe Brucker Oct. 1, 2021, 5:33 p.m. UTC
Allow instantiating a virtio-iommu device on ACPI systems by adding a
Virtual I/O Translation table (VIOT). Enable x86 support for VIOT.

Changes since v3 [1]:
* Cleaned the IOMMU-uniqueness checks. Added patch 6 to have a
  single check on x86.
* Added patch 5 that allows to gracefully propagate errors when setting
  resv_mem properties.
* Squashed table blobs into patch 11.
* Addressed all other comments from v3.

Caveats:

* Series depends on "acpi: refactor error prone build_header() and
  packed structures usage in ACPI tables" [2]

* Since virtio-iommu doesn't support boot-bypass at the moment, firmware
  can't access storage behind the IOMMU to load bootloader or kernel.
  This will be solved by another series currently in flight [3]. In the
  meantime you can use a storage device that bypasses the IOMMU such as
  virtio-blk-pci (without iommu_platform property) or a bypass bridge
  (docs/iommu-bypass.txt).

You can find a description of the VIOT table, which will be included in
next ACPI version, here: https://jpbrucker.net/virtio-iommu/viot/viot-v9.pdf

[1] https://lore.kernel.org/qemu-devel/20210914142004.2433568-1-jean-philippe@linaro.org/
[2] https://lore.kernel.org/qemu-devel/20210924122802.1455362-1-imammedo@redhat.com/
[3] https://lore.kernel.org/qemu-devel/20210930185050.262759-1-jean-philippe@linaro.org/

Jean-Philippe Brucker (11):
  hw/acpi: Add VIOT table
  hw/arm/virt-acpi-build: Add VIOT table for virtio-iommu
  hw/arm/virt: Remove device tree restriction for virtio-iommu
  hw/arm/virt: Reject instantiation of multiple IOMMUs
  hw/arm/virt: Use object_property_set instead of qdev_prop_set
  hw/i386: Move vIOMMU uniqueness check into pc.c
  pc: Allow instantiating a virtio-iommu device
  tests/acpi: allow updates of VIOT expected data files
  tests/acpi: add test cases for VIOT
  tests/acpi: add expected blob for VIOT test on virt machine
  tests/acpi: add expected blobs for VIOT test on q35 machine

 hw/acpi/viot.h                 |  13 ++++
 include/hw/i386/pc.h           |   2 +
 hw/acpi/viot.c                 | 112 +++++++++++++++++++++++++++++++++
 hw/arm/virt-acpi-build.c       |   7 +++
 hw/arm/virt.c                  |  20 +++---
 hw/i386/acpi-build.c           |   5 ++
 hw/i386/pc.c                   |  30 ++++++++-
 hw/i386/x86-iommu.c            |   6 --
 hw/virtio/virtio-iommu-pci.c   |   7 ---
 tests/qtest/bios-tables-test.c |  38 +++++++++++
 hw/acpi/Kconfig                |   4 ++
 hw/acpi/meson.build            |   1 +
 hw/arm/Kconfig                 |   1 +
 hw/i386/Kconfig                |   1 +
 tests/data/acpi/q35/DSDT.viot  | Bin 0 -> 9398 bytes
 tests/data/acpi/q35/VIOT.viot  | Bin 0 -> 112 bytes
 tests/data/acpi/virt/VIOT      | Bin 0 -> 88 bytes
 17 files changed, 223 insertions(+), 24 deletions(-)
 create mode 100644 hw/acpi/viot.h
 create mode 100644 hw/acpi/viot.c
 create mode 100644 tests/data/acpi/q35/DSDT.viot
 create mode 100644 tests/data/acpi/q35/VIOT.viot
 create mode 100644 tests/data/acpi/virt/VIOT

Comments

Michael S. Tsirkin Oct. 5, 2021, 3:45 p.m. UTC | #1
Looks like this can not be applied yet because the bypass bit
isn't in yet. what's up with that?
Jean-Philippe Brucker Oct. 8, 2021, 3:17 p.m. UTC | #2
On Tue, Oct 05, 2021 at 11:45:42AM -0400, Michael S. Tsirkin wrote:
> Looks like this can not be applied yet because the bypass bit
> isn't in yet. what's up with that?

The boot-bypass bit isn't a hard dependency for this series, but it will
be needed for full support eventually. It will be delayed by spec and
Linux header changes

In the meantime we can work around the problem by having the boot disks
bypass the IOMMU (virtio without iommu-platform, or iommu-bypass bus).

Thanks,
Jean
Haiwei Li Oct. 11, 2021, 10:10 a.m. UTC | #3
On Fri, Oct 8, 2021 at 11:18 PM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> On Tue, Oct 05, 2021 at 11:45:42AM -0400, Michael S. Tsirkin wrote:
> > Looks like this can not be applied yet because the bypass bit
> > isn't in yet. what's up with that?
>
> The boot-bypass bit isn't a hard dependency for this series, but it will
> be needed for full support eventually. It will be delayed by spec and
> Linux header changes
>
> In the meantime we can work around the problem by having the boot disks
> bypass the IOMMU (virtio without iommu-platform, or iommu-bypass bus).

Hi Jean,

I have tested on x86 platform with environment:

qemu, your personal repo: http://jpbrucker.net/git/qemu, branch:
virtio-iommu/acpi-2021-10-01
linux kernel, 5.14
qemu command, with '-device virtio-iommu' and boot disk
'virtio-blk-pci,iommu_platform=off'

But the guest boot failed. The print is as follows:

Begin: Loading essential drivers ... done.
Begin: Running /scripts/init-premount ... done.
Begin: Mounting root file system ... Begin: Running /scripts/local-top ... done.
Begin: Running /scripts/local-premount ... [    7.490238] Btrfs
loaded, crc32c=crc32c-generic, zoned=no
Scanning for Btrfs filesystems
done.
[   10.593238] _warn_unseeded_randomness: 300 callbacks suppressed
[   10.593243] random: get_random_u32 called from
bucket_table_alloc.isra.0+0x123/0x430 with crng_init=0
[   37.534015] random: get_random_u64 called from
dup_task_struct+0x415/0xa10 with crng_init=0
[   37.539950] random: get_random_u64 called from
arch_pick_mmap_layout+0x411/0x5e0 with crng_init=0
[   37.539973] random: get_random_u64 called from
arch_pick_mmap_layout+0x381/0x5e0 with crng_init=0
Begin: Waiting for root file system ... [   38.624374]
_warn_unseeded_randomness: 38 callbacks suppressed
[   38.624389] random: get_random_u64 called from
dup_task_struct+0x415/0xa10 with crng_init=0
Begin: Running /scripts/local-block ... [   38.630760] random:
get_random_u64 called from arch_pick_mmap_layout+0x410
[   38.630767] random: get_random_u64 called from
arch_pick_mmap_layout+0x381/0x5e0 with crng_init=0
mdadm: No arrays found in config file or automatically
done.
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
[   39.633914] _warn_unseeded_randomness: 1278 callbacks suppressed
[   39.633918] random: get_random_u64 called from
dup_task_struct+0x415/0xa10 with crng_init=0
[   39.634867] random: get_random_u64 called from
arch_pick_mmap_layout+0x411/0x5e0 with crng_init=0
[   39.634875] random: get_random_u64 called from
arch_pick_mmap_layout+0x381/0x5e0 with crng_init=0
mdadm: error opening /dev/md?*: No such file or directory
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
mdadm: No arrays found in config file or automatically
done.
Gave up waiting for root file system device.  Common problems:
 - Boot args (cat /proc/cmdline)
   - Check rootdelay= (did the system wait long enough?)
 - Missing modules (cat /proc/modules; ls /dev)
ALERT!  UUID=3caf26b5-4d08-43e0-8634-7573269c4f70 does not exist.
Dropping to a shell!

Any suggestions? Thanks.

--
Haiwei
Jean-Philippe Brucker Oct. 11, 2021, 5:34 p.m. UTC | #4
Hi Haiwei,

On Mon, Oct 11, 2021 at 06:10:07PM +0800, Haiwei Li wrote:
[...]
> Gave up waiting for root file system device.  Common problems:
>  - Boot args (cat /proc/cmdline)
>    - Check rootdelay= (did the system wait long enough?)
>  - Missing modules (cat /proc/modules; ls /dev)
> ALERT!  UUID=3caf26b5-4d08-43e0-8634-7573269c4f70 does not exist.
> Dropping to a shell!
> 
> Any suggestions? Thanks.

It's possible that the rootfs is on a disk behind the IOMMU, and the IOMMU
driver doesn't get loaded. That could happen, for example, if the
virtio-iommu module is not present in the initramfs. Since IOMMU drivers
are typically built into the kernel rather than modules, distro tools that
build the initramfs might not pick up IOMMU modules. I'm guessing this
could be the issue here because of the hints and "Dropping to a shell"
line.

The clean solution will be to patch the initramfs tools to learn about
IOMMU drivers (I'm somewhat working on that). In the meantime, if this is
indeed the problem, you could try explicitly adding the virtio-iommu
module to the initramfs, or building the kernel with CONFIG_VIRTIO_IOMMU=y
rather than =m, though that requires VIRTIO and VIRTIO_PCI to be built-in
as well.

Thanks,
Jean
Haiwei Li Oct. 13, 2021, 12:56 a.m. UTC | #5
On Tue, Oct 12, 2021 at 1:34 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Hi Haiwei,
>
> On Mon, Oct 11, 2021 at 06:10:07PM +0800, Haiwei Li wrote:
> [...]
> > Gave up waiting for root file system device.  Common problems:
> >  - Boot args (cat /proc/cmdline)
> >    - Check rootdelay= (did the system wait long enough?)
> >  - Missing modules (cat /proc/modules; ls /dev)
> > ALERT!  UUID=3caf26b5-4d08-43e0-8634-7573269c4f70 does not exist.
> > Dropping to a shell!
> >
> > Any suggestions? Thanks.
>
> It's possible that the rootfs is on a disk behind the IOMMU, and the IOMMU
> driver doesn't get loaded. That could happen, for example, if the
> virtio-iommu module is not present in the initramfs. Since IOMMU drivers
> are typically built into the kernel rather than modules, distro tools that
> build the initramfs might not pick up IOMMU modules. I'm guessing this
> could be the issue here because of the hints and "Dropping to a shell"
> line.
>
> The clean solution will be to patch the initramfs tools to learn about
> IOMMU drivers (I'm somewhat working on that). In the meantime, if this is
> indeed the problem, you could try explicitly adding the virtio-iommu
> module to the initramfs, or building the kernel with CONFIG_VIRTIO_IOMMU=y
> rather than =m, though that requires VIRTIO and VIRTIO_PCI to be built-in
> as well.

Thanks, Jean. It works.

--
Haiwei
Michael S. Tsirkin Oct. 18, 2021, 3:25 p.m. UTC | #6
On Fri, Oct 08, 2021 at 04:17:37PM +0100, Jean-Philippe Brucker wrote:
> On Tue, Oct 05, 2021 at 11:45:42AM -0400, Michael S. Tsirkin wrote:
> > Looks like this can not be applied yet because the bypass bit
> > isn't in yet. what's up with that?
> 
> The boot-bypass bit isn't a hard dependency for this series, but it will
> be needed for full support eventually. It will be delayed by spec and
> Linux header changes
> 
> In the meantime we can work around the problem by having the boot disks
> bypass the IOMMU (virtio without iommu-platform, or iommu-bypass bus).
> 
> Thanks,
> Jean

OK... how do we want to apply all this?
If my tree I either need ack from an ARM maintainers, or
post a partial patchset with just x86 bits.
Jean-Philippe Brucker Oct. 19, 2021, 3:39 p.m. UTC | #7
On Mon, Oct 18, 2021 at 11:25:05AM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 08, 2021 at 04:17:37PM +0100, Jean-Philippe Brucker wrote:
> > On Tue, Oct 05, 2021 at 11:45:42AM -0400, Michael S. Tsirkin wrote:
> > > Looks like this can not be applied yet because the bypass bit
> > > isn't in yet. what's up with that?
> > 
> > The boot-bypass bit isn't a hard dependency for this series, but it will
> > be needed for full support eventually. It will be delayed by spec and
> > Linux header changes
> > 
> > In the meantime we can work around the problem by having the boot disks
> > bypass the IOMMU (virtio without iommu-platform, or iommu-bypass bus).
> > 
> > Thanks,
> > Jean
> 
> OK... how do we want to apply all this?
> If my tree I either need ack from an ARM maintainers, or
> post a partial patchset with just x86 bits.

Either works for me, with preference for keeping a single series
(otherwise I need to split patch 8, or add the tests later). I'll send v5
whole.

Thanks,
Jean
Michael S. Tsirkin Oct. 20, 2021, 3:17 p.m. UTC | #8
On Tue, Oct 19, 2021 at 04:39:20PM +0100, Jean-Philippe Brucker wrote:
> On Mon, Oct 18, 2021 at 11:25:05AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Oct 08, 2021 at 04:17:37PM +0100, Jean-Philippe Brucker wrote:
> > > On Tue, Oct 05, 2021 at 11:45:42AM -0400, Michael S. Tsirkin wrote:
> > > > Looks like this can not be applied yet because the bypass bit
> > > > isn't in yet. what's up with that?
> > > 
> > > The boot-bypass bit isn't a hard dependency for this series, but it will
> > > be needed for full support eventually. It will be delayed by spec and
> > > Linux header changes
> > > 
> > > In the meantime we can work around the problem by having the boot disks
> > > bypass the IOMMU (virtio without iommu-platform, or iommu-bypass bus).
> > > 
> > > Thanks,
> > > Jean
> > 
> > OK... how do we want to apply all this?
> > If my tree I either need ack from an ARM maintainers, or
> > post a partial patchset with just x86 bits.
> 
> Either works for me, with preference for keeping a single series
> (otherwise I need to split patch 8, or add the tests later). I'll send v5
> whole.
> 
> Thanks,
> Jean

Then you will need an ack from arm maintainers.