| Message ID | 20230321140357.24094-12-ayan.kumar.halder@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for 32 bit physical address | expand |
Hi Ayan, On 21/03/2023 14:03, Ayan Kumar Halder wrote: > The pabits, t0sz, root_order and sl0 values are the same as those for > ARM_64. To me this read as the line should be common. But you still duplicate it. In any case, you should justify this change with a pointer to the Arm Arm. Not just saying they are common. > > Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> > --- > > Changes from - > > v1 - New patch. > > v2 - 1. Added Ack. > > v3 - 1. Dropped Ack. > 2. Rebased the patch based on the previous change. > > xen/arch/arm/p2m.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index f34b6e6f11..20beecc6e8 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -2272,8 +2272,9 @@ void __init setup_virt_paging(void) > unsigned int sl0; /* Desired SL0, maximum in comment */ > } pa_range_info[] __initconst = { > #ifdef CONFIG_ARM_32 > - [0] = { 40, 24/*24*/, 1, 1 }, > - [1] = { 0 } /* Invalid */ > + [0] = { 32, 32/*32*/, 0, 1 }, As I pointed out in one of the previous version, the root order is different than ... > + [1] = { 40, 24/*24*/, 1, 1 }, ... here. Yet, you still keep P2M_ROOT_ORDER and P2M_ROOT_LEVEL hardcoded. Your previous patch wants to define p2M_root_order and p2m_root_level (lower-case intended). IOW making more code common between arm64 and arm32. Cheers,
On 30/03/2023 22:45, Julien Grall wrote: > CAUTION: This message has originated from an External Source. Please > use proper judgment and caution when opening attachments, clicking > links, or responding to this email. > > > Hi Ayan, Hi Julien, I need some clarifications. > > On 21/03/2023 14:03, Ayan Kumar Halder wrote: >> The pabits, t0sz, root_order and sl0 values are the same as those for >> ARM_64. > > To me this read as the line should be common. But you still duplicate it. > > In any case, you should justify this change with a pointer to the Arm > Arm. Not just saying they are common. Does the following commit message read fine ? Refer ARM DDI 0406C.d ID040418, B3-1345, 0. "Use of concatenated second-level translation tables A stage 2 translation with an input address range of 31-34 bits can start the translation either: * With a first-level lookup, accessing a first-level translation table with 2-16 entries. * With a second-level lookup, accessing a set of concatenated second-level translation tables" Thus, for 32 bit IPA, there will be only one root level translation tables. This is because as the paragraph explains above, 35 bit IPA is the minimum required to support two root level translation tables. The root order for 32 bit IPA will be 0. (Refer xen/arch/arm/p2m.c "#define P2M_ROOT_PAGES (1<<P2M_ROOT_ORDER)") Please clarify if I misunderstood something. - Ayan > >> >> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> >> --- >> >> Changes from - >> >> v1 - New patch. >> >> v2 - 1. Added Ack. >> >> v3 - 1. Dropped Ack. >> 2. Rebased the patch based on the previous change. >> >> xen/arch/arm/p2m.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index f34b6e6f11..20beecc6e8 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -2272,8 +2272,9 @@ void __init setup_virt_paging(void) >> unsigned int sl0; /* Desired SL0, maximum in comment */ >> } pa_range_info[] __initconst = { >> #ifdef CONFIG_ARM_32 >> - [0] = { 40, 24/*24*/, 1, 1 }, >> - [1] = { 0 } /* Invalid */ >> + [0] = { 32, 32/*32*/, 0, 1 }, > > As I pointed out in one of the previous version, the root order is > different than ... > >> + [1] = { 40, 24/*24*/, 1, 1 }, > > ... here. Yet, you still keep P2M_ROOT_ORDER and P2M_ROOT_LEVEL > hardcoded. Your previous patch wants to define p2M_root_order and > p2m_root_level (lower-case intended). IOW making more code common > between arm64 and arm32. > > Cheers, > > -- > Julien Grall >
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index f34b6e6f11..20beecc6e8 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -2272,8 +2272,9 @@ void __init setup_virt_paging(void) unsigned int sl0; /* Desired SL0, maximum in comment */ } pa_range_info[] __initconst = { #ifdef CONFIG_ARM_32 - [0] = { 40, 24/*24*/, 1, 1 }, - [1] = { 0 } /* Invalid */ + [0] = { 32, 32/*32*/, 0, 1 }, + [1] = { 40, 24/*24*/, 1, 1 }, + [2] = { 0 } /* Invalid */ #else /* T0SZ minimum and SL0 maximum from ARM DDI 0487H.a Table D5-6 */ /* PA size, t0sz(min), root-order, sl0(max) */
The pabits, t0sz, root_order and sl0 values are the same as those for ARM_64. Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com> --- Changes from - v1 - New patch. v2 - 1. Added Ack. v3 - 1. Dropped Ack. 2. Rebased the patch based on the previous change. xen/arch/arm/p2m.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)