diff mbox

[v2,09/10] ARM: KVM: trap VM system registers until MMU and caches are ON

Message ID 1390402602-22777-10-git-send-email-marc.zyngier@arm.com
State New
Headers show

Commit Message

Marc Zyngier Jan. 22, 2014, 2:56 p.m. UTC
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@arm.com>
---
 arch/arm/include/asm/kvm_arm.h |  3 +-
 arch/arm/kvm/coproc.c          | 85 ++++++++++++++++++++++++++++++++++--------
 arch/arm/kvm/coproc_a15.c      |  2 +-
 arch/arm/kvm/coproc_a7.c       |  2 +-
 4 files changed, 73 insertions(+), 19 deletions(-)

Comments

Christoffer Dall Jan. 29, 2014, 8:08 p.m. UTC | #1
On Wed, Jan 22, 2014 at 02:56:41PM +0000, Marc Zyngier 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@arm.com>
> ---
>  arch/arm/include/asm/kvm_arm.h |  3 +-
>  arch/arm/kvm/coproc.c          | 85 ++++++++++++++++++++++++++++++++++--------
>  arch/arm/kvm/coproc_a15.c      |  2 +-
>  arch/arm/kvm/coproc_a7.c       |  2 +-
>  4 files changed, 73 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index a843e74..816db0b 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -55,6 +55,7 @@
>   * The bits we set in HCR:
>   * TAC:		Trap ACTLR
>   * TSC:		Trap SMC
> + * TVM:		Trap VM ops (until MMU and caches are on)
>   * TSW:		Trap cache operations by set/way
>   * TWI:		Trap WFI
>   * TWE:		Trap WFE
> @@ -68,7 +69,7 @@
>   */
>  #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
>  			HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
> -			HCR_TWE | HCR_SWIO | HCR_TIDCP)
> +			HCR_TVM | HCR_TWE | HCR_SWIO | HCR_TIDCP)
>  
>  /* System Control Register (SCTLR) bits */
>  #define SCTLR_TE	(1 << 30)
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 126c90d..1839770 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -23,6 +23,7 @@
>  #include <asm/kvm_host.h>
>  #include <asm/kvm_emulate.h>
>  #include <asm/kvm_coproc.h>
> +#include <asm/kvm_mmu.h>
>  #include <asm/cacheflush.h>
>  #include <asm/cputype.h>
>  #include <trace/events/kvm.h>
> @@ -205,6 +206,55 @@ 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 coproc_params *p,
> +			  const struct coproc_reg *r)
> +{
> +	if (p->is_write) {
> +		vcpu->arch.cp15[r->reg] = *vcpu_reg(vcpu, p->Rt1);
> +		if (p->is_64bit)
> +			vcpu->arch.cp15[r->reg + 1] = *vcpu_reg(vcpu, p->Rt2);
> +	} else {

hmm, the TVM in the ARM ARM v7 says it only traps for write accesses...?

> +		*vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
> +		if (p->is_64bit)
> +			*vcpu_reg(vcpu, p->Rt2) = vcpu->arch.cp15[r->reg + 1];
> +	}
> +
> +	return true;
> +}
> +
> +/*
> + * SCTLR 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.
> + *
> + * Used by the cpu-specific code.
> + */
> +bool access_sctlr(struct kvm_vcpu *vcpu,
> +		  const struct coproc_params *p,
> +		  const struct coproc_reg *r)
> +{
> +	if (p->is_write) {
> +		unsigned long val;
> +
> +		val = *vcpu_reg(vcpu, p->Rt1);
> +		vcpu->arch.cp15[r->reg] = val;

again, would it make sense to just call access_vm_reg and do the check
for caches enabled afterwards?

> +
> +		if ((val & (0b101)) == 0b101) {	/* MMU+Caches enabled? */
> +			vcpu->arch.hcr &= ~HCR_TVM;
> +			stage2_flush_vm(vcpu->kvm);
> +		}

my favorite static inline, again, again, ...

> +	} else {
> +		*vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
> +	}
> +
> +	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.
> @@ -261,33 +311,36 @@ static const struct coproc_reg cp15_regs[] = {
>  	{ CRn( 1), CRm( 0), Op1( 0), Op2( 2), is32,
>  			NULL, reset_val, c1_CPACR, 0x00000000 },
>  
> -	/* TTBR0/TTBR1: swapped by interrupt.S. */
> -	{ CRm64( 2), Op1( 0), is64, NULL, reset_unknown64, c2_TTBR0 },
> -	{ CRm64( 2), Op1( 1), is64, NULL, reset_unknown64, c2_TTBR1 },
> -
> -	/* TTBCR: swapped by interrupt.S. */
> +	/* TTBR0/TTBR1/TTBCR: swapped by interrupt.S. */
> +	{ CRm64( 2), Op1( 0), is64, access_vm_reg, reset_unknown64, c2_TTBR0 },
> +	{ CRn(2), CRm( 0), Op1( 0), Op2( 0), is32,
> +			access_vm_reg, reset_unknown, c2_TTBR0 },
> +	{ CRn(2), CRm( 0), Op1( 0), Op2( 1), is32,
> +			access_vm_reg, reset_unknown, c2_TTBR1 },
>  	{ CRn( 2), CRm( 0), Op1( 0), Op2( 2), is32,
> -			NULL, reset_val, c2_TTBCR, 0x00000000 },
> +			access_vm_reg, reset_val, c2_TTBCR, 0x00000000 },
> +	{ CRm64( 2), Op1( 1), is64, access_vm_reg, reset_unknown64, c2_TTBR1 },
> +
>  
>  	/* DACR: swapped by interrupt.S. */
>  	{ CRn( 3), CRm( 0), Op1( 0), Op2( 0), is32,
> -			NULL, reset_unknown, c3_DACR },
> +			access_vm_reg, reset_unknown, c3_DACR },
>  
>  	/* DFSR/IFSR/ADFSR/AIFSR: swapped by interrupt.S. */
>  	{ CRn( 5), CRm( 0), Op1( 0), Op2( 0), is32,
> -			NULL, reset_unknown, c5_DFSR },
> +			access_vm_reg, reset_unknown, c5_DFSR },
>  	{ CRn( 5), CRm( 0), Op1( 0), Op2( 1), is32,
> -			NULL, reset_unknown, c5_IFSR },
> +			access_vm_reg, reset_unknown, c5_IFSR },
>  	{ CRn( 5), CRm( 1), Op1( 0), Op2( 0), is32,
> -			NULL, reset_unknown, c5_ADFSR },
> +			access_vm_reg, reset_unknown, c5_ADFSR },
>  	{ CRn( 5), CRm( 1), Op1( 0), Op2( 1), is32,
> -			NULL, reset_unknown, c5_AIFSR },
> +			access_vm_reg, reset_unknown, c5_AIFSR },
>  
>  	/* DFAR/IFAR: swapped by interrupt.S. */
>  	{ CRn( 6), CRm( 0), Op1( 0), Op2( 0), is32,
> -			NULL, reset_unknown, c6_DFAR },
> +			access_vm_reg, reset_unknown, c6_DFAR },
>  	{ CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
> -			NULL, reset_unknown, c6_IFAR },
> +			access_vm_reg, reset_unknown, c6_IFAR },
>  
>  	/* PAR swapped by interrupt.S */
>  	{ CRm64( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
> @@ -324,9 +377,9 @@ static const struct coproc_reg cp15_regs[] = {
>  
>  	/* PRRR/NMRR (aka MAIR0/MAIR1): swapped by interrupt.S. */
>  	{ CRn(10), CRm( 2), Op1( 0), Op2( 0), is32,
> -			NULL, reset_unknown, c10_PRRR},
> +			access_vm_reg, reset_unknown, c10_PRRR},
>  	{ CRn(10), CRm( 2), Op1( 0), Op2( 1), is32,
> -			NULL, reset_unknown, c10_NMRR},
> +			access_vm_reg, reset_unknown, c10_NMRR},
>  
>  	/* VBAR: swapped by interrupt.S. */
>  	{ CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
> @@ -334,7 +387,7 @@ static const struct coproc_reg cp15_regs[] = {
>  
>  	/* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */
>  	{ CRn(13), CRm( 0), Op1( 0), Op2( 1), is32,
> -			NULL, reset_val, c13_CID, 0x00000000 },
> +			access_vm_reg, reset_val, c13_CID, 0x00000000 },
>  	{ CRn(13), CRm( 0), Op1( 0), Op2( 2), is32,
>  			NULL, reset_unknown, c13_TID_URW },
>  	{ CRn(13), CRm( 0), Op1( 0), Op2( 3), is32,
> diff --git a/arch/arm/kvm/coproc_a15.c b/arch/arm/kvm/coproc_a15.c
> index bb0cac1..e6f4ae4 100644
> --- a/arch/arm/kvm/coproc_a15.c
> +++ b/arch/arm/kvm/coproc_a15.c
> @@ -34,7 +34,7 @@
>  static const struct coproc_reg a15_regs[] = {
>  	/* SCTLR: swapped by interrupt.S. */
>  	{ CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
> -			NULL, reset_val, c1_SCTLR, 0x00C50078 },
> +			access_sctlr, reset_val, c1_SCTLR, 0x00C50078 },
>  };
>  
>  static struct kvm_coproc_target_table a15_target_table = {
> diff --git a/arch/arm/kvm/coproc_a7.c b/arch/arm/kvm/coproc_a7.c
> index 1df76733..17fc7cd 100644
> --- a/arch/arm/kvm/coproc_a7.c
> +++ b/arch/arm/kvm/coproc_a7.c
> @@ -37,7 +37,7 @@
>  static const struct coproc_reg a7_regs[] = {
>  	/* SCTLR: swapped by interrupt.S. */
>  	{ CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
> -			NULL, reset_val, c1_SCTLR, 0x00C50878 },
> +			access_sctlr, reset_val, c1_SCTLR, 0x00C50878 },
>  };
>  
>  static struct kvm_coproc_target_table a7_target_table = {
> -- 
> 1.8.3.4
> 

Otherwise looks good,
diff mbox

Patch

diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
index a843e74..816db0b 100644
--- a/arch/arm/include/asm/kvm_arm.h
+++ b/arch/arm/include/asm/kvm_arm.h
@@ -55,6 +55,7 @@ 
  * The bits we set in HCR:
  * TAC:		Trap ACTLR
  * TSC:		Trap SMC
+ * TVM:		Trap VM ops (until MMU and caches are on)
  * TSW:		Trap cache operations by set/way
  * TWI:		Trap WFI
  * TWE:		Trap WFE
@@ -68,7 +69,7 @@ 
  */
 #define HCR_GUEST_MASK (HCR_TSC | HCR_TSW | HCR_TWI | HCR_VM | HCR_BSU_IS | \
 			HCR_FB | HCR_TAC | HCR_AMO | HCR_IMO | HCR_FMO | \
-			HCR_TWE | HCR_SWIO | HCR_TIDCP)
+			HCR_TVM | HCR_TWE | HCR_SWIO | HCR_TIDCP)
 
 /* System Control Register (SCTLR) bits */
 #define SCTLR_TE	(1 << 30)
diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
index 126c90d..1839770 100644
--- a/arch/arm/kvm/coproc.c
+++ b/arch/arm/kvm/coproc.c
@@ -23,6 +23,7 @@ 
 #include <asm/kvm_host.h>
 #include <asm/kvm_emulate.h>
 #include <asm/kvm_coproc.h>
+#include <asm/kvm_mmu.h>
 #include <asm/cacheflush.h>
 #include <asm/cputype.h>
 #include <trace/events/kvm.h>
@@ -205,6 +206,55 @@  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 coproc_params *p,
+			  const struct coproc_reg *r)
+{
+	if (p->is_write) {
+		vcpu->arch.cp15[r->reg] = *vcpu_reg(vcpu, p->Rt1);
+		if (p->is_64bit)
+			vcpu->arch.cp15[r->reg + 1] = *vcpu_reg(vcpu, p->Rt2);
+	} else {
+		*vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
+		if (p->is_64bit)
+			*vcpu_reg(vcpu, p->Rt2) = vcpu->arch.cp15[r->reg + 1];
+	}
+
+	return true;
+}
+
+/*
+ * SCTLR 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.
+ *
+ * Used by the cpu-specific code.
+ */
+bool access_sctlr(struct kvm_vcpu *vcpu,
+		  const struct coproc_params *p,
+		  const struct coproc_reg *r)
+{
+	if (p->is_write) {
+		unsigned long val;
+
+		val = *vcpu_reg(vcpu, p->Rt1);
+		vcpu->arch.cp15[r->reg] = val;
+
+		if ((val & (0b101)) == 0b101) {	/* MMU+Caches enabled? */
+			vcpu->arch.hcr &= ~HCR_TVM;
+			stage2_flush_vm(vcpu->kvm);
+		}
+	} else {
+		*vcpu_reg(vcpu, p->Rt1) = vcpu->arch.cp15[r->reg];
+	}
+
+	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.
@@ -261,33 +311,36 @@  static const struct coproc_reg cp15_regs[] = {
 	{ CRn( 1), CRm( 0), Op1( 0), Op2( 2), is32,
 			NULL, reset_val, c1_CPACR, 0x00000000 },
 
-	/* TTBR0/TTBR1: swapped by interrupt.S. */
-	{ CRm64( 2), Op1( 0), is64, NULL, reset_unknown64, c2_TTBR0 },
-	{ CRm64( 2), Op1( 1), is64, NULL, reset_unknown64, c2_TTBR1 },
-
-	/* TTBCR: swapped by interrupt.S. */
+	/* TTBR0/TTBR1/TTBCR: swapped by interrupt.S. */
+	{ CRm64( 2), Op1( 0), is64, access_vm_reg, reset_unknown64, c2_TTBR0 },
+	{ CRn(2), CRm( 0), Op1( 0), Op2( 0), is32,
+			access_vm_reg, reset_unknown, c2_TTBR0 },
+	{ CRn(2), CRm( 0), Op1( 0), Op2( 1), is32,
+			access_vm_reg, reset_unknown, c2_TTBR1 },
 	{ CRn( 2), CRm( 0), Op1( 0), Op2( 2), is32,
-			NULL, reset_val, c2_TTBCR, 0x00000000 },
+			access_vm_reg, reset_val, c2_TTBCR, 0x00000000 },
+	{ CRm64( 2), Op1( 1), is64, access_vm_reg, reset_unknown64, c2_TTBR1 },
+
 
 	/* DACR: swapped by interrupt.S. */
 	{ CRn( 3), CRm( 0), Op1( 0), Op2( 0), is32,
-			NULL, reset_unknown, c3_DACR },
+			access_vm_reg, reset_unknown, c3_DACR },
 
 	/* DFSR/IFSR/ADFSR/AIFSR: swapped by interrupt.S. */
 	{ CRn( 5), CRm( 0), Op1( 0), Op2( 0), is32,
-			NULL, reset_unknown, c5_DFSR },
+			access_vm_reg, reset_unknown, c5_DFSR },
 	{ CRn( 5), CRm( 0), Op1( 0), Op2( 1), is32,
-			NULL, reset_unknown, c5_IFSR },
+			access_vm_reg, reset_unknown, c5_IFSR },
 	{ CRn( 5), CRm( 1), Op1( 0), Op2( 0), is32,
-			NULL, reset_unknown, c5_ADFSR },
+			access_vm_reg, reset_unknown, c5_ADFSR },
 	{ CRn( 5), CRm( 1), Op1( 0), Op2( 1), is32,
-			NULL, reset_unknown, c5_AIFSR },
+			access_vm_reg, reset_unknown, c5_AIFSR },
 
 	/* DFAR/IFAR: swapped by interrupt.S. */
 	{ CRn( 6), CRm( 0), Op1( 0), Op2( 0), is32,
-			NULL, reset_unknown, c6_DFAR },
+			access_vm_reg, reset_unknown, c6_DFAR },
 	{ CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
-			NULL, reset_unknown, c6_IFAR },
+			access_vm_reg, reset_unknown, c6_IFAR },
 
 	/* PAR swapped by interrupt.S */
 	{ CRm64( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
@@ -324,9 +377,9 @@  static const struct coproc_reg cp15_regs[] = {
 
 	/* PRRR/NMRR (aka MAIR0/MAIR1): swapped by interrupt.S. */
 	{ CRn(10), CRm( 2), Op1( 0), Op2( 0), is32,
-			NULL, reset_unknown, c10_PRRR},
+			access_vm_reg, reset_unknown, c10_PRRR},
 	{ CRn(10), CRm( 2), Op1( 0), Op2( 1), is32,
-			NULL, reset_unknown, c10_NMRR},
+			access_vm_reg, reset_unknown, c10_NMRR},
 
 	/* VBAR: swapped by interrupt.S. */
 	{ CRn(12), CRm( 0), Op1( 0), Op2( 0), is32,
@@ -334,7 +387,7 @@  static const struct coproc_reg cp15_regs[] = {
 
 	/* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */
 	{ CRn(13), CRm( 0), Op1( 0), Op2( 1), is32,
-			NULL, reset_val, c13_CID, 0x00000000 },
+			access_vm_reg, reset_val, c13_CID, 0x00000000 },
 	{ CRn(13), CRm( 0), Op1( 0), Op2( 2), is32,
 			NULL, reset_unknown, c13_TID_URW },
 	{ CRn(13), CRm( 0), Op1( 0), Op2( 3), is32,
diff --git a/arch/arm/kvm/coproc_a15.c b/arch/arm/kvm/coproc_a15.c
index bb0cac1..e6f4ae4 100644
--- a/arch/arm/kvm/coproc_a15.c
+++ b/arch/arm/kvm/coproc_a15.c
@@ -34,7 +34,7 @@ 
 static const struct coproc_reg a15_regs[] = {
 	/* SCTLR: swapped by interrupt.S. */
 	{ CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
-			NULL, reset_val, c1_SCTLR, 0x00C50078 },
+			access_sctlr, reset_val, c1_SCTLR, 0x00C50078 },
 };
 
 static struct kvm_coproc_target_table a15_target_table = {
diff --git a/arch/arm/kvm/coproc_a7.c b/arch/arm/kvm/coproc_a7.c
index 1df76733..17fc7cd 100644
--- a/arch/arm/kvm/coproc_a7.c
+++ b/arch/arm/kvm/coproc_a7.c
@@ -37,7 +37,7 @@ 
 static const struct coproc_reg a7_regs[] = {
 	/* SCTLR: swapped by interrupt.S. */
 	{ CRn( 1), CRm( 0), Op1( 0), Op2( 0), is32,
-			NULL, reset_val, c1_SCTLR, 0x00C50878 },
+			access_sctlr, reset_val, c1_SCTLR, 0x00C50878 },
 };
 
 static struct kvm_coproc_target_table a7_target_table = {