On 03/05/2023 18:43, Julien Grall wrote:
Hi Ayan,
Hi Julien,
On 03/05/2023 17:49, Ayan Kumar Halder wrote:
On 03/05/2023 08:40, Julien Grall wrote:
Hi,
Hi Julien,
Title: Did you mean "Enable spin table"?
Yes, that would be more concrete.
On 02/05/2023 11:58, Ayan Kumar Halder wrote:
On some of the Arm32 based systems (eg Cortex-R52), smpboot is
supported.
Same here.
Yes
In these systems PSCI may not always be supported. In case of
Cortex-R52, there
is no EL3 or secure mode. Thus, PSCI is not supported as it
requires EL3.
Thus, we use 'spin-table' mechanism to boot the secondary cpus. The
primary
cpu provides the startup address of the secondary cores. This
address is
provided using the 'cpu-release-addr' property.
To support smpboot, we have copied the code from
xen/arch/arm/arm64/smpboot.c
I would rather prefer if we don't duplicate the code but instead
move the logic in common code.
Ack
with the following changes :-
1. 'enable-method' is an optional property. Refer to the comment in
https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cpus.yaml
" # On ARM 32-bit systems this property is optional"
Looking at this list, "spin-table" doesn't seem to be supported
for 32-bit systems.
However, looking at
https://developer.arm.com/documentation/den0013/d/Multi-core-processors/Booting-SMP-systems/SMP-boot-in-Linux
, it seems "spin-table" is a valid boot mechanism for Armv7 cpus.
I am not able to find the associated code in Linux 32-bit. Do you have
any pointer?
Unfortunately, no.
I see that in linux, "spin-table" support is added in
arch/arm64/kernel/smp_spin_table.c. So, there seems to be no Arm32
support for this.
Can you point me to the discussion/patch where this would be added?
Actually, this is the first discussion I am having with regards to
adding a "spin-table" support on Arm32.
I was asking for the discussion on the Device-Tree/Linux ML or code.
I don't really want to do a "spin-table" support if this is not even
supported in Linux.
I see your point. But that brings me to my next question, how do I parse
cpu node specific properties for Arm32 cpus.
In
https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cpus.yaml
, I see some of the properties valid for Arm32 cpus.
For example:-
secondary-boot-reg
rockchip,pmu
Also, it says "additionalProperties: true" which means I can add platform
specific properties under the cpu node. Please correct me if mistaken.
My cpu nodes will look like this :-
cpu@1 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x00 0x01>;
enable-method = "spin-table";
amd-cpu-release-addr = <0xEB58C010>; /* might also use
"secondary-boot-reg" */
amd-cpu-reset-addr = <0xEB58C000>;
amd-cpu-reset-delay = <0xF00000>;
amd-cpu-re
phandle = <0x03>;
};
cpu@2 {
device_type = "cpu";
compatible = "arm,armv8";
reg = <0x00 0x02>;
enable-method = "spin-table";
amd-cpu-release-addr = <0xEB59C010>; /* might also use
"secondary-boot-reg" */
amd-cpu-reset-addr = <0xEB59C000>;
amd-cpu-reset-delay = <0xF00000>;
amd-cpu-re
phandle = <0x03>;
};
If the reasoning makes sense, then does the following proposed change
looks sane ?
diff --git a/xen/arch/arm/arm32/smpboot.c b/xen/arch/arm/arm32/smpboot.c
index e7368665d5..0b281edb9d 100644
--- a/xen/arch/arm/arm32/smpboot.c
+++ b/xen/arch/arm/arm32/smpboot.c
@@ -10,10 +10,7 @@ int __init arch_smp_init(void)
int __init arch_cpu_init(int cpu, struct dt_device_node *dn)
{
- /* Not needed on ARM32, as there is no relevant information in
- * the CPU device tree node for ARMv7 CPUs.
- */
- return 0;
+ return platform_cpu_init(cpu, dn);
}
int arch_cpu_up(int cpu)
diff --git a/xen/arch/arm/include/asm/platform.h
b/xen/arch/arm/include/asm/platform.h
index d03b261ce8..5cd7af4d0e 100644
--- a/xen/arch/arm/include/asm/platform.h
+++ b/xen/arch/arm/include/asm/platform.h
@@ -18,6 +18,7 @@ struct platform_desc {
/* SMP */
int (*smp_init)(void);
int (*cpu_up)(int cpu);
+ int (*cpu_init)(int cpu, struct dt_device_node *dn);
#endif
/* Specific mapping for dom0 */
int (*specific_mapping)(struct domain *d);
diff --git a/xen/arch/arm/platform.c b/xen/arch/arm/platform.c
index a820665020..ed54b68c20 100644
--- a/xen/arch/arm/platform.c
+++ b/xen/arch/arm/platform.c
@@ -95,6 +95,14 @@ int __init platform_specific_mapping(struct domain *d)
}
#ifdef CONFIG_ARM_32
+int __init platform_cpu_init(int cpu, struct dt_device_node *dn)
+{
+ if ( platform && platform->cpu_init )
+ return platform_cpu_init(cpu, dn); /* parse platform specific
cpu properties */
+
+ return 0;
+}
+
int platform_cpu_up(int cpu)
{
if ( psci_ver )
Let me know your thoughts, please.
Kind regards,
Ayan
Cheers,
