Jan Beulich
2011-Jul-19 08:07 UTC
[Xen-devel] [PATCH] x86: consistently serialize CMOS/RTC accesses on rtc_lock
Since RTC/CMOS accesses aren''t atomic, there are possible races between
code paths setting the index register and subsequently reading/writing
the data register. This is supposed to be dealt with by acquiring
rtc_lock, but two places up to now lacked respective synchronization:
Accesses to the EFI time functions and
smpboot_{setup,restore}_warm_reset_vector().
This in turn requires no longer directly passing through guest writes
to the index register, but instead using a machanism similar to that
for PCI config space method 1 accesses.
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/xen/arch/x86/efi/runtime.c
+++ b/xen/arch/x86/efi/runtime.c
@@ -2,8 +2,9 @@
#include <xen/cache.h>
#include <xen/errno.h>
#include <xen/guest_access.h>
-#include <xen/time.h>
#include <xen/irq.h>
+#include <xen/time.h>
+#include <asm/mc146818rtc.h>
DEFINE_XEN_GUEST_HANDLE(CHAR16);
@@ -81,9 +82,11 @@ unsigned long efi_get_time(void)
{
EFI_TIME time;
EFI_STATUS status;
- unsigned long cr3 = efi_rs_enter();
+ unsigned long cr3 = efi_rs_enter(), flags;
+ spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(&time, NULL);
+ spin_unlock_irqrestore(&rtc_lock, flags);
efi_rs_leave(cr3);
if ( EFI_ERROR(status) )
@@ -224,7 +227,7 @@ static inline EFI_GUID *cast_guid(struct
int efi_runtime_call(struct xenpf_efi_runtime_call *op)
{
- unsigned long cr3;
+ unsigned long cr3, flags;
EFI_STATUS status = EFI_NOT_STARTED;
int rc = 0;
@@ -238,7 +241,9 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(cast_time(&op->u.get_time.time),
&caps);
+ spin_unlock_irqrestore(&rtc_lock, flags);
efi_rs_leave(cr3);
if ( !EFI_ERROR(status) )
@@ -256,7 +261,9 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetTime(cast_time(&op->u.set_time));
+ spin_unlock_irqrestore(&rtc_lock, flags);
efi_rs_leave(cr3);
break;
@@ -268,8 +275,10 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetWakeupTime(&enabled, &pending,
cast_time(&op->u.get_wakeup_time));
+ spin_unlock_irqrestore(&rtc_lock, flags);
efi_rs_leave(cr3);
if ( !EFI_ERROR(status) )
@@ -288,12 +297,14 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetWakeupTime(!!(op->misc &
XEN_EFI_SET_WAKEUP_TIME_ENABLE),
(op->misc &
XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY) ?
NULL :
cast_time(&op->u.set_wakeup_time));
+ spin_unlock_irqrestore(&rtc_lock, flags);
efi_rs_leave(cr3);
op->misc = 0;
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -502,18 +502,10 @@ static void hpet_detach_channel(unsigned
#include <asm/mc146818rtc.h>
-void (*__read_mostly pv_rtc_handler)(unsigned int port, uint8_t value);
+void (*__read_mostly pv_rtc_handler)(uint8_t index, uint8_t value);
-static void handle_rtc_once(unsigned int port, uint8_t value)
+static void handle_rtc_once(uint8_t index, uint8_t value)
{
- static int index;
-
- if ( port == 0x70 )
- {
- index = value;
- return;
- }
-
if ( index != RTC_REG_B )
return;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -69,6 +69,7 @@
#include <asm/hypercall.h>
#include <asm/mce.h>
#include <asm/apic.h>
+#include <asm/mc146818rtc.h>
#include <asm/hpet.h>
#include <public/arch-x86/cpuid.h>
@@ -1640,6 +1641,10 @@ static int admin_io_okay(
if ( (port == 0xcf8) && (bytes == 4) )
return 0;
+ /* We also never permit direct access to the RTC/CMOS registers. */
+ if ( ((port & ~1) == RTC_PORT(0)) )
+ return 0;
+
return ioports_access_permitted(v->domain, port, port + bytes - 1);
}
@@ -1669,6 +1674,21 @@ static uint32_t guest_io_read(
{
sub_data = pv_pit_handler(port, 0, 0);
}
+ else if ( (port == RTC_PORT(0)) )
+ {
+ sub_data = v->domain->arch.cmos_idx;
+ }
+ else if ( (port == RTC_PORT(1)) &&
+ ioports_access_permitted(v->domain, RTC_PORT(0),
+ RTC_PORT(1)) )
+ {
+ unsigned long flags;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ outb(v->domain->arch.cmos_idx & 0x7f, RTC_PORT(0));
+ sub_data = inb(RTC_PORT(1));
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ }
else if ( (port == 0xcf8) && (bytes == 4) )
{
size = 4;
@@ -1702,8 +1722,6 @@ static void guest_io_write(
{
switch ( bytes ) {
case 1:
- if ( ((port == 0x70) || (port == 0x71)) && pv_rtc_handler )
- pv_rtc_handler(port, (uint8_t)data);
outb((uint8_t)data, port);
if ( pv_post_outb_hook )
pv_post_outb_hook(port, (uint8_t)data);
@@ -1726,6 +1744,23 @@ static void guest_io_write(
{
pv_pit_handler(port, (uint8_t)data, 1);
}
+ else if ( (port == RTC_PORT(0)) )
+ {
+ v->domain->arch.cmos_idx = data;
+ }
+ else if ( (port == RTC_PORT(1)) &&
+ ioports_access_permitted(v->domain, RTC_PORT(0),
+ RTC_PORT(1)) )
+ {
+ unsigned long flags;
+
+ if ( pv_rtc_handler )
+ pv_rtc_handler(v->domain->arch.cmos_idx & 0x7f,
data);
+ spin_lock_irqsave(&rtc_lock, flags);
+ outb(v->domain->arch.cmos_idx & 0x7f, RTC_PORT(0));
+ outb(data, RTC_PORT(1));
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ }
else if ( (port == 0xcf8) && (bytes == 4) )
{
size = 4;
@@ -2091,10 +2126,6 @@ static int emulate_privileged_op(struct
goto fail;
if ( admin_io_okay(port, op_bytes, v, regs) )
{
- if ( (op_bytes == 1) &&
- ((port == 0x71) || (port == 0x70)) &&
- pv_rtc_handler )
- pv_rtc_handler(port, regs->eax);
io_emul(regs);
if ( (op_bytes == 1) && pv_post_outb_hook )
pv_post_outb_hook(port, regs->eax);
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -258,6 +258,7 @@ struct arch_domain
/* I/O-port admin-specified access capabilities. */
struct rangeset *ioport_caps;
uint32_t pci_cf8;
+ uint8_t cmos_idx;
struct list_head pdev_list;
--- a/xen/include/asm-x86/hpet.h
+++ b/xen/include/asm-x86/hpet.h
@@ -74,6 +74,6 @@ void hpet_broadcast_exit(void);
int hpet_broadcast_is_available(void);
void hpet_disable_legacy_broadcast(void);
-extern void (*pv_rtc_handler)(unsigned int port, uint8_t value);
+extern void (*pv_rtc_handler)(uint8_t reg, uint8_t value);
#endif /* __X86_HPET_H__ */
--- a/xen/include/asm-x86/mach-default/smpboot_hooks.h
+++ b/xen/include/asm-x86/mach-default/smpboot_hooks.h
@@ -3,7 +3,11 @@
static inline void smpboot_setup_warm_reset_vector(unsigned long start_eip)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&rtc_lock, flags);
CMOS_WRITE(0xa, 0xf);
+ spin_unlock_irqrestore(&rtc_lock, flags);
flush_tlb_local();
Dprintk("1.\n");
*((volatile unsigned short *) TRAMPOLINE_HIGH) = start_eip >> 4;
@@ -14,6 +18,8 @@ static inline void smpboot_setup_warm_re
static inline void smpboot_restore_warm_reset_vector(void)
{
+ unsigned long flags;
+
/*
* Install writable page 0 entry to set BIOS data area.
*/
@@ -23,7 +29,9 @@ static inline void smpboot_restore_warm_
* Paranoid: Set warm reset code and vector here back
* to default values.
*/
+ spin_lock_irqsave(&rtc_lock, flags);
CMOS_WRITE(0, 0xf);
+ spin_unlock_irqrestore(&rtc_lock, flags);
*((volatile int *) maddr_to_virt(0x467)) = 0;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel