Re: [PATCH] usb: xhci: fix Immediate Data Transfer endianness

[Mathias Nyman]

On 20.10.2019 4.53, Samuel Holland wrote:

The arguments to queue_trb are always byteswapped to LE for placement in
the ring, but this should not happen in the case of immediate data; the
bytes copied out of transfer_buffer are already in the correct order.
Add a complementary byteswap so the bytes end up in the ring correctly.

This was observed on BE ppc64 with a "Texas Instruments TUSB73x0
SuperSpeed USB 3.0 xHCI Host Controller [104c:8241]" as a ch341
usb-serial adapter ("1a86:7523 QinHeng Electronics HL-340 USB-Serial
adapter") always transmitting the same character (generally NUL) over
the serial link regardless of the key pressed.



Thanks, nice catch.

It's unfortunate that we ended up with a situation where this fix is the
least intrusive one.
With IDT we would just want to memcpy() bytes an not care about endianness,
but on BE we end up storing data bytes in a u64, and start with a complementary 
u64
byteswap to counter a later u32 byteswap done after splitting the u64 to upper
and lower 32 bit parts.

This because that TRB field is normally used for 64bit data buffer pointers,
and all code is written to support that

adding to queue

-Mathias

[PATCH] usb: xhci: fix Immediate Data Transfer endianness

[Samuel Holland]

The arguments to queue_trb are always byteswapped to LE for placement in
the ring, but this should not happen in the case of immediate data; the
bytes copied out of transfer_buffer are already in the correct order.
Add a complementary byteswap so the bytes end up in the ring correctly.

This was observed on BE ppc64 with a "Texas Instruments TUSB73x0
SuperSpeed USB 3.0 xHCI Host Controller [104c:8241]" as a ch341
usb-serial adapter ("1a86:7523 QinHeng Electronics HL-340 USB-Serial
adapter") always transmitting the same character (generally NUL) over
the serial link regardless of the key pressed.

Cc: sta...@vger.kernel.org
Fixes: 33e39350ebd2 ("usb: xhci: add Immediate Data Transfer support")
Signed-off-by: Samuel Holland 
---
 drivers/usb/host/xhci-ring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 85ceb43e3405..e7aab31fd9a5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3330,6 +3330,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
if (xhci_urb_suitable_for_idt(urb)) {
memcpy(_addr, urb->transfer_buffer,
   trb_buff_len);
+   le64_to_cpus(_addr);
field |= TRB_IDT;
}
}
@@ -3475,6 +3476,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t 
mem_flags,
if (xhci_urb_suitable_for_idt(urb)) {
memcpy(, urb->transfer_buffer,
   urb->transfer_buffer_length);
+   le64_to_cpus();
field |= TRB_IDT;
} else {
addr = (u64) urb->transfer_dma;
-- 
2.21.0