Re: [Qemu-devel] [PATCH v4 1/2] hw/acpi-build: Fix SRAT memory building when there is no memory in node0

[Dou Liyang]

Hi Eduardo,

[...]
> > +            continue;
> >          }
>
> Now the nodes will be out of order, if node 0 has no RAM.  Why?

Because the code parsed the other node with RAM first, then parsed the 
node 0.

> Why not implement this in the same way the PCI 4GB hole is
> already implemented.  e.g.:

Indeed, it is better and more refined. I will do it in the next version.

> #define HOLE_640K_START  (640 * 1024)
> #define HOLE_640K_END   (1024 * 1024)
>
>     for (node = 0; node < pcms->numa_nodes; node++) {
>         mem_base = next_base;
>         mem_len = pcms->node_mem[node];
>         next_base = mem_base + mem_len;
>
>         /* Cut out the 640K hole */
>         if (mem_base <= HOLE_640K_START &&
>             next_base > HOLE_640K_START) {
>             mem_len -= next_base - HOLE_640K;
>             if (mem_len > 0) {
>                 numamem = acpi_data_push(table_data, sizeof *numamem);
>                 build_srat_memory(numamem, mem_base, mem_len, node,
>                                   MEM_AFFINITY_ENABLED);
>             }
>             mem_base = HOLE_640K_END;
>             /* ... */

BTW, in case

   0
   |-----------|-------|--------|-------
   |           |       |        |
mem_base      640K   next_base   1M


 I wanna add a check here,

            /* Check for the rare case: 640K < RAM < 1M */
            if (next_base <= HOLE_640K_END) {
                next_base = HOLE_640K_END;
                continue;
            }
            mem_base = HOLE_640K_END;
            mem_len = next_base - HOLE_640K_END;

I guess no one would set a node with this less RAM,
So, Is that necessary?

Thanks,
        dou.

>         }
>         /* Cut out the ACPI_PCI hole */
>         if (mem_base <= pcms->below_4g_mem_size &&
>             next_base > pcms->below_4g_mem_size) {
>             mem_len -= next_base - pcms->below_4g_mem_size;
>             /* ... *]
>         }
>         numamem = acpi_data_push(table_data, sizeof *numamem);
>         build_srat_memory(numamem, mem_base, mem_len, node,
>                           MEM_AFFINITY_ENABLED);
>         /* ... */
>     }
>
> > -        numamem = acpi_data_push(table_data, sizeof *numamem);
> > -        build_srat_memory(numamem, mem_base, mem_len, i - 1,
> > -                          MEM_AFFINITY_ENABLED);
> > +        mem_base = build_srat_node_entry(table_data, pcms, i,
> > +                                            mem_base, pcms->node_mem[i]);
> >      }
> >      slots = (table_data->len - numa_start) / sizeof *numamem;
> >      for (; slots < pcms->numa_nodes + 2; slots++) {
> > --
> > 2.5.5