On 23.03.2023 15:49, Roger Pau Monné wrote:
> On Mon, Mar 20, 2023 at 09:32:26AM +0100, Jan Beulich wrote:
>> @@ -1249,6 +1252,80 @@ static unsigned long get_cmos_time(void)
>> return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
>> }
>>
>> +static unsigned int __ro_after_init cmos_alias_mask;
>> +
>> +static int __init cf_check probe_cmos_alias(void)
>> +{
>> + unsigned int i, offs;
>> +
>> + if ( acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC )
>> + return 0;
>> +
>> + for ( offs = 2; offs < 8; offs <<= 1 )
>> + {
>> + bool read = true;
>
> You can limit the scope of i to the inner for loop.
>
>> +
>> + for ( i = RTC_REG_D + 1; i < 0x80; ++i )
>> + {
>> + uint8_t normal, alt;
>> + unsigned long flags;
>> +
>> + if ( i == acpi_gbl_FADT.century )
>> + continue;
>> +
>> + spin_lock_irqsave(&rtc_lock, flags);
>> +
>> + normal = CMOS_READ(i);
>> + if ( inb(RTC_PORT(offs)) != i )
>> + read = false;
>> +
>> + alt = inb(RTC_PORT(offs + 1));
>> +
>> + spin_unlock_irqrestore(&rtc_lock, flags);
>> +
>> + if ( normal != alt )
>> + break;
>> +
>> + process_pending_softirqs();
>
> You adding a call to process pending softirqs for every loop
> iteration makes me wonder how long is each of those accesses expected
> to take, since we could be performing a lot of them (0x80 * 3).
>
> I don't think so, but there can not be any side effects from reading
> from the CMOS RAM I would assume, even for cases where the CMOS ports
> are not aliases?
>
> I would assume ports to be either aliased to the CMOS, or otherwise
> reserved. What makes me wonder if it wouldn't be simpler to just
> passthough accesses to all the possible CMOS alias ports.
I'm afraid this assumption doesn't hold, as can be seen from the ICH10
datasheet that I did point you at the other day. There ports 72/73
serve a purpose different from ports 70/71 (and their aliases at 74/75).
Unless (as the datasheet calls it) U128E is clear (wherever that bit
lives), in which case 72/73 (and 76/77) will also be aliases of 70/71.
So we won't get away without probing, and if we deem probing too risky,
then all I can do is drop this patch.
Jan
