patch 8466489ef5ba48272ba4fa4ea9f8f403306de4c7 breaks Renesas USB3 controller functionality

[Marc Zyngier <marc.zyngier@arm.com>]

On Sun, 25 Mar 2018 14:26:58 +0100
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 25 March 2018 at 13:52, Marc Zyngier <marc.zyngier@arm.com> wrote:
> > On Sun, 25 Mar 2018 13:38:19 +0100,
> > Ard Biesheuvel wrote:  
> >>
> >> On 25 March 2018 at 13:31, Marc Zyngier <marc.zyngier@arm.com> wrote:  
> >> > On Sun, 25 Mar 2018 12:57:55 +0100,
> >> > Ard Biesheuvel wrote:  
> >> >>
> >> >> On 25 March 2018 at 12:51, Marc Zyngier <marc.zyngier@arm.com> wrote:  
> >> >> > On Sun, 25 Mar 2018 11:48:35 +0100,
> >> >> > Ard Biesheuvel wrote:
> >> >> >
> >> >> > Hi Ard,
> >> >> >
> >> >> > [...]
> >> >> >  
> >> >> >> > I finally found some time to work on this, and came up with an
> >> >> >> > alternative approach (it turns out that this chip is even more
> >> >> >> > braindead than I thought).
> >> >> >> >
> >> >> >> > It is slightly scary, in the sense that the USB controller seems to
> >> >> >> > perform memory accesses even when halted, and can generate faults,
> >> >> >> > but it works just fine on my system. And with this, we can drop the
> >> >> >> > hard reset at boot time. I'm still on the fence to limit it to systems
> >> >> >> > with an iommu though.
> >> >> >> >  
> >> >> >>
> >> >> >> Hi Marc,
> >> >> >>
> >> >> >> I take it you tested this on Cello?  
> >> >> >
> >> >> > Tested on Cello indeed (I should have mentioned that the first place).
> >> >> >  
> >> >> >> There, it might make sense to
> >> >> >> limit this to systems with an IOMMU, but not in the general case, I
> >> >> >> think. The reason is that it is not guaranteed that the firmware will
> >> >> >> use 32-bit addressable allocations for these data structures, even if
> >> >> >> the kernel is able to without an IOMMU. (UEFI on arm64 will not prefer
> >> >> >> 32-bit addressable memory for PCI DMA if it is available, and usually
> >> >> >> serves heap allocations [such as the ones used for these data
> >> >> >> structures] starting at the top of DRAM)  
> >> >> >
> >> >> > My main worry is that this controller will happily try and DMA from
> >> >> > zero as we wipe the 64bit registers, even when halted. On Seattle (and
> >> >> > thus Cello), this is just fine as there is nothing there, and the
> >> >> > controller aborts with the HSE bit set.
> >> >> >
> >> >> > On other systems, where memory actually exists at 0, who knows what
> >> >> > this is going to do? On the other hand, this is not worse than the
> >> >> > current situation, where we could end-up with any odd address...
> >> >> >  
> >> >>
> >> >> Is the PCI_COMMAND_MASTER bit enabled at this point? What happens if
> >> >> you clear it first?  
> >> >
> >> > Tried that. No difference whatsoever, as I still get a fault with the
> >> > device accessing address 0, and being caught by the iommu.
> >> >  
> >>
> >> Wow so this device is more broken than I thought.  
> >
> > That's my impression as well. I came to the conclusion that the only
> > way to make it behave is to crash it first, and then to reset it.
> >  
> 
> OK, so what if it doesn't crash? Without an IOMMU, it is quite likely
> that putting zeroes in the lower half of a 64-bit memory address
> produces a physical address that is valid, and so the device will
> still misbehave in that case.
> 
> If making it crash is what kicks it into submission, could we perhaps
> use U64_MAX instead?

Just tried that. It gets into some even uglier state, and never
recovers. Even doing U64_MAX, fault, and then zero doesn't help. The
problem is that it dies with something in the top 32 bits, and that's
pretty final.

If really feels that without an iommu in the path, this device is
completely unsafe, and should never be fed 64bit addresses.

	M.