Re: [Xen-devel] [PATCH 04/16] arm: handle xenheap which isn't at the start of RAM.

[Tim Deegan <tim@xxxxxxx>]

At 13:30 +0000 on 03 Sep (1346679044), Ian Campbell wrote:
> Also refactor page_to_virt somewhat in an attempt to make it clearer
> what is happening.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/include/asm-arm/mm.h |   24 +++++++++++++++++++-----
>  1 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index b37bd35..6498322 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -214,17 +214,31 @@ static inline struct page_info *virt_to_page(const void 
> *v)
>      ASSERT(va >= XENHEAP_VIRT_START);
>      ASSERT(va < xenheap_virt_end);
>  
> -    return frame_table + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT);
> +    return frame_table
> +        + ((va - XENHEAP_VIRT_START) >> PAGE_SHIFT)
> +        + xenheap_mfn_start
> +        - frametable_base_mfn;
>  }
>  
>  static inline void *page_to_virt(const struct page_info *pg)
>  {
> +    unsigned long va;
> +    const unsigned long offset =
> +        (xenheap_mfn_start-frametable_base_mfn)*sizeof(*pg);
> +
> +    /*
> +     * Dividing by this on both top and bottom factors out the largest
> +     * common factor of 2 which helps the compiler to use smaller shifts.
> +     */
> +    const unsigned long lcd = (sizeof(*pg) & -sizeof(*pg));
> +
>      ASSERT((unsigned long)pg - FRAMETABLE_VIRT_START < frametable_virt_end);
> -    return (void *)(XENHEAP_VIRT_START +
> -                    ((unsigned long)pg - FRAMETABLE_VIRT_START) /
> -                    (sizeof(*pg) / (sizeof(*pg) & -sizeof(*pg))) *
> -                    (PAGE_SIZE / (sizeof(*pg) & -sizeof(*pg))));
>  
> +    va = (unsigned long)pg;
> +    va = XENHEAP_VIRT_START +
> +        ((va - FRAMETABLE_VIRT_START - offset) / (sizeof(*pg) / lcd)) *
> +        (PAGE_SIZE / lcd);
> +    return (void *)va;

This function is getting a bit too confusing.  I think we should just
use page_to_mfn and mfn_to_virt instead: the magic lcd trick eliminates
a shift but having to construct "offset" to match it adds one.

Tim.

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