From: tchalamarla at caviumnetworks.com <hidden> Date: 2016-02-10 18:46:53
From: Tirumalesh Chalamarla <redacted>
Setting TCR_EL2.PS to 40 bits is wrong on systems with PS size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.
This patch sets TCR_EL2.PS at runtime similar to VTCR_EL2.PS
changes form V2:
- Modified subject line and commit message.
- Removed TCR_EL2_FLAGS.
changes from V1:
- Moved to dynamic configuration.
Signed-off-by: Tirumalesh Chalamarla <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 2 --
arch/arm64/kvm/hyp-init.S | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
From: Christoffer Dall <hidden> Date: 2016-02-11 13:27:08
On Wed, Feb 10, 2016 at 10:46:53AM -0800, tchalamarla at caviumnetworks.com wrote:
quoted hunk
From: Tirumalesh Chalamarla <redacted>
Setting TCR_EL2.PS to 40 bits is wrong on systems with PS size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.
This patch sets TCR_EL2.PS at runtime similar to VTCR_EL2.PS
changes form V2:
- Modified subject line and commit message.
- Removed TCR_EL2_FLAGS.
changes from V1:
- Moved to dynamic configuration.
Signed-off-by: Tirumalesh Chalamarla <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 2 --
arch/arm64/kvm/hyp-init.S | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
This looks reasonable enough for me. Did we limit Linux to PS=40 bits
in the past and have lifted this restriction, or why did we have this
limitation for EL2 translations in the past?
Thanks,
-Christoffer
From: Christoffer Dall <hidden> Date: 2016-02-11 13:34:01
On Thu, Feb 11, 2016 at 02:27:08PM +0100, Christoffer Dall wrote:
On Wed, Feb 10, 2016 at 10:46:53AM -0800, tchalamarla at caviumnetworks.com wrote:
quoted
From: Tirumalesh Chalamarla <redacted>
Setting TCR_EL2.PS to 40 bits is wrong on systems with PS size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.
This patch sets TCR_EL2.PS at runtime similar to VTCR_EL2.PS
changes form V2:
- Modified subject line and commit message.
- Removed TCR_EL2_FLAGS.
changes from V1:
- Moved to dynamic configuration.
Signed-off-by: Tirumalesh Chalamarla <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 2 --
arch/arm64/kvm/hyp-init.S | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
This looks reasonable enough for me. Did we limit Linux to PS=40 bits
in the past and have lifted this restriction, or why did we have this
limitation for EL2 translations in the past?
Hmm, just went back and looked at my comments [1] on Suzuki's 16K page
series, and regarding our stage-2 page table configuration I wrote this:
<quote>
... the constraints of
this whole thing is, and make sure we agree on this:
1. We fix the IPA max width to 40 bits
2. We don't support systems with a PARange smaller than 40 bits (do we
check this anywhere or document this anywhere?)
3. We always assume we are running on a system with PARange of 40
bits
and we are therefore constrained to use concatination.
</quote>
So this change is actually making a bit nervous... Marc, did you check
if all the stuff in arch/arm64/include/asm/kvm_mmu.h still holds after
we relax this?
Thanks,
-Christoffer
[1]: https://lkml.org/lkml/2015/10/10/96
From: Marc Zyngier <hidden> Date: 2016-02-11 13:55:13
On 11/02/16 13:34, Christoffer Dall wrote:
On Thu, Feb 11, 2016 at 02:27:08PM +0100, Christoffer Dall wrote:
quoted
On Wed, Feb 10, 2016 at 10:46:53AM -0800, tchalamarla at caviumnetworks.com wrote:
quoted
From: Tirumalesh Chalamarla <redacted>
Setting TCR_EL2.PS to 40 bits is wrong on systems with PS size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.
This patch sets TCR_EL2.PS at runtime similar to VTCR_EL2.PS
changes form V2:
- Modified subject line and commit message.
- Removed TCR_EL2_FLAGS.
changes from V1:
- Moved to dynamic configuration.
Signed-off-by: Tirumalesh Chalamarla <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 2 --
arch/arm64/kvm/hyp-init.S | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
This looks reasonable enough for me. Did we limit Linux to PS=40 bits
in the past and have lifted this restriction, or why did we have this
limitation for EL2 translations in the past?
Hmm, just went back and looked at my comments [1] on Suzuki's 16K page
series, and regarding our stage-2 page table configuration I wrote this:
<quote>
... the constraints of
this whole thing is, and make sure we agree on this:
1. We fix the IPA max width to 40 bits
2. We don't support systems with a PARange smaller than 40 bits (do we
check this anywhere or document this anywhere?)
3. We always assume we are running on a system with PARange of 40
bits
and we are therefore constrained to use concatination.
</quote>
So this change is actually making a bit nervous... Marc, did you check
if all the stuff in arch/arm64/include/asm/kvm_mmu.h still holds after
we relax this?
I believe this still works for the following reasons:
- This only affects the output address of EL2 Stage-1, and basically
only allows EL2 to reach the same memory as the rest of the host kernel.
- Stage-2 translation already does the same thing, for similar reasons.
- We enforce 40bit IPA by not providing page tables above 40 bits (by
setting T0SZ to 64-40=24). This has the effect of denying accesses above
40bit IPA to the guest.
So I'm quite confident that we're safe here.
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
From: Christoffer Dall <hidden> Date: 2016-02-11 14:41:21
On Thu, Feb 11, 2016 at 01:55:13PM +0000, Marc Zyngier wrote:
On 11/02/16 13:34, Christoffer Dall wrote:
quoted
On Thu, Feb 11, 2016 at 02:27:08PM +0100, Christoffer Dall wrote:
quoted
On Wed, Feb 10, 2016 at 10:46:53AM -0800, tchalamarla at caviumnetworks.com wrote:
quoted
From: Tirumalesh Chalamarla <redacted>
Setting TCR_EL2.PS to 40 bits is wrong on systems with PS size is
less than 40 bits. and with systems where RAM is at higher address,
this will break KVM.
This patch sets TCR_EL2.PS at runtime similar to VTCR_EL2.PS
changes form V2:
- Modified subject line and commit message.
- Removed TCR_EL2_FLAGS.
changes from V1:
- Moved to dynamic configuration.
Signed-off-by: Tirumalesh Chalamarla <redacted>
---
arch/arm64/include/asm/kvm_arm.h | 2 --
arch/arm64/kvm/hyp-init.S | 12 +++++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
This looks reasonable enough for me. Did we limit Linux to PS=40 bits
in the past and have lifted this restriction, or why did we have this
limitation for EL2 translations in the past?
Hmm, just went back and looked at my comments [1] on Suzuki's 16K page
series, and regarding our stage-2 page table configuration I wrote this:
<quote>
... the constraints of
this whole thing is, and make sure we agree on this:
1. We fix the IPA max width to 40 bits
2. We don't support systems with a PARange smaller than 40 bits (do we
check this anywhere or document this anywhere?)
3. We always assume we are running on a system with PARange of 40
bits
and we are therefore constrained to use concatination.
</quote>
So this change is actually making a bit nervous... Marc, did you check
if all the stuff in arch/arm64/include/asm/kvm_mmu.h still holds after
we relax this?
I believe this still works for the following reasons:
- This only affects the output address of EL2 Stage-1, and basically
only allows EL2 to reach the same memory as the rest of the host kernel.
- Stage-2 translation already does the same thing, for similar reasons.
- We enforce 40bit IPA by not providing page tables above 40 bits (by
setting T0SZ to 64-40=24). This has the effect of denying accesses above
40bit IPA to the guest.
So I'm quite confident that we're safe here.
Thoughts?
This patch is indeed harmless as it only touches the EL2 translation
regime to be in line with the rest of the kernel, so we can merge this
patch.
I'm more concerned about the assumptions we had for the Stage-2 page
table not being valid anymore and therefore deserving a second look.
But we can have a review of that whole thing some other day.
Thanks,
-Christoffer