All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mian Yousaf Kaukab <ykaukab@suse.de>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, treding@nvidia.com,
	jonathanh@nvidia.com, linux-tegra@vger.kernel.org
Subject: Re: [PATCH] arm64: tegra: only map accessible sysram
Date: Mon, 30 Sep 2019 12:02:12 +0200	[thread overview]
Message-ID: <20190930100212.GA21324@suse.de> (raw)
In-Reply-To: <5d2e47ec-8304-d648-9c4a-80c7c02050a9@wwwdotorg.org>

On Sun, Sep 29, 2019 at 11:28:43PM -0600, Stephen Warren wrote:
> On 9/29/19 2:08 PM, Mian Yousaf Kaukab wrote:
> > Most of the SysRAM is secure and only accessible by TF-A.
> > Don't map this inaccessible memory in kernel. Only map pages
> > used by bpmp driver.
> 
> I don't believe this change is correct. The actual patch doesn't
> implement mapping a subset of the RAM (a software issue), but rather it
> changes the DT representation of the SYSRAM hardware. The SYSRAM
> hardware always does start at 0x30000000, even if a subset of the
> address range is dedicated to a specific purpose. If the kernel must map
> only part of the RAM, then some additional property should indicate
> this.[...]
I agree the hardware description becomes inaccurate with this change.

In the current setup complete 0x3000_0000 to 0x3005_0000 range is being mapped
as normal memory (MT_NORMAL_NC). Though only 0x3004_E000 to 0x3005_0000 are
accessible by the kernel. I am seeing an issue where a read access (which I
believe is speculative) to inaccessible range causes an SError. Another 
solution for this problem could be to add "no-memory-wc" to SysRAM node so that
it is mapped as device memory (MT_DEVICE_nGnRE). Would that be acceptable?

> [...] Also, I believe it's incorrect to hard-code into the kernel's DT
> the range of addresses used by the secure monitor/OS, since this can
> vary depending on what the user actually chooses to install as the
> secure monitor/OS. Any indication of such regions should be filled in at
> runtime by some boot firmware or the secure monitor/OS itself, or
> retrieved using some runtime API rather than DT.
Secure-OS addresses are not of interest here. SysRAM is partitioned
between secure-OS and BPMP and kernel is only interested in the BPMP
part. The firmware can update these addresses in the device-tree if it
wants to. Would you prefer something similar implemented in u-boot so
that it updates SysRAM node to only expose kernel accessible part of it
to the kernel?

Can u-boot dynamically figure out the Secure-OS vs BPMP partition?

BR,
Yousaf

WARNING: multiple messages have this Message-ID (diff)
From: Mian Yousaf Kaukab <ykaukab@suse.de>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: linux-tegra@vger.kernel.org, treding@nvidia.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, jonathanh@nvidia.com
Subject: Re: [PATCH] arm64: tegra: only map accessible sysram
Date: Mon, 30 Sep 2019 12:02:12 +0200	[thread overview]
Message-ID: <20190930100212.GA21324@suse.de> (raw)
In-Reply-To: <5d2e47ec-8304-d648-9c4a-80c7c02050a9@wwwdotorg.org>

On Sun, Sep 29, 2019 at 11:28:43PM -0600, Stephen Warren wrote:
> On 9/29/19 2:08 PM, Mian Yousaf Kaukab wrote:
> > Most of the SysRAM is secure and only accessible by TF-A.
> > Don't map this inaccessible memory in kernel. Only map pages
> > used by bpmp driver.
> 
> I don't believe this change is correct. The actual patch doesn't
> implement mapping a subset of the RAM (a software issue), but rather it
> changes the DT representation of the SYSRAM hardware. The SYSRAM
> hardware always does start at 0x30000000, even if a subset of the
> address range is dedicated to a specific purpose. If the kernel must map
> only part of the RAM, then some additional property should indicate
> this.[...]
I agree the hardware description becomes inaccurate with this change.

In the current setup complete 0x3000_0000 to 0x3005_0000 range is being mapped
as normal memory (MT_NORMAL_NC). Though only 0x3004_E000 to 0x3005_0000 are
accessible by the kernel. I am seeing an issue where a read access (which I
believe is speculative) to inaccessible range causes an SError. Another 
solution for this problem could be to add "no-memory-wc" to SysRAM node so that
it is mapped as device memory (MT_DEVICE_nGnRE). Would that be acceptable?

> [...] Also, I believe it's incorrect to hard-code into the kernel's DT
> the range of addresses used by the secure monitor/OS, since this can
> vary depending on what the user actually chooses to install as the
> secure monitor/OS. Any indication of such regions should be filled in at
> runtime by some boot firmware or the secure monitor/OS itself, or
> retrieved using some runtime API rather than DT.
Secure-OS addresses are not of interest here. SysRAM is partitioned
between secure-OS and BPMP and kernel is only interested in the BPMP
part. The firmware can update these addresses in the device-tree if it
wants to. Would you prefer something similar implemented in u-boot so
that it updates SysRAM node to only expose kernel accessible part of it
to the kernel?

Can u-boot dynamically figure out the Secure-OS vs BPMP partition?

BR,
Yousaf

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-09-30 10:02 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-29 20:08 [PATCH] arm64: tegra: only map accessible sysram Mian Yousaf Kaukab
2019-09-29 20:08 ` Mian Yousaf Kaukab
2019-09-30  5:28 ` Stephen Warren
2019-09-30  5:28   ` Stephen Warren
2019-09-30  5:28   ` Stephen Warren
2019-09-30 10:02   ` Mian Yousaf Kaukab [this message]
2019-09-30 10:02     ` Mian Yousaf Kaukab
2019-10-02 20:30     ` Stephen Warren
2019-10-02 20:30       ` Stephen Warren

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=20190930100212.GA21324@suse.de \
    --to=ykaukab@suse.de \
    --cc=jonathanh@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=treding@nvidia.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.