On 12/29/25 4:06 PM, Jan Beulich wrote:
On 22.12.2025 17:37, Oleksii Kurochko wrote:
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -1057,3 +1057,188 @@ int map_regions_p2mt(struct domain *d,
return rc;
}
+
+/*
+ * p2m_get_entry() should always return the correct order value, even if an
+ * entry is not present (i.e. the GFN is outside the range):
+ * [p2m->lowest_mapped_gfn, p2m->max_mapped_gfn] (1)
+ *
+ * This ensures that callers of p2m_get_entry() can determine what range of
+ * address space would be altered by a corresponding p2m_set_entry().
+ * Also, it would help to avoid costly page walks for GFNs outside range (1).
+ *
+ * Therefore, this function returns true for GFNs outside range (1), and in
+ * that case the corresponding level is returned via the level_out argument.
+ * Otherwise, it returns false and p2m_get_entry() performs a page walk to
+ * find the proper entry.
+ */
+static bool check_outside_boundary(const struct p2m_domain *p2m, gfn_t gfn,
+ gfn_t boundary, bool is_lower,
+ unsigned int *level_out)
+{
+ unsigned int level = P2M_ROOT_LEVEL(p2m);
+ bool ret = false;
+
+ ASSERT(p2m);
+
+ if ( is_lower ? gfn_x(gfn) < gfn_x(boundary)
+ : gfn_x(gfn) > gfn_x(boundary) )
+ {
+ for ( ; level; level-- )
+ {
+ unsigned long mask = BIT(P2M_GFN_LEVEL_SHIFT(level), UL) - 1;
+
+ if ( is_lower ? (gfn_x(gfn) | mask) < gfn_x(boundary)
+ : (gfn_x(gfn) & ~mask) > gfn_x(boundary) )
+ break;
+ }
+
+ ret = true;
For this case ...
+ }
+
+ if ( level_out )
+ *level_out = level;
... this is correct, but of "ret" is still false it very likely isn't, and
arranging things this way may end up being confusing. Perhaps "level" should
be constrained to the if()'s scope? The caller cares about the value only
when the return value is true, after all.
We could simply move the "|if ( level_out )"| check inside the|if| block, but
is this really a significant issue? We still need to check the return value,
and if it is false,|level_out| should just be ignored and there is not big
difference then if level_out will contain what it contained before the call
of check_outside_boundary() or it will be set to P2M_ROOT_LEVEL(p2m).
Alternatively, could we initialize|level| to a non-existent value in the
"ret=false" case, for example|P2M_MAX_ROOT_LEVEL| + 1, and return that value
via|level_out|?
~ Oleksii
