Re: [RFC PATCH] usb: xhci: use BIT_ULL for CRCR bits

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 <[email protected]>
>> ---
>>
>> 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