RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Jan,

> -----Original Message-----
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> >>>> The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
> >>>
> >>> I inferred the discussion as "we should try to keep consistent between the
> >> value
> >>> used in device tree and ACPI tables". Maybe my inference is wrong.
> >>>
> >>>> Looking at
> >>>> Linux'es Documentation/devicetree/numa.txt, there's no mention of an
> >>>> upper bound on the distance values. It only says that on the diagonal
> >>>> entries should be 10 (i.e. matching ACPI, without really saying so).
> >>>
> >>> I agree that the NUMA device tree binding is a little bit vague. So I 
> >>> cannot
> >>> say the case that you provided is not valid. I would like to ask Arm
> >> maintainers
> >>> (putting them into To:) opinion on this as I think I am not the one to
> decide
> >> the
> >>> expected behavior on Arm.
> >>>
> >>> Bertrand/Julien/Stefano: Would you please kindly share your opinion on
> >> which
> >>> value should be used as the default value of the node distance map? Do
> >> you
> >>> think reusing the "unreachable" distance, i.e. 0xFF, as the default node
> >> distance
> >>> is acceptable here? Thanks!
> >>
> >> My suggestion would be to, rather than rejecting values >= 0xff, to 
> >> saturate
> >> at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping
> >> things
> >> consistent with ACPI).
> >
> > Since it has been a while and there were no feedback from Arm
> maintainers,
> > I would like to follow your suggestions for v5. However while I was writing
> the
> > code for the "saturation", i.e., adding below snippet in
> numa_set_distance():
> > ```
> > if ( distance > NUMA_NO_DISTANCE )
> >         distance = NUMA_MAX_DISTANCE;
> > ```
> > I noticed an issue which needs your clarification:
> > Currently, the default value of the distance map is NUMA_NO_DISTANCE,
> > which indicates the nodes are not reachable. IMHO, if in device tree, we do
> > saturations like above for ACPI invalid distances (distances >= 0xff), by
> saturating
> > the distance to 0xfe, we are making the unreachable nodes to reachable. I
> am
> > not sure if this will lead to problems. Do you have any more thoughts?
> Thanks!
> 
> I can only answer this with a question back: How is "unreachable"
> represented
> in DT? 

Exactly, that is also what we I am trying to find but failed. My understanding
is that, the spec of NUMA is defined in the ACPI, and the DT NUMA binding
only specifies how users can use DT to represent the same set of ACPI data,
instead of define another standard.

By looking into the existing implementation of NUMA for DT,
in Linux, from drivers/base/arch_numa.c: numa_set_distance(), there is a
"if ((u8)distance != distance)" then return. So I think at least in Linux the
distance value is consistent with the ACPI spec. 

> Or is "unreachable" simply expressed by the absence of any data?

Maybe I am wrong but I don't think so, as in the Linux implementation,
drivers/of/of_numa.c: of_numa_parse_distance_map_v1(), the for loop
"for (i = 0; i + 2 < entry_count; i += 3)" basically implies no fields should
be omitted in the distance map entry.

Kind regards,
Henry

> 
> Jan