* [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits
@ 2026-07-30 5:45 Lachlan Hodges
2026-07-30 8:04 ` Michal Pecio
0 siblings, 1 reply; 3+ messages in thread
From: Lachlan Hodges @ 2026-07-30 5:45 UTC (permalink / raw)
To: mathias.nyman, gregkh, oneukum; +Cc: linux-usb, arien.judge, Lachlan Hodges
The CRCR register is 64 bits wide - commit abe93f27cdd7
("xhci: use BIT macro") changed the flag definitions from (1 << n),
a signed int, to BIT(n), an unsigned long. Within
xhci_set_cmd_ring_deq(), the following operation is performed on the
CRCR register:
...
crcr &= ~CMD_RING_PTR_MASK;
crcr |= deq_dma;
crcr &= ~CMD_RING_CYCLE;
crcr |= xhci->cmd_ring->cycle_state;
...
Previously, ~CMD_RING_CYCLE was ~(int)1, a negative signed value
(0xFFFFFFFE with the sign bit set). Widening a negative signed int to
u64 sign-extends it to 0xFFFFFFFFFFFFFFFE, correctly clearing only bit
0 and preserving the 64-bit pointer written two lines above.
After the change when running on 32 bit kernels, ~CMD_RING_CYCLE is
~(unsigned long)1UL. On a 32-bit host this is an unsigned 32-bit
value (0xFFFFFFFE, no sign bit). Widening an unsigned value to u64
zero-extends it instead (0x00000000FFFFFFFE), so the subsequent AND
silently clears bits 63:32 of crcr, truncating the command ring
pointer that was just written before the value reaches hardware.
To fix, similar to how CMD_RING_PTR_MASK is defined, make sure we
use the BIT_ULL variant when defining the CRCR bits.
Fixes: abe93f27cdd7 ("xhci: use BIT macro")
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
---
Hi, I experienced this when running the tip of wireless-next on
a raspberry pi 4B compiled for arm32. The main symptoms were the
following log message:
[ 0.549897] raspberrypi-firmware soc:firmware: Attached to firmware from 2021-02-25T12:11:39
[ 0.626859] xhci_hcd 0000:01:00.0: xHCI Host Controller
[ 0.626889] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 1
[ 0.812619] xhci_hcd 0000:01:00.0: hcc params 0x002841eb hci version 0x100 quirks 0x0000200000000890
[ 0.813188] xhci_hcd 0000:01:00.0: xHCI Host Controller
[ 0.813203] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 2
[ 0.813219] xhci_hcd 0000:01:00.0: Host supports USB 3.0 SuperSpeed
[ 0.813602] hub 1-0:1.0: USB hub found
[ 0.814052] hub 2-0:1.0: USB hub found
[ 0.952714] xhci_hcd 0000:01:00.0: ERROR mismatched command completion event
Additionally running lsusb just hangs. Running the same kernel compiled
for aarch64 worked fine. Bisected to the commit in the Fixes line
above. Additionally a USB device plugged in to the USB3.0 (or 2.0)
did not enumerate. Once this patch is applied the USB device
enumerates properly on the tip of wireless-next 4a0bd262df75 ("wifi:
mac80211: fix per-STA profile length in cross-link CSA parsing").
Happy to test alternate fix proposals! Thanks.
lachlan
---
drivers/usb/host/xhci.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d02046a573e4..df014855c659 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -190,13 +190,13 @@ struct xhci_op_regs {
/* CRCR - Command Ring Control Register - cmd_ring bitmasks */
/* bit 0 - Cycle bit indicates the ownership of the command ring */
-#define CMD_RING_CYCLE BIT(0)
+#define CMD_RING_CYCLE BIT_ULL(0)
/* stop ring operation after completion of the currently executing command */
-#define CMD_RING_PAUSE BIT(1)
+#define CMD_RING_PAUSE BIT_ULL(1)
/* stop ring immediately - abort the currently executing command */
-#define CMD_RING_ABORT BIT(2)
+#define CMD_RING_ABORT BIT_ULL(2)
/* true: command ring is running */
-#define CMD_RING_RUNNING BIT(3)
+#define CMD_RING_RUNNING BIT_ULL(3)
/* bits 63:6 - Command Ring pointer */
#define CMD_RING_PTR_MASK GENMASK_ULL(63, 6)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits
2026-07-30 5:45 [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits Lachlan Hodges
@ 2026-07-30 8:04 ` Michal Pecio
2026-07-30 9:07 ` Mathias Nyman
0 siblings, 1 reply; 3+ messages in thread
From: Michal Pecio @ 2026-07-30 8:04 UTC (permalink / raw)
To: Lachlan Hodges; +Cc: mathias.nyman, gregkh, oneukum, linux-usb, arien.judge
On Thu, 30 Jul 2026 15:45:21 +1000, Lachlan Hodges wrote:
> The CRCR register is 64 bits wide - commit abe93f27cdd7
> ("xhci: use BIT macro") changed the flag definitions from (1 << n),
> a signed int, to BIT(n), an unsigned long. Within
> xhci_set_cmd_ring_deq(), the following operation is performed on the
> CRCR register:
>
> ...
> crcr &= ~CMD_RING_PTR_MASK;
> crcr |= deq_dma;
> crcr &= ~CMD_RING_CYCLE;
> crcr |= xhci->cmd_ring->cycle_state;
> ...
>
> Previously, ~CMD_RING_CYCLE was ~(int)1, a negative signed value
> (0xFFFFFFFE with the sign bit set). Widening a negative signed int to
> u64 sign-extends it to 0xFFFFFFFFFFFFFFFE, correctly clearing only bit
> 0 and preserving the 64-bit pointer written two lines above.
>
> After the change when running on 32 bit kernels, ~CMD_RING_CYCLE is
> ~(unsigned long)1UL. On a 32-bit host this is an unsigned 32-bit
> value (0xFFFFFFFE, no sign bit). Widening an unsigned value to u64
> zero-extends it instead (0x00000000FFFFFFFE), so the subsequent AND
> silently clears bits 63:32 of crcr, truncating the command ring
> pointer that was just written before the value reaches hardware.
>
> To fix, similar to how CMD_RING_PTR_MASK is defined, make sure we
> use the BIT_ULL variant when defining the CRCR bits.
>
> Fixes: abe93f27cdd7 ("xhci: use BIT macro")
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
> ---
>
> Hi, I experienced this when running the tip of wireless-next on
> a raspberry pi 4B compiled for arm32. The main symptoms were the
> following log message:
>
> [ 0.549897] raspberrypi-firmware soc:firmware: Attached to firmware from 2021-02-25T12:11:39
> [ 0.626859] xhci_hcd 0000:01:00.0: xHCI Host Controller
> [ 0.626889] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 1
> [ 0.812619] xhci_hcd 0000:01:00.0: hcc params 0x002841eb hci version 0x100 quirks 0x0000200000000890
> [ 0.813188] xhci_hcd 0000:01:00.0: xHCI Host Controller
> [ 0.813203] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 2
> [ 0.813219] xhci_hcd 0000:01:00.0: Host supports USB 3.0 SuperSpeed
> [ 0.813602] hub 1-0:1.0: USB hub found
> [ 0.814052] hub 2-0:1.0: USB hub found
> [ 0.952714] xhci_hcd 0000:01:00.0: ERROR mismatched command completion event
>
> Additionally running lsusb just hangs. Running the same kernel compiled
> for aarch64 worked fine. Bisected to the commit in the Fixes line
> above. Additionally a USB device plugged in to the USB3.0 (or 2.0)
> did not enumerate. Once this patch is applied the USB device
> enumerates properly on the tip of wireless-next 4a0bd262df75 ("wifi:
> mac80211: fix per-STA profile length in cross-link CSA parsing").
Makes sense, including observed symptoms.
A system with IOMMU would fault on top of that, but the accesses
are reads, so hopefully no corruption in absence of IOMMU.
> Happy to test alternate fix proposals! Thanks.
I think the sort of code you quoted should work without surprises,
so widening these macros to ULL is right.
And there is a few more. They currently don't break anything, but
neither does CMD_RING_ABORT or CMD_RING_RUNNING for that matter.
ERST_EHB
EP_CTX_CYCLE_MASK
Let's see what Mathias thinks about it.
>
> lachlan
> ---
> drivers/usb/host/xhci.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index d02046a573e4..df014855c659 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -190,13 +190,13 @@ struct xhci_op_regs {
>
> /* CRCR - Command Ring Control Register - cmd_ring bitmasks */
> /* bit 0 - Cycle bit indicates the ownership of the command ring */
> -#define CMD_RING_CYCLE BIT(0)
> +#define CMD_RING_CYCLE BIT_ULL(0)
> /* stop ring operation after completion of the currently executing command */
> -#define CMD_RING_PAUSE BIT(1)
> +#define CMD_RING_PAUSE BIT_ULL(1)
> /* stop ring immediately - abort the currently executing command */
> -#define CMD_RING_ABORT BIT(2)
> +#define CMD_RING_ABORT BIT_ULL(2)
> /* true: command ring is running */
> -#define CMD_RING_RUNNING BIT(3)
> +#define CMD_RING_RUNNING BIT_ULL(3)
> /* bits 63:6 - Command Ring pointer */
> #define CMD_RING_PTR_MASK GENMASK_ULL(63, 6)
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits
2026-07-30 8:04 ` Michal Pecio
@ 2026-07-30 9:07 ` Mathias Nyman
0 siblings, 0 replies; 3+ messages in thread
From: Mathias Nyman @ 2026-07-30 9:07 UTC (permalink / raw)
To: Michal Pecio, Lachlan Hodges
Cc: mathias.nyman, gregkh, oneukum, linux-usb, arien.judge
On 7/30/26 11:04, Michal Pecio wrote:
> On Thu, 30 Jul 2026 15:45:21 +1000, Lachlan Hodges wrote:
>> The CRCR register is 64 bits wide - commit abe93f27cdd7
>> ("xhci: use BIT macro") changed the flag definitions from (1 << n),
>> a signed int, to BIT(n), an unsigned long. Within
>> xhci_set_cmd_ring_deq(), the following operation is performed on the
>> CRCR register:
>>
>> ...
>> crcr &= ~CMD_RING_PTR_MASK;
>> crcr |= deq_dma;
>> crcr &= ~CMD_RING_CYCLE;
>> crcr |= xhci->cmd_ring->cycle_state;
>> ...
>>
>> Previously, ~CMD_RING_CYCLE was ~(int)1, a negative signed value
>> (0xFFFFFFFE with the sign bit set). Widening a negative signed int to
>> u64 sign-extends it to 0xFFFFFFFFFFFFFFFE, correctly clearing only bit
>> 0 and preserving the 64-bit pointer written two lines above.
>>
>> After the change when running on 32 bit kernels, ~CMD_RING_CYCLE is
>> ~(unsigned long)1UL. On a 32-bit host this is an unsigned 32-bit
>> value (0xFFFFFFFE, no sign bit). Widening an unsigned value to u64
>> zero-extends it instead (0x00000000FFFFFFFE), so the subsequent AND
>> silently clears bits 63:32 of crcr, truncating the command ring
>> pointer that was just written before the value reaches hardware.
>>
>> To fix, similar to how CMD_RING_PTR_MASK is defined, make sure we
>> use the BIT_ULL variant when defining the CRCR bits.
>>
>> Fixes: abe93f27cdd7 ("xhci: use BIT macro")
>> Assisted-by: Claude:claude-sonnet-5
>> Signed-off-by: Lachlan Hodges <lachlan.hodges@morsemicro.com>
>> ---
>>
>> Hi, I experienced this when running the tip of wireless-next on
>> a raspberry pi 4B compiled for arm32. The main symptoms were the
>> following log message:
>>
>> [ 0.549897] raspberrypi-firmware soc:firmware: Attached to firmware from 2021-02-25T12:11:39
>> [ 0.626859] xhci_hcd 0000:01:00.0: xHCI Host Controller
>> [ 0.626889] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 1
>> [ 0.812619] xhci_hcd 0000:01:00.0: hcc params 0x002841eb hci version 0x100 quirks 0x0000200000000890
>> [ 0.813188] xhci_hcd 0000:01:00.0: xHCI Host Controller
>> [ 0.813203] xhci_hcd 0000:01:00.0: new USB bus registered, assigned bus number 2
>> [ 0.813219] xhci_hcd 0000:01:00.0: Host supports USB 3.0 SuperSpeed
>> [ 0.813602] hub 1-0:1.0: USB hub found
>> [ 0.814052] hub 2-0:1.0: USB hub found
>> [ 0.952714] xhci_hcd 0000:01:00.0: ERROR mismatched command completion event
>>
>> Additionally running lsusb just hangs. Running the same kernel compiled
>> for aarch64 worked fine. Bisected to the commit in the Fixes line
>> above. Additionally a USB device plugged in to the USB3.0 (or 2.0)
>> did not enumerate. Once this patch is applied the USB device
>> enumerates properly on the tip of wireless-next 4a0bd262df75 ("wifi:
>> mac80211: fix per-STA profile length in cross-link CSA parsing").
>
> Makes sense, including observed symptoms.
>
> A system with IOMMU would fault on top of that, but the accesses
> are reads, so hopefully no corruption in absence of IOMMU.
>
>> Happy to test alternate fix proposals! Thanks.
>
> I think the sort of code you quoted should work without surprises,
> so widening these macros to ULL is right.
>
> And there is a few more. They currently don't break anything, but
> neither does CMD_RING_ABORT or CMD_RING_RUNNING for that matter.
>
> ERST_EHB
> EP_CTX_CYCLE_MASK
>
> Let's see what Mathias thinks about it.
>
Fully agree,
We should probably try to get it into 7.2
Thanks Lachlan for debugging and elegantly describing the issue.
-Mathias
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-30 9:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 5:45 [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits Lachlan Hodges
2026-07-30 8:04 ` Michal Pecio
2026-07-30 9:07 ` Mathias Nyman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox