Linux-MIPS Archive mirror
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Rosen Penev <rosenp@gmail.com>
Cc: linux-mips@vger.kernel.org,
	 Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	 Nathan Chancellor <nathan@kernel.org>,
	 Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	 Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	 open list <linux-kernel@vger.kernel.org>,
	 "open list:CLANG/LLVM BUILD SUPPORT:Keyword:b(?i:clang|llvm)b"
	<llvm@lists.linux.dev>
Subject: Re: [PATCH] mips: cps: Assemble jr.hb with an R2 ISA level
Date: Fri, 8 May 2026 13:59:15 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2605080101340.46195@angie.orcam.me.uk> (raw)
In-Reply-To: <20260507232323.489383-1-rosenp@gmail.com>

On Thu, 7 May 2026, Rosen Penev wrote:

> diff --git a/arch/mips/kernel/cps-vec.S b/arch/mips/kernel/cps-vec.S
> index 2ae7034a3d5c..70413c816eb0 100644
> --- a/arch/mips/kernel/cps-vec.S
> +++ b/arch/mips/kernel/cps-vec.S
> @@ -373,8 +373,11 @@ LEAF(mips_cps_boot_vpes)
>  	.set	pop
>  
>  	PTR_LA	t1, 1f
> +	.set	push
> +	.set	MIPS_ISA_LEVEL_RAW
>  	jr.hb	t1
>  	 nop
> +	.set	pop
>  1:	mfc0	t1, CP0_MVPCONTROL
>  	ori	t1, t1, MVPCONTROL_VPC
>  	mtc0	t1, CP0_MVPCONTROL
> @@ -487,8 +490,11 @@ LEAF(mips_cps_boot_vpes)
>  	li	t0, TCHALT_H
>  	mtc0	t0, CP0_TCHALT
>  	PTR_LA	t0, 1f
> +	.set	push
> +	.set	MIPS_ISA_LEVEL_RAW
>  1:	jr.hb	t0
>  	 nop
> +	.set	pop
>  
>  2:
>  

 This seems exceedingly pedantic to me at the cost of cluttering code with 
these .set push/pop blocks.  Why not simply wrap the whole code block down 
to the end of the CONFIG_MIPS_MT_SMP conditional instead, analogously to 
how mips_cps_core_init() has been arranged?  The presence of MT implies R2 
after all.

 Ah, I can see we have MIPS_ISA_LEVEL_RAW always hardcoded to a 64-bit one 
and commit 8dbc1864b74f ("MIPS: CPS: Fix MIPS_ISA_LEVEL_RAW fallout") 
reporting the MOVE macro getting expanded to a 64-bit DADDU instruction on 
64-bit ISAs, which is obviously not incorrect (you get what you asked 
for), though also problematic in this use case.

 This is actually no longer relevant however, as this was changed back in 
2025 with binutils commit 40fc1451c63d ("[MIPS] Map 'move' to 'or'.") 
targetting version 2.26, while our requirement nowadays is 2.30, according 
to Documentation/process/changes.rst.  It is a fairly recent requirement 
though, from Linux 6.16 only, and the previous one was 2.25, so backports 
would be affected and it might be too early to rely on for a bug fix.

 Still my guts' feeling is we should define MIPS_ISA_LEVEL_RAW and the 
other such macros to a 32-bit or 64-bit ISA according to the machine word 
size of the CPU/ISA chosen for the whole kernel build.  This would enable 
the cleanup I suggested at the top with no reservation and remove the risk 
of GAS choosing to produce 64-bit instructions where a 32-bit CPU/ISA has 
been chosen in the configuration.

 You are welcome to do that, however as it stands your patch is not 
incorrect and fixes a real problem, so:

Reviewed-by: Maciej W. Rozycki <macro@orcam.me.uk>

  Maciej

  reply	other threads:[~2026-05-08 12:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-07 23:23 [PATCH] mips: cps: Assemble jr.hb with an R2 ISA level Rosen Penev
2026-05-08 12:59 ` Maciej W. Rozycki [this message]
2026-05-26 14:53 ` Thomas Bogendoerfer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2605080101340.46195@angie.orcam.me.uk \
    --to=macro@orcam.me.uk \
    --cc=justinstitt@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=rosenp@gmail.com \
    --cc=tsbogend@alpha.franken.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).