Hi Julien, > -----Original Message----- > From: Julien Grall <[email protected]> > Sent: 2021年8月25日 21:56 > To: Wei Chen <[email protected]>; [email protected]; > [email protected] > Cc: Bertrand Marquis <[email protected]>; Jan Beulich > <[email protected]> > Subject: Re: [XEN RFC PATCH 24/40] xen/arm: introduce a helper to parse > device tree NUMA distance map > > Hi Wei, > > On 11/08/2021 11:24, Wei Chen wrote: > > A NUMA aware device tree will provide a "distance-map" node to > > describe distance between any two nodes. This patch introduce a > > s/introduce/introduces/
OK > > > new helper to parse this distance map. > > > > Signed-off-by: Wei Chen <[email protected]> > > --- > > xen/arch/arm/numa_device_tree.c | 67 +++++++++++++++++++++++++++++++++ > > 1 file changed, 67 insertions(+) > > > > diff --git a/xen/arch/arm/numa_device_tree.c > b/xen/arch/arm/numa_device_tree.c > > index bbe081dcd1..6e0d1d3d9f 100644 > > --- a/xen/arch/arm/numa_device_tree.c > > +++ b/xen/arch/arm/numa_device_tree.c > > @@ -200,3 +200,70 @@ device_tree_parse_numa_memory_node(const void *fdt, > int node, > > > > return 0; > > } > > + > > +/* Parse NUMA distance map v1 */ > > +int __init > > +device_tree_parse_numa_distance_map_v1(const void *fdt, int node) > > +{ > > + const struct fdt_property *prop; > > + const __be32 *matrix; > > + int entry_count, len, i; > > entry_count and i should be unsigned. len unfortunately can't because > fdt_get_property expects a signed int. > OK > > + > > + printk(XENLOG_INFO "NUMA: parsing numa-distance-map\n"); > > + > > + prop = fdt_get_property(fdt, node, "distance-matrix", &len); > > + if ( !prop ) > > + { > > + printk(XENLOG_WARNING > > + "NUMA: No distance-matrix property in distance-map\n"); > > + > > + return -EINVAL; > > + } > > + > > + if ( len % sizeof(uint32_t) != 0 ) > > + { > > + printk(XENLOG_WARNING > > + "distance-matrix in node is not a multiple of u32\n"); > > + return -EINVAL; > > + } > > + > > + entry_count = len / sizeof(uint32_t); > > + if ( entry_count <= 0 ) > > I understand that entry_count may be 0. But I can't see how it can be > negative as the property len cannot be (even if it is a signed type). So > I think this wants to be "== 0". > From the fdt_get_property's comment, when prop is NULL, the len can be negative. But, yes, prop==NULL check will handled before this code. negative len will not reach here. I am ok to change it to "== 0" > > + { > > + printk(XENLOG_WARNING "NUMA: Invalid distance-matrix\n"); > > + > > + return -EINVAL; > > + } > > + > > + matrix = (const __be32 *)prop->data; > > + for ( i = 0; i + 2 < entry_count; i += 3 ) > > + { > > + uint32_t from, to, distance; > > + > > + from = dt_read_number(matrix, 1); > > + matrix++; > > You can use dt_next_cell() which will update the pointer for you. > Thanks > > + to = dt_read_number(matrix, 1); > > + matrix++; > > + distance = dt_read_number(matrix, 1); > > + matrix++; > > + > > + if ( (from == to && distance != NUMA_LOCAL_DISTANCE) || > > + (from != to && distance <= NUMA_LOCAL_DISTANCE) ) > > + { > > + printk(XENLOG_WARNING > > + "Invalid nodes' distance from node#%d to node#%d > = %d\n", > > + from, to, distance); > > + return -EINVAL; > > + } > > + > > + printk(XENLOG_INFO "NUMA: distance from node#%d to node#%d > = %d\n", > > + from, to, distance); > > + numa_set_distance(from, to, distance); > > + > > + /* Set default distance of node B->A same as A->B */ > > + if (to > from) > > + numa_set_distance(to, from, distance); > > + } > > + > > + return 0; > > +} > > > > Cheers, > > -- > Julien Grall
