From owner-svn-src-head@freebsd.org Thu Jan 18 15:55:24 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7DC39EB3DE4; Thu, 18 Jan 2018 15:55:24 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mx0b-0010f301.pphosted.com (mx0b-0010f301.pphosted.com [148.163.153.244]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.pphosted.com", Issuer "thawte SHA256 SSL CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 41F567467B; Thu, 18 Jan 2018 15:55:23 +0000 (UTC) (envelope-from alc@rice.edu) Received: from pps.filterd (m0102859.ppops.net [127.0.0.1]) by mx0b-0010f301.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w0IFpCis009811; Thu, 18 Jan 2018 09:55:17 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rice.edu; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=ricemail; bh=0ShbhQOEsxxzMaFdK9u8GyXcX6UJbaq3HB2Y2qnc7yw=; b=WQ/zebanP6imsksC+J6WVfqcJ59IBvPUO18hA1K+aRLCLPQu6tjepJXrKD8qljTT8aO+ qI01FjhIyg734p9R+WfQn7r0h4atE/Nb+QrnjjV6NQkyfoa/9l6EG0zi2sndr68E7sd2 StI4UTcWRLU0GDdrcgwgsyLlDNuyziBz45e/b2/mejnxOTdtIT1snNZpa4fk13fT/vkx VEfITF0O4Dc06E0ZfNoHstX1SemaCfpjFA2nhDq4tPdy2vI9Wae88OzXYL/EIsZgXFsR hepGOyI7ubvjdcpn4VLk9NkUr0ctvDwM9kQCDEQ26L6tqlk+/SfcEF8GXWS6qBrA2YAD sA== Received: from mh1.mail.rice.edu (mh1.mail.rice.edu [128.42.201.20]) by mx0b-0010f301.pphosted.com with ESMTP id 2fjb04s881-1; Thu, 18 Jan 2018 09:55:17 -0600 Received-X: from mh1.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh1.mail.rice.edu (Postfix) with ESMTP id 59FD8460E4A; Thu, 18 Jan 2018 09:55:16 -0600 (CST) Received-X: from mh1.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh1.mail.rice.edu (Postfix) with ESMTP id 589AC46051C; Thu, 18 Jan 2018 09:55:16 -0600 (CST) X-Virus-Scanned: by amavis-2.7.0 at mh1.mail.rice.edu, auth channel Received-X: from mh1.mail.rice.edu ([127.0.0.1]) by mh1.mail.rice.edu (mh1.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id 65auO74bfMn2; Thu, 18 Jan 2018 09:55:16 -0600 (CST) Received: from 108-254-203-201.lightspeed.hstntx.sbcglobal.net (108-254-203-201.lightspeed.hstntx.sbcglobal.net [108.254.203.201]) (using TLSv1 with cipher RC4-MD5 (112/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh1.mail.rice.edu (Postfix) with ESMTPSA id E523E460AE8; Thu, 18 Jan 2018 09:55:15 -0600 (CST) Subject: Re: svn commit: r327950 - in head/sys/powerpc: aim include powerpc ps3 To: Konstantin Belousov , Nathan Whitehorn Cc: Marius Strobl , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org References: <20180115111812.GF1684@kib.kiev.ua> <20180115170603.GJ1684@kib.kiev.ua> <9e5554d7-6a0c-5910-8cb6-74f98259536f@freebsd.org> <20180115175335.GK1684@kib.kiev.ua> <20180116193208.GA12364@alchemy.franken.de> <11a7fdd6-cfd6-26c1-ae3c-7d8a63924d5a@freebsd.org> <20180117094413.GF55707@kib.kiev.ua> <57f837ce-1209-1e9a-158f-7eac5ae6d59a@freebsd.org> <20180118153532.GR55707@kib.kiev.ua> From: Alan Cox Message-ID: <2d1644f8-c056-2b07-97b1-7ac211cf8e1c@rice.edu> Date: Thu, 18 Jan 2018 09:55:15 -0600 User-Agent: Mozilla/5.0 (X11; FreeBSD i386; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20180118153532.GR55707@kib.kiev.ua> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Language: en-US X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2018-01-18_07:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=3 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=796 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1801180212 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 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: Thu, 18 Jan 2018 15:55:24 -0000 On 01/18/2018 09:35, Konstantin Belousov wrote: > On Thu, Jan 18, 2018 at 07:24:11AM -0800, Nathan Whitehorn wrote: >> >> On 01/17/18 01:44, Konstantin Belousov wrote: >>> On Tue, Jan 16, 2018 at 09:30:29PM -0800, Nathan Whitehorn wrote: >>>> On 01/16/18 11:32, Marius Strobl wrote: >>>>> On Mon, Jan 15, 2018 at 03:20:49PM -0800, Nathan Whitehorn wrote: >>>>>> On 01/15/18 09:53, Konstantin Belousov wrote: >>>>>>> On Mon, Jan 15, 2018 at 09:32:56AM -0800, Nathan Whitehorn wrote:= >>>>>>>> That seems fine to me. I don't think a less-clumsy way that does= not >>>>>>>> involve extra indirection is possible. The PHYS_TO_DMAP() return= ing NULL >>>>>>>> is about the best thing I can come up with from a clumsiness sta= ndpoint >>>>>>>> since plenty of code checks for null pointers already, but doesn= 't >>>>>>>> cleanly handle the rarer case where you want to test for the exi= stence >>>>>>>> of direct maps in general without testing some potemkin address.= >>>>>>>> >>>>>>>> My one reservation about PMAP_HAS_DMAP or the like as a selector= is that >>>>>>>> it does not encode the full shape of the problem: one could imag= ine >>>>>>>> having a direct map that only covers a limited range of RAM (I a= m not >>>>>>>> sure whether the existence of dmaplimit on amd64 implies this ca= n happen >>>>>>>> with non-device memory in real life), for example. These cases a= re >>>>>>>> currently covered by an assert() in PHYS_TO_DMAP(), whereas havi= ng >>>>>>>> PHYS_TO_DMAP() return NULL allows a more flexible signalling and= the >>>>>>>> potential for the calling code to do something reasonable to han= dle the >>>>>>>> error. A single global flag can't convey information at this kin= d of >>>>>>>> granularity. Is this a reasonable concern? Or am I overthinking = things? >>>>>>> IMO it is overreaction. amd64 assumes that all normal memory is = covered >>>>>>> by DMAP. It must never fail. See, for instance, the implementa= tion >>>>>>> of the sf bufs for it. >>>>>>> >>>>>>> If device memory not covered by DMAP can exists, it is the driver= problem. >>>>>>> For instance, for NVDIMMs I wrote specific mapping code which est= ablishes >>>>>>> kernel mapping for it, when not covered by EFI memory map and cor= respondingly >>>>>>> not included into DMAP. >>>>>>> >>>>>> Fair enough. Here's a patch with a new flag (DIRECT_MAP_AVAILABLE)= =2E I've >>>>>> also retooled the sfbuf code to use this rather than its own flags= that >>>>>> mean the same things. The sparc64 part of the patch is untested. >>>>>> -Nathan >>>>>> Index: sparc64/include/vmparam.h >>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>> --- sparc64/include/vmparam.h (revision 328006) >>>>>> +++ sparc64/include/vmparam.h (working copy) >>>>>> @@ -240,10 +240,12 @@ >>>>>> */ >>>>>> #define ZERO_REGION_SIZE PAGE_SIZE >>>>>> =20 >>>>>> +#include >>>>>> + >>>>>> #define SFBUF >>>>>> #define SFBUF_MAP >>>>>> -#define SFBUF_OPTIONAL_DIRECT_MAP dcache_color_ignore >>>>>> -#include >>>>>> -#define SFBUF_PHYS_DMAP(x) TLB_PHYS_TO_DIRECT(x) >>>>>> =20 >>>>>> +#define DIRECT_MAP_AVAILABLE dcache_color_ignore >>>>>> +#define PHYS_TO_DMAP(x) (DIRECT_MAP_AVAILABLE ? (TLB_PHYS_TO_DIRE= CT(x) : 0) >>>>> What dcache_color_ignore actually indicates is the presence of >>>>> hardware unaliasing support, in other words the ability to enter >>>>> duplicate cacheable mappings into the MMU. While a direct map is >>>>> available and used by MD code on all supported CPUs down to US-I, >>>>> the former feature is only implemented in the line of Fujitsu SPARC= 64 >>>>> processors. IIRC, the sfbuf(9) code can't guarantee that there isn'= t >>>>> already a cacheable mapping from a different VA to the same PA, >>>>> which is why it employs dcache_color_ignore. Is that a general >>>>> constraint of all MI PHYS_TO_DMAP users or are there consumers >>>>> which can guarantee that they are the only users of a mapping >>>>> to the same PA? >>>>> >>>>> Marius >>>>> >>>> With the patch, there are four uses of this in the kernel: the sfbuf= >>>> code, a diagnostic check on page zeroing, part of the EFI runtime co= de, >>>> and part of the Linux KBI compat. The second looks safe from this >>>> perspective and at least some of the others (EFI runtime) are irrele= vant >>>> on sparc64. But I really have no idea what was intended for the >>>> semantics of this API -- I didn't even know it *was* an MI API until= >>>> this commit. Maybe kib can comment? If this is outside the semantics= of >>>> PHYS_TO_DMAP, then we need to keep the existing sfbuf code. >>> sfbufs cannot guarantee that there is no other mapping of the page wh= en >>> the sfbuf is created. For instance, one of the use of sfbufs is to m= ap >>> the image page 0 to read ELF headers when doing the image activation.= >>> The image might be mapped by other processes, and we do not control t= he >>> address at which it mapped. >>> >>> So the direct map accesses must work regardless of the presence of ot= her >>> page mappings, and the check for dcache_color_ignore is needed to all= ow >>> MI code to take advantage of DMAP. >>> >> So: what do you want to happen with PHYS_TO_DMAP()? Do we want to clai= m=20 >> to MI that a direct map is "available" in such circumstances, or=20 >> "unavailable"? Should sfbuf retain a separate API? I have no preferenc= es=20 >> here and just want to close out this issue. > Perhaps DMAP should be conditionally available to the MI layer, same as= > on powerpc ? I.e. your patch cited above looks right to me, unless I > misunderstand the Marius' response. > Yes, it should. Only the sparc64 machines where dcache_color_ignore is true should attempt to use the direct map in MI code. The machine-dependent uiomove_fromphys() on sparc64 shows the hoops one has to jump through in order to correctly use the direct map on sparc64.