| Message ID | bf85f6987c2a2f3374ad47fc0bf1513d69372b1f.1737391102.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes for vmap_to_mfn() and pt_mapping_level | expand |
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
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)
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(-)