Re: [PATCH] libs/xg: allow caller to provide extra memflags for populate physmap

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Dec 19, 2025 at 07:59:59AM +0100, Jan Beulich wrote:
> On 18.12.2025 22:34, Andrew Cooper wrote:
> > On 18/12/2025 3:27 pm, Jan Beulich wrote:
> >> On 18.12.2025 16:17, Roger Pau Monne wrote:
> >>> --- a/tools/libs/guest/xg_dom_x86.c
> >>> +++ b/tools/libs/guest/xg_dom_x86.c
> >>> @@ -1260,14 +1260,15 @@ static int meminit_pv(struct xc_dom_image *dom)
> >>>      /* allocate guest memory */
> >>>      for ( i = 0; i < nr_vmemranges; i++ )
> >>>      {
> >>> -        unsigned int memflags;
> >>> +        unsigned int memflags = dom->memflags;
> >>>          uint64_t pages, super_pages;
> >>>          unsigned int pnode = vnode_to_pnode[vmemranges[i].nid];
> >>>          xen_pfn_t extents[SUPERPAGE_BATCH_SIZE];
> >>>          xen_pfn_t pfn_base_idx;
> >>>  
> >>> -        memflags = 0;
> >>> -        if ( pnode != XC_NUMA_NO_NODE )
> >>> +        if ( pnode != XC_NUMA_NO_NODE &&
> >>> +             /* Only set the node if the caller hasn't done so. */
> >>> +             XENMEMF_get_node(memflags) == 0xFFU )
> >>>              memflags |= XENMEMF_exact_node(pnode);
> >> I'd like to suggest to avoid open-coding the 0xff here and elsewhere.
> >> XENMEMF_get_node(0) will be less fragile overall, imo.
> > 
> > XENMEMF_get_node(0) is even more meaningless than 0xFF, which is at
> > least obviously a sentinel value.
> 
> How that? XENMEMF_get_node(0) is the node that is used when no flags (0)
> were specified. I.e. the equivalent of NUMA_NO_NODE. The underlying
> (pretty abstract) point being that if we ever made a binary-incompatible,
> but source-compatible change to how wide the node representation is in
> the flags (e.g. by the consumer defining some manifest constant to
> engage the alternate behavior), XENMEMF_get_node(0) will continue to
> work while 0xFF won't.

I didn't use XENMEMF_get_node(0) because I found it confusing to read.
When I read that construct I internally associate with node 0, while
it's actually no node set, as the value passed to XENMEMF_get_node()
is the memflags, not a node ID.

> Otherwise at the very least I would strongly suggest libxg to make itself
> a #define for this (repeatedly used) 0xFFU.

I didn't want to introduce that because there's already a define
in libxc for no NUMA node ID, which is wider than 0xff.

Anyway, will do the change and send v2.

Thanks, Roger.