From owner-svn-src-head@freebsd.org Tue Jan 16 02:40:48 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 B11EEEBC69F; Tue, 16 Jan 2018 02:40:48 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from d.mail.sonic.net (d.mail.sonic.net [64.142.111.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8F80F7EC33; Tue, 16 Jan 2018 02:40:48 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from comporellon.tachypleus.net (cpe-75-82-218-62.socal.res.rr.com [75.82.218.62]) (authenticated bits=0) by d.mail.sonic.net (8.15.1/8.15.1) with ESMTPSA id w0G2ekx0016323 (version=TLSv1.2 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Mon, 15 Jan 2018 18:40:46 -0800 Subject: Re: svn commit: r327950 - in head/sys/powerpc: aim include powerpc ps3 To: Konstantin Belousov Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <20180114175211.GD1684@kib.kiev.ua> <20180115111812.GF1684@kib.kiev.ua> <20180115170603.GJ1684@kib.kiev.ua> <9e5554d7-6a0c-5910-8cb6-74f98259536f@freebsd.org> <20180115175335.GK1684@kib.kiev.ua> <20180115234218.GN1684@kib.kiev.ua> From: Nathan Whitehorn Message-ID: Date: Mon, 15 Jan 2018 18:40:46 -0800 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20180115234218.GN1684@kib.kiev.ua> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Sonic-CAuth: UmFuZG9tSVZqbRYg/jATcwsGgnm2CQ9rzsNTlDvSqKpM2X1LYUCSJRpb/wD+x8o+e9X+lJj5yR3OTSu2AwUgX678W36m/98qilZxXiX8hvI= X-Sonic-ID: C;VqiCp2b65xGlUOBQ9eIpJw== M;BsbIp2b65xGlUOBQ9eIpJw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd 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: Tue, 16 Jan 2018 02:40:48 -0000 On 01/15/18 15:42, Konstantin Belousov wrote: > On Mon, Jan 15, 2018 at 03:20:49PM -0800, Nathan Whitehorn wrote: >> Fair enough. Here's a patch with a new flag (DIRECT_MAP_AVAILABLE). 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: amd64/include/vmparam.h >> =================================================================== >> --- amd64/include/vmparam.h (revision 328006) >> +++ amd64/include/vmparam.h (working copy) >> @@ -190,6 +190,7 @@ >> * because the result is not actually accessed until later, but the early >> * vt fb startup needs to be reworked. >> */ >> +#define DIRECT_MAP_AVAILABLE 1 >> #define PHYS_TO_DMAP(x) ({ \ >> KASSERT(dmaplimit == 0 || (x) < dmaplimit, \ >> ("physical address %#jx not covered by the DMAP", \ >> Index: arm64/include/vmparam.h >> =================================================================== >> --- arm64/include/vmparam.h (revision 328006) >> +++ arm64/include/vmparam.h (working copy) >> @@ -176,6 +176,7 @@ >> #define VIRT_IN_DMAP(va) ((va) >= DMAP_MIN_ADDRESS && \ >> (va) < (dmap_max_addr)) >> >> +#define DIRECT_MAP_AVAILABLE > Just define, or define it to 1 ? Yes, sorry for typo. > >> #define PHYS_TO_DMAP(pa) \ >> ({ \ >> KASSERT(PHYS_IN_DMAP(pa), \ >> Index: dev/efidev/efirt.c >> =================================================================== >> --- dev/efidev/efirt.c (revision 328006) >> +++ dev/efidev/efirt.c (working copy) >> @@ -115,6 +115,11 @@ >> return (0); >> } >> efi_systbl = (struct efi_systbl *)PHYS_TO_DMAP(efi_systbl_phys); >> + if (efi_systbl == NULL) { >> + if (bootverbose) >> + printf("EFI systbl not mapped in kernel VA\n"); >> + return (0); >> + } > Is this chunk still needed ? The existing code is a bit of an awkward superposition of the "return NULL" idea and having the flag. Since you think there will never be intermediate cases -- which seems reasonable -- I will rip the conditional logic out and add a KASSERT matching the ones on arm64 and amd64 to the powerpc version. > >> if (efi_systbl->st_hdr.th_sig != EFI_SYSTBL_SIG) { >> efi_systbl = NULL; >> if (bootverbose) >> Index: kern/subr_sfbuf.c >> =================================================================== >> --- kern/subr_sfbuf.c (revision 328006) >> +++ kern/subr_sfbuf.c (working copy) >> @@ -88,8 +88,8 @@ >> vm_offset_t sf_base; >> int i; >> >> -#ifdef SFBUF_OPTIONAL_DIRECT_MAP >> - if (SFBUF_OPTIONAL_DIRECT_MAP) >> +#ifdef DIRECT_MAP_AVAILABLE >> + if (DIRECT_MAP_AVAILABLE) >> return; > Would it make sense to define the symbol on all other arches as 0 then, > and remove #ifdef ? Returning to your initial proposal of relying on the > compiler optimiing if (0) block; out. That is a good idea. > Also, just curious, why did you spelled DMAP as DIRECT_MAP ? > DMAP without the PHYS_TO_ seemed lacking in context and I was worried there might be a collision on DMAP. PMAP_HAS_DMAP would also work; I don't have a preference. Thanks for your patience working this out in real time with me. -Nathan