On 10/2/2020 2:52 AM, Stefano Stabellini wrote:
> On Mon, 28 Sep 2020, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Don't hardcode the lookup start level of the page table walk to 1
>> and instead match the one used in P2M. This should fix scenarios
>> involving SMMU where the start level is different than 1.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>
> Thank you for the patch, I think it is correct, except that smmu.c today
> can be enabled even on arm32 builds, where p2m_root_level would be
> uninitialized.
>
> We need to initialize p2m_root_level at the beginning of
> setup_virt_paging under the #ifdef CONFIG_ARM_32. We can statically
> initialize it to 1 in that case. Or...
>
>
>> ---
>> xen/arch/arm/p2m.c | 2 +-
>> xen/drivers/passthrough/arm/smmu.c | 2 +-
>> xen/include/asm-arm/p2m.h | 1 +
>> 3 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index ce59f2b503..0181b09dc0 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -18,7 +18,6 @@
>>
>> #ifdef CONFIG_ARM_64
>> static unsigned int __read_mostly p2m_root_order;
>> -static unsigned int __read_mostly p2m_root_level;
>> #define P2M_ROOT_ORDER p2m_root_order
>> #define P2M_ROOT_LEVEL p2m_root_level
>> static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
>> @@ -39,6 +38,7 @@ static unsigned int __read_mostly max_vmid =
>> MAX_VMID_8_BIT;
>> * restricted by external entity (e.g. IOMMU).
>> */
>> unsigned int __read_mostly p2m_ipa_bits = 64;
>> +unsigned int __read_mostly p2m_root_level;
>
> ... we could p2m_root_level = 1; here
>
This looks straight forward and in line with what we do with
p2m_ipa_bits, I'll send a v2 right away.
Thanks for the review.
---
Best Regards, Laurentiu
>> /* Helpers to lookup the properties of each level */
>> static const paddr_t level_masks[] =
>> diff --git a/xen/drivers/passthrough/arm/smmu.c
>> b/xen/drivers/passthrough/arm/smmu.c
>> index 94662a8501..85709a136f 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>> @@ -1152,7 +1152,7 @@ static void arm_smmu_init_context_bank(struct
>> arm_smmu_domain *smmu_domain)
>> (TTBCR_RGN_WBWA << TTBCR_IRGN0_SHIFT);
>>
>> if (!stage1)
>> - reg |= (TTBCR_SL0_LVL_1 << TTBCR_SL0_SHIFT);
>> + reg |= (2 - p2m_root_level) << TTBCR_SL0_SHIFT;
>>
>> writel_relaxed(reg, cb_base + ARM_SMMU_CB_TTBCR);
>>
>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>> index 5fdb6e8183..97b5eada2b 100644
>> --- a/xen/include/asm-arm/p2m.h
>> +++ b/xen/include/asm-arm/p2m.h
>> @@ -12,6 +12,7 @@
>>
>> /* Holds the bit size of IPAs in p2m tables. */
>> extern unsigned int p2m_ipa_bits;
>> +extern unsigned int p2m_root_level;
>>
>> struct domain;
>>
>> --
>> 2.17.1
>>
