On 17.02.2026 10:58, Oleksii Kurochko wrote:
>
> 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?
Oh, sorry, I should have made explicit that the request was for patch 2.
Here indeed ...
> 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");
... this may be the best you can do right now (unless you want to introduce
p2m_ram_ro).
Jan
