linux-kernel - Re: [PATCH V7 1/2] arm64: dtsi: Fix SDMMC address range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190110144731.GA25353@ulmo>
Date:   Thu, 10 Jan 2019 15:47:31 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Sowjanya Komatineni <skomatineni@...dia.com>
Cc:     adrian.hunter@...el.com, ulf.hansson@...aro.org,
        jonathanh@...dia.com, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org
Subject: Re: [PATCH V7 1/2] arm64: dtsi: Fix SDMMC address range

On Wed, Jan 02, 2019 at 11:36:47AM -0800, Sowjanya Komatineni wrote:
> This patch fixes the SDMMC Controllers address space to be exact
> defined register address range as per the design.
> 
> SDMMC Controller supporting Command Queue has CQHCI registers at
> offset 0xF000.
> 
> This fix helps to identify the Tegra SDMMC Controllers supporting
> Command Queue based on the size of address space.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra186.dtsi | 6 +++---
>  arch/arm64/boot/dts/nvidia/tegra194.dtsi | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)

After applying these patches I'm having second thoughts about the DT
aspect of this. I know you and Timo had originally argued to advertise
the capability via an extra property in DT instead of updating the reg
property. In retrospect, I think that's the right thing to do, after
all.

The problem I'm running into is that if I apply patch 2/2 without the
first patch, then both Jetson TX2 and Jetson AGX Xavier crash on boot
because they try to access these registers (the I/O memory size is
0x10000 for all controllers). So we're effectively breaking ABI with
existing device trees.

I don't know of a way to work around that other than the separate
property. Would you mind changing the series to contain:

	1) a patch updating the device tree bindings with the new
	   optional property that would mark SDMMC4 as CQE capable
	   (make sure to Cc devicetree@...r.kernel.org on that patch)

	2) replace this patch by one which only adds the new
	   "supports-cqe" property

	3) update the second patch to make the decision based on the
	   flag rather than the I/O memory size

Sorry for the back and forth on this. I evidently hadn't thought this
through.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)