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.
