Bring new microvm goodies to arm virt too.  Wire up
-machine usb=on, add sysbus-xhci in case it is enabled.

Gerd Hoffmann (8):
  tests/acpi: allow updates for expected data files
  tests/acpi: add empty tests/data/acpi/virt/DSDT.usb file
  arm/virt: add support for -machine usb=on
  arm/virt: add device tree node for xhci
  arm/virt: add acpi dsdt entry for xhci
  tests/acpi: add usb testcase for virt
  tests/acpi: update expected data files
  tests/acpi: disallow updates for expected data files

 include/hw/arm/virt.h            |   1 +
 hw/arm/virt-acpi-build.c         |   6 ++++++
 hw/arm/virt.c                    |  36 +++++++++++++++++++++++++++++++
 hw/usb/hcd-xhci-sysbus.c         |   2 ++
 tests/qtest/bios-tables-test.c   |  18 ++++++++++++++++
 hw/arm/Kconfig                   |   1 +
 tests/data/acpi/microvm/DSDT.usb | Bin 414 -> 426 bytes
 tests/data/acpi/virt/DSDT.usb    | Bin 0 -> 5257 bytes
 8 files changed, 64 insertions(+)
 create mode 100644 tests/data/acpi/virt/DSDT.usb

-- 
2.27.0

On Fri, 23 Oct 2020 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> Bring new microvm goodies to arm virt too.  Wire up
> -machine usb=on, add sysbus-xhci in case it is enabled.

So my question here is the usual one -- why can't we
just use a PCI USB controller ?

thanks
-- PMM

On Fri, Oct 23, 2020 at 12:36:05PM +0100, Peter Maydell wrote:
> On Fri, 23 Oct 2020 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > Bring new microvm goodies to arm virt too.  Wire up
> > -machine usb=on, add sysbus-xhci in case it is enabled.
> 
> So my question here is the usual one -- why can't we
> just use a PCI USB controller ?

Well, pci seems to come with some extra resource needs (see -M pc vs.
-M q35 memory footprint differences discussed some months ago).  Thats
why microvm started without pci support, and even now with pcie support
added it is optional (and off by default).

I'm wondering whenever it makes sense for arm/virt to make pcie optional
too.  Adding an OnOffAuto pcie switch is easy.  Some places which
assume pci is present need fixing (-cdrom for example blindly uses
virtio-blk-pci).  Looks doable without too much effort and it would
effectively bring a microvm-ish machine type to the arm/virt world.

So in case pcie is switchable using sysbus-xhci would bring usb support
without requiring pcie support for that.

With pcie being present unconditionally there isn't much of a difference
between sysbus-xhci and qemu-xhci (the pci variant of the device).  The
only problem with machine,usb=on adding a qemu-xhci device automatically
would be that we would have to pick some pci slot where we can place the
device ...

take care,
  Gerd

On Mon, 26 Oct 2020 at 07:01, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Fri, Oct 23, 2020 at 12:36:05PM +0100, Peter Maydell wrote:
> > On Fri, 23 Oct 2020 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > Bring new microvm goodies to arm virt too.  Wire up
> > > -machine usb=on, add sysbus-xhci in case it is enabled.
> >
> > So my question here is the usual one -- why can't we
> > just use a PCI USB controller ?
>
> Well, pci seems to come with some extra resource needs (see -M pc vs.
> -M q35 memory footprint differences discussed some months ago).  Thats
> why microvm started without pci support, and even now with pcie support
> added it is optional (and off by default).

I think PCI is too useful to discard. You can run anything
you want (practically) via PCI. If we make it optional then
we're going to give ourselves the task of reimplementing
memory mapped versions of all the functionality it gives us,
all of which is extra code and all of which adds to the
amount of stuff on the guest-to-QEMU security boundary.

I think to be persuaded that no-PCI was a good idea I'd
want to at least see solid figures based on doing this
for Arm and on having put a lot of effort into slimming
down the PCI handling code/overhead in the guest and still
not being able to get near an MMIO setup. That is, try the
"improve the guest" approach first.

thanks
-- PMM

  Hi,

> > Well, pci seems to come with some extra resource needs (see -M pc vs.
> > -M q35 memory footprint differences discussed some months ago).  Thats
> > why microvm started without pci support, and even now with pcie support
> > added it is optional (and off by default).
> 
> I think PCI is too useful to discard.

Discard is out of question of course.  I'd only add the option to
disable it if not needed.

> You can run anything you want (practically) via PCI. If we make it
> optional then we're going to give ourselves the task of reimplementing
> memory mapped versions of all the functionality it gives us,

No.  Well, at least it would not be *my* plan to reach feature parity.
I'm just trying to make available what we have.

The mmio versions of usb host adapters are needed anyway to emulate some
SoCs.  Describing them in device tree / ACPI is standardized so they
just work without additional changes on the guest side.  So this is
really just adding the device to the machine, adding a device tree node,
adding a acpi dsdt entry (roughly a dozen lines of code each).

Having virtio(-mmio) and usb is enough to cover alot of use cases.
Especially on arm where virtio-gpu is the only display device option.

> all of which is extra code and all of which adds to the
> amount of stuff on the guest-to-QEMU security boundary.

usb is off by default so it doesn't add anything unless you
explicitly ask for it.

Oh, and pci adds some non-trivial stuff to the guest-to-QEMU
security boundary too ...

> I think to be persuaded that no-PCI was a good idea I'd
> want to at least see solid figures based on doing this
> for Arm and on having put a lot of effort into slimming
> down the PCI handling code/overhead in the guest and still
> not being able to get near an MMIO setup. That is, try the
> "improve the guest" approach first.

Ok, fair enough.

take care,
  Gerd

On Fri, Oct 23, 2020 at 09:10:14AM +0200, Gerd Hoffmann wrote:
> Bring new microvm goodies to arm virt too.  Wire up
> -machine usb=on, add sysbus-xhci in case it is enabled.


For ACPI bits:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> Gerd Hoffmann (8):
>   tests/acpi: allow updates for expected data files
>   tests/acpi: add empty tests/data/acpi/virt/DSDT.usb file
>   arm/virt: add support for -machine usb=on
>   arm/virt: add device tree node for xhci
>   arm/virt: add acpi dsdt entry for xhci
>   tests/acpi: add usb testcase for virt
>   tests/acpi: update expected data files
>   tests/acpi: disallow updates for expected data files
> 
>  include/hw/arm/virt.h            |   1 +
>  hw/arm/virt-acpi-build.c         |   6 ++++++
>  hw/arm/virt.c                    |  36 +++++++++++++++++++++++++++++++
>  hw/usb/hcd-xhci-sysbus.c         |   2 ++
>  tests/qtest/bios-tables-test.c   |  18 ++++++++++++++++
>  hw/arm/Kconfig                   |   1 +
>  tests/data/acpi/microvm/DSDT.usb | Bin 414 -> 426 bytes
>  tests/data/acpi/virt/DSDT.usb    | Bin 0 -> 5257 bytes
>  8 files changed, 64 insertions(+)
>  create mode 100644 tests/data/acpi/virt/DSDT.usb
> 
> -- 
> 2.27.0
> 

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/data/acpi/virt/DSDT.usb | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/data/acpi/virt/DSDT.usb

diff --git a/tests/data/acpi/virt/DSDT.usb b/tests/data/acpi/virt/DSDT.usb
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.27.0