Re: [PATCH v2 2/2] hw/usb/hcd-xhci-pci: Add TI TUSB73X0 XHCI controller model

[Bernhard Beschow]

Am 12. Dezember 2024 08:52:07 UTC schrieb Nicholas Piggin <npiggin@gmail.com>:
>The TI TUSB73X0 controller has some interesting differences from NEC,
>notably a separate BAR for MSIX, and PM capabilities. The spec is freely
>available without sign-up.
>
>This controller is accepted by IBM Power proprietary firmware and
>software (when the subsystem IDs are set to Power servers, which is not
>done here). IBM code is picky about device support, so the NEC device
>can not be used.
>
>Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>---
> include/hw/pci/pci_ids.h        |  1 +
> include/hw/usb/xhci.h           |  1 +
> hw/usb/hcd-xhci-ti.c            | 92 +++++++++++++++++++++++++++++++++
> tests/qtest/usb-hcd-xhci-test.c | 21 +++++---
> hw/usb/Kconfig                  |  5 ++
> hw/usb/meson.build              |  1 +
> 6 files changed, 115 insertions(+), 6 deletions(-)
> create mode 100644 hw/usb/hcd-xhci-ti.c
>
>diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
>index f1a53fea8d6..fdb692db513 100644
>--- a/include/hw/pci/pci_ids.h
>+++ b/include/hw/pci/pci_ids.h
>@@ -182,6 +182,7 @@
> #define PCI_VENDOR_ID_HP                 0x103c
> 
> #define PCI_VENDOR_ID_TI                 0x104c
>+#define PCI_DEVICE_ID_TI_TUSB73X0        0x8241
> 
> #define PCI_VENDOR_ID_MOTOROLA           0x1057
> #define PCI_DEVICE_ID_MOTOROLA_MPC106    0x0002
>diff --git a/include/hw/usb/xhci.h b/include/hw/usb/xhci.h
>index 5c90e1373e5..203ec1fca32 100644
>--- a/include/hw/usb/xhci.h
>+++ b/include/hw/usb/xhci.h
>@@ -3,6 +3,7 @@
> 
> #define TYPE_XHCI "base-xhci"
> #define TYPE_NEC_XHCI "nec-usb-xhci"
>+#define TYPE_TI_XHCI "ti-usb-xhci"
> #define TYPE_QEMU_XHCI "qemu-xhci"
> #define TYPE_XHCI_SYSBUS "sysbus-xhci"
> 
>diff --git a/hw/usb/hcd-xhci-ti.c b/hw/usb/hcd-xhci-ti.c
>new file mode 100644
>index 00000000000..6d4b44f6aaf
>--- /dev/null
>+++ b/hw/usb/hcd-xhci-ti.c
>@@ -0,0 +1,92 @@
>+/*
>+ * USB xHCI controller emulation
>+ * Datasheet https://www.ti.com/product/TUSB7340
>+ *
>+ * Copyright (c) 2011 Securiforest
>+ * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
>+ * Based on usb-xhci-nec.c, emulates TI TUSB73X0
>+ *
>+ * 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/usb.h"
>+#include "qemu/module.h"
>+#include "hw/pci/pci.h"
>+#include "hw/qdev-properties.h"
>+
>+#include "hcd-xhci-pci.h"
>+
>+OBJECT_DECLARE_SIMPLE_TYPE(XHCITiState, TI_XHCI)
>+
>+struct XHCITiState {
>+    /*< private >*/
>+    XHCIPciState parent_obj;
>+    /*< public >*/

These markers are obsolete. Instead, a blank line after parent_obj should be 
inserted.

>+    uint32_t intrs;
>+    uint32_t slots;
>+};
>+
>+static Property ti_xhci_properties[] = {

s/static Property/static const Property/ as of recent tree-wide changes.

Best regards,
Bernhard

>+    DEFINE_PROP_ON_OFF_AUTO("msi", XHCIPciState, msi, ON_OFF_AUTO_AUTO),
>+    DEFINE_PROP_ON_OFF_AUTO("msix", XHCIPciState, msix, ON_OFF_AUTO_AUTO),
>+    DEFINE_PROP_UINT32("intrs", XHCITiState, intrs, 8),
>+    DEFINE_PROP_UINT32("slots", XHCITiState, slots, XHCI_MAXSLOTS),
>+    DEFINE_PROP_END_OF_LIST(),
>+};
>+
>+static void ti_xhci_instance_init(Object *obj)
>+{
>+    XHCIPciState *pci = XHCI_PCI(obj);
>+    XHCITiState *ti = TI_XHCI(obj);
>+
>+    pci->xhci.numintrs = ti->intrs;
>+    pci->xhci.numslots = ti->slots;
>+
>+    pci->cache_line_size = 0x0;
>+    pci->pm_cap_off = 0x40;
>+    pci->pcie_cap_off = 0x70;
>+    pci->msi_cap_off = 0x48;
>+    pci->msix_cap_off = 0xc0;
>+    pci->msix_bar_nr = 0x2;
>+    pci->msix_bar_size = 0x800000;
>+    pci->msix_table_off = 0x0;
>+    pci->msix_pba_off = 0x1000;
>+}
>+
>+static void ti_xhci_class_init(ObjectClass *klass, void *data)
>+{
>+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>+    DeviceClass *dc = DEVICE_CLASS(klass);
>+
>+    device_class_set_props(dc, ti_xhci_properties);
>+    k->vendor_id    = PCI_VENDOR_ID_TI;
>+    k->device_id    = PCI_DEVICE_ID_TI_TUSB73X0;
>+    k->revision     = 0x02;
>+}
>+
>+static const TypeInfo ti_xhci_info = {
>+    .name          = TYPE_TI_XHCI,
>+    .parent        = TYPE_XHCI_PCI,
>+    .instance_size = sizeof(XHCITiState),
>+    .instance_init = ti_xhci_instance_init,
>+    .class_init    = ti_xhci_class_init,
>+};
>+
>+static void ti_xhci_register_types(void)
>+{
>+    type_register_static(&ti_xhci_info);
>+}
>+
>+type_init(ti_xhci_register_types)
>diff --git a/tests/qtest/usb-hcd-xhci-test.c b/tests/qtest/usb-hcd-xhci-test.c
>index 93614e55461..d4a0d8cc217 100644
>--- a/tests/qtest/usb-hcd-xhci-test.c
>+++ b/tests/qtest/usb-hcd-xhci-test.c
>@@ -59,6 +59,9 @@ typedef struct XHCIQState {
> #define XHCI_NEC_ID (PCI_DEVICE_ID_NEC_UPD720200 << 16 | \
>                      PCI_VENDOR_ID_NEC)
> 
>+#define XHCI_TI_ID  (PCI_DEVICE_ID_TI_TUSB73X0 << 16 | \
>+                     PCI_VENDOR_ID_TI)
>+
> /**
>  * Locate, verify, and return a handle to the XHCI device.
>  */
>@@ -78,6 +81,8 @@ static QPCIDevice *get_xhci_device(QTestState *qts, uint32_t 
>*fingerprint)
>     switch (xhci_fingerprint) {
>     case XHCI_NEC_ID:
>         break;
>+    case XHCI_TI_ID:
>+        break;
>     default:
>         /* Unknown device. */
>         g_assert_not_reached();
>@@ -134,11 +139,12 @@ static XHCIQState *xhci_boot(const char *cli, ...)
>         va_end(ap);
>     } else {
>         s = xhci_boot("-M q35 "
>-                      "-device nec-usb-xhci,id=xhci,bus=pcie.0,addr=1d.0 "
>+                      "-device %s,id=xhci,bus=pcie.0,addr=1d.0 "
>                       "-drive id=drive0,if=none,file=null-co://,"
>-                          "file.read-zeroes=on,format=raw");
>+                          "file.read-zeroes=on,format=raw",
>+                      qtest_has_device("ti-usb-xhci") ?
>+                          "ti-usb-xhci" : "nec-usb-xhci");
>     }
>-
>     return s;
> }
> 
>@@ -392,10 +398,12 @@ static void pci_xhci_stress_rings(void)
>     int i;
> 
>     s = xhci_boot("-M q35 "
>-            "-device nec-usb-xhci,id=xhci,bus=pcie.0,addr=1d.0 "
>+            "-device %s,id=xhci,bus=pcie.0,addr=1d.0 "
>             "-device usb-storage,bus=xhci.0,drive=drive0 "
>             "-drive id=drive0,if=none,file=null-co://,"
>-                "file.read-zeroes=on,format=raw "
>+                "file.read-zeroes=on,format=raw ",
>+            qtest_has_device("ti-usb-xhci") ?
>+                "ti-usb-xhci" : "nec-usb-xhci"
>             );
> 
>     hcsparams1 = xhci_cap_readl(s, 0x4); /* HCSPARAMS1 */
>@@ -567,7 +575,8 @@ int main(int argc, char **argv)
>         return 0;
>     }
> 
>-    if (!qtest_has_device("nec-usb-xhci")) {
>+    if (!qtest_has_device("nec-usb-xhci") &&
>+        !qtest_has_device("ti-usb-xhci")) {
>         return 0;
>     }
> 
>diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
>index 5fbecd2f43b..8e5c4747af9 100644
>--- a/hw/usb/Kconfig
>+++ b/hw/usb/Kconfig
>@@ -49,6 +49,11 @@ config USB_XHCI_NEC
>     default y if PCI_DEVICES
>     select USB_XHCI_PCI
> 
>+config USB_XHCI_TI
>+    bool
>+    default y if PCI_DEVICES
>+    select USB_XHCI_PCI
>+
> config USB_XHCI_SYSBUS
>     bool
>     select USB_XHCI
>diff --git a/hw/usb/meson.build b/hw/usb/meson.build
>index 1b4d1507e41..b874a93f16e 100644
>--- a/hw/usb/meson.build
>+++ b/hw/usb/meson.build
>@@ -23,6 +23,7 @@ system_ss.add(when: 'CONFIG_USB_XHCI', if_true: 
>files('hcd-xhci.c'))
> system_ss.add(when: 'CONFIG_USB_XHCI_PCI', if_true: files('hcd-xhci-pci.c'))
> system_ss.add(when: 'CONFIG_USB_XHCI_SYSBUS', if_true: 
> files('hcd-xhci-sysbus.c'))
> system_ss.add(when: 'CONFIG_USB_XHCI_NEC', if_true: files('hcd-xhci-nec.c'))
>+system_ss.add(when: 'CONFIG_USB_XHCI_TI', if_true: files('hcd-xhci-ti.c'))
> system_ss.add(when: 'CONFIG_USB_DWC2', if_true: files('hcd-dwc2.c'))
> system_ss.add(when: 'CONFIG_USB_DWC3', if_true: files('hcd-dwc3.c'))
>