Re: USB2 / USB3 compatibility problems: xhci_hcd 0000:00:06.0: WARN Wrong bounce buffer write length: 0 != 512

[Andreas Hartmann <andihartmann@01019freenet.de>]

Hello Mathias,

I'll test it ASAP!

Thanks
Andreas

On 26.01.21 at 18:29 Mathias Nyman wrote:
> 
>>> I'm not sure if it's important for you to know: The driver doesn't use struct scatterlist or num_mapped_sgs at all (if it's meant to be used by the sender at all).
>>>
>>> But it sets URB_NO_TRANSFER_DMA_MAP (for data transfer among others).
>>>
>>> Mlme packets are sent w/o bulk and w/o setting URB_NO_TRANSFER_DMA_MAP. All other packets are sent with URB_NO_TRANSFER_DMA_MAP turned on.
>>>
>>
>> Ok, thanks, I see what's going on here.
>>
>> Short recap of xhci alignment requirements.
>> 1. Data pointed to by a transfer request block (TRB) may not span 64k boundary
>> 2. If a transfer contains several TRBs, and spans over two TRB ringbuffer
>>    segments, then the sum of the TRB data in the first segment must be a 
>>    multiple of max packets in size.
>>  
>> Code assumes that if transfer is split into several blocks,(TRBs) and a block
>> in the middle of a transfer is smaller than max packet size, then it must be sg list.
>>
>> But this is not necessarily the case if data was already DMA mapped beforehand.
>> Data might start just before a 64k boundary, causing first TRB to be less than
>> packet size.
>>
>> I'll start implementing a fix for this.
> 
> Got a first iteration ready,
> any change you could try it out?
> 
> Thanks
> -Mathias
> 
> 8<---
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 5677b81c0915..8df30618aaf1 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -699,11 +699,16 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
>  	dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
>  			 DMA_FROM_DEVICE);
>  	/* for in tranfers we need to copy the data from bounce to sg */
> -	len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
> -			     seg->bounce_len, seg->bounce_offs);
> -	if (len != seg->bounce_len)
> -		xhci_warn(xhci, "WARN Wrong bounce buffer read length: %zu != %d\n",
> -				len, seg->bounce_len);
> +	if (urb->num_sgs) {
> +		len = sg_pcopy_from_buffer(urb->sg, urb->num_sgs, seg->bounce_buf,
> +					   seg->bounce_len, seg->bounce_offs);
> +		if (len != seg->bounce_len)
> +			xhci_warn(xhci, "WARN Wrong bounce buffer read length: %zu != %d\n",
> +				  len, seg->bounce_len);
> +	} else {
> +		memcpy(urb->transfer_buffer + seg->bounce_offs, seg->bounce_buf,
> +		       seg->bounce_len);
> +	}
>  	seg->bounce_len = 0;
>  	seg->bounce_offs = 0;
>  }
> @@ -3275,12 +3280,16 @@ static int xhci_align_td(struct xhci_hcd *xhci, struct urb *urb, u32 enqd_len,
>  
>  	/* create a max max_pkt sized bounce buffer pointed to by last trb */
>  	if (usb_urb_dir_out(urb)) {
> -		len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
> -				   seg->bounce_buf, new_buff_len, enqd_len);
> -		if (len != new_buff_len)
> -			xhci_warn(xhci,
> -				"WARN Wrong bounce buffer write length: %zu != %d\n",
> -				len, new_buff_len);
> +		if (urb->num_sgs) {
> +			len = sg_pcopy_to_buffer(urb->sg, urb->num_sgs,
> +						 seg->bounce_buf, new_buff_len, enqd_len);
> +			if (len != new_buff_len)
> +				xhci_warn(xhci, "WARN Wrong bounce buffer write length: %zu != %d\n",
> +					  len, new_buff_len);
> +		} else {
> +			memcpy(seg->bounce_buf, urb->transfer_buffer + enqd_len, new_buff_len);
> +		}
> +
>  		seg->bounce_dma = dma_map_single(dev, seg->bounce_buf,
>  						 max_pkt, DMA_TO_DEVICE);
>  	} else {
>