[PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 7757 bytes --]

... in order to also intercept accesses through the alias ports.

Also stop intercepting accesses to the CMOS ports if we won't ourselves
use the CMOS RTC.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1544,42 +1544,6 @@ int __hwdom_init xen_in_range(unsigned l
     return 0;
 }
 
-static int __hwdom_init io_bitmap_cb(unsigned long s, unsigned long e,
-                                     void *ctx)
-{
-    struct domain *d = ctx;
-    unsigned int i;
-
-    ASSERT(e <= INT_MAX);
-    for ( i = s; i <= e; i++ )
-        __clear_bit(i, d->arch.hvm_domain.io_bitmap);
-
-    return 0;
-}
-
-void __hwdom_init setup_io_bitmap(struct domain *d)
-{
-    int rc;
-
-    if ( has_hvm_container_domain(d) )
-    {
-        bitmap_fill(d->arch.hvm_domain.io_bitmap, 0x10000);
-        rc = rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
-                                    io_bitmap_cb, d);
-        BUG_ON(rc);
-        /*
-         * NB: we need to trap accesses to 0xcf8 in order to intercept
-         * 4 byte accesses, that need to be handled by Xen in order to
-         * keep consistency.
-         * Access to 1 byte RTC ports also needs to be trapped in order
-         * to keep consistency with PV.
-         */
-        __set_bit(0xcf8, d->arch.hvm_domain.io_bitmap);
-        __set_bit(RTC_PORT(0), d->arch.hvm_domain.io_bitmap);
-        __set_bit(RTC_PORT(1), d->arch.hvm_domain.io_bitmap);
-    }
-}
-
 /*
  * Local variables:
  * mode: C
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -736,7 +736,10 @@ static unsigned long get_cmos_time(void)
         if ( seconds < 60 )
         {
             if ( rtc.sec != seconds )
+            {
                 cmos_rtc_probe = 0;
+                acpi_gbl_FADT.boot_flags &= ~ACPI_FADT_NO_CMOS_RTC;
+            }
             break;
         }
 
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -62,6 +62,7 @@
 #include <asm/xstate.h>
 #include <asm/debugger.h>
 #include <asm/msr.h>
+#include <asm/setup.h>
 #include <asm/shared.h>
 #include <asm/x86_emulate.h>
 #include <asm/traps.h>
@@ -105,6 +106,8 @@ idt_entry_t *idt_tables[NR_CPUS] __read_
 void (*ioemul_handle_quirk)(
     u8 opcode, char *io_emul_stub, struct cpu_user_regs *regs);
 
+static unsigned int __read_mostly cmos_alias_mask;
+
 static int debug_stack_lines = 20;
 integer_param("debug_stack_lines", debug_stack_lines);
 
@@ -1769,8 +1772,12 @@ static bool_t admin_io_okay(unsigned int
     if ( (port == 0xcf8) && (bytes == 4) )
         return 0;
 
-    /* We also never permit direct access to the RTC/CMOS registers. */
-    if ( ((port & ~1) == RTC_PORT(0)) )
+    /*
+     * We also never permit direct access to the RTC/CMOS registers
+     * if we may be accessing the RTC ones ourselves.
+     */
+    if ( !(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) &&
+         ((port & ~(cmos_alias_mask | 1)) == RTC_PORT(0)) )
         return 0;
 
     return ioports_access_permitted(d, port, port + bytes - 1);
@@ -1842,18 +1849,19 @@ uint32_t guest_io_read(unsigned int port
         {
             sub_data = pv_pit_handler(port, 0, 0);
         }
-        else if ( (port == RTC_PORT(0)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(0)) )
         {
             sub_data = currd->arch.cmos_idx;
         }
-        else if ( (port == RTC_PORT(1)) &&
-                  ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(1)) &&
+                  ioports_access_permitted(currd, port - 1, port) )
         {
             unsigned long flags;
 
             spin_lock_irqsave(&rtc_lock, flags);
-            outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0));
-            sub_data = inb(RTC_PORT(1));
+            outb(currd->arch.cmos_idx & (0xff >> (port == RTC_PORT(1))),
+                 port - 1);
+            sub_data = inb(port);
             spin_unlock_irqrestore(&rtc_lock, flags);
         }
         else if ( (port == 0xcf8) && (bytes == 4) )
@@ -1911,20 +1919,22 @@ void guest_io_write(unsigned int port, u
         {
             pv_pit_handler(port, (uint8_t)data, 1);
         }
-        else if ( (port == RTC_PORT(0)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(0)) )
         {
             currd->arch.cmos_idx = data;
         }
-        else if ( (port == RTC_PORT(1)) &&
-                  ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(1)) &&
+                  ioports_access_permitted(currd, port - 1, port) )
         {
+            unsigned int idx = currd->arch.cmos_idx &
+                               (0xff >> (port == RTC_PORT(1)));
             unsigned long flags;
 
             if ( pv_rtc_handler )
-                pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data);
+                pv_rtc_handler(idx, data);
             spin_lock_irqsave(&rtc_lock, flags);
-            outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0));
-            outb(data, RTC_PORT(1));
+            outb(idx, port - 1);
+            outb(data, port);
             spin_unlock_irqrestore(&rtc_lock, flags);
         }
         else if ( (port == 0xcf8) && (bytes == 4) )
@@ -3727,6 +3737,84 @@ void __init trap_init(void)
     open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
+static int __init 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_t read = 1;
+
+        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 = 0;
+
+            alt = inb(RTC_PORT(offs + 1));
+
+            spin_unlock_irqrestore(&rtc_lock, flags);
+
+            if ( normal != alt )
+                break;
+
+            process_pending_softirqs();
+        }
+        if ( i == 0x80 )
+        {
+            cmos_alias_mask |= offs;
+            printk(XENLOG_INFO "CMOS aliased at %02x, index %s\n",
+                   RTC_PORT(offs), read ? "r/w" : "w/o");
+        }
+    }
+
+    return 0;
+}
+__initcall(probe_cmos_alias);
+
+static int __hwdom_init io_bitmap_cb(unsigned long s, unsigned long e,
+                                     void *ctx)
+{
+    const struct domain *d = ctx;
+    unsigned int i;
+
+    ASSERT(e <= INT_MAX);
+    for ( i = s; i <= e; i++ )
+        if ( admin_io_okay(i, 1, d) )
+            __clear_bit(i, d->arch.hvm_domain.io_bitmap);
+
+    return 0;
+}
+
+void __hwdom_init setup_io_bitmap(struct domain *d)
+{
+    if ( !has_hvm_container_domain(d) )
+        return;
+
+    bitmap_fill(d->arch.hvm_domain.io_bitmap, 0x10000);
+    if ( rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
+                                io_bitmap_cb, d) )
+        BUG();
+
+    /*
+     * We need to trap 4-byte accesses to 0xcf8 (see admin_io_okay(),
+     * guest_io_read(), and guest_io_write()), which isn't covered by
+     * the admin_io_okay() check in io_bitmap_cb().
+     */
+    __set_bit(0xcf8, d->arch.hvm_domain.io_bitmap);
+}
+
 long register_guest_nmi_callback(unsigned long address)
 {
     struct vcpu *v = current;



[-- Attachment #2: x86-CMOS-aliasing.patch --]
[-- Type: text/plain, Size: 7812 bytes --]

x86: detect CMOS aliasing on ports other then 0x70/0x71

... in order to also intercept accesses through the alias ports.

Also stop intercepting accesses to the CMOS ports if we won't ourselves
use the CMOS RTC.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1544,42 +1544,6 @@ int __hwdom_init xen_in_range(unsigned l
     return 0;
 }
 
-static int __hwdom_init io_bitmap_cb(unsigned long s, unsigned long e,
-                                     void *ctx)
-{
-    struct domain *d = ctx;
-    unsigned int i;
-
-    ASSERT(e <= INT_MAX);
-    for ( i = s; i <= e; i++ )
-        __clear_bit(i, d->arch.hvm_domain.io_bitmap);
-
-    return 0;
-}
-
-void __hwdom_init setup_io_bitmap(struct domain *d)
-{
-    int rc;
-
-    if ( has_hvm_container_domain(d) )
-    {
-        bitmap_fill(d->arch.hvm_domain.io_bitmap, 0x10000);
-        rc = rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
-                                    io_bitmap_cb, d);
-        BUG_ON(rc);
-        /*
-         * NB: we need to trap accesses to 0xcf8 in order to intercept
-         * 4 byte accesses, that need to be handled by Xen in order to
-         * keep consistency.
-         * Access to 1 byte RTC ports also needs to be trapped in order
-         * to keep consistency with PV.
-         */
-        __set_bit(0xcf8, d->arch.hvm_domain.io_bitmap);
-        __set_bit(RTC_PORT(0), d->arch.hvm_domain.io_bitmap);
-        __set_bit(RTC_PORT(1), d->arch.hvm_domain.io_bitmap);
-    }
-}
-
 /*
  * Local variables:
  * mode: C
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -736,7 +736,10 @@ static unsigned long get_cmos_time(void)
         if ( seconds < 60 )
         {
             if ( rtc.sec != seconds )
+            {
                 cmos_rtc_probe = 0;
+                acpi_gbl_FADT.boot_flags &= ~ACPI_FADT_NO_CMOS_RTC;
+            }
             break;
         }
 
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -62,6 +62,7 @@
 #include <asm/xstate.h>
 #include <asm/debugger.h>
 #include <asm/msr.h>
+#include <asm/setup.h>
 #include <asm/shared.h>
 #include <asm/x86_emulate.h>
 #include <asm/traps.h>
@@ -105,6 +106,8 @@ idt_entry_t *idt_tables[NR_CPUS] __read_
 void (*ioemul_handle_quirk)(
     u8 opcode, char *io_emul_stub, struct cpu_user_regs *regs);
 
+static unsigned int __read_mostly cmos_alias_mask;
+
 static int debug_stack_lines = 20;
 integer_param("debug_stack_lines", debug_stack_lines);
 
@@ -1769,8 +1772,12 @@ static bool_t admin_io_okay(unsigned int
     if ( (port == 0xcf8) && (bytes == 4) )
         return 0;
 
-    /* We also never permit direct access to the RTC/CMOS registers. */
-    if ( ((port & ~1) == RTC_PORT(0)) )
+    /*
+     * We also never permit direct access to the RTC/CMOS registers
+     * if we may be accessing the RTC ones ourselves.
+     */
+    if ( !(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) &&
+         ((port & ~(cmos_alias_mask | 1)) == RTC_PORT(0)) )
         return 0;
 
     return ioports_access_permitted(d, port, port + bytes - 1);
@@ -1842,18 +1849,19 @@ uint32_t guest_io_read(unsigned int port
         {
             sub_data = pv_pit_handler(port, 0, 0);
         }
-        else if ( (port == RTC_PORT(0)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(0)) )
         {
             sub_data = currd->arch.cmos_idx;
         }
-        else if ( (port == RTC_PORT(1)) &&
-                  ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(1)) &&
+                  ioports_access_permitted(currd, port - 1, port) )
         {
             unsigned long flags;
 
             spin_lock_irqsave(&rtc_lock, flags);
-            outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0));
-            sub_data = inb(RTC_PORT(1));
+            outb(currd->arch.cmos_idx & (0xff >> (port == RTC_PORT(1))),
+                 port - 1);
+            sub_data = inb(port);
             spin_unlock_irqrestore(&rtc_lock, flags);
         }
         else if ( (port == 0xcf8) && (bytes == 4) )
@@ -1911,20 +1919,22 @@ void guest_io_write(unsigned int port, u
         {
             pv_pit_handler(port, (uint8_t)data, 1);
         }
-        else if ( (port == RTC_PORT(0)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(0)) )
         {
             currd->arch.cmos_idx = data;
         }
-        else if ( (port == RTC_PORT(1)) &&
-                  ioports_access_permitted(currd, RTC_PORT(0), RTC_PORT(1)) )
+        else if ( ((port & ~cmos_alias_mask) == RTC_PORT(1)) &&
+                  ioports_access_permitted(currd, port - 1, port) )
         {
+            unsigned int idx = currd->arch.cmos_idx &
+                               (0xff >> (port == RTC_PORT(1)));
             unsigned long flags;
 
             if ( pv_rtc_handler )
-                pv_rtc_handler(currd->arch.cmos_idx & 0x7f, data);
+                pv_rtc_handler(idx, data);
             spin_lock_irqsave(&rtc_lock, flags);
-            outb(currd->arch.cmos_idx & 0x7f, RTC_PORT(0));
-            outb(data, RTC_PORT(1));
+            outb(idx, port - 1);
+            outb(data, port);
             spin_unlock_irqrestore(&rtc_lock, flags);
         }
         else if ( (port == 0xcf8) && (bytes == 4) )
@@ -3727,6 +3737,84 @@ void __init trap_init(void)
     open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
+static int __init 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_t read = 1;
+
+        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 = 0;
+
+            alt = inb(RTC_PORT(offs + 1));
+
+            spin_unlock_irqrestore(&rtc_lock, flags);
+
+            if ( normal != alt )
+                break;
+
+            process_pending_softirqs();
+        }
+        if ( i == 0x80 )
+        {
+            cmos_alias_mask |= offs;
+            printk(XENLOG_INFO "CMOS aliased at %02x, index %s\n",
+                   RTC_PORT(offs), read ? "r/w" : "w/o");
+        }
+    }
+
+    return 0;
+}
+__initcall(probe_cmos_alias);
+
+static int __hwdom_init io_bitmap_cb(unsigned long s, unsigned long e,
+                                     void *ctx)
+{
+    const struct domain *d = ctx;
+    unsigned int i;
+
+    ASSERT(e <= INT_MAX);
+    for ( i = s; i <= e; i++ )
+        if ( admin_io_okay(i, 1, d) )
+            __clear_bit(i, d->arch.hvm_domain.io_bitmap);
+
+    return 0;
+}
+
+void __hwdom_init setup_io_bitmap(struct domain *d)
+{
+    if ( !has_hvm_container_domain(d) )
+        return;
+
+    bitmap_fill(d->arch.hvm_domain.io_bitmap, 0x10000);
+    if ( rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
+                                io_bitmap_cb, d) )
+        BUG();
+
+    /*
+     * We need to trap 4-byte accesses to 0xcf8 (see admin_io_okay(),
+     * guest_io_read(), and guest_io_write()), which isn't covered by
+     * the admin_io_okay() check in io_bitmap_cb().
+     */
+    __set_bit(0xcf8, d->arch.hvm_domain.io_bitmap);
+}
+
 long register_guest_nmi_callback(unsigned long address)
 {
     struct vcpu *v = current;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Andrew Cooper]

On 22/09/15 14:10, Jan Beulich wrote:
> +    for ( offs = 2; offs < 8; offs <<= 1 )
> +    {
> +        bool_t read = 1;
> +
> +        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 = 0;
> +
> +            alt = inb(RTC_PORT(offs + 1));
> +
> +            spin_unlock_irqrestore(&rtc_lock, flags);
> +
> +            if ( normal != alt )
> +                break;

Even with a manual to hand, this logic is quite hard to understand. 
Furthermore, I still cant spot how your r/w vs w/o logic is supposed to
work.  It doesn't check the writability of the alias, but of the aliases
index.

However, it is not robust to the system servicing an SMI and altering
the CMOS ram in the middle of this loop.  Such a modification would
cause the loop to believe that this specific 'offs' is not an alias even
when it actually is.

One option would be to reread the non-aliased port again, but that would
add yet more io reads.

~Andrew

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Jan Beulich]

>>> On 23.09.15 at 20:34, <andrew.cooper3@citrix.com> wrote:
> On 22/09/15 14:10, Jan Beulich wrote:
>> +    for ( offs = 2; offs < 8; offs <<= 1 )
>> +    {
>> +        bool_t read = 1;
>> +
>> +        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 = 0;
>> +
>> +            alt = inb(RTC_PORT(offs + 1));
>> +
>> +            spin_unlock_irqrestore(&rtc_lock, flags);
>> +
>> +            if ( normal != alt )
>> +                break;
> 
> Even with a manual to hand, this logic is quite hard to understand. 
> Furthermore, I still cant spot how your r/w vs w/o logic is supposed to
> work.  It doesn't check the writability of the alias, but of the aliases
> index.

But that's the only interesting thing. A w/o alias would be rather
strange. It's the index register that traditionally hasn't been
readable, but has got means added in some chipsets to be read
back. For the moment we don't make use of this information
anyway, i.e. it's more a thing usable for validation that what the
logic determines matches with how the chipset is documented to
behave (if someone wanted to check that).

> However, it is not robust to the system servicing an SMI and altering
> the CMOS ram in the middle of this loop.  Such a modification would
> cause the loop to believe that this specific 'offs' is not an alias even
> when it actually is.
> 
> One option would be to reread the non-aliased port again, but that would
> add yet more io reads.

SMI is always a problem. Even if we re-read the register, we still
couldn't be sure we haven't got hit by another SMI. Considering
the index/data port access model I would anyway consider it quite
bad for firmware to modify the CMOS in an SMI.

Jan

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Andrew Cooper]

On 24/09/15 09:11, Jan Beulich wrote:
>>>> On 23.09.15 at 20:34, <andrew.cooper3@citrix.com> wrote:
>> On 22/09/15 14:10, Jan Beulich wrote:
>>> +    for ( offs = 2; offs < 8; offs <<= 1 )
>>> +    {
>>> +        bool_t read = 1;
>>> +
>>> +        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 = 0;
>>> +
>>> +            alt = inb(RTC_PORT(offs + 1));
>>> +
>>> +            spin_unlock_irqrestore(&rtc_lock, flags);
>>> +
>>> +            if ( normal != alt )
>>> +                break;
>> Even with a manual to hand, this logic is quite hard to understand.
>> Furthermore, I still cant spot how your r/w vs w/o logic is supposed to
>> work.  It doesn't check the writability of the alias, but of the aliases
>> index.
> But that's the only interesting thing. A w/o alias would be rather
> strange. It's the index register that traditionally hasn't been
> readable, but has got means added in some chipsets to be read
> back. For the moment we don't make use of this information
> anyway, i.e. it's more a thing usable for validation that what the
> logic determines matches with how the chipset is documented to
> behave (if someone wanted to check that).
>
>> However, it is not robust to the system servicing an SMI and altering
>> the CMOS ram in the middle of this loop.  Such a modification would
>> cause the loop to believe that this specific 'offs' is not an alias even
>> when it actually is.
>>
>> One option would be to reread the non-aliased port again, but that would
>> add yet more io reads.
> SMI is always a problem. Even if we re-read the register, we still
> couldn't be sure we haven't got hit by another SMI. Considering
> the index/data port access model I would anyway consider it quite
> bad for firmware to modify the CMOS in an SMI.

When one core is executing the SMM handler, all other cores are held 
paused.  (There tends to be some rather complicated cross-socket logic 
to make this happen properly).

It is safe for the SMM handler to use CMOS if it returns the index 
register back to how it found it.  Furthermore, I am willing to bet that 
there real SMM handlers out there which do do this.

~Andrew

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Jan Beulich]

>>> On 25.09.15 at 11:55, <andrew.cooper3@citrix.com> wrote:
> It is safe for the SMM handler to use CMOS if it returns the index 
> register back to how it found it.  Furthermore, I am willing to bet that 
> there real SMM handlers out there which do do this.

So what options do you see then here? Don't do anything (drop the
patch) seems the only one I can see not getting in conflict with SMI.
Yet using the patch as is would not make anything less safe, it would
just have the potential of not always making things as safe as they
could be.

Jan

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Andrew Cooper]

On 25/09/15 12:00, Jan Beulich wrote:
>>>> On 25.09.15 at 11:55, <andrew.cooper3@citrix.com> wrote:
>> It is safe for the SMM handler to use CMOS if it returns the index 
>> register back to how it found it.  Furthermore, I am willing to bet that 
>> there real SMM handlers out there which do do this.
> So what options do you see then here? Don't do anything (drop the
> patch) seems the only one I can see not getting in conflict with SMI.
> Yet using the patch as is would not make anything less safe, it would
> just have the potential of not always making things as safe as they
> could be.

Given some orthogonal interaction with cmos-rtc-probe recently, I am
fairly sure that we can remove all real RTC interaction from Xen.

Dom0 is required to provide fine-grained time via XENPF_settime{32,64}
which gives an absolute time to seed the other domain wallclocks with. 
The RTC was assumed always to be in UTC, which allows full date and time
information to be derived.

It would certainly simplify things to leave all of the RTC in dom0's hand.

~Andrew

Re: [PATCH] x86: detect CMOS aliasing on ports other then 0x70/0x71

[Jan Beulich]

>>> On 29.09.15 at 15:25, <andrew.cooper3@citrix.com> wrote:
> On 25/09/15 12:00, Jan Beulich wrote:
>>>>> On 25.09.15 at 11:55, <andrew.cooper3@citrix.com> wrote:
>>> It is safe for the SMM handler to use CMOS if it returns the index 
>>> register back to how it found it.  Furthermore, I am willing to bet that 
>>> there real SMM handlers out there which do do this.
>> So what options do you see then here? Don't do anything (drop the
>> patch) seems the only one I can see not getting in conflict with SMI.
>> Yet using the patch as is would not make anything less safe, it would
>> just have the potential of not always making things as safe as they
>> could be.
> 
> Given some orthogonal interaction with cmos-rtc-probe recently, I am
> fairly sure that we can remove all real RTC interaction from Xen.
> 
> Dom0 is required to provide fine-grained time via XENPF_settime{32,64}
> which gives an absolute time to seed the other domain wallclocks with. 

Is it? If so, I suppose we should enforce this in some way, e.g. by
failing domain creation until done. Right now I guess we expect that
to happen, but don't really require it.

> The RTC was assumed always to be in UTC, which allows full date and time
> information to be derived.
> 
> It would certainly simplify things to leave all of the RTC in dom0's hand.

Yes, it would.

Jan