svn commit: r336876 - in head/sys: amd64/amd64 amd64/ia32 amd64/include conf dev/hyperv/vmbus/amd64

[Konstantin Belousov]

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 <kib at 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.

> 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