Re: CVS commit: src/sys/arch

[Cherry G. Mathew <cherry%zyx.in@localhost>]

Hi Paul,

Thanks for working on this.

>>>>> "Paul" == Paul Goyette <paul%whooppee.com@localhost> writes:
    > Date: Fri, 13 Jun 2014 17:52:53 -0700 (PDT)
    > From: 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?
    > 

[...]

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

This should probably go in include/hypervisor.h, right under the
definitions for XEN_MAJOR() and XEN_MINOR()

    >  /*
    > @@ -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

The version check should probably be inside xen_pagezero() but that
would require a tiny bit of re-org in pmap.c since the direct map code
is equally culpable.

IMO this can wait for a post-commit sweep.

Looks ok to me otherwise.

Cheers,
-- 
Cherry