On 7/30/18, Konstantin Belousov <kostik...@gmail.com> wrote:
> Please trim useless content.
> Did I missed anything interesting in your mail ?
>
> On Sun, Jul 29, 2018 at 11:57:47PM +0200, Oliver Pinter wrote:
>> On 7/29/18, Konstantin Belousov <k...@freebsd.org> wrote:
>> > +ENTRY(copyin_smap)
>> > +  PUSH_FRAME_POINTER
>> > +  movq    PCPU(CURPCB),%rax
>> > +  movq    $copyin_fault,PCB_ONFAULT(%rax)
>> > +  testq   %rdx,%rdx                       /* anything to do? */
>> > +  jz      done_copyin
>> > +
>> > +  /*
>> > +   * make sure address is valid
>> > +   */
>> > +  movq    %rdi,%rax
>> > +  addq    %rdx,%rax
>> > +  jc      copyin_fault
>> > +  movq    $VM_MAXUSER_ADDRESS,%rcx
>> > +  cmpq    %rcx,%rax
>> > +  ja      copyin_fault
>> > +
>> > +  xchgq   %rdi,%rsi
>> > +  movq    %rdx,%rcx
>> > +  movb    %cl,%al
>> > +  shrq    $3,%rcx                         /* copy longword-wise */
>>
>> missing cld from here
> In fact not.  It is copyin_nosmap that got unneeded cld.
>
> See r327820, apparently I mis-merged this commit into the SMAP branch.
>
>>
>> > +  stac
>> > +  rep
>> > +  movsq
>> > +  movb    %al,%cl
>> > +  andb    $7,%cl                          /* copy remaining bytes */
>> >    je      done_copyin
>> >    rep
>> >    movsb
>> > +  clac
>
>> > +ENTRY(copyinstr_smap)
>> > +  PUSH_FRAME_POINTER
>> > +  movq    %rdx,%r8                        /* %r8 = maxlen */
>> > +  movq    %rcx,%r9                        /* %r9 = *len */
>> > +  xchgq   %rdi,%rsi                       /* %rdi = from, %rsi = to */
>> > +  movq    PCPU(CURPCB),%rcx
>> > +  movq    $cpystrflt,PCB_ONFAULT(%rcx)
>> > +
>> > +  movq    $VM_MAXUSER_ADDRESS,%rax
>> > +
>> > +  /* make sure 'from' is within bounds */
>> > +  subq    %rsi,%rax
>> > +  jbe     cpystrflt
>> > +
>> > +  /* restrict maxlen to <= VM_MAXUSER_ADDRESS-from */
>> > +  cmpq    %rdx,%rax
>> > +  jae     1f
>> > +  movq    %rax,%rdx
>> > +  movq    %rax,%r8
>> > +1:
>> > +  incq    %rdx
>>
>> missing cld here
> Same.
>
>>
>> > +
>> > +2:
>> > +  decq    %rdx
>> > +  jz      copyinstr_succ
>>
>
>> cpystrflt_x:
>>         /* set *lencopied and return %eax */
>>         movq    PCPU(CURPCB),%rcx
>>         movq    $0,PCB_ONFAULT(%rcx)
>>
>>         testq   %r9,%r9
>>         jz      1f
>>         subq    %rdx,%r8
>>         movq    %r8,(%r9) << Here you access user-space, with cleared
>> RFLAGS.AC from the fault handler.
> How does this instruction access userspace ?  I do not see.

As far as I remember from 4 years, the r9 may contained a user-space
address in 10-STABLE
in the case of starting the init. I've a stac/clac pair in my internal
version, but I haven't found
yet the relevant commit message.

 For a quick grep around - http://ix.io/1fje - I haven't found yet
this place, so it's looks good in your version.

>
>> 1:
>>         POP_FRAME_POINTER
>>         ret
>
> So the patch below removes unneeded (mismerged) cld's left in the
> support.S.
>
> diff --git a/sys/amd64/amd64/support.S b/sys/amd64/amd64/support.S
> index 9b8b2a40461..0aa307e6895 100644
> --- a/sys/amd64/amd64/support.S
> +++ b/sys/amd64/amd64/support.S
> @@ -307,7 +307,6 @@ ENTRY(copyout_smap)
>       movq    %rdx,%rcx
>
>       shrq    $3,%rcx
> -     cld
>       stac
>       rep
>       movsq
> @@ -358,7 +357,6 @@ ENTRY(copyin_nosmap)
>       movq    %rdx,%rcx
>       movb    %cl,%al
>       shrq    $3,%rcx                         /* copy longword-wise */
> -     cld
>       rep
>       movsq
>       movb    %al,%cl
> @@ -887,7 +885,6 @@ ENTRY(copyinstr_nosmap)
>       movq    %rax,%r8
>  1:
>       incq    %rdx
> -     cld
>
>  2:
>       decq    %rdx

Looks fine to me.
>
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to