svn commit: r320901 - in head/sys: amd64/amd64 isa x86/isa

[Bruce Evans]

On Mon, 17 Jul 2017, Konstantin Belousov wrote:

> On Mon, Jul 17, 2017 at 10:29:33AM -0600, Ian Lepore wrote:
>> I assumed the reason the efirt code was using the same mutex that
>> protected access to the at rtc was because under the hood, the efi rt
>> clock is really the same hardware too, accessed via bios code.
>
> EFI spec states it explicitely, e.g. in the description of the GetTime()
> function of the Time Services interface:
>
> During runtime, if a PC-AT CMOS device is present in the platform the
> caller must synchronize access to the device before calling GetTime().

The clock_lock mutex still protects individual cmos registers, provided
efi uses rtcin() etc., which it apparently doesn't.

The new atrtc_time_lock mutex doesn't protect even the rtc cmos
registers as a group.  Other subsystems can still access the registers
holding only their own lock or just clock_lock.  Only write accesses
would be harmful.  Read accesses do occur if rtcintr() is active, but
these only change the index register.

Even the rtc subsystem has (groups of) write accesses that are not
properly locked.  These are for (re)initializations.  Reinitializations
occur for changing the rtc rate for profiling and for resuming.  There
is no locking except possibly Giant for even the rtc_status[ab] images
vs their registers.  Giant locking might work for that, but it doesn't
work for settime() calling rtc code, at least after removing Giant
from settime().

If efi accesses the rtc directly, then that is just broken and sharing the
new mutex doesn't help much.  The new mutex only prevents contention between
efi code that uses it and rtc code that use it -- that is, just the rtc
timer parts of the rtc code (as the name of the new mutex suggests).
The following remain unprotected:
- rtc initialization code
- more seriously, rtc interrupt code and rtcin()/writertc() generally.
resettodr_mtx was no better.  But further abuse of clock_lock might have
worked.  Holding clock_lock around efi_runtime->rt_gettime() calls would
prevent both rtc timer code and rtc_intr() from contending.  I can't
find rt_gettime().  I think these sources of contention are normally
inactive if efi is used, but efi can still race with rtc initialization,
the cmos driver, and possibly acpi code.

Bruce