RE: [XEN RFC PATCH 25/40] xen/arm: unified entry to parse all NUMA data from device tree
> -Original Message-
> From: Stefano Stabellini
> Sent: 2021年9月2日 2:31
> To: Julien Grall
> Cc: Stefano Stabellini ; Wei Chen
> ; xen-devel@lists.xenproject.org; jbeul...@suse.com;
> Bertrand Marquis
> Subject: Re: [XEN RFC PATCH 25/40] xen/arm: unified entry to parse all
> NUMA data from device tree
>
> On Tue, 31 Aug 2021, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 31/08/2021 01:54, Stefano Stabellini wrote:
> > > On Wed, 11 Aug 2021, Wei Chen wrote:
> > > > In this API, we scan whole device tree to parse CPU node id, memory
> > > > node id and distance-map. Though early_scan_node will invoke has a
> > > > handler to process memory nodes. If we want to parse memory node id
> > > > in this handler, we have to embeded NUMA parse code in this handler.
> > > > But we still need to scan whole device tree to find CPU NUMA id and
> > > > distance-map. In this case, we include memory NUMA id parse in this
> > > > API too. Another benefit is that we have a unique entry for device
> > > > tree NUMA data parse.
> > > >
> > > > Signed-off-by: Wei Chen
> > > > ---
> > > > xen/arch/arm/numa_device_tree.c | 31 -
> --
> > > > xen/include/asm-arm/numa.h | 1 +
> > > > 2 files changed, 29 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/numa_device_tree.c
> > > > b/xen/arch/arm/numa_device_tree.c
> > > > index 6e0d1d3d9f..27ffb72f7b 100644
> > > > --- a/xen/arch/arm/numa_device_tree.c
> > > > +++ b/xen/arch/arm/numa_device_tree.c
> > > > @@ -131,7 +131,8 @@ save_memblk:
> > > > }
> > > > /* Parse CPU NUMA node info */
> > > > -int __init device_tree_parse_numa_cpu_node(const void *fdt, int
> node)
> > > > +static int __init
> > > > +device_tree_parse_numa_cpu_node(const void *fdt, int node)
> > > > {
> > > > uint32_t nid;
> > > > @@ -147,7 +148,7 @@ int __init
> device_tree_parse_numa_cpu_node(const
> > > > void *fdt, int node)
> > > > }
> > > > /* Parse memory node NUMA info */
> > > > -int __init
> > > > +static int __init
> > > > device_tree_parse_numa_memory_node(const void *fdt, int node,
> > > > const char *name, uint32_t addr_cells, uint32_t size_cells)
> > > > {
> > > > @@ -202,7 +203,7 @@ device_tree_parse_numa_memory_node(const void
> *fdt,
> > > > int node,
> > > > }
> > > > /* Parse NUMA distance map v1 */
> > > > -int __init
> > > > +static int __init
> > > > device_tree_parse_numa_distance_map_v1(const void *fdt, int node)
> > > > {
> > > > const struct fdt_property *prop;
> > > > @@ -267,3 +268,27 @@ device_tree_parse_numa_distance_map_v1(const
> void
> > > > *fdt, int node)
> > > > return 0;
> > > > }
> > > > +
> > > > +static int __init fdt_scan_numa_nodes(const void *fdt,
> > > > +int node, const char *uname, int depth,
> > > > +u32 address_cells, u32 size_cells, void *data)
> > > > +{
> > > > +int ret = 0;
> > > > +
> > > > +if ( fdt_node_check_type(fdt, node, "cpu") == 0 )
> > > > +ret = device_tree_parse_numa_cpu_node(fdt, node);
> > > > +else if ( fdt_node_check_type(fdt, node, "memory") == 0 )
> > > > +ret = device_tree_parse_numa_memory_node(fdt, node, uname,
> > > > +address_cells, size_cells);
> > > > +else if ( fdt_node_check_compatible(fdt, node,
> > > > +"numa-distance-map-v1") == 0 )
> > > > +ret = device_tree_parse_numa_distance_map_v1(fdt, node);
> > > > +
> > > > +return ret;
> > > > +}
> > >
> > > Julien, do you have an opinion on whether it might be worth reusing
> the
> > > existing early_scan_node function for this to avoiding another full
> FDT
> > > scan (to avoid another call to device_tree_for_each_node)?
> >
> > I don't like the full FDT scan and actually drafted an e-mail in reply-
> to [1]
> > to suggest parse all the NUMA information from early_scan_node().
> >
> > However, we don't know whether ACPI or DT will be used at the time
> > early_scan_node() is called. So we will need to revert any change which
> can
> > make the code a little more awkward.
> >
> > So I decided to drop my e-mail because I prefer the full DT scan for now.
> We
> > can look at optimizing later if this becomes a pain point.
>
> Uhm, yes you are right.
>
> We would have to move some of the logic to detect ACPI earlier to
> early_scan_node (e.g. xen/arch/arm/acpi/boot.c:dt_scan_depth1_nodes).
> That could actually be a good idea, but it is true that could be done
> with a separate patch later.
Sounds good to me.
Re: [XEN RFC PATCH 25/40] xen/arm: unified entry to parse all NUMA data from device tree
On Tue, 31 Aug 2021, Julien Grall wrote:
> Hi Stefano,
>
> On 31/08/2021 01:54, Stefano Stabellini wrote:
> > On Wed, 11 Aug 2021, Wei Chen wrote:
> > > In this API, we scan whole device tree to parse CPU node id, memory
> > > node id and distance-map. Though early_scan_node will invoke has a
> > > handler to process memory nodes. If we want to parse memory node id
> > > in this handler, we have to embeded NUMA parse code in this handler.
> > > But we still need to scan whole device tree to find CPU NUMA id and
> > > distance-map. In this case, we include memory NUMA id parse in this
> > > API too. Another benefit is that we have a unique entry for device
> > > tree NUMA data parse.
> > >
> > > Signed-off-by: Wei Chen
> > > ---
> > > xen/arch/arm/numa_device_tree.c | 31 ---
> > > xen/include/asm-arm/numa.h | 1 +
> > > 2 files changed, 29 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/numa_device_tree.c
> > > b/xen/arch/arm/numa_device_tree.c
> > > index 6e0d1d3d9f..27ffb72f7b 100644
> > > --- a/xen/arch/arm/numa_device_tree.c
> > > +++ b/xen/arch/arm/numa_device_tree.c
> > > @@ -131,7 +131,8 @@ save_memblk:
> > > }
> > > /* Parse CPU NUMA node info */
> > > -int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
> > > +static int __init
> > > +device_tree_parse_numa_cpu_node(const void *fdt, int node)
> > > {
> > > uint32_t nid;
> > > @@ -147,7 +148,7 @@ int __init device_tree_parse_numa_cpu_node(const
> > > void *fdt, int node)
> > > }
> > > /* Parse memory node NUMA info */
> > > -int __init
> > > +static int __init
> > > device_tree_parse_numa_memory_node(const void *fdt, int node,
> > > const char *name, uint32_t addr_cells, uint32_t size_cells)
> > > {
> > > @@ -202,7 +203,7 @@ device_tree_parse_numa_memory_node(const void *fdt,
> > > int node,
> > > }
> > > /* Parse NUMA distance map v1 */
> > > -int __init
> > > +static int __init
> > > device_tree_parse_numa_distance_map_v1(const void *fdt, int node)
> > > {
> > > const struct fdt_property *prop;
> > > @@ -267,3 +268,27 @@ device_tree_parse_numa_distance_map_v1(const void
> > > *fdt, int node)
> > > return 0;
> > > }
> > > +
> > > +static int __init fdt_scan_numa_nodes(const void *fdt,
> > > +int node, const char *uname, int depth,
> > > +u32 address_cells, u32 size_cells, void *data)
> > > +{
> > > +int ret = 0;
> > > +
> > > +if ( fdt_node_check_type(fdt, node, "cpu") == 0 )
> > > +ret = device_tree_parse_numa_cpu_node(fdt, node);
> > > +else if ( fdt_node_check_type(fdt, node, "memory") == 0 )
> > > +ret = device_tree_parse_numa_memory_node(fdt, node, uname,
> > > +address_cells, size_cells);
> > > +else if ( fdt_node_check_compatible(fdt, node,
> > > +"numa-distance-map-v1") == 0 )
> > > +ret = device_tree_parse_numa_distance_map_v1(fdt, node);
> > > +
> > > +return ret;
> > > +}
> >
> > Julien, do you have an opinion on whether it might be worth reusing the
> > existing early_scan_node function for this to avoiding another full FDT
> > scan (to avoid another call to device_tree_for_each_node)?
>
> I don't like the full FDT scan and actually drafted an e-mail in reply-to [1]
> to suggest parse all the NUMA information from early_scan_node().
>
> However, we don't know whether ACPI or DT will be used at the time
> early_scan_node() is called. So we will need to revert any change which can
> make the code a little more awkward.
>
> So I decided to drop my e-mail because I prefer the full DT scan for now. We
> can look at optimizing later if this becomes a pain point.
Uhm, yes you are right.
We would have to move some of the logic to detect ACPI earlier to
early_scan_node (e.g. xen/arch/arm/acpi/boot.c:dt_scan_depth1_nodes).
That could actually be a good idea, but it is true that could be done
with a separate patch later.
Re: [XEN RFC PATCH 25/40] xen/arm: unified entry to parse all NUMA data from device tree
Hi Stefano,
On 31/08/2021 01:54, Stefano Stabellini wrote:
On Wed, 11 Aug 2021, Wei Chen wrote:
In this API, we scan whole device tree to parse CPU node id, memory
node id and distance-map. Though early_scan_node will invoke has a
handler to process memory nodes. If we want to parse memory node id
in this handler, we have to embeded NUMA parse code in this handler.
But we still need to scan whole device tree to find CPU NUMA id and
distance-map. In this case, we include memory NUMA id parse in this
API too. Another benefit is that we have a unique entry for device
tree NUMA data parse.
Signed-off-by: Wei Chen
---
xen/arch/arm/numa_device_tree.c | 31 ---
xen/include/asm-arm/numa.h | 1 +
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/numa_device_tree.c b/xen/arch/arm/numa_device_tree.c
index 6e0d1d3d9f..27ffb72f7b 100644
--- a/xen/arch/arm/numa_device_tree.c
+++ b/xen/arch/arm/numa_device_tree.c
@@ -131,7 +131,8 @@ save_memblk:
}
/* Parse CPU NUMA node info */
-int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
+static int __init
+device_tree_parse_numa_cpu_node(const void *fdt, int node)
{
uint32_t nid;
@@ -147,7 +148,7 @@ int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
}
/* Parse memory node NUMA info */
-int __init
+static int __init
device_tree_parse_numa_memory_node(const void *fdt, int node,
const char *name, uint32_t addr_cells, uint32_t size_cells)
{
@@ -202,7 +203,7 @@ device_tree_parse_numa_memory_node(const void *fdt, int
node,
}
/* Parse NUMA distance map v1 */
-int __init
+static int __init
device_tree_parse_numa_distance_map_v1(const void *fdt, int node)
{
const struct fdt_property *prop;
@@ -267,3 +268,27 @@ device_tree_parse_numa_distance_map_v1(const void *fdt,
int node)
return 0;
}
+
+static int __init fdt_scan_numa_nodes(const void *fdt,
+int node, const char *uname, int depth,
+u32 address_cells, u32 size_cells, void *data)
+{
+int ret = 0;
+
+if ( fdt_node_check_type(fdt, node, "cpu") == 0 )
+ret = device_tree_parse_numa_cpu_node(fdt, node);
+else if ( fdt_node_check_type(fdt, node, "memory") == 0 )
+ret = device_tree_parse_numa_memory_node(fdt, node, uname,
+address_cells, size_cells);
+else if ( fdt_node_check_compatible(fdt, node,
+"numa-distance-map-v1") == 0 )
+ret = device_tree_parse_numa_distance_map_v1(fdt, node);
+
+return ret;
+}
Julien, do you have an opinion on whether it might be worth reusing the
existing early_scan_node function for this to avoiding another full FDT
scan (to avoid another call to device_tree_for_each_node)?
I don't like the full FDT scan and actually drafted an e-mail in
reply-to [1] to suggest parse all the NUMA information from
early_scan_node().
However, we don't know whether ACPI or DT will be used at the time
early_scan_node() is called. So we will need to revert any change which
can make the code a little more awkward.
So I decided to drop my e-mail because I prefer the full DT scan for
now. We can look at optimizing later if this becomes a pain point.
Cheers,
[1]
https://lore.kernel.org/xen-devel/db9pr08mb6857604b3d4b690f2b8832bd9e...@db9pr08mb6857.eurprd08.prod.outlook.com/
--
Julien Grall
Re: [XEN RFC PATCH 25/40] xen/arm: unified entry to parse all NUMA data from device tree
On Wed, 11 Aug 2021, Wei Chen wrote:
> In this API, we scan whole device tree to parse CPU node id, memory
> node id and distance-map. Though early_scan_node will invoke has a
> handler to process memory nodes. If we want to parse memory node id
> in this handler, we have to embeded NUMA parse code in this handler.
> But we still need to scan whole device tree to find CPU NUMA id and
> distance-map. In this case, we include memory NUMA id parse in this
> API too. Another benefit is that we have a unique entry for device
> tree NUMA data parse.
>
> Signed-off-by: Wei Chen
> ---
> xen/arch/arm/numa_device_tree.c | 31 ---
> xen/include/asm-arm/numa.h | 1 +
> 2 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/numa_device_tree.c b/xen/arch/arm/numa_device_tree.c
> index 6e0d1d3d9f..27ffb72f7b 100644
> --- a/xen/arch/arm/numa_device_tree.c
> +++ b/xen/arch/arm/numa_device_tree.c
> @@ -131,7 +131,8 @@ save_memblk:
> }
>
> /* Parse CPU NUMA node info */
> -int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
> +static int __init
> +device_tree_parse_numa_cpu_node(const void *fdt, int node)
> {
> uint32_t nid;
>
> @@ -147,7 +148,7 @@ int __init device_tree_parse_numa_cpu_node(const void
> *fdt, int node)
> }
>
> /* Parse memory node NUMA info */
> -int __init
> +static int __init
> device_tree_parse_numa_memory_node(const void *fdt, int node,
> const char *name, uint32_t addr_cells, uint32_t size_cells)
> {
> @@ -202,7 +203,7 @@ device_tree_parse_numa_memory_node(const void *fdt, int
> node,
> }
>
> /* Parse NUMA distance map v1 */
> -int __init
> +static int __init
> device_tree_parse_numa_distance_map_v1(const void *fdt, int node)
> {
> const struct fdt_property *prop;
> @@ -267,3 +268,27 @@ device_tree_parse_numa_distance_map_v1(const void *fdt,
> int node)
>
> return 0;
> }
> +
> +static int __init fdt_scan_numa_nodes(const void *fdt,
> +int node, const char *uname, int depth,
> +u32 address_cells, u32 size_cells, void *data)
> +{
> +int ret = 0;
> +
> +if ( fdt_node_check_type(fdt, node, "cpu") == 0 )
> +ret = device_tree_parse_numa_cpu_node(fdt, node);
> +else if ( fdt_node_check_type(fdt, node, "memory") == 0 )
> +ret = device_tree_parse_numa_memory_node(fdt, node, uname,
> +address_cells, size_cells);
> +else if ( fdt_node_check_compatible(fdt, node,
> +"numa-distance-map-v1") == 0 )
> +ret = device_tree_parse_numa_distance_map_v1(fdt, node);
> +
> +return ret;
> +}
Julien, do you have an opinion on whether it might be worth reusing the
existing early_scan_node function for this to avoiding another full FDT
scan (to avoid another call to device_tree_for_each_node)?
[XEN RFC PATCH 25/40] xen/arm: unified entry to parse all NUMA data from device tree
In this API, we scan whole device tree to parse CPU node id, memory
node id and distance-map. Though early_scan_node will invoke has a
handler to process memory nodes. If we want to parse memory node id
in this handler, we have to embeded NUMA parse code in this handler.
But we still need to scan whole device tree to find CPU NUMA id and
distance-map. In this case, we include memory NUMA id parse in this
API too. Another benefit is that we have a unique entry for device
tree NUMA data parse.
Signed-off-by: Wei Chen
---
xen/arch/arm/numa_device_tree.c | 31 ---
xen/include/asm-arm/numa.h | 1 +
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/numa_device_tree.c b/xen/arch/arm/numa_device_tree.c
index 6e0d1d3d9f..27ffb72f7b 100644
--- a/xen/arch/arm/numa_device_tree.c
+++ b/xen/arch/arm/numa_device_tree.c
@@ -131,7 +131,8 @@ save_memblk:
}
/* Parse CPU NUMA node info */
-int __init device_tree_parse_numa_cpu_node(const void *fdt, int node)
+static int __init
+device_tree_parse_numa_cpu_node(const void *fdt, int node)
{
uint32_t nid;
@@ -147,7 +148,7 @@ int __init device_tree_parse_numa_cpu_node(const void *fdt,
int node)
}
/* Parse memory node NUMA info */
-int __init
+static int __init
device_tree_parse_numa_memory_node(const void *fdt, int node,
const char *name, uint32_t addr_cells, uint32_t size_cells)
{
@@ -202,7 +203,7 @@ device_tree_parse_numa_memory_node(const void *fdt, int
node,
}
/* Parse NUMA distance map v1 */
-int __init
+static int __init
device_tree_parse_numa_distance_map_v1(const void *fdt, int node)
{
const struct fdt_property *prop;
@@ -267,3 +268,27 @@ device_tree_parse_numa_distance_map_v1(const void *fdt,
int node)
return 0;
}
+
+static int __init fdt_scan_numa_nodes(const void *fdt,
+int node, const char *uname, int depth,
+u32 address_cells, u32 size_cells, void *data)
+{
+int ret = 0;
+
+if ( fdt_node_check_type(fdt, node, "cpu") == 0 )
+ret = device_tree_parse_numa_cpu_node(fdt, node);
+else if ( fdt_node_check_type(fdt, node, "memory") == 0 )
+ret = device_tree_parse_numa_memory_node(fdt, node, uname,
+address_cells, size_cells);
+else if ( fdt_node_check_compatible(fdt, node,
+"numa-distance-map-v1") == 0 )
+ret = device_tree_parse_numa_distance_map_v1(fdt, node);
+
+return ret;
+}
+
+/* Initialize NUMA from device tree */
+int __init numa_device_tree_init(const void *fdt)
+{
+return device_tree_for_each_node(fdt, 0, fdt_scan_numa_nodes, NULL);
+}
diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index 756ad82d07..7a3588ac7f 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -26,6 +26,7 @@ typedef u8 nodeid_t;
extern s8 device_tree_numa;
extern void numa_init(bool acpi_off);
+extern int numa_device_tree_init(const void *fdt);
extern void numa_set_distance(nodeid_t from, nodeid_t to, uint32_t distance);
/*
--
2.25.1
