On 2/7/25 2:30 PM, Jan Beulich wrote:
On 07.02.2025 14:13, 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 mfn from pte_t.

To avoid introduce a circular dependency between asm/mm.h and asm/page.h by
including each other, the macro _vmap_to_mfn() is introduced in asm/page.h,
as it uses struct pte_t and pte_is_mapping() from asm/page.h. _vmap_to_mfn()
macro is then reused in the definition of vmap_to_mfn() macro in asm/mm.h.

Fixes: 7db8d2bd9b ("xen/riscv: add minimal stuff to mm.h to build full Xen")
Signed-off-by: Oleksii Kurochko<oleksii.kuroc...@gmail.com>
---
Changes in v3:
- Move vmap_to_mfn_ to asm/page.h to deal with circular dependency.
- Convert vmap_to_mfn_() to macros.
Why both?

Just for consistency. vmap_to_mfn_() could be defined as static inline, I can
send the new patch version with such changes, probably, static inline would be
better in this case.

--- a/xen/arch/riscv/include/asm/page.h
+++ b/xen/arch/riscv/include/asm/page.h
@@ -210,6 +210,13 @@ static inline pte_t pte_from_mfn(mfn_t mfn, unsigned int 
flags)
pte_t pt_walk(vaddr_t va, unsigned int *pte_level); +#define _vmap_to_mfn(va) \
+({                                      \
+    pte_t entry = pt_walk((va), NULL);  \
If this is to remain a macro, va doesn't need parenthesizing (as the argument
passed is just the identifier, not an expression.

Be careful with the naming of macro local variables. Consider a use size (for
whatever reason) having

     unsigned long entry;
     ...
     mfn = vmap_to_mfn(entry);

This is where appending an underscore comes into play.

This could be another reason to use|static inline _vmap_to_mfn(...)| instead of 
a macro.

I think I will rewrite the|_vmap_to_mfn()| macro as a|static inline| function 
in the next
patch series version. I'll send it after receiving comments on the entire patch 
series.

Thanks.

~ Oleksii

Reply via email to