[SLOF] [PATCH] usb: don't use 64bit accessors

[Laurent Vivier]

On 22/07/2020 12:35, David Gibson wrote:
> On Tue, Jul 21, 2020 at 03:13:51PM +0200, Laurent Vivier wrote:
>> A recent change in QEMU[1] breaks the 64bit access for XHCI and makes it
>> unusable.
>> QEMU always reports the AC64 bit so 64bit should be supported, but in fact
>> SLOF doesn't check for this bit and always does a 64bit access.
>>
>> We need to fix QEMU to allow 64bit access when it reports AC64, but we need
>> also to fix SLOF to not use 64bit access when the AC64 bit is not set.
>>
>> The easiest way to do that is, like linux kernel in xhci_read_64() and
>> xhci_write_64() (see drivers/usb/host/xhci.h), to access always a 64bit
>> register using two 32bit accesses, as the specs allow that.
>>
>> [1] 5d971f9e6725 ("memory: Revert "memory: accept mismatching sizes in memory_region_access_valid"")
>>
>> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
> 
> Now that we've fixed this on the qemu side, this isn't so necessary.
> Alexey seems to think there might be some value to it for robustness,
> but we don't need to rush to get this into the SLOF that ships with
> qemu 5.1

I agree. I've posted the patch because I started by fixing SLOF so it
was already written.

Thanks,
Laurent

>> ---
>>  lib/libusb/tools.h | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/libusb/tools.h b/lib/libusb/tools.h
>> index f531175c10a6..5758334b0247 100644
>> --- a/lib/libusb/tools.h
>> +++ b/lib/libusb/tools.h
>> @@ -54,13 +54,21 @@ static inline void write_reg16(uint16_t *reg, uint16_t value)
>>  
>>  static inline uint64_t read_reg64(uint64_t *reg)
>>  {
>> -	return bswap_64(ci_read_64(reg));
>> +	uint32_t *p = (uint32_t*)reg;
>> +	uint32_t low, high;
>> +
>> +	low = read_reg32(p);
>> +	high = read_reg32(p + 1);
>> +
>> +	return low + ((uint64_t)high << 32);
>>  }
>>  
>>  static inline void write_reg64(uint64_t *reg, uint64_t value)
>>  {
>> -	mb();
>> -	ci_write_64(reg, bswap_64(value));
>> +	uint32_t *p = (uint32_t*)reg;
>> +
>> +	write_reg32(p, value);
>> +	write_reg32(p + 1, value >> 32);
>>  }
>>  
>>  static inline uint32_t ci_read_reg(uint32_t *reg)
>