From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH] x86/ACPI: allow CMOS RTC use even when ACPI
 says there is none
Date: Mon, 28 Jul 2014 14:04:59 +0100
Message-ID: <53D64A7B.9080600@citrix.com>
References: <53D28C91020000780002613E@mail.emea.novell.com>
	<53D644AF.8000801@citrix.com>
	<53D6627B02000078000269B3@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta4.messagelabs.com ([85.158.143.247])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <Andrew.Cooper3@citrix.com>) id 1XBkbw-0001n3-7e
	for xen-devel@lists.xenproject.org; Mon, 28 Jul 2014 13:05:04 +0000
In-Reply-To: <53D6627B02000078000269B3@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>, Tim Deegan <tim@xen.org>
List-Id: xen-devel@lists.xenproject.org

On 28/07/14 13:47, Jan Beulich wrote:
>
>>> +
>>> +        start =3D NOW();
>>> +        do { /* must try at least 2.228 ms */
>>> +            t2 =3D 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 >=3D MILLISECS(3) ||
>>> +             rtc.sec >=3D 60 || rtc.min >=3D 60 || rtc.hour >=3D 24 ||
>>> +             !rtc.day || rtc.day > 31 ||
>>> +             !rtc.mon || rtc.mon > 12 )
>>>              break;
>>> -    for ( i =3D 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=92t increased we should declare
the probe to have failed.

>
>>> +        {
>>> +            if ( rtc.sec !=3D seconds )
>>> +                cmos_rtc_probe =3D 0;
>>>              break;
>>> +        }
>>> +
>>> +        process_pending_softirqs();
>>> +
>>> +        seconds =3D rtc.sec;
>>> +    }
>>>  =

>>> -    res =3D __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