Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 19 Nov 2016 20:47:15 -0600
From:      Justin Hibbits <jhibbits@freebsd.org>
To:        Mark Millard <markmi@dsl-only.net>
Cc:        svn-src-head@freebsd.org, FreeBSD Current <freebsd-current@freebsd.org>
Subject:   Re: svn commit: r308817 - head/sys/powerpc/include [Still have pmap_t and struct pmap ppowerpc64 problems as of -r308860]
Message-ID:  <20161119204715.79632a66@zhabar.knownspace>
In-Reply-To: <53258F35-C86E-4DE0-BDF0-5C139E68356D@dsl-only.net>
References:  <39962D4C-29BA-4AA4-B77D-2344A68FDB54@dsl-only.net> <53258F35-C86E-4DE0-BDF0-5C139E68356D@dsl-only.net>

next in thread | previous in thread | raw e-mail | index | archive | help
--MP_/eYKnT8u=tt7OZI3sR.Gf6ts
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Sat, 19 Nov 2016 18:36:39 -0800
Mark Millard <markmi@dsl-only.net> wrote:

> [Quick top post I'm afraid.]
> 
> I think that I figured out why there is a problem even earlier
> --that just did not stop the compiles.
> 
> lib/libutil/kinfo_getallproc.c is built here as part of buildworld
> (stage 4.2 "building libraries" instead of buildkernel. It does not
> have the KERNCONF's AIM vs. BOOKE vs. . . . definitions vs. lack of
> them).
> 
> So if it includes machine/pmap.h that binds to
> sys/powerpc/include/pmap.h which has the structure. . .
> 
> . . .
> #if defined(AIM)
> . . . (definitions here)
> #elif defined(BOOKE)
> . . . (definitions here)
> #endif
> . . .
> 
> it gets no definition now.
> 
> With the older:
> 
> . . .
> #if defined(AIM)
> . . . (definitions here)
> #else
> . . . (definitions here)
> #endif
> . . .
> 
> It got a definition, just not necessarily the right one.
> 
> 
> ===
> Mark Millard
> markmi at dsl-only.net

Can you try the attached patch?  There was a subtle ABI issue that
r308817 exposed, which is that the pmap structs aren't identical such
that the pm_stats are at different locations, and libkvm ends up
reading with the Book-E pmap, getting different stats than expected for
AIM.  This patch fixes that, bumping version to account for this ABI
change.

- Justin
--MP_/eYKnT8u=tt7OZI3sR.Gf6ts
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename=fix_pmap.diff

Index: sys/sys/param.h
===================================================================
--- sys/sys/param.h	(revision 308708)
+++ sys/sys/param.h	(working copy)
@@ -58,7 +58,7 @@
  *		in the range 5 to 9.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 1200014	/* Master, propagated to newvers */
+#define __FreeBSD_version 1200015	/* Master, propagated to newvers */
 
 /*
  * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,
Index: sys/powerpc/include/pmap.h
===================================================================
--- sys/powerpc/include/pmap.h	(revision 308718)
+++ sys/powerpc/include/pmap.h	(working copy)
@@ -74,6 +74,9 @@
 #include <machine/slb.h>
 #include <machine/tlb.h>
 
+struct pmap;
+typedef struct pmap *pmap_t;
+
 #if defined(AIM)
 
 #if !defined(NPMAPS)
@@ -81,8 +84,6 @@
 #endif /* !defined(NPMAPS) */
 
 struct	slbtnode;
-struct	pmap;
-typedef	struct pmap *pmap_t;
 
 struct pvo_entry {
 	LIST_ENTRY(pvo_entry) pvo_vlink;	/* Link to common virt page */
@@ -131,6 +132,7 @@
 #define	PVO_VSID(pvo)		((pvo)->pvo_vpn >> 16)
 
 struct	pmap {
+	struct		pmap_statistics	pm_stats;
 	struct	mtx	pm_mtx;
 	
     #ifdef __powerpc64__
@@ -143,7 +145,6 @@
 	cpuset_t	pm_active;
 
 	struct pmap	*pmap_phys;
-	struct		pmap_statistics	pm_stats;
 	struct pvo_tree pmap_pvo;
 };
 
@@ -179,13 +180,13 @@
 struct slb **slb_alloc_user_cache(void);
 void	slb_free_user_cache(struct slb **);
 
-#else
+#elif defined(BOOKE)
 
 struct pmap {
+	struct pmap_statistics	pm_stats;	/* pmap statistics */
 	struct mtx		pm_mtx;		/* pmap mutex */
 	tlbtid_t		pm_tid[MAXCPU];	/* TID to identify this pmap entries in TLB */
 	cpuset_t		pm_active;	/* active on cpus */
-	struct pmap_statistics	pm_stats;	/* pmap statistics */
 
 	/* Page table directory, array of pointers to page tables. */
 	pte_t			*pm_pdir[PDIR_NENTRIES];
@@ -193,7 +194,6 @@
 	/* List of allocated ptbl bufs (ptbl kva regions). */
 	TAILQ_HEAD(, ptbl_buf)	pm_ptbl_list;
 };
-typedef	struct pmap *pmap_t;
 
 struct pv_entry {
 	pmap_t pv_pmap;
@@ -210,6 +210,16 @@
 #define	pmap_page_get_memattr(m)	VM_MEMATTR_DEFAULT
 #define	pmap_page_is_mapped(m)	(!TAILQ_EMPTY(&(m)->md.pv_list))
 
+#else
+/*
+ * Common pmap members between AIM and BOOKE.
+ * libkvm needs pm_stats at the same location between both, as it doesn't define
+ * AIM nor BOOKE, and is expected to work across all.
+ */
+struct pmap {
+	struct pmap_statistics	pm_stats;	/* pmap statistics */
+	struct mtx		pm_mtx;		/* pmap mutex */
+};
 #endif /* AIM */
 
 extern	struct pmap kernel_pmap_store;
Index: UPDATING
===================================================================
--- UPDATING	(revision 308708)
+++ UPDATING	(working copy)
@@ -51,6 +51,11 @@
 
 ****************************** SPECIAL WARNING: ******************************
 
+20161119:
+	The layout of the pmap structure has changed for powerpc to put the pmap
+	statistics at the front for all CPU variations.  libkvm(3) and all tools
+	that link against it need to be recompiled.
+
 20161030:
 	isl(4) and cyapa(4) drivers now require a new driver,
 	chromebook_platform(4), to work properly on Chromebook-class hardware.

--MP_/eYKnT8u=tt7OZI3sR.Gf6ts--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161119204715.79632a66>