[Crash-utility] [PATCH v2 2/3] [ppc] Support for platform based Virtual address translation

Toshikazu Nakayama nakayama.ts at ncos.nec.co.jp
Fri Feb 3 05:16:23 UTC 2012


(2012/02/02 17:45), Suzuki K. Poulose wrote:
> This patch adds infrastructure for defining Virtual address translation bits
> for each platform and use the specific definition for the platform depending on
> the 'powerpc_base_platform' variable. If a matching platform is not found,
> fallbacks to the default definition.
> 
> Each platform can define the PGDIRSHIT, PTRS_PER_PTE and the size of a
> physical address.
> 
> I have not modified the size of the machdep->pgd allocation. Instead,
> we always read a PAGESIZE() which contains the entry.

This way is very effectual in several angles, I like it
in addtion to your first integration work.

> Signed-off-by: Suzuki K. Poulose<suzuki at in.ibm.com>
> ---
> 
>   defs.h |   14 +++++++++--
>   ppc.c  |   80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
>   2 files changed, 83 insertions(+), 11 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 82d51e5..aba13ea 100755
> --- a/defs.h
> +++ b/defs.h
> @@ -2603,9 +2603,17 @@ struct load_module {
>   #define VTOP(X)            ((unsigned long)(X)-(machdep->kvbase))
>   #define IS_VMALLOC_ADDR(X) (vt->vmalloc_start&&  (ulong)(X)>= vt->vmalloc_start)
> 
> -#define PGDIR_SHIFT   (22)
> -#define PTRS_PER_PTE  (1024)
> -#define PTRS_PER_PGD  (1024)
> +/* Default values for PPC */
> +#define DEFAULT_PGDIR_SHIFT	(22)
> +#define DEFAULT_PTRS_PER_PTE	(1024)
> +#define DEFAULT_PTRS_PER_PGD	(1024)
> +#define DEFAULT_PTE_SIZE	sizeof(ulong)
> +
> +
> +#define PGDIR_SHIFT		(base_platform->pgdir_shift)
> +#define PTRS_PER_PTE		(base_platform->ptrs_per_pte)
> +#define PTRS_PER_PGD		(base_platform->ptrs_per_pgd)
> +#define PTE_SIZE		(base_platform->pte_size)
> 
>   #define _PAGE_PRESENT   0x001   /* software: pte contains a translation */
>   #define _PAGE_USER      0x002   /* matches one of the PP bits */
> diff --git a/ppc.c b/ppc.c
> index cfa3f5f..600df34 100755
> --- a/ppc.c
> +++ b/ppc.c
> @@ -67,10 +67,59 @@ static void ppc_display_machine_stats(void);
>   static void ppc_dump_line_number(ulong);
>   static struct line_number_hook ppc_line_number_hooks[];
> 
> +struct platform {
> +	char 	*name;

const char *name is better?

> +	int	pgdir_shift;
> +	int	ptrs_per_pgd;
> +	int	ptrs_per_pte;
> +	int	pte_size;
> +} ppc_boards[] = {
> +	{
> +		/* Always keep the default as the first entry */
> +		.name = "default",
> +		.pgdir_shift = DEFAULT_PGDIR_SHIFT,
> +		.ptrs_per_pgd = DEFAULT_PTRS_PER_PGD,
> +		.ptrs_per_pte = DEFAULT_PTRS_PER_PTE,
> +		.pte_size = DEFAULT_PTE_SIZE,
> +	},
> +	{
> +		/* Keep this at the end */
> +		.name = NULL,
> +	}
> +};
> +
> +struct platform *base_platform;
> +static struct platform *ppc_get_base_platform(void);
> +
>   /* Defined in diskdump.c */
>   extern void process_elf32_notes(void *, ulong);
> 
>   /*
> + * Find the platform of the crashing system and set the
> + * base_platform accordingly.
> + */
> +struct platform*
> +ppc_get_base_platform(void)
> +{
> +	char platform[32];
> +	struct platform *tmp =&ppc_boards[1]; /* start at 1 */
> +	ulong ppc_platform;
> +
> +	if(!try_get_symbol_data("powerpc_base_platform", sizeof(ulong),&ppc_platform))
> +		return&ppc_boards[0];
> +
> +	if (read_string(ppc_platform, platform, 31) == 0)
> +		return&ppc_boards[0];
> +
> +	for (; tmp->name!= NULL; tmp++)
> +		if (!strcmp(tmp->name, platform))
> +			return tmp;

Is there any reasons to distinct "ppc440gp"?

Since I see that there are no "ppc440" prefix platform other than PPC40
in latest cpu_specs[], and all of ppc440gp's values in ppc_boards[] are the
same as ppc440.
Can it be summarized by using STRNEQ("ppc440") or strncmp("ppc440")?

And I often see similar comparisons as SRTEQ() in crash code,
also "unsigned long long" is "ulonglong".
Although I wonder whether they are unspoken rules or not,
I think it is better to use familiarity STREQ() and ulonglong.

Thanks,
Toshi

> +
> +	/* Use default definitions */
> +	return&ppc_boards[0];
> +}
> +
> +/*
>    *  Do all necessary machine-specific setup here.  This is called twice,
>    *  before and after GDB has been initialized.
>    */
> @@ -104,7 +153,6 @@ ppc_init(int when)
>                   machdep->last_pmd_read = 0;
>                   machdep->last_ptbl_read = 0;
>   		machdep->verify_paddr = generic_verify_paddr;
> -		machdep->ptrs_per_pgd = PTRS_PER_PGD;
>   		break;
> 
>   	case PRE_GDB:
> @@ -130,6 +178,11 @@ ppc_init(int when)
>   		machdep->line_number_hooks = ppc_line_number_hooks;
>   		machdep->value_to_symbol = generic_machdep_value_to_symbol;
>                   machdep->init_kernel_pgd = NULL;
> +		/* Find the platform where we crashed */
> +		base_platform = ppc_get_base_platform();
> +
> +		machdep->ptrs_per_pgd = PTRS_PER_PGD;
> +
>   		break;
> 
>   	case POST_GDB:
> @@ -269,14 +322,21 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr, physaddr_t *paddr, int verbose)
>   	ulong *page_middle;
>   	ulong *page_table;
>   	ulong pgd_pte;
> -	ulong pte;
> +	unsigned long long pte;
> 
> -	if (verbose)
> +	if (verbose) {
> +		fprintf(fp, "Using %s board definitions:\n", base_platform->name);
>   		fprintf(fp, "PAGE DIRECTORY: %lx\n", (ulong)pgd);
> +	}
> 
>   	page_dir = pgd + (vaddr>>  PGDIR_SHIFT);
> 
> -        FILL_PGD(PAGEBASE(pgd), KVADDR, PAGESIZE());
> +	/*
> + 	 * Size of a pgd could be more than a PAGE.
> + 	 * So use PAGEBASE(page_dir), instead of
> + 	 * PAGEBASE(pgd) for FILL_PGD()
> + 	 */
> +        FILL_PGD(PAGEBASE(page_dir), KVADDR, PAGESIZE());
>           pgd_pte = ULONG(machdep->pgd + PAGEOFFSET(page_dir));
> 
>   	if (verbose)
> @@ -288,10 +348,10 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr, physaddr_t *paddr, int verbose)
>   	page_middle = (ulong *)pgd_pte;
> 
>   	if (machdep->flags&  CPU_BOOKE)
> -		page_table = page_middle + (BTOP(vaddr)&  (PTRS_PER_PTE - 1));
> +		page_table = (ulong *)((ulong)page_middle + ((ulong)BTOP(vaddr)&  (PTRS_PER_PTE - 1)) * PTE_SIZE);
>   	else {
>   		page_table = (ulong *)((pgd_pte&  (ulong)machdep->pagemask) + machdep->kvbase);
> -		page_table += ((ulong)BTOP(vaddr)&  (PTRS_PER_PTE-1));
> +		page_table = (ulong *)((ulong)page_table + ((ulong)BTOP(vaddr)&  (PTRS_PER_PTE-1)) * PTE_SIZE);
>   	}
> 
>   	if (verbose)
> @@ -299,10 +359,14 @@ ppc_pgd_vtop(ulong *pgd, ulong vaddr, physaddr_t *paddr, int verbose)
>   			(ulong)page_table);
> 
>           FILL_PTBL(PAGEBASE(page_table), KVADDR, PAGESIZE());
> -        pte = ULONG(machdep->ptbl + PAGEOFFSET(page_table));
> +	if (PTE_SIZE == sizeof(unsigned long long))
> +		pte = ULONGLONG(machdep->ptbl + PAGEOFFSET(page_table));
> +
> +	else	/* Defaults to ulong */
> +	        pte = ULONG(machdep->ptbl + PAGEOFFSET(page_table));
> 
>   	if (verbose)
> -		fprintf(fp, "  PTE: %lx =>  %lx\n", (ulong)page_table, pte);
> +		fprintf(fp, "  PTE: %lx =>  %llx\n", (ulong)page_table, pte);
> 
>   	if (!(pte&  _PAGE_PRESENT)) {
>   		if (pte&&  verbose) {
> 
> 




More information about the Crash-utility mailing list