Re: [Xen-devel] [PATCH for-4.5 v2 2/2] xen: arm: Enable physical address space compression (PDX) on arm32

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

Don't you implement PDX for both arm32 and arm64? If so, you may need to 
update the commit title.
The patch looks good to me. I have only few comments on it.

On 12/09/14 08:15, Ian Campbell wrote:
> This allows us to support sparse physical address maps which we previously
> could not because the frametable would end up taking up an enourmous fraction
enormous

[..]

> -    memset(&frame_table[0], 0, nr_pages * sizeof(struct page_info));
> -    memset(&frame_table[nr_pages], -1,
> -           frametable_size - (nr_pages * sizeof(struct page_info)));
> +    memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
> +    memset(&frame_table[nr_pdxs], -1,
> +           frametable_size - (nr_pdxs * sizeof(struct page_info)));
> +
> +    frametable_virt_end = FRAMETABLE_VIRT_START + (nr_pdxs * sizeof(struct 
> page_info));
NIT: I don't think it's necessary to have a blank line before the end of 
the block. I would drop it.
[..]

> +/* Sets all bits from the most-significant 1-bit down to the LSB */
> +static u64 __init fill_mask(u64 mask)
> +{
> +    while (mask & (mask + 1))
> +        mask |= mask + 1;
> +    return mask;
> +}
The function is the same on x86. Shall we create an helper?

[..]

>           setup_xenheap_mappings(bank_start>>PAGE_SHIFT, bank_size>>PAGE_SHIFT);
You forgot to drop check if ( bank != bootinfo.mem.nr_banks ) which, I 
think, is not relevant anymore.

> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index 1c3abcf..59b2887 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -126,7 +126,12 @@
>   #define CONFIG_SEPARATE_XENHEAP 1
>
>   #define FRAMETABLE_VIRT_START  _AT(vaddr_t,0x02000000)
> -#define VMAP_VIRT_START  _AT(vaddr_t,0x10000000)
> +#define FRAMETABLE_SIZE        MB(128-32)
I would add a comment about why the frametable size is "MB(128-32)".

Regards,

--
Julien Grall

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