Hi Jan,
On 4/8/2024 2:22 PM, Jan Beulich wrote:
On 08.04.2024 05:19, Henry Wang wrote:
On 4/4/2024 5:38 PM, Jan Beulich wrote:
On 03.04.2024 10:16, Henry Wang wrote:
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -41,6 +41,11 @@
#define XENMEMF_exact_node(n) (XENMEMF_node(n) | XENMEMF_exact_node_request)
/* Flag to indicate the node specified is virtual node */
#define XENMEMF_vnode (1<<18)
+/*
+ * Flag to force populate physmap to use pages from domheap instead of 1:1
+ * or static allocation.
+ */
+#define XENMEMF_force_heap_alloc (1<<19)
As before, a separate new sub-op would look to me as being the cleaner
approach, avoiding the need to consume a bit position for something not
even going to be used on all architectures.
Like discussed in v2, I doubt that if introducing a new sub-op, the
helpers added to duplicate mainly populate_physmap() and the toolstack
helpers would be a good idea.
I'm curious what amount of duplication you still see left. By suitably
adding a new parameter, there should be very little left.
The duplication I see so far is basically the exact
xc_domain_populate_physmap(), say
xc_domain_populate_physmap_heap_alloc(). In init-dom0less.c, We can
replace the original call xc_domain_populate_physmap_exact() to call the
newly added xc_domain_populate_physmap_heap_alloc() which evokes the new
sub-op, then from the hypervisor side we set the alias MEMF flag and
share the populate_physmap().
Adding a new parameter to xc_domain_populate_physmap() or maybe even
xc_domain_populate_physmap_exact() is also a good idea (thanks). I was
just worrying there are already too many use cases of these two
functions in the existing code: there are 14 for
xc_domain_populate_physmap_exact() and 8 for
xc_domain_populate_physmap(). Adding a new parameter needs the update of
all these and the function declaration. If you really insist this way, I
can do this, sure.
Similarly as the way that we do for the
MEMF_force_heap_alloc, if in the future we run out of the bit positions,
can't we reuse this bit for different architectures as an alias? Or
maybe we can even alias it now?
I view this as far less desirable in the public interface.
I agree.
Kind regards,
Henry
Jan
