diff mbox series

[2/9] riscv: Fix riscv_online_cpu_vec

Message ID 20250207161939.46139-13-ajones@ventanamicro.com (mailing list archive)
State Superseded
Headers show
Series riscv: Unaligned access speed probing fixes and skipping | expand

Checks

Context Check Description
bjorn/pre-ci_am success Success
bjorn/build-rv32-defconfig success build-rv32-defconfig
bjorn/build-rv64-clang-allmodconfig success build-rv64-clang-allmodconfig
bjorn/build-rv64-gcc-allmodconfig success build-rv64-gcc-allmodconfig
bjorn/build-rv64-nommu-k210-defconfig success build-rv64-nommu-k210-defconfig
bjorn/build-rv64-nommu-k210-virt success build-rv64-nommu-k210-virt
bjorn/checkpatch warning checkpatch
bjorn/dtb-warn-rv64 success dtb-warn-rv64
bjorn/header-inline success header-inline
bjorn/kdoc success kdoc
bjorn/module-param success module-param
bjorn/verify-fixes success verify-fixes
bjorn/verify-signedoff success verify-signedoff

Commit Message

Andrew Jones Feb. 7, 2025, 4:19 p.m. UTC 
We shouldn't probe when we already know vector is unsupported and
we should probe when we see we don't yet know whether it's supported.
Furthermore, we should ensure we've set the access type to
unsupported when we don't have vector at all.

Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Clément Léger Feb. 7, 2025, 4:47 p.m. UTC | #1 
On 07/02/2025 17:19, Andrew Jones wrote:
> We shouldn't probe when we already know vector is unsupported and
> we should probe when we see we don't yet know whether it's supported.
> Furthermore, we should ensure we've set the access type to
> unsupported when we don't have vector at all.
> 
> Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index b7a8ff7ba6df..161964cf2abc 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -367,10 +367,12 @@ static void check_vector_unaligned_access(struct work_struct *work __always_unus
>  
>  static int riscv_online_cpu_vec(unsigned int cpu)
>  {
> -	if (!has_vector())
> +	if (!has_vector()) {
> +		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
>  		return 0;
> +	}
>  
> -	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
> +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
>  		return 0;
>  
>  	check_vector_unaligned_access_emulated(NULL);

Hi Andrew,

Wouldn't it be easier just not to register the hotplug callback in case
!has_vector() ? In which case just set all possible cpus
vector_misaligned_access to RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED
at startup.

Thanks,

Clément
Andrew Jones Feb. 7, 2025, 5:08 p.m. UTC | #2 
On Fri, Feb 07, 2025 at 05:47:28PM +0100, Clément Léger wrote:
> 
> 
> On 07/02/2025 17:19, Andrew Jones wrote:
> > We shouldn't probe when we already know vector is unsupported and
> > we should probe when we see we don't yet know whether it's supported.
> > Furthermore, we should ensure we've set the access type to
> > unsupported when we don't have vector at all.
> > 
> > Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> > index b7a8ff7ba6df..161964cf2abc 100644
> > --- a/arch/riscv/kernel/unaligned_access_speed.c
> > +++ b/arch/riscv/kernel/unaligned_access_speed.c
> > @@ -367,10 +367,12 @@ static void check_vector_unaligned_access(struct work_struct *work __always_unus
> >  
> >  static int riscv_online_cpu_vec(unsigned int cpu)
> >  {
> > -	if (!has_vector())
> > +	if (!has_vector()) {
> > +		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
> >  		return 0;
> > +	}
> >  
> > -	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
> > +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
> >  		return 0;
> >  
> >  	check_vector_unaligned_access_emulated(NULL);
> 
> Hi Andrew,
> 
> Wouldn't it be easier just not to register the hotplug callback in case
> !has_vector() ? In which case just set all possible cpus
> vector_misaligned_access to RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED
> at startup.

We could do that, but I have another use for the hotplug callback that
you'll see near the end of the series.

Thanks,
drew
Clément Léger Feb. 7, 2025, 5:43 p.m. UTC | #3 
On 07/02/2025 18:08, Andrew Jones wrote:
> On Fri, Feb 07, 2025 at 05:47:28PM +0100, Clément Léger wrote:
>>
>>
>> On 07/02/2025 17:19, Andrew Jones wrote:
>>> We shouldn't probe when we already know vector is unsupported and
>>> we should probe when we see we don't yet know whether it's supported.
>>> Furthermore, we should ensure we've set the access type to
>>> unsupported when we don't have vector at all.
>>>
>>> Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
>>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
>>> ---
>>>  arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
>>> index b7a8ff7ba6df..161964cf2abc 100644
>>> --- a/arch/riscv/kernel/unaligned_access_speed.c
>>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
>>> @@ -367,10 +367,12 @@ static void check_vector_unaligned_access(struct work_struct *work __always_unus
>>>  
>>>  static int riscv_online_cpu_vec(unsigned int cpu)
>>>  {
>>> -	if (!has_vector())
>>> +	if (!has_vector()) {
>>> +		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
>>>  		return 0;
>>> +	}
>>>  
>>> -	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
>>> +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
>>>  		return 0;
>>>  
>>>  	check_vector_unaligned_access_emulated(NULL);
>>
>> Hi Andrew,
>>
>> Wouldn't it be easier just not to register the hotplug callback in case
>> !has_vector() ? In which case just set all possible cpus
>> vector_misaligned_access to RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED
>> at startup.
> 
> We could do that, but I have another use for the hotplug callback that
> you'll see near the end of the series.

Ok, I saw patch 7/9.

BTW, what happened to patch 8 ? I thought it was in my spam but even
lore.kernel.org does not have it:
https://lore.kernel.org/linux-riscv/20250207161939.46139-11-ajones@ventanamicro.com/T/#t

Thanks,

Clément

> 
> Thanks,
> drew
Andrew Jones Feb. 7, 2025, 6:08 p.m. UTC | #4 
On Fri, Feb 07, 2025 at 06:43:26PM +0100, Clément Léger wrote:
> 
> 
> On 07/02/2025 18:08, Andrew Jones wrote:
> > On Fri, Feb 07, 2025 at 05:47:28PM +0100, Clément Léger wrote:
> >>
> >>
> >> On 07/02/2025 17:19, Andrew Jones wrote:
> >>> We shouldn't probe when we already know vector is unsupported and
> >>> we should probe when we see we don't yet know whether it's supported.
> >>> Furthermore, we should ensure we've set the access type to
> >>> unsupported when we don't have vector at all.
> >>>
> >>> Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
> >>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> >>> ---
> >>>  arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
> >>>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> >>> index b7a8ff7ba6df..161964cf2abc 100644
> >>> --- a/arch/riscv/kernel/unaligned_access_speed.c
> >>> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> >>> @@ -367,10 +367,12 @@ static void check_vector_unaligned_access(struct work_struct *work __always_unus
> >>>  
> >>>  static int riscv_online_cpu_vec(unsigned int cpu)
> >>>  {
> >>> -	if (!has_vector())
> >>> +	if (!has_vector()) {
> >>> +		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
> >>>  		return 0;
> >>> +	}
> >>>  
> >>> -	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
> >>> +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
> >>>  		return 0;
> >>>  
> >>>  	check_vector_unaligned_access_emulated(NULL);
> >>
> >> Hi Andrew,
> >>
> >> Wouldn't it be easier just not to register the hotplug callback in case
> >> !has_vector() ? In which case just set all possible cpus
> >> vector_misaligned_access to RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED
> >> at startup.
> > 
> > We could do that, but I have another use for the hotplug callback that
> > you'll see near the end of the series.
> 
> Ok, I saw patch 7/9.
> 
> BTW, what happened to patch 8 ? I thought it was in my spam but even
> lore.kernel.org does not have it:
> https://lore.kernel.org/linux-riscv/20250207161939.46139-11-ajones@ventanamicro.com/T/#t

I see it on lore for lkml, so linux-riscv dropped it for no good reason...
I'll send patch 8 again (this time with feeling) just to linux-riscv and
we'll see if gets through.

https://lore.kernel.org/all/20250207161939.46139-19-ajones@ventanamicro.com/

Thanks,
drew
Alexandre Ghiti Feb. 13, 2025, 1:02 p.m. UTC | #5 
On 07/02/2025 17:19, Andrew Jones wrote:
> We shouldn't probe when we already know vector is unsupported and
> we should probe when we see we don't yet know whether it's supported.
> Furthermore, we should ensure we've set the access type to
> unsupported when we don't have vector at all.
>
> Fixes: e7c9d66e313b ("RISC-V: Report vector unaligned access speed hwprobe")
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>   arch/riscv/kernel/unaligned_access_speed.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index b7a8ff7ba6df..161964cf2abc 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -367,10 +367,12 @@ static void check_vector_unaligned_access(struct work_struct *work __always_unus
>   
>   static int riscv_online_cpu_vec(unsigned int cpu)
>   {
> -	if (!has_vector())
> +	if (!has_vector()) {
> +		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
>   		return 0;
> +	}
>   
> -	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
> +	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
>   		return 0;
>   
>   	check_vector_unaligned_access_emulated(NULL);


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex
diff mbox series

Patch

diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index b7a8ff7ba6df..161964cf2abc 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -367,10 +367,12 @@  static void check_vector_unaligned_access(struct work_struct *work __always_unus
 
 static int riscv_online_cpu_vec(unsigned int cpu)
 {
-	if (!has_vector())
+	if (!has_vector()) {
+		per_cpu(vector_misaligned_access, cpu) = RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED;
 		return 0;
+	}
 
-	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNSUPPORTED)
+	if (per_cpu(vector_misaligned_access, cpu) != RISCV_HWPROBE_MISALIGNED_VECTOR_UNKNOWN)
 		return 0;
 
 	check_vector_unaligned_access_emulated(NULL);