On 2016/4/23 15:03, Andrew Jones wrote:
> On Sat, Apr 23, 2016 at 09:17:25AM +0800, Shannon Zhao wrote:
>> >
>> >
>> > 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);
> I noticed that /distance-map is an optional node by the latest version
> of the spec. In its absence default values will be used. Do we plan on
> putting anything other then the 10s and 20s here? If not, then we can
> leave it to Linux to determine what the defaults should be, and it'll
> use them by itself if we leave this node out.
>
Agree. I thought this before. It could not provide this /distance-map
node as well as it doesn't provide SLIT table for ACPI. So I'll drop
this patch.
Thanks,
--
Shannon
