svn commit: r276187 - head/sys/arm/arm

[Ian Lepore]

On Wed, 2014-12-24 at 17:05 -0800, Rui Paulo wrote:
> On Dec 24, 2014, at 14:40, Ian Lepore <ian at FreeBSD.org> wrote:
> > 
> > On Wed, 2014-12-24 at 22:26 +0000, Andrew Turner wrote:
> >> On Wed, 24 Dec 2014 17:12:52 +0000 (UTC)
> >> Ian Lepore <ian at FreeBSD.org> wrote:
> >> 
> >>> Author: ian
> >>> Date: Wed Dec 24 17:12:51 2014
> >>> New Revision: 276187
> >>> URL: https://svnweb.freebsd.org/changeset/base/276187
> >>> 
> >>> Log:
> >>>  Eliminate unnecessary references to pte.h internals by using the
> >>> standard pmap_kenter_temporary() to map pages while dumping.
> >>> 
> >>>  Submitted by:	Svatopluk Kraus <onwahe at gmail.com>,
> >>>  		Michal Meloun <meloun at miracle.cz>
> >>> 
> >>> Modified:
> >>>  head/sys/arm/arm/dump_machdep.c
> >>> 
> >>> Modified: head/sys/arm/arm/dump_machdep.c
> >>> ==============================================================================
> >>> --- head/sys/arm/arm/dump_machdep.c	Wed Dec 24 16:11:15
> >>> 2014	(r276186) +++ head/sys/arm/arm/dump_machdep.c	Wed
> >>> Dec 24 17:12:51 2014	(r276187) @@ -160,11 +160,13 @@ static int
> >>> cb_dumpdata(struct md_pa *mdp, int seqnr, void *arg)
> >>> {
> >>> 	struct dumperinfo *di = (struct dumperinfo*)arg;
> >>> -	vm_paddr_t pa;
> >>> +	vm_paddr_t a, pa;
> >>> +	void *va;
> >>> 	uint32_t pgs;
> >>> 	size_t counter, sz, chunk;
> >>> -	int c, error;
> >>> +	int i, c, error;
> >>> 
> >>> +	va = 0;
> >>> 	error = 0;	/* catch case in which chunk size is 0 */
> >>> 	counter = 0;
> >>> 	pgs = mdp->md_size / PAGE_SIZE;
> >>> @@ -194,16 +196,14 @@ cb_dumpdata(struct md_pa *mdp, int seqnr
> >>> 			printf(" %d", pgs * PAGE_SIZE);
> >>> 			counter &= (1<<24) - 1;
> >>> 		}
> >>> -		if (pa == (pa & L1_ADDR_BITS)) {
> >>> -			pmap_kenter_section(0, pa & L1_ADDR_BITS, 0);
> >>> -			cpu_tlb_flushID_SE(0);
> >>> -			cpu_cpwait();
> >>> +		for (i = 0; i < chunk; i++) {
> >>> +			a = pa + i * PAGE_SIZE;
> >>> +			va = pmap_kenter_temporary(trunc_page(a), i);
> >> 
> >> Is this correct? It may map multiple chunks here.
> >>> 		}
> >>> #ifdef SW_WATCHDOG
> >>> 		wdog_kern_pat(WD_LASTVAL);
> >>> #endif
> >>> -		error = dump_write(di,
> >>> -		    (void *)(pa - (pa & L1_ADDR_BITS)),0, dumplo,
> >>> sz);
> >>> +		error = dump_write(di, va, 0, dumplo, sz);
> >> 
> >> Then uses the last virtual address to dump the data here. Wouldn't this
> >> miss the chunks mapped other than the last one?
> > 
> > Yeah, but a quirk of pmap_kenter_temporary() is that its return value is
> > constant, so this wrong-looking code is actually right.
> 
> I'm a bit surprised about this.  This most likely warrants a comment.

In every architecture and every place it's used, or just arm just here?
This appears to be an idiom, or at least something that has been pasted
in identical form in every arch so far.

-- Ian