I'm surprised that we didn't consume this thus far, and that we got away with also not emulating it for HVM guests. The 1st and 3rd patches are imo candidates for 4.22, whereas the others (more or less associated cleanup) likely aren't. 1: x86/time: use RTC century byte when available 2: x86/time: move BCD_TO_BIN() uses 3: x86/vRTC: support century field 4: x86/vRTC: use available macros for BCD <-> BIN conversion 5: tools/xen-hvmctx: shorten various format strings a little Jan Without this the present logic will misbehave from 2070 onwards. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -XXX,XX +XXX,XX @@ struct rtc_time { static bool __get_cmos_time(struct rtc_time *rtc) { s_time_t start, t1, t2; + unsigned int century = 0; unsigned long flags; spin_lock_irqsave(&rtc_lock, flags); @@ -XXX,XX +XXX,XX @@ static bool __get_cmos_time(struct rtc_t rtc->day = CMOS_READ(RTC_DAY_OF_MONTH); rtc->mon = CMOS_READ(RTC_MONTH); rtc->year = CMOS_READ(RTC_YEAR); + if ( acpi_gbl_FADT.century && acpi_gbl_FADT.century < 0x80 ) + century = CMOS_READ(acpi_gbl_FADT.century); if ( RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) ) { @@ -XXX,XX +XXX,XX @@ static bool __get_cmos_time(struct rtc_t spin_unlock_irqrestore(&rtc_lock, flags); - if ( (rtc->year += 1900) < 1970 ) + if ( century ) + { + BCD_TO_BIN(century); + rtc->year += century * 100; + } + else if ( (rtc->year += 1900) < 1970 ) rtc->year += 100; return t1 <= SECONDS(1) && t2 < MILLISECS(3); ... outside of __get_cmos_time()'s locked region. There's no need to hold the lock for these computations. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- How come RTC_ALWAYS_BCD is compile-time constant 1? And then even with an inverted comment? Looks like we've inherited this from Linux, and even in Linus'es current tree it's still this same way. Yet all half-way recent chipsets I'm aware of properly implement the DM bit in reg B. Might this be another 32-bit leftover? --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -XXX,XX +XXX,XX @@ struct rtc_time { static bool __get_cmos_time(struct rtc_time *rtc) { s_time_t start, t1, t2; + bool bcd; unsigned int century = 0; unsigned long flags; @@ -XXX,XX +XXX,XX @@ static bool __get_cmos_time(struct rtc_t rtc->year = CMOS_READ(RTC_YEAR); if ( acpi_gbl_FADT.century && acpi_gbl_FADT.century < 0x80 ) century = CMOS_READ(acpi_gbl_FADT.century); - - if ( RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) ) + + bcd = RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY); + + spin_unlock_irqrestore(&rtc_lock, flags); + + if ( bcd ) { BCD_TO_BIN(rtc->sec); BCD_TO_BIN(rtc->min); @@ -XXX,XX +XXX,XX @@ static bool __get_cmos_time(struct rtc_t BCD_TO_BIN(rtc->year); } - spin_unlock_irqrestore(&rtc_lock, flags); - if ( century ) { BCD_TO_BIN(century); Both ROMBIOS and SeaBIOS (with CONFIG_QEMU=y, as we build it) blindly assume availability of this field (at its conventional index 0x32); OVMF at least has code to inspect FADT. Hence we ought to have supported it virtually forever. As the index is beyond RTC_CMOS_SIZE, leverage the padding field in struct hvm_hw_rtc to hold its value. Update the field only when involved values are valid BCD century specifiers. Otherwise (for VMs migrated in from an older hypervisor) leave handling to the DM. This makes the Linux rtc-cmos driver report y3k compatibility. While extending xen-hvmctx.c:dump_rtc() also add RTC offset there. Fixes: 4ca161214355 ("[HVM] Move RTC emulation into the hypervisor") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Am I overly paranoid with the checking of the field, considering that Xen 3.x post-dates year 2000 and hence all firmware nowadays usable guests have ever run with should have been aware of the field? Or am I, quite the opposite, still not strict enough? I can't help the impression that this introduces a latency issue for the 2nd of gmtime()'s while() loops: We now allow years up into the 99th century, i.e. over 8000 years away from 1970. 8000 years are very roughly 2^^38 seconds, making for (again very roughly) 5 million iterations there. Did I get my math wrong, or do we need a prereq change to (vastly) reduce the number of iterations of that loop (e.g. along the lines of the other one, first going in 400 year steps)? Isn't day-of-week handling flawed? If the field is brought out of sync with the other values, shouldn't it stay respectively out-of-sync? And isn't it excessive overhead to go through rtc_set_time() when the field is updated while SET is clear? Perhaps we ought to also support alarm day/month features? --- a/tools/libacpi/static_tables.c +++ b/tools/libacpi/static_tables.c @@ -XXX,XX +XXX,XX @@ struct acpi_20_facs Facs = { #define ACPI_PM_TMR_BLK_BIT_WIDTH 0x20 #define ACPI_PM_TMR_BLK_BIT_OFFSET 0x00 +#define CMOS_CENTURY 0x32 /* Conventional index used also without ACPI */ + struct acpi_fadt Fadt = { .header = { .signature = ACPI_FADT_SIGNATURE, @@ -XXX,XX +XXX,XX @@ struct acpi_fadt Fadt = { .register_bit_width = ACPI_PM_TMR_BLK_BIT_WIDTH, .register_bit_offset = ACPI_PM_TMR_BLK_BIT_OFFSET, .address = ACPI_PM_TMR_BLK_ADDRESS_V1, - } + }, + + .century = CMOS_CENTURY, }; struct acpi_20_rsdt Rsdt = { --- a/tools/misc/xen-hvmctx.c +++ b/tools/misc/xen-hvmctx.c @@ -XXX,XX +XXX,XX @@ static void dump_rtc(void) printf(" 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x, index 0x%2.2x\n", r.cmos_data[8], r.cmos_data[9], r.cmos_data[10], r.cmos_data[11], r.cmos_data[12], r.cmos_data[13], r.cmos_index); - + printf(" century 0x%02x offset %"PRId64"\n", r.century, r.rtc_offset); } static void dump_hpet(void) --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -XXX,XX +XXX,XX @@ #define epoch_year 1900 #define get_year(x) ((x) + epoch_year) +static inline bool is_century(unsigned int x) +{ + /* Constant below should match epoch_year above, just as BCD value. */ + return x >= 0x19 && (x & 0xf) < 10 && (x >> 4) < 10; +} + enum rtc_mode { rtc_mode_no_ack, rtc_mode_strict @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque data &= 0x7f; s->hw.cmos_index = data; spin_unlock(&s->lock); + /* RTC_CENTURY always forwarded to DM. */ return (data < RTC_CMOS_SIZE); } - if ( s->hw.cmos_index >= RTC_CMOS_SIZE ) + switch ( s->hw.cmos_index ) { + case 0 ... RTC_CMOS_SIZE - 1: + orig = s->hw.cmos_data[s->hw.cmos_index]; + break; + + case RTC_CENTURY: + orig = s->hw.century; + if ( !is_century(orig) || !is_century(data) ) + { + /* Prevent further use of the field. */ + s->hw.century = 0; + spin_unlock(&s->lock); + return 0; + } + break; + + default: spin_unlock(&s->lock); return 0; } - orig = s->hw.cmos_data[s->hw.cmos_index]; switch ( s->hw.cmos_index ) { case RTC_SECONDS_ALARM: @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque case RTC_DAY_OF_MONTH: case RTC_MONTH: case RTC_YEAR: + case RTC_CENTURY: /* if in set mode, just write the register */ if ( (s->hw.cmos_data[RTC_REG_B] & RTC_SET) ) s->hw.cmos_data[s->hw.cmos_index] = data; @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque /* Fetch the current time and update just this field. */ s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); - s->hw.cmos_data[s->hw.cmos_index] = data; + if ( s->hw.cmos_index != RTC_CENTURY ) + s->hw.cmos_data[s->hw.cmos_index] = data; + else + s->hw.century = data; rtc_set_time(s); } alarm_timer_update(s); @@ -XXX,XX +XXX,XX @@ static void rtc_set_time(RTCState *s) tm->tm_wday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_WEEK]); tm->tm_mday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_MONTH]); tm->tm_mon = from_bcd(s, s->hw.cmos_data[RTC_MONTH]) - 1; - tm->tm_year = from_bcd(s, s->hw.cmos_data[RTC_YEAR]) + 100; + tm->tm_year = from_bcd(s, s->hw.cmos_data[RTC_YEAR]); + if ( is_century(s->hw.century) ) + { + unsigned int century = s->hw.century; + + BCD_TO_BIN(century); + tm->tm_year += century * 100 - epoch_year; + } + else + tm->tm_year += 100; after = mktime(get_year(tm->tm_year), tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); @@ -XXX,XX +XXX,XX @@ static void rtc_copy_date(RTCState *s) s->hw.cmos_data[RTC_DAY_OF_MONTH] = to_bcd(s, tm->tm_mday); s->hw.cmos_data[RTC_MONTH] = to_bcd(s, tm->tm_mon + 1); s->hw.cmos_data[RTC_YEAR] = to_bcd(s, tm->tm_year % 100); + + if ( is_century(s->hw.century) ) + { + s->hw.century = get_year(tm->tm_year) / 100; + BIN_TO_BCD(s->hw.century); + } } static int update_in_progress(RTCState *s) @@ -XXX,XX +XXX,XX @@ static uint32_t rtc_ioport_read(RTCState switch ( s->hw.cmos_index ) { + case RTC_CENTURY: + if ( !is_century(s->hw.century) ) + { + ret = UINT32_MAX; + break; + } + fallthrough; case RTC_SECONDS: case RTC_MINUTES: case RTC_HOURS: @@ -XXX,XX +XXX,XX @@ static uint32_t rtc_ioport_read(RTCState s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); } - ret = s->hw.cmos_data[s->hw.cmos_index]; + if ( s->hw.cmos_index != RTC_CENTURY ) + ret = s->hw.cmos_data[s->hw.cmos_index]; + else + ret = s->hw.century; break; case RTC_REG_A: ret = s->hw.cmos_data[s->hw.cmos_index]; @@ -XXX,XX +XXX,XX @@ static int cf_check handle_rtc_io( *val = 0xff; return X86EMUL_OKAY; } - else if ( vrtc->hw.cmos_index < RTC_CMOS_SIZE ) + else if ( vrtc->hw.cmos_index < RTC_CMOS_SIZE || + vrtc->hw.cmos_index == RTC_CENTURY ) { *val = rtc_ioport_read(vrtc); - return X86EMUL_OKAY; + if ( *val != UINT32_MAX ) + return X86EMUL_OKAY; } return X86EMUL_UNHANDLEABLE; @@ -XXX,XX +XXX,XX @@ void rtc_init(struct domain *d) s->hw.cmos_data[RTC_REG_C] = 0; s->hw.cmos_data[RTC_REG_D] = RTC_VRT; + s->hw.century = 0x20; /* Arbitrary initial value satisfying is_century() */ + s->current_tm = gmtime(get_localtime(d)); s->start_time = NOW(); --- a/xen/arch/x86/include/asm/mc146818rtc.h +++ b/xen/arch/x86/include/asm/mc146818rtc.h @@ -XXX,XX +XXX,XX @@ bool is_cmos_port(unsigned int port, uns #define RTC_REG_C 12 #define RTC_REG_D 13 +/* Conventional index used without (and typically also with) ACPI. */ +#define RTC_CENTURY 0x32 + /********************************************************************** * register details **********************************************************************/ --- a/xen/include/public/arch-x86/hvm/save.h +++ b/xen/include/public/arch-x86/hvm/save.h @@ -XXX,XX +XXX,XX @@ struct hvm_hw_rtc { uint8_t cmos_data[RTC_CMOS_SIZE]; /* Index register for 2-part operations */ uint8_t cmos_index; - uint8_t pad0; + uint8_t century; /* RTC offset from host time */ int64_t rtc_offset; }; There's no need to open-code these. No functional change intended, even if the | changes to + in to_bcd(). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -XXX,XX +XXX,XX @@ static void cf_check rtc_update_timer2(v static unsigned int to_bcd(const RTCState *s, unsigned int a) { - if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY ) - return a; + if ( !(s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY) ) + BIN_TO_BCD(a); - return ((a / 10) << 4) | (a % 10); + return a; } static unsigned int from_bcd(const RTCState *s, unsigned int a) { - if ( s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY ) - return a; + if ( !(s->hw.cmos_data[RTC_REG_B] & RTC_DM_BINARY) ) + BCD_TO_BIN(a); - return ((a >> 4) * 10) + (a & 0x0f); + return a; } /* %4.4x and alike format specifiers can be expressed shorter as %04x or, as e.g. dump_ioapic() has it, %.4x. In dump_fpu()'s XMM register dumping, also move away from showing bogus xmm03 and alike. The proper register name is xmm3 for that particular example. Also strip trailing whitespace from lines touched. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/tools/misc/xen-hvmctx.c +++ b/tools/misc/xen-hvmctx.c @@ -XXX,XX +XXX,XX @@ static void dump_fpu(void *p) struct fpu_regs *r = p; int i; - printf(" FPU: fcw 0x%4.4x fsw 0x%4.4x\n" - " ftw 0x%2.2x (0x%2.2x) fop 0x%4.4x\n" - " fpuip 0x%16.16"PRIx64" fpudp 0x%16.16"PRIx64"\n" - " mxcsr 0x%8.8lx mask 0x%8.8lx\n", + printf(" FPU: fcw 0x%04x fsw 0x%04x\n" + " ftw 0x%02x (0x%02x) fop 0x%04x\n" + " fpuip 0x%016"PRIx64" fpudp 0x%016"PRIx64"\n" + " mxcsr 0x%08lx mask 0x%08lx\n", (unsigned)r->fcw, (unsigned)r->fsw, (unsigned)r->ftw, (unsigned)r->res0, (unsigned)r->fop, r->fpuip, r->fpudp, (unsigned long)r->mxcsr, (unsigned long)r->mxcsr_mask); for ( i = 0 ; i < 8 ; i++ ) - printf(" mm%i 0x%4.4x%16.16"PRIx64" (0x%4.4x%4.4x%4.4x)\n", + printf(" mm%i 0x%04x%016"PRIx64" (0x%04x%04x%04x)\n", i, r->mm[i].hi, r->mm[i].lo, r->mm[i].pad[2], r->mm[i].pad[1], r->mm[i].pad[0]); for ( i = 0 ; i < 16 ; i++ ) - printf(" xmm%2.2i 0x%16.16"PRIx64"%16.16"PRIx64"\n", + printf(" xmm%-2i 0x%016"PRIx64"%016"PRIx64"\n", i, r->xmm[i].hi, r->xmm[i].lo); for ( i = 0 ; i < 6 ; i++ ) - printf(" (0x%16.16"PRIx64"%16.16"PRIx64")\n", + printf(" (0x%016"PRIx64"%016"PRIx64")\n", r->res1[2*i+1], r->res1[2*i]); } @@ -XXX,XX +XXX,XX @@ static void dump_cpu(void) { HVM_SAVE_TYPE(CPU) c; READ(c); - printf(" CPU: rax 0x%16.16llx rbx 0x%16.16llx\n" - " rcx 0x%16.16llx rdx 0x%16.16llx\n" - " rbp 0x%16.16llx rsi 0x%16.16llx\n" - " rdi 0x%16.16llx rsp 0x%16.16llx\n" - " r8 0x%16.16llx r9 0x%16.16llx\n" - " r10 0x%16.16llx r11 0x%16.16llx\n" - " r12 0x%16.16llx r13 0x%16.16llx\n" - " r14 0x%16.16llx r15 0x%16.16llx\n" - " rip 0x%16.16llx rflags 0x%16.16llx\n" - " cr0 0x%16.16llx cr2 0x%16.16llx\n" - " cr3 0x%16.16llx cr4 0x%16.16llx\n" - " dr0 0x%16.16llx dr1 0x%16.16llx\n" - " dr2 0x%16.16llx dr3 0x%16.16llx\n" - " dr6 0x%16.16llx dr7 0x%16.16llx\n" + printf(" CPU: rax 0x%016llx rbx 0x%016llx\n" + " rcx 0x%016llx rdx 0x%016llx\n" + " rbp 0x%016llx rsi 0x%016llx\n" + " rdi 0x%016llx rsp 0x%016llx\n" + " r8 0x%016llx r9 0x%016llx\n" + " r10 0x%016llx r11 0x%016llx\n" + " r12 0x%016llx r13 0x%016llx\n" + " r14 0x%016llx r15 0x%016llx\n" + " rip 0x%016llx rflags 0x%016llx\n" + " cr0 0x%016llx cr2 0x%016llx\n" + " cr3 0x%016llx cr4 0x%016llx\n" + " dr0 0x%016llx dr1 0x%016llx\n" + " dr2 0x%016llx dr3 0x%016llx\n" + " dr6 0x%016llx dr7 0x%016llx\n" " cs %#6.4" PRIx32 " (%#18.8" PRIx64 " + %#10.8" PRIx32 " / %#7.4" PRIx32 ")\n" " es %#6.4" PRIx32 " (%#18.8" PRIx64 " + %#10.8" PRIx32 " / %#7.4" PRIx32 ")\n" " ds %#6.4" PRIx32 " (%#18.8" PRIx64 " + %#10.8" PRIx32 " / %#7.4" PRIx32 ")\n" @@ -XXX,XX +XXX,XX @@ static void dump_cpu(void) " ldtr %#6.4" PRIx32 " (%#18.8" PRIx64 " + %#10.4" PRIx32 " / %#7.4" PRIx32 ")\n" " idtr (%#18.8" PRIx64 " + %#10.4" PRIx32 ")\n" " gdtr (%#18.8" PRIx64 " + %#10.4" PRIx32 ")\n" - " sysenter cs 0x%8.8llx eip 0x%16.16llx esp 0x%16.16llx\n" + " sysenter cs 0x%08llx eip 0x%016llx esp 0x%016llx\n" " shadow gs %#18.16" PRIx64 " efer %#18.8" PRIx64 "\n" " lstar %#18.16" PRIx64 " cstar %#18.16" PRIx64 "\n" " star %#18.16" PRIx64 " sfmask %#18.8" PRIx64 "\n" - " tsc 0x%16.16llx\n" - " event 0x%8.8lx error 0x%8.8lx\n", + " tsc 0x%016llx\n" + " event 0x%08lx error 0x%08lx\n", (unsigned long long) c.rax, (unsigned long long) c.rbx, (unsigned long long) c.rcx, (unsigned long long) c.rdx, (unsigned long long) c.rbp, (unsigned long long) c.rsi, @@ -XXX,XX +XXX,XX @@ static void dump_pci_irq(void) { HVM_SAVE_TYPE(PCI_IRQ) i; READ(i); - printf(" PCI IRQs: 0x%16.16llx%16.16llx\n", + printf(" PCI IRQs: 0x%016llx%016llx\n", (unsigned long long) i.pad[0], (unsigned long long) i.pad[1]); } @@ -XXX,XX +XXX,XX @@ static void dump_isa_irq(void) { HVM_SAVE_TYPE(ISA_IRQ) i; READ(i); - printf(" ISA IRQs: 0x%4.4llx\n", + printf(" ISA IRQs: 0x%04llx\n", (unsigned long long) i.pad[0]); } @@ -XXX,XX +XXX,XX @@ static void dump_rtc(void) { HVM_SAVE_TYPE(RTC) r; READ(r); - printf(" RTC: regs 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x\n", + printf(" RTC: regs 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x\n", r.cmos_data[0], r.cmos_data[1], r.cmos_data[2], r.cmos_data[3], r.cmos_data[4], r.cmos_data[5], r.cmos_data[6], r.cmos_data[7]); - printf(" 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x 0x%2.2x, index 0x%2.2x\n", + printf(" 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x, index 0x%02x\n", r.cmos_data[8], r.cmos_data[9], r.cmos_data[10], r.cmos_data[11], r.cmos_data[12], r.cmos_data[13], r.cmos_index); printf(" century 0x%02x offset %"PRId64"\n", r.century, r.rtc_offset); While meanwhile we at least consume this ourselves, I'm still surprised that we got away with also not emulating it for HVM guests. There's now some other (more or less related) cleanup here as well. 1: x86/time: CMOS RTC may run in binary mode 2: time: shorten year determination loop 3: x86/vRTC: the use_timer field is a boolean one 4: x86/vRTC: support century field Jan Indicating it would always use BCD mode is just wrong (and then the comment there said the opposite). All halfway recent (and really all 64- bit capable) systems having a CMOS RTC should properly indicate the mode in control register B. Make use of the flag, but provide a fallback mechanism in case people run into systems not matching the above assumption. Additionally, when binary mode is indicated and when "cmos-rtc-probe" is in use (but "cmos-rtc-bcd" isn't), probe whether the clock really runs in binary mode. (This probing, sadly, can take up to 10 seconds.) Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: New. --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -XXX,XX +XXX,XX @@ parameter to "stable:socket". Specify the event count threshold for raising Corrected Machine Check Interrupts. Specifying zero disables CMCI handling. +### cmos-rtc-bcd (x86) +> `= <boolean>` + +> Default: `false` + +Flag to indicate the CMOS Real Time Clock uses BCD mode irrespective of +control register B indicating binary mode. + ### cmos-rtc-probe (x86) > `= <boolean>` --- a/xen/arch/x86/include/asm/mc146818rtc.h +++ b/xen/arch/x86/include/asm/mc146818rtc.h @@ -XXX,XX +XXX,XX @@ bool is_cmos_port(unsigned int port, uns #ifndef RTC_PORT #define RTC_PORT(x) (0x70 + (x)) -#define RTC_ALWAYS_BCD 1 /* RTC operates in binary mode */ #endif /* --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -XXX,XX +XXX,XX @@ mktime (unsigned int year, unsigned int )*60 + sec; /* finally seconds */ } +static bool __ro_after_init opt_cmos_rtc_bcd; +boolean_param("cmos-rtc-bcd", opt_cmos_rtc_bcd); + struct rtc_time { unsigned int year, mon, day, hour, min, sec; }; @@ -XXX,XX +XXX,XX @@ static bool __get_cmos_time(struct rtc_t if ( acpi_gbl_FADT.century && acpi_gbl_FADT.century < 0x80 ) century = CMOS_READ(acpi_gbl_FADT.century); - bcd = RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY); + bcd = opt_cmos_rtc_bcd || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY); spin_unlock_irqrestore(&rtc_lock, flags); @@ -XXX,XX +XXX,XX @@ static bool __init cmos_rtc_probe(void) return false; } +static inline bool __init attr_const is_bcd(unsigned int x) +{ + return (x & 0xf) < 10 && (x >> 4) < 10; +} + +static void __init cmos_rtc_probe_bcd(void) +{ + bool bcd; + unsigned long flags; + + if ( opt_cmos_rtc_bcd ) + return; + + spin_lock_irqsave(&rtc_lock, flags); + bcd = !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY); + spin_unlock_irqrestore(&rtc_lock, flags); + + if ( bcd ) + return; + + for ( unsigned int seclo = 0; ; ) + { + struct rtc_time rtc; + + if ( !__get_cmos_time(&rtc) || + !is_bcd(rtc.sec) || + !is_bcd(rtc.min) || + !is_bcd(rtc.hour) || + !is_bcd(rtc.day) || + !is_bcd(rtc.mon) ) + return; + + if ( seclo > (rtc.sec & 0xf) ) + break; + + seclo = rtc.sec & 0xf; + } + + printk(XENLOG_WARNING "CMOS RTC indicates binary mode but uses BCD\n"); + + opt_cmos_rtc_bcd = true; +} static unsigned long cmos_rtc_read(void) { @@ -XXX,XX +XXX,XX @@ static void __init probe_wallclock(void) if ( cmos_rtc_probe() ) { wallclock_source = WALLCLOCK_CMOS; + + if ( opt_cmos_rtc_probe ) + cmos_rtc_probe_bcd(); + return; } if ( efi_enabled(EFI_RS) && efi_get_time() ) For dates very far into the future (the MC146818 RTC's century byte can go up to the 99th century), the present year-wise loop would become somewhat inefficient (taking perhaps several thousand iterations). Prefix that loop with a 400-year granular calculation (somewhat like the earlier loop does for dates in the past). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: New. --- a/xen/common/time.c +++ b/xen/common/time.c @@ -XXX,XX +XXX,XX @@ #define __isleap(year) \ ((year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0)) +#define DAYS_IN_400_YEARS (365 * 303 + 366 * 97) + /* How many days are in each month. */ static const unsigned short int __mon_lengths[2][12] = { /* Normal years. */ @@ -XXX,XX +XXX,XX @@ struct tm gmtime(unsigned long t) while ( t & (1UL<<39) ) { y -= 400; - t += ((unsigned long)(365 * 303 + 366 * 97)) * SECS_PER_DAY; + t += (unsigned long)DAYS_IN_400_YEARS * SECS_PER_DAY; } t &= (1UL << 40) - 1; #endif @@ -XXX,XX +XXX,XX @@ struct tm gmtime(unsigned long t) tbuf.tm_sec = rem % 60; /* January 1, 1970 was a Thursday. */ tbuf.tm_wday = (4 + days) % 7; + if ( days >= DAYS_IN_400_YEARS ) + { + y += (days / DAYS_IN_400_YEARS) * 400; + days %= DAYS_IN_400_YEARS; + } while ( days >= (rem = __isleap(y) ? 366 : 365) ) { ++y; ... and hence wants to be of bool type. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: New. --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -XXX,XX +XXX,XX @@ static void check_update_timer(RTCState if (!(s->hw.cmos_data[RTC_REG_C] & RTC_UF) && !(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) { - s->use_timer = 1; + s->use_timer = true; guest_usec = get_localtime_us(d) % USEC_PER_SEC; if (guest_usec >= (USEC_PER_SEC - 244)) { @@ -XXX,XX +XXX,XX @@ static void check_update_timer(RTCState } } else - s->use_timer = 0; + s->use_timer = false; } static void cf_check rtc_update_timer(void *opaque) @@ -XXX,XX +XXX,XX @@ static uint32_t rtc_ioport_read(RTCState break; case RTC_REG_A: ret = s->hw.cmos_data[s->hw.cmos_index]; - if ((s->use_timer == 0) && update_in_progress(s)) + if ( !s->use_timer && update_in_progress(s) ) ret |= RTC_UIP; break; case RTC_REG_C: --- a/xen/arch/x86/include/asm/hvm/vpt.h +++ b/xen/arch/x86/include/asm/hvm/vpt.h @@ -XXX,XX +XXX,XX @@ typedef struct RTCState { s_time_t check_ticks_since; int period; uint8_t pt_dead_ticks; - uint32_t use_timer; + + bool use_timer; + spinlock_t lock; } RTCState; Both ROMBIOS and SeaBIOS (with CONFIG_QEMU=y, as we build it) blindly assume availability of this field (at its conventional index 0x32); OVMF at least has code to inspect FADT. Hence we ought to have supported it virtually forever. As the index is beyond RTC_CMOS_SIZE, leverage the padding field in struct hvm_hw_rtc to hold its value. Update the field only when involved values are valid BCD century specifiers. Otherwise (for VMs migrated in from an older hypervisor) leave handling to the DM. This makes the Linux rtc-cmos driver report y3k compatibility. In the new rtc_check(), besides checking the new fields also check the pre-existing pad0 field. While extending xen-hvmctx.c:dump_rtc() also add RTC offset there. Fixes: 4ca161214355 ("[HVM] Move RTC emulation into the hypervisor") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Am I overly paranoid with the checking of the field, considering that Xen 3.x post-dates year 2000 and hence all firmware nowadays usable guests have ever run with should have been aware of the field? Or am I, quite the opposite, still not strict enough? Now that we extend struct hvm_hw_rtc, should we perhaps save not only the century, but also its index? Likely more sanity checking could be added to rtc_check(), but that's for a separate patch imo. Isn't day-of-week handling flawed? If the field is brought out of sync with the other values, shouldn't it stay respectively out-of-sync? And isn't it excessive overhead to go through rtc_set_time() when the field is updated while SET is clear? Perhaps we ought to also support alarm day/month features? --- v2: Don't re-purpose pad0 field of struct hvm_hw_rtc. --- a/tools/libacpi/static_tables.c +++ b/tools/libacpi/static_tables.c @@ -XXX,XX +XXX,XX @@ struct acpi_20_facs Facs = { #define ACPI_PM_TMR_BLK_BIT_WIDTH 0x20 #define ACPI_PM_TMR_BLK_BIT_OFFSET 0x00 +#define CMOS_CENTURY 0x32 /* Conventional index used also without ACPI */ + struct acpi_fadt Fadt = { .header = { .signature = ACPI_FADT_SIGNATURE, @@ -XXX,XX +XXX,XX @@ struct acpi_fadt Fadt = { .register_bit_width = ACPI_PM_TMR_BLK_BIT_WIDTH, .register_bit_offset = ACPI_PM_TMR_BLK_BIT_OFFSET, .address = ACPI_PM_TMR_BLK_ADDRESS_V1, - } + }, + + .century = CMOS_CENTURY, }; struct acpi_20_rsdt Rsdt = { --- a/tools/misc/xen-hvmctx.c +++ b/tools/misc/xen-hvmctx.c @@ -XXX,XX +XXX,XX @@ static void dump_rtc(void) printf(" 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x, index 0x%02x\n", r.cmos_data[8], r.cmos_data[9], r.cmos_data[10], r.cmos_data[11], r.cmos_data[12], r.cmos_data[13], r.cmos_index); - + printf(" century 0x%02x offset %"PRId64"\n", r.century, r.rtc_offset); } static void dump_hpet(void) --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque data &= 0x7f; s->hw.cmos_index = data; spin_unlock(&s->lock); - return (data < RTC_CMOS_SIZE); + return data < RTC_CMOS_SIZE || (s->has_century && data == RTC_CENTURY); } - if ( s->hw.cmos_index >= RTC_CMOS_SIZE ) + switch ( s->hw.cmos_index ) { + case 0 ... RTC_CMOS_SIZE - 1: + orig = s->hw.cmos_data[s->hw.cmos_index]; + break; + + case RTC_CENTURY: + if ( s->has_century ) + { + orig = s->hw.century; + break; + } + fallthrough; + default: spin_unlock(&s->lock); return 0; } - orig = s->hw.cmos_data[s->hw.cmos_index]; switch ( s->hw.cmos_index ) { case RTC_SECONDS_ALARM: @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque case RTC_DAY_OF_MONTH: case RTC_MONTH: case RTC_YEAR: + case RTC_CENTURY: /* if in set mode, just write the register */ if ( (s->hw.cmos_data[RTC_REG_B] & RTC_SET) ) s->hw.cmos_data[s->hw.cmos_index] = data; @@ -XXX,XX +XXX,XX @@ static int rtc_ioport_write(void *opaque /* Fetch the current time and update just this field. */ s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); - s->hw.cmos_data[s->hw.cmos_index] = data; + if ( s->hw.cmos_index != RTC_CENTURY ) + s->hw.cmos_data[s->hw.cmos_index] = data; + else + s->hw.century = data; rtc_set_time(s); } alarm_timer_update(s); @@ -XXX,XX +XXX,XX @@ static void rtc_set_time(RTCState *s) tm->tm_wday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_WEEK]); tm->tm_mday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_MONTH]); tm->tm_mon = from_bcd(s, s->hw.cmos_data[RTC_MONTH]) - 1; - tm->tm_year = from_bcd(s, s->hw.cmos_data[RTC_YEAR]) + 100; + tm->tm_year = from_bcd(s, s->hw.cmos_data[RTC_YEAR]); + if ( s->has_century ) + { + unsigned int century = s->hw.century; + + BCD_TO_BIN(century); + tm->tm_year += century * 100 - epoch_year; + } + else + tm->tm_year += 100; after = mktime(get_year(tm->tm_year), tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); @@ -XXX,XX +XXX,XX @@ static void rtc_copy_date(RTCState *s) s->hw.cmos_data[RTC_DAY_OF_MONTH] = to_bcd(s, tm->tm_mday); s->hw.cmos_data[RTC_MONTH] = to_bcd(s, tm->tm_mon + 1); s->hw.cmos_data[RTC_YEAR] = to_bcd(s, tm->tm_year % 100); + + if ( s->has_century ) + { + s->hw.century = get_year(tm->tm_year) / 100; + BIN_TO_BCD(s->hw.century); + } } static int update_in_progress(RTCState *s) @@ -XXX,XX +XXX,XX @@ static uint32_t rtc_ioport_read(RTCState case RTC_DAY_OF_MONTH: case RTC_MONTH: case RTC_YEAR: + case RTC_CENTURY: /* if not in set mode, adjust cmos before reading*/ if (!(s->hw.cmos_data[RTC_REG_B] & RTC_SET)) { s->current_tm = gmtime(get_localtime(d)); rtc_copy_date(s); } - ret = s->hw.cmos_data[s->hw.cmos_index]; + if ( s->hw.cmos_index != RTC_CENTURY ) + ret = s->hw.cmos_data[s->hw.cmos_index]; + else + ret = s->hw.century; break; case RTC_REG_A: ret = s->hw.cmos_data[s->hw.cmos_index]; @@ -XXX,XX +XXX,XX @@ static int cf_check handle_rtc_io( *val = 0xff; return X86EMUL_OKAY; } - else if ( vrtc->hw.cmos_index < RTC_CMOS_SIZE ) + else if ( vrtc->hw.cmos_index < RTC_CMOS_SIZE || + (vrtc->has_century && vrtc->hw.cmos_index == RTC_CENTURY) ) { *val = rtc_ioport_read(vrtc); return X86EMUL_OKAY; @@ -XXX,XX +XXX,XX @@ static int cf_check rtc_save(struct vcpu return rc; } +static int cf_check rtc_check(const struct domain *d, hvm_domain_context_t *h) +{ + const struct hvm_save_descriptor *desc = + (const struct hvm_save_descriptor *)&h->data[h->cur]; + struct hvm_hw_rtc s; + + if ( !has_vrtc(d) ) + return -ENODEV; + + if ( hvm_load_entry_zeroextend(RTC, h, &s) != 0 ) + return -ENODATA; + + if ( s.pad0 ) + return -EINVAL; + + for ( unsigned int i = 0; i < ARRAY_SIZE(s.pad1); ++i ) + if ( s.pad1[i] ) + return -EINVAL; + + if ( desc->length >= endof_field(struct hvm_hw_rtc, century) && + ((s.century & 0xf) >= 10 || (s.century >> 4) >= 10) ) + return -EINVAL; + + return 0; +} + /* Reload the hardware state from a saved domain */ static int cf_check rtc_load(struct domain *d, hvm_domain_context_t *h) { @@ -XXX,XX +XXX,XX @@ static int cf_check rtc_load(struct doma check_update_timer(s); alarm_timer_update(s); + if ( !s->hw.century ) + { + s->has_century = false; + s->hw.century = 0; + } + spin_unlock(&s->lock); return 0; } -HVM_REGISTER_SAVE_RESTORE(RTC, rtc_save, NULL, rtc_load, 1, HVMSR_PER_DOM); +HVM_REGISTER_SAVE_RESTORE(RTC, rtc_save, rtc_check, rtc_load, 1, HVMSR_PER_DOM); void rtc_reset(struct domain *d) { @@ -XXX,XX +XXX,XX @@ void rtc_init(struct domain *d) s->hw.cmos_data[RTC_REG_C] = 0; s->hw.cmos_data[RTC_REG_D] = RTC_VRT; + /* + * By default we make the century byte available, unless an incoming save + * record says otherwise. + */ + s->has_century = true; + s->current_tm = gmtime(get_localtime(d)); s->start_time = NOW(); --- a/xen/arch/x86/include/asm/hvm/vpt.h +++ b/xen/arch/x86/include/asm/hvm/vpt.h @@ -XXX,XX +XXX,XX @@ typedef struct RTCState { bool use_timer; + bool has_century; + spinlock_t lock; } RTCState; --- a/xen/arch/x86/include/asm/mc146818rtc.h +++ b/xen/arch/x86/include/asm/mc146818rtc.h @@ -XXX,XX +XXX,XX @@ bool is_cmos_port(unsigned int port, uns #define RTC_REG_C 12 #define RTC_REG_D 13 +/* Conventional index used without (and typically also with) ACPI. */ +#define RTC_CENTURY 0x32 + /********************************************************************** * register details **********************************************************************/ --- a/xen/include/public/arch-x86/hvm/save.h +++ b/xen/include/public/arch-x86/hvm/save.h @@ -XXX,XX +XXX,XX @@ struct hvm_hw_rtc { uint8_t pad0; /* RTC offset from host time */ int64_t rtc_offset; + uint8_t century; + uint8_t pad1[7]; }; DECLARE_HVM_SAVE_TYPE(RTC, 11, struct hvm_hw_rtc);