On 2/16/26 1:38 PM, 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?
Yes, it should be possible. The reason for now that is working for x86 and Arm
is that:
1. Arm only pass pointer to struct domain to p2m_get_page_from_gfn() so it is
enough just
to have forward declaration for struct domain.
2. x86 uses pointer to p2m_domain in arch_domain so there is no need to include
asm/p2m.h
in asm/domain.h and so forward declaration will be enough. And so there is
no dependency
between xen/sched.h and asm/p2m.h through asm/domain.h which leads to the
issue
mentioned in the commit message.
RISC-V could in principle follow the x86 pattern (avoid including asm/p2m.h),
but the current out-of-line approach is also acceptable, it is simpler and more
robust
against future header reordering problems.
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. 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);
+
+ /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
... this comment would also want re-wording.
As you mentioned in the another reply to this patch, I messed up x86 and Arm
implementation in a bad way, so here should be DOMID_XEN used instead of
"Non-translated".
Based on your reply it seems like the first comment should be also rephrased
as you mentioned that DOMID_XEN can't be called also "not auto-translated".
I think it would be better to write the following:
/*
* Special case for DOMID_XEN as it is the only domain so far that has
* no GFN space.
*/
+ 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?
Incorrectly copied from x86. It should be just:
*t = p2m_ram_rw
here as in RISC-V for MMIO pages owner isn't set to dom_io (and the same is
true for Arm I think).
Thanks.
~ Oleksii
