Date: Tue, 09 Jan 2024 11:40:19 +0000
Message-ID: <87plyau44c.wl-maz@kernel.org>
From: Marc Zyngier <maz@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.linux.dev,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: [PATCH v3 02/13] arm64: cpufeatures: Correctly handle signed values
In-Reply-To: <86a5pfafbv.wl-maz@kernel.org>
References: <20231127114559.990314-1-maz@kernel.org>
	<20231127114559.990314-3-maz@kernel.org>
	<20231211122416.GA25277@willie-the-truck>
	<86a5pfafbv.wl-maz@kernel.org>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
Precedence: bulk
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII

On Mon, 08 Jan 2024 17:46:12 +0000,
Marc Zyngier <maz@kernel.org> wrote:
> 
> On Mon, 11 Dec 2023 12:24:16 +0000,
> Will Deacon <will@kernel.org> wrote:

[...]

> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 646591c67e7a..bc8787f28ffd 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -140,12 +140,42 @@ void dump_cpu_features(void)
> > >  	pr_emerg("0x%*pb\n", ARM64_NCAPS, &system_cpucaps);
> > >  }
> > >  
> > > +#define __ARM64_MAX_POSITIVE(reg, field)				\
> > > +		((reg##_##field##_SIGNED ?				\
> > > +		  BIT(reg##_##field##_WIDTH - 1) :			\
> > > +		  BIT(reg##_##field##_WIDTH)) - 1)
> > > +
> > > +#define __ARM64_MIN_NEGATIVE(reg, field)  BIT(reg##_##field##_WIDTH - 1)
> > 
> > I'm struggling to grok these two macros. For example, let's say I have a
> > 4-bit signed field. In that case, the maximum positive value is 7 (0b0111)
> > and the minimum negative value is -8 (0b1000), but the macros above appear
> > to give 0b1000 for both.
> 
> Crap. Well spotted. The signed maximum needs to be further adjusted
> like this:

Actually, scratch that. The original code is correct and does return
0b0111 for the max of a signed 4 bit value, and 0b1000 for the min.
The clue is in the bracketing (the '- 1' is applied to the result of
the conditional expression in the max case).

	M.

-- 
Without deviation from the norm, progress is not possible.