Thread (14 messages) flat view 14 messages, 4 authors, 2017-09-06

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

From: gengdongjiu <hidden>
Date: 2017-09-06 09:51:03
Also in: kvm, kvmarm, lkml
Subsystem: arm64 port (aarch64 architecture), kernel virtual machine for arm64 (kvm/arm64), the rest · Maintainers: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton, Linus Torvalds

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:
quoted hunk ↗ jump to hunk
Hi Marc,

On 2017/9/6 16:17, Marc Zyngier wrote:
quoted
On 05/09/17 19:58, gengdongjiu wrote:
quoted
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 <redacted>
Signed-off-by: Haibin Zhang <redacted>
---
 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 [off-list ref]
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 [off-list ref]
    Reviewed-by: James Morse [off-list ref]
    Acked-by: Marc Zyngier [off-list ref]
    Cc: [off-list ref] # v4.6+
    Signed-off-by: Christoffer Dall [off-list ref]
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)]

quoted
Thanks,

	M.

[1] https://www.spinics.net/lists/arm-kernel/msg599798.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help