Re: [PATCH v3 31/34] xen/riscv: add minimal stuff to mm.h to build full Xen

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2024-02-05 at 08:46 +0100, Jan Beulich wrote:
> On 02.02.2024 18:30, Oleksii wrote:
> > On Tue, 2024-01-23 at 14:03 +0100, Jan Beulich wrote:
> > > On 22.12.2023 16:13, Oleksii Kurochko wrote:
> > > > +/* Convert between Xen-heap virtual addresses and machine
> > > > frame
> > > > numbers. */
> > > > +#define __virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
> > > > +#define __mfn_to_virt(mfn) maddr_to_virt((paddr_t)(mfn) <<
> > > > PAGE_SHIFT)
> > > 
> > > These would imo better use maddr_to_mfn() and mfn_to_maddr(),
> > > rather
> > > than
> > > kind of open-coding them. The former could also use PFN_DOWN() as
> > > an
> > > alternative.
> > We can't to as __virt_to_mfn() when is used it is usually wrapped
> > by
> > _mfn() which expect to have unsigned long as an argument.
> 
> #define __virt_to_mfn(va)  mfn_x(maddr_to_mfn(virt_to_maddr(va))
> #define __mfn_to_virt(mfn) maddr_to_virt(mfn_to_maddr(_mfn(mfn)))
> 
> ?
I had an issue with the compilation when I tried to define them in
similar way.

I'll double check again.

> 
> > > > +/* Convert between Xen-heap virtual addresses and page-info
> > > > structures. */
> > > > +static inline struct page_info *virt_to_page(const void *v)
> > > > +{
> > > > +    BUG();
> > > > +    return NULL;
> > > > +}
> > > > +
> > > > +/*
> > > > + * We define non-underscored wrappers for above conversion
> > > > functions.
> > > > + * These are overriden in various source files while
> > > > underscored
> > > > version
> > > > + * remain intact.
> > > > + */
> > > > +#define virt_to_mfn(va)     __virt_to_mfn(va)
> > > > +#define mfn_to_virt(mfn)    __mfn_to_virt(mfn)
> > > 
> > > Is this really still needed? Would be pretty nice if in a new
> > > port we
> > > could get to start cleanly right away (i.e. by not needing per-
> > > file
> > > overrides, but using type-safe expansions here right away).
> > We still need __virt_to_mfn and __mfn_to_virt as common code use
> > them:
> >  * xen/common/xenoprof.c:24:#define virt_to_mfn(va)
> > mfn(__virt_to_mfn(va))
> 
> Are you meaning to enable this code on RISC-V.
Yes, that is what I meant.

~ Oleksii

> 
> >  * xen/include/xen/domain_page.h:59:#define
> > domain_page_map_to_mfn(ptr)
> > _mfn(__virt_to_mfn((unsigned long)(ptr)))
> 
> Hmm, yes, we should finally get this sorted. But yeah, not something
> I want
> to ask you to do.
> 
> Jan