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).
Jan
