On Tue, Jul 01, 2014 at 01:13:28PM -0700, Nishanth Aravamudan wrote:
[...]
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 12472c6..cdefafe 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1121,6 +1121,18 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t
> below_4g_mem_size,
> guest_info->ram_size = below_4g_mem_size + above_4g_mem_size;
> guest_info->apic_id_limit = pc_apic_id_limit(max_cpus);
> guest_info->apic_xrupt_override = kvm_allows_irq0_override();
> + /* No support for sparse NUMA node IDs yet: */
> + for (i = max_numa_nodeid - 1; i >= 0; i--) {
> + /* Report large node IDs first, to make mistakes easier to spot */
> + if (!numa_info[i].present) {
> + error_report("numa: Node ID missing: %d", i);
> + exit(EXIT_FAILURE);
> + }
> + }
> +
> + /* This must be always true if all nodes are present */
> + assert(num_numa_nodes == max_numa_nodeid);
> +
I wonder if there's a better place where we could put this check.
> guest_info->numa_nodes = num_numa_nodes;
> guest_info->node_mem = g_malloc0(guest_info->numa_nodes *
> sizeof *guest_info->node_mem);
[...]
> diff --git a/numa.c b/numa.c
> index 5930df0..a689e52 100644
> --- a/numa.c
> +++ b/numa.c
[...]
> @@ -225,9 +220,12 @@ void set_numa_nodes(void)
> * must cope with this anyway, because there are BIOSes out there in
> * real machines which also use this scheme.
> */
> - if (i == num_numa_nodes) {
> - for (i = 0; i < max_cpus; i++) {
> - set_bit(i, numa_info[i % num_numa_nodes].node_cpu);
> + if (i == max_numa_nodeid) {
> + for (i = 0, j = 0; i < max_cpus; i++) {
Doesn't j need to be initialized to -1, here?
Except for that, patch looks good to me. But I would be more comfortable
with it if we had automated tests to help ensure we are not breaking
compatibility of existing NUMA command-line conbinations with these
changes.
> + do {
> + j = (j + 1) % (max_numa_nodeid);
> + } while (!numa_info[j].present);
> + set_bit(i, numa_info[j].node_cpu);
> }
> }
> }
[...]
>
--
Eduardo
