Re: [Xen-devel] [PATCH v2] xen/arm: p2m: configure stage-2 page table to support upto 42-bit PA systems

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

Title: s/upto/ I think?

Also how about: "Properly configure stage-2 for 42-bit PA system".

On 30/05/2019 08:59, Vishnu Pajjuri OS wrote:
> XEN configures stage-2 page table to expose 40 bits of IPA
> (Intermediate Physical Address) bits for systems with 42 bits of PA
I think you want to drop the first "bits".

> (Physical Address). This setting prevents the 42-bit PA systems from booting
> DOM0's kernel since access above 40 bits results in a fault.

This sentence is difficult to read if you don't read the next one first.

> This patch adds support for 42-bit system
> which has a full 42-bit address range.
The line-wrap looks strange here.

> The fix will allocate 8 pages for stage 2 mapping for both DOM0 and guests.
s/mapping/root page-tables/

> It is a bit wasteful but not an issue since most of these systems should have
> sufficiently large memory capacity.
Here a suggestion for the commit message:

"At the moment, on platform supporting 42-bit PA, Xen will only expose 40-bit 
worth of IPA to all domains.
The limitation was to prevent allocating too much memory for the root page 
tables as those platforms only support 3-levels page-tables. At the time, this 
was deemed acceptable because none of the platforms had address wired above 40-bits.
However, newer platforms take advantage of the full address space. This will 
result to break Dom0 boot as it can't access anything above 40-bit.
The only way to support 42-bit IPA is to allocate 8 pages for the root 
page-tables. This is a bit a waste of memory as Xen does not offer per-guest 
stage-2 configuration. But it is considered acceptable as current platforms 
support 42-bit PA have a lot of memory.
In the future, we may want to consider per-guest stage-2 configuration to reduce 
the waste.
"

If you are happy with the commit message, I will update use it on commit.

> Signed-off-by: Feng Kan <fengkan@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Vishnu <vishnu@xxxxxxxxxxxxxxxxxxxxxx>
> ---
It is common to briefly describe after --- the changes you made in the new 
version.

>   xen/arch/arm/p2m.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 92c2413..7712991 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1995,7 +1995,7 @@ void __init setup_virt_paging(void)
>           [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 */
> --
> 1.8.3.1
--
Julien Grall

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