hvm: NUMA guest: extend memops hypercall

This patch extends the memops hypercall in a compatible way to transport a desired NUMA node number. The address_bits field will be limited to 8 bits and is now embedded in the mem_flags member, which additionally contains the node number (limited to 8 bit). Passing a node number of '0' (currently the default) will revert to automatic node selection (based on currently scheduled node).

Signed-off-by Andre Przywara <[email protected]>

Quoted 6 lines · Andre Przywara

This patch extends the memops hypercall in a compatible way to transport a desired NUMA node number. The address_bits field will be limited to 8 bits and is now embedded in the mem_flags member, which additionally contains the node number (limited to 8 bit). Passing a node number of '0' (currently the default) will revert to automatic node selection (based on currently scheduled node).

Should unprivileged domUs be allowed to specify the NUMA node they allocate from, regardless of whether they even run there? Seems like a breakage of guest isolation to me.

Quoted 3 lines

Should unprivileged domUs be allowed to specify the NUMA node they allocate from, regardless of whether they even run there? Seems like a breakage of guest isolation to me.

Good catch (I code in HVM land most of the time), I hope this small (attached) patch fixes this.

Quoted 5 lines · Andre Przywara

Quoted 3 lines

Should unprivileged domUs be allowed to specify the NUMA node they allocate from, regardless of whether they even run there? Seems like a breakage of guest isolation to me.

Good catch (I code in HVM land most of the time), I hope this small (attached) patch fixes this.

Looking some more, I still don't see that this patch can work. Don't the subfunctions in memory.c go and OR in MEMF_node() values on top of what the caller may have specified??

Quoted 3 lines

Looking some more, I still don't see that this patch can work. Don't the subfunctions in memory.c go and OR in MEMF_node() values on top of what the caller may have specified??

Maybe I don't get your question right, but the only part where the caller specified node number is used is the line I handled in the last patch. Later they only use the member memflags of struct memop_args, not struct xen_memory_reservation. If the node number is not specified (or blocked), it will be later determined by looking at the current scheduled pCPU (and thus node), but this is the current behavior anyway.

Quoted 9 lines · Andre Przywara

Quoted 3 lines

Looking some more, I still don't see that this patch can work. Don't the subfunctions in memory.c go and OR in MEMF_node() values on top of what the caller may have specified??

Maybe I don't get your question right, but the only part where the caller specified node number is used is the line I handled in the last patch. Later they only use the member memflags of struct memop_args, not struct xen_memory_reservation. If the node number is not specified (or blocked), it will be later determined by looking at the current scheduled pCPU (and thus node), but this is the current behavior anyway.

Take common/memory.c:populate_physmap() as a specific example. It unconditionally specifies MEMF_node() in its invocation of alloc_domheap_pages(), regardless of whether its caller has already specified a node in the memop_args structure that is passed into it.

Have you applied the patches correctly? From 02_numa_guest.patch:

(quoted diff) +1 -1

@@ -115,7 +113,7 @@
              goto out;
 
          page = alloc_domheap_pages(
-            d, a->extent_order, a->memflags | MEMF_node(node));
+            d, a->extent_order, a->memflags);
          if ( unlikely(page == NULL) )
          {
              gdprintk(XENLOG_INFO, "Could not allocate order=%d extent:"

The other use of MEMF_node is in exchange_memory, which is not given any NUMA node info from the caller, so this is correct.

Quoted 11 lines · Andre Przywara

Have you applied the patches correctly? From 02_numa_guest.patch:

(quoted diff) +1 -1

@@ -115,7 +113,7 @@
              goto out;
 
          page = alloc_domheap_pages(
-            d, a->extent_order, a->memflags | MEMF_node(node));
+            d, a->extent_order, a->memflags);
          if ( unlikely(page == NULL) )
          {
              gdprintk(XENLOG_INFO, "Could not allocate order=%d extent:"

Quoted 2 lines

The other use of MEMF_node is in exchange_memory, which is not given any NUMA node info from the caller, so this is correct.

When sent a patch sequence I expect the patches to apply and work independently (when applied one-by-one in order).

Anyway, your second patch changes the default NUMA allocation policy from allocate on home node for the domain to allocate on node I'm currently executing on. That would seem a net loss for PV guests (whose builder will not be explicitly specifying the numa node for allocations).