Hi Marc, On Mon, Jan 20, 2014 at 7:11 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > Hi Anup, > > On 20/01/14 12:00, Anup Patel wrote: >> On Fri, Jan 17, 2014 at 8:33 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >>> In order to be able to detect the point where the guest enables >>> its MMU and caches, trap all the VM related system registers. >>> >>> Once we see the guest enabling both the MMU and the caches, we >>> can go back to a saner mode of operation, which is to leave these >>> registers in complete control of the guest. >>> >>> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >>> Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx> >>> --- >>> arch/arm64/include/asm/kvm_arm.h | 3 ++- >>> arch/arm64/kvm/sys_regs.c | 58 ++++++++++++++++++++++++++++++++-------- >>> 2 files changed, 49 insertions(+), 12 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h >>> index c98ef47..fd0a651 100644 >>> --- a/arch/arm64/include/asm/kvm_arm.h >>> +++ b/arch/arm64/include/asm/kvm_arm.h >>> @@ -62,6 +62,7 @@ >>> * RW: 64bit by default, can be overriden for 32bit VMs >>> * TAC: Trap ACTLR >>> * TSC: Trap SMC >>> + * TVM: Trap VM ops (until M+C set in SCTLR_EL1) >>> * TSW: Trap cache operations by set/way >>> * TWE: Trap WFE >>> * TWI: Trap WFI >>> @@ -74,7 +75,7 @@ >>> * SWIO: Turn set/way invalidates into set/way clean+invalidate >>> */ >>> #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \ >>> - HCR_BSU_IS | HCR_FB | HCR_TAC | \ >>> + HCR_TVM | HCR_BSU_IS | HCR_FB | HCR_TAC | \ >>> HCR_AMO | HCR_IMO | HCR_FMO | \ >>> HCR_SWIO | HCR_TIDCP | HCR_RW) >>> #define HCR_VIRT_EXCP_MASK (HCR_VA | HCR_VI | HCR_VF) >>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >>> index 02e9d09..5e92b9e 100644 >>> --- a/arch/arm64/kvm/sys_regs.c >>> +++ b/arch/arm64/kvm/sys_regs.c >>> @@ -121,6 +121,42 @@ done: >>> } >>> >>> /* >>> + * Generic accessor for VM registers. Only called as long as HCR_TVM >>> + * is set. >>> + */ >>> +static bool access_vm_reg(struct kvm_vcpu *vcpu, >>> + const struct sys_reg_params *p, >>> + const struct sys_reg_desc *r) >>> +{ >>> + BUG_ON(!p->is_write); >>> + >>> + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt); >>> + return true; >>> +} >>> + >>> +/* >>> + * SCTLR_EL1 accessor. Only called as long as HCR_TVM is set. If the >>> + * guest enables the MMU, we stop trapping the VM sys_regs and leave >>> + * it in complete control of the caches. >>> + */ >>> +static bool access_sctlr_el1(struct kvm_vcpu *vcpu, >>> + const struct sys_reg_params *p, >>> + const struct sys_reg_desc *r) >>> +{ >>> + unsigned long val; >>> + >>> + BUG_ON(!p->is_write); >>> + >>> + val = *vcpu_reg(vcpu, p->Rt); >>> + vcpu_sys_reg(vcpu, r->reg) = val; >>> + >>> + if ((val & (0b101)) == 0b101) /* MMU+Caches enabled? */ >>> + vcpu->arch.hcr_el2 &= ~HCR_TVM; >>> + >>> + return true; >>> +} >>> + >>> +/* >>> * We could trap ID_DFR0 and tell the guest we don't support performance >>> * monitoring. Unfortunately the patch to make the kernel check ID_DFR0 was >>> * NAKed, so it will read the PMCR anyway. >>> @@ -185,32 +221,32 @@ static const struct sys_reg_desc sys_reg_descs[] = { >>> NULL, reset_mpidr, MPIDR_EL1 }, >>> /* SCTLR_EL1 */ >>> { Op0(0b11), Op1(0b000), CRn(0b0001), CRm(0b0000), Op2(0b000), >>> - NULL, reset_val, SCTLR_EL1, 0x00C50078 }, >>> + access_sctlr_el1, reset_val, SCTLR_EL1, 0x00C50078 }, >> >> This patch in its current form breaks Aarch32 VMs on Foundation v8 Model >> because encoding for Aarch64 VM registers we get Op0=0b11 and for Aarch32 >> VM registers we get Op0=0b00 when trapped. >> >> Either its a Foundation v8 Model bug or we need to add more enteries in >> sys_reg_desc[] for Aarch32 VM registers with Op0=0b00. > > That's a good point. But Op0 isn't defined for AArch32, the value is > simply hardcoded in kvm_handle_cp15_32/kvm_handle_cp15_64, which is > obviously horribly broken. > > I'll work on a fix for that, thanks noticing it. > > Does this series otherwise fix your L3 cache issue (assuming you stick > to 64bit guests)? Just started trying your patches today. First tried on Foundation v8 Model. Next we will try on X-Gene. Me or Pranav will soon provide more feedback in this regard. > > Cheers, > > M. > -- > Jazz is not dead. It just smells funny... Thanks, Anup -- 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