Re: [PATCH for 4.20? v3 2/3] xen/riscv: update defintion of vmap_to_mfn()

[Jan Beulich <jbeulich@xxxxxxxx>]

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.kurochko@xxxxxxxxx>
> ---
> Changes in v3:
> - Move vmap_to_mfn_ to asm/page.h to deal with circular dependency.
> - Convert vmap_to_mfn_() to macros.

Why both?

> --- 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.

Jan