From owner-svn-src-head@freebsd.org Mon Dec 3 16:21:48 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 679BD132BAD4; Mon, 3 Dec 2018 16:21:48 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-it1-x142.google.com (mail-it1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id AEA008914F; Mon, 3 Dec 2018 16:21:47 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: by mail-it1-x142.google.com with SMTP id c9so9424443itj.1; Mon, 03 Dec 2018 08:21:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=xkH8ERIRDKa9Xym77l2ToTbhfkNMLt+71fXDiqpFlbA=; b=K8LQmSCPr03htwuR7kKe+8OrfTn/3xX6PfF/Yg/pG54GtVOJFBGC/SSzdjfIFCgfJS vFyjSljSS72/0EFXZHTkJpfHj5b1VrZdljqS5FOqZMC/tcomEcTz54kTAyPVdbLvCxjE e3NSottdL900iG5O8F2B0P91bTFYGeZqUFhQoXlHAzVXFZoE8Fhol+r8vuNocl6hsyPg VOL7aVNhlESIowCnXnfN2LexTo9rrpvceWrUhLuxWtA7zDQvXw+v+0V6IiJA79ZCbChX wjXXXCSRZZGROSeztrVA8fcYkvQl3Q04bUhtMWVV08TFKfUxcjkWXZewn17Wd1ZpeV1U U/0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :in-reply-to:references:mime-version:content-transfer-encoding; bh=xkH8ERIRDKa9Xym77l2ToTbhfkNMLt+71fXDiqpFlbA=; b=OGTqPEIFBqsvLp+sLDrGIqUPCJ+RUh8yTf/6vdQ4VfZNkdDeBojY6tqIeWvz1k98r9 YBSIZI4kyiYUg5yeEWh0QaOkRwDs7LcXuD/FXjhPl74ltvF4XxjV7JUyXxWt7FGFw4KY WGrSIJrDx8D109/8lu7VOJ8ak9qaAcEndd0HI92Uge0bdazmddoXBWPbnV70LB5xH1tg SHQGF6A5z2GQrQ60UFlqsAJ8S7DH6MRlj4mj8k30qwl9L6YigRhgIfenn8aDKOHf9AaP G7DD8qrhhMj5EbzVLAyKro88YfAmp1fEQ4hD88+JNKH75tZo34cI6igcaih35pMh5SYN lwPg== X-Gm-Message-State: AA+aEWYr5RkLnyxwuUwVfWi72Q8WIX5aIpP6vjKehIwErGFcISTSgN9P UDJv4U+d5GblRIlXgS/Zu50= X-Google-Smtp-Source: AFSGD/XYXh2ct3EE3KUeHdTQi7gpLsQzvWdkqyWwy7EYZIl0uqMlI2SrGUcGFCbE6UOyh3jv7aEeQA== X-Received: by 2002:a24:2fcb:: with SMTP id j194mr8698709itj.56.1543854106792; Mon, 03 Dec 2018 08:21:46 -0800 (PST) Received: from ralga.knownspace (173-25-245-129.client.mchsi.com. [173.25.245.129]) by smtp.gmail.com with ESMTPSA id c10sm5156304iok.23.2018.12.03.08.20.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 03 Dec 2018 08:20:43 -0800 (PST) Sender: Justin Hibbits Date: Mon, 3 Dec 2018 10:20:07 -0600 From: Justin Hibbits To: Bruce Evans Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r341103 - head/sys/powerpc/include Message-ID: <20181203102007.4021aa32@ralga.knownspace> In-Reply-To: <20181128151148.X1660@besplex.bde.org> References: <201811280248.wAS2miqW055485@repo.freebsd.org> <20181128151148.X1660@besplex.bde.org> X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.32; powerpc64-portbld-freebsd13.0) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: AEA008914F X-Spamd-Result: default: False [-1.51 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[gmail.com]; NEURAL_HAM_MEDIUM(-0.59)[-0.590,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; NEURAL_HAM_LONG(-0.86)[-0.864,0]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[FreeBSD.org]; TO_DN_SOME(0.00)[]; RCVD_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; MX_GOOD(-0.01)[cached: alt3.gmail-smtp-in.l.google.com]; RCVD_IN_DNSWL_NONE(0.00)[2.4.1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.b.8.f.7.0.6.2.list.dnswl.org : 127.0.5.0]; NEURAL_HAM_SHORT(-0.08)[-0.076,0]; IP_SCORE(0.23)[ip: (4.19), ipnet: 2607:f8b0::/32(-1.66), asn: 15169(-1.29), country: US(-0.09)]; FORGED_SENDER(0.30)[jhibbits@FreeBSD.org,chmeeedalf@gmail.com]; FREEMAIL_TO(0.00)[optusnet.com.au]; RCVD_TLS_LAST(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[jhibbits@FreeBSD.org,chmeeedalf@gmail.com] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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: Mon, 03 Dec 2018 16:21:48 -0000 On Wed, 28 Nov 2018 16:31:33 +1100 (EST) Bruce Evans wrote: > On Wed, 28 Nov 2018, Justin Hibbits wrote: > > > Log: > > powerpc: Fix the powerpc64 build post-r341102 > > > > VM_MIN_KERNEL_ADDRESS is now used in locore.S, but the UL suffix > > isn't permitted in .S files. > > The UL suffix is arguably a style bug in .c files too. It was not > even wrong (it had no effect) this case, but nearby code seems to be > more broken. > > A ULL suffix would be unarguably a style bug everywhere. I'll take a closer look at this eventually. I'm in the process of overhauling a lot of the Book-E bits to play nicer with loader, etc. This does involve touching vmparam.h a lot, so I'll think about cleaning it up for these cases as well. > > > Modified: head/sys/powerpc/include/vmparam.h > > ============================================================================== > > --- head/sys/powerpc/include/vmparam.h Wed Nov 28 02:00:27 > > 2018 (r341102) +++ head/sys/powerpc/include/vmparam.h > > Wed Nov 28 02:48:43 2018 (r341103) @@ -106,8 +106,13 @@ > > #define FREEBSD32_USRSTACK FREEBSD32_SHAREDPAGE > > > > #ifdef __powerpc64__ > > +#ifndef LOCORE > > #define VM_MIN_KERNEL_ADDRESS > > 0xe000000000000000UL #define > > VM_MAX_KERNEL_ADDRESS 0xe0000007ffffffffUL +#else > > +#define VM_MIN_KERNEL_ADDRESS > > 0xe000000000000000 +#define > > VM_MAX_KERNEL_ADDRESS 0xe0000007ffffffff +#endif > > These constants automatically have type unsigned long, since they are > larger that LONG_MAX and smaller than ULONG_MAX. Thus the UL suffix > had no effect except to break in asm files. > > This would not be true for smaller constants. Smaller constants often > need to be combined or shifted back and forth, and then it may be > necessary to use them as unsigned int or unsigned long constants > (signed constants are better handled by implicit conversions between > int and long unless they are mixed with unsigned constants). This is > sometimes done using U or UL or suffixes. I don't like this. Cases > where the natural type doesn't work are delicate and it is better to > not hide the unnatural conversions by implicitly converting using a > suffix on some constants. > > The correct fix is to remove the bogus UL suffixes and not add any > ifdefs. > > On 32-bit arches, the above constants would have natural type > [(long long abomination; should be deleted]. The abominatation is > unavoidable, but badly written code increases it using explicit > [abomination deleted] suffixes. > > > #define VM_MAX_SAFE_KERNEL_ADDRESS > > VM_MAX_KERNEL_ADDRESS #endif > > Nearby code has mounds of unportabilities and style bugs. E.g., > - VM_MIN_ADDRESS for the !LOCORE && 64-bit case has bogus UL suffixes > and bogus parentheses around single tokens. It is 0, so it has > natural type int, so the suffix might be needed to obfuscate > conversions to unsigned long in some cases. > - VM_MIN_ADDRESS for the !LOCORE && 32-bit case casts to vm_offset_t > instead of hard-coding the same conversion using a suffix. The cast > is more technically correct, but is an even larger syntax error -- it > breaks both asm uses and uses in cpp expressions. Indeed, the cast is probably redundant. > - VM_MIN_ADDRESS already had the bogus ifdefs to support LOCORE. > These are expressed in the opposite order (LOCORE is in the outer > ifdef instead of the inner ifdef), which makes them more unreadable. > - similarly for VM_MAXUSER_ADDRESS, plus an extra convolution to > define this in terms of VM_MAXUSER_ADDRESS32 in the 32-bit case, but > only for the !LOCORE case (since VM_MAXUSER_ADDRESS32 doesn't have a > LOCORE ifdef). Agreed. Removing the LOCORE differentiation would certainly clean up the file. > > I used to like casts on constants and macros, like the ones here for > vm_offset_t here, but jake@ convinced me that this was wrong in > connection with PAE and sparc64 work. The wrongness is most obvious > for PAE. vm_paddr_t is 64 bits for PAE, but most vm types for PAE > (especially vm_offset_t) are only 32 bits. If you sprinkle > [abomination deleted] suffixes or casts to uint64_t or vm_paddr_t on > constants or macros, then this is too pessimal and/or confusing for > most vm expressions. Sprinkling UL suffixes and vm_offset_t casts is > relatively harmless only because it usually has no effect, especially > on 32-bit non-PAE arches where even more types are naturally 32 bits. > > The i386 vmparam.h has the following style bugs in this area: > - UL suffixes only for MAXTSIZ and friends > - cast to vm_offset_t only for VM_MIN_KERNEL_ADDRESS > - VM_MAX_KERNEL_ADDRESS and related values are encrypted through about > 10 layers of macros depending on configuration variables like PAE. > Most of the encryption is no longer really used, since it was > mostly to make addresses depend on the user/kernel split. > > The encryptions mostly use signed constants and expressions, but a > critical one is NPDEPTD defined in another file using sizeof(). > Unsigned poisoning from this probably leaks into most macros, but > the resulting types are even less clear than the resulting values > since the resulting values are now documented for the only > supported user/kernel split. > > - bogus extra parentheses only for VM_KMEM_SIZE_SCALE. > > The existence and default value of this macro are also wrong. They > were last almost correct when the default user/kernel split was 3/1 > and the physical memory size was 3 or 4 GB. > > The default for the macro is 3, which is closely related to the > split ratio. This value works to prevent overflow in kva size > calculations when the physical memory size is larger than the total > kva size but less than 3 or 4 GB. Overflow still occurs for PAE > since then the memory size can be larger than 4GB. Other magic > properties of 3 result in the overflows giving reasonable values for > the kva sizes when the memory size is 8GB or 16GB, but not when it is > 12GB (because 12GB / 3 = 4GB = 0GB after overflow, and 0GB is not a > useful kva size). Thanks for the x86 lesson with this. > > Bruce - Justin