linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
@ 2020-05-26 13:39 Anshuman Khandual
  2020-05-26 14:04 ` Suzuki K Poulose
  2020-05-26 15:01 ` Catalin Marinas
  0 siblings, 2 replies; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-26 13:39 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: mark.rutland, Suzuki K Poulose, Catalin Marinas,
	Anshuman Khandual, linux-kernel, Mark Brown, Will Deacon

There is no way to proceed when requested register could not be searched in
arm64_ftr_reg[]. Requesting for a non present register would be an error as
well. Hence lets just WARN_ON() when search fails in get_arm64_ftr_reg()
rather than checking for return value and doing a BUG_ON() instead in some
individual callers. But there are also caller instances that dont error out
when register search fails. Add a new helper get_arm64_ftr_reg_nowarn() for
such cases.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Changes in V2:

- Added get_arm64_ftr_reg_nowarn() per Will
- read_sanitised_ftr_reg() returns 0 when register search fails per Catalin

Changes in V1: (https://patchwork.kernel.org/patch/11559083/)

 arch/arm64/kernel/cpufeature.c | 42 +++++++++++++++++++++++-----------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index bc5048f152c1..f4555b9d145c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -548,16 +548,16 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
 }
 
 /*
- * get_arm64_ftr_reg - Lookup a feature register entry using its
- * sys_reg() encoding. With the array arm64_ftr_regs sorted in the
- * ascending order of sys_id , we use binary search to find a matching
+ * get_arm64_ftr_reg_nowarn - Looks up a feature register entry using
+ * its sys_reg() encoding. With the array arm64_ftr_regs sorted in the
+ * ascending order of sys_id, we use binary search to find a matching
  * entry.
  *
  * returns - Upon success,  matching ftr_reg entry for id.
  *         - NULL on failure. It is upto the caller to decide
  *	     the impact of a failure.
  */
-static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
+static struct arm64_ftr_reg *get_arm64_ftr_reg_nowarn(u32 sys_id)
 {
 	const struct __ftr_reg_entry *ret;
 
@@ -571,6 +571,28 @@ static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
 	return NULL;
 }
 
+/*
+ * get_arm64_ftr_reg - Looks up a feature register entry using
+ * its sys_reg() encoding. This calls get_arm64_ftr_reg_nowarn().
+ *
+ * returns - Upon success,  matching ftr_reg entry for id.
+ *         - NULL on failure but with an WARN_ON().
+ */
+static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id)
+{
+	struct arm64_ftr_reg *reg;
+
+	reg = get_arm64_ftr_reg_nowarn(sys_id);
+
+	/*
+	 * Can not really proceed when the search fails here.
+	 * Requesting for a non existent register search will
+	 * be an error. Warn but let it continue for now.
+	 */
+	WARN_ON(!reg);
+	return reg;
+}
+
 static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
 			       s64 ftr_val)
 {
@@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
 	const struct arm64_ftr_bits *ftrp;
 	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
 
-	BUG_ON(!reg);
-
 	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
 		u64 ftr_mask = arm64_ftr_mask(ftrp);
 		s64 ftr_new = arm64_ftr_value(ftrp, new);
@@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
 {
 	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
 
-	BUG_ON(!regp);
 	update_cpu_ftr_reg(regp, val);
 	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
 		return 0;
@@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
 	const struct arm64_ftr_bits *ftrp;
 	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
 
-	if (WARN_ON(!regp))
-		return;
-
 	for (ftrp = regp->ftr_bits; ftrp->width; ftrp++) {
 		if (ftrp->shift == field) {
 			regp->strict_mask &= ~arm64_ftr_mask(ftrp);
@@ -961,8 +977,8 @@ u64 read_sanitised_ftr_reg(u32 id)
 {
 	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(id);
 
-	/* We shouldn't get a request for an unsupported register */
-	BUG_ON(!regp);
+	if (!regp)
+		return 0;
 	return regp->sys_val;
 }
 
@@ -2565,7 +2581,7 @@ static int emulate_sys_reg(u32 id, u64 *valp)
 	if (sys_reg_CRm(id) == 0)
 		return emulate_id_reg(id, valp);
 
-	regp = get_arm64_ftr_reg(id);
+	regp = get_arm64_ftr_reg_nowarn(id);
 	if (regp)
 		*valp = arm64_ftr_reg_user_value(regp);
 	else
-- 
2.20.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-26 14:04 ` Suzuki K Poulose
@ 2020-05-26 14:03   ` Anshuman Khandual
  0 siblings, 0 replies; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-26 14:03 UTC (permalink / raw)
  To: Suzuki K Poulose, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, linux-kernel



On 05/26/2020 07:34 PM, Suzuki K Poulose wrote:
> On 05/26/2020 02:39 PM, Anshuman Khandual wrote:
>> There is no way to proceed when requested register could not be searched in
>> arm64_ftr_reg[]. Requesting for a non present register would be an error as
>> well. Hence lets just WARN_ON() when search fails in get_arm64_ftr_reg()
>> rather than checking for return value and doing a BUG_ON() instead in some
>> individual callers. But there are also caller instances that dont error out
>> when register search fails. Add a new helper get_arm64_ftr_reg_nowarn() for
>> such cases.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> Changes in V2:
>>
>> - Added get_arm64_ftr_reg_nowarn() per Will
>> - read_sanitised_ftr_reg() returns 0 when register search fails per Catalin
>>
>> Changes in V1: (https://patchwork.kernel.org/patch/11559083/)
>>
>>   arch/arm64/kernel/cpufeature.c | 42 +++++++++++++++++++++++-----------
>>   1 file changed, 29 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index bc5048f152c1..f4555b9d145c 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -548,16 +548,16 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
>>   }
>>   
> 
> ...
> 
>>   static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
>>                      s64 ftr_val)
>>   {
>> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
>>       const struct arm64_ftr_bits *ftrp;
>>       struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>>   -    BUG_ON(!reg);
>> -
>>       for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>>           u64 ftr_mask = arm64_ftr_mask(ftrp);
>>           s64 ftr_new = arm64_ftr_value(ftrp, new);
>> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
>>   {
>>       struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>>   -    BUG_ON(!regp);
>>       update_cpu_ftr_reg(regp, val);
>>       if ((boot & regp->strict_mask) == (val & regp->strict_mask))
>>           return 0;
>> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
>>       const struct arm64_ftr_bits *ftrp;
>>       struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>>   -    if (WARN_ON(!regp))
>> -        return;
>> -
> 
> You need to return here, on !regp. Rest looks fine to me.

Catalin had suggested and agreed on for this change in behavior here.
If the register is not found, there is already some problem.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-26 13:39 [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn() Anshuman Khandual
@ 2020-05-26 14:04 ` Suzuki K Poulose
  2020-05-26 14:03   ` Anshuman Khandual
  2020-05-26 15:01 ` Catalin Marinas
  1 sibling, 1 reply; 7+ messages in thread
From: Suzuki K Poulose @ 2020-05-26 14:04 UTC (permalink / raw)
  To: anshuman.khandual, linux-arm-kernel
  Cc: mark.rutland, catalin.marinas, broonie, will, linux-kernel

On 05/26/2020 02:39 PM, Anshuman Khandual wrote:
> There is no way to proceed when requested register could not be searched in
> arm64_ftr_reg[]. Requesting for a non present register would be an error as
> well. Hence lets just WARN_ON() when search fails in get_arm64_ftr_reg()
> rather than checking for return value and doing a BUG_ON() instead in some
> individual callers. But there are also caller instances that dont error out
> when register search fails. Add a new helper get_arm64_ftr_reg_nowarn() for
> such cases.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V2:
> 
> - Added get_arm64_ftr_reg_nowarn() per Will
> - read_sanitised_ftr_reg() returns 0 when register search fails per Catalin
> 
> Changes in V1: (https://patchwork.kernel.org/patch/11559083/)
> 
>   arch/arm64/kernel/cpufeature.c | 42 +++++++++++++++++++++++-----------
>   1 file changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index bc5048f152c1..f4555b9d145c 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -548,16 +548,16 @@ static int search_cmp_ftr_reg(const void *id, const void *regp)
>   }
>   

...

>   static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
>   			       s64 ftr_val)
>   {
> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
>   	const struct arm64_ftr_bits *ftrp;
>   	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>   
> -	BUG_ON(!reg);
> -
>   	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>   		u64 ftr_mask = arm64_ftr_mask(ftrp);
>   		s64 ftr_new = arm64_ftr_value(ftrp, new);
> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
>   {
>   	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>   
> -	BUG_ON(!regp);
>   	update_cpu_ftr_reg(regp, val);
>   	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
>   		return 0;
> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
>   	const struct arm64_ftr_bits *ftrp;
>   	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>   
> -	if (WARN_ON(!regp))
> -		return;
> -

You need to return here, on !regp. Rest looks fine to me.

Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-26 13:39 [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn() Anshuman Khandual
  2020-05-26 14:04 ` Suzuki K Poulose
@ 2020-05-26 15:01 ` Catalin Marinas
  2020-05-26 19:46   ` Will Deacon
  1 sibling, 1 reply; 7+ messages in thread
From: Catalin Marinas @ 2020-05-26 15:01 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: mark.rutland, Suzuki K Poulose, linux-kernel, Mark Brown,
	Will Deacon, linux-arm-kernel

On Tue, May 26, 2020 at 07:09:13PM +0530, Anshuman Khandual wrote:
> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
>  	const struct arm64_ftr_bits *ftrp;
>  	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>  
> -	BUG_ON(!reg);
> -
>  	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>  		u64 ftr_mask = arm64_ftr_mask(ftrp);
>  		s64 ftr_new = arm64_ftr_value(ftrp, new);
> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
>  {
>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>  
> -	BUG_ON(!regp);
>  	update_cpu_ftr_reg(regp, val);
>  	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
>  		return 0;
> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
>  	const struct arm64_ftr_bits *ftrp;
>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>  
> -	if (WARN_ON(!regp))
> -		return;

I think Will wanted an early return in all these functions not just
removing the BUG_ON(). I'll let him clarify.

-- 
Catalin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-26 15:01 ` Catalin Marinas
@ 2020-05-26 19:46   ` Will Deacon
  2020-05-27  2:26     ` Anshuman Khandual
  0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2020-05-26 19:46 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: mark.rutland, Suzuki K Poulose, Anshuman Khandual, linux-kernel,
	Mark Brown, linux-arm-kernel

On Tue, May 26, 2020 at 04:01:35PM +0100, Catalin Marinas wrote:
> On Tue, May 26, 2020 at 07:09:13PM +0530, Anshuman Khandual wrote:
> > @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
> >  	const struct arm64_ftr_bits *ftrp;
> >  	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
> >  
> > -	BUG_ON(!reg);
> > -
> >  	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> >  		u64 ftr_mask = arm64_ftr_mask(ftrp);
> >  		s64 ftr_new = arm64_ftr_value(ftrp, new);
> > @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
> >  {
> >  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
> >  
> > -	BUG_ON(!regp);
> >  	update_cpu_ftr_reg(regp, val);
> >  	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
> >  		return 0;
> > @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
> >  	const struct arm64_ftr_bits *ftrp;
> >  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
> >  
> > -	if (WARN_ON(!regp))
> > -		return;
> 
> I think Will wanted an early return in all these functions not just
> removing the BUG_ON(). I'll let him clarify.

Yes, the callers need to check the pointer and return early.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-26 19:46   ` Will Deacon
@ 2020-05-27  2:26     ` Anshuman Khandual
  2020-05-27  7:53       ` Will Deacon
  0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2020-05-27  2:26 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: mark.rutland, Mark Brown, linux-kernel, linux-arm-kernel,
	Suzuki K Poulose



On 05/27/2020 01:16 AM, Will Deacon wrote:
> On Tue, May 26, 2020 at 04:01:35PM +0100, Catalin Marinas wrote:
>> On Tue, May 26, 2020 at 07:09:13PM +0530, Anshuman Khandual wrote:
>>> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
>>>  	const struct arm64_ftr_bits *ftrp;
>>>  	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
>>>  
>>> -	BUG_ON(!reg);
>>> -
>>>  	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
>>>  		u64 ftr_mask = arm64_ftr_mask(ftrp);
>>>  		s64 ftr_new = arm64_ftr_value(ftrp, new);
>>> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
>>>  {
>>>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>>>  
>>> -	BUG_ON(!regp);
>>>  	update_cpu_ftr_reg(regp, val);
>>>  	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
>>>  		return 0;
>>> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
>>>  	const struct arm64_ftr_bits *ftrp;
>>>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
>>>  
>>> -	if (WARN_ON(!regp))
>>> -		return;
>>
>> I think Will wanted an early return in all these functions not just
>> removing the BUG_ON(). I'll let him clarify.
> 
> Yes, the callers need to check the pointer and return early.

Sure, will do. But for check_update_ftr_reg(), a feature register search
failure should be treated as a success (0) or a failure (1). What should
it return ? Seems bit tricky, as there are good reasons to go either way.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn()
  2020-05-27  2:26     ` Anshuman Khandual
@ 2020-05-27  7:53       ` Will Deacon
  0 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2020-05-27  7:53 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: mark.rutland, Suzuki K Poulose, Catalin Marinas, linux-kernel,
	Mark Brown, linux-arm-kernel

On Wed, May 27, 2020 at 07:56:30AM +0530, Anshuman Khandual wrote:
> 
> 
> On 05/27/2020 01:16 AM, Will Deacon wrote:
> > On Tue, May 26, 2020 at 04:01:35PM +0100, Catalin Marinas wrote:
> >> On Tue, May 26, 2020 at 07:09:13PM +0530, Anshuman Khandual wrote:
> >>> @@ -632,8 +654,6 @@ static void __init init_cpu_ftr_reg(u32 sys_reg, u64 new)
> >>>  	const struct arm64_ftr_bits *ftrp;
> >>>  	struct arm64_ftr_reg *reg = get_arm64_ftr_reg(sys_reg);
> >>>  
> >>> -	BUG_ON(!reg);
> >>> -
> >>>  	for (ftrp = reg->ftr_bits; ftrp->width; ftrp++) {
> >>>  		u64 ftr_mask = arm64_ftr_mask(ftrp);
> >>>  		s64 ftr_new = arm64_ftr_value(ftrp, new);
> >>> @@ -762,7 +782,6 @@ static int check_update_ftr_reg(u32 sys_id, int cpu, u64 val, u64 boot)
> >>>  {
> >>>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
> >>>  
> >>> -	BUG_ON(!regp);
> >>>  	update_cpu_ftr_reg(regp, val);
> >>>  	if ((boot & regp->strict_mask) == (val & regp->strict_mask))
> >>>  		return 0;
> >>> @@ -776,9 +795,6 @@ static void relax_cpu_ftr_reg(u32 sys_id, int field)
> >>>  	const struct arm64_ftr_bits *ftrp;
> >>>  	struct arm64_ftr_reg *regp = get_arm64_ftr_reg(sys_id);
> >>>  
> >>> -	if (WARN_ON(!regp))
> >>> -		return;
> >>
> >> I think Will wanted an early return in all these functions not just
> >> removing the BUG_ON(). I'll let him clarify.
> > 
> > Yes, the callers need to check the pointer and return early.
> 
> Sure, will do. But for check_update_ftr_reg(), a feature register search
> failure should be treated as a success (0) or a failure (1). What should
> it return ? Seems bit tricky, as there are good reasons to go either way.

We're unable to check it so return 0, otherwise we'll randomly taint the
kernel and print a weird message.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-27  7:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 13:39 [PATCH V2] arm64/cpufeature: Add get_arm64_ftr_reg_nowarn() Anshuman Khandual
2020-05-26 14:04 ` Suzuki K Poulose
2020-05-26 14:03   ` Anshuman Khandual
2020-05-26 15:01 ` Catalin Marinas
2020-05-26 19:46   ` Will Deacon
2020-05-27  2:26     ` Anshuman Khandual
2020-05-27  7:53       ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).