Re: [PATCH] arm64: armv8_deprecated: Fix warning in isndep cpuhp starting process

From: Will Deacon <will@kernel.org>
To: "liwei (GF)" <liwei391@huawei.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Joel Granados <j.granados@samsung.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	xiexiuqi@huawei.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] arm64: armv8_deprecated: Fix warning in isndep cpuhp starting process
Date: Tue, 4 Jun 2024 16:15:36 +0100	[thread overview]
Message-ID: <20240604151535.GA20574@willie-the-truck> (raw)
In-Reply-To: <f1c64afa-91bf-4b2a-aa19-4918b3592ca4@huawei.com>

On Tue, May 21, 2024 at 11:10:43AM +0800, liwei (GF) wrote:
> 
> 
> On 2024/4/30 23:06, Will Deacon wrote:
> > On Tue, Apr 23, 2024 at 05:35:01PM +0800, Wei Li wrote:
> >> The function run_all_insn_set_hw_mode() is registered as startup callback
> >> of 'CPUHP_AP_ARM64_ISNDEP_STARTING', it invokes set_hw_mode() methods of
> >> all emulated instructions.
> >>
> >> As the STARTING callbacks are not expected to fail, if one of the
> >> set_hw_mode() fails, e.g. due to el0 mixed-endian is not supported for
> >> 'setend', it will report a warning:
> >>
> >> ```
> >> CPU[2] cannot support the emulation of setend
> >> CPU 2 UP state arm64/isndep:starting (136) failed (-22)
> >> CPU2: Booted secondary processor 0x0000000002 [0x414fd0c1]
> >> ```
> >>
> >> To fix it, add a check for INSN_UNAVAILABLE status and skip the process.
> >>
> >> Signed-off-by: Wei Li <liwei391@huawei.com>
> >> ---
> >>  arch/arm64/kernel/armv8_deprecated.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> >> index dd6ce86d4332..b776e7424fe9 100644
> >> --- a/arch/arm64/kernel/armv8_deprecated.c
> >> +++ b/arch/arm64/kernel/armv8_deprecated.c
> >> @@ -462,6 +462,9 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
> >>  	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> >>  		struct insn_emulation *insn = insn_emulations[i];
> >>  		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
> >> +		if (insn->status == INSN_UNAVAILABLE)
> >> +			continue;
> >> +
> >>  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
> >>  			pr_warn("CPU[%u] cannot support the emulation of %s",
> >>  				cpu, insn->name);
> > 
> > Isn't this part of the warning useful, though? ^^^
> > 
> 
> Sorry I missed your replay, I think the warning here is unexpected.
> 
> As far as setend is concerned, the 'INSN_UNAVAILABLE' state is marked in
> armv8_deprecated_init(), with a prompt message, and skipped in following
> register process too. So the warning here should be intended to indicate
> the situation where set_hw_mode() should succeed.

Thanks, I agree with you.

Will

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