ACPI memory hotplug

From: Vasilis Liaskovitis <[email protected]>

This patch adds a 'SIZE' type property to qdev.

It will make dimm description more convenient by allowing sizes to be specified with K,M,G,T prefixes instead of number of bytes e.g.:
-device dimm,id=mem0,size=2G,bus=membus.0

Credits go to Ian Molton for original patch. See:
http://patchwork.ozlabs.org/patch/38835/

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

Am 26.06.2013 11:13, schrieb Hu Tao:

Quoted 11 lines

From: Vasilis Liaskovitis <[email protected]>

This patch adds a 'SIZE' type property to qdev.

It will make dimm description more convenient by allowing sizes to be specified with K,M,G,T prefixes instead of number of bytes e.g.:
-device dimm,id=mem0,size=2G,bus=membus.0

Credits go to Ian Molton for original patch. See:
http://patchwork.ozlabs.org/patch/38835/

Quoted 2 lines

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

If the original patch is not from Vasilis, why is this patch not carrying Ian's Signed-off-by from that patch before Vasilis'? (Then we could drop the Patchwork link.)

From: Vasilis Liaskovitis <[email protected]>

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

---
 vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

vl.c +30 -0

index 767e020..9d88a79 100644
@@ -170,6 +170,7 @@ int main(int argc, char **argv)
 
 #include "ui/qemu-spice.h"
 #include "qapi/string-input-visitor.h"
+#include "hw/mem-hotplug/dimm.h"
 
 //#define DEBUG_NET
 //#define DEBUG_SLIRP

@@ -252,6 +253,7 @@ static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
 int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 unsigned long *node_cpumask[MAX_NODES];
+int nb_hp_dimms;
 
 uint8_t qemu_uuid[16];
 

@@ -2338,6 +2340,50 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static int dimmcfg_init_func(QemuOpts *opts, void *opaque)
+{
+    const char *driver;
+    const char *id;
+    uint64_t node, size;
+    uint32_t populated;
+    const char *buf, *busbuf;
+
+    /* DimmDevice configuration needs to be known in order to initialize chipset
+     * with correct memory and pci ranges. But all devices are created after
+     * chipset / machine initialization. In * order to avoid this problem, we
+     * parse dimm information earlier into dimmcfg structs. */
+
+    driver = qemu_opt_get(opts, "driver");
+    if (!strcmp(driver, "dimm")) {
+
+        id = qemu_opts_id(opts);
+        buf = qemu_opt_get(opts, "size");
+        parse_option_size("size", buf, &size, NULL);
+        buf = qemu_opt_get(opts, "node");
+        parse_option_number("node", buf, &node, NULL);
+        busbuf = qemu_opt_get(opts, "bus");
+        buf = qemu_opt_get(opts, "populated");
+        if (!buf) {
+            populated = 0;
+        } else {
+            populated = strcmp(buf, "on") ? 0 : 1;
+        }

vl.c +11 -0

+        nb_hp_dimms++;
+    }
+
+    return 0;
+}
+
 #ifdef CONFIG_VIRTFS
 static int fsdev_init_func(QemuOpts *opts, void *opaque)
 {

@@ -4260,6 +4306,11 @@ int main(int argc, char **argv, char **envp)
     }
     qemu_add_globals();
 
+    /* init generic devices */
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+           dimmcfg_init_func, NULL, 1) != 0) {
+        exit(1);
+    }
     qdev_machine_init();
 
     QEMUMachineInitArgs args = { .ram_size = ram_size,

Il 26/06/2013 11:13, Hu Tao ha scritto:

Quoted 60 lines

From: Vasilis Liaskovitis <[email protected]>

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

---
 vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

vl.c +30 -0

index 767e020..9d88a79 100644
@@ -170,6 +170,7 @@ int main(int argc, char **argv)
 
 #include "ui/qemu-spice.h"
 #include "qapi/string-input-visitor.h"
+#include "hw/mem-hotplug/dimm.h"
 
 //#define DEBUG_NET
 //#define DEBUG_SLIRP

@@ -252,6 +253,7 @@ static QTAILQ_HEAD(, FWBootEntry) fw_boot_order =
 int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 unsigned long *node_cpumask[MAX_NODES];
+int nb_hp_dimms;
 
 uint8_t qemu_uuid[16];
 

@@ -2338,6 +2340,50 @@ static int chardev_init_func(QemuOpts *opts, void *opaque)
     return 0;
 }
 
+static int dimmcfg_init_func(QemuOpts *opts, void *opaque)
+{
+    const char *driver;
+    const char *id;
+    uint64_t node, size;
+    uint32_t populated;
+    const char *buf, *busbuf;
+
+    /* DimmDevice configuration needs to be known in order to initialize chipset
+     * with correct memory and pci ranges. But all devices are created after
+     * chipset / machine initialization. In * order to avoid this problem, we
+     * parse dimm information earlier into dimmcfg structs. */
+
+    driver = qemu_opt_get(opts, "driver");
+    if (!strcmp(driver, "dimm")) {
+
+        id = qemu_opts_id(opts);
+        buf = qemu_opt_get(opts, "size");
+        parse_option_size("size", buf, &size, NULL);
+        buf = qemu_opt_get(opts, "node");
+        parse_option_number("node", buf, &node, NULL);
+        busbuf = qemu_opt_get(opts, "bus");
+        buf = qemu_opt_get(opts, "populated");
+        if (!buf) {
+            populated = 0;
+        } else {
+            populated = strcmp(buf, "on") ? 0 : 1;
+        }

vl.c static int chardev_init_func(QemuOpts *opts, void *opaque) · line 2380

Quoted 10 lines

vl.c +10 -0

+
+        dimm_config_create((char *)id, size, busbuf ? busbuf : "membus.0",
+                           node, nb_hp_dimms);
+
+        /* if !populated, we just keep the config. The real device
+         * will be created in the future with a normal device_add
+         * command. */
+        if (!populated) {
+            qemu_opts_del(opts);
+        }

I think you need another option than -device dimm. For example it could be declared together with the NUMA node. This could declare two NUMA nodes, each with 2G of populated and 2G of unpopulated RAM:

   -numa node,mem-range=0-2G,mem-range-hotplug=4G-6G \
   -numa node,mem-range=2G-4G,mem-range-hotplug=6G-8G

I'm not sure I like the names particularly though. CCing Eduardo, Bandan and Wanlong Gao.

Paolo

Quoted 22 lines

vl.c +11 -0

+        nb_hp_dimms++;
+    }
+
+    return 0;
+}
+
 #ifdef CONFIG_VIRTFS
 static int fsdev_init_func(QemuOpts *opts, void *opaque)
 {

@@ -4260,6 +4306,11 @@ int main(int argc, char **argv, char **envp)
     }
     qemu_add_globals();
 
+    /* init generic devices */
+    if (qemu_opts_foreach(qemu_find_opts("device"),
+           dimmcfg_init_func, NULL, 1) != 0) {
+        exit(1);
+    }
     qdev_machine_init();
 
     QEMUMachineInitArgs args = { .ram_size = ram_size,

Do we really need to specify the memory range? I suspect that we can follow current design of normal memory in hot-plug memory.

Currently,
we just specify the size of normal memory in each node, and the range
in normal memory is node by node. Then I think we can just specify
the memory size of hot-plug in each node, then the hot-plug memory
range is also node by node, and the whole hot-plug memory block is
just located after the normal memory block. If so, the option can
come like:
-numa node,nodeid=0,mem=2G,cpus=0-1,mem-hotplug=2G,mem-policy=membind,mem-hostnode=0-1,mem-hotplug-policy=interleave,mem-hotplug-hostnode=1 -numa node,nodeid=1,mem=2G,cpus=2-3,mem-hotplug=2G,mem-policy=preferred,mem-hostnode=1,mem-hotplug-policy=membind,mem-hotplug-hostnode=0-1

Il 27/06/2013 07:08, Wanlong Gao ha scritto:

Quoted 2 lines

Do we really need to specify the memory range? I suspect that we can follow current design of normal memory in hot-plug memory.

I think we can do both. I'm afraid that the configuration of the VM will not be perfectly reproducible without specifying the range, more so if you allow hotplug.

Quoted 9 lines

Currently,
we just specify the size of normal memory in each node, and the range
in normal memory is node by node. Then I think we can just specify
the memory size of hot-plug in each node, then the hot-plug memory
range is also node by node, and the whole hot-plug memory block is
just located after the normal memory block. If so, the option can
come like:
-numa node,nodeid=0,mem=2G,cpus=0-1,mem-hotplug=2G,mem-policy=membind,mem-hostnode=0-1,mem-hotplug-policy=interleave,mem-hotplug-hostnode=1 -numa node,nodeid=1,mem=2G,cpus=2-3,mem-hotplug=2G,mem-policy=preferred,mem-hostnode=1,mem-hotplug-policy=membind,mem-hotplug-hostnode=0-1

I think specifying different policies and bindings for normal and hotplug memory is too much fine-grained. If you really want that, then you would need something like

    -numa node,nodeid=0,cpus=0-1 \
    -numa mem,nodeid=0,size=2G,policy=membind,hostnode=0-1 \
    -numa mem,nodeid=0,size=2G,policy=interleave,hostnode=1,populated=no

Hmm... this actually doesn't look too bad, and it is much more future-proof. Eduardo, what do you think about it? Should Wanlong redo his patches to support this "-numa mem" syntax? Parsing it should be easy using the QemuOpts visitor, too.

Paolo

Quoted 22 lines · On Thu, 27 Jun 2013 08:55:25 +0200 Paolo Bonzini <[email protected]> wrote:

Il 27/06/2013 07:08, Wanlong Gao ha scritto:

Quoted 2 lines

Do we really need to specify the memory range? I suspect that we can follow current design of normal memory in hot-plug memory.

I think we can do both. I'm afraid that the configuration of the VM will not be perfectly reproducible without specifying the range, more so if you allow hotplug.

Quoted 9 lines

Currently,
we just specify the size of normal memory in each node, and the range
in normal memory is node by node. Then I think we can just specify
the memory size of hot-plug in each node, then the hot-plug memory
range is also node by node, and the whole hot-plug memory block is
just located after the normal memory block. If so, the option can
come like:
-numa node,nodeid=0,mem=2G,cpus=0-1,mem-hotplug=2G,mem-policy=membind,mem-hostnode=0-1,mem-hotplug-policy=interleave,mem-hotplug-hostnode=1 -numa node,nodeid=1,mem=2G,cpus=2-3,mem-hotplug=2G,mem-policy=preferred,mem-hostnode=1,mem-hotplug-policy=membind,mem-hotplug-hostnode=0-1

I think specifying different policies and bindings for normal and hotplug memory is too much fine-grained. If you really want that, then you would need something like

Quoted 8 lines

    -numa node,nodeid=0,cpus=0-1 \
    -numa mem,nodeid=0,size=2G,policy=membind,hostnode=0-1 \
    -numa mem,nodeid=0,size=2G,policy=interleave,hostnode=1,populated=no

Hmm... this actually doesn't look too bad, and it is much more future-proof. Eduardo, what do you think about it? Should Wanlong redo his patches to support this "-numa mem" syntax? Parsing it should be easy using the QemuOpts visitor, too.

Is hot-plugged memory and its bindings to numa nodes have to be specified at startup?

How about extending -m option to support following syntax:

-m initial_mem[, number_of_hotplug_dimms, max_mem]]

and dynamically forming MEMXX._CRS/_PXM resources on demand instead of creating static resources in SSDT.

without necessity to specify to be hot-plugged DIMMs at startup, hot-plug could become more flexible since arbitrarily sized DIMMs with required NUMA mapping could be specified during hot-plug time, for example:

device_add dimm,id=dimm1,bus=membus.0,size=3G,node=1,...

the only limit would be left is a number of increments(DIMM slots), due to need to pre-generate ACPI memory devices that could supply _CRS/_PXM resources later.

Quoted 3 lines

Paolo

Quoted 5 lines

without necessity to specify to be hot-plugged DIMMs at startup, hot-plug could become more flexible since arbitrarily sized DIMMs with required NUMA mapping could be specified during hot-plug time, for example:

device_add dimm,id=dimm1,bus=membus.0,size=3G,node=1,...

Seems impossible as ACPI memory devices' ranges must be specified at startup, but I'll be glad if I'm wrong.

Il 12/07/2013 04:39, Hu Tao ha scritto:

Quoted 6 lines

Quoted 5 lines

without necessity to specify to be hot-plugged DIMMs at startup, hot-plug could become more flexible since arbitrarily sized DIMMs with required NUMA mapping could be specified during hot-plug time, for example:

device_add dimm,id=dimm1,bus=membus.0,size=3G,node=1,...

Quoted 2 lines

Seems impossible as ACPI memory devices' ranges must be specified at startup, but I'll be glad if I'm wrong.

Yeah, I also understood the problem to be the SRAT, not the DSDT... Of course you could reserve a single large part of the address range in "-numa mem", and then add one or more DIMMs covering only part of the address range.

Perhaps "populated" could be a size instead of a boolean, to do something like what Igor suggested.

Paolo

Il 27/06/2013 07:08, Wanlong Gao ha scritto:

Quoted 2 lines

Do we really need to specify the memory range? I suspect that we can follow current design of normal memory in hot-plug memory.

I think we can do both. I'm afraid that the configuration of the VM will not be perfectly reproducible without specifying the range, more so if you allow hotplug.

Quoted 9 lines

Currently,
we just specify the size of normal memory in each node, and the range
in normal memory is node by node. Then I think we can just specify
the memory size of hot-plug in each node, then the hot-plug memory
range is also node by node, and the whole hot-plug memory block is
just located after the normal memory block. If so, the option can
come like:
-numa node,nodeid=0,mem=2G,cpus=0-1,mem-hotplug=2G,mem-policy=membind,mem-hostnode=0-1,mem-hotplug-policy=interleave,mem-hotplug-hostnode=1 -numa node,nodeid=1,mem=2G,cpus=2-3,mem-hotplug=2G,mem-policy=preferred,mem-hostnode=1,mem-hotplug-policy=membind,mem-hotplug-hostnode=0-1

I think specifying different policies and bindings for normal and hotplug memory is too much fine-grained. If you really want that, then you would need something like

    -numa node,nodeid=0,cpus=0-1 \
    -numa mem,nodeid=0,size=2G,policy=membind,hostnode=0-1 \
    -numa mem,nodeid=0,size=2G,policy=interleave,hostnode=1,populated=no

Hmm... this actually doesn't look too bad, and it is much more future-proof. Eduardo, what do you think about it? Should Wanlong redo his patches to support this "-numa mem" syntax? Parsing it should be easy using the QemuOpts visitor, too.

from what i understand, we are currently favoring this numa option? (I saw it mentioned in Gao's numa patchset series as well)

There is still the question of "how many hotpluggable dimm devices does this memory range describe?". With the dimm device that was clearly defined, but not so with this option. Do we choose a default granularity e.g. 1 GB?

Il 15/07/2013 19:05, Vasilis Liaskovitis ha scritto:

Quoted 2 lines

from what i understand, we are currently favoring this numa option? (I saw it mentioned in Gao's numa patchset series as well)

The two patchsets have some overlap, so it's good to find a design that fits both.

Quoted 3 lines

There is still the question of "how many hotpluggable dimm devices does this memory range describe?". With the dimm device that was clearly defined, but not so with this option. Do we choose a default granularity e.g. 1 GB?

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you

stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

Il 15/07/2013 19:05, Vasilis Liaskovitis ha scritto:

Quoted 2 lines

from what i understand, we are currently favoring this numa option? (I saw it mentioned in Gao's numa patchset series as well)

The two patchsets have some overlap, so it's good to find a design that fits both.

Quoted 3 lines

There is still the question of "how many hotpluggable dimm devices does this memory range describe?". With the dimm device that was clearly defined, but not so with this option. Do we choose a default granularity e.g. 1 GB?

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

I'm snipping the rest of the email because I hope this covers all your doubts.

Paolo

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

So we can drop -dimm if we agree on -numa mem?

Il 16/07/2013 03:27, Hu Tao ha scritto:

Quoted 6 lines

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

Quoted 1 line

So we can drop -dimm if we agree on -numa mem?

Yes, the point of the "-numa mem" proposal was to avoid the concept of a "partially initialized device" that you had for DIMMs.

BTW, how do you specify which module you are plugging in? I.e., what if I have three 1G ranges at 0, 1G and 2G, and I want to plug the first and the third?

This is especially important with hot-unplug, because then you can have this kind of hole in the address space. If you migrate the VM, you have to reproduce the situation in the destination command line.

Paolo

Quoted 12 lines · Paolo Bonzini

Il 16/07/2013 03:27, Hu Tao ha scritto:

Quoted 7 lines

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

So we can drop -dimm if we agree on -numa mem?

Yes, the point of the "-numa mem" proposal was to avoid the concept of a "partially initialized device" that you had for DIMMs.

Quoted 3 lines

BTW, how do you specify which module you are plugging in? I.e., what if I have three 1G ranges at 0, 1G and 2G, and I want to plug the first and the third?

I think an id is still needed to identify ranges, which can be shown to user with `info numa' or similar command, along with the corresponding ranges.

Quoted 8 lines

This is especially important with hot-unplug, because then you can have this kind of hole in the address space. If you migrate the VM, you have to reproduce the situation in the destination command line.

Paolo

Il 16/07/2013 03:27, Hu Tao ha scritto:

Quoted 7 lines

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

So we can drop -dimm if we agree on -numa mem?

Yes, the point of the "-numa mem" proposal was to avoid the concept of a "partially initialized device" that you had for DIMMs.

BTW, how do you specify which module you are plugging in? I.e., what if I have three 1G ranges at 0, 1G and 2G, and I want to plug the first and the third?
This is especially important with hot-unplug, because then you can have this kind of hole in the address space. If you migrate the VM, you have to reproduce the situation in the destination command line.

Paolo

Quoted 7 lines · On Tue, 16 Jul 2013 08:19:48 +0200 Paolo Bonzini <[email protected]> wrote:

Il 16/07/2013 03:27, Hu Tao ha scritto:

Quoted 6 lines

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

Quoted 4 lines

Quoted 1 line

So we can drop -dimm if we agree on -numa mem?

Yes, the point of the "-numa mem" proposal was to avoid the concept of a "partially initialized device" that you had for DIMMs.

I've though -numa mem was for mapping initial memory to numa nodes. It seem wrong to use it for representing dimm device and also limiting possible hotplugged regions to specified at startup ranges.

we can leave -numa for initial memory mapping and manage of the mapping of hotpluggable regions with -device dimm,node=X,size=Y.

It that case command line -device dimm will provide a fully initialized dimm device usable at startup (but hot-unplugable) and
(monitor) device_add dimm,,node=X,size=Y
would serve hot-plug case.

That way arbitrary sized dimm could be hot-pluged without specifying them at startup, like it's done on bare-metal.

In addition command line -device would be used in migration case to describe already hot-plugged dimms on target.

Il 16/07/2013 12:19, Igor Mammedov ha scritto:

Quoted 14 lines

Quoted 11 lines · On Tue, 16 Jul 2013 08:19:48 +0200 Paolo Bonzini <[email protected]> wrote:

Il 16/07/2013 03:27, Hu Tao ha scritto:

Quoted 7 lines

Quoted 5 lines

I think it's the same. One "-numa mem" option = one "-device dimm" option; both define one range. Unused memory ranges may remain if you stumble upon a unusable range such as the PCI window. For example two "-numa mem,size=2G" options would allocate memory from 0 to 2G and from 4 to 6G.

So we can drop -dimm if we agree on -numa mem?

Yes, the point of the "-numa mem" proposal was to avoid the concept of a "partially initialized device" that you had for DIMMs.

Quoted 3 lines

I've though -numa mem was for mapping initial memory to numa nodes. It seem wrong to use it for representing dimm device and also limiting possible hotplugged regions to specified at startup ranges.

It's not for DIMM devices, it is for reserving areas of the address space for hot-plugged RAM. DIMM hotplug is done with "device_add dimm" (and you can also use "-numa mem,populated=no,... -device dimm,..." to start a VM with hot-unpluggable memory).

Quoted 8 lines

we can leave -numa for initial memory mapping and manage of the mapping of hotpluggable regions with -device dimm,node=X,size=Y.

It that case command line -device dimm will provide a fully initialized dimm device usable at startup (but hot-unplugable) and
(monitor) device_add dimm,,node=X,size=Y
would serve hot-plug case.

Quoted 2 lines

That way arbitrary sized dimm could be hot-pluged without specifying them at startup, like it's done on bare-metal.

But the memory ranges need to be specified at startup in the ACPI tables, and that's what "-numa mem" is for.

Quoted 3 lines

In addition command line -device would be used in migration case to describe already hot-plugged dimms on target.

Yep.

Paolo

Quoted 8 lines

Quoted 8 lines

we can leave -numa for initial memory mapping and manage of the mapping of hotpluggable regions with -device dimm,node=X,size=Y.

It that case command line -device dimm will provide a fully initialized dimm device usable at startup (but hot-unplugable) and
(monitor) device_add dimm,,node=X,size=Y
would serve hot-plug case.

Quoted 5 lines

Quoted 2 lines

That way arbitrary sized dimm could be hot-pluged without specifying them at startup, like it's done on bare-metal.

But the memory ranges need to be specified at startup in the ACPI tables, and that's what "-numa mem" is for.

not really, there is caveat with windows, which needs a hotplugable SRAT entry that tells it max possible limit (otherwise windows sees new dimm device but refuses to use it saying "server is not configured for hotplug" or something like this), but as far as such entry exists, windows is happily uses dynamic _CRS() and _PXM() if they are below that limit (even if a new range is not in any range defined by SRAT).

And ACPI spec doesn't say that SRAT MUST be populated with hotplug ranges.

It's kind of simplier for bare-metal, where they might do it due to limited supported DIMM capacity by reserving static entries with max supported ranges per DIMM and know in advance DIMM count for platform. But actual _CRS() anyway dynamic since plugged in DIMM could have a smaller capacity then supported max for slot.

To summarize ACPI + windows limitations:
- ACPI needs to pre-allocate memory devices, i.e. number of possible increments OSPM could utilize. It might be possible to overcome limitation be using Load() or LoadTable() in runtime, but I haven't tried it.
- Windows needs to know max supported limit, a fake entry in SRAT from RamSize to max_mem works nicely there (tested with ws2008r2DC and ws2012DC).

That's why I was proposing to extend "-m" option for "slots" number (i.e. nr of memory devices) and 'max_mem' to make Windows happy and cap mgmt tools from going over initially configured limit.

then -device dimm could be used for hotpluggable mem available at startup and device_add fir adding more dimms with user defined sizes to desired nodes at runtime.

Works nice without any need for 'populated=xxx' and predefined ranges.

PS:
I'll be able to post more or less usable RFC that does it on top of mst's ACPI tables in QEMU by the end of this week.

Quoted 7 lines

Quoted 3 lines

In addition command line -device would be used in migration case to describe already hot-plugged dimms on target.

Yep.

Paolo

The numa_fw_cfg paravirt interface is extended to include SRAT information for all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, denoting start address, size and node proximity. The new info is appended after existing numa info, so that the fw_cfg layout does not break. This information is used by Seabios to build hotplug memory device objects at runtime. nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info to SeaBIOS.

v3->v4: numa_fw_cfg needs to be initalized after memory controller sets up dimm ranges. Make changes for pc_piix and pc_q35 to set numa_fw_cfg after i440fx initialization.

v2->v3: setting nb_numa_nodes to 1 is not needed

v1->v2:
Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not to break existing layout
Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

---
 docs/specs/fwcfg.txt    | 28 ++++++++++++++++++++++++++++
 hw/i386/pc.c            | 30 ++++++++++++++++++++++++------
 hw/i386/pc_piix.c       |  1 +
 hw/i386/pc_q35.c        |  7 +++++--
 include/hw/i386/pc.h    |  1 +
 include/sysemu/sysemu.h |  1 +
 6 files changed, 60 insertions(+), 8 deletions(-)
 create mode 100644 docs/specs/fwcfg.txt

docs/specs/fwcfg.txt new file +28 -0

new file mode 100644
index 0000000..e6fcd8f
@@ -0,0 +1,28 @@
+QEMU<->BIOS Paravirt Documentation
+--------------------------------------
+
+This document describes paravirt data structures passed from QEMU to BIOS.
+
+fw_cfg SRAT paravirt info
+--------------------
+The SRAT info passed from QEMU to BIOS has the following layout:
+
+-----------------------------------------------------------------------------------------------
+#nodes | cpu0_pxm | cpu1_pxm | ... | cpulast_pxm | node0_mem | node1_mem | ... | nodelast_mem
+
+-----------------------------------------------------------------------------------------------
+#dimms | dimm0_start | dimm0_sz | dimm0_pxm | ... | dimmlast_start | dimmlast_sz | dimmlast_pxm
+
+Entry 0 contains the number of numa nodes (nb_numa_nodes).
+
+Entries 1..max_cpus: The next max_cpus entries describe node proximity for each
+one of the vCPUs in the system.
+
+Entries max_cpus+1..max_cpus+nb_numa_nodes+1:  The next nb_numa_nodes entries
+describe the memory size for each one of the NUMA nodes in the system.
+
+Entry max_cpus+nb_numa_nodes+1 contains the number of memory dimms (nb_hp_dimms)
+
+The last 3 * nb_hp_dimms entries are organized in triplets: Each triplet contains
+the physical address offset, size (in bytes), and node proximity for the
+respective dimm.

hw/i386/pc.c +24 -6

index 65838a6..b51d3b5 100644
@@ -55,6 +55,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/cpu/icc_bus.h"
 #include "hw/boards.h"
+#include "hw/mem-hotplug/dimm.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ

@@ -606,8 +607,6 @@ static FWCfgState *bochs_bios_init(void)
     FWCfgState *fw_cfg;
     uint8_t *smbios_table;
     size_t smbios_len;
-    uint64_t *numa_fw_cfg;
-    int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);

@@ -640,11 +639,25 @@ static FWCfgState *bochs_bios_init(void)
                      &e820_table, sizeof(e820_table));
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+
+    return fw_cfg;
+}
+
+void bochs_meminfo_bios_init(void *fw_cfg)
+{
+    uint64_t *numa_fw_cfg;
+    uint64_t *hp_dimms_fw_cfg;
+    int i, j;
+    unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
+
     /* allocate memory for the NUMA channel: one (64bit) word for the number
      * of nodes, one word for each VCPU->node and one word for each node to
      * hold the amount of memory.
+     * Finally one word for the number of hotplug memory slots and three words
+     * for each hotplug memory slot (start address, size and node proximity).
      */
-    numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+    numa_fw_cfg = g_new0(uint64_t,
+                         2 + apic_id_limit + nb_numa_nodes  + 3 * nb_hp_dimms);
     numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);

@@ -659,11 +672,16 @@ static FWCfgState *bochs_bios_init(void)
     for (i = 0; i < nb_numa_nodes; i++) {
         numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
     }
+
+    numa_fw_cfg[1 + apic_id_limit + nb_numa_nodes] = cpu_to_le64(nb_hp_dimms);
+
+    hp_dimms_fw_cfg = numa_fw_cfg + 2 + apic_id_limit + nb_numa_nodes;
+    if (nb_hp_dimms) {
+        dimm_setup_fwcfg_layout(hp_dimms_fw_cfg);
+    }
     fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
-                     (1 + apic_id_limit + nb_numa_nodes) *
+                     (2 + apic_id_limit + nb_numa_nodes + 3 * nb_hp_dimms) *
                      sizeof(*numa_fw_cfg));
-
-    return fw_cfg;
 }
 
 static long get_file_size(FILE *f)

hw/i386/pc_piix.c +1 -0

index fb056df..6e18343 100644
@@ -138,6 +138,7 @@ static void pc_init1(MemoryRegion *system_memory,
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline, initrd_filename,
                                 below_4g_mem_size, above_4g_mem_size);
+        bochs_meminfo_bios_init(fw_cfg);
     }
 
     if (kvm_irqchip_in_kernel()) {

hw/i386/pc_q35.c +5 -2

index 5fe14bb..2c14977 100644
@@ -74,6 +74,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
     DeviceState *icc_bridge;
+    void *fw_cfg = NULL;
 
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",

@@ -97,8 +98,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size);
+        fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline,
+                                initrd_filename, below_4g_mem_size,
+                                above_4g_mem_size);
     }
 
     /* irq lines */

@@ -116,6 +118,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     q35_host->mch.address_space_io = get_system_io();
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
+    bochs_meminfo_bios_init(fw_cfg);
     host_bus = q35_host->host.pci.bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,

include/hw/i386/pc.h +1 -0

index 959b92b..4a29e6e 100644
@@ -231,6 +231,7 @@ int pvpanic_init(ISABus *bus);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+void bochs_meminfo_bios_init(void *fw_cfg);
 
 #define PC_COMPAT_1_5 \
         {\

include/sysemu/sysemu.h +1 -0

index 2fb71af..2644faa 100644
@@ -132,6 +132,7 @@ extern QEMUClock *rtc_clock;
 extern int nb_numa_nodes;
 extern uint64_t node_mem[MAX_NODES];
 extern unsigned long *node_cpumask[MAX_NODES];
+extern int nb_hp_dimms;
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

Signature · 1 line

1.8.3.1

Quoted 14 lines · Hu Tao

The numa_fw_cfg paravirt interface is extended to include SRAT information for all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, denoting start address, size and node proximity. The new info is appended after existing numa info, so that the fw_cfg layout does not break. This information is used by Seabios to build hotplug memory device objects at runtime. nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info to SeaBIOS.

v3->v4: numa_fw_cfg needs to be initalized after memory controller sets up dimm ranges. Make changes for pc_piix and pc_q35 to set numa_fw_cfg after i440fx initialization.

v2->v3: setting nb_numa_nodes to 1 is not needed

Quoted 7 lines

v1->v2:
Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not to break existing layout
Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

Please do not add any more fwcfg interfaces - generating ACPI in qemu removes the need for it.

So please rebase on top of that work and generate the appropriate ACPI tables directly.

You can find the latest code gnerating ACPI from qemu here:
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi

This code is work in progress, but once you base on top of that, I can put it on that branch and keep updating if interfaces change.

Quoted 183 lines

---
 docs/specs/fwcfg.txt    | 28 ++++++++++++++++++++++++++++
 hw/i386/pc.c            | 30 ++++++++++++++++++++++++------
 hw/i386/pc_piix.c       |  1 +
 hw/i386/pc_q35.c        |  7 +++++--
 include/hw/i386/pc.h    |  1 +
 include/sysemu/sysemu.h |  1 +
 6 files changed, 60 insertions(+), 8 deletions(-)
 create mode 100644 docs/specs/fwcfg.txt

docs/specs/fwcfg.txt new file +28 -0

new file mode 100644
index 0000000..e6fcd8f
@@ -0,0 +1,28 @@
+QEMU<->BIOS Paravirt Documentation
+--------------------------------------
+
+This document describes paravirt data structures passed from QEMU to BIOS.
+
+fw_cfg SRAT paravirt info
+--------------------
+The SRAT info passed from QEMU to BIOS has the following layout:
+
+-----------------------------------------------------------------------------------------------
+#nodes | cpu0_pxm | cpu1_pxm | ... | cpulast_pxm | node0_mem | node1_mem | ... | nodelast_mem
+
+-----------------------------------------------------------------------------------------------
+#dimms | dimm0_start | dimm0_sz | dimm0_pxm | ... | dimmlast_start | dimmlast_sz | dimmlast_pxm
+
+Entry 0 contains the number of numa nodes (nb_numa_nodes).
+
+Entries 1..max_cpus: The next max_cpus entries describe node proximity for each
+one of the vCPUs in the system.
+
+Entries max_cpus+1..max_cpus+nb_numa_nodes+1:  The next nb_numa_nodes entries
+describe the memory size for each one of the NUMA nodes in the system.
+
+Entry max_cpus+nb_numa_nodes+1 contains the number of memory dimms (nb_hp_dimms)
+
+The last 3 * nb_hp_dimms entries are organized in triplets: Each triplet contains
+the physical address offset, size (in bytes), and node proximity for the
+respective dimm.

hw/i386/pc.c +24 -6

index 65838a6..b51d3b5 100644
@@ -55,6 +55,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/cpu/icc_bus.h"
 #include "hw/boards.h"
+#include "hw/mem-hotplug/dimm.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ

@@ -606,8 +607,6 @@ static FWCfgState *bochs_bios_init(void)
     FWCfgState *fw_cfg;
     uint8_t *smbios_table;
     size_t smbios_len;
-    uint64_t *numa_fw_cfg;
-    int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);

@@ -640,11 +639,25 @@ static FWCfgState *bochs_bios_init(void)
                      &e820_table, sizeof(e820_table));
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+
+    return fw_cfg;
+}
+
+void bochs_meminfo_bios_init(void *fw_cfg)
+{
+    uint64_t *numa_fw_cfg;
+    uint64_t *hp_dimms_fw_cfg;
+    int i, j;
+    unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
+
     /* allocate memory for the NUMA channel: one (64bit) word for the number
      * of nodes, one word for each VCPU->node and one word for each node to
      * hold the amount of memory.
+     * Finally one word for the number of hotplug memory slots and three words
+     * for each hotplug memory slot (start address, size and node proximity).
      */
-    numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+    numa_fw_cfg = g_new0(uint64_t,
+                         2 + apic_id_limit + nb_numa_nodes  + 3 * nb_hp_dimms);
     numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);

@@ -659,11 +672,16 @@ static FWCfgState *bochs_bios_init(void)
     for (i = 0; i < nb_numa_nodes; i++) {
         numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
     }
+
+    numa_fw_cfg[1 + apic_id_limit + nb_numa_nodes] = cpu_to_le64(nb_hp_dimms);
+
+    hp_dimms_fw_cfg = numa_fw_cfg + 2 + apic_id_limit + nb_numa_nodes;
+    if (nb_hp_dimms) {
+        dimm_setup_fwcfg_layout(hp_dimms_fw_cfg);
+    }
     fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
-                     (1 + apic_id_limit + nb_numa_nodes) *
+                     (2 + apic_id_limit + nb_numa_nodes + 3 * nb_hp_dimms) *
                      sizeof(*numa_fw_cfg));
-
-    return fw_cfg;
 }
 
 static long get_file_size(FILE *f)

hw/i386/pc_piix.c +1 -0

index fb056df..6e18343 100644
@@ -138,6 +138,7 @@ static void pc_init1(MemoryRegion *system_memory,
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline, initrd_filename,
                                 below_4g_mem_size, above_4g_mem_size);
+        bochs_meminfo_bios_init(fw_cfg);
     }
 
     if (kvm_irqchip_in_kernel()) {

hw/i386/pc_q35.c +5 -2

index 5fe14bb..2c14977 100644
@@ -74,6 +74,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
     DeviceState *icc_bridge;
+    void *fw_cfg = NULL;
 
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",

@@ -97,8 +98,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size);
+        fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline,
+                                initrd_filename, below_4g_mem_size,
+                                above_4g_mem_size);
     }
 
     /* irq lines */

@@ -116,6 +118,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     q35_host->mch.address_space_io = get_system_io();
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
+    bochs_meminfo_bios_init(fw_cfg);
     host_bus = q35_host->host.pci.bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,

include/hw/i386/pc.h +1 -0

index 959b92b..4a29e6e 100644
@@ -231,6 +231,7 @@ int pvpanic_init(ISABus *bus);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+void bochs_meminfo_bios_init(void *fw_cfg);
 
 #define PC_COMPAT_1_5 \
         {\

include/sysemu/sysemu.h +1 -0

index 2fb71af..2644faa 100644
@@ -132,6 +132,7 @@ extern QEMUClock *rtc_clock;
 extern int nb_numa_nodes;
 extern uint64_t node_mem[MAX_NODES];
 extern unsigned long *node_cpumask[MAX_NODES];
+extern int nb_hp_dimms;
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

Signature · 1 line

1.8.3.1

Quoted 26 lines · On Wed, 10 Jul 2013 13:10:03 +0300 "Michael S. Tsirkin" <[email protected]> wrote:

Quoted 14 lines · Hu Tao

The numa_fw_cfg paravirt interface is extended to include SRAT information for all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, denoting start address, size and node proximity. The new info is appended after existing numa info, so that the fw_cfg layout does not break. This information is used by Seabios to build hotplug memory device objects at runtime. nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info to SeaBIOS.

v3->v4: numa_fw_cfg needs to be initalized after memory controller sets up dimm ranges. Make changes for pc_piix and pc_q35 to set numa_fw_cfg after i440fx initialization.

v2->v3: setting nb_numa_nodes to 1 is not needed

Quoted 7 lines

v1->v2:
Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not to break existing layout
Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

Please do not add any more fwcfg interfaces - generating ACPI in qemu removes the need for it.

Quoted 5 lines

So please rebase on top of that work and generate the appropriate ACPI tables directly.

You can find the latest code gnerating ACPI from qemu here:
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi

will it work with upstream Seabios or custom tree is required for it as well?

Quoted 189 lines

This code is work in progress, but once you base on top of that, I can put it on that branch and keep updating if interfaces change.

Quoted 183 lines

---
 docs/specs/fwcfg.txt    | 28 ++++++++++++++++++++++++++++
 hw/i386/pc.c            | 30 ++++++++++++++++++++++++------
 hw/i386/pc_piix.c       |  1 +
 hw/i386/pc_q35.c        |  7 +++++--
 include/hw/i386/pc.h    |  1 +
 include/sysemu/sysemu.h |  1 +
 6 files changed, 60 insertions(+), 8 deletions(-)
 create mode 100644 docs/specs/fwcfg.txt

docs/specs/fwcfg.txt new file +28 -0

new file mode 100644
index 0000000..e6fcd8f
@@ -0,0 +1,28 @@
+QEMU<->BIOS Paravirt Documentation
+--------------------------------------
+
+This document describes paravirt data structures passed from QEMU to BIOS.
+
+fw_cfg SRAT paravirt info
+--------------------
+The SRAT info passed from QEMU to BIOS has the following layout:
+
+-----------------------------------------------------------------------------------------------
+#nodes | cpu0_pxm | cpu1_pxm | ... | cpulast_pxm | node0_mem | node1_mem | ... | nodelast_mem
+
+-----------------------------------------------------------------------------------------------
+#dimms | dimm0_start | dimm0_sz | dimm0_pxm | ... | dimmlast_start | dimmlast_sz | dimmlast_pxm
+
+Entry 0 contains the number of numa nodes (nb_numa_nodes).
+
+Entries 1..max_cpus: The next max_cpus entries describe node proximity for each
+one of the vCPUs in the system.
+
+Entries max_cpus+1..max_cpus+nb_numa_nodes+1:  The next nb_numa_nodes entries
+describe the memory size for each one of the NUMA nodes in the system.
+
+Entry max_cpus+nb_numa_nodes+1 contains the number of memory dimms (nb_hp_dimms)
+
+The last 3 * nb_hp_dimms entries are organized in triplets: Each triplet contains
+the physical address offset, size (in bytes), and node proximity for the
+respective dimm.

hw/i386/pc.c +24 -6

index 65838a6..b51d3b5 100644
@@ -55,6 +55,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/cpu/icc_bus.h"
 #include "hw/boards.h"
+#include "hw/mem-hotplug/dimm.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ

@@ -606,8 +607,6 @@ static FWCfgState *bochs_bios_init(void)
     FWCfgState *fw_cfg;
     uint8_t *smbios_table;
     size_t smbios_len;
-    uint64_t *numa_fw_cfg;
-    int i, j;
     unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);

@@ -640,11 +639,25 @@ static FWCfgState *bochs_bios_init(void)
                      &e820_table, sizeof(e820_table));
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_cfg, sizeof(hpet_cfg));
+
+    return fw_cfg;
+}
+
+void bochs_meminfo_bios_init(void *fw_cfg)
+{
+    uint64_t *numa_fw_cfg;
+    uint64_t *hp_dimms_fw_cfg;
+    int i, j;
+    unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
+
     /* allocate memory for the NUMA channel: one (64bit) word for the number
      * of nodes, one word for each VCPU->node and one word for each node to
      * hold the amount of memory.
+     * Finally one word for the number of hotplug memory slots and three words
+     * for each hotplug memory slot (start address, size and node proximity).
      */
-    numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+    numa_fw_cfg = g_new0(uint64_t,
+                         2 + apic_id_limit + nb_numa_nodes  + 3 * nb_hp_dimms);
     numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
     for (i = 0; i < max_cpus; i++) {
         unsigned int apic_id = x86_cpu_apic_id_from_index(i);

@@ -659,11 +672,16 @@ static FWCfgState *bochs_bios_init(void)
     for (i = 0; i < nb_numa_nodes; i++) {
         numa_fw_cfg[apic_id_limit + 1 + i] = cpu_to_le64(node_mem[i]);
     }
+
+    numa_fw_cfg[1 + apic_id_limit + nb_numa_nodes] = cpu_to_le64(nb_hp_dimms);
+
+    hp_dimms_fw_cfg = numa_fw_cfg + 2 + apic_id_limit + nb_numa_nodes;
+    if (nb_hp_dimms) {
+        dimm_setup_fwcfg_layout(hp_dimms_fw_cfg);
+    }
     fw_cfg_add_bytes(fw_cfg, FW_CFG_NUMA, numa_fw_cfg,
-                     (1 + apic_id_limit + nb_numa_nodes) *
+                     (2 + apic_id_limit + nb_numa_nodes + 3 * nb_hp_dimms) *
                      sizeof(*numa_fw_cfg));
-
-    return fw_cfg;
 }
 
 static long get_file_size(FILE *f)

hw/i386/pc_piix.c +1 -0

index fb056df..6e18343 100644
@@ -138,6 +138,7 @@ static void pc_init1(MemoryRegion *system_memory,
     if (!xen_enabled()) {
         fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline, initrd_filename,
                                 below_4g_mem_size, above_4g_mem_size);
+        bochs_meminfo_bios_init(fw_cfg);
     }
 
     if (kvm_irqchip_in_kernel()) {

hw/i386/pc_q35.c +5 -2

index 5fe14bb..2c14977 100644
@@ -74,6 +74,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     ICH9LPCState *ich9_lpc;
     PCIDevice *ahci;
     DeviceState *icc_bridge;
+    void *fw_cfg = NULL;
 
     icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
     object_property_add_child(qdev_get_machine(), "icc-bridge",

@@ -97,8 +98,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 
     /* allocate ram and load rom/bios */
     if (!xen_enabled()) {
-        pc_memory_init(kernel_filename, kernel_cmdline,
-                       initrd_filename, below_4g_mem_size, above_4g_mem_size);
+        fw_cfg = pc_memory_init(kernel_filename, kernel_cmdline,
+                                initrd_filename, below_4g_mem_size,
+                                above_4g_mem_size);
     }
 
     /* irq lines */

@@ -116,6 +118,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
     q35_host->mch.address_space_io = get_system_io();
     /* pci */
     qdev_init_nofail(DEVICE(q35_host));
+    bochs_meminfo_bios_init(fw_cfg);
     host_bus = q35_host->host.pci.bus;
     /* create ISA bus */
     lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV,

include/hw/i386/pc.h +1 -0

index 959b92b..4a29e6e 100644
@@ -231,6 +231,7 @@ int pvpanic_init(ISABus *bus);
 #define E820_UNUSABLE   5
 
 int e820_add_entry(uint64_t, uint64_t, uint32_t);
+void bochs_meminfo_bios_init(void *fw_cfg);
 
 #define PC_COMPAT_1_5 \
         {\

include/sysemu/sysemu.h +1 -0

index 2fb71af..2644faa 100644
@@ -132,6 +132,7 @@ extern QEMUClock *rtc_clock;
 extern int nb_numa_nodes;
 extern uint64_t node_mem[MAX_NODES];
 extern unsigned long *node_cpumask[MAX_NODES];
+extern int nb_hp_dimms;
 
 #define MAX_OPTION_ROMS 16
 typedef struct QEMUOptionRom {

Signature · 1 line

1.8.3.1

Quoted 32 lines · Igor Mammedov

Quoted 29 lines · On Wed, 10 Jul 2013 13:10:03 +0300 "Michael S. Tsirkin" <[email protected]> wrote:

Quoted 14 lines · Hu Tao

The numa_fw_cfg paravirt interface is extended to include SRAT information for all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, denoting start address, size and node proximity. The new info is appended after existing numa info, so that the fw_cfg layout does not break. This information is used by Seabios to build hotplug memory device objects at runtime. nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info to SeaBIOS.

v3->v4: numa_fw_cfg needs to be initalized after memory controller sets up dimm ranges. Make changes for pc_piix and pc_q35 to set numa_fw_cfg after i440fx initialization.

v2->v3: setting nb_numa_nodes to 1 is not needed

Quoted 7 lines

v1->v2:
Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not to break existing layout
Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

Please do not add any more fwcfg interfaces - generating ACPI in qemu removes the need for it.

So please rebase on top of that work and generate the appropriate ACPI tables directly.

Quoted 4 lines

Quoted 2 lines

You can find the latest code gnerating ACPI from qemu here:
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi

will it work with upstream Seabios or custom tree is required for it as well?

Yes.

git://git.kernel.org/pub/scm/virt/kvm/mst/seabios.git acpi

Quoted 36 lines · Michael S. Tsirkin

Quoted 35 lines · Igor Mammedov

Quoted 31 lines · On Wed, 10 Jul 2013 13:10:03 +0300 "Michael S. Tsirkin" <[email protected]> wrote:

Quoted 14 lines · Hu Tao

The numa_fw_cfg paravirt interface is extended to include SRAT information for all hotplug-able dimms. There are 3 words for each hotplug-able memory slot, denoting start address, size and node proximity. The new info is appended after existing numa info, so that the fw_cfg layout does not break. This information is used by Seabios to build hotplug memory device objects at runtime. nb_numa_nodes is set to 1 by default (not 0), so that we always pass srat info to SeaBIOS.

v3->v4: numa_fw_cfg needs to be initalized after memory controller sets up dimm ranges. Make changes for pc_piix and pc_q35 to set numa_fw_cfg after i440fx initialization.

v2->v3: setting nb_numa_nodes to 1 is not needed

Quoted 7 lines

v1->v2:
Dimm SRAT info (#dimms) is appended at end of existing numa fw_cfg in order not to break existing layout
Documentation of the new fwcfg layout is included in docs/specs/fwcfg.txt

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

Please do not add any more fwcfg interfaces - generating ACPI in qemu removes the need for it.

So please rebase on top of that work and generate the appropriate ACPI tables directly.

You can find the latest code gnerating ACPI from qemu here:
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi

Quoted 5 lines

Quoted 1 line

will it work with upstream Seabios or custom tree is required for it as well?

Yes.

git://git.kernel.org/pub/scm/virt/kvm/mst/seabios.git acpi

This means I must rebase both qemu and seabios patches on top of your acpi tree. I hope there won't be too much conflicts:)

From: Vasilis Liaskovitis <[email protected]>

Returns total physical memory available to guest in bytes, including hotplugged memory. Note that the number reported here may be different from what the guest sees e.g. if the guest has not logically onlined hotplugged memory.

This functionality is provided independently of a balloon device, since a guest can be using ACPI memory hotplug without using a balloon device.

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

---

+++ b/qapi-schema.json

From: Vasilis Liaskovitis <[email protected]>

"query-dimm-info" and "info dimm" will give current state of all dimms in the system e.g.

+++ b/qapi-schema.json

qapi-schema.json +0 -0

@@ -3621,3 +3621,30 @@
 { 'type': 'MemoryInfo',
   'data': { 'total': 'int' } }
 { 'command': 'query-memory', 'returns': 'MemoryInfo' }

qapi-schema.json +7 -0

+#
+# Since: 1.6
+#
+##
+{ 'type': 'DimmInfo',
+  'data': {'dimm': 'str', 'state': 'bool'} }
+

The memory device generation is guided by qemu paravirt info. Seabios uses the info to setup SRAT entries for the hotplug-able memory slots, and to generate appropriate memory device objects. One memory device (and corresponding SRAT entry) is generated for each hotplug-able qemu memslot. Currently no SSDT memory device is created for initial system memory.

We only support up to 255 DIMMs for now (PackageOp used for the MEON array can only describe an array of at most 255 elements. VarPackageOp would be needed to support more than 255 devices)

Signed-off-by Vasilis Liaskovitis <[email protected]>

Signed-off-by Hu Tao <[email protected]>

---
 src/acpi.c     | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 src/paravirt.c |   8 +++
 2 files changed, 152 insertions(+), 7 deletions(-)

src/acpi.c +31 -0

index ce988e0..e9a0326 100644
@@ -15,6 +15,8 @@
 #include "config.h" // CONFIG_*
 #include "paravirt.h" // RamSize
 #include "dev-q35.h"
+#include "memmap.h"
+#include "paravirt.h"
 
 #include "acpi-dsdt.hex"
 

@@ -250,11 +252,23 @@ encodeLen(u8 *ssdt_ptr, int length, int bytes)
 #define PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
 #define PCI_SLOTS 32
 
+/* 0x5B 0x82 DeviceOp PkgLength NameString DimmID */
+#define MEM_BASE 0xaf80
+#define MEM_AML (ssdm_mem_aml + *ssdt_mem_start)
+#define MEM_SIZEOF (*ssdt_mem_end - *ssdt_mem_start)
+#define MEM_OFFSET_HEX (*ssdt_mem_name - *ssdt_mem_start + 2)
+#define MEM_OFFSET_ID (*ssdt_mem_id - *ssdt_mem_start)
+#define MEM_OFFSET_PXM 31
+#define MEM_OFFSET_START 55
+#define MEM_OFFSET_END   63
+#define MEM_OFFSET_SIZE  79
+
 #define SSDT_SIGNATURE 0x54445353 // SSDT
 #define SSDT_HEADER_LENGTH 36
 
 #include "ssdt-misc.hex"
 #include "ssdt-pcihp.hex"
+#include "ssdt-mem.hex"
 
 #define PCI_RMV_BASE 0xae0c
 

@@ -306,9 +320,100 @@ static void patch_pcihp(int slot, u8 *ssdt_ptr, u32 eject)
     }
 }
 
+static void build_memdev(u8 *ssdt_ptr, int i, u64 mem_base, u64 mem_len, u8 node)
+{
+    memcpy(ssdt_ptr, MEM_AML, MEM_SIZEOF);
+    ssdt_ptr[MEM_OFFSET_HEX] = getHex(i >> 4);
+    ssdt_ptr[MEM_OFFSET_HEX+1] = getHex(i);
+    ssdt_ptr[MEM_OFFSET_ID] = i;
+    ssdt_ptr[MEM_OFFSET_PXM] = node;
+    *(u64*)(ssdt_ptr + MEM_OFFSET_START) = cpu_to_le64(mem_base);
+    *(u64*)(ssdt_ptr + MEM_OFFSET_END) = cpu_to_le64(mem_base + mem_len);
+    *(u64*)(ssdt_ptr + MEM_OFFSET_SIZE) = cpu_to_le64(mem_len);
+}
+
+static u8 *build_memssdt(u8 *ssdt_ptr, int memssdt_len,
+                         u64 *numadimmsmap, int nb_memdevs)
+{
+    u64 mem_base, mem_len;
+    u64 *dimm = numadimmsmap;

src/acpi.c +75 -1

+    for (i = 0; i < nb_memdevs; i++) {
+        mem_base = *dimm++;
+        mem_len = *dimm++;
+        node = *dimm++;
+        build_memdev(ssdt_ptr, i, mem_base, mem_len, node);
+        ssdt_ptr += MEM_SIZEOF;
+    }
+
+    // build "Method(MTFY, 2) {If (LEqual(Arg0, 0x00)) {Notify(CM00, Arg1)} ...}"
+    *(ssdt_ptr++) = 0x14; // MethodOp
+    ssdt_ptr = encodeLen(ssdt_ptr, 2+5+(12*nb_memdevs), 2);
+    *(ssdt_ptr++) = 'M';
+    *(ssdt_ptr++) = 'T';
+    *(ssdt_ptr++) = 'F';
+    *(ssdt_ptr++) = 'Y';
+    *(ssdt_ptr++) = 0x02;
+    for (i=0; i<nb_memdevs; i++) {
+        *(ssdt_ptr++) = 0xA0; // IfOp
+        ssdt_ptr = encodeLen(ssdt_ptr, 11, 1);
+        *(ssdt_ptr++) = 0x93; // LEqualOp
+        *(ssdt_ptr++) = 0x68; // Arg0Op
+        *(ssdt_ptr++) = 0x0A; // BytePrefix
+        *(ssdt_ptr++) = i;
+        *(ssdt_ptr++) = 0x86; // NotifyOp
+        *(ssdt_ptr++) = 'M';
+        *(ssdt_ptr++) = 'P';
+        *(ssdt_ptr++) = getHex(i >> 4);
+        *(ssdt_ptr++) = getHex(i);
+        *(ssdt_ptr++) = 0x69; // Arg1Op
+    }
+
+    // build "Name(MEON, Package() { One, One, ..., Zero, Zero, ... })"
+    *(ssdt_ptr++) = 0x08; // NameOp
+    *(ssdt_ptr++) = 'M';
+    *(ssdt_ptr++) = 'E';
+    *(ssdt_ptr++) = 'O';
+    *(ssdt_ptr++) = 'N';
+    *(ssdt_ptr++) = 0x12; // PackageOp
+    ssdt_ptr = encodeLen(ssdt_ptr, 2+1+(1*nb_memdevs), 2);
+    *(ssdt_ptr++) = nb_memdevs;
+
+    dimm = numadimmsmap;
+    u8 memslot_status = 0, enabled;
+
+    for (i = 0; i < nb_memdevs; i++) {
+        enabled = 0;
+        if (i % 8 == 0)
+            memslot_status = inb(MEM_BASE + i/8);
+        enabled = memslot_status & 1;
+        mem_base = *dimm++;
+        mem_len = *dimm++;
+        dimm++;  // node
+        *(ssdt_ptr++) = enabled ? 0x01 : 0x00;
+        if (enabled)
+            add_e820(mem_base, mem_len, E820_RAM);
+        memslot_status = memslot_status >> 1;
+    }
+
+    return ssdt_ptr;
+}
+
 static void*
 build_ssdt(void)
 {
+    int nb_memdevs;
+    u64 *numadimmsmap;
     int acpi_cpus = MaxCountCPUs > 0xff ? 0xff : MaxCountCPUs;
     int length = (sizeof(ssdp_misc_aml)                     // _S3_ / _S4_ / _S5_
                   + (1+3+4)                                 // Scope(_SB_)

@@ -318,9 +423,20 @@ build_ssdt(void)
                   + (1+3+4)                                 // Scope(PCI0)
                   + ((PCI_SLOTS - 1) * PCIHP_SIZEOF)        // slots
                   + (1+2+5+(12*(PCI_SLOTS - 1))));          // PCNT
-    u8 *ssdt = malloc_high(length);
+
+    numadimmsmap = romfile_loadfile("etc/numa-dimm-map", &nb_memdevs);
+    nb_memdevs /= 3 * sizeof(u64);
+
+    // for build_memssdt
+    int memssdt_length = (1+3+4)
+                         + (nb_memdevs * MEM_SIZEOF)
+                         + (1+2+5+(12*nb_memdevs))
+                         + (6+2+1+(1*nb_memdevs));
+
+    u8 *ssdt = malloc_high(length + memssdt_length);
     if (! ssdt) {
         warn_noalloc();
+        free(numadimmsmap);
         return NULL;
     }
     u8 *ssdt_ptr = ssdt;

src/acpi.c +26 -6

+
     build_header((void*)ssdt, SSDT_SIGNATURE, ssdt_ptr - ssdt, 1);
 
     //hexdump(ssdt, ssdt_ptr - ssdt);
 
+    free(numadimmsmap);
     return ssdt;
 }
 

@@ -458,7 +577,7 @@ acpi_build_srat_memory(struct srat_memory_affinity *numamem,
     numamem->length = sizeof(*numamem);
     memset(numamem->proximity, 0, 4);
     numamem->proximity[0] = node;
-    numamem->flags = cpu_to_le32(!!enabled);
+    numamem->flags = cpu_to_le32(!!enabled) | cpu_to_le32(0x2);
     numamem->base_addr = cpu_to_le64(base);
     numamem->range_length = cpu_to_le64(len);
 }

@@ -466,18 +585,22 @@ acpi_build_srat_memory(struct srat_memory_affinity *numamem,
 static void *
 build_srat(void)
 {
-    int numadatasize, numacpusize;
+    int numadatasize, numacpusize, nb_numa_dimms;
+    u64 *numadimmsmap;
     u64 *numadata = romfile_loadfile("etc/numa-nodes", &numadatasize);
     u64 *numacpumap = romfile_loadfile("etc/numa-cpu-map", &numacpusize);
-    if (!numadata || !numacpumap)
-        goto fail;
+
     int max_cpu = numacpusize / sizeof(u64);
     int nb_numa_nodes = numadatasize / sizeof(u64);
 
+    numadimmsmap = romfile_loadfile("etc/numa-dimm-map", &nb_numa_dimms);
+
+    nb_numa_dimms /=  3 * sizeof(u64);
+
     struct system_resource_affinity_table *srat;
     int srat_size = sizeof(*srat) +
         sizeof(struct srat_processor_affinity) * max_cpu +
-        sizeof(struct srat_memory_affinity) * (nb_numa_nodes + 2);
+        sizeof(struct srat_memory_affinity) * (nb_numa_nodes + nb_numa_dimms + 2);
 
     srat = malloc_high(srat_size);
     if (!srat) {

@@ -512,6 +635,7 @@ build_srat(void)
      */
     struct srat_memory_affinity *numamem = (void*)core;
     int slots = 0;
+    int node;
     u64 mem_len, mem_base, next_base = 0;
 
     acpi_build_srat_memory(numamem, 0, 640*1024, 0, 1);

@@ -541,7 +665,18 @@ build_srat(void)
         numamem++;
         slots++;
     }
-    for (; slots < nb_numa_nodes + 2; slots++) {
+    if (nb_numa_dimms) {
+        for (i = 1; i < nb_numa_dimms + 1; ++i) {
+            mem_base = *numadimmsmap++;
+            mem_len = *numadimmsmap++;
+            node = *numadimmsmap++;
+            acpi_build_srat_memory(numamem, mem_base, mem_len, node, 1);
+            numamem++;
+            slots++;
+        }
+    }
+
+    for (; slots < nb_numa_nodes + nb_numa_dimms + 2; slots++) {
         acpi_build_srat_memory(numamem, 0, 0, 0, 0);
         numamem++;
     }

@@ -550,10 +685,12 @@ build_srat(void)
 
     free(numadata);
     free(numacpumap);
+    free(numadimmsmap);
     return srat;
 fail:
     free(numadata);
     free(numacpumap);
+    free(numadimmsmap);
     return NULL;
 }
 

src/paravirt.c +8 -0

index d1a5d3e..5925c63 100644
@@ -240,6 +240,14 @@ qemu_cfg_legacy(void)
                      , sizeof(numacount) + max_cpu*sizeof(u64)
                      , numacount*sizeof(u64));
 
+    u64 dimm_count;
+    qemu_cfg_select(QEMU_CFG_NUMA);
+    qemu_cfg_skip((1 + max_cpu + numacount) * sizeof(u64));
+    qemu_cfg_read(&dimm_count, sizeof(dimm_count));
+    qemu_romfile_add("etc/numa-dimm-map", QEMU_CFG_NUMA
+                     , (2 + max_cpu + numacount) * sizeof(u64),
+                     dimm_count * 3 * sizeof(u64));
+
     // e820 data
     u32 count32;
     qemu_cfg_read_entry(&count32, QEMU_CFG_E820_TABLE, sizeof(count32));

From: Vasilis Liaskovitis <[email protected]>

Initialize the 32-bit and 64-bit pci starting offsets from values passed in by
the qemu paravirt interface QEMU_CFG_PCI_WINDOW. Qemu calculates the starting
offsets based on initial memory and hotplug-able dimms.

It's possible to avoid the new paravirt interface, and calculate pci ranges from
srat entries. But the code changes are ugly, see:
http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg03548.html

Signed-off-by Vasilis Liaskovitis <[email protected]>

---
 src/paravirt.c | 7 +++++++
 src/paravirt.h | 1 +
 src/pciinit.c  | 9 +++++++++
 3 files changed, 17 insertions(+)

src/paravirt.c +7 -0

index 5925c63..9c1e511 100644
@@ -134,6 +134,7 @@ qemu_platform_setup(void)
 #define QEMU_CFG_BOOT_MENU              0x0e
 #define QEMU_CFG_MAX_CPUS               0x0f
 #define QEMU_CFG_FILE_DIR               0x19
+#define QEMU_CFG_PCI_WINDOW             0x1a
 #define QEMU_CFG_ARCH_LOCAL             0x8000
 #define QEMU_CFG_ACPI_TABLES            (QEMU_CFG_ARCH_LOCAL + 0)
 #define QEMU_CFG_SMBIOS_ENTRIES         (QEMU_CFG_ARCH_LOCAL + 1)

@@ -339,3 +340,9 @@ void qemu_cfg_init(void)
                          , 0, be32_to_cpu(qfile.size));
     }
 }
+
+void qemu_cfg_get_pci_offsets(u64 *pcimem_start, u64 *pcimem64_start)
+{
+    qemu_cfg_read_entry(pcimem_start, QEMU_CFG_PCI_WINDOW, sizeof(u64));
+    qemu_cfg_read((u8*)(pcimem64_start), sizeof(u64));
+}

src/paravirt.h +1 -0

index fce5af9..2c37d0d 100644
@@ -27,5 +27,6 @@ static inline int runningOnKVM(void) {
 void qemu_preinit(void);
 void qemu_platform_setup(void);
 void qemu_cfg_init(void);
+void qemu_cfg_get_pci_offsets(u64 *pcimem_start, u64 *pcimem64_start);
 
 #endif

src/pciinit.c +2 -0

index 8370b96..7e63c5e 100644
@@ -805,6 +805,7 @@ static void pci_bios_map_devices(struct pci_bus
 *busses)
 void
 pci_setup(void)
 {
+    u64 pv_pcimem_start, pv_pcimem64_start;
     if (!CONFIG_QEMU)
         return;

@@ -837,6 +838,14 @@ pci_setup(void)
 
     pci_bios_init_devices();
 
+    /* if qemu gives us other pci window values, it means there are

hotplug-able

src/pciinit.c +7 -1

+     * dimms. Adjust accordingly */
+    qemu_cfg_get_pci_offsets(&pv_pcimem_start, &pv_pcimem64_start);
+    if (pv_pcimem_start > pcimem_start)
+        pcimem_start = pv_pcimem_start;
+    if (pv_pcimem64_start > pcimem64_start)
+        pcimem64_start = pv_pcimem64_start;
+
     free(busses);
 
     pci_enable_default_vga();
--

1.8.3.1