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

[Ian Lepore]

Author: ian
Date: Wed Jul 12 02:42:57 2017
New Revision: 320901
URL: https://svnweb.freebsd.org/changeset/base/320901

Log:
  Protect access to the AT realtime clock with its own mutex.
  
  The mutex protecting access to the registered realtime clock should not be
  overloaded to protect access to the atrtc hardware, which might not even be
  the registered rtc. More importantly, the resettodr mutex needs to be
  eliminated to remove locking/sleeping restrictions on clock drivers, and
  that can't happen if MD code for amd64 depends on it. This change moves the
  protection into what's really being protected: access to the atrtc date and
  time registers.
  
  This change also adds protection when the clock is accessed from
  xentimer_settime(), which bypasses the resettodr locking.
  
  Differential Revision:	https://reviews.freebsd.org/D11483

Modified:
  head/sys/amd64/amd64/efirt.c
  head/sys/isa/rtc.h
  head/sys/x86/isa/atrtc.c

Modified: head/sys/amd64/amd64/efirt.c
==============================================================================
--- head/sys/amd64/amd64/efirt.c	Tue Jul 11 21:55:20 2017	(r320900)
+++ head/sys/amd64/amd64/efirt.c	Wed Jul 12 02:42:57 2017	(r320901)
@@ -46,6 +46,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sysctl.h>
 #include <sys/systm.h>
 #include <sys/vmmeter.h>
+#include <isa/rtc.h>
 #include <machine/fpu.h>
 #include <machine/efi.h>
 #include <machine/metadata.h>
@@ -445,7 +446,7 @@ efi_get_time_locked(struct efi_tm *tm)
 	efi_status status;
 	int error;
 
-	mtx_assert(&resettodr_lock, MA_OWNED);
+	mtx_assert(&atrtc_time_lock, MA_OWNED);
 	error = efi_enter();
 	if (error != 0)
 		return (error);
@@ -462,9 +463,9 @@ efi_get_time(struct efi_tm *tm)
 
 	if (efi_runtime == NULL)
 		return (ENXIO);
-	mtx_lock(&resettodr_lock);
+	mtx_lock(&atrtc_time_lock);
 	error = efi_get_time_locked(tm);
-	mtx_unlock(&resettodr_lock);
+	mtx_unlock(&atrtc_time_lock);
 	return (error);
 }
 
@@ -487,7 +488,7 @@ efi_set_time_locked(struct efi_tm *tm)
 	efi_status status;
 	int error;
 
-	mtx_assert(&resettodr_lock, MA_OWNED);
+	mtx_assert(&atrtc_time_lock, MA_OWNED);
 	error = efi_enter();
 	if (error != 0)
 		return (error);
@@ -504,9 +505,9 @@ efi_set_time(struct efi_tm *tm)
 
 	if (efi_runtime == NULL)
 		return (ENXIO);
-	mtx_lock(&resettodr_lock);
+	mtx_lock(&atrtc_time_lock);
 	error = efi_set_time_locked(tm);
-	mtx_unlock(&resettodr_lock);
+	mtx_unlock(&atrtc_time_lock);
 	return (error);
 }
 

Modified: head/sys/isa/rtc.h
==============================================================================
--- head/sys/isa/rtc.h	Tue Jul 11 21:55:20 2017	(r320900)
+++ head/sys/isa/rtc.h	Wed Jul 12 02:42:57 2017	(r320901)
@@ -113,6 +113,7 @@
 
 #ifdef _KERNEL
 extern  struct mtx clock_lock;
+extern  struct mtx atrtc_time_lock;
 extern	int atrtcclock_disable;
 int	rtcin(int reg);
 void	atrtc_restore(void);

Modified: head/sys/x86/isa/atrtc.c
==============================================================================
--- head/sys/x86/isa/atrtc.c	Tue Jul 11 21:55:20 2017	(r320900)
+++ head/sys/x86/isa/atrtc.c	Wed Jul 12 02:42:57 2017	(r320901)
@@ -53,9 +53,17 @@ __FBSDID("$FreeBSD$");
 #include <machine/intr_machdep.h>
 #include "clock_if.h"
 
+/*
+ * clock_lock protects low-level access to individual hardware registers.
+ * atrtc_time_lock protects the entire sequence of accessing multiple registers
+ * to read or write the date and time.
+ */
 #define	RTC_LOCK	do { if (!kdb_active) mtx_lock_spin(&clock_lock); } while (0)
 #define	RTC_UNLOCK	do { if (!kdb_active) mtx_unlock_spin(&clock_lock); } while (0)
 
+struct mtx atrtc_time_lock;
+MTX_SYSINIT(atrtc_lock_init, &atrtc_time_lock, "atrtc", MTX_DEF);
+
 int	atrtcclock_disable = 0;
 
 static	int	rtc_reg = -1;
@@ -163,6 +171,8 @@ atrtc_set(struct timespec *ts)
 
 	clock_ts_to_ct(ts, &ct);
 
+	mtx_lock(&atrtc_time_lock);
+
 	/* Disable RTC updates and interrupts. */
 	writertc(RTC_STATUSB, RTCSB_HALT | RTCSB_24HR);
 
@@ -181,6 +191,8 @@ atrtc_set(struct timespec *ts)
 	/* Re-enable RTC updates and interrupts. */
 	writertc(RTC_STATUSB, rtc_statusb);
 	rtcin(RTC_INTR);
+
+	mtx_unlock(&atrtc_time_lock);
 }
 
 /**********************************************************************
@@ -352,6 +364,7 @@ atrtc_gettime(device_t dev, struct timespec *ts)
 	 * to make sure that no more than 240us pass after we start reading,
 	 * and try again if so.
 	 */
+	mtx_lock(&atrtc_time_lock);
 	while (rtcin(RTC_STATUSA) & RTCSA_TUP)
 		continue;
 	critical_enter();
@@ -369,6 +382,7 @@ atrtc_gettime(device_t dev, struct timespec *ts)
 	ct.year += (ct.year < 80 ? 2000 : 1900);
 #endif
 	critical_exit();
+	mtx_unlock(&atrtc_time_lock);
 	/* Set dow = -1 because some clocks don't set it correctly. */
 	ct.dow = -1;
 	return (clock_ct_to_ts(&ct, ts));