Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/arch Apply patch proposed in PR port-xen/45975 (this doe...



details:   https://anonhg.NetBSD.org/src/rev/e2cf3da949f9
branches:  trunk
changeset: 777409:e2cf3da949f9
user:      bouyer <bouyer%NetBSD.org@localhost>
date:      Fri Feb 17 18:40:18 2012 +0000

description:
Apply patch proposed in PR port-xen/45975 (this does not solve the exact
problem reported here but is part of the solution):
xen_kpm_sync() is not working as expected,
leading to races between CPUs.
1 the check (xpq_cpu != &x86_curcpu) is always false because we
  have different x86_curcpu symbols with different addresses in the kernel.
  Fortunably, all addresses dissaemble to the same code.
  Because of this we always use the code intended for bootstrap, which doesn't
  use cross-calls or lock.

2 once 1 above is fixed, xen_kpm_sync() will use xcalls to sync other CPUs,
  which cause it to sleep and pmap.c doesn't like that. It triggers this
  KASSERT() in pmap_unmap_ptes():
  KASSERT(pmap->pm_ncsw == curlwp->l_ncsw);
3 pmap->pm_cpus is not safe for the purpose of xen_kpm_sync(), which
  needs to know on which CPU a pmap is loaded *now*:
  pmap->pm_cpus is cleared before cpu_load_pmap() is called to switch
  to a new pmap, leaving a window where a pmap is still in a CPU's
  ci_kpm_pdir but not in pm_cpus. As a virtual CPU may be preempted
  by the hypervisor at any time, it can be large enough to let another
  CPU free the PTP and reuse it as a normal page.

To fix 2), avoid cross-calls and IPIs completely, and instead
use a mutex to update all CPU's ci_kpm_pdir from the local CPU.
It's safe because we just need to update the table page, a tlbflush IPI will
happen later. As a side effect, we don't need a different code for bootstrap,
fixing 1). The mutex added to struct cpu needs a small headers reorganisation.

to fix 3), introduce a pm_xen_ptp_cpus which is updated from
cpu_pmap_load(), whith the ci_kpm_mtx mutex held. Checking it with
ci_kpm_mtx held will avoid overwriting the wrong pmap's ci_kpm_pdir.

While there I removed the unused pmap_is_active() function;
and added some more details to DIAGNOSTIC panics.

diffstat:

 sys/arch/x86/include/cpu.h        |    6 +-
 sys/arch/x86/include/pmap.h       |    4 +-
 sys/arch/x86/x86/cpu.c            |    6 +-
 sys/arch/x86/x86/pmap.c           |   42 ++++++++-------
 sys/arch/xen/include/hypervisor.h |    3 +-
 sys/arch/xen/include/intr.h       |    7 +-
 sys/arch/xen/x86/cpu.c            |  101 ++++++++++++++++++++++---------------
 sys/arch/xen/x86/x86_xpmap.c      |   32 +++++++++--
 sys/arch/xen/x86/xen_ipi.c        |    9 +-
 sys/arch/xen/x86/xen_pmap.c       |  101 ++++++-------------------------------
 10 files changed, 143 insertions(+), 168 deletions(-)

diffs (truncated from 607 to 300 lines):

diff -r 678720e98899 -r e2cf3da949f9 sys/arch/x86/include/cpu.h
--- a/sys/arch/x86/include/cpu.h        Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/x86/include/cpu.h        Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cpu.h,v 1.47 2012/02/12 14:38:18 jym Exp $     */
+/*     $NetBSD: cpu.h,v 1.48 2012/02/17 18:40:18 bouyer Exp $  */
 
 /*-
  * Copyright (c) 1990 The Regents of the University of California.
@@ -70,6 +70,7 @@
 #ifdef XEN
 #include <xen/xen-public/xen.h>
 #include <xen/xen-public/event_channel.h>
+#include <sys/mutex.h>
 #endif /* XEN */
 
 struct intrsource;
@@ -185,6 +186,7 @@
        /* Currently active user PGD (can't use rcr3() with Xen) */
        pd_entry_t *    ci_kpm_pdir;    /* per-cpu PMD (va) */
        paddr_t         ci_kpm_pdirpa;  /* per-cpu PMD (pa) */
+       kmutex_t        ci_kpm_mtx;
 #if defined(__x86_64__)
        /* per-cpu version of normal_pdes */
        pd_entry_t *    ci_normal_pdes[3]; /* Ok to hardcode. only for x86_64 && XEN */
@@ -317,7 +319,7 @@
 void cpu_boot_secondary_processors(void);
 void cpu_init_idle_lwps(void);
 void cpu_init_msrs(struct cpu_info *, bool);
-void cpu_load_pmap(struct pmap *);
+void cpu_load_pmap(struct pmap *, struct pmap *);
 void cpu_broadcast_halt(void);
 void cpu_kick(struct cpu_info *);
 
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/x86/include/pmap.h
--- a/sys/arch/x86/include/pmap.h       Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/x86/include/pmap.h       Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.h,v 1.49 2011/12/04 16:24:13 chs Exp $    */
+/*     $NetBSD: pmap.h,v 1.50 2012/02/17 18:40:18 bouyer Exp $ */
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -165,6 +165,8 @@
        uint32_t pm_cpus;               /* mask of CPUs using pmap */
        uint32_t pm_kernel_cpus;        /* mask of CPUs using kernel part
                                         of pmap */
+       uint32_t pm_xen_ptp_cpus;       /* mask of CPUs which have this pmap's
+                                        ptp mapped */
        uint64_t pm_ncsw;               /* for assertions */
        struct vm_page *pm_gc_ptp;      /* pages from pmap g/c */
 };
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/x86/x86/cpu.c
--- a/sys/arch/x86/x86/cpu.c    Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/x86/x86/cpu.c    Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cpu.c,v 1.96 2011/10/18 05:16:02 jruoho Exp $  */
+/*     $NetBSD: cpu.c,v 1.97 2012/02/17 18:40:19 bouyer Exp $  */
 
 /*-
  * Copyright (c) 2000, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.96 2011/10/18 05:16:02 jruoho Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.97 2012/02/17 18:40:19 bouyer Exp $");
 
 #include "opt_ddb.h"
 #include "opt_mpbios.h"                /* for MPDEBUG */
@@ -1228,7 +1228,7 @@
  * Loads pmap for the current CPU.
  */
 void
-cpu_load_pmap(struct pmap *pmap)
+cpu_load_pmap(struct pmap *pmap, struct pmap *oldpmap)
 {
 #ifdef PAE
        int i, s;
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/x86/x86/pmap.c
--- a/sys/arch/x86/x86/pmap.c   Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/x86/x86/pmap.c   Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: pmap.c,v 1.164 2012/02/11 18:59:41 chs Exp $   */
+/*     $NetBSD: pmap.c,v 1.165 2012/02/17 18:40:19 bouyer Exp $        */
 
 /*-
  * Copyright (c) 2008, 2010 The NetBSD Foundation, Inc.
@@ -171,7 +171,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.164 2012/02/11 18:59:41 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.165 2012/02/17 18:40:19 bouyer Exp $");
 
 #include "opt_user_ldt.h"
 #include "opt_lockdebug.h"
@@ -561,7 +561,6 @@
 static void             pmap_free_ptp(struct pmap *, struct vm_page *,
                                       vaddr_t, pt_entry_t *,
                                       pd_entry_t * const *);
-static bool             pmap_is_active(struct pmap *, struct cpu_info *, bool);
 static bool             pmap_remove_pte(struct pmap *, struct vm_page *,
                                         pt_entry_t *, vaddr_t,
                                         struct pv_entry **);
@@ -680,19 +679,6 @@
 }
 
 /*
- * pmap_is_active: is this pmap loaded into the specified processor's %cr3?
- */
-
-inline static bool
-pmap_is_active(struct pmap *pmap, struct cpu_info *ci, bool kernel)
-{
-
-       return (pmap == pmap_kernel() ||
-           (pmap->pm_cpus & ci->ci_cpumask) != 0 ||
-           (kernel && (pmap->pm_kernel_cpus & ci->ci_cpumask) != 0));
-}
-
-/*
  *     Add a reference to the specified pmap.
  */
 
@@ -781,7 +767,7 @@
                ci->ci_tlbstate = TLBSTATE_VALID;
                atomic_or_32(&pmap->pm_cpus, cpumask);
                atomic_or_32(&pmap->pm_kernel_cpus, cpumask);
-               cpu_load_pmap(pmap);
+               cpu_load_pmap(pmap, curpmap);
        }
        pmap->pm_ncsw = l->l_ncsw;
        *pmap2 = curpmap;
@@ -2239,6 +2225,7 @@
        pmap->pm_flags = 0;
        pmap->pm_cpus = 0;
        pmap->pm_kernel_cpus = 0;
+       pmap->pm_xen_ptp_cpus = 0;
        pmap->pm_gc_ptp = NULL;
 
        /* init the LDT */
@@ -2329,9 +2316,26 @@
        }
 
 #ifdef DIAGNOSTIC
-       for (CPU_INFO_FOREACH(cii, ci))
+       for (CPU_INFO_FOREACH(cii, ci)) {
                if (ci->ci_pmap == pmap)
                        panic("destroying pmap being used");
+#if defined(XEN) && defined(__x86_64__)
+               for (i = 0; i < PDIR_SLOT_PTE; i++) {
+                       if (pmap->pm_pdir[i] != 0 &&
+                           ci->ci_kpm_pdir[i] == pmap->pm_pdir[i]) {
+                               printf("pmap_destroy(%p) pmap_kernel %p "
+                                   "curcpu %d cpu %d ci_pmap %p "
+                                   "ci->ci_kpm_pdir[%d]=%" PRIx64
+                                   " pmap->pm_pdir[%d]=%" PRIx64 "\n",
+                                   pmap, pmap_kernel(), curcpu()->ci_index,
+                                   ci->ci_index, ci->ci_pmap,
+                                   i, ci->ci_kpm_pdir[i],
+                                   i, pmap->pm_pdir[i]);
+                               panic("pmap_destroy: used pmap");
+                       }
+               }
+#endif
+       }
 #endif /* DIAGNOSTIC */
 
        /*
@@ -2760,7 +2764,7 @@
        lldt(pmap->pm_ldt_sel);
 
        u_int gen = uvm_emap_gen_return();
-       cpu_load_pmap(pmap);
+       cpu_load_pmap(pmap, oldpmap);
        uvm_emap_update(gen);
 
        ci->ci_want_pmapload = 0;
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/xen/include/hypervisor.h
--- a/sys/arch/xen/include/hypervisor.h Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/xen/include/hypervisor.h Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: hypervisor.h,v 1.36 2011/12/07 15:47:42 cegger Exp $   */
+/*     $NetBSD: hypervisor.h,v 1.37 2012/02/17 18:40:19 bouyer Exp $   */
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -91,7 +91,6 @@
 #include <xen/xen-public/io/netif.h>
 #include <xen/xen-public/io/blkif.h>
 
-#include <machine/cpu.h>
 #include <machine/hypercalls.h>
 
 #undef u8
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/xen/include/intr.h
--- a/sys/arch/xen/include/intr.h       Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/xen/include/intr.h       Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: intr.h,v 1.33 2011/08/11 17:58:59 cherry Exp $ */
+/*     $NetBSD: intr.h,v 1.34 2012/02/17 18:40:19 bouyer Exp $ */
 /*     NetBSD intr.h,v 1.15 2004/10/31 10:39:34 yamt Exp       */
 
 /*-
@@ -39,12 +39,13 @@
 #include <xen/xen.h>
 #include <xen/hypervisor.h>
 #include <xen/evtchn.h>
-#include <machine/cpu.h>
 #include <machine/pic.h>
 #include <sys/evcnt.h>
 
 #include "opt_xen.h"
 
+
+struct cpu_info;
 /*
  * Struct describing an event channel. 
  */
@@ -152,8 +153,6 @@
  * Stub declarations.
  */
 
-struct cpu_info;
-
 struct pcibus_attach_args;
 
 #ifdef MULTIPROCESSOR
diff -r 678720e98899 -r e2cf3da949f9 sys/arch/xen/x86/cpu.c
--- a/sys/arch/xen/x86/cpu.c    Fri Feb 17 16:57:57 2012 +0000
+++ b/sys/arch/xen/x86/cpu.c    Fri Feb 17 18:40:18 2012 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cpu.c,v 1.80 2012/02/13 23:54:58 jym Exp $     */
+/*     $NetBSD: cpu.c,v 1.81 2012/02/17 18:40:20 bouyer Exp $  */
 /* NetBSD: cpu.c,v 1.18 2004/02/20 17:35:01 yamt Exp  */
 
 /*-
@@ -66,7 +66,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.80 2012/02/13 23:54:58 jym Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cpu.c,v 1.81 2012/02/17 18:40:20 bouyer Exp $");
 
 #include "opt_ddb.h"
 #include "opt_multiprocessor.h"
@@ -595,6 +595,9 @@
        /* No user PGD mapped for this CPU yet */
        ci->ci_xen_current_user_pgd = 0;
 #endif
+#if defined(__x86_64__) || defined(PAE)
+       mutex_init(&ci->ci_kpm_mtx, MUTEX_DEFAULT, IPL_VM);
+#endif
 
        atomic_or_32(&cpus_running, ci->ci_cpumask);
        atomic_or_32(&ci->ci_flags, CPUF_RUNNING);
@@ -1172,62 +1175,76 @@
  * Loads pmap for the current CPU.
  */
 void
-cpu_load_pmap(struct pmap *pmap)
+cpu_load_pmap(struct pmap *pmap, struct pmap *oldpmap)
 {
+#if defined(__x86_64__) || defined(PAE)
+       struct cpu_info *ci = curcpu();
+       uint32_t cpumask = ci->ci_cpumask;
+
+       mutex_enter(&ci->ci_kpm_mtx);
+       /* make new pmap visible to pmap_kpm_sync_xcall() */
+       atomic_or_32(&pmap->pm_xen_ptp_cpus, cpumask);
+#endif
 #ifdef i386
 #ifdef PAE
-       int i, s;
-       struct cpu_info *ci;
-
-       s = splvm(); /* just to be safe */
-       ci = curcpu();
-       paddr_t l3_pd = xpmap_ptom_masked(ci->ci_pae_l3_pdirpa);
-       /* don't update the kernel L3 slot */
-       for (i = 0 ; i < PDP_SIZE - 1; i++) {
-               xpq_queue_pte_update(l3_pd + i * sizeof(pd_entry_t),
-                   xpmap_ptom(pmap->pm_pdirpa[i]) | PG_V);
+       {
+               int i;
+               paddr_t l3_pd = xpmap_ptom_masked(ci->ci_pae_l3_pdirpa);
+               /* don't update the kernel L3 slot */
+               for (i = 0 ; i < PDP_SIZE - 1; i++) {
+                       xpq_queue_pte_update(l3_pd + i * sizeof(pd_entry_t),
+                           xpmap_ptom(pmap->pm_pdirpa[i]) | PG_V);
+               }
+               tlbflush();
        }
-       splx(s);
-       tlbflush();
 #else /* PAE */
        lcr3(pmap_pdirpa(pmap, 0));
 #endif /* PAE */
 #endif /* i386 */



Home | Main Index | Thread Index | Old Index