Re: CVS commit: src/sys/arch

[Paul Goyette <paul%whooppee.com@localhost>]

I modified the newer patch so that things still compile on non-XEN 
kernels, and added the version check to an additional invocation of 
xen_pagezero().  The patch is attached, and I have verified that it 
successfully boots under the pre-3.4 hypervisor.

Is there any reason why this should not be committed?



On Thu, 12 Jun 2014, Paul Goyette wrote:

> Unfortunately, the patch breaks building of a (non-XEN) GENERIC kernel.
>
> Please see attached log...
>
>
> On Thu, 12 Jun 2014, Cherry G. Mathew wrote:
>
> > > > > > > "Paul" == Paul Goyette <paul%whooppee.com@localhost> writes:
> >
> >    >> Module Name: src Committed By: cherry Date: Tue May 6 04:26:24
> >    >> UTC 2014
> >    >>
> >    >> Modified Files: src/sys/arch/x86/x86: pmap.c
> >    >> src/sys/arch/xen/include: xenpmap.h src/sys/arch/xen/x86:
> >    >> x86_xpmap.c
> >    >>
> >    >> Log Message: Use the hypervisor to copy/zero pages. This saves us
> >    >> the extra overheads of setting up temporary kernel
> >    >> mapping/unmapping.
> >    >>
> >    >> riz@ reports savings of about 2s on a 120s kernel build.
> >
> >    Paul> This commit seems to have broken the ability to boot NetBSD
> >    Paul> under at least some hypervisors.  My VPS at prgmr.com boots
> >    Paul> fine with a kernel from just before this change.  However,
> >    Paul> with this commit, the following panic occurs:
> >
> > Sorry, the first patch was incorrect ( it skips over some newer legit
> > versions! ) - thanks abs@.
> >
> > Thanks to Gary Duzan for the initial suggestion and abs@ for catching
> > the error in my first patch.
> >
> > --
> > Cherry
>
> -------------------------------------------------------------------------
> | Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
> | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
> | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
> | Kernel Developer |                          | pgoyette at netbsd.org  |
> -------------------------------------------------------------------------
>
> !DSPAM:53999ec680703563610749!

-------------------------------------------------------------------------
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------

Index: src/sys/arch/x86/x86/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/pmap.c,v
retrieving revision 1.182
diff -u -p -r1.182 pmap.c
--- src/sys/arch/x86/x86/pmap.c 6 May 2014 04:26:23 -0000       1.182
+++ src/sys/arch/x86/x86/pmap.c 14 Jun 2014 00:51:56 -0000
@@ -211,6 +211,15 @@ __KERNEL_RCSID(0, "$NetBSD: pmap.c,v 1.1
 #ifdef XEN
 #include <xen/xen-public/xen.h>
 #include <xen/hypervisor.h>
+
+/* 
+ * Does the hypervisor we're running on support an api
+ * call at the requested version number ?
+ */
+#define XEN_VERSION_SUPPORTED(major, minor)            \
+       (XEN_MAJOR(xen_version) > (major) ||            \
+        (XEN_MAJOR(xen_version) == (major) &&          \
+         XEN_MINOR(xen_version) >= (minor)))
 #endif
 
 /*
@@ -3013,9 +3022,11 @@ pmap_zero_page(paddr_t pa)
 {
 #if defined(__HAVE_DIRECT_MAP)
        pagezero(PMAP_DIRECT_MAP(pa));
-#elif defined(XEN)
-       xen_pagezero(pa);
 #else
+#if defined(XEN)
+       if (XEN_VERSION_SUPPORTED(3, 4))
+               xen_pagezero(pa);
+#endif
        pt_entry_t *zpte;
        void *zerova;
        int id;
@@ -3041,7 +3052,7 @@ pmap_zero_page(paddr_t pa)
        pmap_pte_flush();
 #endif
        kpreempt_enable();
-#endif
+#endif /* defined(__HAVE_DIRECT_MAP) */
 }
 
 /*
@@ -3096,9 +3107,13 @@ pmap_copy_page(paddr_t srcpa, paddr_t ds
        vaddr_t dstva = PMAP_DIRECT_MAP(dstpa);
 
        memcpy((void *)dstva, (void *)srcva, PAGE_SIZE);
-#elif defined(XEN)
-       xen_copy_page(srcpa, dstpa);
 #else
+#if defined(XEN)
+       if (XEN_VERSION_SUPPORTED(3, 4)) {
+               xen_copy_page(srcpa, dstpa);
+               return;
+       }
+#endif
        pt_entry_t *spte;
        pt_entry_t *dpte;
        void *csrcva;
@@ -3128,7 +3143,7 @@ pmap_copy_page(paddr_t srcpa, paddr_t ds
        pmap_pte_flush();
 #endif
        kpreempt_enable();
-#endif
+#endif /* defined(__HAVE_DIRECT_MAP) */
 }
 
 static pt_entry_t *
@@ -4105,11 +4120,15 @@ pmap_get_physpage(vaddr_t va, int level,
 
                if (!uvm_page_physget(paddrp))
                        panic("pmap_get_physpage: out of memory");
-#ifdef __HAVE_DIRECT_MAP
+#if defined(__HAVE_DIRECT_MAP)
                pagezero(PMAP_DIRECT_MAP(*paddrp));
-#elif defined(XEN)
-               xen_pagezero(*paddrp);
 #else
+#if defined(XEN)
+               if (XEN_VERSION_SUPPORTED(3, 4)) {
+                       xen_pagezero(*paddrp);
+                       return true;
+               }
+#endif
                kpreempt_disable();
                pmap_pte_set(early_zero_pte,
                    pmap_pa2pte(*paddrp) | PG_V | PG_RW | PG_k);
@@ -4121,7 +4140,7 @@ pmap_get_physpage(vaddr_t va, int level,
                pmap_pte_flush();
 #endif /* defined(DIAGNOSTIC) */
                kpreempt_enable();
-#endif
+#endif /* defined(__HAVE_DIRECT_MAP) */
        } else {
                /* XXX */
                ptp = uvm_pagealloc(NULL, 0, NULL,