| Message ID | 20170412095111.11728-2-xiaoguangrong@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/04/2017 11:51, guangrong.xiao@gmail.com wrote: > From: Xiao Guangrong <xiaoguangrong@tencent.com> > > Currently, the timer is updated whenever RegA or RegB is written > even if the periodic timer related configuration is not changed > > This patch optimizes it slightly to make the update happen only > if its period or enable-status is changed, also later patches are > depend on this optimization > > Signed-off-by: Xiao Guangrong <xiaoguangrong@tencent.com> > --- > hw/timer/mc146818rtc.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c > index 4165450..749e206 100644 > --- a/hw/timer/mc146818rtc.c > +++ b/hw/timer/mc146818rtc.c > @@ -387,10 +387,25 @@ static void rtc_update_timer(void *opaque) > check_update_timer(s); > } > > +static bool rtc_periodic_timer_updated_by_regA(RTCState *s, uint64_t data) > +{ > + uint8_t orig = s->cmos_data[RTC_REG_A]; > + > + return (orig & 0x0f) != (data & 0x0f); > +} > + > +static bool rtc_periodic_timer_updated_by_regB(RTCState *s, uint64_t data) > +{ > + uint8_t orig = s->cmos_data[RTC_REG_B]; > + > + return (orig & REG_B_PIE) != (data & REG_B_PIE); > +} > + > static void cmos_ioport_write(void *opaque, hwaddr addr, > uint64_t data, unsigned size) > { > RTCState *s = opaque; > + bool update_periodic_timer; > > if ((addr & 1) == 0) { > s->cmos_index = data & 0x7f; > @@ -423,6 +438,8 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, > } > break; > case RTC_REG_A: > + update_periodic_timer = rtc_periodic_timer_updated_by_regA(s, data); I think you can change it to a... > if ((data & 0x60) == 0x60) { > if (rtc_running(s)) { > rtc_update_time(s); > @@ -445,10 +462,16 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, > /* UIP bit is read only */ ... inlined update_periodic_timer assignment here: update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_A_PERIOD; > s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) | > (s->cmos_data[RTC_REG_A] & REG_A_UIP); > - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); > + > + if (update_periodic_timer) { > + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); > + } > + > check_update_timer(s); > break; > case RTC_REG_B: > + update_periodic_timer = rtc_periodic_timer_updated_by_regB(s, data); > + > if (data & REG_B_SET) { > /* update cmos to when the rtc was stopping */ > if (rtc_running(s)) { > @@ -475,7 +498,11 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, > qemu_irq_lower(s->irq); > } Same here: update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_B_PIE; Thanks, Paolo > s->cmos_data[RTC_REG_B] = data; > - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); > + > + if (update_periodic_timer) { > + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); > + } > + > check_update_timer(s); > break; > case RTC_REG_C: >
On 05/03/2017 11:42 PM, Paolo Bonzini wrote: > > > On 12/04/2017 11:51, guangrong.xiao@gmail.com wrote: >> From: Xiao Guangrong <xiaoguangrong@tencent.com> >> >> Currently, the timer is updated whenever RegA or RegB is written >> even if the periodic timer related configuration is not changed >> >> This patch optimizes it slightly to make the update happen only >> if its period or enable-status is changed, also later patches are >> depend on this optimization >> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@tencent.com> >> --- >> hw/timer/mc146818rtc.c | 31 +++++++++++++++++++++++++++++-- >> 1 file changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c >> index 4165450..749e206 100644 >> --- a/hw/timer/mc146818rtc.c >> +++ b/hw/timer/mc146818rtc.c >> @@ -387,10 +387,25 @@ static void rtc_update_timer(void *opaque) >> check_update_timer(s); >> } >> >> +static bool rtc_periodic_timer_updated_by_regA(RTCState *s, uint64_t data) >> +{ >> + uint8_t orig = s->cmos_data[RTC_REG_A]; >> + >> + return (orig & 0x0f) != (data & 0x0f); >> +} >> + >> +static bool rtc_periodic_timer_updated_by_regB(RTCState *s, uint64_t data) >> +{ >> + uint8_t orig = s->cmos_data[RTC_REG_B]; >> + >> + return (orig & REG_B_PIE) != (data & REG_B_PIE); >> +} >> + >> static void cmos_ioport_write(void *opaque, hwaddr addr, >> uint64_t data, unsigned size) >> { >> RTCState *s = opaque; >> + bool update_periodic_timer; >> >> if ((addr & 1) == 0) { >> s->cmos_index = data & 0x7f; >> @@ -423,6 +438,8 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, >> } >> break; >> case RTC_REG_A: >> + update_periodic_timer = rtc_periodic_timer_updated_by_regA(s, data); > > I think you can change it to a... > >> if ((data & 0x60) == 0x60) { >> if (rtc_running(s)) { >> rtc_update_time(s); >> @@ -445,10 +462,16 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, >> /* UIP bit is read only */ > > ... inlined update_periodic_timer assignment here: > > update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_A_PERIOD; > >> s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) | >> (s->cmos_data[RTC_REG_A] & REG_A_UIP); >> - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); >> + >> + if (update_periodic_timer) { >> + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); >> + } >> + >> check_update_timer(s); >> break; >> case RTC_REG_B: >> + update_periodic_timer = rtc_periodic_timer_updated_by_regB(s, data); >> + >> if (data & REG_B_SET) { >> /* update cmos to when the rtc was stopping */ >> if (rtc_running(s)) { >> @@ -475,7 +498,11 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, >> qemu_irq_lower(s->irq); >> } > > Same here: > > update_periodic_timer = (s->cmos_data[RTC_REG_A] ^ data) & REG_B_PIE; > Okay, will do, thanks!
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 4165450..749e206 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -387,10 +387,25 @@ static void rtc_update_timer(void *opaque) check_update_timer(s); } +static bool rtc_periodic_timer_updated_by_regA(RTCState *s, uint64_t data) +{ + uint8_t orig = s->cmos_data[RTC_REG_A]; + + return (orig & 0x0f) != (data & 0x0f); +} + +static bool rtc_periodic_timer_updated_by_regB(RTCState *s, uint64_t data) +{ + uint8_t orig = s->cmos_data[RTC_REG_B]; + + return (orig & REG_B_PIE) != (data & REG_B_PIE); +} + static void cmos_ioport_write(void *opaque, hwaddr addr, uint64_t data, unsigned size) { RTCState *s = opaque; + bool update_periodic_timer; if ((addr & 1) == 0) { s->cmos_index = data & 0x7f; @@ -423,6 +438,8 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, } break; case RTC_REG_A: + update_periodic_timer = rtc_periodic_timer_updated_by_regA(s, data); + if ((data & 0x60) == 0x60) { if (rtc_running(s)) { rtc_update_time(s); @@ -445,10 +462,16 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, /* UIP bit is read only */ s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) | (s->cmos_data[RTC_REG_A] & REG_A_UIP); - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); + + if (update_periodic_timer) { + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); + } + check_update_timer(s); break; case RTC_REG_B: + update_periodic_timer = rtc_periodic_timer_updated_by_regB(s, data); + if (data & REG_B_SET) { /* update cmos to when the rtc was stopping */ if (rtc_running(s)) { @@ -475,7 +498,11 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, qemu_irq_lower(s->irq); } s->cmos_data[RTC_REG_B] = data; - periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); + + if (update_periodic_timer) { + periodic_timer_update(s, qemu_clock_get_ns(rtc_clock)); + } + check_update_timer(s); break; case RTC_REG_C: