diff mbox

[RFC] hw/arm/virt: Add a "generic-ehci" USB controller to ARM virt model

Message ID 1428046250-5258-1-git-send-email-j.fanguede@virtualopensystems.com
State New
Headers show

Commit Message

Jérémy Fanguède April 3, 2015, 7:30 a.m. UTC 
This patch enables the attachment of USB devices (emulated or
passthrough) on the ARM virt model, without the need of a PCI USB
controller, by adding a generic EHCI USB controller to the system bus.

Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Jérémy Fanguède <j.fanguede@virtualopensystems.com>
---

A new EHCI USB controller named "generic_ehci" is introduced
(hw/usb/hcd-ehci-sysbus.c). Then, in the virt model, this controller
is added to the system bus. For this purpose a new function,
create_usb, modeled on other create_* functions, is in charge to
create this device and dynamically add the node needed to the device
tree.

The Linux guest OS should include the EHCI platform driver
(CONFIG_USB_EHCI_HCD_PLATFORM).

This patch works fine with TCG on x86 and ARM machines, for both
emulated and host devices, with this kind of cmdline:
-usb \
-device usb-tablet \
-device usb-host,hostbus=1,hostaddr=5

Unfortunately, with KVM, only emulated devices works, for host devices
(passthrough), a random kernel panic occurred. Tests for KVM are
performed on a Samsung Chromebook, with upstream kernel and a USB
stick.
---
 hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
 hw/usb/hcd-ehci-sysbus.c | 16 ++++++++++++++++
 hw/usb/hcd-ehci.h        |  1 +
 3 files changed, 46 insertions(+)

Comments

Peter Maydell April 3, 2015, 11:19 a.m. UTC | #1 
On 3 April 2015 at 08:30, Jérémy Fanguède
<j.fanguede@virtualopensystems.com> wrote:
> This patch enables the attachment of USB devices (emulated or
> passthrough) on the ARM virt model, without the need of a PCI USB
> controller, by adding a generic EHCI USB controller to the system bus.

What's the use case for doing this rather than just plugging a
USB controller into the PCIE controller that we have now?

thanks
-- PMM
Jérémy Fanguède April 3, 2015, 3:30 p.m. UTC | #2 
On Fri, Apr 3, 2015 at 1:19 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On 3 April 2015 at 08:30, Jérémy Fanguède
> <j.fanguede@virtualopensystems.com> wrote:
> > This patch enables the attachment of USB devices (emulated or
> > passthrough) on the ARM virt model, without the need of a PCI USB
> > controller, by adding a generic EHCI USB controller to the system bus.
>
> What's the use case for doing this rather than just plugging a
> USB controller into the PCIE controller that we have now?
>
> thanks
> -- PMM

Hi,

You are right, since it is now possible to use a PCI controller, we
can use USB through it. This patch for completeness doesn't rely on
PCI.
Also, we have this patch for a long time, we just share it with the
world, this kind of modifications could also be implemented in other
models, like vexpress, if it's needed.

Regards,

Jérémy
Peter Maydell April 3, 2015, 5:23 p.m. UTC | #3 
On 3 April 2015 at 16:30, Jérémy Fanguède
<j.fanguede@virtualopensystems.com> wrote:
> You are right, since it is now possible to use a PCI controller, we
> can use USB through it. This patch for completeness doesn't rely on
> PCI.
> Also, we have this patch for a long time, we just share it with the
> world, this kind of modifications could also be implemented in other
> models, like vexpress, if it's needed.

OK. My feeling on this is:
(1) for virt, I'd like to keep it minimal to the extent that
we can, since every new device model in it is extra attack
surface for somebody trying to break out of the VM; so USB
via PCI seems preferable (since the person configuring the VM
can easily not provide USB if they don't want it)

(2) for vexpress USB would definitely be nice, but we would
need a model of the USB controller on the vexpress board
(it's a Philips ISP1761).

-- PMM
diff mbox

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7d082e2..c362685 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -39,6 +39,7 @@ 
 #include "sysemu/kvm.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
+#include "hw/usb/hcd-ehci.h"
 #include "exec/address-spaces.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
@@ -71,6 +72,7 @@  enum {
     VIRT_RTC,
     VIRT_FW_CFG,
     VIRT_PCIE,
+    VIRT_USB,
 };
 
 typedef struct MemMapEntry {
@@ -129,6 +131,7 @@  static const MemMapEntry a15memmap[] = {
     [VIRT_UART] =       { 0x09000000, 0x00001000 },
     [VIRT_RTC] =        { 0x09010000, 0x00001000 },
     [VIRT_FW_CFG] =     { 0x09020000, 0x0000000a },
+    [VIRT_USB] =        { 0x09030000, 0x00001000 },
     [VIRT_MMIO] =       { 0x0a000000, 0x00000200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     /*
@@ -146,6 +149,7 @@  static const int a15irqmap[] = {
     [VIRT_UART] = 1,
     [VIRT_RTC] = 2,
     [VIRT_PCIE] = 3, /* ... to 6 */
+    [VIRT_USB] = 7,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 };
 
@@ -448,6 +452,27 @@  static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_usb(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_USB].base;
+    hwaddr size = vbi->memmap[VIRT_USB].size;
+    int irq = vbi->irqmap[VIRT_USB];
+    const char compat[] = "generic-ehci";
+
+    sysbus_create_simple(TYPE_GENERIC_EHCI, base, pic[irq]);
+
+    nodename = g_strdup_printf("/ehci@%" PRIx64, base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
+    g_free(nodename);
+}
+
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -821,6 +846,10 @@  static void machvirt_init(MachineState *machine)
 
     create_pcie(vbi, pic, gic_phandle);
 
+    if (usb_enabled()) {
+        create_usb(vbi, pic);
+    }
+
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
      * no backend is created the transport will just sit harmlessly idle.
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index cd1cc14..4cf8cc1 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -217,6 +217,21 @@  static const TypeInfo ehci_fusbh200_type_info = {
     .class_init    = fusbh200_ehci_class_init,
 };
 
+static void generic_ehci_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    sec->capsbase = 0x0;
+    sec->opregbase = 0x10;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo ehci_generic_type_info = {
+    .name          = TYPE_GENERIC_EHCI,
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = generic_ehci_class_init,
+};
 static void ehci_sysbus_register_types(void)
 {
     type_register_static(&ehci_type_info);
@@ -224,6 +239,7 @@  static void ehci_sysbus_register_types(void)
     type_register_static(&ehci_exynos4210_type_info);
     type_register_static(&ehci_tegra2_type_info);
     type_register_static(&ehci_fusbh200_type_info);
+    type_register_static(&ehci_generic_type_info);
 }
 
 type_init(ehci_sysbus_register_types)
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 87b240f..9f1d759 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -343,6 +343,7 @@  typedef struct EHCIPCIState {
 #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
 #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
+#define TYPE_GENERIC_EHCI "generic-ehci-usb"
 
 #define SYS_BUS_EHCI(obj) \
     OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)