lkml.org 
[lkml]   [2020]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] arm64/cpufeature: Move BUG_ON() inside get_arm64_ftr_reg()
On Mon, May 25, 2020 at 05:22:23AM +0530, Anshuman Khandual wrote:
> On 05/21/2020 10:29 PM, Catalin Marinas wrote:
> > On Thu, May 21, 2020 at 05:22:15PM +0100, Will Deacon wrote:
> >> On Thu, May 21, 2020 at 08:45:38AM +0530, Anshuman Khandual wrote:
> >>> The existing code has BUG_ON() in three different callers doing exactly the
> >>> same thing that can easily be taken care in get_arm64_ftr_reg() itself. As
> >>> mentioned before an enum variable (as preferred - over a bool) can still
> >>> preserve the existing behavior for emulate_sys_reg().
> >>>
> >>> IMHO these are very good reasons for us to change the code which will make
> >>> it cleaner while also removing three redundant BUG_ON() instances. Hence I
> >>> will request you to please reconsider this proposal.
> >>
> >> Hmm, then how about trying my proposal with the WARN_ON(), but having a
> >> get_arm64_ftr_reg_nowarn() variant for the user emulation case?
[...]
> > read_sanitised_ftr_reg() would need to return something though. Would
> > all 0s be ok? I think it works as long as we don't have negative CPUID
> > fields.
>
> Just trying to understand. If get_arm64_ftr_reg() returns NULL, then
> read_sanitised_ftr_reg() should also return 0 for that non existent
> register (already been warned in get_arm64_ftr_reg).
>
> @@ -961,8 +972,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;
> }

Yes, as long as we also get the WARN_ON().

--
Catalin

\
 
 \ /
  Last update: 2020-05-26 14:20    [W:0.070 / U:0.884 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site