On 11/25/2025 1:12 PM, Peter Krempa wrote:
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<[email protected]> Signed-off-by: Nathan Chen<[email protected]> --- 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.
Ok, I will separate it out in the next revision.
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.
Will do, thanks.
+ + 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.
Ok, I will take some time to simplify the structure here in the next revision, either separating out the logic from qemuBuildMemoryDimmBackendStr and qemuBuildNumaCommandLine or making the implementation simpler within these functions. Thanks for taking a preliminary look at these!
-Nathan
