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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.07.14 at 14:40, <andrew.cooper3@xxxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel