Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1726941 > unrolled thread

[PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

Started bygengdongjiu <gengdongjiu@huawei.com>
First post2017-09-05 21:10 +0200
Last post2017-09-06 12:00 +0200
Articles 13 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-05 21:10 +0200
    Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 07:30 +0200
    Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits Marc Zyngier <marc.zyngier@arm.com> - 2017-09-06 10:20 +0200
      Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 11:40 +0200
        Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits Vladimir Murzin <vladimir.murzin@arm.com> - 2017-09-06 11:50 +0200
          Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 12:40 +0200
            Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits Vladimir Murzin <vladimir.murzin@arm.com> - 2017-09-06 14:10 +0200
              Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 14:20 +0200
                Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits Vladimir Murzin <vladimir.murzin@arm.com> - 2017-09-06 14:40 +0200
                  Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 14:50 +0200
                    Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits Vladimir Murzin <vladimir.murzin@arm.com> - 2017-09-06 15:10 +0200
              Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 14:40 +0200
        Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu <gengdongjiu@huawei.com> - 2017-09-06 12:00 +0200

#1726941 — [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-05 21:10 +0200
Subject[PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
Message-ID<umrov-6Vh-37@gated-at.bofh.it>
when exit from guest, some host PSTATE bits may be lost, such as
PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
in the EL2, host PSTATE value cannot be saved and restored via
SPSR_EL2. So if guest has changed the PSTATE, host continues with
a wrong value guest has set.

Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
---
 arch/arm64/include/asm/kvm_host.h |  8 +++++++
 arch/arm64/include/asm/kvm_hyp.h  |  2 ++
 arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
 arch/arm64/kvm/hyp/entry.S        |  2 --
 arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
 arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
 6 files changed, 100 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e923b58..cba7d3e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -193,6 +193,12 @@ struct kvm_cpu_context {
 	};
 };
 
+struct kvm_cpu_host_pstate {
+	u64 daif;
+	u64 uao;
+	u64 pan;
+};
+
 typedef struct kvm_cpu_context kvm_cpu_context_t;
 
 struct kvm_vcpu_arch {
@@ -227,6 +233,8 @@ struct kvm_vcpu_arch {
 
 	/* Pointer to host CPU context */
 	kvm_cpu_context_t *host_cpu_context;
+	/* Host PSTATE value */
+	struct kvm_cpu_host_pstate host_pstate;
 	struct {
 		/* {Break,watch}point registers */
 		struct kvm_guest_debug_arch regs;
diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 4572a9b..a75587a 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -134,6 +134,8 @@
 
 void __sysreg_save_host_state(struct kvm_cpu_context *ctxt);
 void __sysreg_restore_host_state(struct kvm_cpu_context *ctxt);
+void __sysreg_save_host_pstate(struct kvm_vcpu *vcpu);
+void __sysreg_restore_host_pstate(struct kvm_vcpu *vcpu);
 void __sysreg_save_guest_state(struct kvm_cpu_context *ctxt);
 void __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt);
 void __sysreg32_save_state(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 248339e..efdcf40 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -295,6 +295,29 @@
 #define SYS_ICH_LR14_EL2		__SYS__LR8_EL2(6)
 #define SYS_ICH_LR15_EL2		__SYS__LR8_EL2(7)
 
+#define REG_PSTATE_PAN			sys_reg(3, 0, 4, 2, 3)
+#define REG_PSTATE_UAO			sys_reg(3, 0, 4, 2, 4)
+
+#define GET_PSTATE_PAN					\
+	({						\
+		u64 reg;				\
+		asm volatile(ALTERNATIVE("mov %0, #0",	\
+				"mrs_s %0, " __stringify(REG_PSTATE_PAN),\
+				ARM64_HAS_PAN)\
+				: "=r" (reg));\
+		reg;				\
+	})
+
+#define GET_PSTATE_UAO					\
+	({						\
+		u64 reg;					\
+		asm volatile(ALTERNATIVE("mov %0, #0",\
+				"mrs_s %0, " __stringify(REG_PSTATE_UAO),\
+				ARM64_HAS_UAO)\
+				: "=r" (reg));\
+		reg;			\
+	})
+
 /* Common SCTLR_ELx flags. */
 #define SCTLR_ELx_EE    (1 << 25)
 #define SCTLR_ELx_I	(1 << 12)
diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 12ee62d..7662ef5 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -96,8 +96,6 @@ ENTRY(__guest_exit)
 
 	add	x1, x1, #VCPU_CONTEXT
 
-	ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
-
 	// Store the guest regs x2 and x3
 	stp	x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
 
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index 945e79c..9b380a1 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -278,6 +278,26 @@ static void __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
 	write_sysreg_el2(*vcpu_pc(vcpu), elr);
 }
 
+static void __hyp_text __save_host_state(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+
+	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+
+	__sysreg_save_host_state(host_ctxt);
+	__sysreg_save_host_pstate(vcpu);
+}
+
+static void __hyp_text __restore_host_state(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpu_context *host_ctxt;
+
+	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
+
+	__sysreg_restore_host_state(host_ctxt);
+	__sysreg_restore_host_pstate(vcpu);
+}
+
 int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
@@ -291,7 +311,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
 	guest_ctxt = &vcpu->arch.ctxt;
 
-	__sysreg_save_host_state(host_ctxt);
+	__save_host_state(vcpu);
 	__debug_cond_save_host_state(vcpu);
 
 	__activate_traps(vcpu);
@@ -374,7 +394,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	__deactivate_traps(vcpu);
 	__deactivate_vm(vcpu);
 
-	__sysreg_restore_host_state(host_ctxt);
+	__restore_host_state(vcpu);
 
 	if (fp_enabled) {
 		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..ea8f437 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -22,7 +22,11 @@
 #include <asm/kvm_hyp.h>
 
 /* Yes, this does nothing, on purpose */
-static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
+static void __hyp_text __sysreg_do_nothing_state(struct kvm_cpu_context *ctxt)
+{ }
+static void __hyp_text __sysreg_do_nothing_pstate(struct kvm_vcpu *vcpu)
+{ }
+
 
 /*
  * Non-VHE: Both host and guest must save everything.
@@ -69,7 +73,7 @@ static void __hyp_text __sysreg_save_state(struct kvm_cpu_context *ctxt)
 }
 
 static hyp_alternate_select(__sysreg_call_save_host_state,
-			    __sysreg_save_state, __sysreg_do_nothing,
+			    __sysreg_save_state, __sysreg_do_nothing_state,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
 void __hyp_text __sysreg_save_host_state(struct kvm_cpu_context *ctxt)
@@ -122,7 +126,7 @@ static void __hyp_text __sysreg_restore_state(struct kvm_cpu_context *ctxt)
 }
 
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-			    __sysreg_restore_state, __sysreg_do_nothing,
+			    __sysreg_restore_state, __sysreg_do_nothing_state,
 			    ARM64_HAS_VIRT_HOST_EXTN);
 
 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
@@ -137,6 +141,44 @@ void __hyp_text __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt)
 	__sysreg_restore_common_state(ctxt);
 }
 
+static void __hyp_text __sysreg_save_pstate(struct kvm_vcpu *vcpu)
+{
+	vcpu->arch.host_pstate.daif = read_sysreg(daif);
+	vcpu->arch.host_pstate.pan = GET_PSTATE_PAN;
+	vcpu->arch.host_pstate.uao = GET_PSTATE_UAO;
+}
+
+static hyp_alternate_select(__sysreg_call_save_host_pstate,
+			    __sysreg_save_pstate, __sysreg_do_nothing_pstate,
+			    ARM64_HAS_VIRT_HOST_EXTN);
+
+void __hyp_text __sysreg_save_host_pstate(struct kvm_vcpu *vcpu)
+{
+	__sysreg_call_save_host_pstate()(vcpu);
+}
+
+static void __hyp_text __sysreg_restore_pstate(struct kvm_vcpu *vcpu)
+{
+	u8 value = !!(vcpu->arch.host_pstate.pan);
+
+	write_sysreg(vcpu->arch.host_pstate.daif, daif);
+	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(value), ARM64_HAS_PAN,
+			CONFIG_ARM64_PAN));
+
+	value = !!(vcpu->arch.host_pstate.uao);
+	asm(ALTERNATIVE("nop", SET_PSTATE_UAO(value), ARM64_HAS_UAO,
+			CONFIG_ARM64_UAO));
+}
+
+static hyp_alternate_select(__sysreg_call_restore_host_pstate,
+			    __sysreg_restore_pstate, __sysreg_do_nothing_pstate,
+			    ARM64_HAS_VIRT_HOST_EXTN);
+
+void __hyp_text __sysreg_restore_host_pstate(struct kvm_vcpu *vcpu)
+{
+	__sysreg_call_restore_host_pstate()(vcpu);
+}
+
 void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
 {
 	u64 *spsr, *sysreg;
-- 
1.9.1

[toc] | [next] | [standalone]

#1727162

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 07:30 +0200
Message-ID<umB4u-5b7-17@gated-at.bofh.it>
In reply to#1726941
CC Catalin


On 2017/9/6 2:58, gengdongjiu wrote:
> when exit from guest, some host PSTATE bits may be lost, such as
> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
> in the EL2, host PSTATE value cannot be saved and restored via
> SPSR_EL2. So if guest has changed the PSTATE, host continues with
> a wrong value guest has set.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 +++++++
>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>  arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
>  arch/arm64/kvm/hyp/entry.S        |  2 --
>  arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
>  arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
>  6 files changed, 100 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e923b58..cba7d3e 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>  	};
>  };
>  
> +struct kvm_cpu_host_pstate {
> +	u64 daif;
> +	u64 uao;
> +	u64 pan;
> +};
> +
>  typedef struct kvm_cpu_context kvm_cpu_context_t;
>  
>  struct kvm_vcpu_arch {
> @@ -227,6 +233,8 @@ struct kvm_vcpu_arch {
>  
>  	/* Pointer to host CPU context */
>  	kvm_cpu_context_t *host_cpu_context;
> +	/* Host PSTATE value */
> +	struct kvm_cpu_host_pstate host_pstate;
>  	struct {
>  		/* {Break,watch}point registers */
>  		struct kvm_guest_debug_arch regs;
> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> index 4572a9b..a75587a 100644
> --- a/arch/arm64/include/asm/kvm_hyp.h
> +++ b/arch/arm64/include/asm/kvm_hyp.h
> @@ -134,6 +134,8 @@
>  
>  void __sysreg_save_host_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_host_state(struct kvm_cpu_context *ctxt);
> +void __sysreg_save_host_pstate(struct kvm_vcpu *vcpu);
> +void __sysreg_restore_host_pstate(struct kvm_vcpu *vcpu);
>  void __sysreg_save_guest_state(struct kvm_cpu_context *ctxt);
>  void __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt);
>  void __sysreg32_save_state(struct kvm_vcpu *vcpu);
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 248339e..efdcf40 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -295,6 +295,29 @@
>  #define SYS_ICH_LR14_EL2		__SYS__LR8_EL2(6)
>  #define SYS_ICH_LR15_EL2		__SYS__LR8_EL2(7)
>  
> +#define REG_PSTATE_PAN			sys_reg(3, 0, 4, 2, 3)
> +#define REG_PSTATE_UAO			sys_reg(3, 0, 4, 2, 4)
> +
> +#define GET_PSTATE_PAN					\
> +	({						\
> +		u64 reg;				\
> +		asm volatile(ALTERNATIVE("mov %0, #0",	\
> +				"mrs_s %0, " __stringify(REG_PSTATE_PAN),\
> +				ARM64_HAS_PAN)\
> +				: "=r" (reg));\
> +		reg;				\
> +	})
> +
> +#define GET_PSTATE_UAO					\
> +	({						\
> +		u64 reg;					\
> +		asm volatile(ALTERNATIVE("mov %0, #0",\
> +				"mrs_s %0, " __stringify(REG_PSTATE_UAO),\
> +				ARM64_HAS_UAO)\
> +				: "=r" (reg));\
> +		reg;			\
> +	})
> +
>  /* Common SCTLR_ELx flags. */
>  #define SCTLR_ELx_EE    (1 << 25)
>  #define SCTLR_ELx_I	(1 << 12)
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 12ee62d..7662ef5 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,8 +96,6 @@ ENTRY(__guest_exit)
>  
>  	add	x1, x1, #VCPU_CONTEXT
>  
> -	ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> -
>  	// Store the guest regs x2 and x3
>  	stp	x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
>  
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index 945e79c..9b380a1 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -278,6 +278,26 @@ static void __hyp_text __skip_instr(struct kvm_vcpu *vcpu)
>  	write_sysreg_el2(*vcpu_pc(vcpu), elr);
>  }
>  
> +static void __hyp_text __save_host_state(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +
> +	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> +
> +	__sysreg_save_host_state(host_ctxt);
> +	__sysreg_save_host_pstate(vcpu);
> +}
> +
> +static void __hyp_text __restore_host_state(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpu_context *host_ctxt;
> +
> +	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> +
> +	__sysreg_restore_host_state(host_ctxt);
> +	__sysreg_restore_host_pstate(vcpu);
> +}
> +
>  int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpu_context *host_ctxt;
> @@ -291,7 +311,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
>  	guest_ctxt = &vcpu->arch.ctxt;
>  
> -	__sysreg_save_host_state(host_ctxt);
> +	__save_host_state(vcpu);
>  	__debug_cond_save_host_state(vcpu);
>  
>  	__activate_traps(vcpu);
> @@ -374,7 +394,7 @@ int __hyp_text __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>  	__deactivate_traps(vcpu);
>  	__deactivate_vm(vcpu);
>  
> -	__sysreg_restore_host_state(host_ctxt);
> +	__restore_host_state(vcpu);
>  
>  	if (fp_enabled) {
>  		__fpsimd_save_state(&guest_ctxt->gp_regs.fp_regs);
> diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
> index 9341376..ea8f437 100644
> --- a/arch/arm64/kvm/hyp/sysreg-sr.c
> +++ b/arch/arm64/kvm/hyp/sysreg-sr.c
> @@ -22,7 +22,11 @@
>  #include <asm/kvm_hyp.h>
>  
>  /* Yes, this does nothing, on purpose */
> -static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }
> +static void __hyp_text __sysreg_do_nothing_state(struct kvm_cpu_context *ctxt)
> +{ }
> +static void __hyp_text __sysreg_do_nothing_pstate(struct kvm_vcpu *vcpu)
> +{ }
> +
>  
>  /*
>   * Non-VHE: Both host and guest must save everything.
> @@ -69,7 +73,7 @@ static void __hyp_text __sysreg_save_state(struct kvm_cpu_context *ctxt)
>  }
>  
>  static hyp_alternate_select(__sysreg_call_save_host_state,
> -			    __sysreg_save_state, __sysreg_do_nothing,
> +			    __sysreg_save_state, __sysreg_do_nothing_state,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
>  void __hyp_text __sysreg_save_host_state(struct kvm_cpu_context *ctxt)
> @@ -122,7 +126,7 @@ static void __hyp_text __sysreg_restore_state(struct kvm_cpu_context *ctxt)
>  }
>  
>  static hyp_alternate_select(__sysreg_call_restore_host_state,
> -			    __sysreg_restore_state, __sysreg_do_nothing,
> +			    __sysreg_restore_state, __sysreg_do_nothing_state,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
>  void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)
> @@ -137,6 +141,44 @@ void __hyp_text __sysreg_restore_guest_state(struct kvm_cpu_context *ctxt)
>  	__sysreg_restore_common_state(ctxt);
>  }
>  
> +static void __hyp_text __sysreg_save_pstate(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.host_pstate.daif = read_sysreg(daif);
> +	vcpu->arch.host_pstate.pan = GET_PSTATE_PAN;
> +	vcpu->arch.host_pstate.uao = GET_PSTATE_UAO;
> +}
> +
> +static hyp_alternate_select(__sysreg_call_save_host_pstate,
> +			    __sysreg_save_pstate, __sysreg_do_nothing_pstate,
> +			    ARM64_HAS_VIRT_HOST_EXTN);
> +
> +void __hyp_text __sysreg_save_host_pstate(struct kvm_vcpu *vcpu)
> +{
> +	__sysreg_call_save_host_pstate()(vcpu);
> +}
> +
> +static void __hyp_text __sysreg_restore_pstate(struct kvm_vcpu *vcpu)
> +{
> +	u8 value = !!(vcpu->arch.host_pstate.pan);
> +
> +	write_sysreg(vcpu->arch.host_pstate.daif, daif);
> +	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(value), ARM64_HAS_PAN,
> +			CONFIG_ARM64_PAN));
> +
> +	value = !!(vcpu->arch.host_pstate.uao);
> +	asm(ALTERNATIVE("nop", SET_PSTATE_UAO(value), ARM64_HAS_UAO,
> +			CONFIG_ARM64_UAO));
> +}
> +
> +static hyp_alternate_select(__sysreg_call_restore_host_pstate,
> +			    __sysreg_restore_pstate, __sysreg_do_nothing_pstate,
> +			    ARM64_HAS_VIRT_HOST_EXTN);
> +
> +void __hyp_text __sysreg_restore_host_pstate(struct kvm_vcpu *vcpu)
> +{
> +	__sysreg_call_restore_host_pstate()(vcpu);
> +}
> +
>  void __hyp_text __sysreg32_save_state(struct kvm_vcpu *vcpu)
>  {
>  	u64 *spsr, *sysreg;
>

[toc] | [prev] | [next] | [standalone]

#1727211

FromMarc Zyngier <marc.zyngier@arm.com>
Date2017-09-06 10:20 +0200
Message-ID<umDJ0-7kw-17@gated-at.bofh.it>
In reply to#1726941
On 05/09/17 19:58, gengdongjiu wrote:
> when exit from guest, some host PSTATE bits may be lost, such as
> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
> in the EL2, host PSTATE value cannot be saved and restored via
> SPSR_EL2. So if guest has changed the PSTATE, host continues with
> a wrong value guest has set.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |  8 +++++++
>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>  arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
>  arch/arm64/kvm/hyp/entry.S        |  2 --
>  arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
>  arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
>  6 files changed, 100 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e923b58..cba7d3e 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>  	};
>  };
>  
> +struct kvm_cpu_host_pstate {
> +	u64 daif;
> +	u64 uao;
> +	u64 pan;
> +};

I love it. This is the most expensive way of saving/restoring a single
32bit value.

More seriously, please see the discussion between James and Christoffer
there[1]. I expect James to address the PAN/UAO states together with the
debug state in the next iteration of his patch.

Thanks,

	M.

[1] https://www.spinics.net/lists/arm-kernel/msg599798.html
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1727292

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 11:40 +0200
Message-ID<umEYp-89a-1@gated-at.bofh.it>
In reply to#1727211
Hi Marc,

On 2017/9/6 16:17, Marc Zyngier wrote:
> On 05/09/17 19:58, gengdongjiu wrote:
>> when exit from guest, some host PSTATE bits may be lost, such as
>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>> in the EL2, host PSTATE value cannot be saved and restored via
>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>> a wrong value guest has set.
>>
>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>> Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
>> ---
>>  arch/arm64/include/asm/kvm_host.h |  8 +++++++
>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>  arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
>>  arch/arm64/kvm/hyp/entry.S        |  2 --
>>  arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
>>  arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index e923b58..cba7d3e 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>  	};
>>  };
>>  
>> +struct kvm_cpu_host_pstate {
>> +	u64 daif;
>> +	u64 uao;
>> +	u64 pan;
>> +};
> 
> I love it. This is the most expensive way of saving/restoring a single
> 32bit value.
> 
> More seriously, please see the discussion between James and Christoffer
> there[1]. I expect James to address the PAN/UAO states together with the
> debug state in the next iteration of his patch.

I roughly see the discussion between James and Christoffer, Seems Christoffer does not suggest save and
restore it, and suggest to do below, and UAO/PAN may not use the same ways.

      __kvm_vcpu_run(struct kvm_vcpu *vcpu)
      {
          if (has_vhe())
              asm("msr     daifset, #0xf");

 	...
         exit_code = __guest_enter(vcpu, host_ctxt);
 	...

 	if (has_vhe())
              asm("msr     daifclr, #0xd");
      }


If not save/restore them, the KVM will set them according to the CPU capability. For example below fixing, it will check CPU capability, If CPU supports PAN,
the kvm will always enable the PAN for the host. But in fact, the host may be not enable the PAN.
Of course for the UAO, we can use the similar fixing if Marc or Christoffer is agreed. but seems not make sense.

commit cb96408da4e11698674abd04aeac941c1bed2038
Author: Vladimir Murzin <vladimir.murzin@arm.com>
Date:   Thu Sep 1 15:29:03 2016 +0100

    arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2

    SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
    exception. However, this bit has no effect on the PSTATE.PAN when
    HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
    exception taken from a guest PSTATE.PAN bit left unchanged and we
    continue with a value guest has set.

    To address that always reset PSTATE.PAN on entry from EL1.

    Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 mode")

    Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
    Reviewed-by: James Morse <james.morse@arm.com>
    Acked-by: Marc Zyngier <marc.zyngier@arm.com>
    Cc: <stable@vger.kernel.org> # v4.6+
    Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
index 3967c231..b5926ee 100644
--- a/arch/arm64/kvm/hyp/entry.S
+++ b/arch/arm64/kvm/hyp/entry.S
@@ -96,6 +96,8 @@ ENTRY(__guest_exit)

        add     x1, x1, #VCPU_CONTEXT

+       ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
+
        // Store the guest regs x2 and x3
        stp     x2, x3,   [x1, #CPU_XREG_OFFSET(2)]


> 
> Thanks,
> 
> 	M.
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg599798.html
>

[toc] | [prev] | [next] | [standalone]

#1727297

FromVladimir Murzin <vladimir.murzin@arm.com>
Date2017-09-06 11:50 +0200
Message-ID<umF85-8in-13@gated-at.bofh.it>
In reply to#1727292
On 06/09/17 10:32, gengdongjiu wrote:
> Hi Marc,
> 
> On 2017/9/6 16:17, Marc Zyngier wrote:
>> On 05/09/17 19:58, gengdongjiu wrote:
>>> when exit from guest, some host PSTATE bits may be lost, such as
>>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>>> in the EL2, host PSTATE value cannot be saved and restored via
>>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>>> a wrong value guest has set.
>>>
>>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>>> Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
>>> ---
>>>  arch/arm64/include/asm/kvm_host.h |  8 +++++++
>>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>>  arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
>>>  arch/arm64/kvm/hyp/entry.S        |  2 --
>>>  arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
>>>  arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
>>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>> index e923b58..cba7d3e 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>>  	};
>>>  };
>>>  
>>> +struct kvm_cpu_host_pstate {
>>> +	u64 daif;
>>> +	u64 uao;
>>> +	u64 pan;
>>> +};
>>
>> I love it. This is the most expensive way of saving/restoring a single
>> 32bit value.
>>
>> More seriously, please see the discussion between James and Christoffer
>> there[1]. I expect James to address the PAN/UAO states together with the
>> debug state in the next iteration of his patch.
> 
> I roughly see the discussion between James and Christoffer, Seems Christoffer does not suggest save and
> restore it, and suggest to do below, and UAO/PAN may not use the same ways.
> 
>       __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>       {
>           if (has_vhe())
>               asm("msr     daifset, #0xf");
> 
>  	...
>          exit_code = __guest_enter(vcpu, host_ctxt);
>  	...
> 
>  	if (has_vhe())
>               asm("msr     daifclr, #0xd");
>       }
> 
> 
> If not save/restore them, the KVM will set them according to the CPU capability. For example below fixing, it will check CPU capability, If CPU supports PAN,
> the kvm will always enable the PAN for the host. But in fact, the host may be not enable the PAN.

Can you please elaborate on cases where PAN is not enabled?

Thanks
Vladimir

> Of course for the UAO, we can use the similar fixing if Marc or Christoffer is agreed. but seems not make sense.
> 
> commit cb96408da4e11698674abd04aeac941c1bed2038
> Author: Vladimir Murzin <vladimir.murzin@arm.com>
> Date:   Thu Sep 1 15:29:03 2016 +0100
> 
>     arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2
> 
>     SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
>     exception. However, this bit has no effect on the PSTATE.PAN when
>     HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
>     exception taken from a guest PSTATE.PAN bit left unchanged and we
>     continue with a value guest has set.
> 
>     To address that always reset PSTATE.PAN on entry from EL1.
> 
>     Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 mode")
> 
>     Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>     Reviewed-by: James Morse <james.morse@arm.com>
>     Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>     Cc: <stable@vger.kernel.org> # v4.6+
>     Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> 
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 3967c231..b5926ee 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,6 +96,8 @@ ENTRY(__guest_exit)
> 
>         add     x1, x1, #VCPU_CONTEXT
> 
> +       ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +
>         // Store the guest regs x2 and x3
>         stp     x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
> 
> 
>>
>> Thanks,
>>
>> 	M.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg599798.html
>>
> 
>

[toc] | [prev] | [next] | [standalone]

#1727326

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 12:40 +0200
Message-ID<umFUv-oN-7@gated-at.bofh.it>
In reply to#1727297
Vladimir,

On 2017/9/6 17:41, Vladimir Murzin wrote:
> Can you please elaborate on cases where PAN is not enabled?

I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.

[toc] | [prev] | [next] | [standalone]

#1727385

FromVladimir Murzin <vladimir.murzin@arm.com>
Date2017-09-06 14:10 +0200
Message-ID<umHjA-1yw-23@gated-at.bofh.it>
In reply to#1727326
On 06/09/17 11:35, gengdongjiu wrote:
> Vladimir,
> 
> On 2017/9/6 17:41, Vladimir Murzin wrote:
>> Can you please elaborate on cases where PAN is not enabled?
> 
> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
> 
> 

So, in short, there is no real issue with PAN, right? What about UAO?

Cheers
Vladimir

[toc] | [prev] | [next] | [standalone]

#1727388

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 14:20 +0200
Message-ID<umHtf-1Ck-1@gated-at.bofh.it>
In reply to#1727385

On 2017/9/6 20:00, Vladimir Murzin wrote:
> On 06/09/17 11:35, gengdongjiu wrote:
>> Vladimir,
>>
>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>> Can you please elaborate on cases where PAN is not enabled?
>>
>> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
>> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
>> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
>>
>>
> 
> So, in short, there is no real issue with PAN, right? What about UAO?
For the PAN, if host OS dynamically enable/disable PAN should have issue.
Do you think that is not a issue as above description?

"host OS dynamically disable the PAN, but after go back from the guest OS, The PAN is unexpectedly enabled"

> 
> Cheers
> Vladimir
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1727400

FromVladimir Murzin <vladimir.murzin@arm.com>
Date2017-09-06 14:40 +0200
Message-ID<umHMD-1JN-21@gated-at.bofh.it>
In reply to#1727388
On 06/09/17 13:14, gengdongjiu wrote:
> 
> 
> On 2017/9/6 20:00, Vladimir Murzin wrote:
>> On 06/09/17 11:35, gengdongjiu wrote:
>>> Vladimir,
>>>
>>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>>> Can you please elaborate on cases where PAN is not enabled?
>>>
>>> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
>>> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
>>> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
>>>
>>>
>>
>> So, in short, there is no real issue with PAN, right? What about UAO?
> For the PAN, if host OS dynamically enable/disable PAN should have issue.
> Do you think that is not a issue as above description?
> 
> "host OS dynamically disable the PAN, but after go back from the guest OS, The PAN is unexpectedly enabled"

Do you see effect of "PAN is unexpectedly enabled"?

Cheers
Vladimir

> 
>>
>> Cheers
>> Vladimir
>>
>> .
>>
> 
>

[toc] | [prev] | [next] | [standalone]

#1727406

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 14:50 +0200
Message-ID<umHWh-1Qd-3@gated-at.bofh.it>
In reply to#1727400

On 2017/9/6 20:30, Vladimir Murzin wrote:
> On 06/09/17 13:14, gengdongjiu wrote:
>>
>>
>> On 2017/9/6 20:00, Vladimir Murzin wrote:
>>> On 06/09/17 11:35, gengdongjiu wrote:
>>>> Vladimir,
>>>>
>>>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>>>> Can you please elaborate on cases where PAN is not enabled?
>>>>
>>>> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
>>>> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
>>>> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
>>>>
>>>>
>>>
>>> So, in short, there is no real issue with PAN, right? What about UAO?
>> For the PAN, if host OS dynamically enable/disable PAN should have issue.
>> Do you think that is not a issue as above description?
>>
>> "host OS dynamically disable the PAN, but after go back from the guest OS, The PAN is unexpectedly enabled"
> 
> Do you see effect of "PAN is unexpectedly enabled"?
In fact I did not encounter this case, but I think it can exist.
I think if host OS dynamically disable PAN, it wants the host kernel access the user space address space not through copy_to/from_user API.
Now if it is unexpectedly enabled, when host kernel still accesses the user space address,  it will happen MMU fault exception.


> 
> Cheers
> Vladimir
> 
>>
>>>
>>> Cheers
>>> Vladimir
>>>
>>> .
>>>
>>
>>
> 
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1727439

FromVladimir Murzin <vladimir.murzin@arm.com>
Date2017-09-06 15:10 +0200
Message-ID<umIfF-2cQ-27@gated-at.bofh.it>
In reply to#1727406
On 06/09/17 13:44, gengdongjiu wrote:
> 
> 
> On 2017/9/6 20:30, Vladimir Murzin wrote:
>> On 06/09/17 13:14, gengdongjiu wrote:
>>>
>>>
>>> On 2017/9/6 20:00, Vladimir Murzin wrote:
>>>> On 06/09/17 11:35, gengdongjiu wrote:
>>>>> Vladimir,
>>>>>
>>>>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>>>>> Can you please elaborate on cases where PAN is not enabled?
>>>>>
>>>>> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
>>>>> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
>>>>> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
>>>>>
>>>>>
>>>>
>>>> So, in short, there is no real issue with PAN, right? What about UAO?
>>> For the PAN, if host OS dynamically enable/disable PAN should have issue.
>>> Do you think that is not a issue as above description?
>>>
>>> "host OS dynamically disable the PAN, but after go back from the guest OS, The PAN is unexpectedly enabled"
>>
>> Do you see effect of "PAN is unexpectedly enabled"?
> In fact I did not encounter this case, but I think it can exist.
> I think if host OS dynamically disable PAN, it wants the host kernel access the user space address space not through copy_to/from_user API.
> Now if it is unexpectedly enabled, when host kernel still accesses the user space address,  it will happen MMU fault exception.

And this is expected! The only allowed channel for kernel <-> user is uaccess
API.

I guess that you have test (and that great!) which violates that rule (for
testing purpose, why not?) and now you are trying to fit kernel into it...

Cheers
Vladimir

> 
> 
>>
>> Cheers
>> Vladimir
>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
> 
>

[toc] | [prev] | [next] | [standalone]

#1727397

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 14:40 +0200
Message-ID<umHMC-1JN-17@gated-at.bofh.it>
In reply to#1727385

On 2017/9/6 20:00, Vladimir Murzin wrote:
> On 06/09/17 11:35, gengdongjiu wrote:
>> Vladimir,
>>
>> On 2017/9/6 17:41, Vladimir Murzin wrote:
>>> Can you please elaborate on cases where PAN is not enabled?
>>
>> I mean the informal private usage, For example, he disabled the PAN dynamically to let kernel space to access the user space.
>> After he dynamic disabled the PAN, then switched to guest OS. after return to host. he found the PAN stage is modified.
>> Of cause this is not a formal usage, in our host kernel, it is always enabled, no dynamic change, but I means it may exist such cases.
>>
>>
> 
> So, in short, there is no real issue with PAN, right? What about UAO?
For the pstate.UAO, current code has issue from my test. Because after switching from guest os, it does not set pstate.UAO again.
PAN is set again in your previous patch when switch to host, but UAO is not.
If you have concern about the save/restore PSTATE bits, may be we can use below modification to fix UAO issue.

diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..c3dd761 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -21,6 +21,8 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_hyp.h>

+#include <asm/exec.h>

 /* Yes, this does nothing, on purpose */
 static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }

@@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct kvm_cpu_context *ctxt)
 	write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
 }

+static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context *ctxt)
+{
+    uao_thread_switch(current);
+}
+
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-			    __sysreg_restore_state, __sysreg_do_nothing,
+			    __sysreg_restore_state, __sysreg_restore_state_vhe,
 			    ARM64_HAS_VIRT_HOST_EXTN);

 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)


> 
> Cheers
> Vladimir
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1727305

Fromgengdongjiu <gengdongjiu@huawei.com>
Date2017-09-06 12:00 +0200
Message-ID<umFhN-8my-39@gated-at.bofh.it>
In reply to#1727292
For UAO, if not save/restore PSTATE.UAO, we can use below fixing.

diff --git a/arch/arm64/kvm/hyp/sysreg-sr.c b/arch/arm64/kvm/hyp/sysreg-sr.c
index 9341376..c3dd761 100644
--- a/arch/arm64/kvm/hyp/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/sysreg-sr.c
@@ -21,6 +21,8 @@
 #include <asm/kvm_asm.h>
 #include <asm/kvm_hyp.h>

+#include <asm/exec.h>
+
 /* Yes, this does nothing, on purpose */
 static void __hyp_text __sysreg_do_nothing(struct kvm_cpu_context *ctxt) { }

@@ -121,8 +123,13 @@ static void __hyp_text __sysreg_restore_state(struct kvm_cpu_context *ctxt)
 	write_sysreg_el1(ctxt->gp_regs.spsr[KVM_SPSR_EL1],spsr);
 }

+static void __hyp_text __sysreg_restore_state_vhe(struct kvm_cpu_context *ctxt)
+{
+    uao_thread_switch(current);
+}
+
 static hyp_alternate_select(__sysreg_call_restore_host_state,
-			    __sysreg_restore_state, __sysreg_do_nothing,
+			    __sysreg_restore_state, __sysreg_restore_state_vhe,
 			    ARM64_HAS_VIRT_HOST_EXTN);

 void __hyp_text __sysreg_restore_host_state(struct kvm_cpu_context *ctxt)



On 2017/9/6 17:32, gengdongjiu wrote:
> Hi Marc,
> 
> On 2017/9/6 16:17, Marc Zyngier wrote:
>> On 05/09/17 19:58, gengdongjiu wrote:
>>> when exit from guest, some host PSTATE bits may be lost, such as
>>> PSTATE.PAN or PSTATE.UAO. It is because host and hypervisor all run
>>> in the EL2, host PSTATE value cannot be saved and restored via
>>> SPSR_EL2. So if guest has changed the PSTATE, host continues with
>>> a wrong value guest has set.
>>>
>>> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
>>> Signed-off-by: Haibin Zhang <zhanghaibin7@huawei.com>
>>> ---
>>>  arch/arm64/include/asm/kvm_host.h |  8 +++++++
>>>  arch/arm64/include/asm/kvm_hyp.h  |  2 ++
>>>  arch/arm64/include/asm/sysreg.h   | 23 +++++++++++++++++++
>>>  arch/arm64/kvm/hyp/entry.S        |  2 --
>>>  arch/arm64/kvm/hyp/switch.c       | 24 ++++++++++++++++++--
>>>  arch/arm64/kvm/hyp/sysreg-sr.c    | 48 ++++++++++++++++++++++++++++++++++++---
>>>  6 files changed, 100 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>>> index e923b58..cba7d3e 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -193,6 +193,12 @@ struct kvm_cpu_context {
>>>  	};
>>>  };
>>>  
>>> +struct kvm_cpu_host_pstate {
>>> +	u64 daif;
>>> +	u64 uao;
>>> +	u64 pan;
>>> +};
>>
>> I love it. This is the most expensive way of saving/restoring a single
>> 32bit value.
>>
>> More seriously, please see the discussion between James and Christoffer
>> there[1]. I expect James to address the PAN/UAO states together with the
>> debug state in the next iteration of his patch.
> 
> I roughly see the discussion between James and Christoffer, Seems Christoffer does not suggest save and
> restore it, and suggest to do below, and UAO/PAN may not use the same ways.
> 
>       __kvm_vcpu_run(struct kvm_vcpu *vcpu)
>       {
>           if (has_vhe())
>               asm("msr     daifset, #0xf");
> 
>  	...
>          exit_code = __guest_enter(vcpu, host_ctxt);
>  	...
> 
>  	if (has_vhe())
>               asm("msr     daifclr, #0xd");
>       }
> 
> 
> If not save/restore them, the KVM will set them according to the CPU capability. For example below fixing, it will check CPU capability, If CPU supports PAN,
> the kvm will always enable the PAN for the host. But in fact, the host may be not enable the PAN.
> Of course for the UAO, we can use the similar fixing if Marc or Christoffer is agreed. but seems not make sense.
> 
> commit cb96408da4e11698674abd04aeac941c1bed2038
> Author: Vladimir Murzin <vladimir.murzin@arm.com>
> Date:   Thu Sep 1 15:29:03 2016 +0100
> 
>     arm64: KVM: VHE: reset PSTATE.PAN on entry to EL2
> 
>     SCTLR_EL2.SPAN bit controls what happens with the PSTATE.PAN bit on an
>     exception. However, this bit has no effect on the PSTATE.PAN when
>     HCR_EL2.E2H or HCR_EL2.TGE is unset. Thus when VHE is used and
>     exception taken from a guest PSTATE.PAN bit left unchanged and we
>     continue with a value guest has set.
> 
>     To address that always reset PSTATE.PAN on entry from EL1.
> 
>     Fixes: 1f364c8c48a0 ("arm64: VHE: Add support for running Linux in EL2 mode")
> 
>     Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>     Reviewed-by: James Morse <james.morse@arm.com>
>     Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>     Cc: <stable@vger.kernel.org> # v4.6+
>     Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
> 
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 3967c231..b5926ee 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,6 +96,8 @@ ENTRY(__guest_exit)
> 
>         add     x1, x1, #VCPU_CONTEXT
> 
> +       ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +
>         // Store the guest regs x2 and x3
>         stp     x2, x3,   [x1, #CPU_XREG_OFFSET(2)]
> 
> 
>>
>> Thanks,
>>
>> 	M.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg599798.html
>>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web