On Tue, Nov 25, 2025 at 11:17:03 -0800, Nathan Chen via Devel wrote:
Add qemu CLI support for EGM memory device model: - Specify EGM device path to memory-backend-file object - Support acpi-egm-memory object with id, pci-dev, and node attributes - Consolidate all acpi-egm-memory objects' memory into a single memory-backend-file per EGM chardev specified.
Signed-off-by: Ian May <ianm@nvidia.com> Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- src/qemu/qemu_alias.c | 7 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 158 ++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.c | 13 ++- src/qemu/qemu_domain_address.c | 3 + src/qemu/qemu_driver.c | 1 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_monitor_json.c | 1 + src/qemu/qemu_postparse.c | 1 + src/qemu/qemu_process.c | 2 + src/qemu/qemu_validate.c | 6 ++ 12 files changed, 180 insertions(+), 16 deletions(-)
Note that I'm replying to this patch just due to the issue with qemu capabilities. This is not a full review. [...]
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index f180844e66..3eb12235f4 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -730,6 +730,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISK_TIMED_STATS, /* timed stats support ('stats-intervals' property of disk frontends) */ QEMU_CAPS_QUERY_ACCELERATORS, /* query-accelerators command */ QEMU_CAPS_MSHV, /* -accel mshv */ + QEMU_CAPS_DEVICE_ACPI_EGM_MEMORY, /* For using extended GPU memory */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;
For any further postin please separate the addition of the capability along with the detection and any change to the detected capabilities into a separate patch.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b69fe23236..33848aa781 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3573,12 +3593,17 @@ qemuBuildMemoryDimmBackendStr(virCommand *cmd, virDomainMemoryDef *mem, virDomainDef *def, virQEMUDriverConfig *cfg, - qemuDomainObjPrivate *priv) + qemuDomainObjPrivate *priv, + GHashTable *egmBackends) { g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) tcProps = NULL; virBitmap *nodemask = NULL; g_autofree char *alias = NULL; + unsigned long long originalSize = 0; + bool isEGM = (mem->model == VIR_DOMAIN_MEMORY_MODEL_EGM); + bool shouldCreateBackend = true; + qemuEGMBackendInfo *egmInfo = NULL;
if (!mem->info.alias) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -3588,19 +3613,65 @@ qemuBuildMemoryDimmBackendStr(virCommand *cmd,
alias = g_strdup_printf("mem%s", mem->info.alias);
- if (qemuBuildMemoryBackendProps(&props, alias, cfg, priv, - def, mem, true, false, &nodemask) < 0) - return -1; + /* Handle EGM shared backend logic */ + if (isEGM && egmBackends) { + const char *egmPath = mem->source.egm.path; + egmInfo = g_hash_table_lookup(egmBackends, egmPath);
- if (qemuBuildThreadContextProps(&tcProps, &props, def, priv, nodemask) < 0) - return -1; + if (egmInfo) { + alias = g_strdup(egmInfo->alias); + if (egmInfo->created) { + /* Backend already created, skip backend creation */ + shouldCreateBackend = false; + } else { + /* First device for this path - temporarily use accumulated size */ + originalSize = mem->size; + mem->size = egmInfo->totalSize; + egmInfo->created = true; + } + } + }
- if (tcProps && - qemuBuildObjectCommandlineFromJSON(cmd, tcProps) < 0) - return -1; + if (shouldCreateBackend) { + /* Use existing function unchanged */ + if (qemuBuildMemoryBackendProps(&props, alias, cfg, priv, + def, mem, true, false, &nodemask) < 0) { + if (originalSize > 0) + mem->size = originalSize; /* Restore on error */ + return -1; + }
- if (qemuBuildObjectCommandlineFromJSON(cmd, props) < 0) - return -1; + /* Restore original size after backend props are built */ + if (originalSize > 0) + mem->size = originalSize; + + if (qemuBuildThreadContextProps(&tcProps, &props, def, priv, nodemask) < 0) + return -1; + + if (tcProps && + qemuBuildObjectCommandlineFromJSON(cmd, tcProps) < 0) + return -1; + + if (qemuBuildObjectCommandlineFromJSON(cmd, props) < 0) + return -1; + } + + if (isEGM) { + g_autofree char *egmObjStr = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + + virBufferAsprintf(&buf, "acpi-egm-memory,id=%s", mem->info.alias); + + if (mem->target.egm.pciDev) + virBufferAsprintf(&buf, ",pci-dev=%s", mem->target.egm.pciDev); + + if (mem->targetNode >= 0) + virBufferAsprintf(&buf, ",node=%d", mem->targetNode); + + egmObjStr = virBufferContentAndReset(&buf); + + virCommandAddArgList(cmd, "-object", egmObjStr, NULL); + }
return 0; }
[...]
@@ -7835,6 +7910,37 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, hmat = true; }
+ /* Pre-scan EGM devices to group by path and calculate total sizes */ + egmBackends = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, + (GDestroyNotify)qemuEGMBackendInfoFree);
For creating hash tables use virHashNew, which uses our hashing function.
+ + for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_EGM) { + const char *egmPath = def->mems[i]->source.egm.path; + qemuEGMBackendInfo *info = g_hash_table_lookup(egmBackends, egmPath); + + if (!info) { + info = g_new0(qemuEGMBackendInfo, 1); + info->alias = g_strdup_printf("memegm%zu", egmBackendCount); + egmBackendCount++; + info->totalSize = def->mems[i]->size; + info->created = false; + info->firstMem = def->mems[i]; + g_hash_table_insert(egmBackends, g_strdup(egmPath), info); + } else { + info->totalSize += def->mems[i]->size; + } + } + } + + /* Build the actual backend and device objects */ + for (i = 0; i < def->nmems; i++) { + if (def->mems[i]->model == VIR_DOMAIN_MEMORY_MODEL_EGM) { + if (qemuBuildMemoryDimmBackendStr(cmd, def->mems[i], def, cfg, priv, egmBackends) < 0) + goto cleanup; + } + } + nodeBackends = g_new0(virJSONValue *, ncells); nodemask = g_new0(virBitmap *, ncells);
@@ -7870,8 +7976,18 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg, for (i = 0; i < ncells; i++) { ssize_t initiator = virDomainNumaGetNodeInitiator(def->numa, i); unsigned long long memSize = virDomainNumaGetNodeMemorySize(def->numa, i); + bool egmBacked = false; + size_t k; + + for (k = 0; k < def->nmems; k++) { + if (def->mems[k]->model == VIR_DOMAIN_MEMORY_MODEL_EGM && + def->mems[k]->targetNode == (int)i) { + egmBacked = true; + break; + } + }
- if (needBackend && memSize > 0) { + if (needBackend && memSize > 0 && !egmBacked) { g_autoptr(virJSONValue) tcProps = NULL;
if (qemuBuildThreadContextProps(&tcProps, &nodeBackends[i], @@ -7901,7 +8017,15 @@ qemuBuildNumaCommandLine(virQEMUDriverConfig *cfg,
if (memSize > 0) { if (needBackend) { - virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); + if (egmBacked) { + /* Look up the actual backend alias for EGM */ + const char *egmPath = def->mems[k]->source.egm.path; + qemuEGMBackendInfo *egmInfo = g_hash_table_lookup(egmBackends, egmPath); + const char *backendAlias = egmInfo ? egmInfo->alias : def->mems[k]->info.alias; + virBufferAsprintf(&buf, ",memdev=%s", backendAlias); + } else { + virBufferAsprintf(&buf, ",memdev=ram-node%zu", i); + } } else { virBufferAsprintf(&buf, ",mem=%llu", memSize / 1024); }
Hmm 'qemuBuildMemoryDimmBackendStr' and 'qemuBuildNumaCommandLine' are getting quite out of hand these patches (they're kind of a mess already in the current state) and all of that to support some niche hardware. Please consider refactoring the code first to simplify it. This will be a nightmare to review otherwise.