All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
To: Jan Luebbe <jlu@pengutronix.de>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Andrew Lunn <andrew@lunn.ch>, Jason Cooper <jason@lakedaemon.net>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] bus: mbus: fix window size calculation for 4GB windows
Date: Tue, 19 Sep 2017 16:43:18 +0200	[thread overview]
Message-ID: <87wp4urbp5.fsf@free-electrons.com> (raw)
In-Reply-To: <20170828152517.24506-2-jlu@pengutronix.de> (Jan Luebbe's message of "Mon, 28 Aug 2017 17:25:16 +0200")

SGkgSmFuLAogCiBPbiBsdW4uLCBhb8O7dCAyOCAyMDE3LCBKYW4gTHVlYmJlIDxqbHVAcGVuZ3V0
cm9uaXguZGU+IHdyb3RlOgoKPiBBdCBsZWFzdCB0aGUgQXJtYWRhIFhQIFNvQyBzdXBwb3J0cyA0
R0Igb24gYSBzaW5nbGUgRFJBTSB3aW5kb3cuIEJlY2F1c2UKPiB0aGUgc2l6ZSByZWdpc3RlciB2
YWx1ZXMgY29udGFpbiB0aGUgYWN0dWFsIHNpemUgLSAxLCB0aGUgTVNCIGlzIHNldCBpbgo+IHRo
YXQgY2FzZS4gRm9yIGV4YW1wbGUsIHRoZSBTRFJBTSB3aW5kb3cncyBjb250cm9sIHJlZ2lzdGVy
J3MgdmFsdWUgaXMKPiAweGZmZmZmZmUxIGZvciA0R0IgKGJpdHMgMzEgdG8gMjQgY29udGFpbiB0
aGUgc2l6ZSkuCj4KPiBUaGUgTUJVUyBkcml2ZXIgcmVhZHMgYmFjayBlYWNoIHdpbmRvdydzIHNp
emUgZnJvbSByZWdpc3RlcnMgYW5kCj4gY2FsY3VsYXRlcyB0aGUgYWN0dWFsIHNpemUgYXMgKGNv
bnRyb2xfcmVnIHwgfkREUl9TSVpFX01BU0spICsgMSwgd2hpY2gKPiBvdmVyZmxvd3MgZm9yIDMy
IGJpdCB2YWx1ZXMsIHJlc3VsdGluZyBpbiBvdGhlciBtaXNjYWxjdWxhdGlvbnMgZnVydGhlcgo+
IG9uIChhIGJhZCBSQU0gd2luZG93IGZvciB0aGUgQ0VTQSBjcnlwdG8gZW5naW5lIGNhbGN1bGF0
ZWQgYnkKPiBtdmVidV9tYnVzX3NldHVwX2NwdV90YXJnZXRfbm9vdmVybGFwKCkgaW4gbXkgY2Fz
ZSkuCj4KPiBUaGlzIHBhdGNoIGNoYW5nZXMgdGhlIHR5cGUgaW4gJ3N0cnVjdCBtYnVzX2RyYW1f
d2luZG93JyBmcm9tIHUzMiB0bwo+IHU2NCwgd2hpY2ggYWxsb3dzIHVzIHRvIGtlZXAgdXNpbmcg
dGhlIHNhbWUgcmVnaXN0ZXIgY2FsY3VsYXRpb24gY29kZSBpbgo+IG1vc3QgTUJVUy11c2luZyBk
cml2ZXJzICh3aGljaCBjYWxjdWxhdGUgLT5zaXplIC0gMSBhZ2FpbikuCj4KCllvdXIgcGF0Y2gg
bG9va3MgZ29vZCwgYnV0IGFzIGl0IGlzIGEgZml4IHdlIHNob3VsZCBhbHNvIGFwcGx5IGl0IG9u
CnN0YWJsZSwgY291bGQgeW91IHByb3ZpZGUgdGhlIGNvbW1pdCB0byBmaXg/CgpUaGFua3MsCgpH
cmVnb3J5Cgo+IFNpZ25lZC1vZmYtYnk6IEphbiBMdWViYmUgPGpsdUBwZW5ndXRyb25peC5kZT4K
PiAtLS0KPiAgZHJpdmVycy9idXMvbXZlYnUtbWJ1cy5jIHwgMiArLQo+ICBpbmNsdWRlL2xpbnV4
L21idXMuaCAgICAgfCA0ICsrLS0KPiAgMiBmaWxlcyBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKyks
IDMgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9idXMvbXZlYnUtbWJ1cy5j
IGIvZHJpdmVycy9idXMvbXZlYnUtbWJ1cy5jCj4gaW5kZXggYzdmMzk2OTAzMTg0Li43MGRiNGQ1
NjM4YTYgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9idXMvbXZlYnUtbWJ1cy5jCj4gKysrIGIvZHJp
dmVycy9idXMvbXZlYnUtbWJ1cy5jCj4gQEAgLTcyMCw3ICs3MjAsNyBAQCBtdmVidV9tYnVzX2Rl
ZmF1bHRfc2V0dXBfY3B1X3RhcmdldChzdHJ1Y3QgbXZlYnVfbWJ1c19zdGF0ZSAqbWJ1cykKPiAg
CQkJaWYgKG1idXMtPmh3X2lvX2NvaGVyZW5jeSkKPiAgCQkJCXctPm1idXNfYXR0ciB8PSBBVFRS
X0hXX0NPSEVSRU5DWTsKPiAgCQkJdy0+YmFzZSA9IGJhc2UgJiBERFJfQkFTRV9DU19MT1dfTUFT
SzsKPiAtCQkJdy0+c2l6ZSA9IChzaXplIHwgfkREUl9TSVpFX01BU0spICsgMTsKPiArCQkJdy0+
c2l6ZSA9ICh1NjQpKHNpemUgfCB+RERSX1NJWkVfTUFTSykgKyAxOwo+ICAJCX0KPiAgCX0KPiAg
CW12ZWJ1X21idXNfZHJhbV9pbmZvLm51bV9jcyA9IGNzOwo+IGRpZmYgLS1naXQgYS9pbmNsdWRl
L2xpbnV4L21idXMuaCBiL2luY2x1ZGUvbGludXgvbWJ1cy5oCj4gaW5kZXggMGQzZjE0ZmQyNjIx
Li40NzczMTQ1MjQ2ZWQgMTAwNjQ0Cj4gLS0tIGEvaW5jbHVkZS9saW51eC9tYnVzLmgKPiArKysg
Yi9pbmNsdWRlL2xpbnV4L21idXMuaAo+IEBAIC0zMSw4ICszMSw4IEBAIHN0cnVjdCBtYnVzX2Ry
YW1fdGFyZ2V0X2luZm8KPiAgCXN0cnVjdCBtYnVzX2RyYW1fd2luZG93IHsKPiAgCQl1OAljc19p
bmRleDsKPiAgCQl1OAltYnVzX2F0dHI7Cj4gLQkJdTMyCWJhc2U7Cj4gLQkJdTMyCXNpemU7Cj4g
KwkJdTY0CWJhc2U7Cj4gKwkJdTY0CXNpemU7Cj4gIAl9IGNzWzRdOwo+ICB9Owo+ICAKPiAtLSAK
PiAyLjExLjAKPgoKLS0gCkdyZWdvcnkgQ2xlbWVudCwgRnJlZSBFbGVjdHJvbnMKS2VybmVsLCBk
cml2ZXJzLCByZWFsLXRpbWUgYW5kIGVtYmVkZGVkIExpbnV4CmRldmVsb3BtZW50LCBjb25zdWx0
aW5nLCB0cmFpbmluZyBhbmQgc3VwcG9ydC4KaHR0cDovL2ZyZWUtZWxlY3Ryb25zLmNvbQoKX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJtLWtl
cm5lbCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3JnCmh0
dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5l
bAo=

WARNING: multiple messages have this Message-ID (diff)
From: gregory.clement@free-electrons.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] bus: mbus: fix window size calculation for 4GB windows
Date: Tue, 19 Sep 2017 16:43:18 +0200	[thread overview]
Message-ID: <87wp4urbp5.fsf@free-electrons.com> (raw)
In-Reply-To: <20170828152517.24506-2-jlu@pengutronix.de> (Jan Luebbe's message of "Mon, 28 Aug 2017 17:25:16 +0200")

Hi Jan,
 
 On lun., ao?t 28 2017, Jan Luebbe <jlu@pengutronix.de> wrote:

> At least the Armada XP SoC supports 4GB on a single DRAM window. Because
> the size register values contain the actual size - 1, the MSB is set in
> that case. For example, the SDRAM window's control register's value is
> 0xffffffe1 for 4GB (bits 31 to 24 contain the size).
>
> The MBUS driver reads back each window's size from registers and
> calculates the actual size as (control_reg | ~DDR_SIZE_MASK) + 1, which
> overflows for 32 bit values, resulting in other miscalculations further
> on (a bad RAM window for the CESA crypto engine calculated by
> mvebu_mbus_setup_cpu_target_nooverlap() in my case).
>
> This patch changes the type in 'struct mbus_dram_window' from u32 to
> u64, which allows us to keep using the same register calculation code in
> most MBUS-using drivers (which calculate ->size - 1 again).
>

Your patch looks good, but as it is a fix we should also apply it on
stable, could you provide the commit to fix?

Thanks,

Gregory

> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>  drivers/bus/mvebu-mbus.c | 2 +-
>  include/linux/mbus.h     | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index c7f396903184..70db4d5638a6 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -720,7 +720,7 @@ mvebu_mbus_default_setup_cpu_target(struct mvebu_mbus_state *mbus)
>  			if (mbus->hw_io_coherency)
>  				w->mbus_attr |= ATTR_HW_COHERENCY;
>  			w->base = base & DDR_BASE_CS_LOW_MASK;
> -			w->size = (size | ~DDR_SIZE_MASK) + 1;
> +			w->size = (u64)(size | ~DDR_SIZE_MASK) + 1;
>  		}
>  	}
>  	mvebu_mbus_dram_info.num_cs = cs;
> diff --git a/include/linux/mbus.h b/include/linux/mbus.h
> index 0d3f14fd2621..4773145246ed 100644
> --- a/include/linux/mbus.h
> +++ b/include/linux/mbus.h
> @@ -31,8 +31,8 @@ struct mbus_dram_target_info
>  	struct mbus_dram_window {
>  		u8	cs_index;
>  		u8	mbus_attr;
> -		u32	base;
> -		u32	size;
> +		u64	base;
> +		u64	size;
>  	} cs[4];
>  };
>  
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

WARNING: multiple messages have this Message-ID (diff)
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
To: Jan Luebbe <jlu@pengutronix.de>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Jason Cooper <jason@lakedaemon.net>,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH 1/2] bus: mbus: fix window size calculation for 4GB windows
Date: Tue, 19 Sep 2017 16:43:18 +0200	[thread overview]
Message-ID: <87wp4urbp5.fsf@free-electrons.com> (raw)
In-Reply-To: <20170828152517.24506-2-jlu@pengutronix.de> (Jan Luebbe's message of "Mon, 28 Aug 2017 17:25:16 +0200")

Hi Jan,
 
 On lun., août 28 2017, Jan Luebbe <jlu@pengutronix.de> wrote:

> At least the Armada XP SoC supports 4GB on a single DRAM window. Because
> the size register values contain the actual size - 1, the MSB is set in
> that case. For example, the SDRAM window's control register's value is
> 0xffffffe1 for 4GB (bits 31 to 24 contain the size).
>
> The MBUS driver reads back each window's size from registers and
> calculates the actual size as (control_reg | ~DDR_SIZE_MASK) + 1, which
> overflows for 32 bit values, resulting in other miscalculations further
> on (a bad RAM window for the CESA crypto engine calculated by
> mvebu_mbus_setup_cpu_target_nooverlap() in my case).
>
> This patch changes the type in 'struct mbus_dram_window' from u32 to
> u64, which allows us to keep using the same register calculation code in
> most MBUS-using drivers (which calculate ->size - 1 again).
>

Your patch looks good, but as it is a fix we should also apply it on
stable, could you provide the commit to fix?

Thanks,

Gregory

> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> ---
>  drivers/bus/mvebu-mbus.c | 2 +-
>  include/linux/mbus.h     | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
> index c7f396903184..70db4d5638a6 100644
> --- a/drivers/bus/mvebu-mbus.c
> +++ b/drivers/bus/mvebu-mbus.c
> @@ -720,7 +720,7 @@ mvebu_mbus_default_setup_cpu_target(struct mvebu_mbus_state *mbus)
>  			if (mbus->hw_io_coherency)
>  				w->mbus_attr |= ATTR_HW_COHERENCY;
>  			w->base = base & DDR_BASE_CS_LOW_MASK;
> -			w->size = (size | ~DDR_SIZE_MASK) + 1;
> +			w->size = (u64)(size | ~DDR_SIZE_MASK) + 1;
>  		}
>  	}
>  	mvebu_mbus_dram_info.num_cs = cs;
> diff --git a/include/linux/mbus.h b/include/linux/mbus.h
> index 0d3f14fd2621..4773145246ed 100644
> --- a/include/linux/mbus.h
> +++ b/include/linux/mbus.h
> @@ -31,8 +31,8 @@ struct mbus_dram_target_info
>  	struct mbus_dram_window {
>  		u8	cs_index;
>  		u8	mbus_attr;
> -		u32	base;
> -		u32	size;
> +		u64	base;
> +		u64	size;
>  	} cs[4];
>  };
>  
> -- 
> 2.11.0
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

  reply	other threads:[~2017-09-19 14:43 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-28 15:25 [PATCH 0/2] fix 4GB DRAM window support on mvebu Jan Luebbe
2017-08-28 15:25 ` Jan Luebbe
2017-08-28 15:25 ` [PATCH 1/2] bus: mbus: fix window size calculation for 4GB windows Jan Luebbe
2017-08-28 15:25   ` Jan Luebbe
2017-08-28 15:25   ` Jan Luebbe
2017-09-19 14:43   ` Gregory CLEMENT [this message]
2017-09-19 14:43     ` Gregory CLEMENT
2017-09-19 14:43     ` Gregory CLEMENT
2017-09-20 16:37     ` Uwe Kleine-König
2017-09-20 16:37       ` Uwe Kleine-König
2017-09-20 16:37       ` Uwe Kleine-König
2017-08-28 15:25 ` [PATCH 2/2] PCI: mvebu: Check DRAM window size Jan Luebbe
2017-08-28 15:25   ` Jan Luebbe
2017-09-25 23:56   ` Bjorn Helgaas
2017-09-25 23:56     ` Bjorn Helgaas
2017-09-25 23:56     ` Bjorn Helgaas
2017-10-05 21:16     ` Bjorn Helgaas
2017-10-05 21:16       ` Bjorn Helgaas
2017-11-06 19:17       ` Bjorn Helgaas
2017-11-06 19:17         ` Bjorn Helgaas
2017-11-06 19:17         ` Bjorn Helgaas
2017-08-28 15:30 ` [RFC] ARM: Orion: " Jan Luebbe
2017-08-28 15:30   ` Jan Luebbe
2017-08-28 15:51   ` Andrew Lunn
2017-08-28 15:51     ` Andrew Lunn
2017-08-28 15:51     ` Andrew Lunn
2017-08-30 20:07 ` [PATCH 0/2] fix 4GB DRAM window support on mvebu Bjorn Helgaas
2017-08-30 20:07   ` Bjorn Helgaas
2017-08-30 20:07   ` Bjorn Helgaas
2017-08-31  8:11   ` Gregory CLEMENT
2017-08-31  8:11     ` Gregory CLEMENT
2017-08-31  8:11     ` Gregory CLEMENT

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wp4urbp5.fsf@free-electrons.com \
    --to=gregory.clement@free-electrons.com \
    --cc=andrew@lunn.ch \
    --cc=jason@lakedaemon.net \
    --cc=jlu@pengutronix.de \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=thomas.petazzoni@free-electrons.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.