[Qemu-devel] [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Qemu-devel] [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2787 bytes --]

This patch aligns the KVM-related layout and encoding of the CPU state
to be saved to disk or migrated with qemu-kvm. The major differences are
reordering of fields and a compressed interrupt_bitmap into a single
number as there can be no more than one pending IRQ at a time.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 target-i386/machine.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/target-i386/machine.c b/target-i386/machine.c
index bb8b9db..259302c 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -28,7 +28,8 @@ void cpu_save(QEMUFile *f, void *opaque)
     uint16_t fptag, fpus, fpuc, fpregs_format;
     uint32_t hflags;
     int32_t a20_mask;
-    int i;
+    int32_t pending_irq;
+    int i, bit;
 
     cpu_synchronize_state(env, 0);
 
@@ -141,11 +142,21 @@ void cpu_save(QEMUFile *f, void *opaque)
         qemu_put_be64s(f, &env->mtrr_var[i].mask);
     }
 
-    for (i = 0; i < sizeof(env->interrupt_bitmap)/8; i++) {
-        qemu_put_be64s(f, &env->interrupt_bitmap[i]);
+    /* KVM-related states */
+
+    /* There can only be one pending IRQ set in the bitmap at a time, so try
+       to find it and save its number instead (-1 for none). */
+    pending_irq = -1;
+    for (i = 0; i < ARRAY_SIZE(env->interrupt_bitmap); i++) {
+        bit = ffsll(env->interrupt_bitmap[i]);
+        if (bit) {
+            pending_irq = i * 64 + bit - 1;
+            break;
+        }
     }
-    qemu_put_be64s(f, &env->tsc);
+    qemu_put_sbe32s(f, &pending_irq);
     qemu_put_be32s(f, &env->mp_state);
+    qemu_put_be64s(f, &env->tsc);
 }
 
 #ifdef USE_X86LDOUBLE
@@ -179,6 +190,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     uint32_t hflags;
     uint16_t fpus, fpuc, fptag, fpregs_format;
     int32_t a20_mask;
+    int32_t pending_irq;
 
     if (version_id < 3 || version_id > CPU_SAVE_VERSION)
         return -EINVAL;
@@ -324,12 +336,16 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
             qemu_get_be64s(f, &env->mtrr_var[i].mask);
         }
     }
+
     if (version_id >= 9) {
-        for (i = 0; i < sizeof(env->interrupt_bitmap)/8; i++) {
-            qemu_get_be64s(f, &env->interrupt_bitmap[i]);
+        qemu_get_sbe32s(f, &pending_irq);
+        memset(&env->interrupt_bitmap, 0, sizeof(env->interrupt_bitmap));
+        if (pending_irq >= 0) {
+            env->interrupt_bitmap[pending_irq / 64] |=
+                (uint64_t)1 << (pending_irq % 64);
         }
-        qemu_get_be64s(f, &env->tsc);
         qemu_get_be32s(f, &env->mp_state);
+        qemu_get_be64s(f, &env->tsc);
     }
 
     /* XXX: ensure compatiblity for halted bit ? */


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Blue Swirl]

On 6/27/09, Jan Kiszka <jan.kiszka@web.de> wrote:
> This patch aligns the KVM-related layout and encoding of the CPU state
>  to be saved to disk or migrated with qemu-kvm. The major differences are
>  reordering of fields and a compressed interrupt_bitmap into a single
>  number as there can be no more than one pending IRQ at a time.

This changes the savevm format, shouldn't you bump the version?

[Qemu-devel] Re: [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

Blue Swirl wrote:
> On 6/27/09, Jan Kiszka <jan.kiszka@web.de> wrote:
>> This patch aligns the KVM-related layout and encoding of the CPU state
>>  to be saved to disk or migrated with qemu-kvm. The major differences are
>>  reordering of fields and a compressed interrupt_bitmap into a single
>>  number as there can be no more than one pending IRQ at a time.
> 
> This changes the savevm format, shouldn't you bump the version?

(Hmm, kind of FAQ. I should have included this in the changelog...)

No, it's a correction of f8d926e9cd. And there was no official version
released with this layout.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Anthony Liguori]

Blue Swirl wrote:
> On 6/27/09, Jan Kiszka <jan.kiszka@web.de> wrote:
>   
>> This patch aligns the KVM-related layout and encoding of the CPU state
>>  to be saved to disk or migrated with qemu-kvm. The major differences are
>>  reordering of fields and a compressed interrupt_bitmap into a single
>>  number as there can be no more than one pending IRQ at a time.
>>     
>
> This changes the savevm format, shouldn't you bump the version?
>   

Practically speaking, it's been bumped for KVM already in this release 
cycle so changing the format without bumping the version should be 
okay.  It may interfere with kvm-XX releases but I'm not sure whether 
that's an issue for us and how we should handle that long term.

I'm assuming Jan thought all of this through ahead of time and just 
forgot to explicitly state the reasoning for not bumping the version :-)

Regards,

Anthony Liguori

Re: [Qemu-devel] [RESEND][PATCH] KVM: x86: Refactor persistent CPU state

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Anthony Liguori wrote:
> Blue Swirl wrote:
>> On 6/27/09, Jan Kiszka <jan.kiszka@web.de> wrote:
>>  
>>> This patch aligns the KVM-related layout and encoding of the CPU state
>>>  to be saved to disk or migrated with qemu-kvm. The major differences
>>> are
>>>  reordering of fields and a compressed interrupt_bitmap into a single
>>>  number as there can be no more than one pending IRQ at a time.
>>>     
>>
>> This changes the savevm format, shouldn't you bump the version?
>>   
> 
> Practically speaking, it's been bumped for KVM already in this release
> cycle so changing the format without bumping the version should be
> okay.  It may interfere with kvm-XX releases but I'm not sure whether
> that's an issue for us and how we should handle that long term.

That particular patch is already part of kvm-87. Here we are just
aligning both code bases, and that even on the same format version number.

> 
> I'm assuming Jan thought all of this through ahead of time and just
> forgot to explicitly state the reasoning for not bumping the version :-)

Of course, I considered every possible case in every detail. :)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]