On 2016/4/22 20:25, Andrew Jones wrote:
> On Thu, Apr 21, 2016 at 02:23:50PM +0800, Shannon Zhao wrote:
>> > From: Shannon Zhao <shannon.z...@linaro.org>
>> >
>> > This /distance-map node is used to describe the accessing distance
>> > between NUMA nodes.
>> >
>> > Signed-off-by: Shannon Zhao <shannon.z...@linaro.org>
>> > ---
>> > hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++
>> > 1 file changed, 30 insertions(+)
>> >
>> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> > index 56d35c7..814a1eb 100644
>> > --- a/hw/arm/virt.c
>> > +++ b/hw/arm/virt.c
>> > @@ -40,6 +40,7 @@
>> > #include "sysemu/device_tree.h"
>> > #include "sysemu/sysemu.h"
>> > #include "sysemu/kvm.h"
>> > +#include "sysemu/numa.h"
>> > #include "hw/boards.h"
>> > #include "hw/loader.h"
>> > #include "exec/address-spaces.h"
>> > @@ -203,6 +204,9 @@ static VirtBoardInfo *find_machine_info(const char
>> > *cpu)
>> >
>> > static void create_fdt(VirtBoardInfo *vbi)
>> > {
>> > + unsigned int i, j, number, count;
> s/count/index/ ?
>
>> > + uint64_t *matrix;
>> > +
>> > void *fdt = create_device_tree(&vbi->fdt_size);
>> >
>> > if (!fdt) {
>> > @@ -239,6 +243,32 @@ static void create_fdt(VirtBoardInfo *vbi)
>> > "clk24mhz");
>> > qemu_fdt_setprop_cell(fdt, "/apb-pclk", "phandle",
>> > vbi->clock_phandle);
>> >
>> > + if (nb_numa_nodes <= 0) {
>> > + return;
>> > + }
>> > +
>> > + /* Add /distance-map node for NUMA */
>> > + qemu_fdt_add_subnode(fdt, "/distance-map");
>> > + qemu_fdt_setprop_string(fdt, "/distance-map", "compatible",
>> > + "numa-distance-map-v1");
>> > +
>> > + number = nb_numa_nodes * nb_numa_nodes * 6;
>> > + matrix = g_malloc0(number * sizeof(uint64_t));
>> > + for (i = 0; i < nb_numa_nodes; i++) {
>> > + for (j = 0; j < nb_numa_nodes; j++) {
>> > + count = (i * nb_numa_nodes + j) * 6;
>> > + matrix[count++] = 1;
>> > + matrix[count++] = i;
>> > + matrix[count++] = 1;
>> > + matrix[count++] = j;
>> > + matrix[count++] = 1;
>> > + matrix[count++] = (i == j) ? 10 : 20;
>> > + }
>> > + }
>> > + qemu_fdt_setprop_sized_cells_from_array(fdt, "/distance-map",
>> > + "distance-matrix", number / 2,
>> > + matrix);
> I had to read qemu_fdt_setprop_sized_cells_from_array to understand why
> above we're using 6 instead of 3, and then placing all the 1's in every
> other slot, and then dividing number by 2 here. Is using this function
> worth the confusion?
>
> I think the following would greatly improve reviewability, and shave off
> a bit of boot time (by not having to alloc more mem and copy the matrix).
>
> uint32_t *matrix;
>
> number = nb_numa_nodes * nb_numa_nodes * 3;
> matrix = g_malloc0(number * sizeof(uint32_t));
> for (i = 0; i < nb_numa_nodes; i++) {
> for (j = 0; j < nb_numa_nodes; j++) {
> count = (i * nb_numa_nodes + j) * 3;
> matrix[count++] = cpu_to_be32(i);
> matrix[count++] = cpu_to_be32(j);
> matrix[count++] = cpu_to_be32(i == j ? 10 : 20);
> }
> }
> qemu_fdt_setprop(fdt, "/distance-map", "distance-matrix",
> matrix, number * sizeof(uint32_t));
>
>
>> > + g_free(matrix);
> Also, I think it would nicer if all this was put in its own function, and
> then just add the following to create_fdt.
>
> if (nb_numa_nodes) {
> virt_fdt_create_distance_map(fdt);
> }
>
Ok, will update this.
Thanks,
--
Shannon
