Re: [PATCH v1 1/3] xen/riscv: implement virt_to_maddr()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.09.2024 17:08, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/include/asm/mm.h
> +++ b/xen/arch/riscv/include/asm/mm.h
> @@ -28,7 +28,20 @@ static inline void *maddr_to_virt(paddr_t ma)
>      return NULL;
>  }
>  
> -#define virt_to_maddr(va) ({ BUG_ON("unimplemented"); 0; })
> +static inline unsigned long virt_to_maddr(unsigned long va)
> +{
> +    ASSERT(va >= (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE));
> +    if ((va >= DIRECTMAP_VIRT_START) &&
> +        (va < (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE)))
> +        return directmapoff_to_maddr(va - DIRECTMAP_VIRT_START);

While the cover letter states a dependency on another series, I'm unable
to spot directmapoff_to_maddr() in the tree or in that other series.

> +    BUILD_BUG_ON(XEN_VIRT_SIZE != MB(2));
> +    ASSERT(((long)va >> (PAGETABLE_ORDER + PAGE_SHIFT)) ==
> +           ((long)XEN_VIRT_START >> (PAGETABLE_ORDER + PAGE_SHIFT)));

What's the point of the casts here? va is unsigned long and XEN_VIRT_START
is a literal number (which probably ought to have a UL suffix).

> +    return phys_offset + va;

Don't you need to subtract XEN_VIRT_START here?

Jan