<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> <div class="moz-cite-prefix">On 4/26/21 3:55 PM, Peter Krempa wrote: </div> <blockquote type="cite" cite="mid:YIZx2Y%2FqAG+mpWuB@angien.pipo.sk"> <pre class="moz-quote-pre" wrap="">On Sun, Apr 25, 2021 at 17:33:31 +0800, Zhiyong Ye wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Limit the amount of ram below 4G. This can increase the address space used by PCI devices below 4G and it can be used by adding attributes in XML like this: <domain> ... <memory unit="MiB" below4g="2048">4096</memory> </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This illustrates that sharing the 'unit' argument between 'below4g' and the actual memory size is wrong (pointed out also below). In case the user modifies the unit but doesn't change below4g value it may be misrepresented and since below4g is new some tools might not actually know how to handle that. below4g must either be by default in kiB, or have an own unit (which will probably be ugly). </pre> </blockquote> <pre><span style="color: rgb(43, 47, 54); font-size: 14px; font-style: normal; font-variant-ligatures: none; font-variant-caps: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">So, how should we place 'blow4g'? I have summarized that we may have two ways: The first way is to put 'below4g' in the existing element, as Daniel suggested, that 'below4g' does not have its own 'unit' and let it default to KiB, e.g, <memory unit = "MiB" below4g = "2048"> 4096 </memory> The second way is to add a new element and 'below4g' has its own 'unit', such as:<span style="color: rgb(43, 47, 54); font-family: LarkHackSafariFont, LarkEmojiFont, LarkChineseQuote, -apple-system, system-ui, "Helvetica Neue", Arial, "Segoe UI", "PingFang SC", "Microsoft Yahei", "Hiragino Sans GB", sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"; font-size: 14px; font-style: normal; font-variant-ligatures: none; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"> <memory unit = 'MiB'> 4096 </memory> <span style="color: rgb(43, 47, 54); font-size: 14px; font-style: normal; font-variant-ligatures: none; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(255, 255, 255); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"><below4gMemory unit = 'MiB'> 2048 </below4gMemory></pre> <blockquote type="cite" cite="mid:YIZx2Y%2FqAG+mpWuB@angien.pipo.sk"> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> ... </domain> Signed-off-by: Zhiyong Ye <a class="moz-txt-link-rfc2396E" href="mailto:yezhiyong@bytedance.com"><yezhiyong@bytedance.com></a> Signed-off-by: zhenwei pi <a class="moz-txt-link-rfc2396E" href="mailto:pizhenwei@bytedance.com"><pizhenwei@bytedance.com></a> Signed-off-by: zhangruien <a class="moz-txt-link-rfc2396E" href="mailto:zhangruien@bytedance.com"><zhangruien@bytedance.com></a> --- src/conf/domain_conf.c | 19 +++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/qemu/qemu_command.c | 4 ++++ </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Missing docs (doc/formatdomain.rst) and test changes. Also we usually don't mix conf and qemu changes. You need at least a test case for qemuxml2argvtest and qemuxml2xmltest. The conf change is also missing a check in the ABI stability checking code. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> 3 files changed, 26 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a72d58f488..c211a69ed1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4720,6 +4720,19 @@ virDomainDefPostParseMemory(virDomainDef *def, return -1; } + if (def->mem.max_ram_below_4g && + def->mem.max_ram_below_4g < (1ULL << 10)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary is too small, " + "BIOS may not work with less than 1MiB")); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> "may not work" seems like you picked an arbitrary limit, and the same way it may not work with more than 1MiB. In cases when there isn't a strict technical limit, but rather "it usually doesn't work" like this the check is almost pointless.</pre> </blockquote> <pre>This is seen from QEMU code. When it is less than 1MiB, QEMU cannot create a new virtual machine. </pre> <blockquote type="cite" cite="mid:YIZx2Y%2FqAG+mpWuB@angien.pipo.sk"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ return -1; + } else if (def->mem.max_ram_below_4g > (1ULL << 22)) { </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> we usually prefer to describe the limit as 4 * 1024 * 1024 </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ virReportError(VIR_ERR_XML_ERROR, "%s", + _("maximum memory size below the 4GiB boundary must be " + "less than or equal to 4GiB")); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> don't break up error messages into multiple lines </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ return -1; </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This belongs to conf/domain_validate.c You'll need to add a new function e.g. 'virDomainDefValidateMemory' call it from virDomainDefValidateInternal and add those checks there. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ } + return 0; } @@ -19786,6 +19799,10 @@ virDomainDefParseMemory(virDomainDef *def, &def->mem.max_memory, false, false) < 0) goto error; + if (virDomainParseMemory("./memory[1]/@below4g", "./memory[1]/@unit", ctxt, + &def->mem.max_ram_below_4g, false, true) < 0) </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This uses the 'unit' which is used for memory for the 'below4g' attribute. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ goto error; + if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Failed to parse memory slot count")); @@ -28844,6 +28861,8 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, if (def->mem.dump_core) virBufferAsprintf(buf, " dumpCore='%s'", virTristateSwitchTypeToString(def->mem.dump_core)); + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(buf, " below4g='%llu'", def->mem.max_ram_below_4g); virBufferAsprintf(buf, " unit='KiB'>%llu</memory>\n", virDomainDefGetMemoryTotal(def)); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4838687edf..a939d43e93 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2597,6 +2597,9 @@ struct _virDomainMemtune { unsigned long long max_memory; /* in kibibytes */ unsigned int memory_slots; /* maximum count of RAM memory slots */ + /* maximum memory below the 4GiB boundary (32bit boundary) */ + unsigned long long max_ram_below_4g; /* in kibibytes */ + bool nosharepages; bool locked; int dump_core; /* enum virTristateSwitch */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index be93182092..c69ad781e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6961,6 +6961,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, cfg->dumpGuestCore ? "on" : "off"); } + if (def->mem.max_ram_below_4g > 0) + virBufferAsprintf(&buf, ",max-ram-below-4g=%llu", + def->mem.max_ram_below_4g * 1024); + if (def->mem.nosharepages) virBufferAddLit(&buf, ",mem-merge=off"); -- 2.24.3 (Apple Git-128) </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> </pre> </blockquote> </body> </html>