Re: [PATCH v2 16/17] xen/riscv: implement mfn_valid() and page reference, ownership handling helpers

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 7/21/25 3:42 PM, Jan Beulich wrote:

> On 18.07.2025 16:49, Oleksii Kurochko wrote:
> > On 7/2/25 12:09 PM, Jan Beulich wrote:
> > > On 10.06.2025 15:05, Oleksii Kurochko wrote:
> > > > Implement the mfn_valid() macro to verify whether a given MFN is valid by
> > > > checking that it falls within the range [start_page, max_page).
> > > > These bounds are initialized based on the start and end addresses of RAM.
> > > >
> > > > As part of this patch, start_page is introduced and initialized with the
> > > > PFN of the first RAM page.
> > > >
> > > > Also, after providing a non-stub implementation of the mfn_valid() macro,
> > > > the following compilation errors started to occur:
> > > >    riscv64-linux-gnu-ld: prelink.o: in function `__next_node':
> > > >    /build/xen/./include/xen/nodemask.h:202: undefined reference to `page_is_ram_type'
> > > >    riscv64-linux-gnu-ld: prelink.o: in function `get_free_buddy':
> > > >    /build/xen/common/page_alloc.c:881: undefined reference to `page_is_ram_type'
> > > >    riscv64-linux-gnu-ld: prelink.o: in function `alloc_heap_pages':
> > > >    /build/xen/common/page_alloc.c:1043: undefined reference to `page_get_owner_and_reference'
> > > >    riscv64-linux-gnu-ld: /build/xen/common/page_alloc.c:1098: undefined reference to `page_is_ram_type'
> > > >    riscv64-linux-gnu-ld: prelink.o: in function `ns16550_interrupt':
> > > >    /build/xen/drivers/char/ns16550.c:205: undefined reference to `get_page'
> > > >    riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol `page_get_owner_and_reference' isn't defined
> > > >    riscv64-linux-gnu-ld: final link failed: bad value
> > > >    make[2]: *** [arch/riscv/Makefile:35: xen-syms] Error 1
> > > > To resolve these errors, the following functions have also been introduced,
> > > > based on their Arm counterparts:
> > > > - page_get_owner_and_reference() and its variant to safely acquire a
> > > >    reference to a page and retrieve its owner.
> > > > - put_page() and put_page_nr() to release page references and free the page
> > > >    when the count drops to zero.
> > > >    For put_page_nr(), code related to static memory configuration is wrapped
> > > >    with CONFIG_STATIC_MEMORY, as this configuration has not yet been moved to
> > > >    common code. Therefore, PGC_static and free_domstatic_page() are not
> > > >    introduced for RISC-V. However, since this configuration could be useful
> > > >    in the future, the relevant code is retained and conditionally compiled.
> > > > - A stub for page_is_ram_type() that currently always returns 0 and asserts
> > > >    unreachable, as RAM type checking is not yet implemented.
> > > >           
> > > How does this end up working when common code references the function?
> > >         
> > Based on the following commit message:
> >      Callers are VT-d (so x86 specific) and various bits of page offlining
> >      support, which although it looks generic (and is in xen/common) does
> >      things like diving into page_info->count_info which is not generic.
> >      
> >      In any case on this is only reachable via XEN_SYSCTL_page_offline_op,
> >      which clearly shouldn't be called on ARM just yet.
> >       
> What commit message are you talking about? Nothing like the above is anywhere
> in this patch.
>     
It's pretty old commit:
  commit 214c4cd94a80bcaf042f25158eaa7d0e5bbc3b5b
  Author: Ian Campbell <ian.campbell@xxxxxxxxxx>
  Date:   Wed Dec 19 14:16:23 2012 +0000

But since that time page_is_ram_type() hasn't been changed for Arm.

> > There is no callers for page_is_ram_type() for Arm now, and I expect something similar
> > for RISC-V. As we don't even introduce hypercalls for RISC-V, we can just live
> > without it.
> >       
> If there's no caller, why the stub?
>     
Because that parts of common code which are using it aren't under ifdef, for example, this
one function:
  int offline_page(mfn_t mfn, int broken, uint32_t *status)
And specifically this function is called when XEN_SYSCTL_page_offline_op is handled.

But I agree that it seems like nothing prevents to call XEN_SYSCTL_page_offline_op.

~ Oleksii