[Qemu-devel] [PULL 0/7] Usb 20190503 v2 patches

[Qemu-devel] [PULL 0/7] Usb 20190503 v2 patches

[Gerd Hoffmann]

The following changes since commit f75d15231e56cb0f2bafe19faf1229c459a60731:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-04-30 17:06:57 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20190503-v2-pull-request

for you to fetch changes up to ccb799313a5926a6aa49018bbc67fe6165fad7f3:

  hw/usb: avoid format truncation warning when formatting port name (2019-05-03 08:56:58 +0200)

----------------------------------------------------------------
usb: bugfixes for mtp and xhci, split ohci-pci.

----------------------------------------------------------------

Bandan Das (1):
  usb-mtp: change default to success for usb_mtp_update_object

Daniel P. Berrangé (3):
  usb-mtp: fix string length for filename when writing metadata
  usb-mtp: fix alignment of access of ObjectInfo filename field
  hw/usb: avoid format truncation warning when formatting port name

Longpeng (1):
  usb/xhci: avoid trigger assertion if guest write wrong epid

Thomas Huth (2):
  hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in
    ohci_die()
  hw/usb/hcd-ohci: Move PCI-related code into a separate file

 hw/usb/hcd-ohci.h     | 104 ++++++++++++++++++++
 hw/usb/dev-mtp.c      |  10 +-
 hw/usb/hcd-ohci-pci.c | 163 +++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c     | 219 ++++--------------------------------------
 hw/usb/hcd-xhci.c     |   6 +-
 hw/sh4/Kconfig        |   2 +-
 hw/usb/Kconfig        |   6 +-
 hw/usb/Makefile.objs  |   1 +
 8 files changed, 302 insertions(+), 209 deletions(-)
 create mode 100644 hw/usb/hcd-ohci.h
 create mode 100644 hw/usb/hcd-ohci-pci.c

-- 
2.18.1

[Qemu-devel] [PULL 0/7] Usb 20190503 v2 patches

[Gerd Hoffmann]

The following changes since commit f75d15231e56cb0f2bafe19faf1229c459a60731:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-04-30 17:06:57 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20190503-v2-pull-request

for you to fetch changes up to ccb799313a5926a6aa49018bbc67fe6165fad7f3:

  hw/usb: avoid format truncation warning when formatting port name (2019-05-03 08:56:58 +0200)

----------------------------------------------------------------
usb: bugfixes for mtp and xhci, split ohci-pci.

----------------------------------------------------------------

Bandan Das (1):
  usb-mtp: change default to success for usb_mtp_update_object

Daniel P. Berrangé (3):
  usb-mtp: fix string length for filename when writing metadata
  usb-mtp: fix alignment of access of ObjectInfo filename field
  hw/usb: avoid format truncation warning when formatting port name

Longpeng (1):
  usb/xhci: avoid trigger assertion if guest write wrong epid

Thomas Huth (2):
  hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in
    ohci_die()
  hw/usb/hcd-ohci: Move PCI-related code into a separate file

 hw/usb/hcd-ohci.h     | 104 ++++++++++++++++++++
 hw/usb/dev-mtp.c      |  10 +-
 hw/usb/hcd-ohci-pci.c | 163 +++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c     | 219 ++++--------------------------------------
 hw/usb/hcd-xhci.c     |   6 +-
 hw/sh4/Kconfig        |   2 +-
 hw/usb/Kconfig        |   6 +-
 hw/usb/Makefile.objs  |   1 +
 8 files changed, 302 insertions(+), 209 deletions(-)
 create mode 100644 hw/usb/hcd-ohci.h
 create mode 100644 hw/usb/hcd-ohci-pci.c

-- 
2.18.1

[Qemu-devel] [PULL 1/7] usb-mtp: fix string length for filename when writing metadata

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo 'length' field provides the length of the
wide character string filename. This is then converted to
a multi-byte character string. This may have a different
byte count to the wide character string. We should use the
C string length of the multi-byte string instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-2-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 99548b012d1f..6b7d1296e430 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1721,7 +1721,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
         return;
     }
 
-    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
+    o = usb_mtp_object_lookup_name(p, filename, -1);
     if (o != NULL) {
         next_handle = o->handle;
     }
-- 
2.18.1

[Qemu-devel] [PULL 1/7] usb-mtp: fix string length for filename when writing metadata

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo 'length' field provides the length of the
wide character string filename. This is then converted to
a multi-byte character string. This may have a different
byte count to the wide character string. We should use the
C string length of the multi-byte string instead.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-2-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 99548b012d1f..6b7d1296e430 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1721,7 +1721,7 @@ static void usb_mtp_write_metadata(MTPState *s, uint64_t dlen)
         return;
     }
 
-    o = usb_mtp_object_lookup_name(p, filename, dataset->length);
+    o = usb_mtp_object_lookup_name(p, filename, -1);
     if (o != NULL) {
         next_handle = o->handle;
     }
-- 
2.18.1

[Qemu-devel] [PULL 2/7] usb-mtp: fix alignment of access of ObjectInfo filename field

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e430..963449ec7de8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.18.1

[Qemu-devel] [PULL 2/7] usb-mtp: fix alignment of access of ObjectInfo filename field

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e430..963449ec7de8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.18.1

[Qemu-devel] [PULL 3/7] usb-mtp: change default to success for usb_mtp_update_object

[Gerd Hoffmann]

From: Bandan Das <bsd@redhat.com>

Commit c5ead51f90cf (usb-mtp: return incomplete transfer on a lstat
failure) checks if lstat succeeded when updating attributes of a
file. However, it also changed behavior to return an error by
default. This is incorrect because for smaller file sizes, Qemu
will attempt to write the file in one go and there won't be
an object for it.

Fixes: c5ead51f90cf
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: jpgwojv9pwv.fsf@linux.bootlegged.copy
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 963449ec7de8..d90b336d53f4 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1587,7 +1587,7 @@ done:
 
 static int usb_mtp_update_object(MTPObject *parent, char *name)
 {
-    int ret = -1;
+    int ret = 0;
 
     MTPObject *o =
         usb_mtp_object_lookup_name(parent, name, strlen(name));
-- 
2.18.1

[Qemu-devel] [PULL 3/7] usb-mtp: change default to success for usb_mtp_update_object

[Gerd Hoffmann]

From: Bandan Das <bsd@redhat.com>

Commit c5ead51f90cf (usb-mtp: return incomplete transfer on a lstat
failure) checks if lstat succeeded when updating attributes of a
file. However, it also changed behavior to return an error by
default. This is incorrect because for smaller file sizes, Qemu
will attempt to write the file in one go and there won't be
an object for it.

Fixes: c5ead51f90cf
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: jpgwojv9pwv.fsf@linux.bootlegged.copy
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 963449ec7de8..d90b336d53f4 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -1587,7 +1587,7 @@ done:
 
 static int usb_mtp_update_object(MTPObject *parent, char *name)
 {
-    int ret = -1;
+    int ret = 0;
 
     MTPObject *o =
         usb_mtp_object_lookup_name(parent, name, strlen(name));
-- 
2.18.1

[Qemu-devel] [PULL 4/7] usb/xhci: avoid trigger assertion if guest write wrong epid

[Gerd Hoffmann]

From: Longpeng <longpeng2@huawei.com>

we found the following core in our environment:
0  0x00007fc6b06c2237 in raise ()
1  0x00007fc6b06c3928 in abort ()
2  0x00007fc6b06bb056 in __assert_fail_base ()
3  0x00007fc6b06bb102 in __assert_fail ()
4  0x0000000000702e36 in xhci_kick_ep (...)
5  0x000000000047897a in memory_region_write_accessor (...)
6  0x000000000047767f in access_with_adjusted_size (...)
7  0x000000000047944d in memory_region_dispatch_write (...)
(mr=mr@entry=0x7fc6a0138df0, addr=addr@entry=156, data=1648892416,
size=size@entry=4, attrs=attrs@entry=...)
8  0x000000000042df17 in address_space_write_continue (...)
10 0x000000000043084d in address_space_rw (...)
11 0x000000000047451b in kvm_cpu_exec (cpu=cpu@entry=0x1ab11b0)
12 0x000000000045dcf5 in qemu_kvm_cpu_thread_fn (arg=0x1ab11b0)
13 0x0000000000870631 in qemu_thread_start (args=args@entry=0x1acfb50)
14 0x00000000008959a7 in thread_entry_for_hotfix (pthread_cb=<optimized out>)
15 0x00007fc6b0a60dd5 in start_thread ()
16 0x00007fc6b078a59d in clone ()

(gdb) f 5
5  0x000000000047897a in memory_region_write_accessor (...)
529	    mr->ops->write(mr->opaque, addr, tmp, size);
(gdb) p /x tmp
$9 = 0x62481a00 <-- last byte 0x00 is @epid

xhci_doorbell_write() already check the upper bound of @slotid an @epid,
it also need to check the lower bound.

Cc: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Longpeng <longpeng2@huawei.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 1556605301-44112-1-git-send-email-longpeng2@huawei.com

[ kraxel: fixed typo in subject line ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index ec28bee31963..d8472b4fea7f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3137,7 +3137,7 @@ static void xhci_doorbell_write(void *ptr, hwaddr reg,
         streamid = (val >> 16) & 0xffff;
         if (reg > xhci->numslots) {
             DPRINTF("xhci: bad doorbell %d\n", (int)reg);
-        } else if (epid > 31) {
+        } else if (epid == 0 || epid > 31) {
             DPRINTF("xhci: bad doorbell %d write: 0x%x\n",
                     (int)reg, (uint32_t)val);
         } else {
-- 
2.18.1

[Qemu-devel] [PULL 4/7] usb/xhci: avoid trigger assertion if guest write wrong epid

[Gerd Hoffmann]

From: Longpeng <longpeng2@huawei.com>

we found the following core in our environment:
0  0x00007fc6b06c2237 in raise ()
1  0x00007fc6b06c3928 in abort ()
2  0x00007fc6b06bb056 in __assert_fail_base ()
3  0x00007fc6b06bb102 in __assert_fail ()
4  0x0000000000702e36 in xhci_kick_ep (...)
5  0x000000000047897a in memory_region_write_accessor (...)
6  0x000000000047767f in access_with_adjusted_size (...)
7  0x000000000047944d in memory_region_dispatch_write (...)
(mr=mr@entry=0x7fc6a0138df0, addr=addr@entry=156, data=1648892416,
size=size@entry=4, attrs=attrs@entry=...)
8  0x000000000042df17 in address_space_write_continue (...)
10 0x000000000043084d in address_space_rw (...)
11 0x000000000047451b in kvm_cpu_exec (cpu=cpu@entry=0x1ab11b0)
12 0x000000000045dcf5 in qemu_kvm_cpu_thread_fn (arg=0x1ab11b0)
13 0x0000000000870631 in qemu_thread_start (args=args@entry=0x1acfb50)
14 0x00000000008959a7 in thread_entry_for_hotfix (pthread_cb=<optimized out>)
15 0x00007fc6b0a60dd5 in start_thread ()
16 0x00007fc6b078a59d in clone ()

(gdb) f 5
5  0x000000000047897a in memory_region_write_accessor (...)
529	    mr->ops->write(mr->opaque, addr, tmp, size);
(gdb) p /x tmp
$9 = 0x62481a00 <-- last byte 0x00 is @epid

xhci_doorbell_write() already check the upper bound of @slotid an @epid,
it also need to check the lower bound.

Cc: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Longpeng <longpeng2@huawei.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 1556605301-44112-1-git-send-email-longpeng2@huawei.com

[ kraxel: fixed typo in subject line ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index ec28bee31963..d8472b4fea7f 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3137,7 +3137,7 @@ static void xhci_doorbell_write(void *ptr, hwaddr reg,
         streamid = (val >> 16) & 0xffff;
         if (reg > xhci->numslots) {
             DPRINTF("xhci: bad doorbell %d\n", (int)reg);
-        } else if (epid > 31) {
+        } else if (epid == 0 || epid > 31) {
             DPRINTF("xhci: bad doorbell %d write: 0x%x\n",
                     (int)reg, (uint32_t)val);
         } else {
-- 
2.18.1

[Qemu-devel] [PULL 5/7] hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in ohci_die()

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

The ohci_die() function always assumes to be running with a PCI OHCI
controller and calls the PCI-specific functions pci_set_word(). However,
this function might also get called for the sysbus OHCI devices, so it
likely fails in that case. To fix this issue, change the code now, so that
there are two implementations now, one for sysbus and one for PCI, and
use the right function via a function pointer in the OHCIState structure.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-2-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 81cf5ab7a5a7..6d3f556989fb 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -52,7 +52,7 @@ typedef struct OHCIPort {
     uint32_t ctrl;
 } OHCIPort;
 
-typedef struct {
+typedef struct OHCIState {
     USBBus bus;
     qemu_irq irq;
     MemoryRegion mem;
@@ -108,6 +108,7 @@ typedef struct {
     uint32_t async_td;
     bool async_complete;
 
+    void (*ohci_die)(struct OHCIState *ohci);
 } OHCIState;
 
 /* Host Controller Communications Area */
@@ -302,7 +303,10 @@ struct ohci_iso_td {
 
 #define OHCI_HRESET_FSBIR       (1 << 0)
 
-static void ohci_die(OHCIState *ohci);
+static void ohci_die(OHCIState *ohci)
+{
+    ohci->ohci_die(ohci);
+}
 
 /* Update IRQ levels */
 static inline void ohci_intr_update(OHCIState *ohci)
@@ -1854,13 +1858,14 @@ static USBBusOps ohci_bus_ops = {
 
 static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                           uint32_t num_ports, dma_addr_t localmem_base,
-                          char *masterbus, uint32_t firstport,
-                          AddressSpace *as, Error **errp)
+                          char *masterbus, uint32_t firstport, AddressSpace *as,
+                          void (*ohci_die_fn)(struct OHCIState *), Error **errp)
 {
     Error *err = NULL;
     int i;
 
     ohci->as = as;
+    ohci->ohci_die = ohci_die_fn;
 
     if (num_ports > OHCI_MAX_PORTS) {
         error_setg(errp, "OHCI num-ports=%u is too big (limit is %u ports)",
@@ -1933,18 +1938,28 @@ typedef struct {
     uint32_t firstport;
 } OHCIPCIState;
 
-/** A typical O/EHCI will stop operating, set itself into error state
- * (which can be queried by MMIO) and will set PERR in its config
- * space to signal that it got an error
+/**
+ * A typical OHCI will stop operating and set itself into error state
+ * (which can be queried by MMIO) to signal that it got an error.
  */
-static void ohci_die(OHCIState *ohci)
+static void ohci_sysbus_die(struct OHCIState *ohci)
 {
-    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
-
     trace_usb_ohci_die();
 
     ohci_set_interrupt(ohci, OHCI_INTR_UE);
     ohci_bus_stop(ohci);
+}
+
+/**
+ * A typical PCI OHCI will additionally set PERR in its configspace to
+ * signal that it got an error.
+ */
+static void ohci_pci_die(struct OHCIState *ohci)
+{
+    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
+
+    ohci_sysbus_die(ohci);
+
     pci_set_word(dev->parent_obj.config + PCI_STATUS,
                  PCI_STATUS_DETECTED_PARITY);
 }
@@ -1959,7 +1974,7 @@ static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
 
     usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
                   ohci->masterbus, ohci->firstport,
-                  pci_get_address_space(dev), &err);
+                  pci_get_address_space(dev), ohci_pci_die, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -2023,7 +2038,7 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 
     usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
                   s->masterbus, s->firstport,
-                  &address_space_memory, &err);
+                  &address_space_memory, ohci_sysbus_die, &err);
     if (err) {
         error_propagate(errp, err);
         return;
-- 
2.18.1

[Qemu-devel] [PULL 5/7] hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in ohci_die()

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

The ohci_die() function always assumes to be running with a PCI OHCI
controller and calls the PCI-specific functions pci_set_word(). However,
this function might also get called for the sysbus OHCI devices, so it
likely fails in that case. To fix this issue, change the code now, so that
there are two implementations now, one for sysbus and one for PCI, and
use the right function via a function pointer in the OHCIState structure.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-2-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 81cf5ab7a5a7..6d3f556989fb 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -52,7 +52,7 @@ typedef struct OHCIPort {
     uint32_t ctrl;
 } OHCIPort;
 
-typedef struct {
+typedef struct OHCIState {
     USBBus bus;
     qemu_irq irq;
     MemoryRegion mem;
@@ -108,6 +108,7 @@ typedef struct {
     uint32_t async_td;
     bool async_complete;
 
+    void (*ohci_die)(struct OHCIState *ohci);
 } OHCIState;
 
 /* Host Controller Communications Area */
@@ -302,7 +303,10 @@ struct ohci_iso_td {
 
 #define OHCI_HRESET_FSBIR       (1 << 0)
 
-static void ohci_die(OHCIState *ohci);
+static void ohci_die(OHCIState *ohci)
+{
+    ohci->ohci_die(ohci);
+}
 
 /* Update IRQ levels */
 static inline void ohci_intr_update(OHCIState *ohci)
@@ -1854,13 +1858,14 @@ static USBBusOps ohci_bus_ops = {
 
 static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                           uint32_t num_ports, dma_addr_t localmem_base,
-                          char *masterbus, uint32_t firstport,
-                          AddressSpace *as, Error **errp)
+                          char *masterbus, uint32_t firstport, AddressSpace *as,
+                          void (*ohci_die_fn)(struct OHCIState *), Error **errp)
 {
     Error *err = NULL;
     int i;
 
     ohci->as = as;
+    ohci->ohci_die = ohci_die_fn;
 
     if (num_ports > OHCI_MAX_PORTS) {
         error_setg(errp, "OHCI num-ports=%u is too big (limit is %u ports)",
@@ -1933,18 +1938,28 @@ typedef struct {
     uint32_t firstport;
 } OHCIPCIState;
 
-/** A typical O/EHCI will stop operating, set itself into error state
- * (which can be queried by MMIO) and will set PERR in its config
- * space to signal that it got an error
+/**
+ * A typical OHCI will stop operating and set itself into error state
+ * (which can be queried by MMIO) to signal that it got an error.
  */
-static void ohci_die(OHCIState *ohci)
+static void ohci_sysbus_die(struct OHCIState *ohci)
 {
-    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
-
     trace_usb_ohci_die();
 
     ohci_set_interrupt(ohci, OHCI_INTR_UE);
     ohci_bus_stop(ohci);
+}
+
+/**
+ * A typical PCI OHCI will additionally set PERR in its configspace to
+ * signal that it got an error.
+ */
+static void ohci_pci_die(struct OHCIState *ohci)
+{
+    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
+
+    ohci_sysbus_die(ohci);
+
     pci_set_word(dev->parent_obj.config + PCI_STATUS,
                  PCI_STATUS_DETECTED_PARITY);
 }
@@ -1959,7 +1974,7 @@ static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
 
     usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
                   ohci->masterbus, ohci->firstport,
-                  pci_get_address_space(dev), &err);
+                  pci_get_address_space(dev), ohci_pci_die, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -2023,7 +2038,7 @@ static void ohci_realize_pxa(DeviceState *dev, Error **errp)
 
     usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
                   s->masterbus, s->firstport,
-                  &address_space_memory, &err);
+                  &address_space_memory, ohci_sysbus_die, &err);
     if (err) {
         error_propagate(errp, err);
         return;
-- 
2.18.1

[Qemu-devel] [PULL 6/7] hw/usb/hcd-ohci: Move PCI-related code into a separate file

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

Some machines (like the pxa2xx-based ARM machines) only have a sysbus
OHCI controller, but no PCI. With the new Kconfig-style build system,
it will soon be possible to create QEMU binaries that only contain
such PCI-less machines. However, the two OHCI controllers, for sysbus
and for PCI, are currently both located in one file, so the PCI code
is still required for linking here. Move the OHCI-PCI device code
into a separate file, so that it is possible to use the sysbus OHCI
device also without the PCI dependency.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-3-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.h     | 104 ++++++++++++++++++++
 hw/usb/hcd-ohci-pci.c | 163 +++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c     | 216 ++----------------------------------------
 hw/sh4/Kconfig        |   2 +-
 hw/usb/Kconfig        |   6 +-
 hw/usb/Makefile.objs  |   1 +
 6 files changed, 284 insertions(+), 208 deletions(-)
 create mode 100644 hw/usb/hcd-ohci.h
 create mode 100644 hw/usb/hcd-ohci-pci.c

diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
new file mode 100644
index 000000000000..16e3f1e13a31
--- /dev/null
+++ b/hw/usb/hcd-ohci.h
@@ -0,0 +1,104 @@
+/*
+ * QEMU USB OHCI Emulation
+ * Copyright (c) 2004 Gianni Tedesco
+ * Copyright (c) 2006 CodeSourcery
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HCD_OHCI_H
+#define HCD_OHCI_H
+
+#include "sysemu/dma.h"
+
+/* Number of Downstream Ports on the root hub: */
+#define OHCI_MAX_PORTS 15
+
+typedef struct OHCIPort {
+    USBPort port;
+    uint32_t ctrl;
+} OHCIPort;
+
+typedef struct OHCIState {
+    USBBus bus;
+    qemu_irq irq;
+    MemoryRegion mem;
+    AddressSpace *as;
+    uint32_t num_ports;
+    const char *name;
+
+    QEMUTimer *eof_timer;
+    int64_t sof_time;
+
+    /* OHCI state */
+    /* Control partition */
+    uint32_t ctl, status;
+    uint32_t intr_status;
+    uint32_t intr;
+
+    /* memory pointer partition */
+    uint32_t hcca;
+    uint32_t ctrl_head, ctrl_cur;
+    uint32_t bulk_head, bulk_cur;
+    uint32_t per_cur;
+    uint32_t done;
+    int32_t done_count;
+
+    /* Frame counter partition */
+    uint16_t fsmps;
+    uint8_t fit;
+    uint16_t fi;
+    uint8_t frt;
+    uint16_t frame_number;
+    uint16_t padding;
+    uint32_t pstart;
+    uint32_t lst;
+
+    /* Root Hub partition */
+    uint32_t rhdesc_a, rhdesc_b;
+    uint32_t rhstatus;
+    OHCIPort rhport[OHCI_MAX_PORTS];
+
+    /* PXA27x Non-OHCI events */
+    uint32_t hstatus;
+    uint32_t hmask;
+    uint32_t hreset;
+    uint32_t htest;
+
+    /* SM501 local memory offset */
+    dma_addr_t localmem_base;
+
+    /* Active packets.  */
+    uint32_t old_ctl;
+    USBPacket usb_packet;
+    uint8_t usb_buf[8192];
+    uint32_t async_td;
+    bool async_complete;
+
+    void (*ohci_die)(struct OHCIState *ohci);
+} OHCIState;
+
+extern const VMStateDescription vmstate_ohci_state;
+
+void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
+                   dma_addr_t localmem_base, char *masterbus,
+                   uint32_t firstport, AddressSpace *as,
+                   void (*ohci_die_fn)(struct OHCIState *), Error **errp);
+void ohci_bus_stop(OHCIState *ohci);
+void ohci_stop_endpoints(OHCIState *ohci);
+void ohci_hard_reset(OHCIState *ohci);
+void ohci_sysbus_die(struct OHCIState *ohci);
+
+#endif
diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c
new file mode 100644
index 000000000000..e8f372c6ad30
--- /dev/null
+++ b/hw/usb/hcd-ohci-pci.c
@@ -0,0 +1,163 @@
+/*
+ * QEMU USB OHCI Emulation
+ * Copyright (c) 2004 Gianni Tedesco
+ * Copyright (c) 2006 CodeSourcery
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "qapi/error.h"
+#include "qemu/timer.h"
+#include "hw/usb.h"
+#include "hw/pci/pci.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-dma.h"
+#include "trace.h"
+#include "hcd-ohci.h"
+
+#define TYPE_PCI_OHCI "pci-ohci"
+#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
+
+typedef struct {
+    /*< private >*/
+    PCIDevice parent_obj;
+    /*< public >*/
+
+    OHCIState state;
+    char *masterbus;
+    uint32_t num_ports;
+    uint32_t firstport;
+} OHCIPCIState;
+
+/**
+ * A typical PCI OHCI will additionally set PERR in its configspace to
+ * signal that it got an error.
+ */
+static void ohci_pci_die(struct OHCIState *ohci)
+{
+    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
+
+    ohci_sysbus_die(ohci);
+
+    pci_set_word(dev->parent_obj.config + PCI_STATUS,
+                 PCI_STATUS_DETECTED_PARITY);
+}
+
+static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
+{
+    Error *err = NULL;
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+
+    dev->config[PCI_CLASS_PROG] = 0x10; /* OHCI */
+    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
+
+    usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
+                  ohci->masterbus, ohci->firstport,
+                  pci_get_address_space(dev), ohci_pci_die, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    ohci->state.irq = pci_allocate_irq(dev);
+    pci_register_bar(dev, 0, 0, &ohci->state.mem);
+}
+
+static void usb_ohci_exit(PCIDevice *dev)
+{
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    trace_usb_ohci_exit(s->name);
+    ohci_bus_stop(s);
+
+    if (s->async_td) {
+        usb_cancel_packet(&s->usb_packet);
+        s->async_td = 0;
+    }
+    ohci_stop_endpoints(s);
+
+    if (!ohci->masterbus) {
+        usb_bus_release(&s->bus);
+    }
+
+    timer_del(s->eof_timer);
+    timer_free(s->eof_timer);
+}
+
+static void usb_ohci_reset_pci(DeviceState *d)
+{
+    PCIDevice *dev = PCI_DEVICE(d);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    ohci_hard_reset(s);
+}
+
+static Property ohci_pci_properties[] = {
+    DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
+    DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
+    DEFINE_PROP_UINT32("firstport", OHCIPCIState, firstport, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vmstate_ohci = {
+    .name = "ohci",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(parent_obj, OHCIPCIState),
+        VMSTATE_STRUCT(state, OHCIPCIState, 1, vmstate_ohci_state, OHCIState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void ohci_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = usb_ohci_realize_pci;
+    k->exit = usb_ohci_exit;
+    k->vendor_id = PCI_VENDOR_ID_APPLE;
+    k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
+    k->class_id = PCI_CLASS_SERIAL_USB;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+    dc->desc = "Apple USB Controller";
+    dc->props = ohci_pci_properties;
+    dc->hotpluggable = false;
+    dc->vmsd = &vmstate_ohci;
+    dc->reset = usb_ohci_reset_pci;
+}
+
+static const TypeInfo ohci_pci_info = {
+    .name          = TYPE_PCI_OHCI,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(OHCIPCIState),
+    .class_init    = ohci_pci_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
+static void ohci_pci_register_types(void)
+{
+    type_register_static(&ohci_pci_info);
+}
+
+type_init(ohci_pci_register_types)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 6d3f556989fb..aaba09058896 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -30,87 +30,19 @@
 #include "qapi/error.h"
 #include "qemu/timer.h"
 #include "hw/usb.h"
-#include "hw/pci/pci.h"
 #include "hw/sysbus.h"
 #include "hw/qdev-dma.h"
 #include "trace.h"
+#include "hcd-ohci.h"
 
 /* This causes frames to occur 1000x slower */
 //#define OHCI_TIME_WARP 1
 
-/* Number of Downstream Ports on the root hub.  */
-
-#define OHCI_MAX_PORTS 15
-
 #define ED_LINK_LIMIT 32
 
 static int64_t usb_frame_time;
 static int64_t usb_bit_time;
 
-typedef struct OHCIPort {
-    USBPort port;
-    uint32_t ctrl;
-} OHCIPort;
-
-typedef struct OHCIState {
-    USBBus bus;
-    qemu_irq irq;
-    MemoryRegion mem;
-    AddressSpace *as;
-    uint32_t num_ports;
-    const char *name;
-
-    QEMUTimer *eof_timer;
-    int64_t sof_time;
-
-    /* OHCI state */
-    /* Control partition */
-    uint32_t ctl, status;
-    uint32_t intr_status;
-    uint32_t intr;
-
-    /* memory pointer partition */
-    uint32_t hcca;
-    uint32_t ctrl_head, ctrl_cur;
-    uint32_t bulk_head, bulk_cur;
-    uint32_t per_cur;
-    uint32_t done;
-    int32_t done_count;
-
-    /* Frame counter partition */
-    uint16_t fsmps;
-    uint8_t fit;
-    uint16_t fi;
-    uint8_t frt;
-    uint16_t frame_number;
-    uint16_t padding;
-    uint32_t pstart;
-    uint32_t lst;
-
-    /* Root Hub partition */
-    uint32_t rhdesc_a, rhdesc_b;
-    uint32_t rhstatus;
-    OHCIPort rhport[OHCI_MAX_PORTS];
-
-    /* PXA27x Non-OHCI events */
-    uint32_t hstatus;
-    uint32_t hmask;
-    uint32_t hreset;
-    uint32_t htest;
-
-    /* SM501 local memory offset */
-    dma_addr_t localmem_base;
-
-    /* Active packets.  */
-    uint32_t old_ctl;
-    USBPacket usb_packet;
-    uint8_t usb_buf[8192];
-    uint32_t async_td;
-    bool async_complete;
-
-    void (*ohci_die)(struct OHCIState *ohci);
-} OHCIState;
-
 /* Host Controller Communications Area */
 struct ohci_hcca {
     uint32_t intr[32];
@@ -123,7 +55,6 @@ struct ohci_hcca {
 #define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
 #define ED_WBACK_SIZE   4
 
-static void ohci_bus_stop(OHCIState *ohci);
 static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev);
 
 /* Bitfields for the first word of an Endpoint Desciptor.  */
@@ -430,7 +361,7 @@ static USBDevice *ohci_find_device(OHCIState *ohci, uint8_t addr)
     return NULL;
 }
 
-static void ohci_stop_endpoints(OHCIState *ohci)
+void ohci_stop_endpoints(OHCIState *ohci)
 {
     USBDevice *dev;
     int i, j;
@@ -502,7 +433,7 @@ static void ohci_soft_reset(OHCIState *ohci)
     ohci->lst = OHCI_LS_THRESH;
 }
 
-static void ohci_hard_reset(OHCIState *ohci)
+void ohci_hard_reset(OHCIState *ohci)
 {
     ohci_soft_reset(ohci);
     ohci->ctl = 0;
@@ -1376,7 +1307,7 @@ static int ohci_bus_start(OHCIState *ohci)
 }
 
 /* Stop sending SOF tokens on the bus */
-static void ohci_bus_stop(OHCIState *ohci)
+void ohci_bus_stop(OHCIState *ohci)
 {
     trace_usb_ohci_stop(ohci->name);
     timer_del(ohci->eof_timer);
@@ -1856,10 +1787,10 @@ static USBPortOps ohci_port_ops = {
 static USBBusOps ohci_bus_ops = {
 };
 
-static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
-                          uint32_t num_ports, dma_addr_t localmem_base,
-                          char *masterbus, uint32_t firstport, AddressSpace *as,
-                          void (*ohci_die_fn)(struct OHCIState *), Error **errp)
+void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
+                   dma_addr_t localmem_base, char *masterbus,
+                   uint32_t firstport, AddressSpace *as,
+                   void (*ohci_die_fn)(struct OHCIState *), Error **errp)
 {
     Error *err = NULL;
     int i;
@@ -1924,25 +1855,11 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                                    ohci_frame_boundary, ohci);
 }
 
-#define TYPE_PCI_OHCI "pci-ohci"
-#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
-
-typedef struct {
-    /*< private >*/
-    PCIDevice parent_obj;
-    /*< public >*/
-
-    OHCIState state;
-    char *masterbus;
-    uint32_t num_ports;
-    uint32_t firstport;
-} OHCIPCIState;
-
 /**
  * A typical OHCI will stop operating and set itself into error state
  * (which can be queried by MMIO) to signal that it got an error.
  */
-static void ohci_sysbus_die(struct OHCIState *ohci)
+void ohci_sysbus_die(struct OHCIState *ohci)
 {
     trace_usb_ohci_die();
 
@@ -1950,71 +1867,6 @@ static void ohci_sysbus_die(struct OHCIState *ohci)
     ohci_bus_stop(ohci);
 }
 
-/**
- * A typical PCI OHCI will additionally set PERR in its configspace to
- * signal that it got an error.
- */
-static void ohci_pci_die(struct OHCIState *ohci)
-{
-    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
-
-    ohci_sysbus_die(ohci);
-
-    pci_set_word(dev->parent_obj.config + PCI_STATUS,
-                 PCI_STATUS_DETECTED_PARITY);
-}
-
-static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
-{
-    Error *err = NULL;
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-
-    dev->config[PCI_CLASS_PROG] = 0x10; /* OHCI */
-    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
-
-    usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
-                  ohci->masterbus, ohci->firstport,
-                  pci_get_address_space(dev), ohci_pci_die, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-
-    ohci->state.irq = pci_allocate_irq(dev);
-    pci_register_bar(dev, 0, 0, &ohci->state.mem);
-}
-
-static void usb_ohci_exit(PCIDevice *dev)
-{
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-    OHCIState *s = &ohci->state;
-
-    trace_usb_ohci_exit(s->name);
-    ohci_bus_stop(s);
-
-    if (s->async_td) {
-        usb_cancel_packet(&s->usb_packet);
-        s->async_td = 0;
-    }
-    ohci_stop_endpoints(s);
-
-    if (!ohci->masterbus) {
-        usb_bus_release(&s->bus);
-    }
-
-    timer_del(s->eof_timer);
-    timer_free(s->eof_timer);
-}
-
-static void usb_ohci_reset_pci(DeviceState *d)
-{
-    PCIDevice *dev = PCI_DEVICE(d);
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-    OHCIState *s = &ohci->state;
-
-    ohci_hard_reset(s);
-}
-
 #define TYPE_SYSBUS_OHCI "sysbus-ohci"
 #define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
 
@@ -2055,13 +1907,6 @@ static void usb_ohci_reset_sysbus(DeviceState *dev)
     ohci_hard_reset(ohci);
 }
 
-static Property ohci_pci_properties[] = {
-    DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
-    DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
-    DEFINE_PROP_UINT32("firstport", OHCIPCIState, firstport, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static const VMStateDescription vmstate_ohci_state_port = {
     .name = "ohci-core/port",
     .version_id = 1,
@@ -2090,7 +1935,7 @@ static const VMStateDescription vmstate_ohci_eof_timer = {
     },
 };
 
-static const VMStateDescription vmstate_ohci_state = {
+const VMStateDescription vmstate_ohci_state = {
     .name = "ohci-core",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -2137,46 +1982,6 @@ static const VMStateDescription vmstate_ohci_state = {
     }
 };
 
-static const VMStateDescription vmstate_ohci = {
-    .name = "ohci",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(parent_obj, OHCIPCIState),
-        VMSTATE_STRUCT(state, OHCIPCIState, 1, vmstate_ohci_state, OHCIState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static void ohci_pci_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = usb_ohci_realize_pci;
-    k->exit = usb_ohci_exit;
-    k->vendor_id = PCI_VENDOR_ID_APPLE;
-    k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
-    k->class_id = PCI_CLASS_SERIAL_USB;
-    set_bit(DEVICE_CATEGORY_USB, dc->categories);
-    dc->desc = "Apple USB Controller";
-    dc->props = ohci_pci_properties;
-    dc->hotpluggable = false;
-    dc->vmsd = &vmstate_ohci;
-    dc->reset = usb_ohci_reset_pci;
-}
-
-static const TypeInfo ohci_pci_info = {
-    .name          = TYPE_PCI_OHCI,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(OHCIPCIState),
-    .class_init    = ohci_pci_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
 static Property ohci_sysbus_properties[] = {
     DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
@@ -2205,7 +2010,6 @@ static const TypeInfo ohci_sysbus_info = {
 
 static void ohci_register_types(void)
 {
-    type_register_static(&ohci_pci_info);
     type_register_static(&ohci_sysbus_info);
 }
 
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index 593662d28ade..4cbce3a0ed5e 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -6,7 +6,7 @@ config R2D
     select I82378 if TEST_DEVICES
     select IDE_MMIO
     select PFLASH_CFI02
-    select USB_OHCI
+    select USB_OHCI_PCI
     select PCI
     select SM501
     select SH4
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index a1b7acb12a37..564305e28339 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -8,10 +8,14 @@ config USB_UHCI
     select USB
 
 config USB_OHCI
+    bool
+    select USB
+
+config USB_OHCI_PCI
     bool
     default y if PCI_DEVICES
     depends on PCI
-    select USB
+    select USB_OHCI
 
 config USB_EHCI
     bool
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 2b929649acd0..81688f6e7062 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_USB) += desc.o desc-msos.o
 # usb host adapters
 common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
 common-obj-$(CONFIG_USB_OHCI) += hcd-ohci.o
+common-obj-$(CONFIG_USB_OHCI_PCI) += hcd-ohci-pci.o
 common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o
 common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci.o hcd-ehci-sysbus.o
 common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
-- 
2.18.1

[Qemu-devel] [PULL 6/7] hw/usb/hcd-ohci: Move PCI-related code into a separate file

[Gerd Hoffmann]

From: Thomas Huth <thuth@redhat.com>

Some machines (like the pxa2xx-based ARM machines) only have a sysbus
OHCI controller, but no PCI. With the new Kconfig-style build system,
it will soon be possible to create QEMU binaries that only contain
such PCI-less machines. However, the two OHCI controllers, for sysbus
and for PCI, are currently both located in one file, so the PCI code
is still required for linking here. Move the OHCI-PCI device code
into a separate file, so that it is possible to use the sysbus OHCI
device also without the PCI dependency.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190419075625.24251-3-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-ohci.h     | 104 ++++++++++++++++++++
 hw/usb/hcd-ohci-pci.c | 163 +++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c     | 216 ++----------------------------------------
 hw/sh4/Kconfig        |   2 +-
 hw/usb/Kconfig        |   6 +-
 hw/usb/Makefile.objs  |   1 +
 6 files changed, 284 insertions(+), 208 deletions(-)
 create mode 100644 hw/usb/hcd-ohci.h
 create mode 100644 hw/usb/hcd-ohci-pci.c

diff --git a/hw/usb/hcd-ohci.h b/hw/usb/hcd-ohci.h
new file mode 100644
index 000000000000..16e3f1e13a31
--- /dev/null
+++ b/hw/usb/hcd-ohci.h
@@ -0,0 +1,104 @@
+/*
+ * QEMU USB OHCI Emulation
+ * Copyright (c) 2004 Gianni Tedesco
+ * Copyright (c) 2006 CodeSourcery
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HCD_OHCI_H
+#define HCD_OHCI_H
+
+#include "sysemu/dma.h"
+
+/* Number of Downstream Ports on the root hub: */
+#define OHCI_MAX_PORTS 15
+
+typedef struct OHCIPort {
+    USBPort port;
+    uint32_t ctrl;
+} OHCIPort;
+
+typedef struct OHCIState {
+    USBBus bus;
+    qemu_irq irq;
+    MemoryRegion mem;
+    AddressSpace *as;
+    uint32_t num_ports;
+    const char *name;
+
+    QEMUTimer *eof_timer;
+    int64_t sof_time;
+
+    /* OHCI state */
+    /* Control partition */
+    uint32_t ctl, status;
+    uint32_t intr_status;
+    uint32_t intr;
+
+    /* memory pointer partition */
+    uint32_t hcca;
+    uint32_t ctrl_head, ctrl_cur;
+    uint32_t bulk_head, bulk_cur;
+    uint32_t per_cur;
+    uint32_t done;
+    int32_t done_count;
+
+    /* Frame counter partition */
+    uint16_t fsmps;
+    uint8_t fit;
+    uint16_t fi;
+    uint8_t frt;
+    uint16_t frame_number;
+    uint16_t padding;
+    uint32_t pstart;
+    uint32_t lst;
+
+    /* Root Hub partition */
+    uint32_t rhdesc_a, rhdesc_b;
+    uint32_t rhstatus;
+    OHCIPort rhport[OHCI_MAX_PORTS];
+
+    /* PXA27x Non-OHCI events */
+    uint32_t hstatus;
+    uint32_t hmask;
+    uint32_t hreset;
+    uint32_t htest;
+
+    /* SM501 local memory offset */
+    dma_addr_t localmem_base;
+
+    /* Active packets.  */
+    uint32_t old_ctl;
+    USBPacket usb_packet;
+    uint8_t usb_buf[8192];
+    uint32_t async_td;
+    bool async_complete;
+
+    void (*ohci_die)(struct OHCIState *ohci);
+} OHCIState;
+
+extern const VMStateDescription vmstate_ohci_state;
+
+void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
+                   dma_addr_t localmem_base, char *masterbus,
+                   uint32_t firstport, AddressSpace *as,
+                   void (*ohci_die_fn)(struct OHCIState *), Error **errp);
+void ohci_bus_stop(OHCIState *ohci);
+void ohci_stop_endpoints(OHCIState *ohci);
+void ohci_hard_reset(OHCIState *ohci);
+void ohci_sysbus_die(struct OHCIState *ohci);
+
+#endif
diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c
new file mode 100644
index 000000000000..e8f372c6ad30
--- /dev/null
+++ b/hw/usb/hcd-ohci-pci.c
@@ -0,0 +1,163 @@
+/*
+ * QEMU USB OHCI Emulation
+ * Copyright (c) 2004 Gianni Tedesco
+ * Copyright (c) 2006 CodeSourcery
+ * Copyright (c) 2006 Openedhand Ltd.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "qapi/error.h"
+#include "qemu/timer.h"
+#include "hw/usb.h"
+#include "hw/pci/pci.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-dma.h"
+#include "trace.h"
+#include "hcd-ohci.h"
+
+#define TYPE_PCI_OHCI "pci-ohci"
+#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
+
+typedef struct {
+    /*< private >*/
+    PCIDevice parent_obj;
+    /*< public >*/
+
+    OHCIState state;
+    char *masterbus;
+    uint32_t num_ports;
+    uint32_t firstport;
+} OHCIPCIState;
+
+/**
+ * A typical PCI OHCI will additionally set PERR in its configspace to
+ * signal that it got an error.
+ */
+static void ohci_pci_die(struct OHCIState *ohci)
+{
+    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
+
+    ohci_sysbus_die(ohci);
+
+    pci_set_word(dev->parent_obj.config + PCI_STATUS,
+                 PCI_STATUS_DETECTED_PARITY);
+}
+
+static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
+{
+    Error *err = NULL;
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+
+    dev->config[PCI_CLASS_PROG] = 0x10; /* OHCI */
+    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
+
+    usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
+                  ohci->masterbus, ohci->firstport,
+                  pci_get_address_space(dev), ohci_pci_die, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    ohci->state.irq = pci_allocate_irq(dev);
+    pci_register_bar(dev, 0, 0, &ohci->state.mem);
+}
+
+static void usb_ohci_exit(PCIDevice *dev)
+{
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    trace_usb_ohci_exit(s->name);
+    ohci_bus_stop(s);
+
+    if (s->async_td) {
+        usb_cancel_packet(&s->usb_packet);
+        s->async_td = 0;
+    }
+    ohci_stop_endpoints(s);
+
+    if (!ohci->masterbus) {
+        usb_bus_release(&s->bus);
+    }
+
+    timer_del(s->eof_timer);
+    timer_free(s->eof_timer);
+}
+
+static void usb_ohci_reset_pci(DeviceState *d)
+{
+    PCIDevice *dev = PCI_DEVICE(d);
+    OHCIPCIState *ohci = PCI_OHCI(dev);
+    OHCIState *s = &ohci->state;
+
+    ohci_hard_reset(s);
+}
+
+static Property ohci_pci_properties[] = {
+    DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
+    DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
+    DEFINE_PROP_UINT32("firstport", OHCIPCIState, firstport, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vmstate_ohci = {
+    .name = "ohci",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(parent_obj, OHCIPCIState),
+        VMSTATE_STRUCT(state, OHCIPCIState, 1, vmstate_ohci_state, OHCIState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void ohci_pci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->realize = usb_ohci_realize_pci;
+    k->exit = usb_ohci_exit;
+    k->vendor_id = PCI_VENDOR_ID_APPLE;
+    k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
+    k->class_id = PCI_CLASS_SERIAL_USB;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+    dc->desc = "Apple USB Controller";
+    dc->props = ohci_pci_properties;
+    dc->hotpluggable = false;
+    dc->vmsd = &vmstate_ohci;
+    dc->reset = usb_ohci_reset_pci;
+}
+
+static const TypeInfo ohci_pci_info = {
+    .name          = TYPE_PCI_OHCI,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(OHCIPCIState),
+    .class_init    = ohci_pci_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
+static void ohci_pci_register_types(void)
+{
+    type_register_static(&ohci_pci_info);
+}
+
+type_init(ohci_pci_register_types)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 6d3f556989fb..aaba09058896 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -30,87 +30,19 @@
 #include "qapi/error.h"
 #include "qemu/timer.h"
 #include "hw/usb.h"
-#include "hw/pci/pci.h"
 #include "hw/sysbus.h"
 #include "hw/qdev-dma.h"
 #include "trace.h"
+#include "hcd-ohci.h"
 
 /* This causes frames to occur 1000x slower */
 //#define OHCI_TIME_WARP 1
 
-/* Number of Downstream Ports on the root hub.  */
-
-#define OHCI_MAX_PORTS 15
-
 #define ED_LINK_LIMIT 32
 
 static int64_t usb_frame_time;
 static int64_t usb_bit_time;
 
-typedef struct OHCIPort {
-    USBPort port;
-    uint32_t ctrl;
-} OHCIPort;
-
-typedef struct OHCIState {
-    USBBus bus;
-    qemu_irq irq;
-    MemoryRegion mem;
-    AddressSpace *as;
-    uint32_t num_ports;
-    const char *name;
-
-    QEMUTimer *eof_timer;
-    int64_t sof_time;
-
-    /* OHCI state */
-    /* Control partition */
-    uint32_t ctl, status;
-    uint32_t intr_status;
-    uint32_t intr;
-
-    /* memory pointer partition */
-    uint32_t hcca;
-    uint32_t ctrl_head, ctrl_cur;
-    uint32_t bulk_head, bulk_cur;
-    uint32_t per_cur;
-    uint32_t done;
-    int32_t done_count;
-
-    /* Frame counter partition */
-    uint16_t fsmps;
-    uint8_t fit;
-    uint16_t fi;
-    uint8_t frt;
-    uint16_t frame_number;
-    uint16_t padding;
-    uint32_t pstart;
-    uint32_t lst;
-
-    /* Root Hub partition */
-    uint32_t rhdesc_a, rhdesc_b;
-    uint32_t rhstatus;
-    OHCIPort rhport[OHCI_MAX_PORTS];
-
-    /* PXA27x Non-OHCI events */
-    uint32_t hstatus;
-    uint32_t hmask;
-    uint32_t hreset;
-    uint32_t htest;
-
-    /* SM501 local memory offset */
-    dma_addr_t localmem_base;
-
-    /* Active packets.  */
-    uint32_t old_ctl;
-    USBPacket usb_packet;
-    uint8_t usb_buf[8192];
-    uint32_t async_td;
-    bool async_complete;
-
-    void (*ohci_die)(struct OHCIState *ohci);
-} OHCIState;
-
 /* Host Controller Communications Area */
 struct ohci_hcca {
     uint32_t intr[32];
@@ -123,7 +55,6 @@ struct ohci_hcca {
 #define ED_WBACK_OFFSET offsetof(struct ohci_ed, head)
 #define ED_WBACK_SIZE   4
 
-static void ohci_bus_stop(OHCIState *ohci);
 static void ohci_async_cancel_device(OHCIState *ohci, USBDevice *dev);
 
 /* Bitfields for the first word of an Endpoint Desciptor.  */
@@ -430,7 +361,7 @@ static USBDevice *ohci_find_device(OHCIState *ohci, uint8_t addr)
     return NULL;
 }
 
-static void ohci_stop_endpoints(OHCIState *ohci)
+void ohci_stop_endpoints(OHCIState *ohci)
 {
     USBDevice *dev;
     int i, j;
@@ -502,7 +433,7 @@ static void ohci_soft_reset(OHCIState *ohci)
     ohci->lst = OHCI_LS_THRESH;
 }
 
-static void ohci_hard_reset(OHCIState *ohci)
+void ohci_hard_reset(OHCIState *ohci)
 {
     ohci_soft_reset(ohci);
     ohci->ctl = 0;
@@ -1376,7 +1307,7 @@ static int ohci_bus_start(OHCIState *ohci)
 }
 
 /* Stop sending SOF tokens on the bus */
-static void ohci_bus_stop(OHCIState *ohci)
+void ohci_bus_stop(OHCIState *ohci)
 {
     trace_usb_ohci_stop(ohci->name);
     timer_del(ohci->eof_timer);
@@ -1856,10 +1787,10 @@ static USBPortOps ohci_port_ops = {
 static USBBusOps ohci_bus_ops = {
 };
 
-static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
-                          uint32_t num_ports, dma_addr_t localmem_base,
-                          char *masterbus, uint32_t firstport, AddressSpace *as,
-                          void (*ohci_die_fn)(struct OHCIState *), Error **errp)
+void usb_ohci_init(OHCIState *ohci, DeviceState *dev, uint32_t num_ports,
+                   dma_addr_t localmem_base, char *masterbus,
+                   uint32_t firstport, AddressSpace *as,
+                   void (*ohci_die_fn)(struct OHCIState *), Error **errp)
 {
     Error *err = NULL;
     int i;
@@ -1924,25 +1855,11 @@ static void usb_ohci_init(OHCIState *ohci, DeviceState *dev,
                                    ohci_frame_boundary, ohci);
 }
 
-#define TYPE_PCI_OHCI "pci-ohci"
-#define PCI_OHCI(obj) OBJECT_CHECK(OHCIPCIState, (obj), TYPE_PCI_OHCI)
-
-typedef struct {
-    /*< private >*/
-    PCIDevice parent_obj;
-    /*< public >*/
-
-    OHCIState state;
-    char *masterbus;
-    uint32_t num_ports;
-    uint32_t firstport;
-} OHCIPCIState;
-
 /**
  * A typical OHCI will stop operating and set itself into error state
  * (which can be queried by MMIO) to signal that it got an error.
  */
-static void ohci_sysbus_die(struct OHCIState *ohci)
+void ohci_sysbus_die(struct OHCIState *ohci)
 {
     trace_usb_ohci_die();
 
@@ -1950,71 +1867,6 @@ static void ohci_sysbus_die(struct OHCIState *ohci)
     ohci_bus_stop(ohci);
 }
 
-/**
- * A typical PCI OHCI will additionally set PERR in its configspace to
- * signal that it got an error.
- */
-static void ohci_pci_die(struct OHCIState *ohci)
-{
-    OHCIPCIState *dev = container_of(ohci, OHCIPCIState, state);
-
-    ohci_sysbus_die(ohci);
-
-    pci_set_word(dev->parent_obj.config + PCI_STATUS,
-                 PCI_STATUS_DETECTED_PARITY);
-}
-
-static void usb_ohci_realize_pci(PCIDevice *dev, Error **errp)
-{
-    Error *err = NULL;
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-
-    dev->config[PCI_CLASS_PROG] = 0x10; /* OHCI */
-    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
-
-    usb_ohci_init(&ohci->state, DEVICE(dev), ohci->num_ports, 0,
-                  ohci->masterbus, ohci->firstport,
-                  pci_get_address_space(dev), ohci_pci_die, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-
-    ohci->state.irq = pci_allocate_irq(dev);
-    pci_register_bar(dev, 0, 0, &ohci->state.mem);
-}
-
-static void usb_ohci_exit(PCIDevice *dev)
-{
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-    OHCIState *s = &ohci->state;
-
-    trace_usb_ohci_exit(s->name);
-    ohci_bus_stop(s);
-
-    if (s->async_td) {
-        usb_cancel_packet(&s->usb_packet);
-        s->async_td = 0;
-    }
-    ohci_stop_endpoints(s);
-
-    if (!ohci->masterbus) {
-        usb_bus_release(&s->bus);
-    }
-
-    timer_del(s->eof_timer);
-    timer_free(s->eof_timer);
-}
-
-static void usb_ohci_reset_pci(DeviceState *d)
-{
-    PCIDevice *dev = PCI_DEVICE(d);
-    OHCIPCIState *ohci = PCI_OHCI(dev);
-    OHCIState *s = &ohci->state;
-
-    ohci_hard_reset(s);
-}
-
 #define TYPE_SYSBUS_OHCI "sysbus-ohci"
 #define SYSBUS_OHCI(obj) OBJECT_CHECK(OHCISysBusState, (obj), TYPE_SYSBUS_OHCI)
 
@@ -2055,13 +1907,6 @@ static void usb_ohci_reset_sysbus(DeviceState *dev)
     ohci_hard_reset(ohci);
 }
 
-static Property ohci_pci_properties[] = {
-    DEFINE_PROP_STRING("masterbus", OHCIPCIState, masterbus),
-    DEFINE_PROP_UINT32("num-ports", OHCIPCIState, num_ports, 3),
-    DEFINE_PROP_UINT32("firstport", OHCIPCIState, firstport, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
 static const VMStateDescription vmstate_ohci_state_port = {
     .name = "ohci-core/port",
     .version_id = 1,
@@ -2090,7 +1935,7 @@ static const VMStateDescription vmstate_ohci_eof_timer = {
     },
 };
 
-static const VMStateDescription vmstate_ohci_state = {
+const VMStateDescription vmstate_ohci_state = {
     .name = "ohci-core",
     .version_id = 1,
     .minimum_version_id = 1,
@@ -2137,46 +1982,6 @@ static const VMStateDescription vmstate_ohci_state = {
     }
 };
 
-static const VMStateDescription vmstate_ohci = {
-    .name = "ohci",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(parent_obj, OHCIPCIState),
-        VMSTATE_STRUCT(state, OHCIPCIState, 1, vmstate_ohci_state, OHCIState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static void ohci_pci_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->realize = usb_ohci_realize_pci;
-    k->exit = usb_ohci_exit;
-    k->vendor_id = PCI_VENDOR_ID_APPLE;
-    k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
-    k->class_id = PCI_CLASS_SERIAL_USB;
-    set_bit(DEVICE_CATEGORY_USB, dc->categories);
-    dc->desc = "Apple USB Controller";
-    dc->props = ohci_pci_properties;
-    dc->hotpluggable = false;
-    dc->vmsd = &vmstate_ohci;
-    dc->reset = usb_ohci_reset_pci;
-}
-
-static const TypeInfo ohci_pci_info = {
-    .name          = TYPE_PCI_OHCI,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(OHCIPCIState),
-    .class_init    = ohci_pci_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
 static Property ohci_sysbus_properties[] = {
     DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
     DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
@@ -2205,7 +2010,6 @@ static const TypeInfo ohci_sysbus_info = {
 
 static void ohci_register_types(void)
 {
-    type_register_static(&ohci_pci_info);
     type_register_static(&ohci_sysbus_info);
 }
 
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index 593662d28ade..4cbce3a0ed5e 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -6,7 +6,7 @@ config R2D
     select I82378 if TEST_DEVICES
     select IDE_MMIO
     select PFLASH_CFI02
-    select USB_OHCI
+    select USB_OHCI_PCI
     select PCI
     select SM501
     select SH4
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index a1b7acb12a37..564305e28339 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -8,10 +8,14 @@ config USB_UHCI
     select USB
 
 config USB_OHCI
+    bool
+    select USB
+
+config USB_OHCI_PCI
     bool
     default y if PCI_DEVICES
     depends on PCI
-    select USB
+    select USB_OHCI
 
 config USB_EHCI
     bool
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 2b929649acd0..81688f6e7062 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_USB) += desc.o desc-msos.o
 # usb host adapters
 common-obj-$(CONFIG_USB_UHCI) += hcd-uhci.o
 common-obj-$(CONFIG_USB_OHCI) += hcd-ohci.o
+common-obj-$(CONFIG_USB_OHCI_PCI) += hcd-ohci-pci.o
 common-obj-$(CONFIG_USB_EHCI) += hcd-ehci.o hcd-ehci-pci.o
 common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci.o hcd-ehci-sysbus.o
 common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
-- 
2.18.1

[Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
tion=]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                                  ^~
hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~

The xhci code formats the port name into a fixed length
buffer which is only large enough to hold port numbers
upto 5 digits in decimal representation. We're never
going to have a port number that large, so aserting the
port number is sensible is sufficient to tell GCC the
formatted string won't be truncated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190412121626.19829-5-berrange@redhat.com>

[ kraxel: also s/int/unsigned int/ to tell gcc they can't
          go negative. ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d8472b4fea7f..2e9a839f2bf9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
 {
     DeviceState *dev = DEVICE(xhci);
     XHCIPort *port;
-    int i, usbports, speedmask;
+    unsigned int i, usbports, speedmask;
 
     xhci->usbsts = USBSTS_HCH;
 
@@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
                 USB_SPEED_MASK_LOW  |
                 USB_SPEED_MASK_FULL |
                 USB_SPEED_MASK_HIGH;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
             speedmask |= port->speedmask;
         }
@@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
             }
             port->uport = &xhci->uports[i];
             port->speedmask = USB_SPEED_MASK_SUPER;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
             speedmask |= port->speedmask;
         }
-- 
2.18.1

[Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
tion=]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                                  ^~
hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
 3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~

The xhci code formats the port name into a fixed length
buffer which is only large enough to hold port numbers
upto 5 digits in decimal representation. We're never
going to have a port number that large, so aserting the
port number is sensible is sufficient to tell GCC the
formatted string won't be truncated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190412121626.19829-5-berrange@redhat.com>

[ kraxel: also s/int/unsigned int/ to tell gcc they can't
          go negative. ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d8472b4fea7f..2e9a839f2bf9 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
 {
     DeviceState *dev = DEVICE(xhci);
     XHCIPort *port;
-    int i, usbports, speedmask;
+    unsigned int i, usbports, speedmask;
 
     xhci->usbsts = USBSTS_HCH;
 
@@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
                 USB_SPEED_MASK_LOW  |
                 USB_SPEED_MASK_FULL |
                 USB_SPEED_MASK_HIGH;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
             speedmask |= port->speedmask;
         }
@@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
             }
             port->uport = &xhci->uports[i];
             port->speedmask = USB_SPEED_MASK_SUPER;
+            assert(i < MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
             speedmask |= port->speedmask;
         }
-- 
2.18.1

Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

[Philippe Mathieu-Daudé]

Hello,

On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
> hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
> tion=]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                                  ^~
> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>       |                                                      ^~~~~~~~~~~~~~~
> 
> The xhci code formats the port name into a fixed length
> buffer which is only large enough to hold port numbers
> upto 5 digits in decimal representation. We're never
> going to have a port number that large, so aserting the
> port number is sensible is sufficient to tell GCC the
> formatted string won't be truncated.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <20190412121626.19829-5-berrange@redhat.com>
> 
> [ kraxel: also s/int/unsigned int/ to tell gcc they can't
>           go negative. ]
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/hcd-xhci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index d8472b4fea7f..2e9a839f2bf9 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
>  {
>      DeviceState *dev = DEVICE(xhci);
>      XHCIPort *port;
> -    int i, usbports, speedmask;
> +    unsigned int i, usbports, speedmask;
>  
>      xhci->usbsts = USBSTS_HCH;
>  
> @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
>                  USB_SPEED_MASK_LOW  |
>                  USB_SPEED_MASK_FULL |
>                  USB_SPEED_MASK_HIGH;
> +            assert(i < MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
>              }
>              port->uport = &xhci->uports[i];
>              port->speedmask = USB_SPEED_MASK_SUPER;
> +            assert(i < MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> 

I am confused, I upgraded Fedora 32 -> 33 and am now getting this
error back, the assertion being apparently ignored:

C compiler for the host machine: cc (gcc 10.2.1 "cc (GCC) 10.2.1
20201125 (Red Hat 10.2.1-9)")

...
                      QEMU_CFLAGS: -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
-m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -m32
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
                     QEMU_LDFLAGS: -Wl,--warn-common -Wl,-z,relro
-Wl,-z,now -m32  -m32 -fstack-protector-strong
...

[889/5130] Compiling C object libcommon.fa.p/hw_usb_hcd-xhci.c.o
FAILED: libcommon.fa.p/hw_usb_hcd-xhci.c.o
cc -Ilibcommon.fa.p -I. -I.. -I../slirp -I../slirp/src
-I../capstone/include/capstone -I../dtc/libfdt -Iqapi -Itrace -Iui
-Iui/shader -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0
-I/usr/include/pixman-1 -I/usr/include/p11-kit-1
-fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99
-O2 -g -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -m32 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -m32 -Wold-style-declaration
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi
-fstack-protector-strong -isystem linux-headers -isystem linux-headers
-iquote tcg/i386 -iquote . -iquote accel/tcg -iquote include -iquote
disas/libvixl -pthread -fPIC -MD -MQ libcommon.fa.p/hw_usb_hcd-xhci.c.o
-MF libcommon.fa.p/hw_usb_hcd-xhci.c.o.d -o
libcommon.fa.p/hw_usb_hcd-xhci.c.o -c ../hw/usb/hcd-xhci.c
../hw/usb/hcd-xhci.c: In function 'usb_xhci_realize':
../hw/usb/hcd-xhci.c:3309:54: error: '%d' directive output may be
truncated writing between 1 and 8 bytes into a region of size 5
[-Werror=format-truncation=]
 3309 |             snprintf(port->name, sizeof(port->name), "usb2 port
#%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3309:54: note: directive argument in the range [1,
89478486]
In file included from /usr/include/stdio.h:866,
                 from include/qemu/osdep.h:85,
                 from ../hw/usb/hcd-xhci.c:22:
/usr/include/bits/stdio2.h:70:10: note: '__builtin___snprintf_chk'
output between 13 and 20 bytes into a destination of size 16
   70 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL
- 1,
      |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   71 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3323:54: error: '%d' directive output may be
truncated writing between 1 and 8 bytes into a region of size 5
[-Werror=format-truncation=]
 3323 |             snprintf(port->name, sizeof(port->name), "usb3 port
#%d", i+1);
      |                                                      ^~~~~~~~~~~~~~~
../hw/usb/hcd-xhci.c:3323:54: note: directive argument in the range [1,
89478486]
In file included from /usr/include/stdio.h:866,
                 from include/qemu/osdep.h:85,
                 from ../hw/usb/hcd-xhci.c:22:
/usr/include/bits/stdio2.h:70:10: note: '__builtin___snprintf_chk'
output between 13 and 20 bytes into a destination of size 16
   70 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL
- 1,
      |
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   71 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

[Daniel P. Berrangé]

On Mon, Jan 18, 2021 at 12:31:07PM +0100, Philippe Mathieu-Daudé wrote:
> Hello,
> 
> On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
> > hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
> > tion=]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                                  ^~
> > hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
> >  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >       |                                                      ^~~~~~~~~~~~~~~
> > 
> > The xhci code formats the port name into a fixed length
> > buffer which is only large enough to hold port numbers
> > upto 5 digits in decimal representation. We're never
> > going to have a port number that large, so aserting the
> > port number is sensible is sufficient to tell GCC the
> > formatted string won't be truncated.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > Message-Id: <20190412121626.19829-5-berrange@redhat.com>
> > 
> > [ kraxel: also s/int/unsigned int/ to tell gcc they can't
> >           go negative. ]
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> >  hw/usb/hcd-xhci.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index d8472b4fea7f..2e9a839f2bf9 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >  {
> >      DeviceState *dev = DEVICE(xhci);
> >      XHCIPort *port;
> > -    int i, usbports, speedmask;
> > +    unsigned int i, usbports, speedmask;
> >  
> >      xhci->usbsts = USBSTS_HCH;
> >  
> > @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >                  USB_SPEED_MASK_LOW  |
> >                  USB_SPEED_MASK_FULL |
> >                  USB_SPEED_MASK_HIGH;
> > +            assert(i < MAXPORTS);
> >              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
> >              speedmask |= port->speedmask;
> >          }
> > @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
> >              }
> >              port->uport = &xhci->uports[i];
> >              port->speedmask = USB_SPEED_MASK_SUPER;
> > +            assert(i < MAXPORTS);
> >              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
> >              speedmask |= port->speedmask;
> >          }
> > 
> 
> I am confused, I upgraded Fedora 32 -> 33 and am now getting this
> error back, the assertion being apparently ignored:

I'm not seeing this on F33 myself, but our CI is still F32. We
should upgrade that.

What are your configure args ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PULL 7/7] hw/usb: avoid format truncation warning when formatting port name

[Philippe Mathieu-Daudé]

On 1/18/21 12:35 PM, Daniel P. Berrangé wrote:
> On Mon, Jan 18, 2021 at 12:31:07PM +0100, Philippe Mathieu-Daudé wrote:
>> Hello,
>>
>> On 5/3/19 8:59 AM, Gerd Hoffmann wrote:
>>> From: Daniel P. Berrangé <berrange@redhat.com>
>>>
>>> hw/usb/hcd-xhci.c: In function ‘usb_xhci_realize’:
>>> hw/usb/hcd-xhci.c:3339:66: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 5 [-Wformat-trunca\
>>> tion=]
>>>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>       |                                                                  ^~
>>> hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 2147483647]
>>>  3339 |             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>       |                                                      ^~~~~~~~~~~~~~~
>>>
>>> The xhci code formats the port name into a fixed length
>>> buffer which is only large enough to hold port numbers
>>> upto 5 digits in decimal representation. We're never
>>> going to have a port number that large, so aserting the
>>> port number is sensible is sufficient to tell GCC the
>>> formatted string won't be truncated.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Message-Id: <20190412121626.19829-5-berrange@redhat.com>
>>>
>>> [ kraxel: also s/int/unsigned int/ to tell gcc they can't
>>>           go negative. ]
>>>
>>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>>> ---
>>>  hw/usb/hcd-xhci.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>>> index d8472b4fea7f..2e9a839f2bf9 100644
>>> --- a/hw/usb/hcd-xhci.c
>>> +++ b/hw/usb/hcd-xhci.c
>>> @@ -3306,7 +3306,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>  {
>>>      DeviceState *dev = DEVICE(xhci);
>>>      XHCIPort *port;
>>> -    int i, usbports, speedmask;
>>> +    unsigned int i, usbports, speedmask;
>>>  
>>>      xhci->usbsts = USBSTS_HCH;
>>>  
>>> @@ -3336,6 +3336,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>                  USB_SPEED_MASK_LOW  |
>>>                  USB_SPEED_MASK_FULL |
>>>                  USB_SPEED_MASK_HIGH;
>>> +            assert(i < MAXPORTS);
>>>              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>>>              speedmask |= port->speedmask;
>>>          }
>>> @@ -3349,6 +3350,7 @@ static void usb_xhci_init(XHCIState *xhci)
>>>              }
>>>              port->uport = &xhci->uports[i];
>>>              port->speedmask = USB_SPEED_MASK_SUPER;
>>> +            assert(i < MAXPORTS);
>>>              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
>>>              speedmask |= port->speedmask;
>>>          }
>>>
>>
>> I am confused, I upgraded Fedora 32 -> 33 and am now getting this
>> error back, the assertion being apparently ignored:
> 
> I'm not seeing this on F33 myself, but our CI is still F32. We
> should upgrade that.
> 
> What are your configure args ?

Ah sorry, this is my --extra-cflags=-m32 config directory.

Re: [Qemu-devel] [PULL 0/7] Usb 20190503 v2 patches

[Peter Maydell]

On Fri, 3 May 2019 at 08:02, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit f75d15231e56cb0f2bafe19faf1229c459a60731:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-04-30 17:06:57 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/usb-20190503-v2-pull-request
>
> for you to fetch changes up to ccb799313a5926a6aa49018bbc67fe6165fad7f3:
>
>   hw/usb: avoid format truncation warning when formatting port name (2019-05-03 08:56:58 +0200)
>
> ----------------------------------------------------------------
> usb: bugfixes for mtp and xhci, split ohci-pci.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PULL 0/7] Usb 20190503 v2 patches

[Peter Maydell]

On Fri, 3 May 2019 at 08:02, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit f75d15231e56cb0f2bafe19faf1229c459a60731:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-04-30 17:06:57 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/usb-20190503-v2-pull-request
>
> for you to fetch changes up to ccb799313a5926a6aa49018bbc67fe6165fad7f3:
>
>   hw/usb: avoid format truncation warning when formatting port name (2019-05-03 08:56:58 +0200)
>
> ----------------------------------------------------------------
> usb: bugfixes for mtp and xhci, split ohci-pci.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

[Qemu-devel] [PULL 0/7] Usb 20190502 patches

[Gerd Hoffmann]

The following changes since commit f75d15231e56cb0f2bafe19faf1229c459a60731:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-04-30 17:06:57 +0100)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20190502-pull-request

for you to fetch changes up to f3ea801df82991e852862bcaede23a4607859dd8:

  hw/usb: avoid format truncation warning when formatting port name (2019-05-02 09:34:13 +0200)

----------------------------------------------------------------
usb: bugfixes for mtp and xhci, split ohci-pci.

----------------------------------------------------------------

Bandan Das (1):
  usb-mtp: change default to success for usb_mtp_update_object

Daniel P. Berrangé (3):
  usb-mtp: fix string length for filename when writing metadata
  usb-mtp: fix alignment of access of ObjectInfo filename field
  hw/usb: avoid format truncation warning when formatting port name

Longpeng (1):
  usb/xhci: avoid trigger assertion if guest write wrong epid

Thomas Huth (2):
  hw/usb/hcd-ohci: Do not use PCI functions with sysbus devices in
    ohci_die()
  hw/usb/hcd-ohci: Move PCI-related code into a separate file

 hw/usb/hcd-ohci.h     | 104 ++++++++++++++++++++
 hw/usb/dev-mtp.c      |  10 +-
 hw/usb/hcd-ohci-pci.c | 163 +++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c     | 219 ++++--------------------------------------
 hw/usb/hcd-xhci.c     |   4 +-
 hw/sh4/Kconfig        |   2 +-
 hw/usb/Kconfig        |   6 +-
 hw/usb/Makefile.objs  |   1 +
 8 files changed, 301 insertions(+), 208 deletions(-)
 create mode 100644 hw/usb/hcd-ohci.h
 create mode 100644 hw/usb/hcd-ohci-pci.c

-- 
2.18.1

[Qemu-devel] [PULL 2/7] usb-mtp: fix alignment of access of ObjectInfo filename field

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e430..963449ec7de8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.18.1

[Qemu-devel] [PULL 2/7] usb-mtp: fix alignment of access of ObjectInfo filename field

[Gerd Hoffmann]

From: Daniel P. Berrangé <berrange@redhat.com>

The ObjectInfo struct's "filename" field is following a uint8_t
field in a packed struct and thus has bad alignment for a 16-bit
field. Switch the field to to uint8_t and use the helper function
for accessing unaligned 16-bit data.

Note that although the MTP spec specifies big endian, when transported
over the USB protocol, data is little endian.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415154503.6758-4-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-mtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 6b7d1296e430..963449ec7de8 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -226,7 +226,7 @@ typedef struct {
     uint32_t assoc_desc;
     uint32_t seq_no; /*unused*/
     uint8_t length; /*part of filename field*/
-    uint16_t filename[0];
+    uint8_t filename[0]; /* UTF-16 encoded */
     char date_created[0]; /*unused*/
     char date_modified[0]; /*unused*/
     char keywords[0]; /*unused*/
@@ -1551,7 +1551,7 @@ static void usb_mtp_cancel_packet(USBDevice *dev, USBPacket *p)
     fprintf(stderr, "%s\n", __func__);
 }
 
-static char *utf16_to_str(uint8_t len, uint16_t *arr)
+static char *utf16_to_str(uint8_t len, uint8_t *str16)
 {
     wchar_t *wstr = g_new0(wchar_t, len + 1);
     int count, dlen;
@@ -1559,7 +1559,7 @@ static char *utf16_to_str(uint8_t len, uint16_t *arr)
 
     for (count = 0; count < len; count++) {
         /* FIXME: not working for surrogate pairs */
-        wstr[count] = (wchar_t)arr[count];
+        wstr[count] = lduw_le_p(str16 + (count * 2));
     }
     wstr[count] = 0;
 
-- 
2.18.1