Re: Merging few files from i386/amd64 into x86

[David Young <dyoung%pobox.com@localhost>]

On Wed, Mar 25, 2009 at 12:16:24PM +0000, Mindaugas Rasiukevicius wrote:
> Hello,
> 
> I would like to merge couple files from i386 and amd64 into x86, since they
> basically duplicate the functionality:
> 
> sys/arch/{i386|amd64}/vm_machdep.c
> sys/arch/{i386|amd64}/ipifuncs.c
> sys/arch/{i386|amd64}/mem.c
> 
> The ipifuncs.c would be merged with existing x86/ipi.c file. Other potential
> candidates for merge would be trap.c and syscall.c files. Few #ifdefs and
> macros would help to keep i386 and amd64 in sync.
> 
> Any objections?

Can you avoid using new #ifdefs, or else confine them to the top of the
file?  They make even straightforward code hard to read. :-/

The vm_machdep.c are so alike that I wonder if you can pull out most of
the code that is the same and stick it into sys/arch/x86/ ?

I had a look at vm_machdep.c with a mind to reduce purely cosmetic
differences.  I made the attached compilable but untested patch.  ISTR
the differences can be reduced further by a few minor changes: for
example, rename npxsave_lwp to fpusave_lwp.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933

Index: sys/arch/i386/i386/vm_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/vm_machdep.c,v
retrieving revision 1.143
diff -p -u -u -p -r1.143 vm_machdep.c
--- sys/arch/i386/i386/vm_machdep.c     20 Dec 2008 13:09:44 -0000      1.143
+++ sys/arch/i386/i386/vm_machdep.c     28 Mar 2009 22:01:54 -0000
@@ -107,12 +107,11 @@ __KERNEL_RCSID(0, "$NetBSD: vm_machdep.c
 
 #include "npx.h"
 
-static void setredzone(struct lwp *l);
+static void setredzone(struct lwp *);
 
 void
 cpu_proc_fork(struct proc *p1, struct proc *p2)
 {
-
        p2->p_md.md_flags = p1->p_md.md_flags;
 }
 
@@ -136,7 +135,7 @@ cpu_proc_fork(struct proc *p1, struct pr
  */
 void
 cpu_lwp_fork(struct lwp *l1, struct lwp *l2, void *stack, size_t stacksize,
-    void (*func)(void *), void *arg)
+            void (*func)(void *), void *arg)
 {
        struct pcb *pcb = &l2->l_addr->u_pcb;
        struct trapframe *tf;
@@ -211,14 +210,12 @@ cpu_setfunc(struct lwp *l, void (*func)(
 void
 cpu_swapin(struct lwp *l)
 {
-
        setredzone(l);
 }
 
 void
 cpu_swapout(struct lwp *l)
 {
-
 #if NNPX > 0
        /*
         * Make sure we save the FP state before the user area vanishes.
@@ -235,7 +232,6 @@ cpu_swapout(struct lwp *l)
 void
 cpu_lwp_free(struct lwp *l, int proc)
 {
-
 #if NNPX > 0
        /* If we were using the FPU, forget about it. */
        if (l->l_addr->u_pcb.pcb_fpcpu != NULL)
@@ -278,7 +274,7 @@ setredzone(struct lwp *l)
  * Convert kernel VA to physical address
  */
 int
-kvtop(register void *addr)
+kvtop(void *addr)
 {
        paddr_t pa;
 
@@ -300,10 +296,11 @@ vmapbuf(struct buf *bp, vsize_t len)
 
        if ((bp->b_flags & B_PHYS) == 0)
                panic("vmapbuf");
-       faddr = trunc_page((vaddr_t)(bp->b_saveaddr = bp->b_data));
+       bp->b_saveaddr = bp->b_data;
+       faddr = trunc_page((vaddr_t)bp->b_data);
        off = (vaddr_t)bp->b_data - faddr;
        len = round_page(off + len);
-       taddr = uvm_km_alloc(phys_map, len, 0, UVM_KMF_VAONLY|UVM_KMF_WAITVA);
+       taddr = uvm_km_alloc(phys_map, len, 0, UVM_KMF_VAONLY | UVM_KMF_WAITVA);
        bp->b_data = (void *)(taddr + off);
        /*
         * The region is locked, so we expect that pmap_pte() will return
Index: sys/arch/amd64/amd64/vm_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/amd64/amd64/vm_machdep.c,v
retrieving revision 1.37
diff -p -u -u -p -r1.37 vm_machdep.c
--- sys/arch/amd64/amd64/vm_machdep.c   20 Dec 2008 12:42:36 -0000      1.37
+++ sys/arch/amd64/amd64/vm_machdep.c   28 Mar 2009 22:01:54 -0000
@@ -118,9 +118,9 @@ cpu_proc_fork(struct proc *p1, struct pr
 }
 
 /*
- * Finish a new thread operation, with lwp l2 nearly set up.
+ * Finish a new thread operation, with LWP l2 nearly set up.
  * Copy and update the pcb and trap frame, making the child ready to run.
- * 
+ *
  * Rig the child's kernel stack so that it will start out in
  * lwp_trampoline() and call child_return() with l2 as an
  * argument. This causes the newly-created child process to go
@@ -162,7 +162,7 @@ cpu_lwp_fork(struct lwp *l1, struct lwp 
        }
 #ifdef DIAGNOSTIC
        else if (l1 != &lwp0)
-               panic("cpu_fork: curproc");
+               panic("cpu_lwp_fork: curlwp");
 #endif
        *pcb = l1->l_addr->u_pcb;
 #if defined(XEN)
@@ -224,7 +224,6 @@ cpu_swapin(struct lwp *l)
 void
 cpu_swapout(struct lwp *l)
 {
-
        /*
         * Make sure we save the FP state before the user area vanishes.
         */
@@ -263,7 +262,7 @@ setredzone(struct lwp *l)
        addr = USER_TO_UAREA(l->l_addr);
        pmap_remove(pmap_kernel(), addr, addr + PAGE_SIZE);
        pmap_update(pmap_kernel());
-#endif
+#endif /* DIAGNOSTIC */
 }
 
 /*
@@ -276,13 +275,13 @@ kvtop(void *addr)
 
        if (pmap_extract(pmap_kernel(), (vaddr_t)addr, &pa) == false)
                panic("kvtop: zero page frame");
-       return((int)pa);
+       return (int)pa;
 }
 
 /*
  * Map a user I/O request into kernel virtual address space.
  * Note: the pages are already locked by uvm_vslock(), so we
- * do not need to pass an access_type to pmap_enter().   
+ * do not need to pass an access_type to pmap_enter().
  */
 void
 vmapbuf(struct buf *bp, vsize_t len)
@@ -302,7 +301,7 @@ vmapbuf(struct buf *bp, vsize_t len)
         * The region is locked, so we expect that pmap_pte() will return
         * non-NULL.
         * XXX: unwise to expect this in a multithreaded environment.
-        * anything can happen to a pmap between the time we lock a 
+        * anything can happen to a pmap between the time we lock a
         * region, release the pmap lock, and then relock it for
         * the pmap_extract().
         *