Hi Julien Grall,
It is a pleasure for your review on Xen patch.
And Thanks for your commit suggestion and we completely agree with your commit suggestion.
And very grateful for taking this patch into the mainstream.
Best Regards,
-Vishnu.
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 31 May 2019 23:22
To: Vishnu Pajjuri OS; xen-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Open Source Submission; sstabellini@xxxxxxxxxx; Feng Kan OS
Subject: Re: [PATCH v2] xen/arm: p2m: configure stage-2 page table to support upto 42-bit PA systems
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
|
