[PATCH 1/2] riscv: cpu_ops: Change return value type of cpu_is_stopped() to bool

[Danil Skrebenkov]

> In the original sbi_cpu_is_stopped(), if rc doesn't equal to the
> SBI_HSM_STATE_STOPPED, it will return rc to the caller directly. But
> there is a hidden problem, the rc could be SBI_HSM_STATE_STARTED, if
> so, this function will report cpu stopped while the cpu isn't really
> stopped.
>
> Furthermore, from the name of cpu_is_stopped(), it gives a sense the
> return value is a bool type, true means the cpu is stopped, conversely
> false means the cpu is not stopped.
>
> Here change the return value type to bool and change the callers
> accordingly. This could fix the above two issues.
>
> Fixes: f1e58583b9c7c ("RISC-V: Support cpu hotplug")
> Signed-off-by: Hui Wang <hui.wang at canonical.com>
> ---
>   arch/riscv/include/asm/cpu_ops.h | 2 +-
>   arch/riscv/kernel/cpu-hotplug.c  | 2 +-
>   arch/riscv/kernel/cpu_ops_sbi.c  | 6 ++----
>   3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/riscv/include/asm/cpu_ops.h b/arch/riscv/include/asm/cpu_ops.h
> index 176b570ef982..065811fca594 100644
> --- a/arch/riscv/include/asm/cpu_ops.h
> +++ b/arch/riscv/include/asm/cpu_ops.h
> @@ -24,7 +24,7 @@ struct cpu_operations {
>   				     struct task_struct *tidle);
>   #ifdef CONFIG_HOTPLUG_CPU
>   	void		(*cpu_stop)(void);
> -	int		(*cpu_is_stopped)(unsigned int cpu);
> +	bool		(*cpu_is_stopped)(unsigned int cpu);
>   #endif
>   };
>   
> diff --git a/arch/riscv/kernel/cpu-hotplug.c b/arch/riscv/kernel/cpu-hotplug.c
> index a0ee426f6d93..c240e9be9fba 100644
> --- a/arch/riscv/kernel/cpu-hotplug.c
> +++ b/arch/riscv/kernel/cpu-hotplug.c
> @@ -57,7 +57,7 @@ void arch_cpuhp_cleanup_dead_cpu(unsigned int cpu)
>   	/* Verify from the firmware if the cpu is really stopped*/
>   	if (cpu_ops->cpu_is_stopped)
>   		ret = cpu_ops->cpu_is_stopped(cpu);
> -	if (ret)
> +	if (!ret)
>   		pr_warn("CPU%u may not have stopped: %d\n", cpu, ret);
>   }
>   
> diff --git a/arch/riscv/kernel/cpu_ops_sbi.c b/arch/riscv/kernel/cpu_ops_sbi.c
> index 00aff669f5f2..a17b7576b77d 100644
> --- a/arch/riscv/kernel/cpu_ops_sbi.c
> +++ b/arch/riscv/kernel/cpu_ops_sbi.c
> @@ -88,16 +88,14 @@ static void sbi_cpu_stop(void)
>   	pr_crit("Unable to stop the cpu %d (%d)\n", smp_processor_id(), ret);
>   }
>   
> -static int sbi_cpu_is_stopped(unsigned int cpuid)
> +static bool sbi_cpu_is_stopped(unsigned int cpuid)
>   {
>   	int rc;
>   	unsigned long hartid = cpuid_to_hartid_map(cpuid);
>   
>   	rc = sbi_hsm_hart_get_status(hartid);
>   
> -	if (rc == SBI_HSM_STATE_STOPPED)
> -		return 0;
> -	return rc;
> +	return (rc == SBI_HSM_STATE_STOPPED);
>   }
>   #endif
>   

Here is a some ambiguous behavior because pr_warn() in cpu-hotplug.c 
must send a cpu state number.
I would say it is needed to use sbi_hsm_hart_get_status(cpu) in pr_warn 
like this:

pr_warn("CPU%u may not have stopped: %d\n", cpu, 
sbi_hsm_hart_get_status(cpu))

so that we can define what is the current state of hart in case it is 
not SBI_HSM_STATE_STOPPED.