In the subsequent patch, we introduce "CONFIG_ARM_PA_32" to support
32 bit physical addresses. Thus, the code specific to
"Large Page Address Extension" (ie LPAE) should be enclosed within
"ifndef CONFIG_ARM_PA_32".

Refer xen/arch/arm/include/asm/short-desc.h, "short_desc_l1_supersec_t"
unsigned int extbase1:4;    /* Extended base address, PA[35:32] */
unsigned int extbase2:4;    /* Extended base address, PA[39:36] */

Thus, extbase1 and extbase2 are not valid when 32 bit physical addresses
are supported.

Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
---

Changes from -
v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".

 xen/arch/arm/guest_walk.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
index 43d3215304..0feb7b76ec 100644
--- a/xen/arch/arm/guest_walk.c
+++ b/xen/arch/arm/guest_walk.c
@@ -154,8 +154,10 @@ static bool guest_walk_sd(const struct vcpu *v,
             mask = (1ULL << L1DESC_SUPERSECTION_SHIFT) - 1;
             *ipa = gva & mask;
             *ipa |= (paddr_t)(pte.supersec.base) << L1DESC_SUPERSECTION_SHIFT;
+#ifndef CONFIG_ARM_PA_32
             *ipa |= (paddr_t)(pte.supersec.extbase1) << L1DESC_SUPERSECTION_EXT_BASE1_SHIFT;
             *ipa |= (paddr_t)(pte.supersec.extbase2) << L1DESC_SUPERSECTION_EXT_BASE2_SHIFT;
+#endif /* CONFIG_ARM_PA_32 */
         }
 
         /* Set permissions so that the caller can check the flags by herself. */
-- 
2.17.1

On Tue, 17 Jan 2023, Ayan Kumar Halder wrote:
> In the subsequent patch, we introduce "CONFIG_ARM_PA_32" to support
> 32 bit physical addresses. Thus, the code specific to
> "Large Page Address Extension" (ie LPAE) should be enclosed within
> "ifndef CONFIG_ARM_PA_32".
> 
> Refer xen/arch/arm/include/asm/short-desc.h, "short_desc_l1_supersec_t"
> unsigned int extbase1:4;    /* Extended base address, PA[35:32] */
> unsigned int extbase2:4;    /* Extended base address, PA[39:36] */
> 
> Thus, extbase1 and extbase2 are not valid when 32 bit physical addresses
> are supported.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>

This patch should be merged with patch 9: we shouldn't start to use a
new kconfig symbol before it is defined.


> ---
> 
> Changes from -
> v1 - 1. Extracted from "[XEN v1 8/9] xen/arm: Other adaptations required to support 32bit paddr".
> 
>  xen/arch/arm/guest_walk.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c
> index 43d3215304..0feb7b76ec 100644
> --- a/xen/arch/arm/guest_walk.c
> +++ b/xen/arch/arm/guest_walk.c
> @@ -154,8 +154,10 @@ static bool guest_walk_sd(const struct vcpu *v,
>              mask = (1ULL << L1DESC_SUPERSECTION_SHIFT) - 1;
>              *ipa = gva & mask;
>              *ipa |= (paddr_t)(pte.supersec.base) << L1DESC_SUPERSECTION_SHIFT;
> +#ifndef CONFIG_ARM_PA_32
>              *ipa |= (paddr_t)(pte.supersec.extbase1) << L1DESC_SUPERSECTION_EXT_BASE1_SHIFT;
>              *ipa |= (paddr_t)(pte.supersec.extbase2) << L1DESC_SUPERSECTION_EXT_BASE2_SHIFT;
> +#endif /* CONFIG_ARM_PA_32 */
>          }
>  
>          /* Set permissions so that the caller can check the flags by herself. */
> -- 
> 2.17.1
>

Hi Stefano,

On 19/01/2023 23:43, Stefano Stabellini wrote:
> On Tue, 17 Jan 2023, Ayan Kumar Halder wrote:
>> In the subsequent patch, we introduce "CONFIG_ARM_PA_32" to support
>> 32 bit physical addresses. Thus, the code specific to
>> "Large Page Address Extension" (ie LPAE) should be enclosed within
>> "ifndef CONFIG_ARM_PA_32".
>>
>> Refer xen/arch/arm/include/asm/short-desc.h, "short_desc_l1_supersec_t"
>> unsigned int extbase1:4;    /* Extended base address, PA[35:32] */
>> unsigned int extbase2:4;    /* Extended base address, PA[39:36] */
>>
>> Thus, extbase1 and extbase2 are not valid when 32 bit physical addresses
>> are supported.
>>
>> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@amd.com>
> 
> This patch should be merged with patch 9: we shouldn't start to use a
> new kconfig symbol before it is defined.

I have asked the adaptation to be in a separate patch to avoid having a 
melting-pot patch.

So if you want the new Kconfig to be defined first, then we should 
reshuffle it.

BTW, the reshuffle will likely be necessary if you want to check for 
truncation in earlier patch.

Cheers,

-- 
Julien Grall