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