* [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
@ 2014-07-25 14:57 Jan Beulich
2014-07-28 12:40 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-07-25 14:57 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 5045 bytes --]
HP is setting the ACPI_FADT_NO_CMOS_RTC flag on newer systems,
regardless of whether they're being booted from UEFI. Add a command
line option to allow probing for a working RTC in that case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -218,6 +218,14 @@ If set, override Xen's calculation of th
If set, override Xen's default choice for the platform timer.
+### cmos-rtc-probe
+> `= <boolean>`
+
+> Default: `false`
+
+Flag to indicate whether to probe for a CMOS Real Time Clock irrespective of
+ACPI indicating none to be there.
+
### com1,com2
> `= <baud>[/<clock_hz>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -654,37 +654,40 @@ mktime (unsigned int year, unsigned int
)*60 + sec; /* finally seconds */
}
-static unsigned long __get_cmos_time(void)
-{
+struct rtc_time {
unsigned int year, mon, day, hour, min, sec;
+};
- sec = CMOS_READ(RTC_SECONDS);
- min = CMOS_READ(RTC_MINUTES);
- hour = CMOS_READ(RTC_HOURS);
- day = CMOS_READ(RTC_DAY_OF_MONTH);
- mon = CMOS_READ(RTC_MONTH);
- year = CMOS_READ(RTC_YEAR);
+static void __get_cmos_time(struct rtc_time *rtc)
+{
+ rtc->sec = CMOS_READ(RTC_SECONDS);
+ rtc->min = CMOS_READ(RTC_MINUTES);
+ rtc->hour = CMOS_READ(RTC_HOURS);
+ rtc->day = CMOS_READ(RTC_DAY_OF_MONTH);
+ rtc->mon = CMOS_READ(RTC_MONTH);
+ rtc->year = CMOS_READ(RTC_YEAR);
if ( RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) )
{
- BCD_TO_BIN(sec);
- BCD_TO_BIN(min);
- BCD_TO_BIN(hour);
- BCD_TO_BIN(day);
- BCD_TO_BIN(mon);
- BCD_TO_BIN(year);
+ BCD_TO_BIN(rtc->sec);
+ BCD_TO_BIN(rtc->min);
+ BCD_TO_BIN(rtc->hour);
+ BCD_TO_BIN(rtc->day);
+ BCD_TO_BIN(rtc->mon);
+ BCD_TO_BIN(rtc->year);
}
- if ( (year += 1900) < 1970 )
- year += 100;
-
- return mktime(year, mon, day, hour, min, sec);
+ if ( (rtc->year += 1900) < 1970 )
+ rtc->year += 100;
}
static unsigned long get_cmos_time(void)
{
unsigned long res, flags;
- int i;
+ struct rtc_time rtc;
+ unsigned int seconds = 60;
+ static bool_t __read_mostly cmos_rtc_probe;
+ boolean_param("cmos-rtc-probe", cmos_rtc_probe);
if ( efi_enabled )
{
@@ -693,23 +696,58 @@ static unsigned long get_cmos_time(void)
return res;
}
- if ( unlikely(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) )
- panic("System without CMOS RTC must be booted from EFI");
-
- spin_lock_irqsave(&rtc_lock, flags);
-
- /* read RTC exactly on falling edge of update flag */
- for ( i = 0 ; i < 1000000 ; i++ ) /* may take up to 1 second... */
- if ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
+ if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) )
+ cmos_rtc_probe = 0;
+ else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe )
+ panic("System with no CMOS RTC advertised must be booted from EFI"
+ " (or with command line option \"cmos-rtc-probe\")");
+
+ for ( ; ; )
+ {
+ s_time_t start, t1, t2;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+
+ /* read RTC exactly on falling edge of update flag */
+ start = NOW();
+ do { /* may take up to 1 second... */
+ t1 = NOW() - start;
+ } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+ t1 <= SECONDS(1) );
+
+ start = NOW();
+ do { /* must try at least 2.228 ms */
+ t2 = NOW() - start;
+ } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+ t2 < MILLISECS(3) );
+
+ __get_cmos_time(&rtc);
+
+ spin_unlock_irqrestore(&rtc_lock, flags);
+
+ if ( likely(!cmos_rtc_probe) ||
+ t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
+ rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
+ !rtc.day || rtc.day > 31 ||
+ !rtc.mon || rtc.mon > 12 )
break;
- for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
- if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
+
+ if ( seconds < 60 )
+ {
+ if ( rtc.sec != seconds )
+ cmos_rtc_probe = 0;
break;
+ }
+
+ process_pending_softirqs();
+
+ seconds = rtc.sec;
+ }
- res = __get_cmos_time();
+ if ( unlikely(cmos_rtc_probe) )
+ panic("No CMOS RTC found - system must be booted from EFI");
- spin_unlock_irqrestore(&rtc_lock, flags);
- return res;
+ return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
}
/***************************************************************************
[-- Attachment #2: x86-RTC-probe.patch --]
[-- Type: text/plain, Size: 5107 bytes --]
x86/ACPI: allow CMOS RTC use even when ACPI says there is none
HP is setting the ACPI_FADT_NO_CMOS_RTC flag on newer systems,
regardless of whether they're being booted from UEFI. Add a command
line option to allow probing for a working RTC in that case.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -218,6 +218,14 @@ If set, override Xen's calculation of th
If set, override Xen's default choice for the platform timer.
+### cmos-rtc-probe
+> `= <boolean>`
+
+> Default: `false`
+
+Flag to indicate whether to probe for a CMOS Real Time Clock irrespective of
+ACPI indicating none to be there.
+
### com1,com2
> `= <baud>[/<clock_hz>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -654,37 +654,40 @@ mktime (unsigned int year, unsigned int
)*60 + sec; /* finally seconds */
}
-static unsigned long __get_cmos_time(void)
-{
+struct rtc_time {
unsigned int year, mon, day, hour, min, sec;
+};
- sec = CMOS_READ(RTC_SECONDS);
- min = CMOS_READ(RTC_MINUTES);
- hour = CMOS_READ(RTC_HOURS);
- day = CMOS_READ(RTC_DAY_OF_MONTH);
- mon = CMOS_READ(RTC_MONTH);
- year = CMOS_READ(RTC_YEAR);
+static void __get_cmos_time(struct rtc_time *rtc)
+{
+ rtc->sec = CMOS_READ(RTC_SECONDS);
+ rtc->min = CMOS_READ(RTC_MINUTES);
+ rtc->hour = CMOS_READ(RTC_HOURS);
+ rtc->day = CMOS_READ(RTC_DAY_OF_MONTH);
+ rtc->mon = CMOS_READ(RTC_MONTH);
+ rtc->year = CMOS_READ(RTC_YEAR);
if ( RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) )
{
- BCD_TO_BIN(sec);
- BCD_TO_BIN(min);
- BCD_TO_BIN(hour);
- BCD_TO_BIN(day);
- BCD_TO_BIN(mon);
- BCD_TO_BIN(year);
+ BCD_TO_BIN(rtc->sec);
+ BCD_TO_BIN(rtc->min);
+ BCD_TO_BIN(rtc->hour);
+ BCD_TO_BIN(rtc->day);
+ BCD_TO_BIN(rtc->mon);
+ BCD_TO_BIN(rtc->year);
}
- if ( (year += 1900) < 1970 )
- year += 100;
-
- return mktime(year, mon, day, hour, min, sec);
+ if ( (rtc->year += 1900) < 1970 )
+ rtc->year += 100;
}
static unsigned long get_cmos_time(void)
{
unsigned long res, flags;
- int i;
+ struct rtc_time rtc;
+ unsigned int seconds = 60;
+ static bool_t __read_mostly cmos_rtc_probe;
+ boolean_param("cmos-rtc-probe", cmos_rtc_probe);
if ( efi_enabled )
{
@@ -693,23 +696,58 @@ static unsigned long get_cmos_time(void)
return res;
}
- if ( unlikely(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) )
- panic("System without CMOS RTC must be booted from EFI");
-
- spin_lock_irqsave(&rtc_lock, flags);
-
- /* read RTC exactly on falling edge of update flag */
- for ( i = 0 ; i < 1000000 ; i++ ) /* may take up to 1 second... */
- if ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
+ if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) )
+ cmos_rtc_probe = 0;
+ else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe )
+ panic("System with no CMOS RTC advertised must be booted from EFI"
+ " (or with command line option \"cmos-rtc-probe\")");
+
+ for ( ; ; )
+ {
+ s_time_t start, t1, t2;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+
+ /* read RTC exactly on falling edge of update flag */
+ start = NOW();
+ do { /* may take up to 1 second... */
+ t1 = NOW() - start;
+ } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+ t1 <= SECONDS(1) );
+
+ start = NOW();
+ do { /* must try at least 2.228 ms */
+ t2 = NOW() - start;
+ } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+ t2 < MILLISECS(3) );
+
+ __get_cmos_time(&rtc);
+
+ spin_unlock_irqrestore(&rtc_lock, flags);
+
+ if ( likely(!cmos_rtc_probe) ||
+ t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
+ rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
+ !rtc.day || rtc.day > 31 ||
+ !rtc.mon || rtc.mon > 12 )
break;
- for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
- if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
+
+ if ( seconds < 60 )
+ {
+ if ( rtc.sec != seconds )
+ cmos_rtc_probe = 0;
break;
+ }
+
+ process_pending_softirqs();
+
+ seconds = rtc.sec;
+ }
- res = __get_cmos_time();
+ if ( unlikely(cmos_rtc_probe) )
+ panic("No CMOS RTC found - system must be booted from EFI");
- spin_unlock_irqrestore(&rtc_lock, flags);
- return res;
+ return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
}
/***************************************************************************
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
2014-07-25 14:57 [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none Jan Beulich
@ 2014-07-28 12:40 ` Andrew Cooper
2014-07-28 12:47 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-07-28 12:40 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Tim Deegan, Keir Fraser
On 25/07/14 15:57, Jan Beulich wrote:
> HP is setting the ACPI_FADT_NO_CMOS_RTC flag on newer systems,
> regardless of whether they're being booted from UEFI. Add a command
> line option to allow probing for a working RTC in that case.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -218,6 +218,14 @@ If set, override Xen's calculation of th
>
> If set, override Xen's default choice for the platform timer.
>
> +### cmos-rtc-probe
> +> `= <boolean>`
> +
> +> Default: `false`
> +
> +Flag to indicate whether to probe for a CMOS Real Time Clock irrespective of
> +ACPI indicating none to be there.
> +
> ### com1,com2
> > `= <baud>[/<clock_hz>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
>
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -654,37 +654,40 @@ mktime (unsigned int year, unsigned int
> )*60 + sec; /* finally seconds */
> }
>
> -static unsigned long __get_cmos_time(void)
> -{
> +struct rtc_time {
> unsigned int year, mon, day, hour, min, sec;
> +};
>
> - sec = CMOS_READ(RTC_SECONDS);
> - min = CMOS_READ(RTC_MINUTES);
> - hour = CMOS_READ(RTC_HOURS);
> - day = CMOS_READ(RTC_DAY_OF_MONTH);
> - mon = CMOS_READ(RTC_MONTH);
> - year = CMOS_READ(RTC_YEAR);
> +static void __get_cmos_time(struct rtc_time *rtc)
> +{
> + rtc->sec = CMOS_READ(RTC_SECONDS);
> + rtc->min = CMOS_READ(RTC_MINUTES);
> + rtc->hour = CMOS_READ(RTC_HOURS);
> + rtc->day = CMOS_READ(RTC_DAY_OF_MONTH);
> + rtc->mon = CMOS_READ(RTC_MONTH);
> + rtc->year = CMOS_READ(RTC_YEAR);
>
> if ( RTC_ALWAYS_BCD || !(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) )
> {
> - BCD_TO_BIN(sec);
> - BCD_TO_BIN(min);
> - BCD_TO_BIN(hour);
> - BCD_TO_BIN(day);
> - BCD_TO_BIN(mon);
> - BCD_TO_BIN(year);
> + BCD_TO_BIN(rtc->sec);
> + BCD_TO_BIN(rtc->min);
> + BCD_TO_BIN(rtc->hour);
> + BCD_TO_BIN(rtc->day);
> + BCD_TO_BIN(rtc->mon);
> + BCD_TO_BIN(rtc->year);
> }
>
> - if ( (year += 1900) < 1970 )
> - year += 100;
> -
> - return mktime(year, mon, day, hour, min, sec);
> + if ( (rtc->year += 1900) < 1970 )
> + rtc->year += 100;
> }
>
> static unsigned long get_cmos_time(void)
> {
> unsigned long res, flags;
> - int i;
> + struct rtc_time rtc;
> + unsigned int seconds = 60;
> + static bool_t __read_mostly cmos_rtc_probe;
> + boolean_param("cmos-rtc-probe", cmos_rtc_probe);
>
> if ( efi_enabled )
> {
> @@ -693,23 +696,58 @@ static unsigned long get_cmos_time(void)
> return res;
> }
>
> - if ( unlikely(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) )
> - panic("System without CMOS RTC must be booted from EFI");
> -
> - spin_lock_irqsave(&rtc_lock, flags);
> -
> - /* read RTC exactly on falling edge of update flag */
> - for ( i = 0 ; i < 1000000 ; i++ ) /* may take up to 1 second... */
> - if ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
> + if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) )
> + cmos_rtc_probe = 0;
> + else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe )
> + panic("System with no CMOS RTC advertised must be booted from EFI"
> + " (or with command line option \"cmos-rtc-probe\")");
> +
> + for ( ; ; )
> + {
> + s_time_t start, t1, t2;
> +
> + spin_lock_irqsave(&rtc_lock, flags);
> +
> + /* read RTC exactly on falling edge of update flag */
> + start = NOW();
> + do { /* may take up to 1 second... */
> + t1 = NOW() - start;
> + } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
> + t1 <= SECONDS(1) );
Can we not break early if we exceed 1 second an have not seen an UIP ?
> +
> + start = NOW();
> + do { /* must try at least 2.228 ms */
> + t2 = NOW() - start;
> + } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
> + t2 < MILLISECS(3) );
> +
> + __get_cmos_time(&rtc);
> +
> + spin_unlock_irqrestore(&rtc_lock, flags);
> +
> + if ( likely(!cmos_rtc_probe) ||
> + t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
> + rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
> + !rtc.day || rtc.day > 31 ||
> + !rtc.mon || rtc.mon > 12 )
> break;
> - for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
> - if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
> +
> + if ( seconds < 60 )
Seconds doesn't appear to be updated before this point, meaning that we
will reprobe even if we find a plausible RTC.
> + {
> + if ( rtc.sec != seconds )
> + cmos_rtc_probe = 0;
> break;
> + }
> +
> + process_pending_softirqs();
> +
> + seconds = rtc.sec;
> + }
>
> - res = __get_cmos_time();
> + if ( unlikely(cmos_rtc_probe) )
> + panic("No CMOS RTC found - system must be booted from EFI");
What happens in the case that we broke because of the validity checks
for t1,t2 or rtc ? Do we want to differentiate between "no RTC" and
"RTC giving bogus values" ?
~Andrew
>
> - spin_unlock_irqrestore(&rtc_lock, flags);
> - return res;
> + return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
> }
>
> /***************************************************************************
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
2014-07-28 12:40 ` Andrew Cooper
@ 2014-07-28 12:47 ` Jan Beulich
2014-07-28 13:04 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-07-28 12:47 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser, Tim Deegan
>>> On 28.07.14 at 14:40, <andrew.cooper3@citrix.com> wrote:
> On 25/07/14 15:57, Jan Beulich wrote:
>> + for ( ; ; )
>> + {
>> + s_time_t start, t1, t2;
>> +
>> + spin_lock_irqsave(&rtc_lock, flags);
>> +
>> + /* read RTC exactly on falling edge of update flag */
>> + start = NOW();
>> + do { /* may take up to 1 second... */
>> + t1 = NOW() - start;
>> + } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
>> + t1 <= SECONDS(1) );
>
> Can we not break early if we exceed 1 second an have not seen an UIP ?
Maybe, but I didn't want to make changes to the logic where not
necessary.
>> +
>> + start = NOW();
>> + do { /* must try at least 2.228 ms */
>> + t2 = NOW() - start;
>> + } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
>> + t2 < MILLISECS(3) );
>> +
>> + __get_cmos_time(&rtc);
>> +
>> + spin_unlock_irqrestore(&rtc_lock, flags);
>> +
>> + if ( likely(!cmos_rtc_probe) ||
>> + t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
>> + rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
>> + !rtc.day || rtc.day > 31 ||
>> + !rtc.mon || rtc.mon > 12 )
>> break;
>> - for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
>> - if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
>> +
>> + if ( seconds < 60 )
>
> Seconds doesn't appear to be updated before this point, meaning that we
> will reprobe even if we find a plausible RTC.
But that's exactly the point: We want to go through the loop twice.
Only if the second round results in updated seconds do we consider
the RTC okay for use.
>> + {
>> + if ( rtc.sec != seconds )
>> + cmos_rtc_probe = 0;
>> break;
>> + }
>> +
>> + process_pending_softirqs();
>> +
>> + seconds = rtc.sec;
>> + }
>>
>> - res = __get_cmos_time();
>> + if ( unlikely(cmos_rtc_probe) )
>> + panic("No CMOS RTC found - system must be booted from EFI");
>
> What happens in the case that we broke because of the validity checks
> for t1,t2 or rtc ? Do we want to differentiate between "no RTC" and
> "RTC giving bogus values" ?
How would you suggest to tell one from the other?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
2014-07-28 12:47 ` Jan Beulich
@ 2014-07-28 13:04 ` Andrew Cooper
2014-07-28 13:32 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2014-07-28 13:04 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Tim Deegan
On 28/07/14 13:47, Jan Beulich wrote:
>
>>> +
>>> + start = NOW();
>>> + do { /* must try at least 2.228 ms */
>>> + t2 = NOW() - start;
>>> + } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
>>> + t2 < MILLISECS(3) );
>>> +
>>> + __get_cmos_time(&rtc);
>>> +
>>> + spin_unlock_irqrestore(&rtc_lock, flags);
>>> +
>>> + if ( likely(!cmos_rtc_probe) ||
>>> + t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
>>> + rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
>>> + !rtc.day || rtc.day > 31 ||
>>> + !rtc.mon || rtc.mon > 12 )
>>> break;
>>> - for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
>>> - if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
>>> +
>>> + if ( seconds < 60 )
>> Seconds doesn't appear to be updated before this point, meaning that we
>> will reprobe even if we find a plausible RTC.
> But that's exactly the point: We want to go through the loop twice.
> Only if the second round results in updated seconds do we consider
> the RTC okay for use.
Right, but in the case that the RTC is handing back static values (which
is slightly more likely if we are probing something which might not be a
CMOS RTC), we will sit in the loop forever.
If on the second iteration seconds haven’t increased we should declare
the probe to have failed.
>
>>> + {
>>> + if ( rtc.sec != seconds )
>>> + cmos_rtc_probe = 0;
>>> break;
>>> + }
>>> +
>>> + process_pending_softirqs();
>>> +
>>> + seconds = rtc.sec;
>>> + }
>>>
>>> - res = __get_cmos_time();
>>> + if ( unlikely(cmos_rtc_probe) )
>>> + panic("No CMOS RTC found - system must be booted from EFI");
>> What happens in the case that we broke because of the validity checks
>> for t1,t2 or rtc ? Do we want to differentiate between "no RTC" and
>> "RTC giving bogus values" ?
> How would you suggest to tell one from the other?
>
> Jan
>
Now you put it like that, those two cases are rather hard to disentangle.
~Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
2014-07-28 13:04 ` Andrew Cooper
@ 2014-07-28 13:32 ` Jan Beulich
2014-07-28 13:46 ` Andrew Cooper
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2014-07-28 13:32 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser, Tim Deegan
>>> On 28.07.14 at 15:04, <andrew.cooper3@citrix.com> wrote:
> On 28/07/14 13:47, Jan Beulich wrote:
>>
>>>> +
>>>> + start = NOW();
>>>> + do { /* must try at least 2.228 ms */
>>>> + t2 = NOW() - start;
>>>> + } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
>>>> + t2 < MILLISECS(3) );
>>>> +
>>>> + __get_cmos_time(&rtc);
>>>> +
>>>> + spin_unlock_irqrestore(&rtc_lock, flags);
>>>> +
>>>> + if ( likely(!cmos_rtc_probe) ||
>>>> + t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
>>>> + rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
>>>> + !rtc.day || rtc.day > 31 ||
>>>> + !rtc.mon || rtc.mon > 12 )
>>>> break;
>>>> - for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
>>>> - if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
>>>> +
>>>> + if ( seconds < 60 )
>>> Seconds doesn't appear to be updated before this point, meaning that we
>>> will reprobe even if we find a plausible RTC.
>> But that's exactly the point: We want to go through the loop twice.
>> Only if the second round results in updated seconds do we consider
>> the RTC okay for use.
>
> Right, but in the case that the RTC is handing back static values (which
> is slightly more likely if we are probing something which might not be a
> CMOS RTC), we will sit in the loop forever.
We won't: If seconds is an invalid value, we bail in the first iteration.
If seconds is a valid value but unchanged on the second iteration,
we bail there (see the still available context below.
Jan
>>>> + {
>>>> + if ( rtc.sec != seconds )
>>>> + cmos_rtc_probe = 0;
>>>> break;
>>>> + }
>>>> +
>>>> + process_pending_softirqs();
>>>> +
>>>> + seconds = rtc.sec;
>>>> + }
>>>>
>>>> - res = __get_cmos_time();
>>>> + if ( unlikely(cmos_rtc_probe) )
>>>> + panic("No CMOS RTC found - system must be booted from EFI");
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none
2014-07-28 13:32 ` Jan Beulich
@ 2014-07-28 13:46 ` Andrew Cooper
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2014-07-28 13:46 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Tim Deegan
On 28/07/14 14:32, Jan Beulich wrote:
>>>> On 28.07.14 at 15:04, <andrew.cooper3@citrix.com> wrote:
>> On 28/07/14 13:47, Jan Beulich wrote:
>>>>> +
>>>>> + start = NOW();
>>>>> + do { /* must try at least 2.228 ms */
>>>>> + t2 = NOW() - start;
>>>>> + } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
>>>>> + t2 < MILLISECS(3) );
>>>>> +
>>>>> + __get_cmos_time(&rtc);
>>>>> +
>>>>> + spin_unlock_irqrestore(&rtc_lock, flags);
>>>>> +
>>>>> + if ( likely(!cmos_rtc_probe) ||
>>>>> + t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
>>>>> + rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
>>>>> + !rtc.day || rtc.day > 31 ||
>>>>> + !rtc.mon || rtc.mon > 12 )
>>>>> break;
>>>>> - for ( i = 0 ; i < 1000000 ; i++ ) /* must try at least 2.228 ms */
>>>>> - if ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) )
>>>>> +
>>>>> + if ( seconds < 60 )
>>>> Seconds doesn't appear to be updated before this point, meaning that we
>>>> will reprobe even if we find a plausible RTC.
>>> But that's exactly the point: We want to go through the loop twice.
>>> Only if the second round results in updated seconds do we consider
>>> the RTC okay for use.
>> Right, but in the case that the RTC is handing back static values (which
>> is slightly more likely if we are probing something which might not be a
>> CMOS RTC), we will sit in the loop forever.
> We won't: If seconds is an invalid value, we bail in the first iteration.
> If seconds is a valid value but unchanged on the second iteration,
> we bail there (see the still available context below.
>
> Jan
Silly me. I had mentally included the break in the if ( rtc.sec !=
seconds ) scope.
In which case, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Unfortunately we don't have affected hardware to test with.
~Andrew
>
>>>>> + {
>>>>> + if ( rtc.sec != seconds )
>>>>> + cmos_rtc_probe = 0;
>>>>> break;
>>>>> + }
>>>>> +
>>>>> + process_pending_softirqs();
>>>>> +
>>>>> + seconds = rtc.sec;
>>>>> + }
>>>>>
>>>>> - res = __get_cmos_time();
>>>>> + if ( unlikely(cmos_rtc_probe) )
>>>>> + panic("No CMOS RTC found - system must be booted from EFI");
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-28 13:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-25 14:57 [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none Jan Beulich
2014-07-28 12:40 ` Andrew Cooper
2014-07-28 12:47 ` Jan Beulich
2014-07-28 13:04 ` Andrew Cooper
2014-07-28 13:32 ` Jan Beulich
2014-07-28 13:46 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.