| Message ID | 1421856072-25026-8-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
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
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 --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);
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(-)