Attachment 145332 Details for Bug 221239 – Replace inappropriate calls to domain_crash_synchronous

[patch] Replace inappropriate calls to domain_crash_synchronous

p (text/plain), 9.24 KB, created by Herbert Xu on 2007-01-11 11:33:08 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Herbert Xu

Created: 2007-01-11 11:33:08 UTC

Size: 9.24 KB

patch

obsolete

>Stephen C. Tweedie <sct@redhat.com> wrote:
>>
>>> 1) add it as is to RHEL5;
>>> 2) audit each domain_crash_synchronous call individually?
>> 
>> For RHEL-5 we're in final lockdown for RC.  Doing general cleanups atm
>> is out of the question, I think.
>> 
>> Small, targeted, critical bugfixes only is the order of the day right
>> now.  If any of the chunks here qualify, then sure, we should bugzilla
>> them and apply.
>
>OK.  Here is the result of pruning irrelevant and non-trivial changes
>from the original patch.  I've also found a number other patches which
>also removed domain_crash_synchronous calls that are relevant.
>
>I have excluded two shadow patches which I deemed too complex and are
>unlikely to trigger.  I'll post them in a separate email.
>
>They are:
>
># HG changeset patch
># User kfraser@localhost.localdomain
># Node ID b7ffbec0e307f38e8d2322169cccbec699f12d2f
># Parent  f6b7ae6ed5041bfe1168a1070bbcc1e94b89036d
>If domain_crash_synchronous() is called in the context of a multicall,
>the (physical) CPU this was executing on did not get its multicall
>state cleared and would reject any further multicalls, likely leading
>to crashing dom0 at some point.
>
>Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
># HG changeset patch
># User kfraser@localhost.localdomain
># Node ID e821c15b7cfeacbc6eac6e506f8fc1748b98a418
># Parent  fda15861731e39f27d3423c31a0220c2fe2f1efb
>[HVM] vlapic: No use of domain_crash_synchronous().
>The safer domain_crash() is sufficient.
>Signed-off-by: Keir Fraser <keir@xensource.com>
>
>Here is the final patch.  Please ACK.
>
>diff -ur xen.orig/arch/x86/hvm/hvm.c xen/arch/x86/hvm/hvm.c
>--- xen.orig/arch/x86/hvm/hvm.c	2007-01-04 18:13:28.000000000 +1100
>+++ xen/arch/x86/hvm/hvm.c	2007-01-11 09:13:30.000000000 +1100
>@@ -633,7 +633,8 @@
>     if ( bsp->vcpu_id != 0 )
>     {
>         DPRINTK("Not calling hvm_bringup_ap from BSP context.\n");
>-        domain_crash_synchronous();
>+        domain_crash(bsp->domain);
>+        return -EINVAL;
>     }
> 
>     if ( (v = d->vcpu[vcpuid]) == NULL )
>diff -ur xen.orig/arch/x86/hvm/vlapic.c xen/arch/x86/hvm/vlapic.c
>--- xen.orig/arch/x86/hvm/vlapic.c	2007-01-04 17:21:27.000000000 +1100
>+++ xen/arch/x86/hvm/vlapic.c	2007-01-11 09:29:09.000000000 +1100
>@@ -255,7 +255,7 @@
>             if ( test_and_clear_bit(_VCPUF_initialised, &v->vcpu_flags) )
>             {
>                 printk("Reset hvm vcpu not supported yet\n");
>-                domain_crash_synchronous();
>+                goto exit_and_crash;
>             }
>             v->arch.hvm_vcpu.init_sipi_sipi_state =
>                 HVM_VCPU_INIT_SIPI_SIPI_STATE_WAIT_SIPI;
>@@ -274,7 +274,7 @@
>         if ( test_bit(_VCPUF_initialised, &v->vcpu_flags) )
>         {
>             printk("SIPI for initialized vcpu vcpuid %x\n", v->vcpu_id);
>-            domain_crash_synchronous();
>+            goto exit_and_crash;
>         }
> 
>         if ( hvm_bringup_ap(v->vcpu_id, vector) != 0 )
>@@ -283,11 +283,14 @@
> 
>     default:
>         printk("TODO: not support interrupt type %x\n", delivery_mode);
>-        domain_crash_synchronous();
>-        break;
>+        goto exit_and_crash;
>     }
> 
>     return result;
>+
>+ exit_and_crash:
>+    domain_crash(v->domain);
>+    return 0;
> }
> 
> /*
>@@ -538,14 +541,17 @@
> 
>     default:
>         printk("Local APIC read with len=0x%lx, should be 4 instead.\n", len);
>-        domain_crash_synchronous();
>-        break;
>+        goto exit_and_crash;
>     }
> 
>     HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset 0x%x with length 0x%lx, "
>                 "and the result is 0x%lx.", offset, len, result);
> 
>     return result;
>+
>+ exit_and_crash:
>+    domain_crash(v->domain);
>+    return 0;
> }
> 
> static void vlapic_write(struct vcpu *v, unsigned long address,
>@@ -585,23 +591,18 @@
>             if ( alignment != 0x0 && alignment != 0x2 )
>             {
>                 printk("alignment error for vlapic with len == 2\n");
>-                domain_crash_synchronous();
>+                goto exit_and_crash;
>             }
> 
>             val = (tmp & ~(0xffff << (8*alignment))) |
>                   ((val & 0xffff) << (8*alignment));
>             break;
> 
>-        case 3:
>-            /* will it happen? */
>-            printk("vlapic_write with len = 3 !!!\n");
>-            domain_crash_synchronous();
>-            break;
>-
>         default:
>             printk("Local APIC write with len = %lx, should be 4 instead\n", len);
>-            domain_crash_synchronous();
>-            break;
>+        exit_and_crash:
>+            domain_crash(v->domain);
>+            return;
>         }
>     }
> 
>diff -ur xen.orig/arch/x86/mm.c xen/arch/x86/mm.c
>--- xen.orig/arch/x86/mm.c	2007-01-04 18:13:28.000000000 +1100
>+++ xen/arch/x86/mm.c	2007-01-11 09:14:00.000000000 +1100
>@@ -1726,7 +1726,7 @@
>     unsigned long old_base_mfn;
> 
>     if ( hvm_guest(v) && !hvm_paging_enabled(v) )
>-        domain_crash_synchronous();
>+        return 0;
> 
>     if ( shadow_mode_refcounts(d) )
>     {
>diff -ur xen.orig/arch/x86/traps.c xen/arch/x86/traps.c
>--- xen.orig/arch/x86/traps.c	2007-01-04 18:13:28.000000000 +1100
>+++ xen/arch/x86/traps.c	2007-01-11 09:12:57.000000000 +1100
>@@ -1328,8 +1328,10 @@
> 
>         case 3: /* Write CR3 */
>             LOCK_BIGLOCK(v->domain);
>-            (void)new_guest_cr3(gmfn_to_mfn(v->domain, xen_cr3_to_pfn(*reg)));
>+            rc = new_guest_cr3(gmfn_to_mfn(v->domain, xen_cr3_to_pfn(*reg)));
>             UNLOCK_BIGLOCK(v->domain);
>+            if ( rc == 0 ) /* not okay */
>+                goto fail;
>             break;
> 
>         case 4:
>diff -ur xen.orig/arch/x86/x86_32/traps.c xen/arch/x86/x86_32/traps.c
>--- xen.orig/arch/x86/x86_32/traps.c	2007-01-04 17:21:27.000000000 +1100
>+++ xen/arch/x86/x86_32/traps.c	2007-01-11 09:16:48.000000000 +1100
>@@ -187,16 +187,16 @@
> 
>     /* Check worst-case stack frame for overlap with Xen protected area. */
>     if ( unlikely(!access_ok(regs->esp, 40)) )
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
> 
>     /* Pop and restore EAX (clobbered by hypercall). */
>     if ( unlikely(__copy_from_user(&regs->eax, (void __user *)regs->esp, 4)) )
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
>     regs->esp += 4;
> 
>     /* Pop and restore CS and EIP. */
>     if ( unlikely(__copy_from_user(&regs->eip, (void __user *)regs->esp, 8)) )
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
>     regs->esp += 8;
> 
>     /*
>@@ -204,7 +204,7 @@
>      * to avoid firing the BUG_ON(IOPL) check in arch_getdomaininfo_ctxt.
>      */
>     if ( unlikely(__copy_from_user(&eflags, (void __user *)regs->esp, 4)) )
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
>     regs->esp += 4;
>     regs->eflags = (eflags & ~X86_EFLAGS_IOPL) | X86_EFLAGS_IF;
> 
>@@ -212,17 +212,17 @@
>     {
>         /* Return to VM86 mode: pop and restore ESP,SS,ES,DS,FS and GS. */
>         if ( __copy_from_user(&regs->esp, (void __user *)regs->esp, 24) )
>-            domain_crash_synchronous();
>+            goto exit_and_crash;
>     }
>     else if ( unlikely(ring_0(regs)) )
>     {
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
>     }
>     else if ( !ring_1(regs) )
>     {
>         /* Return to ring 2/3: pop and restore ESP and SS. */
>         if ( __copy_from_user(&regs->esp, (void __user *)regs->esp, 8) )
>-            domain_crash_synchronous();
>+            goto exit_and_crash;
>     }
> 
>     /* No longer in NMI context. */
>@@ -236,6 +236,11 @@
>      * value.
>      */
>     return regs->eax;
>+
>+ exit_and_crash:
>+    DPRINTK("Fatal error\n");
>+    domain_crash(current->domain);
>+    return 0;
> }
> 
> #include <asm/asm_defns.h>
>diff -ur xen.orig/arch/x86/x86_64/traps.c xen/arch/x86/x86_64/traps.c
>--- xen.orig/arch/x86/x86_64/traps.c	2007-01-04 17:21:27.000000000 +1100
>+++ xen/arch/x86/x86_64/traps.c	2007-01-11 09:17:04.000000000 +1100
>@@ -207,7 +207,7 @@
>                                  sizeof(iret_saved))) )
>     {
>         DPRINTK("Fault while reading IRET context from guest stack\n");
>-        domain_crash_synchronous();
>+        goto exit_and_crash;
>     }
> 
>     /* Returning to user mode? */
>@@ -216,7 +216,7 @@
>         if ( unlikely(pagetable_is_null(v->arch.guest_table_user)) )
>         {
>             DPRINTK("Guest switching to user mode with no user page tables\n");
>-            domain_crash_synchronous();
>+            goto exit_and_crash;
>         }
>         toggle_guest_mode(v);
>     }
>@@ -242,6 +242,11 @@
> 
>     /* Saved %rax gets written back to regs->rax in entry.S. */
>     return iret_saved.rax;
>+
>+ exit_and_crash:
>+    DPRINTK("Fatal error\n");
>+    domain_crash(v->domain);
>+    return 0;
> }
> 
> asmlinkage void syscall_enter(void);
>diff -ur xen.orig/common/domain.c xen/common/domain.c
>--- xen.orig/common/domain.c	2007-01-04 18:13:28.000000000 +1100
>+++ xen/common/domain.c	2007-01-11 09:06:21.000000000 +1100
>@@ -22,6 +22,7 @@
> #include <xen/delay.h>
> #include <xen/shutdown.h>
> #include <xen/percpu.h>
>+#include <xen/multicall.h>
> #include <asm/debugger.h>
> #include <public/sched.h>
> #include <public/vcpu.h>
>@@ -239,6 +240,10 @@
> void __domain_crash_synchronous(void)
> {
>     __domain_crash(current->domain);
>+
>+    /* Flush multicall state before dying. */
>+    this_cpu(mc_state).flags = 0;
>+
>     for ( ; ; )
>         do_softirq();
> }
>-- 
>Visit Openswan at http://www.openswan.org/
>Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
>Home Page: http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Actions: View | Diff

Attachments on bug 221239: 144684 | 145332