Re: port-alpha/48488: pmap.h modify cause Kernel Panic on NetBSD/alpha current version.

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

The following reply was made to PR port-alpha/48488; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: martin%duskware.de@localhost, nullnilaki%gmail.com@localhost, 
tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: port-alpha/48488: pmap.h modify cause Kernel Panic on NetBSD/alpha
         current version.
Date: Thu, 2 Jan 2014 01:16:41 +0900

 martin@
 
 >  > I think this modify is wrong.
 >  > 
 > http://cvsweb.netbsd.org/cgi-bin/cvsweb.cgi/src/sys/arch/alpha/include/pmap.h.diff?r1=1.77&r2=1.78&f=h
 >  
 >  This change looks correct (and is necessary to make alpha kernels
 >  compilable with modern compilers),
 
 I don't think so.
 The definition was an (invalid) hack to have "variable sized arrays"
 so disabling out-of-range warnings is still better for now.
 
 >  but the question is: where is sizeof
 >  used incorrectly on this struct.
 
 struct pmap was defined as:
 http://nxr.netbsd.org/xref/src/sys/arch/alpha/include/pmap.h?r=1.77#136
 ---
     136 struct pmap {
     137        TAILQ_ENTRY(pmap)       pm_list;        /* list of all pmaps */
     138        pt_entry_t              *pm_lev1map;    /* level 1 map */
     139        int                     pm_count;       /* pmap reference count 
*/
     140        kmutex_t                pm_lock;        /* lock on pmap */
     141        struct pmap_statistics  pm_stats;       /* pmap statistics */
     142        unsigned long           pm_cpus;        /* mask of CPUs using 
pmap */
     143        unsigned long           pm_needisync;   /* mask of CPUs needing 
isync */
     144        struct pmap_asn_info    pm_asni[1];     /* ASN information */
     145                        /*      variable length         */
     146 };
 
 and it's allocated by pool(9) and its size is deteremined at runtime
 (to adjust it per a number of CPUs):
 http://nxr.netbsd.org/xref/src/sys/arch/alpha/alpha/pmap.c?r=1.258#888
 ---
     888        pool_cache_bootstrap(&pmap_pmap_cache, 
PMAP_SIZEOF(pmap_ncpuids), 0,
     889            0, 0, "pmap", NULL, IPL_NONE, NULL, NULL, NULL);
 ---
 
 http://nxr.netbsd.org/xref/src/sys/arch/alpha/alpha/pmap.c?r=1.258#1143
 ---
    1143        pmap = pool_cache_get(&pmap_pmap_cache, PR_WAITOK);
    1144        memset(pmap, 0, sizeof(*pmap));
    1145 
    1146        /*
    1147         * Defer allocation of a new level 1 page table until
    1148         * the first new mapping is entered; just take a reference
    1149         * to the kernel kernel_lev1map.
    1150         */
    1151        pmap->pm_lev1map = kernel_lev1map;
    1152 
    1153        pmap->pm_count = 1;
    1154        for (i = 0; i < pmap_ncpuids; i++) {
    1155                pmap->pm_asni[i].pma_asn = PMAP_ASN_RESERVED;
    1156                /* XXX Locking? */
    1157                pmap->pm_asni[i].pma_asngen = 
pmap_asn_info[i].pma_asngen;
    1158        }
 ---
 
 
 PMAP_SIZEOF() is defined as:
 http://nxr.netbsd.org/xref/src/sys/arch/alpha/include/pmap.h?r=1.77#148
 ---
     152 #define        PMAP_SIZEOF(x)                                          
        \
     153        (ALIGN(sizeof(struct pmap) +                                    
\
     154               (sizeof(struct pmap_asn_info) * ((x) - 1))))
 
 ---
 
 So if you really want to appease the warning, I think you have to
 define strcut pmap_asn_info array as pm_asni[ALPHA_MAXPROCS]
 and adjust all reference of sizeof(struct pmap), including
 the above memset() in pmap_create() etc.
 
 ---
 Izumi Tsutsui