On 11/02/16 13:34, Christoffer Dall wrote: > On Thu, Feb 11, 2016 at 02:27:08PM +0100, Christoffer Dall wrote: >> On Wed, Feb 10, 2016 at 10:46:53AM -0800, tchalamarla@xxxxxxxxxxxxxxxxxx wrote: >>> From: Tirumalesh Chalamarla <tchalamarla@xxxxxxxxxxxxxxxxxx> >>> >>> 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 <tchalamarla@xxxxxxxxxxxxxxxxxx> >>> --- >>> arch/arm64/include/asm/kvm_arm.h | 2 -- >>> arch/arm64/kvm/hyp-init.S | 12 +++++++----- >>> 2 files changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >>> index 738a95f..3776db0 100644 >>> --- a/arch/arm64/include/asm/kvm_arm.h >>> +++ b/arch/arm64/include/asm/kvm_arm.h >>> @@ -107,8 +107,6 @@ >>> #define TCR_EL2_MASK (TCR_EL2_TG0 | TCR_EL2_SH0 | \ >>> TCR_EL2_ORGN0 | TCR_EL2_IRGN0 | TCR_EL2_T0SZ) >>> >>> -#define TCR_EL2_FLAGS (TCR_EL2_RES1 | TCR_EL2_PS_40B) >>> - >>> /* VTCR_EL2 Registers bits */ >>> #define VTCR_EL2_RES1 (1 << 31) >>> #define VTCR_EL2_PS_MASK (7 << 16) >>> diff --git a/arch/arm64/kvm/hyp-init.S b/arch/arm64/kvm/hyp-init.S >>> index 3e568dc..b5ab1b0 100644 >>> --- a/arch/arm64/kvm/hyp-init.S >>> +++ b/arch/arm64/kvm/hyp-init.S >>> @@ -64,7 +64,7 @@ __do_hyp_init: >>> mrs x4, tcr_el1 >>> ldr x5, =TCR_EL2_MASK >>> and x4, x4, x5 >>> - ldr x5, =TCR_EL2_FLAGS >>> + ldr x5, =TCR_EL2_RES1 >>> orr x4, x4, x5 >>> >>> #ifndef CONFIG_ARM64_VA_BITS_48 >>> @@ -85,15 +85,17 @@ __do_hyp_init: >>> ldr_l x5, idmap_t0sz >>> bfi x4, x5, TCR_T0SZ_OFFSET, TCR_TxSZ_WIDTH >>> #endif >>> - msr tcr_el2, x4 >>> - >>> - ldr x4, =VTCR_EL2_FLAGS >>> /* >>> * Read the PARange bits from ID_AA64MMFR0_EL1 and set the PS bits in >>> - * VTCR_EL2. >>> + * TCR_EL2 and VTCR_EL2. >>> */ >>> mrs x5, ID_AA64MMFR0_EL1 >>> bfi x4, x5, #16, #3 >>> + >>> + msr tcr_el2, x4 >>> + >>> + ldr x4, =VTCR_EL2_FLAGS >>> + bfi x4, x5, #16, #3 >>> /* >>> * Read the VMIDBits bits from ID_AA64MMFR1_EL1 and set the VS bit in >>> * VTCR_EL2. >>> -- >>> 2.1.0 >>> >> >> 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... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
