Re: [PATCH v3 4/4] xen/memory, tools: Avoid hardcoding GUEST_MAGIC_BASE in init-dom0less

[Henry Wang <xin.wang2@xxxxxxx>]

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