[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] hpet: recover timer offset correctly
From: |
Pavel Dovgalyuk |
Subject: |
Re: [Qemu-devel] [PATCH v2] hpet: recover timer offset correctly |
Date: |
Wed, 10 Jan 2018 13:10:04 +0300 |
> -----Original Message-----
> From: Juan Quintela [mailto:address@hidden
> Sent: Wednesday, January 10, 2018 12:51 PM
> To: Pavel Dovgalyuk
> Cc: 'Pavel Dovgalyuk'; address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden
> Subject: Re: [PATCH v2] hpet: recover timer offset correctly
>
> "Pavel Dovgalyuk" <address@hidden> wrote:
> >> From: Juan Quintela [mailto:address@hidden
> >> "Pavel Dovgalyuk" <address@hidden> wrote:
> >> >> From: Juan Quintela [mailto:address@hidden
> >> If you *don't* use a needed function then please just increase the
> >> version. You are just breaking compatibility anyways. The whole point
> >> of subsections is that they are optional. If they are mandatory (this
> >> case), then they bring no advantage at all.
> >
> > Thanks, I thought that the sections are skipped automatically when
> > there is no code for loading
> > them.
> >
> >> What dave is asked for your previous version is that you disable the
> >> section for old machine types. Look at how to use DEFINE_PROP_* for
> >> this use case.
> >
> > How do you like this one?
>
> Much better, thanks.
>
> > +static bool hpet_offset_needed(void *opaque)
> > +{
> > + HPETState *s = opaque;
> > +
> > + return s->hpet_offset_saved;
> > +}
> > +
>
> If this is only one optimization, this test is ok. If it makes things
> go worse, you can add something there like && hpet_enabled() or
> whatever. Remember that I don't understand HPET.
Right. Please check the new version of the patch.
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 263de97..8897302 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -13,6 +13,10 @@
> > .driver = "virtio-tablet-device",\
> > .property = "wheel-axis",\
> > .value = "false",\
> > + },{\
> > + .driver = "hpet",\
> > + .property = "hpet-offset-saved",\
> > + .value = "off",\
> > },
> >
> > #define HW_COMPAT_2_9 \
>
> This should be on 2_11 not 2_10 O:-)
>
> But for the vmstate bits:
>
> Reviewed-by: Juan Quintela <address@hidden>
Thanks.
Pavel Dovgalyuk