[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] arm,smmu: match start level of page table walk with P2M



Hi,

On 02/10/2020 10:29, Laurentiu Tudor wrote:


On 10/2/2020 11:18 AM, Julien Grall wrote:
Hi,

On 02/10/2020 00:52, Stefano Stabellini wrote:
On Mon, 28 Sep 2020, laurentiu.tudor@xxxxxxx wrote:
From: Laurentiu Tudor <laurentiu.tudor@xxxxxxx>

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.tudor@xxxxxxx>

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

IMHO, this is going to make the code quite confusing given that only the
SMMU would use this variable for arm32.

The P2M root level also cannot be changed by the SMMU (at least for
now). So I would suggest to introduce a helper (maybe
p2m_get_root_level()) and use it in the SMMU code.

An alternative would be to move the definition of P2M_ROOT_{ORDER,
LEVEL} in p2m.h

Alright, I'll go with this second option if that's ok with you.

I am fine with that.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.