On 2/17/26 10:10 AM, Jan Beulich wrote:
On 17.02.2026 10:01, Oleksii Kurochko wrote:
On 2/16/26 1:38 PM, Jan Beulich wrote:
On 12.02.2026 17:21, Oleksii Kurochko wrote:
--- 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.
*/
Simply say that dom_xen isn't a "normal" domain?
+ 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).
May I suggest that right away you use the construct that I suggested Arm to
switch to (you were Cc-ed on that patch, I think)? Despite the absence of
p2m_ram_ro on RISC-V, that'll be usable, and it will allow keeping the code
untouched when p2m_ram_ro is introduced (sooner or later you will need it,
I expect).
Sure, but doesn't that patch is connected to another function
(translate_get_page())
and just fixing the handling of what get_page_from_gfn() in *t?
For get_page_from_gfn() to not miss the case when new type is introduced it make
sense to do the following:
if ( page->u.inuse.type_info & PGT_writable_page )
*t = p2m_ram_rw;
else
BUG_ON("unimplemented");
~ Oleksii
