On 12/9/24 3:23 PM, Jan Beulich wrote:
On 27.11.2024 13:50, Oleksii Kurochko wrote:
Introduce the destroy_xen_mappings() function, which removes page
mappings in Xen's page tables between a start address s and an end
address e.
The function ensures that both s and e are page-aligned
and verifies that the start address is less than or equal to the end
address before calling pt_update() to invalidate the mappings.
The pt_update() function is called with INVALID_MFN and PTE_VALID=0
in the flags, which tell pt_update() to remove mapping. No additional
ASSERT() is required to check these arguments, as they are hardcoded in
the call to pt_update() within destroy_xen_mappings().
Signed-off-by: Oleksii Kurochko<oleksii.kuroc...@gmail.com>
Acked-by: Jan Beulich<jbeul...@suse.com>
However, ...
--- a/xen/arch/riscv/pt.c
+++ b/xen/arch/riscv/pt.c
@@ -421,6 +421,14 @@ int map_pages_to_xen(unsigned long virt,
return pt_update(virt, mfn, nr_mfns, flags);
}
+int destroy_xen_mappings(unsigned long s, unsigned long e)
+{
+ ASSERT(IS_ALIGNED(s, PAGE_SIZE));
+ ASSERT(IS_ALIGNED(e, PAGE_SIZE));
+ ASSERT(s <= e);
+ return pt_update(s, INVALID_MFN, PFN_DOWN(e - s), 0);
+}
... I'm unconvinced the constraints need to be this strict. You could,
for example, very well just avoiding to call pt_update() when s > e
(or really when s >= e). Thoughts?
On one hand, we could simply avoid calling |pt_update()|, but on the
other hand, this approach might cause us to miss a bug without any
notification.
Given that this is an|ASSERT()| that only triggers in debug builds and is
unlikely to occur,
I believe it is not critical to include the|ASSERT()| here. Additionally,
avoiding an extra
|if| condition helps prevent any potential performance impact. However, the|if|
condition
would likely evaluate to true most of the time, allowing hardware optimizations
to handle
it efficiently.
~ Oleksii
