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>]

On 6/3/19 8:13 AM, Vishnu Pajjuri OS wrote:
> Hi Julien Grall,
Hi,

Sorry for the late reply.


>      It is a pleasure for your review on Xen patch.
>
> And Thanks for your commit suggestion and we completely agree with your 
> commit suggestion.
Thank you for the confirmation, I have now merged the patch in staging.

Cheers,

> 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
--
Julien Grall

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