[v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

[PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

Posted by Xenia Ragiadakou 1 year, 6 months ago

Currently, the fields 'root_order' and 'sl0' of the pa_range_info for
the 52-bit pa range have the values 3 and 3, respectively.
This configuration does not match any of the valid root table configurations
for 4KB granule and t0sz 12, described in ARM DDI 0487I.a D8.2.7.

More specifically, according to ARM DDI 0487I.a D8.2.7, in order to support
the 52-bit pa size with 4KB granule, the p2m root table needs to be configured
either as a single table at level -1 or as 16 concatenated tables at level 0.
Since, currently there is not support for level -1, set the 'root_order' and
'sl0' fields of the 52-bit pa_range_info according to the second approach.

Note that the values of those fields are not used so far. This patch updates
their values only for the sake of correctness.

Fixes: 407b13a71e32 ("xen/arm: p2m don't fall over on FEAT_LPA enabled hw")
Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---

Changes in v2:
- add Fixes tag
- provide a reference to the Arm Arm (paragraph + version)
- change wording in the commit log to not make assumptions on value
  interpretations that may lead to confusion
- state clearly that these values are not used so far

 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 f17500ddf3..c824d62806 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -2251,7 +2251,7 @@ void __init setup_virt_paging(void)
         [3] = { 42,      22/*22*/,  3,          1 },
         [4] = { 44,      20/*20*/,  0,          2 },
         [5] = { 48,      16/*16*/,  0,          2 },
-        [6] = { 52,      12/*12*/,  3,          3 },
+        [6] = { 52,      12/*12*/,  4,          2 },
         [7] = { 0 }  /* Invalid */
     };
 
-- 
2.34.1

Re: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

Posted by Julien Grall 1 year, 6 months ago

(+ Henry)

Hi Xenia,

On 19/10/2022 15:49, Xenia Ragiadakou wrote:
> Currently, the fields 'root_order' and 'sl0' of the pa_range_info for
> the 52-bit pa range have the values 3 and 3, respectively.
> This configuration does not match any of the valid root table configurations
> for 4KB granule and t0sz 12, described in ARM DDI 0487I.a D8.2.7.
> 
> More specifically, according to ARM DDI 0487I.a D8.2.7, in order to support
> the 52-bit pa size with 4KB granule, the p2m root table needs to be configured
> either as a single table at level -1 or as 16 concatenated tables at level 0.
> Since, currently there is not support for level -1, set the 'root_order' an

Typo: s/not/no/ (I can fix it while committing)

> 'sl0' fields of the 52-bit pa_range_info according to the second approach.
> 
> Note that the values of those fields are not used so far. This patch updates
> their values only for the sake of correctness.
> 
> Fixes: 407b13a71e32 ("xen/arm: p2m don't fall over on FEAT_LPA enabled hw")
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Regarding 4.17, I am a bit split whether this should be included. On one 
hand, it would be good to have the value correct (not that I expect 
anymore to try using 52-bit on 4.17...). On the other hand, this is not 
used so there is no bug (this could also be an argument to add it 
because it is nearly risk free).

If we don't include it, I will definitely add in my list of potential 
backports.

Henry, any thoughts?

Cheers,

-- 
Julien Grall

[4.17] RE: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

Posted by Henry Wang 1 year, 6 months ago

Hi Julien and Xenia,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> <Volodymyr_Babchuk@epam.com>; Henry Wang <Henry.Wang@arm.com>
> Subject: Re: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range
> 
> (+ Henry)
> 
> Hi Xenia,
> 
> On 19/10/2022 15:49, Xenia Ragiadakou wrote:
> > Currently, the fields 'root_order' and 'sl0' of the pa_range_info for
> > the 52-bit pa range have the values 3 and 3, respectively.
> > This configuration does not match any of the valid root table configurations
> > for 4KB granule and t0sz 12, described in ARM DDI 0487I.a D8.2.7.
> >
> > More specifically, according to ARM DDI 0487I.a D8.2.7, in order to support
> > the 52-bit pa size with 4KB granule, the p2m root table needs to be
> configured
> > either as a single table at level -1 or as 16 concatenated tables at level 0.
> > Since, currently there is not support for level -1, set the 'root_order' an
> 
> Typo: s/not/no/ (I can fix it while committing)
> 
> > 'sl0' fields of the 52-bit pa_range_info according to the second approach.
> >
> > Note that the values of those fields are not used so far. This patch updates
> > their values only for the sake of correctness.
> >
> > Fixes: 407b13a71e32 ("xen/arm: p2m don't fall over on FEAT_LPA enabled
> hw")
> > Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> Regarding 4.17, I am a bit split whether this should be included. On one
> hand, it would be good to have the value correct (not that I expect
> anymore to try using 52-bit on 4.17...). On the other hand, this is not
> used so there is no bug (this could also be an argument to add it
> because it is nearly risk free).
> 
> If we don't include it, I will definitely add in my list of potential
> backports.
> 
> Henry, any thoughts?

I am actually monitoring this patch for the same question that if
we need this patch for 4.17.

I see no reason to exclude this patch since (1) we want to make sure
our code is correct (2) I am pretty sure we are not using 52 bit PA so
as indicated by commit message this patch is just for correctness and
no potential harm to include this patch in the release (probably even
backporting this patch till the 52 bit PA was introduced?).

So if you wouldn't mind committing this patch, you can of course have
my:

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall

Re: [4.17] RE: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

Posted by Julien Grall 1 year, 6 months ago


On 21/10/2022 02:42, Henry Wang wrote:
> Hi Julien and Xenia,

Hi Henry,


>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> <Volodymyr_Babchuk@epam.com>; Henry Wang <Henry.Wang@arm.com>
>> Subject: Re: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range
>>
>> (+ Henry)
>>
>> Hi Xenia,
>>
>> On 19/10/2022 15:49, Xenia Ragiadakou wrote:
>>> Currently, the fields 'root_order' and 'sl0' of the pa_range_info for
>>> the 52-bit pa range have the values 3 and 3, respectively.
>>> This configuration does not match any of the valid root table configurations
>>> for 4KB granule and t0sz 12, described in ARM DDI 0487I.a D8.2.7.
>>>
>>> More specifically, according to ARM DDI 0487I.a D8.2.7, in order to support
>>> the 52-bit pa size with 4KB granule, the p2m root table needs to be
>> configured
>>> either as a single table at level -1 or as 16 concatenated tables at level 0.
>>> Since, currently there is not support for level -1, set the 'root_order' an
>>
>> Typo: s/not/no/ (I can fix it while committing)
>>
>>> 'sl0' fields of the 52-bit pa_range_info according to the second approach.
>>>
>>> Note that the values of those fields are not used so far. This patch updates
>>> their values only for the sake of correctness.
>>>
>>> Fixes: 407b13a71e32 ("xen/arm: p2m don't fall over on FEAT_LPA enabled
>> hw")
>>> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
>>
>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>>
>> Regarding 4.17, I am a bit split whether this should be included. On one
>> hand, it would be good to have the value correct (not that I expect
>> anymore to try using 52-bit on 4.17...). On the other hand, this is not
>> used so there is no bug (this could also be an argument to add it
>> because it is nearly risk free).
>>
>> If we don't include it, I will definitely add in my list of potential
>> backports.
>>
>> Henry, any thoughts?
> 
> I am actually monitoring this patch for the same question that if
> we need this patch for 4.17.
> 
> I see no reason to exclude this patch since (1) we want to make sure
> our code is correct (2) I am pretty sure we are not using 52 bit PA so
> as indicated by commit message this patch is just for correctness and
> no potential harm to include this patch in the release (probably even
> backporting this patch till the 52 bit PA was introduced?).
> 
> So if you wouldn't mind committing this patch, you can of course have
> my:

Thanks for the feedback. I am happy to commit it. So...

> 
> Release-acked-by: Henry Wang <Henry.Wang@arm.com>

I have now done it with your tag added.

Cheers,

-- 
Julien Grall

Re: [PATCH v2] xen/arm: p2m: fix pa_range_info for 52-bit pa range

Posted by Michal Orzel 1 year, 6 months ago

Hi Xenia,

On 19/10/2022 16:49, Xenia Ragiadakou wrote:
> 
> 
> Currently, the fields 'root_order' and 'sl0' of the pa_range_info for
> the 52-bit pa range have the values 3 and 3, respectively.
> This configuration does not match any of the valid root table configurations
> for 4KB granule and t0sz 12, described in ARM DDI 0487I.a D8.2.7.
> 
> More specifically, according to ARM DDI 0487I.a D8.2.7, in order to support
> the 52-bit pa size with 4KB granule, the p2m root table needs to be configured
> either as a single table at level -1 or as 16 concatenated tables at level 0.
> Since, currently there is not support for level -1, set the 'root_order' and
> 'sl0' fields of the 52-bit pa_range_info according to the second approach.
> 
> Note that the values of those fields are not used so far. This patch updates
> their values only for the sake of correctness.
> 
> Fixes: 407b13a71e32 ("xen/arm: p2m don't fall over on FEAT_LPA enabled hw")
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal