diff mbox

[PATCHv2,7/7] openpic: fix up loadvm under -M mac99

Message ID 1421856072-25026-8-git-send-email-mark.cave-ayland@ilande.co.uk
State New
Headers show

Commit Message

Mark Cave-Ayland Jan. 21, 2015, 4:01 p.m. UTC
Issuing loadvm under -M mac99 would fail for two reasons: firstly an incorrect
version number for openpic would cause openpic_load() to abort, and secondly
a cut/paste error when restoring the IVPR and IDR registers caused subsequent
vmstate sections to become misaligned and abort early.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/intc/openpic.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Alexander Graf Jan. 22, 2015, 1:39 p.m. UTC | #1
On 21.01.15 17:01, Mark Cave-Ayland wrote:
> Issuing loadvm under -M mac99 would fail for two reasons: firstly an incorrect
> version number for openpic would cause openpic_load() to abort, and secondly
> a cut/paste error when restoring the IVPR and IDR registers caused subsequent
> vmstate sections to become misaligned and abort early.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Thanks a lot for all the work you put into this. Do you think you
understand enough to the OpenPIC code by now to be able to convert it to
VMState instead?

That would get rid of the whole class of problems altogether and make
sure we didn't overlook something subtle somewhere else in the code.


Alex
Mark Cave-Ayland Jan. 26, 2015, 10:13 p.m. UTC | #2
On 22/01/15 13:39, Alexander Graf wrote:

> On 21.01.15 17:01, Mark Cave-Ayland wrote:
>> Issuing loadvm under -M mac99 would fail for two reasons: firstly an incorrect
>> version number for openpic would cause openpic_load() to abort, and secondly
>> a cut/paste error when restoring the IVPR and IDR registers caused subsequent
>> vmstate sections to become misaligned and abort early.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Thanks a lot for all the work you put into this. Do you think you
> understand enough to the OpenPIC code by now to be able to convert it to
> VMState instead?
> 
> That would get rid of the whole class of problems altogether and make
> sure we didn't overlook something subtle somewhere else in the code.

I'm interested to have a go in time for the 2.3 release, but I'm not
exactly sure when that will be. This was mainly a festive distraction in
an attempt to help work out why the existing macio code doesn't work
correctly in its current form.

I'd be fine with you applying the existing patchset and I'll submit an
attempt to convert to VMState a bit later, hopefully after your
migration stream patches have also been merged to make the job a bit
easier ;)

Also slightly off-topic, but my rework of the macio code
(https://github.com/mcayland/qemu/commits/for-kevin), while being much
more readable in my opinion, also sadly appears to suffer from the same
corruption bug as the existing implementation.

If you think that what I have there is an improvement in terms of
readability then I'll aim to get that tidied up and submitted at some
point during the 2.3 cycle too.


ATB,

Mark.
diff mbox

Patch

diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 8699a4a..4194cef 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -1366,7 +1366,7 @@  static int openpic_load(QEMUFile* f, void *opaque, int version_id)
     OpenPICState *opp = (OpenPICState *)opaque;
     unsigned int i, nb_cpus;
 
-    if (version_id != 1) {
+    if (version_id != 2) {
         return -EINVAL;
     }
 
@@ -1399,12 +1399,10 @@  static int openpic_load(QEMUFile* f, void *opaque, int version_id)
         uint32_t val;
 
         val = qemu_get_be32(f);
-        write_IRQreg_idr(opp, i, val);
-        val = qemu_get_be32(f);
         write_IRQreg_ivpr(opp, i, val);
+        val = qemu_get_be32(f);
+        write_IRQreg_idr(opp, i, val);
 
-        qemu_get_be32s(f, &opp->src[i].ivpr);
-        qemu_get_be32s(f, &opp->src[i].idr);
         qemu_get_be32s(f, &opp->src[i].destmask);
         qemu_get_sbe32s(f, &opp->src[i].last_cpu);
         qemu_get_sbe32s(f, &opp->src[i].pending);