WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] x86: consistently serialize CMOS/RTC accesses on rtc

To: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] x86: consistently serialize CMOS/RTC accesses on rtc_lock
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Tue, 19 Jul 2011 09:07:43 +0100
Delivery-date: Tue, 19 Jul 2011 01:08:35 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxx>

--- 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;
 }


Attachment: x86-cmos-lock.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] x86: consistently serialize CMOS/RTC accesses on rtc_lock, Jan Beulich <=