On 16.02.2026 13:38, Jan Beulich wrote: > On 12.02.2026 17:21, Oleksii Kurochko wrote: >> Provide a RISC-V implementation of get_page_from_gfn(), matching the >> semantics used by other architectures. >> >> For translated guests, this is implemented as a wrapper around >> p2m_get_page_from_gfn(). For DOMID_XEN, which is not auto-translated, >> provide a 1:1 RAM/MMIO mapping and perform the required validation and >> reference counting. >> >> The function is implemented out-of-line rather than as a static inline, >> to avoid header ordering issues where struct domain is incomplete when >> asm/p2m.h is included, leading to build failures: >> In file included from ./arch/riscv/include/asm/domain.h:10, >> from ./include/xen/domain.h:16, >> from ./include/xen/sched.h:11, >> from ./include/xen/event.h:12, >> from common/cpu.c:3: >> ./arch/riscv/include/asm/p2m.h: In function 'get_page_from_gfn': >> ./arch/riscv/include/asm/p2m.h:50:33: error: invalid use of undefined type >> 'struct domain' >> 50 | #define p2m_get_hostp2m(d) (&(d)->arch.p2m) >> | ^~ >> ./arch/riscv/include/asm/p2m.h:180:38: note: in expansion of macro >> 'p2m_get_hostp2m' >> 180 | return p2m_get_page_from_gfn(p2m_get_hostp2m(d), >> _gfn(gfn), t); >> | ^~~~~~~~~~~~~~~ >> make[2]: *** [Rules.mk:253: common/cpu.o] Error 1 >> make[1]: *** [build.mk:72: common] Error 2 >> make: *** [Makefile:623: xen] Error 2 > > Surely this can be addressed, when x86 and Arm have the function as inline? > >> Signed-off-by: Oleksii Kurochko <[email protected]> >> --- >> Does it make sense to make this function almost fully generic? >> >> It looks like most of the logic here is architecture-independent and >> identical >> across architectures, except for the following points: >> >> 1. ``` >> if ( likely(d != dom_xen) ) >> ``` >> >> This could be made generic by introducing paging_mode_translate() for ARM >> and defining it as `(d != dom_xen)` there. >> >> 2. ``` >> if ( t ) >> *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct_io; >> ``` >> >> Here, only `p2m_mmio_direct_io` appears to be architecture-specific. This >> could be abstracted via a helper such as `dom_io_p2m_type()` and used here >> instead. > > With P2M stuff I'd be careful. Abstracting the two aspects above may make > future arch-specific changes there more difficult. > >> --- a/xen/arch/riscv/p2m.c >> +++ b/xen/arch/riscv/p2m.c >> @@ -1557,3 +1557,31 @@ void p2m_handle_vmenter(void) >> flush_tlb_guest_local(); >> } >> } >> + >> +struct page_info *get_page_from_gfn(struct domain *d, unsigned long gfn, >> + p2m_type_t *t, p2m_query_t q) >> +{ >> + struct page_info *page; >> + >> + /* >> + * Special case for DOMID_XEN as it is the only domain so far that is >> + * not auto-translated. >> + */ > > Once again something taken verbatim from Arm.
Actually it's a mix, up to ... > Yes, dom_xen can in fact appear > here, but it's not a real domain, has no memory truly assigned to it, has no > GFN space, and hence calling it translated (or not) is simply wrong (at best: > misleading). IOW ... > >> + if ( likely(d != dom_xen) ) >> + return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t); ... here it's Arm code, but what follows is x86 code. Why did you create such a mix? Jan >> + /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */ > > ... this comment would also want re-wording. > >> + if ( t ) >> + *t = p2m_invalid; >> + >> + page = mfn_to_page(_mfn(gfn)); >> + >> + if ( !mfn_valid(_mfn(gfn)) || !get_page(page, d) ) >> + return NULL; >> + >> + if ( t ) >> + *t = likely(d != dom_io) ? p2m_ram_rw : p2m_mmio_direct_io; > > If only dom_xen can make it here, why the check for dom_io? > > Jan
