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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

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

Jan