qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations
@ 2020-03-13  1:45 Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

This patch series wires up the USB controllers on fsl-imx6 and fsl-imx6ul
emulations.

The first patch provides a basic implementation of the USB PHY controller
used in i.MX28 and later chips. Only reset bit handling in the control
register is actually implemented. Basic USB PHY support is needed to make
the USB ports operational in Linux.

The second patch fixes USB and USB PHY interrupt numbers for i.MX6UL.

The third patch instantiates unimplemented pwm and can devices. This patch
is necessary to avoid crashes in Linux when it tries to access those
devices. The crashes are observed when trying to boot Linux v4.21 or later.

The final two patches instantiate the USB controllers for i.mMX6 and
i.MX6UL.

v3:
- Minor cleanup in patch 1/5 (see details in patch)
- Added patch to fix USB and USB PHY interrupt numbers for fsl-imx6ul.
- Added patch to instantiate unimplemented pwm and CAN devices.
- Instantiate USB and USB PHY separately. They are logically different,
  and the number of instances is not always the same.

v2:
- Implement and instantiate basic USB PHY implementation
  instead of emulating a single USB PHY register

----------------------------------------------------------------
Guenter Roeck (5):
      hw/usb: Add basic i.MX USB Phy support
      hw/arm/fsl-imx6ul: Fix USB interrupt numbers
      hw/arm/fsl-imx6ul: Instantiate unimplemented pwm and can devices
      hw/arm/fsl-imx6ul: Wire up USB controllers
      hw/arm/fsl-imx6: Wire up USB controllers

 MAINTAINERS                  |   2 +
 hw/arm/Kconfig               |   1 +
 hw/arm/fsl-imx6.c            |  36 +++++++
 hw/arm/fsl-imx6ul.c          |  49 ++++++++++
 hw/usb/Kconfig               |   5 +
 hw/usb/Makefile.objs         |   2 +
 hw/usb/imx-usb-phy.c         | 225 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/fsl-imx6.h    |   6 ++
 include/hw/arm/fsl-imx6ul.h  |  16 ++-
 include/hw/usb/imx-usb-phy.h |  53 ++++++++++
 10 files changed, 392 insertions(+), 3 deletions(-)
 create mode 100644 hw/usb/imx-usb-phy.c
 create mode 100644 include/hw/usb/imx-usb-phy.h


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
@ 2020-03-13  1:45 ` Guenter Roeck
  2021-05-19 19:00   ` Philippe Mathieu-Daudé
  2023-03-16 13:41   ` Peter Maydell
  2020-03-13  1:45 ` [PATCH v3 2/5] hw/arm/fsl-imx6ul: Fix USB interrupt numbers Guenter Roeck
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
and i.MX7 SoCs.

The only support really needed - at least to boot Linux - is support
for soft reset, which needs to reset various registers to their initial
value. Otherwise, just record register values.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Added Reviewed-by:;
    dropped duplicate "the" in comments;
    added new files to MAINTAINERS
v2: New patch, replacing dummy STMP register support with basic USB PHY
    emulation.

 MAINTAINERS                  |   2 +
 hw/arm/Kconfig               |   1 +
 hw/usb/Kconfig               |   5 +
 hw/usb/Makefile.objs         |   2 +
 hw/usb/imx-usb-phy.c         | 225 +++++++++++++++++++++++++++++++++++
 include/hw/usb/imx-usb-phy.h |  53 +++++++++
 6 files changed, 288 insertions(+)
 create mode 100644 hw/usb/imx-usb-phy.c
 create mode 100644 include/hw/usb/imx-usb-phy.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d881ba7d9c..1cfdeeae32 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -748,6 +748,8 @@ F: hw/arm/sabrelite.c
 F: hw/arm/fsl-imx6.c
 F: hw/misc/imx6_*.c
 F: hw/ssi/imx_spi.c
+F: hw/usb/imx-usb-phy.c
+F: include/hw/usb/imx-usb-phy.h
 F: include/hw/arm/fsl-imx6.h
 F: include/hw/misc/imx6_*.h
 F: include/hw/ssi/imx_spi.h
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index bc54fd61f9..21c627c3b7 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -361,6 +361,7 @@ config FSL_IMX6
     select IMX
     select IMX_FEC
     select IMX_I2C
+    select IMX_USBPHY
     select SDHCI
 
 config ASPEED_SOC
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 5e70ed5f7b..464348ba14 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -91,3 +91,8 @@ config USB_STORAGE_MTP
     bool
     default y
     depends on USB
+
+config IMX_USBPHY
+    bool
+    default y
+    depends on USB
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 2b10868937..66835e5bf7 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -61,3 +61,5 @@ common-obj-$(CONFIG_XEN) += xen-usb.o
 xen-usb.o-cflags := $(LIBUSB_CFLAGS)
 xen-usb.o-libs := $(LIBUSB_LIBS)
 endif
+
+common-obj-$(CONFIG_IMX_USBPHY) += imx-usb-phy.o
diff --git a/hw/usb/imx-usb-phy.c b/hw/usb/imx-usb-phy.c
new file mode 100644
index 0000000000..e705a03a1f
--- /dev/null
+++ b/hw/usb/imx-usb-phy.c
@@ -0,0 +1,225 @@
+/*
+ * i.MX USB PHY
+ *
+ * Copyright (c) 2020 Guenter Roeck <linux@roeck-us.net>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * We need to implement basic reset control in the PHY control register.
+ * For everything else, it is sufficient to set whatever is written.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/usb/imx-usb-phy.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+
+static const VMStateDescription vmstate_imx_usbphy = {
+    .name = TYPE_IMX_USBPHY,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(usbphy, IMXUSBPHYState, USBPHY_MAX),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void imx_usbphy_softreset(IMXUSBPHYState *s)
+{
+    s->usbphy[USBPHY_PWD] = 0x001e1c00;
+    s->usbphy[USBPHY_TX] = 0x10060607;
+    s->usbphy[USBPHY_RX] = 0x00000000;
+    s->usbphy[USBPHY_CTRL] = 0xc0200000;
+}
+
+static void imx_usbphy_reset(DeviceState *dev)
+{
+    IMXUSBPHYState *s = IMX_USBPHY(dev);
+
+    s->usbphy[USBPHY_STATUS] = 0x00000000;
+    s->usbphy[USBPHY_DEBUG] = 0x7f180000;
+    s->usbphy[USBPHY_DEBUG0_STATUS] = 0x00000000;
+    s->usbphy[USBPHY_DEBUG1] = 0x00001000;
+    s->usbphy[USBPHY_VERSION] = 0x04020000;
+
+    imx_usbphy_softreset(s);
+}
+
+static uint64_t imx_usbphy_read(void *opaque, hwaddr offset, unsigned size)
+{
+    IMXUSBPHYState *s = (IMXUSBPHYState *)opaque;
+    uint32_t index = offset >> 2;
+    uint32_t value;
+
+    switch (index) {
+    case USBPHY_PWD_SET:
+    case USBPHY_TX_SET:
+    case USBPHY_RX_SET:
+    case USBPHY_CTRL_SET:
+    case USBPHY_DEBUG_SET:
+    case USBPHY_DEBUG1_SET:
+        /*
+         * All REG_NAME_SET register access are in fact targeting the
+         * REG_NAME register.
+         */
+        value = s->usbphy[index - 1];
+        break;
+    case USBPHY_PWD_CLR:
+    case USBPHY_TX_CLR:
+    case USBPHY_RX_CLR:
+    case USBPHY_CTRL_CLR:
+    case USBPHY_DEBUG_CLR:
+    case USBPHY_DEBUG1_CLR:
+        /*
+         * All REG_NAME_CLR register access are in fact targeting the
+         * REG_NAME register.
+         */
+        value = s->usbphy[index - 2];
+        break;
+    case USBPHY_PWD_TOG:
+    case USBPHY_TX_TOG:
+    case USBPHY_RX_TOG:
+    case USBPHY_CTRL_TOG:
+    case USBPHY_DEBUG_TOG:
+    case USBPHY_DEBUG1_TOG:
+        /*
+         * All REG_NAME_TOG register access are in fact targeting the
+         * REG_NAME register.
+         */
+        value = s->usbphy[index - 3];
+        break;
+    default:
+        value = s->usbphy[index];
+        break;
+    }
+    return (uint64_t)value;
+}
+
+static void imx_usbphy_write(void *opaque, hwaddr offset, uint64_t value,
+                             unsigned size)
+{
+    IMXUSBPHYState *s = (IMXUSBPHYState *)opaque;
+    uint32_t index = offset >> 2;
+
+    switch (index) {
+    case USBPHY_CTRL:
+        s->usbphy[index] = value;
+        if (value & USBPHY_CTRL_SFTRST) {
+            imx_usbphy_softreset(s);
+        }
+        break;
+    case USBPHY_PWD:
+    case USBPHY_TX:
+    case USBPHY_RX:
+    case USBPHY_STATUS:
+    case USBPHY_DEBUG:
+    case USBPHY_DEBUG1:
+        s->usbphy[index] = value;
+        break;
+    case USBPHY_CTRL_SET:
+        s->usbphy[index - 1] |= value;
+        if (value & USBPHY_CTRL_SFTRST) {
+            imx_usbphy_softreset(s);
+        }
+        break;
+    case USBPHY_PWD_SET:
+    case USBPHY_TX_SET:
+    case USBPHY_RX_SET:
+    case USBPHY_DEBUG_SET:
+    case USBPHY_DEBUG1_SET:
+        /*
+         * All REG_NAME_SET register access are in fact targeting the
+         * REG_NAME register. So we change the value of the REG_NAME
+         * register, setting bits passed in the value.
+         */
+        s->usbphy[index - 1] |= value;
+        break;
+    case USBPHY_PWD_CLR:
+    case USBPHY_TX_CLR:
+    case USBPHY_RX_CLR:
+    case USBPHY_CTRL_CLR:
+    case USBPHY_DEBUG_CLR:
+    case USBPHY_DEBUG1_CLR:
+        /*
+         * All REG_NAME_CLR register access are in fact targeting the
+         * REG_NAME register. So we change the value of the REG_NAME
+         * register, unsetting bits passed in the value.
+         */
+        s->usbphy[index - 2] &= ~value;
+        break;
+    case USBPHY_CTRL_TOG:
+        s->usbphy[index - 3] ^= value;
+        if ((value & USBPHY_CTRL_SFTRST) &&
+            (s->usbphy[index - 3] & USBPHY_CTRL_SFTRST)) {
+            imx_usbphy_softreset(s);
+        }
+        break;
+    case USBPHY_PWD_TOG:
+    case USBPHY_TX_TOG:
+    case USBPHY_RX_TOG:
+    case USBPHY_DEBUG_TOG:
+    case USBPHY_DEBUG1_TOG:
+        /*
+         * All REG_NAME_TOG register access are in fact targeting the
+         * REG_NAME register. So we change the value of the REG_NAME
+         * register, toggling bits passed in the value.
+         */
+        s->usbphy[index - 3] ^= value;
+        break;
+    default:
+        /* Other registers are read-only */
+        break;
+    }
+}
+
+static const struct MemoryRegionOps imx_usbphy_ops = {
+    .read = imx_usbphy_read,
+    .write = imx_usbphy_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        /*
+         * Our device would not work correctly if the guest was doing
+         * unaligned access. This might not be a limitation on the real
+         * device but in practice there is no reason for a guest to access
+         * this device unaligned.
+         */
+        .min_access_size = 4,
+        .max_access_size = 4,
+        .unaligned = false,
+    },
+};
+
+static void imx_usbphy_realize(DeviceState *dev, Error **errp)
+{
+    IMXUSBPHYState *s = IMX_USBPHY(dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &imx_usbphy_ops, s,
+                          "imx-usbphy", 0x1000);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
+}
+
+static void imx_usbphy_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = imx_usbphy_reset;
+    dc->vmsd = &vmstate_imx_usbphy;
+    dc->desc = "i.MX USB PHY Module";
+    dc->realize = imx_usbphy_realize;
+}
+
+static const TypeInfo imx_usbphy_info = {
+    .name          = TYPE_IMX_USBPHY,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(IMXUSBPHYState),
+    .class_init    = imx_usbphy_class_init,
+};
+
+static void imx_usbphy_register_types(void)
+{
+    type_register_static(&imx_usbphy_info);
+}
+
+type_init(imx_usbphy_register_types)
diff --git a/include/hw/usb/imx-usb-phy.h b/include/hw/usb/imx-usb-phy.h
new file mode 100644
index 0000000000..07f0235d10
--- /dev/null
+++ b/include/hw/usb/imx-usb-phy.h
@@ -0,0 +1,53 @@
+#ifndef IMX_USB_PHY_H
+#define IMX_USB_PHY_H
+
+#include "hw/sysbus.h"
+#include "qemu/bitops.h"
+
+enum IMXUsbPhyRegisters {
+    USBPHY_PWD,
+    USBPHY_PWD_SET,
+    USBPHY_PWD_CLR,
+    USBPHY_PWD_TOG,
+    USBPHY_TX,
+    USBPHY_TX_SET,
+    USBPHY_TX_CLR,
+    USBPHY_TX_TOG,
+    USBPHY_RX,
+    USBPHY_RX_SET,
+    USBPHY_RX_CLR,
+    USBPHY_RX_TOG,
+    USBPHY_CTRL,
+    USBPHY_CTRL_SET,
+    USBPHY_CTRL_CLR,
+    USBPHY_CTRL_TOG,
+    USBPHY_STATUS,
+    USBPHY_DEBUG = 0x14,
+    USBPHY_DEBUG_SET,
+    USBPHY_DEBUG_CLR,
+    USBPHY_DEBUG_TOG,
+    USBPHY_DEBUG0_STATUS,
+    USBPHY_DEBUG1 = 0x1c,
+    USBPHY_DEBUG1_SET,
+    USBPHY_DEBUG1_CLR,
+    USBPHY_DEBUG1_TOG,
+    USBPHY_VERSION,
+    USBPHY_MAX
+};
+
+#define USBPHY_CTRL_SFTRST BIT(31)
+
+#define TYPE_IMX_USBPHY "imx.usbphy"
+#define IMX_USBPHY(obj) OBJECT_CHECK(IMXUSBPHYState, (obj), TYPE_IMX_USBPHY)
+
+typedef struct IMXUSBPHYState {
+    /* <private> */
+    SysBusDevice parent_obj;
+
+    /* <public> */
+    MemoryRegion iomem;
+
+    uint32_t usbphy[USBPHY_MAX];
+} IMXUSBPHYState;
+
+#endif /* IMX_USB_PHY_H */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/5] hw/arm/fsl-imx6ul: Fix USB interrupt numbers
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
@ 2020-03-13  1:45 ` Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 3/5] hw/arm/fsl-imx6ul: Instantiate unimplemented pwm and can devices Guenter Roeck
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

USB1 and USB2 interrupt numbers were swapped. USB_PHY2 interrupt number
is 45. That didn't really matter up to now since the interrupts were not
used, but it needs to be fixed to be able to wire up the USB controllers.

Fixes: 31cbf933f0e ("i.MX6UL: Add i.MX6UL SOC")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: New patch

 include/hw/arm/fsl-imx6ul.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index eda389aec7..5a420785b9 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -241,10 +241,10 @@ enum FslIMX6ULIRQs {
     FSL_IMX6UL_UART7_IRQ    = 39,
     FSL_IMX6UL_UART8_IRQ    = 40,
 
-    FSL_IMX6UL_USB1_IRQ     = 42,
-    FSL_IMX6UL_USB2_IRQ     = 43,
+    FSL_IMX6UL_USB1_IRQ     = 43,
+    FSL_IMX6UL_USB2_IRQ     = 42,
     FSL_IMX6UL_USB_PHY1_IRQ = 44,
-    FSL_IMX6UL_USB_PHY2_IRQ = 44,
+    FSL_IMX6UL_USB_PHY2_IRQ = 45,
 
     FSL_IMX6UL_CAAM_JQ2_IRQ = 46,
     FSL_IMX6UL_CAAM_ERR_IRQ = 47,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/5] hw/arm/fsl-imx6ul: Instantiate unimplemented pwm and can devices
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 2/5] hw/arm/fsl-imx6ul: Fix USB interrupt numbers Guenter Roeck
@ 2020-03-13  1:45 ` Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 4/5] hw/arm/fsl-imx6ul: Wire up USB controllers Guenter Roeck
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

Recent Linux kernels (post v4.20) crash due to accesses to flexcan
and pwm controllers. Instantiate as unimplemented devices to work
around the problem.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: New patch

 hw/arm/fsl-imx6ul.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index c405b68d1d..a0bcc6f895 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -516,6 +516,20 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
      */
     create_unimplemented_device("sdma", FSL_IMX6UL_SDMA_ADDR, 0x4000);
 
+    /*
+     * PWM
+     */
+    create_unimplemented_device("pwm1", FSL_IMX6UL_PWM1_ADDR, 0x4000);
+    create_unimplemented_device("pwm2", FSL_IMX6UL_PWM2_ADDR, 0x4000);
+    create_unimplemented_device("pwm3", FSL_IMX6UL_PWM3_ADDR, 0x4000);
+    create_unimplemented_device("pwm4", FSL_IMX6UL_PWM4_ADDR, 0x4000);
+
+    /*
+     * CAN
+     */
+    create_unimplemented_device("can1", FSL_IMX6UL_CAN1_ADDR, 0x4000);
+    create_unimplemented_device("can2", FSL_IMX6UL_CAN2_ADDR, 0x4000);
+
     /*
      * APHB_DMA
      */
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/5] hw/arm/fsl-imx6ul: Wire up USB controllers
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
                   ` (2 preceding siblings ...)
  2020-03-13  1:45 ` [PATCH v3 3/5] hw/arm/fsl-imx6ul: Instantiate unimplemented pwm and can devices Guenter Roeck
@ 2020-03-13  1:45 ` Guenter Roeck
  2020-03-13  1:45 ` [PATCH v3 5/5] hw/arm/fsl-imx6: " Guenter Roeck
  2020-03-13 11:25 ` [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Peter Maydell
  5 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

IMX6UL USB controllers are quite similar to IMX7 USB controllers.
Wire them up the same way.

The only real difference is that wiring up phy devices is necessary
to avoid phy reset timeouts in the Linux kernel.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Wire up USB and USB PHY controllers in separate loops
    While the number of USB controllers matches the number of USB PHYs,
    they are logically different.
v2: Use USB PHY emulation

 hw/arm/fsl-imx6ul.c         | 35 +++++++++++++++++++++++++++++++++++
 include/hw/arm/fsl-imx6ul.h | 10 ++++++++++
 2 files changed, 45 insertions(+)

diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index a0bcc6f895..99a5859a4e 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -20,6 +20,7 @@
 #include "qapi/error.h"
 #include "hw/arm/fsl-imx6ul.h"
 #include "hw/misc/unimp.h"
+#include "hw/usb/imx-usb-phy.h"
 #include "hw/boards.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
@@ -133,6 +134,18 @@ static void fsl_imx6ul_init(Object *obj)
                               TYPE_IMX_ENET);
     }
 
+    /* USB */
+    for (i = 0; i < FSL_IMX6UL_NUM_USB_PHYS; i++) {
+        snprintf(name, NAME_SIZE, "usbphy%d", i);
+        sysbus_init_child_obj(obj, name, &s->usbphy[i], sizeof(s->usbphy[i]),
+                              TYPE_IMX_USBPHY);
+    }
+    for (i = 0; i < FSL_IMX6UL_NUM_USBS; i++) {
+        snprintf(name, NAME_SIZE, "usb%d", i);
+        sysbus_init_child_obj(obj, name, &s->usb[i], sizeof(s->usb[i]),
+                              TYPE_CHIPIDEA);
+    }
+
     /*
      * SDHCI
      */
@@ -456,6 +469,28 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
                                             FSL_IMX6UL_ENETn_TIMER_IRQ[i]));
     }
 
+    /* USB */
+    for (i = 0; i < FSL_IMX6UL_NUM_USB_PHYS; i++) {
+        object_property_set_bool(OBJECT(&s->usbphy[i]), true, "realized",
+                                 &error_abort);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->usbphy[i]), 0,
+                        FSL_IMX6UL_USBPHY1_ADDR + i * 0x1000);
+    }
+
+    for (i = 0; i < FSL_IMX6UL_NUM_USBS; i++) {
+        static const int FSL_IMX6UL_USBn_IRQ[] = {
+            FSL_IMX6UL_USB1_IRQ,
+            FSL_IMX6UL_USB2_IRQ,
+        };
+        object_property_set_bool(OBJECT(&s->usb[i]), true, "realized",
+                                 &error_abort);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->usb[i]), 0,
+                        FSL_IMX6UL_USBO2_USB_ADDR + i * 0x200);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->usb[i]), 0,
+                           qdev_get_gpio_in(DEVICE(&s->a7mpcore),
+                                            FSL_IMX6UL_USBn_IRQ[i]));
+    }
+
     /*
      * USDHC
      */
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 5a420785b9..1a0bab8daa 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -34,6 +34,8 @@
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/imx_spi.h"
 #include "hw/net/imx_fec.h"
+#include "hw/usb/chipidea.h"
+#include "hw/usb/imx-usb-phy.h"
 #include "exec/memory.h"
 #include "cpu.h"
 
@@ -54,6 +56,8 @@ enum FslIMX6ULConfiguration {
     FSL_IMX6UL_NUM_I2CS         = 4,
     FSL_IMX6UL_NUM_ECSPIS       = 4,
     FSL_IMX6UL_NUM_ADCS         = 2,
+    FSL_IMX6UL_NUM_USB_PHYS     = 2,
+    FSL_IMX6UL_NUM_USBS         = 2,
 };
 
 typedef struct FslIMX6ULState {
@@ -77,6 +81,8 @@ typedef struct FslIMX6ULState {
     IMXFECState        eth[FSL_IMX6UL_NUM_ETHS];
     SDHCIState         usdhc[FSL_IMX6UL_NUM_USDHCS];
     IMX2WdtState       wdt[FSL_IMX6UL_NUM_WDTS];
+    IMXUSBPHYState     usbphy[FSL_IMX6UL_NUM_USB_PHYS];
+    ChipideaState      usb[FSL_IMX6UL_NUM_USBS];
     MemoryRegion       rom;
     MemoryRegion       caam;
     MemoryRegion       ocram;
@@ -145,6 +151,10 @@ enum FslIMX6ULMemoryMap {
     FSL_IMX6UL_EPIT2_ADDR           = 0x020D4000,
     FSL_IMX6UL_EPIT1_ADDR           = 0x020D0000,
     FSL_IMX6UL_SNVS_HP_ADDR         = 0x020CC000,
+    FSL_IMX6UL_USBPHY2_ADDR         = 0x020CA000,
+    FSL_IMX6UL_USBPHY2_SIZE         = (4 * 1024),
+    FSL_IMX6UL_USBPHY1_ADDR         = 0x020C9000,
+    FSL_IMX6UL_USBPHY1_SIZE         = (4 * 1024),
     FSL_IMX6UL_ANALOG_ADDR          = 0x020C8000,
     FSL_IMX6UL_CCM_ADDR             = 0x020C4000,
     FSL_IMX6UL_WDOG2_ADDR           = 0x020C0000,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/5] hw/arm/fsl-imx6: Wire up USB controllers
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
                   ` (3 preceding siblings ...)
  2020-03-13  1:45 ` [PATCH v3 4/5] hw/arm/fsl-imx6ul: Wire up USB controllers Guenter Roeck
@ 2020-03-13  1:45 ` Guenter Roeck
  2020-03-13 11:25 ` [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Peter Maydell
  5 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2020-03-13  1:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel, qemu-arm, Gerd Hoffmann, Guenter Roeck,
	Jean-Christophe Dubois

With this patch, the USB controllers on 'sabrelite' are detected
and can be used to boot the system.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Wire up USB and USB PHY controllers separately.
    The number of USB controllers does not match the number of USB PHYs,
    and they are logically different. 
v2: Use USB PHY emulation

 hw/arm/fsl-imx6.c         | 36 ++++++++++++++++++++++++++++++++++++
 include/hw/arm/fsl-imx6.h |  6 ++++++
 2 files changed, 42 insertions(+)

diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index ecc62855f2..e095e4abc6 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/arm/fsl-imx6.h"
+#include "hw/usb/imx-usb-phy.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/sysemu.h"
@@ -86,6 +87,17 @@ static void fsl_imx6_init(Object *obj)
                               TYPE_IMX_USDHC);
     }
 
+    for (i = 0; i < FSL_IMX6_NUM_USB_PHYS; i++) {
+        snprintf(name, NAME_SIZE, "usbphy%d", i);
+        sysbus_init_child_obj(obj, name, &s->usbphy[i], sizeof(s->usbphy[i]),
+                              TYPE_IMX_USBPHY);
+    }
+    for (i = 0; i < FSL_IMX6_NUM_USBS; i++) {
+        snprintf(name, NAME_SIZE, "usb%d", i);
+        sysbus_init_child_obj(obj, name, &s->usb[i], sizeof(s->usb[i]),
+                              TYPE_CHIPIDEA);
+    }
+
     for (i = 0; i < FSL_IMX6_NUM_ECSPIS; i++) {
         snprintf(name, NAME_SIZE, "spi%d", i + 1);
         sysbus_init_child_obj(obj, name, &s->spi[i], sizeof(s->spi[i]),
@@ -349,6 +361,30 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
                                             esdhc_table[i].irq));
     }
 
+    /* USB */
+    for (i = 0; i < FSL_IMX6_NUM_USB_PHYS; i++) {
+        object_property_set_bool(OBJECT(&s->usbphy[i]), true, "realized",
+                                 &error_abort);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->usbphy[i]), 0,
+                        FSL_IMX6_USBPHY1_ADDR + i * 0x1000);
+    }
+    for (i = 0; i < FSL_IMX6_NUM_USBS; i++) {
+        static const int FSL_IMX6_USBn_IRQ[] = {
+            FSL_IMX6_USB_OTG_IRQ,
+            FSL_IMX6_USB_HOST1_IRQ,
+            FSL_IMX6_USB_HOST2_IRQ,
+            FSL_IMX6_USB_HOST3_IRQ,
+        };
+
+        object_property_set_bool(OBJECT(&s->usb[i]), true, "realized",
+                                 &error_abort);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->usb[i]), 0,
+                        FSL_IMX6_USBOH3_USB_ADDR + i * 0x200);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->usb[i]), 0,
+                           qdev_get_gpio_in(DEVICE(&s->a9mpcore),
+                                            FSL_IMX6_USBn_IRQ[i]));
+    }
+
     /* Initialize all ECSPI */
     for (i = 0; i < FSL_IMX6_NUM_ECSPIS; i++) {
         static const struct {
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 60eadccb42..973bcb72f7 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -30,6 +30,8 @@
 #include "hw/sd/sdhci.h"
 #include "hw/ssi/imx_spi.h"
 #include "hw/net/imx_fec.h"
+#include "hw/usb/chipidea.h"
+#include "hw/usb/imx-usb-phy.h"
 #include "exec/memory.h"
 #include "cpu.h"
 
@@ -44,6 +46,8 @@
 #define FSL_IMX6_NUM_ESDHCS 4
 #define FSL_IMX6_NUM_ECSPIS 5
 #define FSL_IMX6_NUM_WDTS 2
+#define FSL_IMX6_NUM_USB_PHYS 2
+#define FSL_IMX6_NUM_USBS 4
 
 typedef struct FslIMX6State {
     /*< private >*/
@@ -62,6 +66,8 @@ typedef struct FslIMX6State {
     SDHCIState     esdhc[FSL_IMX6_NUM_ESDHCS];
     IMXSPIState    spi[FSL_IMX6_NUM_ECSPIS];
     IMX2WdtState   wdt[FSL_IMX6_NUM_WDTS];
+    IMXUSBPHYState usbphy[FSL_IMX6_NUM_USB_PHYS];
+    ChipideaState  usb[FSL_IMX6_NUM_USBS];
     IMXFECState    eth;
     MemoryRegion   rom;
     MemoryRegion   caam;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations
  2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
                   ` (4 preceding siblings ...)
  2020-03-13  1:45 ` [PATCH v3 5/5] hw/arm/fsl-imx6: " Guenter Roeck
@ 2020-03-13 11:25 ` Peter Maydell
  5 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2020-03-13 11:25 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: QEMU Developers, qemu-arm, Gerd Hoffmann, Jean-Christophe Dubois

On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
>
> This patch series wires up the USB controllers on fsl-imx6 and fsl-imx6ul
> emulations.
>
> The first patch provides a basic implementation of the USB PHY controller
> used in i.MX28 and later chips. Only reset bit handling in the control
> register is actually implemented. Basic USB PHY support is needed to make
> the USB ports operational in Linux.
>
> The second patch fixes USB and USB PHY interrupt numbers for i.MX6UL.
>
> The third patch instantiates unimplemented pwm and can devices. This patch
> is necessary to avoid crashes in Linux when it tries to access those
> devices. The crashes are observed when trying to boot Linux v4.21 or later.
>
> The final two patches instantiate the USB controllers for i.mMX6 and
> i.MX6UL.
>
> v3:
> - Minor cleanup in patch 1/5 (see details in patch)
> - Added patch to fix USB and USB PHY interrupt numbers for fsl-imx6ul.
> - Added patch to instantiate unimplemented pwm and CAN devices.
> - Instantiate USB and USB PHY separately. They are logically different,
>   and the number of instances is not always the same.
>
> v2:
> - Implement and instantiate basic USB PHY implementation
>   instead of emulating a single USB PHY register



Applied to target-arm.next, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
@ 2021-05-19 19:00   ` Philippe Mathieu-Daudé
  2023-03-16 13:41   ` Peter Maydell
  1 sibling, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-19 19:00 UTC (permalink / raw)
  To: Guenter Roeck, Peter Maydell
  Cc: Jean-Christophe Dubois, qemu-arm, qemu-devel, Gerd Hoffmann

On 3/13/20 2:45 AM, Guenter Roeck wrote:
> Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
> and i.MX7 SoCs.
> 
> The only support really needed - at least to boot Linux - is support
> for soft reset, which needs to reset various registers to their initial
> value. Otherwise, just record register values.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v3: Added Reviewed-by:;
>     dropped duplicate "the" in comments;
>     added new files to MAINTAINERS
> v2: New patch, replacing dummy STMP register support with basic USB PHY
>     emulation.
> 
>  MAINTAINERS                  |   2 +
>  hw/arm/Kconfig               |   1 +
>  hw/usb/Kconfig               |   5 +
>  hw/usb/Makefile.objs         |   2 +
>  hw/usb/imx-usb-phy.c         | 225 +++++++++++++++++++++++++++++++++++
>  include/hw/usb/imx-usb-phy.h |  53 +++++++++
>  6 files changed, 288 insertions(+)
>  create mode 100644 hw/usb/imx-usb-phy.c
>  create mode 100644 include/hw/usb/imx-usb-phy.h

> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index bc54fd61f9..21c627c3b7 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -361,6 +361,7 @@ config FSL_IMX6
>      select IMX
>      select IMX_FEC
>      select IMX_I2C
> +    select IMX_USBPHY
>      select SDHCI

I know it is merged, but FYI this change belongs to patch 5
of this series "hw/arm/fsl-imx6: Wire up USB controllers"
where you add the dependency to the machine.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
  2021-05-19 19:00   ` Philippe Mathieu-Daudé
@ 2023-03-16 13:41   ` Peter Maydell
  2023-03-16 14:12     ` Guenter Roeck
  1 sibling, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2023-03-16 13:41 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean-Christophe Dubois, Gerd Hoffmann, qemu-devel, qemu-arm

On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
>
> Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
> and i.MX7 SoCs.
>
> The only support really needed - at least to boot Linux - is support
> for soft reset, which needs to reset various registers to their initial
> value. Otherwise, just record register values.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Hi Guenter; we've had a fuzzer report that this device model
accesses off the end of the usbphy[] array:
https://gitlab.com/qemu-project/qemu/-/issues/1408

> +static uint64_t imx_usbphy_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    IMXUSBPHYState *s = (IMXUSBPHYState *)opaque;
> +    uint32_t index = offset >> 2;
> +    uint32_t value;


> +    default:
> +        value = s->usbphy[index];

No bounds check in the default case (or ditto in the write function)...

> +        break;
> +    }
> +    return (uint64_t)value;
> +}

> +static void imx_usbphy_realize(DeviceState *dev, Error **errp)
> +{
> +    IMXUSBPHYState *s = IMX_USBPHY(dev);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &imx_usbphy_ops, s,
> +                          "imx-usbphy", 0x1000);

...and the memory region is created at size 0x1000 so the read/write
fns can be called with offsets up to that size...

> +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
> +}

> +enum IMXUsbPhyRegisters {
> +    USBPHY_PWD,
> +    USBPHY_PWD_SET,
> +    USBPHY_PWD_CLR,
> +    USBPHY_PWD_TOG,
> +    USBPHY_TX,
> +    USBPHY_TX_SET,
> +    USBPHY_TX_CLR,
> +    USBPHY_TX_TOG,
> +    USBPHY_RX,
> +    USBPHY_RX_SET,
> +    USBPHY_RX_CLR,
> +    USBPHY_RX_TOG,
> +    USBPHY_CTRL,
> +    USBPHY_CTRL_SET,
> +    USBPHY_CTRL_CLR,
> +    USBPHY_CTRL_TOG,
> +    USBPHY_STATUS,
> +    USBPHY_DEBUG = 0x14,
> +    USBPHY_DEBUG_SET,
> +    USBPHY_DEBUG_CLR,
> +    USBPHY_DEBUG_TOG,
> +    USBPHY_DEBUG0_STATUS,
> +    USBPHY_DEBUG1 = 0x1c,
> +    USBPHY_DEBUG1_SET,
> +    USBPHY_DEBUG1_CLR,
> +    USBPHY_DEBUG1_TOG,
> +    USBPHY_VERSION,
> +    USBPHY_MAX
> +};
> +
> +#define USBPHY_CTRL_SFTRST BIT(31)
> +
> +#define TYPE_IMX_USBPHY "imx.usbphy"
> +#define IMX_USBPHY(obj) OBJECT_CHECK(IMXUSBPHYState, (obj), TYPE_IMX_USBPHY)
> +
> +typedef struct IMXUSBPHYState {
> +    /* <private> */
> +    SysBusDevice parent_obj;
> +
> +    /* <public> */
> +    MemoryRegion iomem;
> +
> +    uint32_t usbphy[USBPHY_MAX];

...but the array is only created with USBPHY_MAX entries.

Do you know what the device is supposed to do with these
off-the-end acceses? We could either reduce the memory region
size or bounds check and RAZ/WI the out-of-range accesses.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2023-03-16 13:41   ` Peter Maydell
@ 2023-03-16 14:12     ` Guenter Roeck
  2023-03-16 14:51       ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2023-03-16 14:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Jean-Christophe Dubois, Gerd Hoffmann, qemu-devel, qemu-arm

Hi Peter,

On 3/16/23 06:41, Peter Maydell wrote:
> On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
>> and i.MX7 SoCs.
>>
>> The only support really needed - at least to boot Linux - is support
>> for soft reset, which needs to reset various registers to their initial
>> value. Otherwise, just record register values.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> 
> Hi Guenter; we've had a fuzzer report that this device model
> accesses off the end of the usbphy[] array:
> https://gitlab.com/qemu-project/qemu/-/issues/1408
> 

Good catch. And an obvious bug, sorry.

>> +static uint64_t imx_usbphy_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +    IMXUSBPHYState *s = (IMXUSBPHYState *)opaque;
>> +    uint32_t index = offset >> 2;
>> +    uint32_t value;
> 
> 
>> +    default:
>> +        value = s->usbphy[index];
> 
> No bounds check in the default case (or ditto in the write function)...
> 
>> +        break;
>> +    }
>> +    return (uint64_t)value;
>> +}
> 
>> +static void imx_usbphy_realize(DeviceState *dev, Error **errp)
>> +{
>> +    IMXUSBPHYState *s = IMX_USBPHY(dev);
>> +
>> +    memory_region_init_io(&s->iomem, OBJECT(s), &imx_usbphy_ops, s,
>> +                          "imx-usbphy", 0x1000);
> 
> ...and the memory region is created at size 0x1000 so the read/write
> fns can be called with offsets up to that size...
> 
>> +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
>> +}
> 
>> +enum IMXUsbPhyRegisters {
>> +    USBPHY_PWD,
>> +    USBPHY_PWD_SET,
>> +    USBPHY_PWD_CLR,
>> +    USBPHY_PWD_TOG,
>> +    USBPHY_TX,
>> +    USBPHY_TX_SET,
>> +    USBPHY_TX_CLR,
>> +    USBPHY_TX_TOG,
>> +    USBPHY_RX,
>> +    USBPHY_RX_SET,
>> +    USBPHY_RX_CLR,
>> +    USBPHY_RX_TOG,
>> +    USBPHY_CTRL,
>> +    USBPHY_CTRL_SET,
>> +    USBPHY_CTRL_CLR,
>> +    USBPHY_CTRL_TOG,
>> +    USBPHY_STATUS,
>> +    USBPHY_DEBUG = 0x14,
>> +    USBPHY_DEBUG_SET,
>> +    USBPHY_DEBUG_CLR,
>> +    USBPHY_DEBUG_TOG,
>> +    USBPHY_DEBUG0_STATUS,
>> +    USBPHY_DEBUG1 = 0x1c,
>> +    USBPHY_DEBUG1_SET,
>> +    USBPHY_DEBUG1_CLR,
>> +    USBPHY_DEBUG1_TOG,
>> +    USBPHY_VERSION,
>> +    USBPHY_MAX
>> +};
>> +
>> +#define USBPHY_CTRL_SFTRST BIT(31)
>> +
>> +#define TYPE_IMX_USBPHY "imx.usbphy"
>> +#define IMX_USBPHY(obj) OBJECT_CHECK(IMXUSBPHYState, (obj), TYPE_IMX_USBPHY)
>> +
>> +typedef struct IMXUSBPHYState {
>> +    /* <private> */
>> +    SysBusDevice parent_obj;
>> +
>> +    /* <public> */
>> +    MemoryRegion iomem;
>> +
>> +    uint32_t usbphy[USBPHY_MAX];
> 
> ...but the array is only created with USBPHY_MAX entries.
> 
> Do you know what the device is supposed to do with these
> off-the-end acceses? We could either reduce the memory region
> size or bounds check and RAZ/WI the out-of-range accesses.
> 

I have no idea what the real hardware would do. The datasheets (at
least the ones I checked) don't say, only that the region size is 4k.
I would suggest a bounds check, ignore out-of-bounds writes (maybe
with a log message), and return 0 for reads (which I think is what
you suggest with RAZ/WI).

Want me to send a patch ?

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2023-03-16 14:12     ` Guenter Roeck
@ 2023-03-16 14:51       ` Peter Maydell
  2023-03-16 17:07         ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2023-03-16 14:51 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean-Christophe Dubois, Gerd Hoffmann, qemu-devel, qemu-arm

On Thu, 16 Mar 2023 at 14:12, Guenter Roeck <linux@roeck-us.net> wrote:
>
> Hi Peter,
>
> On 3/16/23 06:41, Peter Maydell wrote:
> > On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
> >>
> >> Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
> >> and i.MX7 SoCs.
> >>
> >> The only support really needed - at least to boot Linux - is support
> >> for soft reset, which needs to reset various registers to their initial
> >> value. Otherwise, just record register values.
> >>
> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >
> > Hi Guenter; we've had a fuzzer report that this device model
> > accesses off the end of the usbphy[] array:
> > https://gitlab.com/qemu-project/qemu/-/issues/1408
> >
>
> Good catch. And an obvious bug, sorry.


>
> > Do you know what the device is supposed to do with these
> > off-the-end acceses? We could either reduce the memory region
> > size or bounds check and RAZ/WI the out-of-range accesses.
> >
>
> I have no idea what the real hardware would do. The datasheets (at
> least the ones I checked) don't say, only that the region size is 4k.
> I would suggest a bounds check, ignore out-of-bounds writes (maybe
> with a log message), and return 0 for reads (which I think is what
> you suggest with RAZ/WI).
>
> Want me to send a patch ?

If you have the time, that would be great. I expect you're
better set up to test it than I am...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
  2023-03-16 14:51       ` Peter Maydell
@ 2023-03-16 17:07         ` Guenter Roeck
  0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2023-03-16 17:07 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Jean-Christophe Dubois, Gerd Hoffmann, qemu-devel, qemu-arm

On Thu, Mar 16, 2023 at 02:51:23PM +0000, Peter Maydell wrote:
> On Thu, 16 Mar 2023 at 14:12, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > Hi Peter,
> >
> > On 3/16/23 06:41, Peter Maydell wrote:
> > > On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@roeck-us.net> wrote:
> > >>
> > >> Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6,
> > >> and i.MX7 SoCs.
> > >>
> > >> The only support really needed - at least to boot Linux - is support
> > >> for soft reset, which needs to reset various registers to their initial
> > >> value. Otherwise, just record register values.
> > >>
> > >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > >> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > >
> > > Hi Guenter; we've had a fuzzer report that this device model
> > > accesses off the end of the usbphy[] array:
> > > https://gitlab.com/qemu-project/qemu/-/issues/1408
> > >
> >
> > Good catch. And an obvious bug, sorry.
> 
> 
> >
> > > Do you know what the device is supposed to do with these
> > > off-the-end acceses? We could either reduce the memory region
> > > size or bounds check and RAZ/WI the out-of-range accesses.
> > >
> >
> > I have no idea what the real hardware would do. The datasheets (at
> > least the ones I checked) don't say, only that the region size is 4k.
> > I would suggest a bounds check, ignore out-of-bounds writes (maybe
> > with a log message), and return 0 for reads (which I think is what
> > you suggest with RAZ/WI).
> >
> > Want me to send a patch ?
> 
> If you have the time, that would be great. I expect you're
> better set up to test it than I am...
> 

I prepared a patch. Currently testing.

Guenter


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-16 17:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13  1:45 [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Guenter Roeck
2020-03-13  1:45 ` [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support Guenter Roeck
2021-05-19 19:00   ` Philippe Mathieu-Daudé
2023-03-16 13:41   ` Peter Maydell
2023-03-16 14:12     ` Guenter Roeck
2023-03-16 14:51       ` Peter Maydell
2023-03-16 17:07         ` Guenter Roeck
2020-03-13  1:45 ` [PATCH v3 2/5] hw/arm/fsl-imx6ul: Fix USB interrupt numbers Guenter Roeck
2020-03-13  1:45 ` [PATCH v3 3/5] hw/arm/fsl-imx6ul: Instantiate unimplemented pwm and can devices Guenter Roeck
2020-03-13  1:45 ` [PATCH v3 4/5] hw/arm/fsl-imx6ul: Wire up USB controllers Guenter Roeck
2020-03-13  1:45 ` [PATCH v3 5/5] hw/arm/fsl-imx6: " Guenter Roeck
2020-03-13 11:25 ` [PATCH v3 0/5] Wire up USB controllers in i.MX6 emulations Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).