[PATCH v2 6/6] xen/riscv: fix level_map_mask truncation on load_start

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

check_pgtbl_mode_support() declares level_map_mask as bare `unsigned`, i.e.
a 32-bit type while it derives from a paddr_t which is in both RV32/RV64 a
64-bit type. Storing that value into a 32-bit local silently drops any set
bits above bit 31.

The mask is then used as:

    aligned_load_start = load_start & level_map_mask;

load_start is `unsigned long` (64-bit on riscv64) and if it requires more
than 32 bits to represent, because load_start zero-extend to 64 bits, we
would drop some load_start's bits during the AND.

Widen level_map_mask to `unsigned long`, matching the width of the physical
address.

Fixes: e66003e7be19 ("xen/riscv: introduce setup_initial_pages")
Reported-by: Zheng Zhang <zhangzheng@xxxxxxxxxxx>
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
---
Changes since v1:
- new patch
---
Question:
I would think replacing unsigned long by paddr_t would be better in this
case but for consistency with other variables in the function I just kept
unsigned long.

However, there are many variables in mm.c which are unsigned long while
they are, in reality, physical addresses and could technically be paddr_t.
Using paddr_t would also let us bypass the compiler's decision on what
unsigned long extends to (u32 or u64, depending on the target), and
therefore be more generic. I've seen similar code in Arm using this
convention, and found nothing on the mailing list explaining the original
choice of unsigned long over paddr_t.

Replacing every such field would be a fairly large change, so I'm asking
for your opinion on whether it's worth doing.
---
 xen/arch/riscv/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 53bebbcabf..e7f2491257 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -180,7 +180,7 @@ static bool __init check_pgtbl_mode_support(struct mmu_desc 
*mmu_desc,
     bool is_mode_supported = false;
     unsigned int index;
     unsigned int page_table_level = (mmu_desc->num_levels - 1);
-    unsigned level_map_mask = XEN_PT_LEVEL_MAP_MASK(page_table_level);
+    unsigned long level_map_mask = XEN_PT_LEVEL_MAP_MASK(page_table_level);
 
     unsigned long aligned_load_start = load_start & level_map_mask;
     unsigned long aligned_page_size = XEN_PT_LEVEL_SIZE(page_table_level);

-- 
2.55.0