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.
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.
- 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).
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).
Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"