[PATCH 0/8] arm/virt: add usb support

[PATCH 0/8] arm/virt: add usb support

[Gerd Hoffmann]

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

[PATCH 1/8] tests/acpi: allow updates for expected data files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..5d523ded1282 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/microvm/DSDT.usb",
+"tests/data/acpi/virt/DSDT.usb",
-- 
2.27.0

[PATCH 2/8] tests/acpi: add empty tests/data/acpi/virt/DSDT.usb file

[Gerd Hoffmann]

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

[PATCH 3/8] arm/virt: add support for -machine usb=on

[Gerd Hoffmann]

Wire up sysbus-xhci in case usb is enabled.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/arm/virt.h |  1 +
 hw/arm/virt.c         | 23 +++++++++++++++++++++++
 hw/arm/Kconfig        |  1 +
 3 files changed, 25 insertions(+)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index aad6d698412a..7c745fbf8b19 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -85,6 +85,7 @@ enum {
     VIRT_ACPI_GED,
     VIRT_NVDIMM_ACPI,
     VIRT_PVTIME,
+    VIRT_XHCI,
     VIRT_LOWMEMMAP_LAST,
 };
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e465a988d683..f2b34fd33be4 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -77,6 +77,7 @@
 #include "hw/acpi/generic_event_device.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
+#include "hw/usb/xhci.h"
 #include "qemu/guest-random.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
@@ -152,6 +153,7 @@ static const MemMapEntry base_memmap[] = {
     [VIRT_ACPI_GED] =           { 0x09080000, ACPI_GED_EVT_SEL_LEN },
     [VIRT_NVDIMM_ACPI] =        { 0x09090000, NVDIMM_ACPI_IO_LEN},
     [VIRT_PVTIME] =             { 0x090a0000, 0x00010000 },
+    [VIRT_XHCI] =               { 0x090b0000, XHCI_LEN_REGS },
     [VIRT_MMIO] =               { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     [VIRT_PLATFORM_BUS] =       { 0x0c000000, 0x02000000 },
@@ -188,6 +190,7 @@ static const int a15irqmap[] = {
     [VIRT_GPIO] = 7,
     [VIRT_SECURE_UART] = 8,
     [VIRT_ACPI_GED] = 9,
+    [VIRT_XHCI] = 10,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
     [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
     [VIRT_SMMU] = 74,    /* ...to 74 + NUM_SMMU_IRQS - 1 */
@@ -806,6 +809,22 @@ static void create_rtc(const VirtMachineState *vms)
     g_free(nodename);
 }
 
+static void create_xhci(const VirtMachineState *vms)
+{
+    hwaddr base = base_memmap[VIRT_XHCI].base;
+    int irq = vms->irqmap[VIRT_XHCI];
+    DeviceState *dev;
+
+    dev = qdev_new(TYPE_XHCI_SYSBUS);
+    qdev_prop_set_uint32(dev, "intrs", 1);
+    qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);
+    qdev_prop_set_uint32(dev, "p2", 8);
+    qdev_prop_set_uint32(dev, "p3", 8);
+    sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(vms->gic, irq));
+}
+
 static DeviceState *gpio_key_dev;
 static void virt_powerdown_req(Notifier *n, void *opaque)
 {
@@ -1981,6 +2000,10 @@ static void machvirt_init(MachineState *machine)
 
     create_pcie(vms);
 
+    if (machine_usb(MACHINE(vms))) {
+        create_xhci(vms);
+    }
+
     if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
         vms->acpi_dev = create_acpi_ged(vms);
     } else {
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7d040827af4f..338ebc6f97d5 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -27,6 +27,7 @@ config ARM_VIRT
     select ACPI_HW_REDUCED
     select ACPI_NVDIMM
     select ACPI_APEI
+    select USB_XHCI_SYSBUS
 
 config CHEETAH
     bool
-- 
2.27.0

[PATCH 4/8] arm/virt: add device tree node for xhci

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/arm/virt.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index f2b34fd33be4..4319dd95ef78 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -812,7 +812,10 @@ static void create_rtc(const VirtMachineState *vms)
 static void create_xhci(const VirtMachineState *vms)
 {
     hwaddr base = base_memmap[VIRT_XHCI].base;
+    hwaddr size = base_memmap[VIRT_XHCI].size;
     int irq = vms->irqmap[VIRT_XHCI];
+    const char compat[] = "generic-xhci";
+    char *nodename;
     DeviceState *dev;
 
     dev = qdev_new(TYPE_XHCI_SYSBUS);
@@ -823,6 +826,16 @@ static void create_xhci(const VirtMachineState *vms)
     sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(vms->gic, irq));
+
+    nodename = g_strdup_printf("/usb@%" PRIx64, base);
+    qemu_fdt_add_subnode(vms->fdt, nodename);
+    qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    g_free(nodename);
 }
 
 static DeviceState *gpio_key_dev;
-- 
2.27.0

[PATCH 5/8] arm/virt: add acpi dsdt entry for xhci

[Gerd Hoffmann]

Also tweak xhci_sysbus_build_aml, add _UID and _CCA.
Without _CCA the linux kernel's xhci probe fails on arm.

ASL changes:

         Device (XHCI)
         {
             Name (_HID, EisaId ("PNP0D10") /* XHCI USB Controller with debug */)  // _HID: Hardware ID
+            Name (_UID, Zero)  // _UID: Unique ID
+            Name (_CCA, One)  // _CCA: Cache Coherency Attribute
             Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
             {
                 Memory32Fixed (ReadWrite,
                     0xFE900000,         // Address Base
                     0x00004000,         // Address Length
                     )
                 Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive, ,, )
                 {
                     0x0000000A,
                 }
             })
         }

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/arm/virt-acpi-build.c | 6 ++++++
 hw/usb/hcd-xhci-sysbus.c | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9747a6458f0b..147332b37597 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -54,6 +54,7 @@
 #include "kvm_arm.h"
 #include "migration/vmstate.h"
 #include "hw/acpi/ghes.h"
+#include "hw/usb/xhci.h"
 
 #define ARM_SPI_BASE 32
 
@@ -631,6 +632,11 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         }
     }
 
+    if (machine_usb(MACHINE(vms))) {
+        xhci_sysbus_build_aml(scope, memmap[VIRT_XHCI].base,
+                              irqmap[VIRT_XHCI] + ARM_SPI_BASE);
+    }
+
     acpi_dsdt_add_power_button(scope);
     acpi_dsdt_add_tpm(scope, vms);
 
diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 29185d2261fb..fa963a5de4fa 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -79,6 +79,8 @@ void xhci_sysbus_build_aml(Aml *scope, uint32_t mmio, unsigned int irq)
                                   AML_EXCLUSIVE, &irq, 1));
 
     aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0D10")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+    aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
     aml_append(dev, aml_name_decl("_CRS", crs));
     aml_append(scope, dev);
 }
-- 
2.27.0

[PATCH 6/8] tests/acpi: add usb testcase for virt

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 3830a40d1041..438cea38a177 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1230,6 +1230,23 @@ static void test_acpi_virt_tcg(void)
     free_test_data(&data);
 }
 
+static void test_acpi_virt_tcg_usb(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .tcg_only = true,
+        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+        .ram_start = 0x40000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+    };
+
+    data.variant = ".usb";
+    test_acpi_one("-cpu cortex-a57 -machine usb=on", &data);
+    free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -1277,6 +1294,7 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/virt", test_acpi_virt_tcg);
         qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
         qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
+        qtest_add_func("acpi/virt/usb", test_acpi_virt_tcg_usb);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
-- 
2.27.0

[PATCH 7/8] tests/acpi: update expected data files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/data/acpi/microvm/DSDT.usb | Bin 414 -> 426 bytes
 tests/data/acpi/virt/DSDT.usb    | Bin 0 -> 5257 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/tests/data/acpi/microvm/DSDT.usb b/tests/data/acpi/microvm/DSDT.usb
index 14423381ea235ed42f6f0f7d98e793c271a4e7c1..bceef778d2633793d5192846e02fb4719de503a0 100644
GIT binary patch
delta 57
zcmbQoyo#C2CD<io6(a)!)6<Du(o8Pm8`aGjrL7}8oIN?>Jv?1_953(+aKwjtx-f9W
NJ3BivPR?aC1OT2T4c`C&

delta 45
zcmZ3*Jdc^nCD<io9wP$-Q_(~&X(kV`jq2u%{Q40d&Ym3c9-b~dju&_ZCRZ{V0s!~<
B3f}+#

diff --git a/tests/data/acpi/virt/DSDT.usb b/tests/data/acpi/virt/DSDT.usb
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..a5a46e044bb788da9028a572f55bc28036ce38bf 100644
GIT binary patch
literal 5257
zcmZvg%WoT16o>EFlh__VGIpHz!zq*s3RHeHP5LDE*oj@@#CX!CQYyI;0#UN5M1@o;
zq!6J(yQ7g<2PqQ&1`;b4NZob8nhpN|JIuLrXOiDh&qx{1nYrhi`;Cv|<J<I`JI{+s
z#r|w;du@NIaj)k!>71pM(#QJM{eIoLKkyray~A0rTrMXE?M^dW_j(-)R#b9ur_)q>
zhu6IAZgj2RZI{cHN_Z_f@cMp!)|q;iQW}_2u2RoOm1Uiy=n3VnuJ3y5hWkmYdxyAH
z%Jp~ZPMVUvx@8b~Cub0`)O46xx9|3&DQ#}`w<&e`?6#+wXE%3LEWCLc5fyWHj?lM#
z0})eah)RfP=vh=HgcFk(HZZElsELfY#25ue4H=h3hAlCYz_^TzWsxx=F&tnlBjbw5
z7?l_qU|d1QipWSvj2tjlkg+N<k`luO#ws$dij0)RC;;OsFe>4PgO`}&NQ@#dt|3D#
zQ~n`|N_`R-*O4LDX-1mIG%#);L$1@T#Fz!fO=QS*nv)pwz_^7BxlYF<#v(92LWW$Y
zuEbaZ#u_r@I?YRrw}4SchFqrwiE$nn4P?l5IxaCT0K)@DRj$*b#CR7NO=QS*Iw3JG
z0;7ctxlSh~#s|PyM}}OdQxfA6FgB1O*XgvxC<CL747pBcBt{h&9c0LLIx8_Q1LHO_
z<T{;`7*~MtF*4*jotGG^z}Q5FT&D{X;~Fr!z^KV}x+pPj0AmXoa-EhW#w}oMhm5UX
zdtN7;M{lcFu9sJ-`S7ScGEq%;F|7^SHyT@~ZHd3r&zGF}p7K;jeWsRFnf{t|^tF1Z
zKBZTsAwlEO>+g(je&rk#(sp_{R2zP$tjw`R#}!q`=<`mDf@&1RDX6JJ*3JxvmeM&i
zE{t>*4a}KqK^_f_LY+zcU@Ub>W^5Sg%xEaJK>Cg}G{~e-XGV942bs|f;!0;mL*pis
zh6b4w>P(tZa2GqsjAmd5nbA;cnL^7TlS1?yVV)yFX5=|yGNZ{;h@PX&lbTYRPc)QT
zEcB!hJrm3`!8{Y(lS1@NGS4LQq?QalDMZf{^Q3m#JiXwNq_`)A=;<&|hj~&9hn^Io
zXPSAYnP-}NQiz@z=9yuh)bgPxh3J`Oo>}IZ<(?FxXO4O1m?yP}=t&`Zjxo<M<~hba
zDMU~DIb&)(sA89SQcH=R6ryLIdFGjCo_kV=o(1MvV4l>1q9=vuInF%Cnddn7q!2yn
zhaz|unJ2ZZ=t&`ZPB702<~hMVDMZgn<~hkcsl`Q43ej_lc}_9UDeg%jdeV<e@SJ9z
z)Doj7h3GlMJZG5a4ELlEJ!hHcEc2um8a*jQ&pGBf$2{k_Cxz%a&phXuC$-$@Ng;YJ
zFwX_%xxhUsM9)R$xyU@JMMqBx(X+%nOU$#xJt;Jvdxsrczm?|aoBmSv%Il|J#64P;
z$T{g?DdO$mE1kY~-#Aws9r%6U3Q=Llz)-YKfT;5V%A7kz-PW@T8(fCc>jlVqM_`$A
zoE0{@oJH#f$hsh~%(;+NbO&Ceu1@HGsB+h6XC6m8;XM8^t77!;oVKCfSwGOHD?{By
zu(`wGfBI(`ZNp1+#mZ0*U^Z%6XdBX1OTBm>a)QeT`kLN?=xnFYKT%2lPOqnu-cEm0
z71vXPZQrwB>?^OiTRCngj=so`JMLchuiC$*`L&;SAJ>1o`TFx0`;JWms`Q$q*V-c;
zXpTOpY1C7_b4q%D?7lpDXnjjpD#7T=@#M>+Xq-9i>Wh!`V152%|CgLOuWwR0*)M)L
zX!>3I_?(%|tCc_g_Lslu{xtPZcjkAmdz?D@Qk~@EmZf*J(ABm3EuU4RUg**_S98j5
zJfGk_pr=z>J9{+xh`w*kx>+M}|B_yr+h{chro_#%TtzpYv@DmU3nAasdE9!>q#m^T
z)R)pbnoK$N-gjvodM&@Txl~*jwC?Vt9_ir0v3vCNfusAqXT9Bi!#UVHyne6kb%L8^
j>aVh{S^gu3pkHMRed&~13)k8%h7+FA6#OuHW}@nU`oteS

literal 0
HcmV?d00001

-- 
2.27.0

[PATCH 8/8] tests/acpi: disallow updates for expected data files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 5d523ded1282..dfb8523c8bf4 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/microvm/DSDT.usb",
-"tests/data/acpi/virt/DSDT.usb",
-- 
2.27.0

Re: [PATCH 0/8] arm/virt: add usb support

[Peter Maydell]

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

Re: [PATCH 0/8] arm/virt: add usb support

[Michael S. Tsirkin]

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
> 

Re: [PATCH 0/8] arm/virt: add usb support

[Gerd Hoffmann]

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

Re: [PATCH 0/8] arm/virt: add usb support

[Peter Maydell]

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

Re: [PATCH 0/8] arm/virt: add usb support

[Gerd Hoffmann]

  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