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
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
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.
---
---
xen/arch/riscv/include/asm/p2m.h | 8 ++------
xen/arch/riscv/p2m.c | 28 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/xen/arch/riscv/include/asm/p2m.h b/xen/arch/riscv/include/asm/p2m.h
index 0cdd3dc44683..c68494593fd9 100644
--- a/xen/arch/riscv/include/asm/p2m.h
+++ b/xen/arch/riscv/include/asm/p2m.h
@@ -171,12 +171,8 @@ typedef unsigned int p2m_query_t;
#define P2M_ALLOC (1u<<0) /* Populate PoD and paged-out entries */
#define P2M_UNSHARE (1u<<1) /* Break CoW sharing */
-static inline struct page_info *get_page_from_gfn(
- struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
-{
- BUG_ON("unimplemented");
- return NULL;
-}
+struct page_info *get_page_from_gfn(struct domain *d, unsigned long gfn,
+ p2m_type_t *t, p2m_query_t q);
static inline void memory_type_changed(struct domain *d)
{
diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 275c38020ae2..f5b03e1e3264 100644
--- 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.
+ */
+ 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 */
+
+ 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;
+
+ return page;
+}
--
2.52.0On 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 <oleksii.kurochko@gmail.com>
> ---
> 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.
> + 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
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<oleksii.kurochko@gmail.com>
>> ---
>> 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
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
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
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
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 <oleksii.kurochko@gmail.com>
>> ---
>> 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
© 2016 - 2026 Red Hat, Inc.