Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI says there is none

[Andrew Cooper <andrew.cooper3@citrix.com>]

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