* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-05 18:58 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-05 18:58 UTC (permalink / raw)
To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
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
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-05 18:58 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-05 18:58 UTC (permalink / raw)
To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
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
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-05 18:58 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-05 18:58 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-05 18:58 ` gengdongjiu
(?)
(?)
@ 2017-09-06 5:26 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 5:26 UTC (permalink / raw)
To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
catalin.marinas, james.morse, mark.rutland, suzuki.poulose,
Will Deacon
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;
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 5:26 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 5:26 UTC (permalink / raw)
To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
catalin.marinas, james.morse, mark.rutland, suzuki.poulose,
Will Deacon
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;
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 5:26 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 5:26 UTC (permalink / raw)
To: christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
catalin.marinas, james.morse, mark.rutland, suzuki.poulose,
Will Deacon
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;
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 5:26 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 5:26 UTC (permalink / raw)
To: linux-arm-kernel
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;
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-05 18:58 ` gengdongjiu
(?)
@ 2017-09-06 8:17 ` Marc Zyngier
-1 siblings, 0 replies; 62+ messages in thread
From: Marc Zyngier @ 2017-09-06 8:17 UTC (permalink / raw)
To: gengdongjiu, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
Cc: James Morse
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...
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 8:17 ` Marc Zyngier
0 siblings, 0 replies; 62+ messages in thread
From: Marc Zyngier @ 2017-09-06 8:17 UTC (permalink / raw)
To: gengdongjiu, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
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...
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 8:17 ` Marc Zyngier
0 siblings, 0 replies; 62+ messages in thread
From: Marc Zyngier @ 2017-09-06 8:17 UTC (permalink / raw)
To: linux-arm-kernel
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...
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 8:17 ` Marc Zyngier
(?)
(?)
@ 2017-09-06 9:32 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:32 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:32 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:32 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:32 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 9:32 ` gengdongjiu
(?)
@ 2017-09-06 9:41 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 9:41 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:41 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 9:41 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:41 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 9:41 UTC (permalink / raw)
To: linux-arm-kernel
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
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 9:32 ` gengdongjiu
(?)
(?)
@ 2017-09-06 9:49 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:49 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:49 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:49 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:49 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:49 UTC (permalink / raw)
To: Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 9:49 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 9:49 UTC (permalink / raw)
To: linux-arm-kernel
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
>>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 9:41 ` Vladimir Murzin
(?)
(?)
@ 2017-09-06 10:35 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 10:35 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 10:35 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 10:35 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 10:35 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 10:35 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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.
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 10:35 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 10:35 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 10:35 ` gengdongjiu
(?)
@ 2017-09-06 12:00 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:00 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:00 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:00 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:00 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:00 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 12:00 ` Vladimir Murzin
(?)
(?)
@ 2017-09-06 12:14 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:14 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:14 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:14 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:14 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:14 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:14 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:14 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 12:14 ` gengdongjiu
(?)
@ 2017-09-06 12:30 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:30 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:30 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:30 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:30 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 12:30 UTC (permalink / raw)
To: linux-arm-kernel
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
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 12:00 ` Vladimir Murzin
(?)
(?)
@ 2017-09-06 12:32 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:32 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>
> .
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:32 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>
> .
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:32 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>
> .
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:32 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:32 UTC (permalink / raw)
To: linux-arm-kernel
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
>
> .
>
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 12:30 ` Vladimir Murzin
(?)
(?)
@ 2017-09-06 12:44 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:44 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>>
>>> .
>>>
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:44 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:44 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>>>
>>> .
>>>
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:44 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:44 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, James Morse, zhanghaibin7
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
>>>
>>> .
>>>
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 12:44 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 12:44 UTC (permalink / raw)
To: linux-arm-kernel
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
>>>
>>> .
>>>
>>
>>
>
>
> .
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 12:44 ` gengdongjiu
(?)
@ 2017-09-06 13:00 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 13:00 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, zhanghaibin7, Huangshaoyu
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
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 13:00 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 13:00 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: Huangshaoyu, zhanghaibin7
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
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 13:00 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 13:00 UTC (permalink / raw)
To: linux-arm-kernel
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
>>>>
>>>> .
>>>>
>>>
>>>
>>
>>
>> .
>>
>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-05 18:58 ` gengdongjiu
(?)
@ 2017-09-06 20:49 ` kbuild test robot
-1 siblings, 0 replies; 62+ messages in thread
From: kbuild test robot @ 2017-09-06 20:49 UTC (permalink / raw)
To: gengdongjiu
Cc: kbuild-all, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Hi gengdongjiu,
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13 next-20170906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/gengdongjiu/arm64-KVM-VHE-save-and-restore-some-PSTATE-bits/20170907-013418
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
/tmp/ccT6UpbP.s: Assembler messages:
>> /tmp/ccT6UpbP.s:1415: Error: constant expression required
/tmp/ccT6UpbP.s:1439: Error: constant expression required
>> /tmp/ccT6UpbP.s:1420: Error: attempt to move .org backwards
/tmp/ccT6UpbP.s:1444: Error: attempt to move .org backwards
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36347 bytes --]
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 20:49 ` kbuild test robot
0 siblings, 0 replies; 62+ messages in thread
From: kbuild test robot @ 2017-09-06 20:49 UTC (permalink / raw)
To: gengdongjiu
Cc: kbuild-all, christoffer.dall, marc.zyngier, pbonzini, rkrcmar,
vladimir.murzin, linux-arm-kernel, kvmarm, kvm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
Hi gengdongjiu,
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13 next-20170906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/gengdongjiu/arm64-KVM-VHE-save-and-restore-some-PSTATE-bits/20170907-013418
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
/tmp/ccT6UpbP.s: Assembler messages:
>> /tmp/ccT6UpbP.s:1415: Error: constant expression required
/tmp/ccT6UpbP.s:1439: Error: constant expression required
>> /tmp/ccT6UpbP.s:1420: Error: attempt to move .org backwards
/tmp/ccT6UpbP.s:1444: Error: attempt to move .org backwards
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36347 bytes --]
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 20:49 ` kbuild test robot
0 siblings, 0 replies; 62+ messages in thread
From: kbuild test robot @ 2017-09-06 20:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi gengdongjiu,
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13 next-20170906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/gengdongjiu/arm64-KVM-VHE-save-and-restore-some-PSTATE-bits/20170907-013418
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
/tmp/ccT6UpbP.s: Assembler messages:
>> /tmp/ccT6UpbP.s:1415: Error: constant expression required
/tmp/ccT6UpbP.s:1439: Error: constant expression required
>> /tmp/ccT6UpbP.s:1420: Error: attempt to move .org backwards
/tmp/ccT6UpbP.s:1444: Error: attempt to move .org backwards
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 36347 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170907/e1a08923/attachment-0001.gz>
^ permalink raw reply [flat|nested] 62+ messages in thread
* re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 22:09 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 22:09 UTC (permalink / raw)
To: vladimir.murzin, marc.zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, Catalin Marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
[-- Attachment #1.1: Type: text/plain, Size: 3860 bytes --]
The negative effect is that kernel can not access kernel space adress through copy_to/from_user even KERNEL_DS is set
发件人：vladimir.murzin
收件人：耿东久,marc.zyngier,christoffer.dall,pbonzini,rkrcmar,linux-arm-kernel,kvmarm,kvm,linux-kernel,suzuki.poulose,mark.rutland,Catalin Marinas
抄送：James.Morse,张海斌,黄韶宇
时间：2017-09-06 23:19:38
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN，when the guest traps to el2，it will reset the pstate.PAN <http://pstate.PAN> to 0, and continue run。In fact the host pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2。so after swich to host，need to check whether set pstate.UAO <http://pstate.UAO> again。
What negative effect do you see with change UAO to 0 (i.e. do not override)?
P. S.
Please, avoid HTML and top posting
Vladimir
> *发件人：*Vladimir Murzin
> *收件人：*耿东久,marc.zyngier,christoffer.dall@linaro.org,pbonzini@redhat.com,rkrcmar@redhat.com,linux-arm-kernel@lists.infradead.org,kvmarm@lists.cs.columbia.edu,kvm@vger.kernel.org,linux-kernel,suzuki.poulose@arm.com,mark.rutland@arm.com,Catalin Marinas
> *抄送：*James Morse,张海斌,黄韶宇
> *时间：*2017-09-06 22:41:09
> *主题:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
>
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>>
>>>>> 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...
>>
>>
>> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
>
> It would help if you give precise description on "pstate.UAO issue".
>
> Thanks
> Vladimir
>
>>
>> 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
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>
>
[-- Attachment #1.2: Type: text/html, Size: 6823 bytes --]
[-- Attachment #2: Type: text/plain, Size: 151 bytes --]
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 62+ messages in thread
* re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 22:09 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 22:09 UTC (permalink / raw)
To: vladimir.murzin, marc.zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, Catalin Marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
[-- Attachment #1.1: Type: text/plain, Size: 3860 bytes --]
The negative effect is that kernel can not access kernel space adress through copy_to/from_user even KERNEL_DS is set
发件人：vladimir.murzin
收件人：耿东久,marc.zyngier,christoffer.dall,pbonzini,rkrcmar,linux-arm-kernel,kvmarm,kvm,linux-kernel,suzuki.poulose,mark.rutland,Catalin Marinas
抄送：James.Morse,张海斌,黄韶宇
时间：2017-09-06 23:19:38
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN，when the guest traps to el2，it will reset the pstate.PAN <http://pstate.PAN> to 0, and continue run。In fact the host pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2。so after swich to host，need to check whether set pstate.UAO <http://pstate.UAO> again。
What negative effect do you see with change UAO to 0 (i.e. do not override)?
P. S.
Please, avoid HTML and top posting
Vladimir
> *发件人：*Vladimir Murzin
> *收件人：*耿东久,marc.zyngier,christoffer.dall@linaro.org,pbonzini@redhat.com,rkrcmar@redhat.com,linux-arm-kernel@lists.infradead.org,kvmarm@lists.cs.columbia.edu,kvm@vger.kernel.org,linux-kernel,suzuki.poulose@arm.com,mark.rutland@arm.com,Catalin Marinas
> *抄送：*James Morse,张海斌,黄韶宇
> *时间：*2017-09-06 22:41:09
> *主题:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
>
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>>
>>>>> 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...
>>
>>
>> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
>
> It would help if you give precise description on "pstate.UAO issue".
>
> Thanks
> Vladimir
>
>>
>> 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
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>
>
[-- Attachment #1.2: Type: text/html, Size: 6823 bytes --]
[-- Attachment #2: Type: text/plain, Size: 151 bytes --]
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 15:08 ` gengdongjiu
@ 2017-09-06 15:19 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 15:19 UTC (permalink / raw)
To: gengdongjiu, marc.zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, Catalin Marinas
Cc: James.Morse, Zhanghaibin (Euler), Huangshaoyu
On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN，when the guest traps to el2，it will reset the pstate.PAN <http://pstate.PAN> to 0, and continue run。In fact the host pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2。so after swich to host，need to check whether set pstate.UAO <http://pstate.UAO> again。
What negative effect do you see with change UAO to 0 (i.e. do not override)?
P. S.
Please, avoid HTML and top posting
Vladimir
> *发件人：*Vladimir Murzin
> *收件人：*耿东久,marc.zyngier,christoffer.dall@linaro.org,pbonzini@redhat.com,rkrcmar@redhat.com,linux-arm-kernel@lists.infradead.org,kvmarm@lists.cs.columbia.edu,kvm@vger.kernel.org,linux-kernel,suzuki.poulose@arm.com,mark.rutland@arm.com,Catalin Marinas
> *抄送：*James Morse,张海斌,黄韶宇
> *时间：*2017-09-06 22:41:09
> *主题:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
>
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>>
>>>>> 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...
>>
>>
>> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
>
> It would help if you give precise description on "pstate.UAO issue".
>
> Thanks
> Vladimir
>
>>
>> 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
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 15:19 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 15:19 UTC (permalink / raw)
To: linux-arm-kernel
On 06/09/17 16:08, gengdongjiu wrote:
> It is similar with the PAN?when the guest traps to el2?it will reset the pstate.PAN <http://pstate.PAN> to 0, and continue run?In fact the host pstate.UAO <http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2?so after swich to host?need to check whether set pstate.UAO <http://pstate.UAO> again?
What negative effect do you see with change UAO to 0 (i.e. do not override)?
P. S.
Please, avoid HTML and top posting
Vladimir
> *????*Vladimir Murzin
> *????*???,marc.zyngier,christoffer.dall at linaro.org,pbonzini at redhat.com,rkrcmar at redhat.com,linux-arm-kernel at lists.infradead.org,kvmarm at lists.cs.columbia.edu,kvm at vger.kernel.org,linux-kernel,suzuki.poulose at arm.com,mark.rutland at arm.com,Catalin Marinas
> *???*James Morse,???,???
> *???*2017-09-06 22:41:09
> *??:*Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
>
> On 06/09/17 15:10, gengdongjiu wrote:
>> Hi, Vladimir
>>
>>>>> 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...
>>
>>
>> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
>> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
>
> It would help if you give precise description on "pstate.UAO issue".
>
> Thanks
> Vladimir
>
>>
>> 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
>>>
>>>>
>>>>
>>>>>
>>>>> Cheers
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Cheers
>>>>>>> Vladimir
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>>
>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 14:40 ` Vladimir Murzin
@ 2017-09-06 15:08 ` gengdongjiu
-1 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 15:08 UTC (permalink / raw)
To: vladimir.murzin, marc.zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, Catalin Marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]
It is similar with the PAN，when the guest traps to el2，it will reset the pstate.PAN<http://pstate.PAN> to 0, and continue run。In fact the host pstate.UAO<http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2。so after swich to host，need to check whether set pstate.UAO<http://pstate.UAO> again。
发件人：Vladimir Murzin
收件人：耿东久,marc.zyngier,christoffer.dall@linaro.org,pbonzini@redhat.com,rkrcmar@redhat.com,linux-arm-kernel@lists.infradead.org,kvmarm@lists.cs.columbia.edu,kvm@vger.kernel.org,linux-kernel,suzuki.poulose@arm.com,mark.rutland@arm.com,Catalin Marinas
抄送：James Morse,张海斌,黄韶宇
时间：2017-09-06 22:41:09
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
[-- Attachment #1.2: Type: text/html, Size: 5398 bytes --]
[-- Attachment #2: Type: text/plain, Size: 151 bytes --]
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 62+ messages in thread
* re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 15:08 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 15:08 UTC (permalink / raw)
To: vladimir.murzin, marc.zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, Catalin Marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]
It is similar with the PAN，when the guest traps to el2，it will reset the pstate.PAN<http://pstate.PAN> to 0, and continue run。In fact the host pstate.UAO<http://pstate.UAO> can be 1, but guest change it to 0 when trap to EL2。so after swich to host，need to check whether set pstate.UAO<http://pstate.UAO> again。
发件人：Vladimir Murzin
收件人：耿东久,marc.zyngier,christoffer.dall@linaro.org,pbonzini@redhat.com,rkrcmar@redhat.com,linux-arm-kernel@lists.infradead.org,kvmarm@lists.cs.columbia.edu,kvm@vger.kernel.org,linux-kernel,suzuki.poulose@arm.com,mark.rutland@arm.com,Catalin Marinas
抄送：James Morse,张海斌,黄韶宇
时间：2017-09-06 22:41:09
主题:Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
[-- Attachment #1.2: Type: text/html, Size: 5398 bytes --]
[-- Attachment #2: Type: text/plain, Size: 151 bytes --]
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
2017-09-06 14:10 ` gengdongjiu
(?)
(?)
@ 2017-09-06 14:40 ` Vladimir Murzin
-1 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 14:40 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: James Morse, Zhanghaibin (Euler), Huangshaoyu
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:40 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 14:40 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:40 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 14:40 UTC (permalink / raw)
To: gengdongjiu, Marc Zyngier, christoffer.dall, pbonzini, rkrcmar,
linux-arm-kernel, kvmarm, kvm, linux-kernel, suzuki.poulose,
mark.rutland, catalin.marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:40 ` Vladimir Murzin
0 siblings, 0 replies; 62+ messages in thread
From: Vladimir Murzin @ 2017-09-06 14:40 UTC (permalink / raw)
To: linux-arm-kernel
On 06/09/17 15:10, gengdongjiu wrote:
> Hi, Vladimir
>
>>>> 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...
>
>
> If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
> For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
It would help if you give precise description on "pstate.UAO issue".
Thanks
Vladimir
>
> 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
>>
>>>
>>>
>>>>
>>>> Cheers
>>>> Vladimir
>>>>
>>>>>
>>>>>>
>>>>>> Cheers
>>>>>> Vladimir
>>>>>>
>>>>>> .
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:10 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 14:10 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: James Morse, Zhanghaibin (Euler), Huangshaoyu
Hi, Vladimir
> >> 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...
If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
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
>
> >
> >
> >>
> >> Cheers
> >> Vladimir
> >>
> >>>
> >>>>
> >>>> Cheers
> >>>> Vladimir
> >>>>
> >>>> .
> >>>>
> >>>
> >>>
> >>
> >>
> >> .
> >>
> >
> >
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:10 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 14:10 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
Hi, Vladimir
> >> 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...
If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
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
>
> >
> >
> >>
> >> Cheers
> >> Vladimir
> >>
> >>>
> >>>>
> >>>> Cheers
> >>>> Vladimir
> >>>>
> >>>> .
> >>>>
> >>>
> >>>
> >>
> >>
> >> .
> >>
> >
> >
^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits
@ 2017-09-06 14:10 ` gengdongjiu
0 siblings, 0 replies; 62+ messages in thread
From: gengdongjiu @ 2017-09-06 14:10 UTC (permalink / raw)
To: Vladimir Murzin, Marc Zyngier, christoffer.dall, pbonzini,
rkrcmar, linux-arm-kernel, kvmarm, kvm, linux-kernel,
suzuki.poulose, mark.rutland, catalin.marinas
Cc: Huangshaoyu, Zhanghaibin (Euler)
Hi, Vladimir
> >> 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...
If you think that makes sense for it, we do not consider the paste.PAN in the world-switch.
For the pstate.UAO issue, do you agree my fixing or you have other suggestion? Also to other reviewer. Thanks.
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
>
> >
> >
> >>
> >> Cheers
> >> Vladimir
> >>
> >>>
> >>>>
> >>>> Cheers
> >>>> Vladimir
> >>>>
> >>>> .
> >>>>
> >>>
> >>>
> >>
> >>
> >> .
> >>
> >
> >
^ permalink raw reply related [flat|nested] 62+ messages in thread
end of thread, other threads:[~2017-09-06 22:11 UTC | newest]
Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 18:58 [PATCH] arm64: KVM: VHE: save and restore some PSTATE bits gengdongjiu
2017-09-05 18:58 ` gengdongjiu
2017-09-05 18:58 ` gengdongjiu
2017-09-06 5:26 ` gengdongjiu
2017-09-06 5:26 ` gengdongjiu
2017-09-06 5:26 ` gengdongjiu
2017-09-06 5:26 ` gengdongjiu
2017-09-06 8:17 ` Marc Zyngier
2017-09-06 8:17 ` Marc Zyngier
2017-09-06 8:17 ` Marc Zyngier
2017-09-06 9:32 ` gengdongjiu
2017-09-06 9:32 ` gengdongjiu
2017-09-06 9:32 ` gengdongjiu
2017-09-06 9:32 ` gengdongjiu
2017-09-06 9:41 ` Vladimir Murzin
2017-09-06 9:41 ` Vladimir Murzin
2017-09-06 9:41 ` Vladimir Murzin
2017-09-06 10:35 ` gengdongjiu
2017-09-06 10:35 ` gengdongjiu
2017-09-06 10:35 ` gengdongjiu
2017-09-06 10:35 ` gengdongjiu
2017-09-06 12:00 ` Vladimir Murzin
2017-09-06 12:00 ` Vladimir Murzin
2017-09-06 12:00 ` Vladimir Murzin
2017-09-06 12:14 ` gengdongjiu
2017-09-06 12:14 ` gengdongjiu
2017-09-06 12:14 ` gengdongjiu
2017-09-06 12:14 ` gengdongjiu
2017-09-06 12:30 ` Vladimir Murzin
2017-09-06 12:30 ` Vladimir Murzin
2017-09-06 12:30 ` Vladimir Murzin
2017-09-06 12:44 ` gengdongjiu
2017-09-06 12:44 ` gengdongjiu
2017-09-06 12:44 ` gengdongjiu
2017-09-06 12:44 ` gengdongjiu
2017-09-06 13:00 ` Vladimir Murzin
2017-09-06 13:00 ` Vladimir Murzin
2017-09-06 13:00 ` Vladimir Murzin
2017-09-06 12:32 ` gengdongjiu
2017-09-06 12:32 ` gengdongjiu
2017-09-06 12:32 ` gengdongjiu
2017-09-06 12:32 ` gengdongjiu
2017-09-06 9:49 ` gengdongjiu
2017-09-06 9:49 ` gengdongjiu
2017-09-06 9:49 ` gengdongjiu
2017-09-06 9:49 ` gengdongjiu
2017-09-06 20:49 ` kbuild test robot
2017-09-06 20:49 ` kbuild test robot
2017-09-06 20:49 ` kbuild test robot
2017-09-06 14:10 gengdongjiu
2017-09-06 14:10 ` gengdongjiu
2017-09-06 14:10 ` gengdongjiu
2017-09-06 14:40 ` Vladimir Murzin
2017-09-06 14:40 ` Vladimir Murzin
2017-09-06 14:40 ` Vladimir Murzin
2017-09-06 14:40 ` Vladimir Murzin
2017-09-06 15:08 ` gengdongjiu
2017-09-06 15:08 ` gengdongjiu
2017-09-06 15:19 ` Vladimir Murzin
2017-09-06 15:19 ` Vladimir Murzin
2017-09-06 22:09 gengdongjiu
2017-09-06 22:09 ` gengdongjiu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.