Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Jul 2018 01:31:58 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Oliver Pinter <oliver.pinter@hardenedbsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r336876 - in head/sys: amd64/amd64 amd64/ia32 amd64/include conf dev/hyperv/vmbus/amd64
Message-ID:  <20180729223158.GL40119@kib.kiev.ua>
In-Reply-To: <CAPQ4ffsHqf8OiqUF-WBJ_0KbZGgAOs-6npyLjqWYFN%2BoCq=8Pw@mail.gmail.com>
References:  <201807292047.w6TKl0hV004691@repo.freebsd.org> <CAPQ4ffsHqf8OiqUF-WBJ_0KbZGgAOs-6npyLjqWYFN%2BoCq=8Pw@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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@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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180729223158.GL40119>