On Fri, Aug 01, 2014 at 10:11:52AM +0100, Alex Benn?e wrote:

Christoffer Dall writes:

quoted

On Thu, Jul 31, 2014 at 04:14:51PM +0100, Alex Benn?e wrote:

quoted

Christoffer Dall writes:

quoted

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

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.

the KVM_REG_ARM_COPROC_MASK is part of the uapi IIRC, so that's not the
issue, but that mask doesn't cover all the upper bits, so it feels weird
to use that to me.

Yeah I missed that. I could do a:

#define KVM_REG_ARM_COPROC_INDEX_MASK   ((1<<KVM_REG_ARM_COPROC_SHIFT)-1)

and use that. I'm generally try to avoid hardcoded numbers but I could
be being a little OCD here ;-)

quoted

quoted

quoted

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

Same patch? Sure.

really up to you if you want to split it up into two patches, but I
think it's small enough that you can just create one patch.

Given the similarity of this code between arm and arm64 I'm wondering if
it's worth doing a arch/arm/kvm/guest_common.c or something to reduce
the amount of copy paste stuff?

We've gotten by without it so far.  I fear we end up with a bunch of
complications due to differences in sizeof(unsigned long) etc., but I
may be wrong.

The amount of code that is copied should be trivial boilerplate stuff,
but if you think it's worth unifying, then I'd be happy to review the
patch.

Thanks,
-Christoffer