Re: [PATCH v2 1/4] x86/time: CMOS RTC may run in binary mode

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Wed, Sep 02, 2026 at 11:02:43AM +0200, Jan Beulich wrote:
> On 02.09.2026 10:04, Roger Pau Monné wrote:
> > On Thu, Jul 02, 2026 at 11:29:11AM +0200, Jan Beulich wrote:
> >> --- a/docs/misc/xen-command-line.pandoc
> >> +++ b/docs/misc/xen-command-line.pandoc
> >> @@ -339,6 +339,14 @@ 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.
> >> +
> > 
> > Likely too late for it now, but I get the feeling we should have
> > introduced a cmos option, with rtc-bcd and rtc-probe as boolean sub
> > options:
> > 
> > cmos = [ rtc-probe, rtc-bcd ]
> 
> I was thinking the same - would be nice, but here we are.
> 
> >> @@ -1353,6 +1356,48 @@ 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;
> > 
> > Is there a risk of this loop triggering the watchdog, and hence we
> > should process softirqs in the loop? (or otherwise have some kind of
> > hard loop stop after certain iterations / time)
> > 
> > Oh, I now see the mention in the commit message and also note this is
> > done ahead of SMP and also ahead of the watchdog being enabled, hence
> > it can't trigger the watchdog.
> 
> Right. I can't conclude whether you're asking for any change here, as
> there also was no ack.

Hehe, I guess I was probing whether you wanted to do anything about
the proliferation of cmos related top-level options.

Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>

Albeit I think it would be nice to introduce a common cmos option in a
future patch and deprecate the separate top-level ones.

Thanks, Roger.