> On Mon, 28 Jan 2019 16:14:53 +0000
> Mark Johnston <ma...@freebsd.org> wrote:
> 
> > Author: markj
> > Date: Mon Jan 28 16:14:53 2019
> > New Revision: 343527
> > URL: https://svnweb.freebsd.org/changeset/base/343527
> > 
> > Log:
> >   MFC r343274, r343275:
> >   Optimize RISC-V copyin(9)/copyout(9) routines.
> 
> Was this subjected to any benchmarks?  I'd bet that placing
> 
> addi  a2, a2, -XLEN_BYTES
> 
> before
> 
> sd    a4, 0(a1)
> 
> instead of being scheduled after (the same goes for the byte copy loop)
> would make the loops run faster on most in-order RV cores out there...
> 
> Marko
> 
> 
> > 
> > Modified:
> >   stable/12/sys/riscv/include/riscvreg.h
> >   stable/12/sys/riscv/riscv/copyinout.S
> > Directory Properties:
> >   stable/12/   (props changed)
> > 
> > Modified: stable/12/sys/riscv/include/riscvreg.h
> > ==============================================================================
> > --- stable/12/sys/riscv/include/riscvreg.h  Mon Jan 28 14:34:59
> > 2019        (r343526) +++
> > stable/12/sys/riscv/include/riscvreg.h      Mon Jan 28 16:14:53
> > 2019        (r343527) @@ -155,7 +155,8 @@ #define
> > SATP_MODE_SV39      (8ULL << SATP_MODE_S) #define
> > SATP_MODE_SV48      (9ULL << SATP_MODE_S) 
> > -#define    XLEN            8
> > +#define    XLEN            __riscv_xlen
> > +#define    XLEN_BYTES      (XLEN / 8)
> >  #define    INSN_SIZE       4
> >  #define    INSN_C_SIZE     2
> >  
> > 
> > Modified: stable/12/sys/riscv/riscv/copyinout.S
> > ==============================================================================
> > --- stable/12/sys/riscv/riscv/copyinout.S   Mon Jan 28 14:34:59
> > 2019        (r343526) +++
> > stable/12/sys/riscv/riscv/copyinout.S       Mon Jan 28 16:14:53
> > 2019        (r343527) @@ -1,5 +1,6 @@ /*-
> >   * Copyright (c) 2015-2018 Ruslan Bukin <b...@bsdpad.com>
> > + * Copyright (c) 2019 Mitchell Horne

Didnt notice this in original commit, Placing a copyright
between an existing copyright and its all rights reserved
clause is a bad idea.  Is it you have made it appear as
if Ruslan is not asserting all rights and YOU are.

That is also why it is bad form and has been shown in
style(9) that the All rights reserved belongs on the
same line as the copyright.

> >   * All rights reserved.
> >   *
> >   * Portions of this software were developed by SRI International and
> > the @@ -52,60 +53,94 @@ copyio_fault_nopcb:
> >  END(copyio_fault)
> >  
> >  /*
> > + * copycommon - common copy routine
> > + *
> > + * a0 - Source address
> > + * a1 - Destination address
> > + * a2 - Size of copy
> > + */
> > +   .macro copycommon
> > +   la      a6, copyio_fault        /* Get the handler address
> > */
> > +   SET_FAULT_HANDLER(a6, a7)       /* Set the handler */
> > +   ENTER_USER_ACCESS(a7)
> > +
> > +   li      t2, XLEN_BYTES
> > +   blt     a2, t2, 3f              /* Byte-copy if len <
> > XLEN_BYTES */ +
> > +   /*
> > +    * Compare lower bits of src and dest.
> > +    * If they are aligned with each other, we can do word copy.
> > +    */
> > +   andi    t0, a0, (XLEN_BYTES-1)  /* Low bits of src
> > */
> > +   andi    t1, a1, (XLEN_BYTES-1)  /* Low bits of
> > dest */
> > +   bne     t0, t1, 3f              /* Misaligned. Go to
> > byte copy */
> > +   beqz    t0, 2f                  /* Already
> > word-aligned, skip ahead */ +
> > +   /* Byte copy until the first word-aligned address */
> > +1: lb      a4, 0(a0)               /* Load byte from src */
> > +   addi    a0, a0, 1
> > +   sb      a4, 0(a1)               /* Store byte in dest */
> > +   addi    a1, a1, 1
> > +   addi    a2, a2, -1              /* len-- */
> > +   andi    t0, a0, (XLEN_BYTES-1)
> > +   bnez    t0, 1b
> > +
> > +   /* Copy words */
> > +2: ld      a4, 0(a0)               /* Load word from src */
> > +   addi    a0, a0, XLEN_BYTES
> > +   sd      a4, 0(a1)               /* Store word in dest */
> > +   addi    a1, a1, XLEN_BYTES
> > +   addi    a2, a2, -XLEN_BYTES     /* len -= XLEN_BYTES
> > */
> > +   bgeu    a2, t2, 2b              /* Again if len >=
> > XLEN_BYTES */ +
> > +   /* Check if we're finished */
> > +   beqz    a2, 4f
> > +
> > +   /* Copy any remaining bytes */
> > +3: lb      a4, 0(a0)               /* Load byte from src */
> > +   addi    a0, a0, 1
> > +   sb      a4, 0(a1)               /* Store byte in dest */
> > +   addi    a1, a1, 1
> > +   addi    a2, a2, -1              /* len-- */
> > +   bnez    a2, 3b
> > +
> > +4: EXIT_USER_ACCESS(a7)
> > +   SET_FAULT_HANDLER(x0, a7)       /* Clear the handler */
> > +   .endm
> > +
> > +/*
> >   * Copies from a kernel to user address
> >   *
> >   * int copyout(const void *kaddr, void *udaddr, size_t len)
> >   */
> >  ENTRY(copyout)
> > -   beqz    a2, 2f          /* If len == 0 then skip
> > loop */
> > +   beqz    a2, copyout_end /* If len == 0 then skip
> > loop */ add a3, a1, a2
> >     li      a4, VM_MAXUSER_ADDRESS
> >     bgt     a3, a4, copyio_fault_nopcb
> >  
> > -   la      a6, copyio_fault /* Get the handler address */
> > -   SET_FAULT_HANDLER(a6, a7) /* Set the handler */
> > -   ENTER_USER_ACCESS(a7)
> > +   copycommon
> >  
> > -1: lb      a4, 0(a0)       /* Load from kaddr */
> > -   addi    a0, a0, 1
> > -   sb      a4, 0(a1)       /* Store in uaddr */
> > -   addi    a1, a1, 1
> > -   addi    a2, a2, -1      /* len-- */
> > -   bnez    a2, 1b
> > -
> > -   EXIT_USER_ACCESS(a7)
> > -   SET_FAULT_HANDLER(x0, a7) /* Clear the handler */
> > -
> > -2: li      a0, 0           /* return 0 */
> > +copyout_end:
> > +   li      a0, 0           /* return 0 */
> >     ret
> >  END(copyout)
> >  
> >  /*
> >   * Copies from a user to kernel address
> >   *
> > - * int copyin(const void *uaddr, void *kdaddr, size_t len)
> > + * int copyin(const void *uaddr, void *kaddr, size_t len)
> >   */
> >  ENTRY(copyin)
> > -   beqz    a2, 2f          /* If len == 0 then skip
> > loop */
> > +   beqz    a2, copyin_end  /* If len == 0 then skip
> > loop */ add a3, a0, a2
> >     li      a4, VM_MAXUSER_ADDRESS
> >     bgt     a3, a4, copyio_fault_nopcb
> >  
> > -   la      a6, copyio_fault /* Get the handler address */
> > -   SET_FAULT_HANDLER(a6, a7) /* Set the handler */
> > -   ENTER_USER_ACCESS(a7)
> > +   copycommon
> >  
> > -1: lb      a4, 0(a0)       /* Load from uaddr */
> > -   addi    a0, a0, 1
> > -   sb      a4, 0(a1)       /* Store in kaddr */
> > -   addi    a1, a1, 1
> > -   addi    a2, a2, -1      /* len-- */
> > -   bnez    a2, 1b
> > -
> > -   EXIT_USER_ACCESS(a7)
> > -   SET_FAULT_HANDLER(x0, a7) /* Clear the handler */
> > -
> > -2: li      a0, 0           /* return 0 */
> > +copyin_end:
> > +   li      a0, 0           /* return 0 */
> >     ret
> >  END(copyin)
> >  
> > 
> 
> 
> 

-- 
Rod Grimes                                                 rgri...@freebsd.org
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to