vmap_to_mfn() uses virt_to_maddr(), which is designed to work with VA from
either the direct map region or Xen's linkage region (XEN_VIRT_START).
An assertion will occur if it is used with other regions, in particular for
the VMAP region.

Since RISC-V lacks a hardware feature to request the MMU to translate a VA to
a PA (as Arm does, for example), software page table walking (pt_walk()) is
used for the VMAP region to obtain the PA.

Fixes: 7db8d2bd9b ("xen/riscv: add minimal stuff to mm.h to build full Xen")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/include/asm/mm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index d46018c132..528e1e257f 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -25,7 +25,7 @@ paddr_t pt_walk(vaddr_t va);
 #define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
 #define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
 #define maddr_to_mfn(ma)    _mfn(paddr_to_pfn(ma))
-#define vmap_to_mfn(va)     maddr_to_mfn(virt_to_maddr((vaddr_t)(va)))
+#define vmap_to_mfn(va)     maddr_to_mfn(pt_walk((vaddr_t)(va)))
 #define vmap_to_page(va)    mfn_to_page(vmap_to_mfn(va))
 
 static inline void *maddr_to_virt(paddr_t ma)
-- 
2.48.1

On 20.01.2025 17:54, Oleksii Kurochko wrote:
> vmap_to_mfn() uses virt_to_maddr(), which is designed to work with VA from
> either the direct map region or Xen's linkage region (XEN_VIRT_START).
> An assertion will occur if it is used with other regions, in particular for
> the VMAP region.
> 
> Since RISC-V lacks a hardware feature to request the MMU to translate a VA to
> a PA (as Arm does, for example), software page table walking (pt_walk()) is
> used for the VMAP region to obtain the PA.
> 
> Fixes: 7db8d2bd9b ("xen/riscv: add minimal stuff to mm.h to build full Xen")
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> --- a/xen/arch/riscv/include/asm/mm.h
> +++ b/xen/arch/riscv/include/asm/mm.h
> @@ -25,7 +25,7 @@ paddr_t pt_walk(vaddr_t va);
>  #define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
>  #define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
>  #define maddr_to_mfn(ma)    _mfn(paddr_to_pfn(ma))
> -#define vmap_to_mfn(va)     maddr_to_mfn(virt_to_maddr((vaddr_t)(va)))
> +#define vmap_to_mfn(va)     maddr_to_mfn(pt_walk((vaddr_t)(va)))

With this being the first use of pt_walk(), I wonder whether the function might
better return mfn_t (and simply ignore the low 12 bits of Vthe incoming VA; see
my respective comment on the earlier patch). After all it is quite natural for
a page table walk to return a page frame number, not a physical address.

Jan

On 1/27/25 11:09 AM, Jan Beulich wrote:
> On 20.01.2025 17:54, Oleksii Kurochko wrote:
>> vmap_to_mfn() uses virt_to_maddr(), which is designed to work with VA from
>> either the direct map region or Xen's linkage region (XEN_VIRT_START).
>> An assertion will occur if it is used with other regions, in particular for
>> the VMAP region.
>>
>> Since RISC-V lacks a hardware feature to request the MMU to translate a VA to
>> a PA (as Arm does, for example), software page table walking (pt_walk()) is
>> used for the VMAP region to obtain the PA.
>>
>> Fixes: 7db8d2bd9b ("xen/riscv: add minimal stuff to mm.h to build full Xen")
>> Signed-off-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> Reviewed-by: Jan Beulich<jbeulich@suse.com>
>
>> --- a/xen/arch/riscv/include/asm/mm.h
>> +++ b/xen/arch/riscv/include/asm/mm.h
>> @@ -25,7 +25,7 @@ paddr_t pt_walk(vaddr_t va);
>>   #define gaddr_to_gfn(ga)    _gfn(paddr_to_pfn(ga))
>>   #define mfn_to_maddr(mfn)   pfn_to_paddr(mfn_x(mfn))
>>   #define maddr_to_mfn(ma)    _mfn(paddr_to_pfn(ma))
>> -#define vmap_to_mfn(va)     maddr_to_mfn(virt_to_maddr((vaddr_t)(va)))
>> +#define vmap_to_mfn(va)     maddr_to_mfn(pt_walk((vaddr_t)(va)))
> With this being the first use of pt_walk(), I wonder whether the function might
> better return mfn_t (and simply ignore the low 12 bits of Vthe incoming VA; see
> my respective comment on the earlier patch). After all it is quite natural for
> a page table walk to return a page frame number, not a physical address.

I think it would be really better to return mfn_t (now I understand your comment on the earlier
patch better,if only I had read this comment first...)

Thanks.

~ Oleksii