From mboxrd@z Thu Jan  1 00:00:00 1970
From: shijie.huang@arm.com (Huang Shijie)
Date: Thu, 2 Jun 2016 09:48:13 +0800
Subject: [PATCH 1/2] arm64: mm: dump: make page table dumping reusable
In-Reply-To: <20160601095818.GA9280@leverpostej>
References: <1464702542-24394-1-git-send-email-mark.rutland@arm.com>
 <1464702542-24394-2-git-send-email-mark.rutland@arm.com>
 <20160601030133.GB14411@sha-win-210.asiapac.arm.com>
 <20160601095818.GA9280@leverpostej>
Message-ID: <20160602014811.GA8902@sha-win-210.asiapac.arm.com>
To: linux-arm-kernel@lists.infradead.org
List-Id: linux-arm-kernel.lists.infradead.org

On Wed, Jun 01, 2016 at 10:58:18AM +0100, Mark Rutland wrote:
> On Wed, Jun 01, 2016 at 11:01:35AM +0800, Huang Shijie wrote:
> > On Tue, May 31, 2016 at 02:49:01PM +0100, Mark Rutland wrote:
> > > +struct ptdump_info {
> > > +	struct mm_struct		*mm;
> > > +	const struct addr_marker	*markers;
> > > +	unsigned long			base_addr;
> > > +	unsigned long			max_addr;
> > > +};
> > > +
> > > +int ptdump_register(struct ptdump_info *info, const char *name);
> > Since we export this to other page tables,  I guess the @base_addr in
> > the ptdump_info{} may not equal to the VA_START.
> 
> Yes, that is the intent.
> 
> The only requirement is that this is only VA_START or 0, as these are the only
> addresses aligned to VA_BITS which actually correspond to regions page tables
> can cover.
> 
> > But the current dump.c does _NOT_ use the @start address been
> > passed in, it use the 0 as the start address for the walk_pgd/walk_pud/walk_pmd/walk_pte.
> 
> Yes, this is deliberate. The trick is that start must be aligned to
> VA_BITS, and the page table accessors do the right thing, masking out
yes, this is the trick...

> bits which do not matter for their respective indices, e.g.
> 
> #define pgd_index(addr)			(((addr) >> PGDIR_SHIFT) & (PTRS_PER_PGD - 1))
> #define pgd_offset_raw(pgd, addr)	((pgd) + pgd_index(addr))
> #define pgd_offset(mm, addr)		(pgd_offset_raw((mm)->pgd, (addr)))
> 
> This allows us to either provide a virtual address to the accessors
> (which can be in the low or high half), or an offset relative to the
> start of each pgd, pud, pmd, or pte.
> 
> So for walk_pgd:
> 
> static void walk_pgd(struct pg_state *st, struct mm_struct *mm, unsigned long start)
> {
> 	pgd_t *pgd = pgd_offset(mm, 0UL);
> 	unsigned i;
> 	unsigned long addr;
> 
> 	for (i = 0; i < PTRS_PER_PGD; i++, pgd++) {
> 		addr = start + i * PGDIR_SIZE;
> 		if (pgd_none(*pgd)) {
> 			note_page(st, addr, 1, pgd_val(*pgd));
> 		} else {
> 			BUG_ON(pgd_bad(*pgd));
> 			walk_pud(st, pgd, addr);
> 		}
> 	}
> }
> 
> Here, the 0UL we pass to pgd_offset is the offset from the start of the
> pgd, not the absolute virtual address. 
> 
> In the loop, we generate the virtual address each pgd_t corresponds to,
> and we pass this down to note_page or walk_pud as appropriate. We do
> likewise in walk_pud, walk_pmd, and walk_pte.
> 
> So when we reach note_page, we should always have the right virtual
> address in the addr parameter.
> 
> > It is wrong in logic, since the start address is VA_START, the code gets
> > the right result coincidentally.
> 
> As above, I think that the logic is correct.
> 
> Hopefully the explanation above allays your fears?
Thanks for the explanation.

Huang Shijie