[PATCH 1/2] firmware: tegra: bpmp: do only aligned access to IPC memory area

[PATCH 1/2] firmware: tegra: bpmp: do only aligned access to IPC memory area

[Mikko Perttunen]

From: Timo Alho <talho@nvidia.com>

Use memcpy_toio and memcpy_fromio variants of memcpy to guarantee
no unaligned access to IPC memory area. This is to allow the IPC
memory to be mapped as Device memory to further suppress speculative
reads from happening within the 64kB memory area above the IPC memory
when 64kB memory pages are used.

Signed-off-by: Timo Alho <talho@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/firmware/tegra/bpmp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index 5654c5e9862b..037db21de510 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -201,7 +201,7 @@ static ssize_t __tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
 	int err;
 
 	if (data && size > 0)
-		memcpy(data, channel->ib->data, size);
+		memcpy_fromio(data, channel->ib->data, size);
 
 	err = tegra_bpmp_ack_response(channel);
 	if (err < 0)
@@ -245,7 +245,7 @@ static ssize_t __tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,
 	channel->ob->flags = flags;
 
 	if (data && size > 0)
-		memcpy(channel->ob->data, data, size);
+		memcpy_toio(channel->ob->data, data, size);
 
 	return tegra_bpmp_post_request(channel);
 }
@@ -420,7 +420,7 @@ void tegra_bpmp_mrq_return(struct tegra_bpmp_channel *channel, int code,
 	channel->ob->code = code;
 
 	if (data && size > 0)
-		memcpy(channel->ob->data, data, size);
+		memcpy_toio(channel->ob->data, data, size);
 
 	err = tegra_bpmp_post_response(channel);
 	if (WARN_ON(err < 0))
-- 
2.36.1

[PATCH 2/2] arm64: tegra: Mark BPMP channels as no-memory-wc

[Mikko Perttunen]

From: Mikko Perttunen <mperttunen@nvidia.com>

The Tegra SYSRAM contains regions access to which is restricted to
certain hardware blocks on the system, and speculative accesses to
those will cause issues.

Patch 'misc: sram: Only map reserved areas in Tegra SYSRAM' attempted
to resolve this by only mapping the regions specified in the device
tree on the assumption that there are no such restricted areas within
the 64K-aligned area of memory that contains the memory we wish to map.

Turns out this assumption is wrong, as there are such areas above the
4K pages described in the device trees. As such, we need to use the
bigger hammer that is no-memory-wc, which causes the memory to be
mapped as Device memory to which speculative accesses are disallowed.

As such, the previous patch in the series,
  'firmware: tegra: bpmp: do only aligned access to IPC memory area',
is required with this patch to make the BPMP driver only issue aligned
memory accesses as those are also required with Device memory.

Fixes: fec29bf04994 ("misc: sram: Only map reserved areas in Tegra SYSRAM")
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 1 +
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 0e9afc3e2f26..9eca18b54698 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1820,6 +1820,7 @@ sram@30000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x30000000 0x50000>;
+		no-memory-wc;
 
 		cpu_bpmp_tx: sram@4e000 {
 			reg = <0x4e000 0x1000>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index d1f8248c00f4..3fdb0b852718 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -2684,6 +2684,7 @@ sram@40000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x40000000 0x50000>;
+		no-memory-wc;
 
 		cpu_bpmp_tx: sram@4e000 {
 			reg = <0x4e000 0x1000>;
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index cb3af539e477..0213a7e3dad0 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -1325,6 +1325,7 @@ sram@40000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0x40000000 0x80000>;
+		no-memory-wc;
 
 		cpu_bpmp_tx: sram@70000 {
 			reg = <0x70000 0x1000>;
-- 
2.36.1

Re: [PATCH 2/2] arm64: tegra: Mark BPMP channels as no-memory-wc

[Mikko Perttunen]

On 22.6.2022 16.23, Mikko Perttunen wrote:
> From: Mikko Perttunen <mperttunen@nvidia.com>
> 
> The Tegra SYSRAM contains regions access to which is restricted to
> certain hardware blocks on the system, and speculative accesses to
> those will cause issues.
> 
> Patch 'misc: sram: Only map reserved areas in Tegra SYSRAM' attempted
> to resolve this by only mapping the regions specified in the device
> tree on the assumption that there are no such restricted areas within
> the 64K-aligned area of memory that contains the memory we wish to map.
> 
> Turns out this assumption is wrong, as there are such areas above the
> 4K pages described in the device trees. As such, we need to use the
> bigger hammer that is no-memory-wc, which causes the memory to be
> mapped as Device memory to which speculative accesses are disallowed.
> 
> As such, the previous patch in the series,
>    'firmware: tegra: bpmp: do only aligned access to IPC memory area',
> is required with this patch to make the BPMP driver only issue aligned
> memory accesses as those are also required with Device memory.
> 
> Fixes: fec29bf04994 ("misc: sram: Only map reserved areas in Tegra SYSRAM")
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---

FWIW, with this, the aforementioned patch to misc/sram is redundant. It 
doesn't hurt, but doesn't really help either. Whether or not it should 
be reverted, I have no opinion.

Thanks,
Mikko

>   arch/arm64/boot/dts/nvidia/tegra186.dtsi | 1 +
>   arch/arm64/boot/dts/nvidia/tegra194.dtsi | 1 +
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 1 +
>   3 files changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> index 0e9afc3e2f26..9eca18b54698 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> @@ -1820,6 +1820,7 @@ sram@30000000 {
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   		ranges = <0x0 0x0 0x30000000 0x50000>;
> +		no-memory-wc;
>   
>   		cpu_bpmp_tx: sram@4e000 {
>   			reg = <0x4e000 0x1000>;
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index d1f8248c00f4..3fdb0b852718 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -2684,6 +2684,7 @@ sram@40000000 {
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   		ranges = <0x0 0x0 0x40000000 0x50000>;
> +		no-memory-wc;
>   
>   		cpu_bpmp_tx: sram@4e000 {
>   			reg = <0x4e000 0x1000>;
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index cb3af539e477..0213a7e3dad0 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -1325,6 +1325,7 @@ sram@40000000 {
>   		#address-cells = <1>;
>   		#size-cells = <1>;
>   		ranges = <0x0 0x0 0x40000000 0x80000>;
> +		no-memory-wc;
>   
>   		cpu_bpmp_tx: sram@70000 {
>   			reg = <0x70000 0x1000>;

Re: [PATCH 2/2] arm64: tegra: Mark BPMP channels as no-memory-wc

[Yousaf Kaukab]

On Wed, Jun 22, 2022 at 04:29:03PM +0300, Mikko Perttunen wrote:
> On 22.6.2022 16.23, Mikko Perttunen wrote:
> > From: Mikko Perttunen <mperttunen@nvidia.com>
> > 
> > The Tegra SYSRAM contains regions access to which is restricted to
> > certain hardware blocks on the system, and speculative accesses to
> > those will cause issues.
> > 
> > Patch 'misc: sram: Only map reserved areas in Tegra SYSRAM' attempted
> > to resolve this by only mapping the regions specified in the device
> > tree on the assumption that there are no such restricted areas within
> > the 64K-aligned area of memory that contains the memory we wish to map.
> > 
> > Turns out this assumption is wrong, as there are such areas above the
> > 4K pages described in the device trees. As such, we need to use the
> > bigger hammer that is no-memory-wc, which causes the memory to be
> > mapped as Device memory to which speculative accesses are disallowed.
> > 
> > As such, the previous patch in the series,
> >    'firmware: tegra: bpmp: do only aligned access to IPC memory area',
> > is required with this patch to make the BPMP driver only issue aligned
> > memory accesses as those are also required with Device memory.
> > 
> > Fixes: fec29bf04994 ("misc: sram: Only map reserved areas in Tegra SYSRAM")
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> 
> FWIW, with this, the aforementioned patch to misc/sram is redundant. It
> doesn't hurt, but doesn't really help either. Whether or not it should be
> reverted, I have no opinion.
I am in favor of reverting commit fec29bf04994 ("misc: sram: Only map
reserved areas in Tegra SYSRAM"). Tegra platforms are the only consumer
of this code. I consider it to be redundant after your series.
For both patches:
Reviewed-by: Yousaf Kaukab <ykaukab@suse.de>
> 
> Thanks,
> Mikko
BR,
Yousaf