From owner-svn-src-head@freebsd.org Sun Jul 29 23:26:52 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D05AB1066AB7 for ; Sun, 29 Jul 2018 23:26:52 +0000 (UTC) (envelope-from oliver.pinter@hardenedbsd.org) Received: from mail-yw0-x22a.google.com (mail-yw0-x22a.google.com [IPv6:2607:f8b0:4002:c05::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 682007EA2F for ; Sun, 29 Jul 2018 23:26:52 +0000 (UTC) (envelope-from oliver.pinter@hardenedbsd.org) Received: by mail-yw0-x22a.google.com with SMTP id r184-v6so3748822ywg.6 for ; Sun, 29 Jul 2018 16:26:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hardenedbsd.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=XYRuRUVmntnbYdM6jiMcV82dPI8UhKg76nxLlKYvlqI=; b=GM94xX2igvUNhfJDeKYZ9mtPjdmQD4uYqnWaWIRc/xiN4+oUCA4CmEOE8JF73GLvDm fdLjVfrBJ6YVMioaKlG7/bX2YO4KGThNp459cRh2fBc6XiPbC3BV+xkof6RUWGIZqYRI nQGI08y9xutSiuwL7BQNUGoXQTvGG7m+nRjTulOpD2Wax7DHxQrFqt5J74SaiXsd0kvN aqGNLYciTxUSAntggExjBKZp4gP8WBUdyfnkTMKmjBLAVX++W3YheqgTtZZSpH4qRJ3s aXzjvOELYL7sgXtXz4Og+0JdXJzipj78NJuqqG1gMtd3f5It4ktrGFMucXRTe2gEFVq6 5hAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=XYRuRUVmntnbYdM6jiMcV82dPI8UhKg76nxLlKYvlqI=; b=NQJIz7dJDrfsokqVg/fKHLj5bI1nl6X61p2/KAYTxBhyV/HyXQjJVLDJvnB/VKdB98 0IoJag8y5eYDZIKx0j5GCbEIgzRsSolWFRDtisihbqmsfLXhEkTABUKKFXl2kXWaM1C1 SEZdAeUFW3mcGQkUuGmqgq4xzzqSWHlyM4v/ce48lplhCLozhW0hK0ftuVFYoKkYR5oX JUj5t21ZYddd/Hce/gj95eh/IbE3V0d7W7S+2vO9CA8nocTICDbIFnsTUiUs0yfa59z1 Jpxcd1f3osHcVdAq1Brw/NEWabP7NVjR+OsrSzVDPGW7tO1gF3n8tw6wmEMxD2ZG+xG2 ywOg== X-Gm-Message-State: AOUpUlFn5sfuH5IulRNcE7jWF3xgA6m+tdu/qSsg9Qma4lQRpxmhQV/3 qEUWMlEKl6OmXSoCqs7R+WHxiYLRHTzGW2mTz6Fjyw== X-Google-Smtp-Source: AAOMgpcLf0MNcV6usSR8TrL3E/USvPM8l8AOZpA/j4iRHG3e8Ee4UdoMkU5loKVQR+z7XvjWNpuJpn8RpSb31BIv/eg= X-Received: by 2002:a81:83c3:: with SMTP id t186-v6mr7446412ywf.455.1532906811896; Sun, 29 Jul 2018 16:26:51 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a25:b90:0:0:0:0:0 with HTTP; Sun, 29 Jul 2018 16:26:51 -0700 (PDT) In-Reply-To: <20180729223158.GL40119@kib.kiev.ua> References: <201807292047.w6TKl0hV004691@repo.freebsd.org> <20180729223158.GL40119@kib.kiev.ua> From: Oliver Pinter Date: Mon, 30 Jul 2018 01:26:51 +0200 Message-ID: Subject: Re: svn commit: r336876 - in head/sys: amd64/amd64 amd64/ia32 amd64/include conf dev/hyperv/vmbus/amd64 To: Konstantin Belousov Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Jul 2018 23:26:53 -0000 On 7/30/18, Konstantin Belousov 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 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. >