Christoffer Dall writes:

On Wed, Jul 09, 2014 at 02:55:12PM +0100, Alex Benn?e wrote:

quoted

To cleanly restore an SMP VM we need to ensure that the current pause
state of each vcpu is correctly recorded. Things could get confused if
the CPU starts running after migration restore completes when it was
paused before it state was captured.

<snip>

quoted

+/* Power state (PSCI), not real registers */
+#define KVM_REG_ARM_PSCI		(0x0014 << KVM_REG_ARM_COPROC_SHIFT)
+#define KVM_REG_ARM_PSCI_REG(n) \
+	(KVM_REG_ARM64 | KVM_REG_SIZE_U64 | KVM_REG_ARM_PSCI | \
+         (n & ~KVM_REG_ARM_COPROC_MASK))

I don't understand this mask, why isn't this
            (n & 0xffff))

I was trying to use the existing masks, but of course if anyone changes
that it would be an ABI change so probably not worth it.

quoted

+#define KVM_REG_ARM_PSCI_STATE          KVM_REG_ARM_PSCI_REG(0)
+#define NUM_KVM_PSCI_REGS               1
+

you're missing updates to Documentation/virtual/kvm/api.txt here.

Will add.

quoted

 /* Device Control API: ARM VGIC */
 #define KVM_DEV_ARM_VGIC_GRP_ADDR	0
 #define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 205f0d8..31d6439 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c

@@ -189,6 +189,54 @@ static int get_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 }
 
 /**
+ * PSCI State
+ *
+ * These are not real registers as they do not actually exist in the
+ * hardware but represent the current power state of the vCPU

full stop

quoted

+ */
+
+static bool is_psci_reg(u64 index)
+{
+	switch (index) {
+	case KVM_REG_ARM_PSCI_STATE:
+		return true;
+	}
+	return false;
+}
+
+static int copy_psci_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
+{
+	if (put_user(KVM_REG_ARM_PSCI_STATE, uindices))
+		return -EFAULT;
+	return 0;
+}
+
+static int set_psci_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
+{
+	void __user *uaddr = (void __user *)(long)reg->addr;
+	u64 val;
+	int ret;
+
+	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
+	if (ret != 0)
+		return ret;
+
+        vcpu->arch.pause = (val & 0x1) ? false : true;

tabs

Hmmm, apparently the GNU Emacs "linux" style doesn't actually enforce
that. Who knew? I'll beat the editor into submission.

I really need the documentation of the ABI, why is bit[0] == 1 not
paused?

I figured 1 == running, but I can switch it round if you want to to map
directly to the .pause flag.

If we are not complaining when setting the pause value to false if it
was true before, then we probably also need to wake up the thread in
case this is called from another thread, right?

or perhaps we should just return an error if you're trying to un-pause a
CPU through this interface, hmmmm.

Wouldn't it be an error to mess with any register when the system is not
in a quiescent state? I was assuming that the wake state is dealt with
when the run loop finally restarts. 

<snip>

please check for use of tabs versus spaces, checkpatch.pl should
complain.

Can you add the 32-bit counterpart as part of this patch?

Same patch? Sure.

Thanks,
-Christoffer

-- 
Alex Benn?e