Re: [Xen-devel] [PATCH] xen/arm: p2m: configure pa_range_info table to support 42 bit PA systems.

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

Thank you for posting the patch.

Title: No need for full stop in the commit title.

On 15/03/2019 08:34, Vishnu Pajjuri OS wrote:
> Current pa_range_info table's configuration prevents 42 bit PA systems
> from booting DOM0. This patch modifies t0sz=22 and root_order=3
> to expose 42-bit IPA (Intermediate Physical Address).
>
> It is required for hardware which is having peripherals above 40 bits.
The commit message is a bit confusing, you first say Xen does not boot on 
42-bits PA systems but towards the end of the commit you restrict to only 
platform that have address wired above 40-bits.
The limitation was introduced because existing platform back then had all 
address wired below 40-bits. So we can limit to 40-bits and allocate 2 pages 
rather than 8 pages for each page-tables.
While I am perfectly fine to see this for Dom0, I am still unsure this is the 
right things for guest. Do you currently have any use case for 42-bits (4TB) guest?
I can't remember which other platforms support 42-bits PA. I think at that time 
it was X-Gene. As long as no current embedded platform we support use 42-bit PA, 
this change may be ok. Stefano do you recall what was the platform?
In any case, the new behavior (and consequence) needs to be clearly explained in 
the commit message.
> Signed-off-by: Vishnu <vishnu@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Feng Kan <fengkan@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>   xen/arch/arm/p2m.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index c38bd7e16e..c8e7598d81 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1992,10 +1992,13 @@ void __init setup_virt_paging(void)
>       } pa_range_info[] = {
>           /* T0SZ minimum and SL0 maximum from ARM DDI 0487A.b Table D4-5 */
>           /*      PA size, t0sz(min), root-order, sl0(max) */
> +       /* TODO: 42 bit PA systems allocate 8 pages by default
> +        * for both DOM0 and guests.
> +        */
Xen uses soft-tab and not hard-tab. Also, multi-lines comment in Xen looks like:

/*
 * Foo
 */

Lastly, what you have after the TODO is more a fact than a TODO. If we decide to 
keep the TODO, then it should explain what we need to do.
>           [0] = { 32,      32/*32*/,  0,          1 },
>           [1] = { 36,      28/*28*/,  0,          1 },
>           [2] = { 40,      24/*24*/,  1,          1 },
> -        [3] = { 42,      24/*22*/,  1,          1 },
> +        [3] = { 42,      22/*22*/,  3,          1 },
>           [4] = { 44,      20/*20*/,  0,          2 },
>           [5] = { 48,      16/*16*/,  0,          2 },
>           [6] = { 0 }, /* Invalid */
> --
> 2.19.1
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel