(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@xxxxxxxxxx>
> ---
>
> 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) {
>
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
