Re: [Qemu-devel] [Query] Live Migration between machines with different processor ids

[Jaggi, Manish]

> On 04-Sep-2018, at 3:24 PM, Andrew Jones <address@hidden> wrote:
> 
> External Email
> 
> On Tue, Sep 04, 2018 at 09:16:58AM +0000, Jaggi, Manish wrote:
>>> On 31-Aug-2018, at 4:41 PM, Andrew Jones <address@hidden> wrote:
>>> I think the sequence should look something like this:
>>> 
>>> 1) Guest running on Host A with processor a
>>> 2) Stop guest and save its state for migration
>>> 3) Migrate guest to Host B with processor b (b is "close enough" to a)
>>> 4) Restore guest state after migration
>>>    If guest is running with '-cpu host'
>>>      4.a) Inform KVM of any configuration that impacts invariant registers
>>>      4.b) Update the guest's view of all invariant registers to match the
>>>           host
>>>    EndIf
>>> 5) Run guest
>>> 
>>> 4.a and 4.b require new code both in QEMU and KVM. 4.a may require a new
>>> KVM API, unless the existing API can be leveraged.
>>> 
>>> The definition of "close enough" is left to the users and/or higher layers
>>> of the Virt stack.
>>> 
>> 
>> Thanks for detailing the sequence.
>> I got another comment from Juan and David which is not to use -cpu host, see 
>> below
>> 
>> "I really think that the right approach here is not using -cpu host.  You
>> do the full work, create a model as David says, and be sure that you car
>> run that model on both cpus.  It is a lot of work, but it is the only
>> way to make sure that this is going to work long term.”
>> 
>> Not using -cpu host is orthogonal to the sequence we have been discussing in 
>> this thread.
>> Use something like -cpu cortex-a57 (this however does not work so far)
>> This would avoid close-enough definition, but would need substantial work.
>> 
>> So which approach should be taken here, whats your take...
>> 
> 
> Inventing a base-AArch64 cpu model that can then be extended with optional
> features is a nice way to extend the migratability of a guest, however
> it's hard to do because of errata. Since errata workarounds are enabled
> per MIDR, then we'd need to invent our own MIDR and also some way to
> communicate which errata we want to enable, possibly through some paravirt
> mechanism or through some implementation defined system registers that
> KVM would need to reserve and define.
> 
> That's not just a ton of work for the entire virt stack (not just KVM and
> QEMU, but also all the layers above), but it's possible that it won't be
> useful in the end anyway. There's risk that enabling just one erratum
> workaround would restrict the guest to hosts of the exact same type
> anyway. For each erratum that needs to be enabled, the probability of
> enabling an incompatible one goes up, so it may not be likely to do much
> better than '-cpu host' in the end. I'm afraid that until errata are
> primarily showing up in optional CPU features that can simply be disabled
> for the workaround, that we're stuck with '-cpu host'. I'd be happy to
> discuss it more though.
> 
> In short, I'd go with the proposal above, for now, with possibly one
> change. libvirt folk (Andrea Bolognani and Pino Toscano) suggest that
> the guest invariant register updating on the destination host only be
> done if the user opts-in to it. This is because right now if a user
> tries to migrate to a host that is not 100% identical the migration
> will fail, which makes the "mistake" clear. If we silently change the
> behavior to allow it, then what could have been a mistake, because
> the hosts aren't actually "close enough", may go unnoticed. I'm not
> 100% sure we need another user opt-in flag to be set, though, as I
> think the '-cpu host' indicates the user expects the VCPU to look
> like the host CPU, and even after migration that expectation should be
> met. Simply, users that migrate '-cpu host' VMs need to know what they're
> doing.
> 

Followup to the sequence you have outlined, I have added below pseudocode,
This is only for processors which differ in invariant registers like MIDR but
have no underlying CPU/GIC changes or any errata that affects guest kernel.
And user opts in for it. 
(not sending full patch just to check if the logic is ok..)

(a) Changes in KVM:

- Introducing a specific error code (KVM_EINVARIANT) in case of invariant 
writes.
This should not change anything to API SET_ONE_REG KVM API.
Not sure which is the best place to put the define…
I have added in include/uapi/linux/kvm_para.h.

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 22fbbdbece3c..c8a4fbe8a8bb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1111,7 +1111,7 @@ static int __set_id_reg(const struct sys_reg_desc *rd, 
void __user *uaddr,
 
        /* This is what we mean by invariant: you can't change it. */
        if (val != read_id_reg(rd, raz))
-               return -EINVAL;
+               return -KVM_EINVARIANT;
 
        return 0;
 }
diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.h
index 6c0ce49931e5..3a49a321d0df 100644
--- a/include/uapi/linux/kvm_para.h
+++ b/include/uapi/linux/kvm_para.h
@@ -17,6 +17,7 @@
 #define KVM_E2BIG              E2BIG
 #define KVM_EPERM              EPERM
 #define KVM_EOPNOTSUPP         95
+#define KVM_EINVARIANT          96
 
 #define KVM_HC_VAPIC_POLL_IRQ          1
 #define KVM_HC_MMU_OP                  2

(b) Changes in Qemu code

1. Handling of new error code, which would update
guest state with hosts invariant reg values.

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 65f867d569..0cf14323a2 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -452,7 +452,15 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
             abort();
         }
         ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
-        if (ret) {
+        if (ignore_invariant && (ret == -KVM_EINVARIANT)) {
+            /* Update Guest invariant to match with migrated host regs*/
+            ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &r);
+            if (ret)
+                ok = false;
+            else
+                cpu->cpreg_values[i] = r.addr;
+        }
+        else if (ret) {
             /* We might fail for "unknown register" and also for
              * "you tried to set a register which is constant with
              * a different value from what it actually contains".

2. ignore_invariant is the flag I was referring to which is what you also
mentioned opt-in. This can be supplied as a command line parameter to qemu on 
Machine B.

PS: I will add code to put warning logs as suggested by Dave.

-manish


> Thanks,
> drew