Date: Thu, 14 Jan 2016 09:58:46 -0800 From: John Baldwin <jhb@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r293979 - head/lib/libkvm Message-ID: <2052462.S7a40kjnu6@ralph.baldwin.cx> In-Reply-To: <20160115031903.W9581@besplex.bde.org> References: <201601141551.u0EFpDTf052097@repo.freebsd.org> <20160115031903.W9581@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday, January 15, 2016 04:34:51 AM Bruce Evans wrote: > On Thu, 14 Jan 2016, John Baldwin wrote: > > > Log: > > Fix building with GCC since PAGE_MASK is signed on i386. > > ... > > > Modified: head/lib/libkvm/kvm_i386.h > > ============================================================================== > > --- head/lib/libkvm/kvm_i386.h Thu Jan 14 15:50:13 2016 (r293978) > > +++ head/lib/libkvm/kvm_i386.h Thu Jan 14 15:51:13 2016 (r293979) > > @@ -70,7 +70,7 @@ _Static_assert(NBPDR == I386_NBPDR, "NBP > > > > _Static_assert(PG_V == I386_PG_V, "PG_V mismatch"); > > _Static_assert(PG_PS == I386_PG_PS, "PG_PS mismatch"); > > -_Static_assert(PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch"); > > +_Static_assert((u_int)PG_FRAME == I386_PG_FRAME, "PG_FRAME mismatch"); > > _Static_assert(PG_PS_FRAME == I386_PG_PS_FRAME, "PG_PS_FRAME mismatch"); > > #endif > > This breaks the warning. PG_FRAME still has the fragile type int and > the fragile value -4096. The issue is that PG_FRAME in stable/9 can't retroactively be fixed very easily. However, even PG_FRAME in HEAD still uses (~PAGE_MASK), but head is using clang which doesn't warn about this. I have not tried to build head with a modern gcc to see if it would complain. > libkvm has lots of nearby style bugs like using the long long abomination > as a suffix for the literal constants, and bogus parentheses around > single literals. All of these are copied from the kernel (except their > ames are changed by adding an I386_ prefix). The assertions aren't very > useful when half of the constants use lexically identically definitions > with equal ugliness. The 2 PAE constants are the main ones that aren't > asserted to be equal. The assertions are mostly paranoia. However, it might be better to use fixed constants for all of them as the hope is that the I386_* constants have the same values on all platforms. > I think PAE in the kernel could even use the signedness of PG_FRAME. > (vm_paddr_t)-4096 first sign extends to (int64_t) with value -4096 > and then overflows to the correct mask of 0xfffffffffffff000 with the > correct type. So -4096 could work for both PAE and !PAE. But PAE > actually uses an ifdef to to define PG_FRAME as (0x000ffffffffff000ull). > This has the 2 style bugs mentioned above, and seems to have a nonsense > value. I think PAE is only 36 bits, but the mask is for 52 bits. According to the Intel SDM, PAE can support up to 52 bits as reported by cpuid (what the kernel stores in cpu_maxphysaddr). If the cpuid leaf is not supported, then 36 bits is the maximum PA bits supported. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2052462.S7a40kjnu6>