[U-Boot] [RFC PATCH] usb: ehci: do not invalidate a NULL buffer

[Marek Vasut]

On 11/17/2017 03:28 PM, Dirk Behme wrote:
> Its a valid use case to call ehci_submit_async() with a NULL buffer
> with length 0. E.g. from usb_set_configuration().
> 
> As invalidate_dcache_range() isn't able to judge if the address
> NULL is valid or not (depending on the SoC hardware configuration it
> might be valid) do the check in ehci_submit_async() as here we know
> that we don't have to invalidate such a buffer.
> 
> Signed-off-by: Dirk Behme <dirk.behme at de.bosch.com>

Looks OK, what about the other cache ops in EHCI, are they also affected?

> ---
> Notes:
> 
> This was found on an older vendor specific U-Boot on an ARMv8 SoC
> with a MMU configuration where address 0x00000000 is invalid.
> 
> The callstack I've seen is
> 
> usb_set_configuration(), calls
>  -> usb_control_msg() with *data == NULL and size == 0
>   -> submit_control_msg()
>    -> _ehci_submit_control_msg()
>     -> ehci_submit_async()
> 
> ehci_submit_async() called  __asm_invalidate_dcache_range() from
> arch/arm/cpu/armv8/cache.S, then. Resulting in an exception trying
> to use address 0 in x0 (dc ivac, x0).
> 
> I'm slightly unsure why this hasn't been catched or complained about
> previously, already. Maybe I missed anything while working with an older
> vendor U-Boot? Sorry in this case.
> 
> Best regards
> 
> Dirk
> ---
>  drivers/usb/host/ehci-hcd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index be3e842dcc..32c661b37e 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -595,8 +595,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
>  	 * dangerous operation, it's responsibility of the calling
>  	 * code to make sure enough space is reserved.
>  	 */
> -	invalidate_dcache_range((unsigned long)buffer,
> -		ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN));
> +	if (buffer != NULL && length > 0)
> +		invalidate_dcache_range((unsigned long)buffer,
> +			ALIGN((unsigned long)buffer + length, ARCH_DMA_MINALIGN));
>  
>  	/* Check that the TD processing happened */
>  	if (QT_TOKEN_GET_STATUS(token) & QT_TOKEN_STATUS_ACTIVE)
> 


-- 
Best regards,
Marek Vasut