[U-Boot] [PATCH] riscv: cpu: Skip unavailable hart in the get_count() op
We should not count in hart that is marked as not available in the device tree in riscv_cpu_get_count().
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/cpu/riscv_cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index f77c126..28ad0aa 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -46,6 +46,10 @@ static int riscv_cpu_get_count(struct udevice *dev) ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) { const char *device_type;
+ /* skip if hart is marked as not available in the device tree */ + if (!ofnode_is_available(node)) + continue; + device_type = ofnode_read_string(node, "device_type"); if (!device_type) continue;
Hi Bin,
On Thu, 2019-08-08 at 00:52 -0700, Bin Meng wrote:
We should not count in hart that is marked as not available in the device tree in riscv_cpu_get_count().
I think it might make sense to also exclude harts that are not listed as available in the available_harts mask. So the same logic as in arch/riscv/lib/smp.c. In this case, the bind function should probably check the mask as well.
Looks good otherwise!
Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
Thanks, Lukas
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/cpu/riscv_cpu.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c index f77c126..28ad0aa 100644 --- a/drivers/cpu/riscv_cpu.c +++ b/drivers/cpu/riscv_cpu.c @@ -46,6 +46,10 @@ static int riscv_cpu_get_count(struct udevice *dev) ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) { const char *device_type;
/* skip if hart is marked as not available in the device tree */if (!ofnode_is_available(node))continue;- device_type = ofnode_read_string(node, "device_type"); if (!device_type) continue;
Hi Lukas,
On Thu, Aug 8, 2019 at 7:22 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Thu, 2019-08-08 at 00:52 -0700, Bin Meng wrote:
We should not count in hart that is marked as not available in the device tree in riscv_cpu_get_count().
I think it might make sense to also exclude harts that are not listed as available in the available_harts mask. So the same logic as in arch/riscv/lib/smp.c. In this case, the bind function should probably check the mask as well.
Yes, the check to available_harts mask can be added in riscv_cpu_get_count(). I will do that.
However, I doubt we could do available_harts mask check in the bind function. If it returns error for non-available harts, the whole initialization process fails.
Looks good otherwise!
Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
Regards, Bin
Hi Bin,
On Thu, 2019-08-08 at 21:25 +0800, Bin Meng wrote:
Hi Lukas,
On Thu, Aug 8, 2019 at 7:22 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Thu, 2019-08-08 at 00:52 -0700, Bin Meng wrote:
We should not count in hart that is marked as not available in the device tree in riscv_cpu_get_count().
I think it might make sense to also exclude harts that are not listed as available in the available_harts mask. So the same logic as in arch/riscv/lib/smp.c. In this case, the bind function should probably check the mask as well.
Yes, the check to available_harts mask can be added in riscv_cpu_get_count(). I will do that.
However, I doubt we could do available_harts mask check in the bind function. If it returns error for non-available harts, the whole initialization process fails.
You are right, I did not think about that. Perhaps it's best to ignore the available_harts mask in the count function as well then.
It just checked where cpu_get_count() is used. The Andes PLIC driver (lib/andes_plic.c) uses it in a way that the patch might break the driver. It iterates over all harts, assuming the first hart is 0 and the last hart the result of cpu_get_count(). If any of the harts is set to disabled, this will no longer be true. It's probably best to update the driver to use the ofnode_for_each_subnode() macro to iterate over all harts. Rick, can you update the driver?
Thanks, Lukas
Looks good otherwise!
Reviewed-by: Lukas Auer lukas.auer@aisec.fraunhofer.de
Regards, Bin
Hi Lukas,
On Thu, Aug 8, 2019 at 10:03 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Thu, 2019-08-08 at 21:25 +0800, Bin Meng wrote:
Hi Lukas,
On Thu, Aug 8, 2019 at 7:22 PM Auer, Lukas lukas.auer@aisec.fraunhofer.de wrote:
Hi Bin,
On Thu, 2019-08-08 at 00:52 -0700, Bin Meng wrote:
We should not count in hart that is marked as not available in the device tree in riscv_cpu_get_count().
I think it might make sense to also exclude harts that are not listed as available in the available_harts mask. So the same logic as in arch/riscv/lib/smp.c. In this case, the bind function should probably check the mask as well.
Yes, the check to available_harts mask can be added in riscv_cpu_get_count(). I will do that.
However, I doubt we could do available_harts mask check in the bind function. If it returns error for non-available harts, the whole initialization process fails.
You are right, I did not think about that. Perhaps it's best to ignore the available_harts mask in the count function as well then.
Agreed. I will leave this patch as it is.
It just checked where cpu_get_count() is used. The Andes PLIC driver (lib/andes_plic.c) uses it in a way that the patch might break the driver. It iterates over all harts, assuming the first hart is 0 and the last hart the result of cpu_get_count(). If any of the harts is set to disabled, this will no longer be true. It's probably best to update the driver to use the ofnode_for_each_subnode() macro to iterate over all harts. Rick, can you update the driver?
Good catch!
Regards, Bin
participants (2)
-
Auer, Lukas -
Bin Meng