Re: [PATCH] Fix NKPT kernel config option

On Friday, September 02, 2011 1:23:16 am Robert Millan wrote:
> 2011/9/1 John Baldwin <jhb_at_freebsd.org>:
> > In general we force the relevant C files to use opt_*.h includes and avoid
> > nested includes of those in headers.
> 
> With this approach I can't trust that this feature will do the right
> thing.  I would rather modify pmap.h by hand than run the unnecessary
> risk.
> 
> > Do you know of any C files that do
> > are using NKPT (or values derived from it) without including opt_pmap.h?
> 
> Many.  To obtain a full list, I suggest you put something like:
> 
> #ifdef NKPT
> #warning nkpt:good
> #else
> #warning nkpt:bad
> #endif
> 
> in pmap.h and build with "WERROR="

This is rather pointless as it dosen't catch the places that actually _use_
NKPT.  The <machine/pmap.h> header is used in lots of places for other things
it defines such as 'struct pmap' itself.  However, NKPT is a purely MD item
that is only used in the amd64 code itself.  Specifically, it is used here:

amd64/minidump_machdep.c:       for (va = VM_MIN_KERNEL_ADDRESS; va < MAX(KERNBASE + NKPT * NBPDR,
amd64/minidump_machdep.c:       for (va = VM_MIN_KERNEL_ADDRESS; va < MAX(KERNBASE + NKPT * NBPDR,
amd64/pmap.c:   KPTphys = allocpages(firstaddr, NKPT);
amd64/pmap.c:   for (i = 0; i < NKPT; i++) {
amd64/pmap.c:   for (i = 0; i < NKPT; i++) {
amd64/pmap.c:   if (KERNBASE < addr && addr <= KERNBASE + NKPT * NBPDR)
conf/NOTES:options      NKPT=31
include/pmap.h:#ifndef NKPT
include/pmap.h:#define  NKPT            32
include/pmap.h:#define NKPDPE           howmany(NKPT, NPDEPG)/* number of kernel PDP slots */

This gives uses in amd64/minidump_machdep.c and amd64/pmap.c and to define
the macro NKPDPE.  That macro is also MD and is only used here:

amd64/pmap.c:   KPDphys = allocpages(firstaddr, NKPDPE);
amd64/pmap.c:   for (i = 0; i < NKPDPE; i++) {
include/pmap.h:#define NKPDPE           howmany(NKPT, NPDEPG)/* number of kernel PDP slots */

Thus, the only two files that use NKPT are amd64/minidump_machdep.c and
amd64/pmap.c.  Both of these files include "opt_pmap.h":

amd64/minidump_machdep.c:#include "opt_pmap.h"
amd64/pmap.c:#include "opt_pmap.h"

A more useful test would be to alter pmap.h so it says:

#ifndef NKPT
#define NKPT doesnt_compile
#endif

and build a kernel config that contains 'options NKPT=31'.  It will build
just fine as the sabotaged default value will never get used.

Were NKPT to be widely used, the correct fix would still not be to add an
include of opt_pmap.h to <machine/pmap.h>.  Instead, the corect fix would be
to move the option to opt_global.h as is done for things like SMP,
INVARIANTS, etc.

-- 
John Baldwin