[PATCH 0/2] Add property to support writing ERSTBA in high-low order

[PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

This series is needed to support the USB interface on imx8mp-evk when
booting the Linux kernel.

According to the XHCI specification, ERSTBA should be written in Low-High
order. The Linux kernel writes the high word first. This results in an
initialization failure.

The following information is found in the Linux kernel commit log.

[Synopsys]- The host controller was design to support ERST setting
during the RUN state. But since there is a limitation in controller
in supporting separate ERSTBA_HI and ERSTBA_LO programming,
It is supported when the ERSTBA is programmed in 64bit,
or in 32 bit mode ERSTBA_HI before ERSTBA_LO

[Synopsys]- The internal initialization of event ring fetches
the "Event Ring Segment Table Entry" based on the indication of
ERSTBA_LO written.

Add property to support writing the high word first. Enable it
for dwc3.

----------------------------------------------------------------
Guenter Roeck (2):
      hw: usb: xhci: Add property to support writing ERSTBA in high-low order
      hw/usb/hcd-dwc3: Set erstba-hi-lo property

 hw/usb/hcd-dwc3.c | 1 +
 hw/usb/hcd-xhci.c | 8 +++++++-
 hw/usb/hcd-xhci.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

[PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

According to the XHCI specification, ERSTBA should be written in Low-High
order. The Linux kernel writes the high word first. This results in an
initialization failure.

The following information is found in the Linux kernel commit log.

[Synopsys]- The host controller was design to support ERST setting
during the RUN state. But since there is a limitation in controller
in supporting separate ERSTBA_HI and ERSTBA_LO programming,
It is supported when the ERSTBA is programmed in 64bit,
or in 32 bit mode ERSTBA_HI before ERSTBA_LO

[Synopsys]- The internal initialization of event ring fetches
the "Event Ring Segment Table Entry" based on the indication of
ERSTBA_LO written.

Add property to support writing the high word first.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/usb/hcd-xhci.c | 8 +++++++-
 hw/usb/hcd-xhci.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 64c3a23b9b..8c0ba569c8 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
         } else {
             intr->erstba_low = val & 0xffffffc0;
         }
+        if (xhci->erstba_hi_lo) {
+            xhci_er_reset(xhci, v);
+        }
         break;
     case 0x14: /* ERSTBA high */
         intr->erstba_high = val;
-        xhci_er_reset(xhci, v);
+        if (!xhci->erstba_hi_lo) {
+            xhci_er_reset(xhci, v);
+        }
         break;
     case 0x18: /* ERDP low */
         if (val & ERDP_EHB) {
@@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
     DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
     DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
                      DeviceState *),
+    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
 };
 
 static void xhci_class_init(ObjectClass *klass, void *data)
diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 9c3974f148..cf3f074261 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -189,6 +189,7 @@ typedef struct XHCIState {
     uint32_t numports_3;
     uint32_t numintrs;
     uint32_t numslots;
+    bool erstba_hi_lo;
     uint32_t flags;
     uint32_t max_pstreams_mask;
     void (*intr_update)(XHCIState *s, int n, bool enable);
-- 
2.45.2

[PATCH 2/2] hw/usb/hcd-dwc3: Set erstba-hi-lo property

[Guenter Roeck]

The dwc3 hardware requires the ERSTBA address to be written in
high-low order.

From information found in the Linux kernel:

[Synopsys]- The host controller was design to support ERST setting
during the RUN state. But since there is a limitation in controller
in supporting separate ERSTBA_HI and ERSTBA_LO programming,
It is supported when the ERSTBA is programmed in 64bit,
or in 32 bit mode ERSTBA_HI before ERSTBA_LO

[Synopsys]- The internal initialization of event ring fetches
the "Event Ring Segment Table Entry" based on the indication of
ERSTBA_LO written.

Inform the XHCI core to expect ERSTBA to be written in high-low order.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 hw/usb/hcd-dwc3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/hcd-dwc3.c b/hw/usb/hcd-dwc3.c
index 0bceee2712..6783d55526 100644
--- a/hw/usb/hcd-dwc3.c
+++ b/hw/usb/hcd-dwc3.c
@@ -603,6 +603,7 @@ static void usb_dwc3_realize(DeviceState *dev, Error **errp)
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     Error *err = NULL;
 
+    qdev_prop_set_bit(DEVICE(&s->sysbus_xhci), "erstba-hi-lo", true);
     sysbus_realize(SYS_BUS_DEVICE(&s->sysbus_xhci), &err);
     if (err) {
         error_propagate(errp, err);
-- 
2.45.2

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Philippe Mathieu-Daudé]

Hi Guenter,

On 5/4/25 16:00, Guenter Roeck wrote:
> This series is needed to support the USB interface on imx8mp-evk when
> booting the Linux kernel.
> 
> According to the XHCI specification, ERSTBA should be written in Low-High
> order. The Linux kernel writes the high word first. This results in an
> initialization failure.
> 
> The following information is found in the Linux kernel commit log.
> 
> [Synopsys]- The host controller was design to support ERST setting
> during the RUN state. But since there is a limitation in controller
> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
> It is supported when the ERSTBA is programmed in 64bit,
> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
> 
> [Synopsys]- The internal initialization of event ring fetches
> the "Event Ring Segment Table Entry" based on the indication of
> ERSTBA_LO written.
> 
> Add property to support writing the high word first. Enable it
> for dwc3.
> 
> ----------------------------------------------------------------
> Guenter Roeck (2):
>        hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>        hw/usb/hcd-dwc3: Set erstba-hi-lo property

What about using .impl.min_access_size = 8 instead?

Could you try this patch, or provide me with a reproducer?

-- >8 --
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 64c3a23b9b7..bce61a287bf 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3043,7 +3043,4 @@ static uint64_t xhci_runtime_read(void *ptr, 
hwaddr reg,
          switch (reg & 0x1f) {
-        case 0x00: /* IMAN */
-            ret = intr->iman;
-            break;
-        case 0x04: /* IMOD */
-            ret = intr->imod;
+        case 0x00: /* IMAN & IMOD */
+            ret = deposit64(intr->iman, 32, 32, intr->imod);
              break;
@@ -3052,13 +3049,7 @@ static uint64_t xhci_runtime_read(void *ptr, 
hwaddr reg,
              break;
-        case 0x10: /* ERSTBA low */
-            ret = intr->erstba_low;
+        case 0x10: /* ERSTBA */
+            ret = deposit64(intr->erstba_low, 32, 32, intr->erstba_high);
              break;
-        case 0x14: /* ERSTBA high */
-            ret = intr->erstba_high;
-            break;
-        case 0x18: /* ERDP low */
-            ret = intr->erdp_low;
-            break;
-        case 0x1c: /* ERDP high */
-            ret = intr->erdp_high;
+        case 0x18: /* ERDP */
+            ret = deposit64(intr->erdp_low, 32, 32, intr->erdp_high);
              break;
@@ -3088,3 +3079,3 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
      switch (reg & 0x1f) {
-    case 0x00: /* IMAN */
+    case 0x00: /* IMAN & IMOD */
          if (val & IMAN_IP) {
@@ -3094,23 +3085,19 @@ static void xhci_runtime_write(void *ptr, hwaddr 
reg,
          intr->iman |= val & IMAN_IE;
+        intr->imod = extract64(val, 32, 32);
          xhci_intr_update(xhci, v);
          break;
-    case 0x04: /* IMOD */
-        intr->imod = val;
-        break;
      case 0x08: /* ERSTSZ */
-        intr->erstsz = val & 0xffff;
+        intr->erstsz = extract64(val, 0, 16);
          break;
-    case 0x10: /* ERSTBA low */
+    case 0x10: /* ERSTBA */
          if (xhci->nec_quirks) {
              /* NEC driver bug: it doesn't align this to 64 bytes */
-            intr->erstba_low = val & 0xfffffff0;
+            intr->erstba_low = extract64(val, 4, 28);
          } else {
-            intr->erstba_low = val & 0xffffffc0;
+            intr->erstba_low = extract64(val, 6, 26);
          }
-        break;
-    case 0x14: /* ERSTBA high */
-        intr->erstba_high = val;
+        intr->erstba_high = extract64(val, 32, 32);
          xhci_er_reset(xhci, v);
          break;
-    case 0x18: /* ERDP low */
+    case 0x18: /* ERDP */
          if (val & ERDP_EHB) {
@@ -3128,5 +3115,3 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
          }
-        break;
-    case 0x1c: /* ERDP high */
-        intr->erdp_high = val;
+        intr->erdp_high = extract64(val, 32, 32);
          break;
@@ -3216,2 +3201,3 @@ static const MemoryRegionOps xhci_runtime_ops = {
      .write = xhci_runtime_write,
+    .impl.min_access_size = 8,
      .valid.min_access_size = 4,
---

Regards,

Phil.

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
> Hi Guenter,
> 
> On 5/4/25 16:00, Guenter Roeck wrote:
>> This series is needed to support the USB interface on imx8mp-evk when
>> booting the Linux kernel.
>>
>> According to the XHCI specification, ERSTBA should be written in Low-High
>> order. The Linux kernel writes the high word first. This results in an
>> initialization failure.
>>
>> The following information is found in the Linux kernel commit log.
>>
>> [Synopsys]- The host controller was design to support ERST setting
>> during the RUN state. But since there is a limitation in controller
>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>> It is supported when the ERSTBA is programmed in 64bit,
>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>
>> [Synopsys]- The internal initialization of event ring fetches
>> the "Event Ring Segment Table Entry" based on the indication of
>> ERSTBA_LO written.
>>
>> Add property to support writing the high word first. Enable it
>> for dwc3.
>>
>> ----------------------------------------------------------------
>> Guenter Roeck (2):
>>        hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>        hw/usb/hcd-dwc3: Set erstba-hi-lo property
> 
> What about using .impl.min_access_size = 8 instead?
> 

That would seem to be a bit excessive. The problem only affects the ERSTBA
register. I don't see an indication that other registers are affected,
or that all accesses would have to be 64 bit wide. The information above
also suggests that the controller can operate in both 32-bit and 64-bit
mode.

> Could you try this patch, or provide me with a reproducer?
> 

That is a bit too invasive for my liking, and it would affect all users,
not just dwc3. I'll leave such changes to you. I'll cook up a minimal
reproducer and point you to it.

Thanks,
Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
> Hi Guenter,
> 
> On 5/4/25 16:00, Guenter Roeck wrote:
>> This series is needed to support the USB interface on imx8mp-evk when
>> booting the Linux kernel.
>>
>> According to the XHCI specification, ERSTBA should be written in Low-High
>> order. The Linux kernel writes the high word first. This results in an
>> initialization failure.
>>
>> The following information is found in the Linux kernel commit log.
>>
>> [Synopsys]- The host controller was design to support ERST setting
>> during the RUN state. But since there is a limitation in controller
>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>> It is supported when the ERSTBA is programmed in 64bit,
>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>
>> [Synopsys]- The internal initialization of event ring fetches
>> the "Event Ring Segment Table Entry" based on the indication of
>> ERSTBA_LO written.
>>
>> Add property to support writing the high word first. Enable it
>> for dwc3.
>>
>> ----------------------------------------------------------------
>> Guenter Roeck (2):
>>        hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>        hw/usb/hcd-dwc3: Set erstba-hi-lo property
> 
> What about using .impl.min_access_size = 8 instead?
> 
> Could you try this patch, or provide me with a reproducer?
> 

You should find everything you need to reproduce the problem at

http://server.roeck-us.net/qemu/xhci/

Please let me know if you need anything else.

Thanks,
Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>> Hi Guenter,
>> 
>> On 5/4/25 16:00, Guenter Roeck wrote:
>>> This series is needed to support the USB interface on imx8mp-evk when
>>> booting the Linux kernel.
>>> 
>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>> order. The Linux kernel writes the high word first. This results in an
>>> initialization failure.
>>> 
>>> The following information is found in the Linux kernel commit log.
>>> 
>>> [Synopsys]- The host controller was design to support ERST setting
>>> during the RUN state. But since there is a limitation in controller
>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>> It is supported when the ERSTBA is programmed in 64bit,
>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>> 
>>> [Synopsys]- The internal initialization of event ring fetches
>>> the "Event Ring Segment Table Entry" based on the indication of
>>> ERSTBA_LO written.
>>> 
>>> Add property to support writing the high word first. Enable it
>>> for dwc3.
>>> 
>>> ----------------------------------------------------------------
>>> Guenter Roeck (2):
>>>        hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>        hw/usb/hcd-dwc3: Set erstba-hi-lo property
>> 
>> What about using .impl.min_access_size = 8 instead?
>> 
>> Could you try this patch, or provide me with a reproducer?
>> 
>
>You should find everything you need to reproduce the problem at

Hi Guenter,

thanks for testing the new board, much appreciated.

>
>http://server.roeck-us.net/qemu/xhci/

I like your approach of pruning the device tree automatically: <http://server.roeck-us.net/qemu/xhci/patches/0002-imx8mp-evk-Remove-unimplemented-properties-and-nodes.patch> Would you mind if I upstream it with a few modifications like in <https://github.com/shentok/qemu/commits/upstream/imx8mp-test/> ? If yes, please let me know if I attribute your work there correctly.

Best regards,
Bernhard

>
>Please let me know if you need anything else.
>
>Thanks,
>Guenter
>

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

Hi Bernhard,

On 4/5/25 12:28, Bernhard Beschow wrote:
> 
> 
> Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>>> Hi Guenter,
>>>
>>> On 5/4/25 16:00, Guenter Roeck wrote:
>>>> This series is needed to support the USB interface on imx8mp-evk when
>>>> booting the Linux kernel.
>>>>
>>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>>> order. The Linux kernel writes the high word first. This results in an
>>>> initialization failure.
>>>>
>>>> The following information is found in the Linux kernel commit log.
>>>>
>>>> [Synopsys]- The host controller was design to support ERST setting
>>>> during the RUN state. But since there is a limitation in controller
>>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>>> It is supported when the ERSTBA is programmed in 64bit,
>>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>>>
>>>> [Synopsys]- The internal initialization of event ring fetches
>>>> the "Event Ring Segment Table Entry" based on the indication of
>>>> ERSTBA_LO written.
>>>>
>>>> Add property to support writing the high word first. Enable it
>>>> for dwc3.
>>>>
>>>> ----------------------------------------------------------------
>>>> Guenter Roeck (2):
>>>>         hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>>         hw/usb/hcd-dwc3: Set erstba-hi-lo property
>>>
>>> What about using .impl.min_access_size = 8 instead?
>>>
>>> Could you try this patch, or provide me with a reproducer?
>>>
>>
>> You should find everything you need to reproduce the problem at
> 
> Hi Guenter,
> 
> thanks for testing the new board, much appreciated.
> 
>>
>> http://server.roeck-us.net/qemu/xhci/
> 
> I like your approach of pruning the device tree automatically: <http://server.roeck-us.net/qemu/xhci/patches/0002-imx8mp-evk-Remove-unimplemented-properties-and-nodes.patch> Would you mind if I upstream it with a few modifications like in <https://github.com/shentok/qemu/commits/upstream/imx8mp-test/> ? If yes, please let me know if I attribute your work there correctly.
> 

Please feel free to go ahead. Attribution is fine.

Note that I had submitted a similar patch a couple of years ago; that
was rejected with the argument that I should not use the dts file
from the Linux kernel in the first place. That is why I did not bother
to submit the patch myself.

Anyway, just for reference: the inspiration for this change is from
commit bf1da4b308 ("hw/arm/raspi4b: Temporarily disable unimplemented
rpi4b devices").

Thanks,
Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/5/25 12:28, Bernhard Beschow wrote:
> 
> 
> Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>>> Hi Guenter,
>>>
>>> On 5/4/25 16:00, Guenter Roeck wrote:
>>>> This series is needed to support the USB interface on imx8mp-evk when
>>>> booting the Linux kernel.
>>>>
>>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>>> order. The Linux kernel writes the high word first. This results in an
>>>> initialization failure.
>>>>
>>>> The following information is found in the Linux kernel commit log.
>>>>
>>>> [Synopsys]- The host controller was design to support ERST setting
>>>> during the RUN state. But since there is a limitation in controller
>>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>>> It is supported when the ERSTBA is programmed in 64bit,
>>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>>>
>>>> [Synopsys]- The internal initialization of event ring fetches
>>>> the "Event Ring Segment Table Entry" based on the indication of
>>>> ERSTBA_LO written.
>>>>
>>>> Add property to support writing the high word first. Enable it
>>>> for dwc3.
>>>>
>>>> ----------------------------------------------------------------
>>>> Guenter Roeck (2):
>>>>         hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>>         hw/usb/hcd-dwc3: Set erstba-hi-lo property
>>>
>>> What about using .impl.min_access_size = 8 instead?
>>>
>>> Could you try this patch, or provide me with a reproducer?
>>>
>>
>> You should find everything you need to reproduce the problem at
> 
> Hi Guenter,
> 
> thanks for testing the new board, much appreciated.
> 

In this context: Did you get the PCIe interface to work ?
It instantiates for me, but interrupts don't get through.
This is with the latest Linux kernel.

Thanks,
Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 6. April 2025 01:31:49 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/5/25 12:28, Bernhard Beschow wrote:
>> 
>> 
>> Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>>> On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>>>> Hi Guenter,
>>>> 
>>>> On 5/4/25 16:00, Guenter Roeck wrote:
>>>>> This series is needed to support the USB interface on imx8mp-evk when
>>>>> booting the Linux kernel.
>>>>> 
>>>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>>>> order. The Linux kernel writes the high word first. This results in an
>>>>> initialization failure.
>>>>> 
>>>>> The following information is found in the Linux kernel commit log.
>>>>> 
>>>>> [Synopsys]- The host controller was design to support ERST setting
>>>>> during the RUN state. But since there is a limitation in controller
>>>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>>>> It is supported when the ERSTBA is programmed in 64bit,
>>>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>>>> 
>>>>> [Synopsys]- The internal initialization of event ring fetches
>>>>> the "Event Ring Segment Table Entry" based on the indication of
>>>>> ERSTBA_LO written.
>>>>> 
>>>>> Add property to support writing the high word first. Enable it
>>>>> for dwc3.
>>>>> 
>>>>> ----------------------------------------------------------------
>>>>> Guenter Roeck (2):
>>>>>         hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>>>         hw/usb/hcd-dwc3: Set erstba-hi-lo property
>>>> 
>>>> What about using .impl.min_access_size = 8 instead?
>>>> 
>>>> Could you try this patch, or provide me with a reproducer?
>>>> 
>>> 
>>> You should find everything you need to reproduce the problem at
>> 
>> Hi Guenter,
>> 
>> thanks for testing the new board, much appreciated.
>> 
>
>In this context: Did you get the PCIe interface to work ?
>It instantiates for me, but interrupts don't get through.
>This is with the latest Linux kernel.

Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.

However, even with your USB patches applied, PCIe and USB are non-functional on the latest aarch64 Arch Linux kernel (v6.14 [1]):

  [   21.102444] platform 32f10108.usb: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready
  [   21.102914] platform 32f00000.pcie-phy: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready

I suspect that some bits need to indicate stable PLLs or similar, but that needs further investigation.

Best regards,
Bernhard

[1] <https://archlinuxarm.org/packages/aarch64/linux-aarch64>

>
>Thanks,
>Guenter
>

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/6/25 04:03, Bernhard Beschow wrote:
> 
> 
> Am 6. April 2025 01:31:49 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> On 4/5/25 12:28, Bernhard Beschow wrote:
>>>
>>>
>>> Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>>>> On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>>>>> Hi Guenter,
>>>>>
>>>>> On 5/4/25 16:00, Guenter Roeck wrote:
>>>>>> This series is needed to support the USB interface on imx8mp-evk when
>>>>>> booting the Linux kernel.
>>>>>>
>>>>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>>>>> order. The Linux kernel writes the high word first. This results in an
>>>>>> initialization failure.
>>>>>>
>>>>>> The following information is found in the Linux kernel commit log.
>>>>>>
>>>>>> [Synopsys]- The host controller was design to support ERST setting
>>>>>> during the RUN state. But since there is a limitation in controller
>>>>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>>>>> It is supported when the ERSTBA is programmed in 64bit,
>>>>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>>>>>
>>>>>> [Synopsys]- The internal initialization of event ring fetches
>>>>>> the "Event Ring Segment Table Entry" based on the indication of
>>>>>> ERSTBA_LO written.
>>>>>>
>>>>>> Add property to support writing the high word first. Enable it
>>>>>> for dwc3.
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> Guenter Roeck (2):
>>>>>>          hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>>>>          hw/usb/hcd-dwc3: Set erstba-hi-lo property
>>>>>
>>>>> What about using .impl.min_access_size = 8 instead?
>>>>>
>>>>> Could you try this patch, or provide me with a reproducer?
>>>>>
>>>>
>>>> You should find everything you need to reproduce the problem at
>>>
>>> Hi Guenter,
>>>
>>> thanks for testing the new board, much appreciated.
>>>
>>
>> In this context: Did you get the PCIe interface to work ?
>> It instantiates for me, but interrupts don't get through.
>> This is with the latest Linux kernel.
> 
> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
> 

I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
(such as nvme or scsi adapters) to work.

> However, even with your USB patches applied, PCIe and USB are non-functional on the latest aarch64 Arch Linux kernel (v6.14 [1]):
> 
>    [   21.102444] platform 32f10108.usb: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready
>    [   21.102914] platform 32f00000.pcie-phy: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready
> 
> I suspect that some bits need to indicate stable PLLs or similar, but that needs further investigation.
> 

Hmm, I had that too, but then I found that it works for me with the latest upstream
kernel. Try the patches at http://server.roeck-us.net/qemu/xhci/patches-pca/
to see if it helps. The PMIC (PCA9450) may have to be present.

You might also need to check the configuration. I just tried with 6.14 again, and
it works for me even without above patches. I played around with the configuration
to get to that point, but I don't remember what exactly what I did to make it work
(sorry).

Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 6. April 2025 15:30:42 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/6/25 04:03, Bernhard Beschow wrote:
>> 
>> 
>> Am 6. April 2025 01:31:49 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>>> On 4/5/25 12:28, Bernhard Beschow wrote:
>>>> 
>>>> 
>>>> Am 5. April 2025 17:26:14 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>>>>> On 4/5/25 07:25, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Guenter,
>>>>>> 
>>>>>> On 5/4/25 16:00, Guenter Roeck wrote:
>>>>>>> This series is needed to support the USB interface on imx8mp-evk when
>>>>>>> booting the Linux kernel.
>>>>>>> 
>>>>>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>>>>>> order. The Linux kernel writes the high word first. This results in an
>>>>>>> initialization failure.
>>>>>>> 
>>>>>>> The following information is found in the Linux kernel commit log.
>>>>>>> 
>>>>>>> [Synopsys]- The host controller was design to support ERST setting
>>>>>>> during the RUN state. But since there is a limitation in controller
>>>>>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>>>>>> It is supported when the ERSTBA is programmed in 64bit,
>>>>>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>>>>>> 
>>>>>>> [Synopsys]- The internal initialization of event ring fetches
>>>>>>> the "Event Ring Segment Table Entry" based on the indication of
>>>>>>> ERSTBA_LO written.
>>>>>>> 
>>>>>>> Add property to support writing the high word first. Enable it
>>>>>>> for dwc3.
>>>>>>> 
>>>>>>> ----------------------------------------------------------------
>>>>>>> Guenter Roeck (2):
>>>>>>>          hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>>>>>>>          hw/usb/hcd-dwc3: Set erstba-hi-lo property
>>>>>> 
>>>>>> What about using .impl.min_access_size = 8 instead?
>>>>>> 
>>>>>> Could you try this patch, or provide me with a reproducer?
>>>>>> 
>>>>> 
>>>>> You should find everything you need to reproduce the problem at
>>>> 
>>>> Hi Guenter,
>>>> 
>>>> thanks for testing the new board, much appreciated.
>>>> 
>>> 
>>> In this context: Did you get the PCIe interface to work ?
>>> It instantiates for me, but interrupts don't get through.
>>> This is with the latest Linux kernel.
>> 
>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>> 
>
>I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>(such as nvme or scsi adapters) to work.

I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.

>
>> However, even with your USB patches applied, PCIe and USB are non-functional on the latest aarch64 Arch Linux kernel (v6.14 [1]):
>> 
>>    [   21.102444] platform 32f10108.usb: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready
>>    [   21.102914] platform 32f00000.pcie-phy: deferred probe pending: platform: supplier 32f10000.blk-ctrl not ready
>> 
>> I suspect that some bits need to indicate stable PLLs or similar, but that needs further investigation.
>> 
>
>Hmm, I had that too, but then I found that it works for me with the latest upstream
>kernel.

I have this issue with Arch Linux but not with Buildroot. Strange.

Best regards,
Bernhard

>Try the patches at http://server.roeck-us.net/qemu/xhci/patches-pca/
>to see if it helps. The PMIC (PCA9450) may have to be present.
>
>You might also need to check the configuration. I just tried with 6.14 again, and
>it works for me even without above patches. I played around with the configuration
>to get to that point, but I don't remember what exactly what I did to make it work
>(sorry).
>
>Guenter
>

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/6/25 11:08, Bernhard Beschow wrote:
[ .. ]

>>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>>>
>>
>> I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>> (such as nvme or scsi adapters) to work.
> 
> I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.
> 

Following up on this, my problem is that adding "-netdev user,id=net0 -device virtio-net-pci,netdev=net0"
to the command line adds a _second_ Ethernet interface, in addition to the default one.
This results in
	qemu-system-arm: warning: nic imx.enet.0 has no peer
reported when qemu starts.

I can not get that second interface to work, probably because of some userspace issue.

Anyway, I never see any interrupts on the virtual PCI interface. From /proc/interrupts:

277:          0  PCI-MSI 524288 Edge      virtio0-config
278:          0  PCI-MSI 524289 Edge      virtio0-input.0
279:          0  PCI-MSI 524290 Edge      virtio0-output.0

That may work for virtio-net-pci, but it doesn't work for other PCI(e) drivers.
If I try to attach any other PCIe devices, the device is reported with lspci but
then its initialization times out because it does not get any interrupts.

Tt turns out that sabrelite has the same problem.

Please let me know if you have an idea how to get the interrupts to work.

Thanks,
Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 5. April 2025 14:00:00 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>This series is needed to support the USB interface on imx8mp-evk when
>booting the Linux kernel.
>
>According to the XHCI specification, ERSTBA should be written in Low-High
>order. The Linux kernel writes the high word first. This results in an
>initialization failure.
>
>The following information is found in the Linux kernel commit log.
>
>[Synopsys]- The host controller was design to support ERST setting
>during the RUN state. But since there is a limitation in controller
>in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>It is supported when the ERSTBA is programmed in 64bit,
>or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>
>[Synopsys]- The internal initialization of event ring fetches
>the "Event Ring Segment Table Entry" based on the indication of
>ERSTBA_LO written.
>
>Add property to support writing the high word first. Enable it
>for dwc3.
>
>----------------------------------------------------------------
>Guenter Roeck (2):
>      hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>      hw/usb/hcd-dwc3: Set erstba-hi-lo property
>
> hw/usb/hcd-dwc3.c | 1 +
> hw/usb/hcd-xhci.c | 8 +++++++-
> hw/usb/hcd-xhci.h | 1 +
> 3 files changed, 9 insertions(+), 1 deletion(-)

Series:
Tested-by: Bernhard Beschow <shentey@gmail.com>

... on imx8mp-evk board with 6.14 defconfig kernel. It indeed helps to get USB working with this kernel.

Best regards,
Bernhard

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 8. April 2025 16:09:58 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/6/25 11:08, Bernhard Beschow wrote:
>[ .. ]
>
>>>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>>>> 
>>> 
>>> I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>>> (such as nvme or scsi adapters) to work.
>> 
>> I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.
>> 
>
>Following up on this, my problem is that adding "-netdev user,id=net0 -device virtio-net-pci,netdev=net0"
>to the command line adds a _second_ Ethernet interface, in addition to the default one.
>This results in
>	qemu-system-arm: warning: nic imx.enet.0 has no peer
>reported when qemu starts.

I get this too when using virtio-net-pci successfully.

>
>I can not get that second interface to work, probably because of some userspace issue.
>
>Anyway, I never see any interrupts on the virtual PCI interface. From /proc/interrupts:
>
>277:          0  PCI-MSI 524288 Edge      virtio0-config
>278:          0  PCI-MSI 524289 Edge      virtio0-input.0
>279:          0  PCI-MSI 524290 Edge      virtio0-output.0

I get:

206:          0          0          0          0  PCI-MSI 524288 Edge      virtio0-config
207:          3          0          0          0  PCI-MSI 524289 Edge      virtio0-input.0
208:          8          0          0          0  PCI-MSI 524290 Edge      virtio0-output.0

Note that I'm using four CPUs, i.e. `-smp 4`.

>
>That may work for virtio-net-pci, but it doesn't work for other PCI(e) drivers.
>If I try to attach any other PCIe devices, the device is reported with lspci but
>then its initialization times out because it does not get any interrupts.

Indeed, trying with e1000e:

205:          0          0          0          0  PCI-MSI   0 Edge      PCIe PME
206:         74          0          0          0  PCI-MSI 524288 Edge      eth1-rx-0
207:         20          0          0          0  PCI-MSI 524289 Edge      eth1-tx-0
208:         32          0          0          0  PCI-MSI 524290 Edge      eth1

But I get this repeatedly with varying CPUs:

[   14.657163] e1000e 0000:01:00.0 eth1: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx
[   19.980452] e1000e 0000:01:00.0 eth1: NETDEV WATCHDOG: CPU: 0: transmit queue 0 timed out 5312 ms
[   19.982491] e1000e 0000:01:00.0 eth1: Reset adapter unexpectedly

>
>Tt turns out that sabrelite has the same problem.

Did it work with QEMU 9.2?

Best regards,
Bernhard

>
>Please let me know if you have an idea how to get the interrupts to work.
>
>Thanks,
>Guenter
>

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/8/25 12:57, Bernhard Beschow wrote:
> 
> 
> Am 8. April 2025 16:09:58 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> On 4/6/25 11:08, Bernhard Beschow wrote:
>> [ .. ]
>>
>>>>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>>>>>
>>>>
>>>> I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>>>> (such as nvme or scsi adapters) to work.
>>>
>>> I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.
>>>
>>
>> Following up on this, my problem is that adding "-netdev user,id=net0 -device virtio-net-pci,netdev=net0"
>> to the command line adds a _second_ Ethernet interface, in addition to the default one.
>> This results in
>> 	qemu-system-arm: warning: nic imx.enet.0 has no peer
>> reported when qemu starts.
> 
> I get this too when using virtio-net-pci successfully.
> 
>>
>> I can not get that second interface to work, probably because of some userspace issue.
>>
>> Anyway, I never see any interrupts on the virtual PCI interface. From /proc/interrupts:
>>
>> 277:          0  PCI-MSI 524288 Edge      virtio0-config
>> 278:          0  PCI-MSI 524289 Edge      virtio0-input.0
>> 279:          0  PCI-MSI 524290 Edge      virtio0-output.0
> 
> I get:
> 
> 206:          0          0          0          0  PCI-MSI 524288 Edge      virtio0-config
> 207:          3          0          0          0  PCI-MSI 524289 Edge      virtio0-input.0
> 208:          8          0          0          0  PCI-MSI 524290 Edge      virtio0-output.0
> 
> Note that I'm using four CPUs, i.e. `-smp 4`.
> 

I must be missing something. Can you send me your complete qemu command line ?
I'll also try building a buildroot image to see where it gets me.

>>
>> That may work for virtio-net-pci, but it doesn't work for other PCI(e) drivers.
>> If I try to attach any other PCIe devices, the device is reported with lspci but
>> then its initialization times out because it does not get any interrupts.
> 
> Indeed, trying with e1000e:
> 
> 205:          0          0          0          0  PCI-MSI   0 Edge      PCIe PME
> 206:         74          0          0          0  PCI-MSI 524288 Edge      eth1-rx-0
> 207:         20          0          0          0  PCI-MSI 524289 Edge      eth1-tx-0
> 208:         32          0          0          0  PCI-MSI 524290 Edge      eth1
> 
> But I get this repeatedly with varying CPUs:
> 
> [   14.657163] e1000e 0000:01:00.0 eth1: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx
> [   19.980452] e1000e 0000:01:00.0 eth1: NETDEV WATCHDOG: CPU: 0: transmit queue 0 timed out 5312 ms
> [   19.982491] e1000e 0000:01:00.0 eth1: Reset adapter unexpectedly
> 
>>
>> Tt turns out that sabrelite has the same problem.
> 
> Did it work with QEMU 9.2?
> 

No, the pcie interfaces on sabrelite don't instantiate for me with qemu 9.2 (9.2.3,
more specifically). I see the pcie root port, but nothing behind it.

Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/8/25 12:57, Bernhard Beschow wrote:
> 
> 
> Am 8. April 2025 16:09:58 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> On 4/6/25 11:08, Bernhard Beschow wrote:
>> [ .. ]
>>
>>>>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>>>>>
>>>>
>>>> I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>>>> (such as nvme or scsi adapters) to work.
>>>
>>> I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.
>>>
>>
>> Following up on this, my problem is that adding "-netdev user,id=net0 -device virtio-net-pci,netdev=net0"
>> to the command line adds a _second_ Ethernet interface, in addition to the default one.
>> This results in
>> 	qemu-system-arm: warning: nic imx.enet.0 has no peer
>> reported when qemu starts.
> 
> I get this too when using virtio-net-pci successfully.
> 
>>
>> I can not get that second interface to work, probably because of some userspace issue.
>>
>> Anyway, I never see any interrupts on the virtual PCI interface. From /proc/interrupts:
>>
>> 277:          0  PCI-MSI 524288 Edge      virtio0-config
>> 278:          0  PCI-MSI 524289 Edge      virtio0-input.0
>> 279:          0  PCI-MSI 524290 Edge      virtio0-output.0
> 
> I get:
> 
> 206:          0          0          0          0  PCI-MSI 524288 Edge      virtio0-config
> 207:          3          0          0          0  PCI-MSI 524289 Edge      virtio0-input.0
> 208:          8          0          0          0  PCI-MSI 524290 Edge      virtio0-output.0
> 
> Note that I'm using four CPUs, i.e. `-smp 4`.
> 

I think I found it. Previously I needed to add .psci_conduit to struct arm_boot_info.
No idea why; without it, the kernel would crash almost immediately. Now it seems
that _adding_ that line causes the PCIe hiccup. Oh well :-(. Sorry for the noise.

Guenter

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 8. April 2025 20:31:58 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/8/25 12:57, Bernhard Beschow wrote:
>> 
>> 
>> Am 8. April 2025 16:09:58 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>>> On 4/6/25 11:08, Bernhard Beschow wrote:
>>> [ .. ]
>>> 
>>>>>> Yeah, it works with Buildroot as described in the handbook. When I append `-netdev user,id=net0 -device virtio-net-pci,netdev=net0` on the cli I can `wget http://www.google.com` successfully. When I omit it there is no network connectivity. This is with a 6.6.23 vendor kernel.
>>>>>> 
>>>>> 
>>>>> I had no luck with virtio-net-pci. virtio-pci works for me, but I can not get real PCI devices
>>>>> (such as nvme or scsi adapters) to work.
>>>> 
>>>> I now tested with the latest Buildroot recipe, changing to upstream kernel version 6.14 and using the defconfig. The `wget` command still works for me with virtio-net-pci. However, I can confirm that I need your xhci patches for the usb storage device to be detected.
>>>> 
>>> 
>>> Following up on this, my problem is that adding "-netdev user,id=net0 -device virtio-net-pci,netdev=net0"
>>> to the command line adds a _second_ Ethernet interface, in addition to the default one.
>>> This results in
>>> 	qemu-system-arm: warning: nic imx.enet.0 has no peer
>>> reported when qemu starts.
>> 
>> I get this too when using virtio-net-pci successfully.
>> 
>>> 
>>> I can not get that second interface to work, probably because of some userspace issue.
>>> 
>>> Anyway, I never see any interrupts on the virtual PCI interface. From /proc/interrupts:
>>> 
>>> 277:          0  PCI-MSI 524288 Edge      virtio0-config
>>> 278:          0  PCI-MSI 524289 Edge      virtio0-input.0
>>> 279:          0  PCI-MSI 524290 Edge      virtio0-output.0
>> 
>> I get:
>> 
>> 206:          0          0          0          0  PCI-MSI 524288 Edge      virtio0-config
>> 207:          3          0          0          0  PCI-MSI 524289 Edge      virtio0-input.0
>> 208:          8          0          0          0  PCI-MSI 524290 Edge      virtio0-output.0
>> 
>> Note that I'm using four CPUs, i.e. `-smp 4`.
>> 
>
>I must be missing something. Can you send me your complete qemu command line ?
>I'll also try building a buildroot image to see where it gets me.

Will send you tomorrow.

>
>>> 
>>> That may work for virtio-net-pci, but it doesn't work for other PCI(e) drivers.
>>> If I try to attach any other PCIe devices, the device is reported with lspci but
>>> then its initialization times out because it does not get any interrupts.
>> 
>> Indeed, trying with e1000e:
>> 
>> 205:          0          0          0          0  PCI-MSI   0 Edge      PCIe PME
>> 206:         74          0          0          0  PCI-MSI 524288 Edge      eth1-rx-0
>> 207:         20          0          0          0  PCI-MSI 524289 Edge      eth1-tx-0
>> 208:         32          0          0          0  PCI-MSI 524290 Edge      eth1
>> 
>> But I get this repeatedly with varying CPUs:
>> 
>> [   14.657163] e1000e 0000:01:00.0 eth1: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx
>> [   19.980452] e1000e 0000:01:00.0 eth1: NETDEV WATCHDOG: CPU: 0: transmit queue 0 timed out 5312 ms
>> [   19.982491] e1000e 0000:01:00.0 eth1: Reset adapter unexpectedly
>> 
>>> 
>>> Tt turns out that sabrelite has the same problem.
>> 
>> Did it work with QEMU 9.2?
>> 
>
>No, the pcie interfaces on sabrelite don't instantiate for me with qemu 9.2 (9.2.3,
>more specifically). I see the pcie root port, but nothing behind it.

You need to add `bus=dw-pcie` to the pci devices' options in QEMU 9.2.x and earler, otherwise it will end up on the wrong bus. This is fixed in master.

Best regards,
Bernhard

>
>Guenter
>

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/8/25 14:56, Bernhard Beschow wrote:

>>>> Tt turns out that sabrelite has the same problem.
>>>
>>> Did it work with QEMU 9.2?
>>>
>>
>> No, the pcie interfaces on sabrelite don't instantiate for me with qemu 9.2 (9.2.3,
>> more specifically). I see the pcie root port, but nothing behind it.
> 
> You need to add `bus=dw-pcie` to the pci devices' options in QEMU 9.2.x and earler, otherwise it will end up on the wrong bus. This is fixed in master.
> 

Just to give you a quick update, I needed the diff below to get virtio-net-pci
to work on sabrelite.

No luck with any other non-virtio PCI controllers, though.

Guenter

---
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 9da32fc189..7325f3f2c4 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -456,10 +456,10 @@ struct FslIMX6State {
  #define FSL_IMX6_MLB150_LOW_IRQ 117
  #define FSL_IMX6_ENET_MAC_IRQ 118
  #define FSL_IMX6_ENET_MAC_1588_IRQ 119
-#define FSL_IMX6_PCIE1_IRQ 120
-#define FSL_IMX6_PCIE2_IRQ 121
-#define FSL_IMX6_PCIE3_IRQ 122
-#define FSL_IMX6_PCIE4_MSI_IRQ 123
+#define FSL_IMX6_PCIE1_IRQ 123
+#define FSL_IMX6_PCIE2_IRQ 122
+#define FSL_IMX6_PCIE3_IRQ 121
+#define FSL_IMX6_PCIE4_MSI_IRQ 120
  #define FSL_IMX6_DCIC1_IRQ 124
  #define FSL_IMX6_DCIC2_IRQ 125
  #define FSL_IMX6_MLB150_HIGH_IRQ 126

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 11. April 2025 03:53:27 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/8/25 14:56, Bernhard Beschow wrote:
>
>>>>> Tt turns out that sabrelite has the same problem.
>>>> 
>>>> Did it work with QEMU 9.2?
>>>> 
>>> 
>>> No, the pcie interfaces on sabrelite don't instantiate for me with qemu 9.2 (9.2.3,
>>> more specifically). I see the pcie root port, but nothing behind it.
>> 
>> You need to add `bus=dw-pcie` to the pci devices' options in QEMU 9.2.x and earler, otherwise it will end up on the wrong bus. This is fixed in master.
>> 
>
>Just to give you a quick update, I needed the diff below to get virtio-net-pci
>to work on sabrelite.
>
>No luck with any other non-virtio PCI controllers, though.
>
>Guenter
>
>---
>diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
>index 9da32fc189..7325f3f2c4 100644
>--- a/include/hw/arm/fsl-imx6.h
>+++ b/include/hw/arm/fsl-imx6.h
>@@ -456,10 +456,10 @@ struct FslIMX6State {
> #define FSL_IMX6_MLB150_LOW_IRQ 117
> #define FSL_IMX6_ENET_MAC_IRQ 118
> #define FSL_IMX6_ENET_MAC_1588_IRQ 119
>-#define FSL_IMX6_PCIE1_IRQ 120
>-#define FSL_IMX6_PCIE2_IRQ 121
>-#define FSL_IMX6_PCIE3_IRQ 122
>-#define FSL_IMX6_PCIE4_MSI_IRQ 123
>+#define FSL_IMX6_PCIE1_IRQ 123
>+#define FSL_IMX6_PCIE2_IRQ 122
>+#define FSL_IMX6_PCIE3_IRQ 121
>+#define FSL_IMX6_PCIE4_MSI_IRQ 120
> #define FSL_IMX6_DCIC1_IRQ 124
> #define FSL_IMX6_DCIC2_IRQ 125
> #define FSL_IMX6_MLB150_HIGH_IRQ 126
>

Yeah, that matches the device tree. Mind sending a patch?

Re: [PATCH 0/2] Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 5. April 2025 14:00:00 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>This series is needed to support the USB interface on imx8mp-evk when
>booting the Linux kernel.
>
>According to the XHCI specification, ERSTBA should be written in Low-High
>order. The Linux kernel writes the high word first. This results in an
>initialization failure.
>
>The following information is found in the Linux kernel commit log.
>
>[Synopsys]- The host controller was design to support ERST setting
>during the RUN state. But since there is a limitation in controller
>in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>It is supported when the ERSTBA is programmed in 64bit,
>or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>
>[Synopsys]- The internal initialization of event ring fetches
>the "Event Ring Segment Table Entry" based on the indication of
>ERSTBA_LO written.
>
>Add property to support writing the high word first. Enable it
>for dwc3.
>
>----------------------------------------------------------------
>Guenter Roeck (2):
>      hw: usb: xhci: Add property to support writing ERSTBA in high-low order
>      hw/usb/hcd-dwc3: Set erstba-hi-lo property
>
> hw/usb/hcd-dwc3.c | 1 +
> hw/usb/hcd-xhci.c | 8 +++++++-
> hw/usb/hcd-xhci.h | 1 +
> 3 files changed, 9 insertions(+), 1 deletion(-)

Series:
Reviewed-by: Bernhard Beschow <shentey@gmail.com>

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Nicholas Piggin]

On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
> According to the XHCI specification, ERSTBA should be written in Low-High
> order. The Linux kernel writes the high word first. This results in an
> initialization failure.

This should probably be reworded, it's not so much that Linux does it,
this kind of implies a Linux bug. It is that the hardware requires it
and Linux works around such quirk.

  According to the XHCI specification, ERSTBA should be written in Low-High
  order, however some controllers have a quirk that requires the low
  word to be written last.

>
> The following information is found in the Linux kernel commit log.
>
> [Synopsys]- The host controller was design to support ERST setting
> during the RUN state. But since there is a limitation in controller
> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
> It is supported when the ERSTBA is programmed in 64bit,
> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>
> [Synopsys]- The internal initialization of event ring fetches
> the "Event Ring Segment Table Entry" based on the indication of
> ERSTBA_LO written.

Could you include a reference to the commit in the normal form?

The following information is found in the changelog for Linux kernel
commit sha ("blah").

>
> Add property to support writing the high word first.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  hw/usb/hcd-xhci.c | 8 +++++++-
>  hw/usb/hcd-xhci.h | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index 64c3a23b9b..8c0ba569c8 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
>          } else {
>              intr->erstba_low = val & 0xffffffc0;
>          }
> +        if (xhci->erstba_hi_lo) {
> +            xhci_er_reset(xhci, v);
> +        }
>          break;
>      case 0x14: /* ERSTBA high */
>          intr->erstba_high = val;
> -        xhci_er_reset(xhci, v);
> +        if (!xhci->erstba_hi_lo) {
> +            xhci_er_reset(xhci, v);
> +        }
>          break;
>      case 0x18: /* ERDP low */
>          if (val & ERDP_EHB) {
> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
>      DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
>      DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
>                       DeviceState *),
> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
>  };
>  
>  static void xhci_class_init(ObjectClass *klass, void *data)
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> index 9c3974f148..cf3f074261 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -189,6 +189,7 @@ typedef struct XHCIState {
>      uint32_t numports_3;
>      uint32_t numintrs;
>      uint32_t numslots;
> +    bool erstba_hi_lo;

Could you use the "quirk" prefix for the device and property name?

With those changes,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

With your patch, if the target does do a 64-bit write to the address,
what happens? I wonder if that's something the device is supposed to
cope with but doesn't work or just works by luck today... I would say
that's a separate problem though, if you can get Linux working okay
with this approach.

Thanks,
Nick

>      uint32_t flags;
>      uint32_t max_pstreams_mask;
>      void (*intr_update)(XHCIState *s, int n, bool enable);

Re: [PATCH 2/2] hw/usb/hcd-dwc3: Set erstba-hi-lo property

[Nicholas Piggin]

On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
> The dwc3 hardware requires the ERSTBA address to be written in
> high-low order.
>
> From information found in the Linux kernel:

In fact this info could be contained within this patch rather
than duplicated in both. This is the one for the particular
hardware.

>
> [Synopsys]- The host controller was design to support ERST setting
> during the RUN state. But since there is a limitation in controller
> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
> It is supported when the ERSTBA is programmed in 64bit,
> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>
> [Synopsys]- The internal initialization of event ring fetches
> the "Event Ring Segment Table Entry" based on the indication of
> ERSTBA_LO written.
>
> Inform the XHCI core to expect ERSTBA to be written in high-low order.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Should this go to qemu-stable?

Thanks,
Nick

> ---
>  hw/usb/hcd-dwc3.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/usb/hcd-dwc3.c b/hw/usb/hcd-dwc3.c
> index 0bceee2712..6783d55526 100644
> --- a/hw/usb/hcd-dwc3.c
> +++ b/hw/usb/hcd-dwc3.c
> @@ -603,6 +603,7 @@ static void usb_dwc3_realize(DeviceState *dev, Error **errp)
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      Error *err = NULL;
>  
> +    qdev_prop_set_bit(DEVICE(&s->sysbus_xhci), "erstba-hi-lo", true);
>      sysbus_realize(SYS_BUS_DEVICE(&s->sysbus_xhci), &err);
>      if (err) {
>          error_propagate(errp, err);

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/11/25 00:40, Nicholas Piggin wrote:
> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
>> According to the XHCI specification, ERSTBA should be written in Low-High
>> order. The Linux kernel writes the high word first. This results in an
>> initialization failure.
> 
> This should probably be reworded, it's not so much that Linux does it,
> this kind of implies a Linux bug. It is that the hardware requires it
> and Linux works around such quirk.
> 
>    According to the XHCI specification, ERSTBA should be written in Low-High
>    order, however some controllers have a quirk that requires the low
>    word to be written last.
> 
>>
>> The following information is found in the Linux kernel commit log.
>>
>> [Synopsys]- The host controller was design to support ERST setting
>> during the RUN state. But since there is a limitation in controller
>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>> It is supported when the ERSTBA is programmed in 64bit,
>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>
>> [Synopsys]- The internal initialization of event ring fetches
>> the "Event Ring Segment Table Entry" based on the indication of
>> ERSTBA_LO written.
> 
> Could you include a reference to the commit in the normal form?
> 
> The following information is found in the changelog for Linux kernel
> commit sha ("blah").
> 
>>
>> Add property to support writing the high word first.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   hw/usb/hcd-xhci.c | 8 +++++++-
>>   hw/usb/hcd-xhci.h | 1 +
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>> index 64c3a23b9b..8c0ba569c8 100644
>> --- a/hw/usb/hcd-xhci.c
>> +++ b/hw/usb/hcd-xhci.c
>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
>>           } else {
>>               intr->erstba_low = val & 0xffffffc0;
>>           }
>> +        if (xhci->erstba_hi_lo) {
>> +            xhci_er_reset(xhci, v);
>> +        }
>>           break;
>>       case 0x14: /* ERSTBA high */
>>           intr->erstba_high = val;
>> -        xhci_er_reset(xhci, v);
>> +        if (!xhci->erstba_hi_lo) {
>> +            xhci_er_reset(xhci, v);
>> +        }
>>           break;
>>       case 0x18: /* ERDP low */
>>           if (val & ERDP_EHB) {
>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
>>       DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
>>       DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
>>                        DeviceState *),
>> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
>>   };
>>   
>>   static void xhci_class_init(ObjectClass *klass, void *data)
>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
>> index 9c3974f148..cf3f074261 100644
>> --- a/hw/usb/hcd-xhci.h
>> +++ b/hw/usb/hcd-xhci.h
>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
>>       uint32_t numports_3;
>>       uint32_t numintrs;
>>       uint32_t numslots;
>> +    bool erstba_hi_lo;
> 
> Could you use the "quirk" prefix for the device and property name?
> 
> With those changes,
> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
> With your patch, if the target does do a 64-bit write to the address,
> what happens? I wonder if that's something the device is supposed to

Sorry, I have no means to test this, so I have no idea what happens
in this situation.

> cope with but doesn't work or just works by luck today... I would say
> that's a separate problem though, if you can get Linux working okay
> with this approach.
> 

Linux always writes four bytes at a time, so any change in qemu to explicitly
handle 8-byte writes would not make a difference.

Guenter

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On 4/11/25 00:40, Nicholas Piggin wrote:
> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
>> According to the XHCI specification, ERSTBA should be written in Low-High
>> order. The Linux kernel writes the high word first. This results in an
>> initialization failure.
> 
> This should probably be reworded, it's not so much that Linux does it,
> this kind of implies a Linux bug. It is that the hardware requires it
> and Linux works around such quirk.
> 
>    According to the XHCI specification, ERSTBA should be written in Low-High
>    order, however some controllers have a quirk that requires the low
>    word to be written last.
> 
>>
>> The following information is found in the Linux kernel commit log.
>>
>> [Synopsys]- The host controller was design to support ERST setting
>> during the RUN state. But since there is a limitation in controller
>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>> It is supported when the ERSTBA is programmed in 64bit,
>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>
>> [Synopsys]- The internal initialization of event ring fetches
>> the "Event Ring Segment Table Entry" based on the indication of
>> ERSTBA_LO written.
> 
> Could you include a reference to the commit in the normal form?
> 
> The following information is found in the changelog for Linux kernel
> commit sha ("blah").
> 
>>
>> Add property to support writing the high word first.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   hw/usb/hcd-xhci.c | 8 +++++++-
>>   hw/usb/hcd-xhci.h | 1 +
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>> index 64c3a23b9b..8c0ba569c8 100644
>> --- a/hw/usb/hcd-xhci.c
>> +++ b/hw/usb/hcd-xhci.c
>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
>>           } else {
>>               intr->erstba_low = val & 0xffffffc0;
>>           }
>> +        if (xhci->erstba_hi_lo) {
>> +            xhci_er_reset(xhci, v);
>> +        }
>>           break;
>>       case 0x14: /* ERSTBA high */
>>           intr->erstba_high = val;
>> -        xhci_er_reset(xhci, v);
>> +        if (!xhci->erstba_hi_lo) {
>> +            xhci_er_reset(xhci, v);
>> +        }
>>           break;
>>       case 0x18: /* ERDP low */
>>           if (val & ERDP_EHB) {
>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
>>       DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
>>       DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
>>                        DeviceState *),
>> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
>>   };
>>   
>>   static void xhci_class_init(ObjectClass *klass, void *data)
>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
>> index 9c3974f148..cf3f074261 100644
>> --- a/hw/usb/hcd-xhci.h
>> +++ b/hw/usb/hcd-xhci.h
>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
>>       uint32_t numports_3;
>>       uint32_t numintrs;
>>       uint32_t numslots;
>> +    bool erstba_hi_lo;
> 
> Could you use the "quirk" prefix for the device and property name?
> 

Done all, except then I noticed that you want me to prepend "quirk"
and I appended it out of habit. So I'll need to start over :-(.

Guenter

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 13. April 2025 16:03:16 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On 4/11/25 00:40, Nicholas Piggin wrote:
>> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
>>> According to the XHCI specification, ERSTBA should be written in Low-High
>>> order. The Linux kernel writes the high word first. This results in an
>>> initialization failure.
>> 
>> This should probably be reworded, it's not so much that Linux does it,
>> this kind of implies a Linux bug. It is that the hardware requires it
>> and Linux works around such quirk.
>> 
>>    According to the XHCI specification, ERSTBA should be written in Low-High
>>    order, however some controllers have a quirk that requires the low
>>    word to be written last.
>> 
>>> 
>>> The following information is found in the Linux kernel commit log.
>>> 
>>> [Synopsys]- The host controller was design to support ERST setting
>>> during the RUN state. But since there is a limitation in controller
>>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>>> It is supported when the ERSTBA is programmed in 64bit,
>>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>>> 
>>> [Synopsys]- The internal initialization of event ring fetches
>>> the "Event Ring Segment Table Entry" based on the indication of
>>> ERSTBA_LO written.
>> 
>> Could you include a reference to the commit in the normal form?
>> 
>> The following information is found in the changelog for Linux kernel
>> commit sha ("blah").
>> 
>>> 
>>> Add property to support writing the high word first.
>>> 
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>>   hw/usb/hcd-xhci.c | 8 +++++++-
>>>   hw/usb/hcd-xhci.h | 1 +
>>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>>> index 64c3a23b9b..8c0ba569c8 100644
>>> --- a/hw/usb/hcd-xhci.c
>>> +++ b/hw/usb/hcd-xhci.c
>>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
>>>           } else {
>>>               intr->erstba_low = val & 0xffffffc0;
>>>           }
>>> +        if (xhci->erstba_hi_lo) {
>>> +            xhci_er_reset(xhci, v);
>>> +        }
>>>           break;
>>>       case 0x14: /* ERSTBA high */
>>>           intr->erstba_high = val;
>>> -        xhci_er_reset(xhci, v);
>>> +        if (!xhci->erstba_hi_lo) {
>>> +            xhci_er_reset(xhci, v);
>>> +        }
>>>           break;
>>>       case 0x18: /* ERDP low */
>>>           if (val & ERDP_EHB) {
>>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
>>>       DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
>>>       DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
>>>                        DeviceState *),
>>> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
>>>   };
>>>     static void xhci_class_init(ObjectClass *klass, void *data)
>>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
>>> index 9c3974f148..cf3f074261 100644
>>> --- a/hw/usb/hcd-xhci.h
>>> +++ b/hw/usb/hcd-xhci.h
>>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
>>>       uint32_t numports_3;
>>>       uint32_t numintrs;
>>>       uint32_t numslots;
>>> +    bool erstba_hi_lo;
>> 
>> Could you use the "quirk" prefix for the device and property name?
>> 
>
>Done all, except then I noticed that you want me to prepend "quirk"
>and I appended it out of habit. So I'll need to start over :-(.

Ping

Any news? Would be nice if a fix made it into 10.1.

>
>Guenter
>

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Guenter Roeck]

On Wed, Jul 16, 2025 at 07:59:16PM +0000, Bernhard Beschow wrote:
> 
> 
> Am 13. April 2025 16:03:16 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
> >On 4/11/25 00:40, Nicholas Piggin wrote:
> >> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
> >>> According to the XHCI specification, ERSTBA should be written in Low-High
> >>> order. The Linux kernel writes the high word first. This results in an
> >>> initialization failure.
> >> 
> >> This should probably be reworded, it's not so much that Linux does it,
> >> this kind of implies a Linux bug. It is that the hardware requires it
> >> and Linux works around such quirk.
> >> 
> >>    According to the XHCI specification, ERSTBA should be written in Low-High
> >>    order, however some controllers have a quirk that requires the low
> >>    word to be written last.
> >> 
> >>> 
> >>> The following information is found in the Linux kernel commit log.
> >>> 
> >>> [Synopsys]- The host controller was design to support ERST setting
> >>> during the RUN state. But since there is a limitation in controller
> >>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
> >>> It is supported when the ERSTBA is programmed in 64bit,
> >>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
> >>> 
> >>> [Synopsys]- The internal initialization of event ring fetches
> >>> the "Event Ring Segment Table Entry" based on the indication of
> >>> ERSTBA_LO written.
> >> 
> >> Could you include a reference to the commit in the normal form?
> >> 
> >> The following information is found in the changelog for Linux kernel
> >> commit sha ("blah").
> >> 
> >>> 
> >>> Add property to support writing the high word first.
> >>> 
> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> >>> ---
> >>>   hw/usb/hcd-xhci.c | 8 +++++++-
> >>>   hw/usb/hcd-xhci.h | 1 +
> >>>   2 files changed, 8 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> >>> index 64c3a23b9b..8c0ba569c8 100644
> >>> --- a/hw/usb/hcd-xhci.c
> >>> +++ b/hw/usb/hcd-xhci.c
> >>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
> >>>           } else {
> >>>               intr->erstba_low = val & 0xffffffc0;
> >>>           }
> >>> +        if (xhci->erstba_hi_lo) {
> >>> +            xhci_er_reset(xhci, v);
> >>> +        }
> >>>           break;
> >>>       case 0x14: /* ERSTBA high */
> >>>           intr->erstba_high = val;
> >>> -        xhci_er_reset(xhci, v);
> >>> +        if (!xhci->erstba_hi_lo) {
> >>> +            xhci_er_reset(xhci, v);
> >>> +        }
> >>>           break;
> >>>       case 0x18: /* ERDP low */
> >>>           if (val & ERDP_EHB) {
> >>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
> >>>       DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
> >>>       DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
> >>>                        DeviceState *),
> >>> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
> >>>   };
> >>>     static void xhci_class_init(ObjectClass *klass, void *data)
> >>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> >>> index 9c3974f148..cf3f074261 100644
> >>> --- a/hw/usb/hcd-xhci.h
> >>> +++ b/hw/usb/hcd-xhci.h
> >>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
> >>>       uint32_t numports_3;
> >>>       uint32_t numintrs;
> >>>       uint32_t numslots;
> >>> +    bool erstba_hi_lo;
> >> 
> >> Could you use the "quirk" prefix for the device and property name?
> >> 
> >
> >Done all, except then I noticed that you want me to prepend "quirk"
> >and I appended it out of habit. So I'll need to start over :-(.
> 
> Ping
> 
> Any news? Would be nice if a fix made it into 10.1.
> 

Sorry, I ran out of time and won't be able to look into this again
anytime soon. I don't mind if someone else wants to pick it up.

Guenter

Re: [PATCH 1/2] hw: usb: xhci: Add property to support writing ERSTBA in high-low order

[Bernhard Beschow]

Am 16. Juli 2025 20:07:25 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>On Wed, Jul 16, 2025 at 07:59:16PM +0000, Bernhard Beschow wrote:
>> 
>> 
>> Am 13. April 2025 16:03:16 UTC schrieb Guenter Roeck <linux@roeck-us.net>:
>> >On 4/11/25 00:40, Nicholas Piggin wrote:
>> >> On Sun Apr 6, 2025 at 12:00 AM AEST, Guenter Roeck wrote:
>> >>> According to the XHCI specification, ERSTBA should be written in Low-High
>> >>> order. The Linux kernel writes the high word first. This results in an
>> >>> initialization failure.
>> >> 
>> >> This should probably be reworded, it's not so much that Linux does it,
>> >> this kind of implies a Linux bug. It is that the hardware requires it
>> >> and Linux works around such quirk.
>> >> 
>> >>    According to the XHCI specification, ERSTBA should be written in Low-High
>> >>    order, however some controllers have a quirk that requires the low
>> >>    word to be written last.
>> >> 
>> >>> 
>> >>> The following information is found in the Linux kernel commit log.
>> >>> 
>> >>> [Synopsys]- The host controller was design to support ERST setting
>> >>> during the RUN state. But since there is a limitation in controller
>> >>> in supporting separate ERSTBA_HI and ERSTBA_LO programming,
>> >>> It is supported when the ERSTBA is programmed in 64bit,
>> >>> or in 32 bit mode ERSTBA_HI before ERSTBA_LO
>> >>> 
>> >>> [Synopsys]- The internal initialization of event ring fetches
>> >>> the "Event Ring Segment Table Entry" based on the indication of
>> >>> ERSTBA_LO written.
>> >> 
>> >> Could you include a reference to the commit in the normal form?
>> >> 
>> >> The following information is found in the changelog for Linux kernel
>> >> commit sha ("blah").
>> >> 
>> >>> 
>> >>> Add property to support writing the high word first.
>> >>> 
>> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> >>> ---
>> >>>   hw/usb/hcd-xhci.c | 8 +++++++-
>> >>>   hw/usb/hcd-xhci.h | 1 +
>> >>>   2 files changed, 8 insertions(+), 1 deletion(-)
>> >>> 
>> >>> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
>> >>> index 64c3a23b9b..8c0ba569c8 100644
>> >>> --- a/hw/usb/hcd-xhci.c
>> >>> +++ b/hw/usb/hcd-xhci.c
>> >>> @@ -3107,10 +3107,15 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
>> >>>           } else {
>> >>>               intr->erstba_low = val & 0xffffffc0;
>> >>>           }
>> >>> +        if (xhci->erstba_hi_lo) {
>> >>> +            xhci_er_reset(xhci, v);
>> >>> +        }
>> >>>           break;
>> >>>       case 0x14: /* ERSTBA high */
>> >>>           intr->erstba_high = val;
>> >>> -        xhci_er_reset(xhci, v);
>> >>> +        if (!xhci->erstba_hi_lo) {
>> >>> +            xhci_er_reset(xhci, v);
>> >>> +        }
>> >>>           break;
>> >>>       case 0x18: /* ERDP low */
>> >>>           if (val & ERDP_EHB) {
>> >>> @@ -3636,6 +3641,7 @@ static const Property xhci_properties[] = {
>> >>>       DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
>> >>>       DEFINE_PROP_LINK("host",    XHCIState, hostOpaque, TYPE_DEVICE,
>> >>>                        DeviceState *),
>> >>> +    DEFINE_PROP_BOOL("erstba-hi-lo", XHCIState, erstba_hi_lo, false),
>> >>>   };
>> >>>     static void xhci_class_init(ObjectClass *klass, void *data)
>> >>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
>> >>> index 9c3974f148..cf3f074261 100644
>> >>> --- a/hw/usb/hcd-xhci.h
>> >>> +++ b/hw/usb/hcd-xhci.h
>> >>> @@ -189,6 +189,7 @@ typedef struct XHCIState {
>> >>>       uint32_t numports_3;
>> >>>       uint32_t numintrs;
>> >>>       uint32_t numslots;
>> >>> +    bool erstba_hi_lo;
>> >> 
>> >> Could you use the "quirk" prefix for the device and property name?
>> >> 
>> >
>> >Done all, except then I noticed that you want me to prepend "quirk"
>> >and I appended it out of habit. So I'll need to start over :-(.
>> 
>> Ping
>> 
>> Any news? Would be nice if a fix made it into 10.1.
>> 
>
>Sorry, I ran out of time and won't be able to look into this again
>anytime soon. I don't mind if someone else wants to pick it up.

Okay, I will look into it as soon as I find some time.

Bernhard

>
>Guenter