Re: [Xen-devel] [PATCH 02/38] arm: handy function to print a walk of the hypervisor page tables

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 15:39 +0000 on 01 Jun (1338565171), Ian Campbell wrote:
> +void dump_pt_walk(uint32_t addr)
> +{
> +    paddr_t second_ma, third_ma;
> +    lpae_t *first = NULL, *second = NULL, *third = NULL;
> +    uint64_t httbr = READ_CP64(HTTBR);
> +
> +    printk("Walking Hypervisor VA %#"PRIx32" via HTTBR %#"PRIx64"\n",
> +           addr, httbr);
> +
> +    BUG_ON( (lpae_t *)(unsigned long)(httbr - xen_phys_offset) != 
> xen_pgtable );
> +    first = xen_pgtable;
> +    printk("1ST[%#03"PRIx32"] = %p[%#03"PRIx32"] = %#llx = %#016llx\n",
> +           first_table_offset(addr),
> +           first, first_table_offset(addr),
> +           virt_to_maddr(&first[first_table_offset(addr)]),
> +           first[first_table_offset(addr)].bits);
> +
> +    if ( !first[first_table_offset(addr)].pt.valid ||
> +         !first[first_table_offset(addr)].pt.table )
> +        goto done;

This could probably be a for loop rather than open-coding three
almost-identical printks.

> +
> +    second_ma = (paddr_t)first[first_table_offset(addr)].pt.base << 
> PAGE_SHIFT;
> +    second = (lpae_t *)(unsigned long)(second_ma - xen_phys_offset);
> +    printk("2ND[%#03"PRIx32"] = %p[%#03"PRIx32"] = %#llx = %#016llx\n",
> +           second_table_offset(addr),
> +           second, second_table_offset(addr),
> +           virt_to_maddr(&second[second_table_offset(addr)]),
> +           second[second_table_offset(addr)].bits);
> +    if ( !second[second_table_offset(addr)].pt.valid ||
> +         !second[second_table_offset(addr)].pt.table )
> +        goto done;
> +
> +    third_ma = (paddr_t)second[second_table_offset(addr)].pt.base << 
> PAGE_SHIFT;
> +    third = (lpae_t *)(unsigned long)(third_ma - xen_phys_offset);
> +    printk("3RD[%#03"PRIx32"] = %p[%#03"PRIx32"] = %#llx = %#016llx\n",
> +           third_table_offset(addr),
> +           third, third_table_offset(addr),
> +           virt_to_maddr(&third[third_table_offset(addr)]),
> +           third[third_table_offset(addr)].bits);
> +    if ( !third[third_table_offset(addr)].pt.valid ||
> +         !third[third_table_offset(addr)].pt.table )
> +        goto done;
> +
> +done:
> +    return;

Maybe use return above instead of goto?

> +}
> +
>  /* Map a 4k page in a fixmap entry */
>  void set_fixmap(unsigned map, unsigned long mfn, unsigned attributes)
>  {
> @@ -173,8 +222,8 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset)
>      flush_xen_data_tlb_va(dest_va);
>  
>      /* Calculate virt-to-phys offset for the new location */
> -    phys_offset = xen_paddr - (unsigned long) _start;
> -
> +    xen_phys_offset = phys_offset = xen_paddr - (unsigned long) _start;
> +    

Just make phys_offset file-scope static; no need to add a new variable
for this.

>      /* Copy */
>      memcpy((void *) dest_va, _start, _end - _start);
>  
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index b6df64e..22c56b5 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -312,6 +312,8 @@ static inline uint64_t gva_to_ipa(uint32_t va)
>  /* Bits in the PAR returned by va_to_par */
>  #define PAR_FAULT 0x1
>  
> +extern void dump_pt_walk(uint32_t addr);
> +

Maybe a comment?

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel