On Wed, 2023-03-01 at 23:47 +0100, BALATON Zoltan wrote: > On Wed, 1 Mar 2023, David Woodhouse wrote: > > On Wed, 2023-03-01 at 19:01 +0100, BALATON Zoltan wrote: > > > > > > > It isn't a *correct* fix without a little bit more typing, but does > > > > this make it work? > > > > > > > > diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c > > > > index 17910f3bcb..36ebcff025 100644 > > > > --- a/hw/intc/i8259.c > > > > +++ b/hw/intc/i8259.c > > > > @@ -246,6 +246,7 @@ static void pic_ioport_write(void *opaque, hwaddr > > > > addr64, > > > > if (val & 0x08) { > > > > qemu_log_mask(LOG_UNIMP, > > > > "i8259: level sensitive irq not > > > > supported\n"); > > > > + s->elcr = 0xff; > > > > > > This works too. I guess the log can be then removed too. Could you submit > > > a proper patch or you want me to do that so we can drop the workaround for > > > it? Thanks for looking into it. > > > > > > Happy for you to do the rest of the typing ... :) > > I don't mind the typing but this is quite a bit more involved than I > expected. You've lost me at the vmstate stuff which I don't quite know how > to change or test. If these were stored as bits in an ISW1 register as > described by the docs I've looked at now then no change in migration would > be needed but this isn't how it seems to be in QEMU so I give up on that > in this case. Could you please do the typing then?
Yeah, it is a bit weird that we store the ICW1 byte in *separate* elements of persistent state, each of *them* a uint8_t which holds only a single bit of ICW1: s->init4 = val & 1; s->single_mode = val & 2; s->ltim = val & 8; Still, it's a pattern that's easy enough to follow. As for the rest of the typing, I'd basically done the bulk of it already when showing how to adjust the existing (s->elcr&mask) checks; there was only one more of those to type. And then the vmstate part is basically just a cut and paste of the bit in docs/devel/migration.rst which tells you exactly how to do it. Patch follows. It builds, but I'll let you do the actual testing, including migration to/from the new version, checking with scripts/analyze-migration.py that the ltim is there when it should be, and isn't when it shouldn't, and any other review feedback.
smime.p7s
Description: S/MIME cryptographic signature