Subject: Re: port-xen/30635
To: None <port-xen-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Jed Davis <jld@panix.com>
List: netbsd-bugs
Date: 08/13/2005 00:53:02
The following reply was made to PR port-xen/30635; it has been noted by GNATS.

From: jld@panix.com (Jed Davis)
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-xen/30635
Date: Fri, 12 Aug 2005 20:52:15 -0400

 --=-=-=
 
 
 I have here a patch that locks the page given to IOCTL_PRIVCMD_MMAP
 before altering the pmap; this avoids having the UVM structures
 continue to show the page as still nonresident, as well as preventing
 the page from being swapped out/in as if it were a normal anon page.
 
 It fixes the panic, but xend still misbehaves (destroys one domU, then
 exits and won't start again); that problem way well lie elsewhere.  So:
 
 
 --=-=-=
 Content-Type: text/x-patch
 Content-Disposition: inline; filename=privcmd_mlock.patch
 Content-Description: Patch to lock memory in IOCTL_PRIVCMD_MMAP
 
 Index: sys/arch/xen/xen/privcmd.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/xen/xen/privcmd.c,v
 retrieving revision 1.2.2.2
 diff -u -2 -p -r1.2.2.2 privcmd.c
 --- sys/arch/xen/xen/privcmd.c  18 Jun 2005 10:43:37 -0000      1.2.2.2
 +++ sys/arch/xen/xen/privcmd.c  13 Aug 2005 00:07:28 -0000
 @@ -105,9 +105,12 @@ privcmd_ioctl(void *v)
                 vaddr_t va;
                 u_long ma;
 -               //printf("IOCTL_PRIVCMD_MMAP: %d entries\n", mcmd->num);
 +               struct vm_map *vmm;
 +               pmap_t pmap;
  
 -               pmap_t pmap = vm_map_pmap(&ap->a_p->p_vmspace->vm_map);
 +               vmm = &ap->a_p->p_vmspace->vm_map;
 +               pmap = vm_map_pmap(vmm);
                 for (i = 0; i < mcmd->num; i++) {
 -                       error = copyin(mcmd->entry, &mentry, sizeof(mentry));
 +                       error = copyin(&mcmd->entry[i], &mentry,
 +                           sizeof(mentry));
                         if (error)
                                 return error;
 @@ -124,7 +127,27 @@ privcmd_ioctl(void *v)
                         for (j = 0; j < mentry.npages; j++) {
                                 //printf("remap va 0x%lx to 0x%lx\n", va, ma);
 -#if 0
 -                               if (!pmap_extract(pmap, va, NULL))
 -                                       return EINVAL; /* XXX */
 +                               /*
 +                                * Lock the memory before changing the pmap;
 +                                * this way the vm_map and pmap are only
 +                                * slightly inconsistent.  Panics and 
 +                                * mysterious failures are thus avoided,
 +                                * at the cost of some physical memory.
 +                                *    -- jld@panix.com
 +                                */
 +                               error = uvm_map_pageable(vmm, va,
 +                                   va + PAGE_SIZE, FALSE, 0);
 +                               if (error != 0) {
 +                                       printf("IOCTL_PRIVCMD_MMAP: "
 +                                           "uvm_map_pageable: %d\n", error);
 +                                       if (error == EFAULT)
 +                                               error = ENOMEM;
 +                                       return error;
 +                               }
 +#ifdef DIAGNOSTIC
 +                               if (!pmap_extract(pmap, va, NULL)) {
 +                                       printf("IOCTL_PRIVCMD_MMAP: no "
 +                                           "existing mapping\n");
 +                                       return EINVAL;
 +                               }
  #endif
                                 if ((error = pmap_remap_pages(pmap, va, ma, 1,
 
 --=-=-=
 
 
 There's also a slightly unrelated change in that diff: copying in the
 correct element of mcmd->entry rather than the first.  This doesn't
 make a difference with xend, as libxc never sends more than one at a
 time (so i==0 always), but it could be important for alternative tools.
 
 
 -- 
 Jed Davis
 jld@panix.com
 
 
 --=-=-=--