Re: [Qemu-devel] [PATCH v5 05/14] vl: handle "-device dimm"

[Wanlong Gao]

On 06/26/2013 05:46 PM, Paolo Bonzini wrote:
> Il 26/06/2013 11:13, Hu Tao ha scritto:
>> From: Vasilis Liaskovitis <address@hidden>
>>
>> Signed-off-by: Vasilis Liaskovitis <address@hidden>
>> Signed-off-by: Hu Tao <address@hidden>
>> ---
>>  vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/vl.c b/vl.c
>> index 767e020..9d88a79 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -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;
>> +        }
>> +
>> +        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.

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

And each hot-plug memory device size can be assigned through "-device 
dimm,size=1G",
assume that we specify 4 hot-plug memory devices and each 1G, then first two 
devices
as we ranged belong to node0, and other two belong to node1. Then the hot-plug 
memory
will have no effect on current normal memory design.


Thanks,
Wanlong Gao

> 
> Paolo
> 
>> +        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,
>>
> 
>