diff mbox series

[2/5] riscv: cpu_ops_spinwait: Support for 64bit hartid

Message ID 20220525151106.2176147-3-sunilvl@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series Support for 64bit hartid on RV64 platforms | expand

Commit Message

Sunil V L May 25, 2022, 3:11 p.m. UTC
The hartid can be a 64bit value on RV64 platforms. This patch modifies
the hartid variable type to unsigned long so that it can hold 64bit
value on RV64 platforms.

Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
---
 arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Heinrich Schuchardt May 25, 2022, 3:27 p.m. UTC | #1
On 5/25/22 17:11, Sunil V L wrote:
> The hartid can be a 64bit value on RV64 platforms. This patch modifies
> the hartid variable type to unsigned long so that it can hold 64bit
> value on RV64 platforms.
> 
> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> ---
>   arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
> index 346847f6c41c..51ac07514a62 100644
> --- a/arch/riscv/kernel/cpu_ops_spinwait.c
> +++ b/arch/riscv/kernel/cpu_ops_spinwait.c
> @@ -18,7 +18,7 @@ void *__cpu_spinwait_task_pointer[NR_CPUS] __section(".data");
>   static void cpu_update_secondary_bootdata(unsigned int cpuid,
>   				   struct task_struct *tidle)
>   {
> -	int hartid = cpuid_to_hartid_map(cpuid);
> +	unsigned long hartid = cpuid_to_hartid_map(cpuid);
>   
>   	/*
>   	 * The hartid must be less than NR_CPUS to avoid out-of-bound access

This line follows:

if (hartid == INVALID_HARTID || hartid >= NR_CPUS)

INVALID_HARTID is defined as ULONG_MAX. Please, mention that you are 
fixing a bug:

Fixes: c78f94f35cf648 ("RISC-V: Use __cpu_up_stack/task_pointer only for 
spinwait method")

NR_CPUS alias CONFIG_NR_CPUS is an int. You should convert it to 
unsigned before comparing it to hartid to avoid build warnings.

Best regards

Heinrich
Sunil V L May 26, 2022, 10:15 a.m. UTC | #2
On Wed, May 25, 2022 at 05:27:51PM +0200, Heinrich Schuchardt wrote:
> On 5/25/22 17:11, Sunil V L wrote:
> > The hartid can be a 64bit value on RV64 platforms. This patch modifies
> > the hartid variable type to unsigned long so that it can hold 64bit
> > value on RV64 platforms.
> > 
> > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com>
> > ---
> >   arch/riscv/kernel/cpu_ops_spinwait.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
> > index 346847f6c41c..51ac07514a62 100644
> > --- a/arch/riscv/kernel/cpu_ops_spinwait.c
> > +++ b/arch/riscv/kernel/cpu_ops_spinwait.c
> > @@ -18,7 +18,7 @@ void *__cpu_spinwait_task_pointer[NR_CPUS] __section(".data");
> >   static void cpu_update_secondary_bootdata(unsigned int cpuid,
> >   				   struct task_struct *tidle)
> >   {
> > -	int hartid = cpuid_to_hartid_map(cpuid);
> > +	unsigned long hartid = cpuid_to_hartid_map(cpuid);
> >   	/*
> >   	 * The hartid must be less than NR_CPUS to avoid out-of-bound access
> 
> This line follows:
> 
> if (hartid == INVALID_HARTID || hartid >= NR_CPUS)
> 
> INVALID_HARTID is defined as ULONG_MAX. Please, mention that you are fixing
> a bug:
> 
> Fixes: c78f94f35cf648 ("RISC-V: Use __cpu_up_stack/task_pointer only for
> spinwait method")
> 
> NR_CPUS alias CONFIG_NR_CPUS is an int. You should convert it to unsigned
> before comparing it to hartid to avoid build warnings.

Thank you for the feedback. Have modified the patch and commit message
as per your suggestion in V2 version. Please check.

Thanks
Sunil
> 
> Best regards
> 
> Heinrich
>
diff mbox series

Patch

diff --git a/arch/riscv/kernel/cpu_ops_spinwait.c b/arch/riscv/kernel/cpu_ops_spinwait.c
index 346847f6c41c..51ac07514a62 100644
--- a/arch/riscv/kernel/cpu_ops_spinwait.c
+++ b/arch/riscv/kernel/cpu_ops_spinwait.c
@@ -18,7 +18,7 @@  void *__cpu_spinwait_task_pointer[NR_CPUS] __section(".data");
 static void cpu_update_secondary_bootdata(unsigned int cpuid,
 				   struct task_struct *tidle)
 {
-	int hartid = cpuid_to_hartid_map(cpuid);
+	unsigned long hartid = cpuid_to_hartid_map(cpuid);
 
 	/*
 	 * The hartid must be less than NR_CPUS to avoid out-of-bound access