Re: [Qemu-devel] [PATCH v3 2/7] hpet: Save/restore cached RTC IRQ level

[Anthony Liguori]

On 01/31/2012 04:05 PM, Jan Kiszka wrote:
> On 2012-01-31 22:02, Anthony Liguori wrote:
> > On 01/31/2012 11:41 AM, Jan Kiszka wrote:
> > > In legacy mode, the HPET suppresses the RTC interrupt delivery via IRQ
> > > 8 but keeps track of the RTC output level and applies it when legacy
> > > mode is turned off again. This value has to be preserved across save/
> > > restore as it cannot be reconstructed otherwise.
> > >
> > > Signed-off-by: Jan Kiszka<address@hidden>
> > > ---
> > >    hw/hpet.c |   26 ++++++++++++++++++++++++++
> > >    1 files changed, 26 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/hw/hpet.c b/hw/hpet.c
> > > index aba9ea9..39b291f 100644
> > > --- a/hw/hpet.c
> > > +++ b/hw/hpet.c
> > > @@ -240,6 +240,24 @@ static int hpet_post_load(void *opaque, int
> > > version_id)
> > >        return 0;
> > >    }
> > >
> > > +static bool hpet_rtc_irq_level_needed(void *opaque)
> > > +{
> > > +    HPETState *s = opaque;
> > > +
> > > +    return s->rtc_irq_level != 0;
> > > +}
> > > +
> > > +static const VMStateDescription vmstate_hpet_rtc_irq_level = {
> > > +    .name = "hpet/rtc_irq_level",
> > > +    .version_id = 1,
> > > +    .minimum_version_id = 1,
> > > +    .minimum_version_id_old = 1,
> > > +    .fields      = (VMStateField[]) {
> > > +        VMSTATE_UINT8(rtc_irq_level, HPETState),
> > > +        VMSTATE_END_OF_LIST()
> > > +    }
> > > +};
> > > +
> >
> > This won't work.  We don't clear rtc_irq_level on reset so rtc_irq_level
> > may be high or low after reset.  As such, we can't use a subsection
> > here.  We need to bump the savevm state.
>
> rtc_irq_level is updated during reset by the connected RTC device. It
> clears its IRQ line.

Ah, that's subtle and warrants a comment :-)

Probably worth adding an s->rtc_irq_level = 0 in reset just to be extra safe.

Regards,

Anthony Liguori

> Jan