* Re: [Qemu-devel] [PATCH] hw/mc146818rtc.c: Fix reading and writing of time registers
[not found] <1360828461-2970-1-git-send-email-barsamin@gmail.com>
@ 2013-04-02 15:01 ` mdroth
0 siblings, 0 replies; only message in thread
From: mdroth @ 2013-04-02 15:01 UTC (permalink / raw)
To: Antoine Mathys; +Cc: bonzini, qemu-devel, qemu-stable
On Thu, Feb 14, 2013 at 08:54:20AM +0100, Antoine Mathys wrote:
> This patch consolidates the bit twidling involved in reading and
> writing the time registers to four functions that are used consistently.
>
> This also has the effect of fixing bug 1090558.
>
> Signed-off-by: Antoine Mathys <barsamin@gmail.com>
This issue still seems to be present upstream and I'm looking at pulling
it in for stable. Does anyone want to apply this, or are we still waiting
on a test case for the bug it fixes?
> ---
> hw/mc146818rtc.c | 163 +++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 99 insertions(+), 64 deletions(-)
>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 2fb11f6..646cbd0 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -86,8 +86,14 @@ typedef struct RTCState {
>
> static void rtc_set_time(RTCState *s);
> static void rtc_update_time(RTCState *s);
> +
> +/* data encoding / decoding */
> +static inline uint8_t rtc_encode(RTCState *s, uint8_t a);
> +static inline int rtc_decode(RTCState *s, uint8_t a);
> +static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour);
> +static inline int rtc_hour_decode(RTCState *s, uint8_t a);
> +
> static void rtc_set_cmos(RTCState *s, const struct tm *tm);
> -static inline int rtc_from_bcd(RTCState *s, int a);
> static uint64_t get_next_alarm(RTCState *s);
>
> static inline bool rtc_running(RTCState *s)
> @@ -254,17 +260,84 @@ static void check_update_timer(RTCState *s)
> }
> }
>
> -static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
> +/* data encoding / decoding */
> +
> +static inline uint8_t rtc_encode(RTCState *s, uint8_t a)
> +{
> + if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> + return a;
> + } else {
> + return to_bcd(a);
> + }
> +}
> +
> +static inline int rtc_decode(RTCState *s, uint8_t a)
> +{
> + if ((a & 0xc0) == 0xc0) {
> + return -1;
> + }
> + if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> + return a;
> + } else {
> + return from_bcd(a);
> + }
> +}
> +
> +/*
> + Note: The next two functions implement the following mapping between
> + the 12 hour and 24 hour formats:
> +
> + 0 <-> 12 AM
> + 1-11 <-> 1 - 11 AM
> + 12 <-> 12 PM
> + 13-23 <-> 1 - 11 PM
> +*/
> +
> +static inline uint8_t rtc_hour_encode(RTCState *s, uint8_t hour)
> +{
> + uint8_t tmp;
> +
> + if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
> + /* 24 hour format */
> + tmp = rtc_encode(s, hour);
> + } else {
> + /* 12 hour format */
> + uint8_t h = (hour % 12) ? (hour % 12) : 12;
> + tmp = rtc_encode(s, h);
> + if (hour >= 12) {
> + tmp |= 0x80;
> + }
> + }
> + return tmp;
> +}
> +
> +static inline int rtc_hour_decode(RTCState *s, uint8_t a)
> {
> - if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
> + uint8_t hour;
> +
> + /* Note: in 12 hour mode we clear bit 7 before calling
> + rtc_decode(), hence we cannot rely on the later to check the
> + don't care condition. While we could skip this check in 24 hour
> + mode it is simpler to do it in any case. */
> + if ((a & 0xc0) == 0xc0) {
> + return -1;
> + }
> +
> + if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
> + /* 24 hour format */
> + hour = rtc_decode(s, a);
> + } else {
> + /* 12 hour format */
> + hour = rtc_decode(s, a & 0x7f);
> hour %= 12;
> - if (s->cmos_data[RTC_HOURS] & 0x80) {
> + if (a & 0x80) {
> hour += 12;
> }
> }
> return hour;
> }
>
> +
> static uint64_t get_next_alarm(RTCState *s)
> {
> int32_t alarm_sec, alarm_min, alarm_hour, cur_hour, cur_min, cur_sec;
> @@ -272,15 +345,13 @@ static uint64_t get_next_alarm(RTCState *s)
>
> rtc_update_time(s);
>
> - alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
> - alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
> - alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
> - alarm_hour = alarm_hour == -1 ? -1 : convert_hour(s, alarm_hour);
> + alarm_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS_ALARM]);
> + alarm_min = rtc_decode(s, s->cmos_data[RTC_MINUTES_ALARM]);
> + alarm_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS_ALARM]);
>
> - cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
> - cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
> - cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
> - cur_hour = convert_hour(s, cur_hour);
> + cur_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS]);
> + cur_min = rtc_decode(s, s->cmos_data[RTC_MINUTES]);
> + cur_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS]);
>
> if (alarm_hour == -1) {
> alarm_hour = cur_hour;
> @@ -486,44 +557,17 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
> }
> }
>
> -static inline int rtc_to_bcd(RTCState *s, int a)
> -{
> - if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> - return a;
> - } else {
> - return ((a / 10) << 4) | (a % 10);
> - }
> -}
> -
> -static inline int rtc_from_bcd(RTCState *s, int a)
> -{
> - if ((a & 0xc0) == 0xc0) {
> - return -1;
> - }
> - if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
> - return a;
> - } else {
> - return ((a >> 4) * 10) + (a & 0x0f);
> - }
> -}
> -
> static void rtc_get_time(RTCState *s, struct tm *tm)
> {
> - tm->tm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
> - tm->tm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
> - tm->tm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS] & 0x7f);
> - if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
> - tm->tm_hour %= 12;
> - if (s->cmos_data[RTC_HOURS] & 0x80) {
> - tm->tm_hour += 12;
> - }
> - }
> - tm->tm_wday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
> - tm->tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]);
> - tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
> + tm->tm_sec = rtc_decode(s, s->cmos_data[RTC_SECONDS]);
> + tm->tm_min = rtc_decode(s, s->cmos_data[RTC_MINUTES]);
> + tm->tm_hour = rtc_hour_decode(s, s->cmos_data[RTC_HOURS]);
> + tm->tm_wday = rtc_decode(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1;
> + tm->tm_mday = rtc_decode(s, s->cmos_data[RTC_DAY_OF_MONTH]);
> + tm->tm_mon = rtc_decode(s, s->cmos_data[RTC_MONTH]) - 1;
> tm->tm_year =
> - rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year +
> - rtc_from_bcd(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900;
> + rtc_decode(s, s->cmos_data[RTC_YEAR]) + s->base_year +
> + rtc_decode(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900;
> }
>
> static void rtc_set_time(RTCState *s)
> @@ -541,24 +585,15 @@ static void rtc_set_cmos(RTCState *s, const struct tm *tm)
> {
> int year;
>
> - s->cmos_data[RTC_SECONDS] = rtc_to_bcd(s, tm->tm_sec);
> - s->cmos_data[RTC_MINUTES] = rtc_to_bcd(s, tm->tm_min);
> - if (s->cmos_data[RTC_REG_B] & REG_B_24H) {
> - /* 24 hour format */
> - s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, tm->tm_hour);
> - } else {
> - /* 12 hour format */
> - int h = (tm->tm_hour % 12) ? tm->tm_hour % 12 : 12;
> - s->cmos_data[RTC_HOURS] = rtc_to_bcd(s, h);
> - if (tm->tm_hour >= 12)
> - s->cmos_data[RTC_HOURS] |= 0x80;
> - }
> - s->cmos_data[RTC_DAY_OF_WEEK] = rtc_to_bcd(s, tm->tm_wday + 1);
> - s->cmos_data[RTC_DAY_OF_MONTH] = rtc_to_bcd(s, tm->tm_mday);
> - s->cmos_data[RTC_MONTH] = rtc_to_bcd(s, tm->tm_mon + 1);
> + s->cmos_data[RTC_SECONDS] = rtc_encode(s, tm->tm_sec);
> + s->cmos_data[RTC_MINUTES] = rtc_encode(s, tm->tm_min);
> + s->cmos_data[RTC_HOURS] = rtc_hour_encode(s, tm->tm_hour);
> + s->cmos_data[RTC_DAY_OF_WEEK] = rtc_encode(s, tm->tm_wday + 1);
> + s->cmos_data[RTC_DAY_OF_MONTH] = rtc_encode(s, tm->tm_mday);
> + s->cmos_data[RTC_MONTH] = rtc_encode(s, tm->tm_mon + 1);
> year = tm->tm_year + 1900 - s->base_year;
> - s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year % 100);
> - s->cmos_data[RTC_CENTURY] = rtc_to_bcd(s, year / 100);
> + s->cmos_data[RTC_YEAR] = rtc_encode(s, year % 100);
> + s->cmos_data[RTC_CENTURY] = rtc_encode(s, year / 100);
> }
>
> static void rtc_update_time(RTCState *s)
> --
> 1.7.10.4
>
>
^ permalink raw reply [flat|nested] only message in thread