public inbox for linux-riscv@lists.infradead.org
 help / color / mirror / Atom feed
From: Chen Pei <cp0613@linux.alibaba.com>
To: wangruikang@iscas.ac.cn
Cc: pjw@kernel.org, anup@brainfault.org,
	andrew.jones@oss.qualcomm.com, guoren@kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	opensbi@lists.infradead.org
Subject: Re: [PATCH v2] riscv: smp: Align secondary_start_sbi to 4 bytes
Date: Wed, 15 Apr 2026 20:37:57 +0800	[thread overview]
Message-ID: <20260415123757.123581-1-cp0613@linux.alibaba.com> (raw)
In-Reply-To: <83397314-c7f3-4122-9587-80c29b1d13aa@iscas.ac.cn>

On Wed, 15 Apr 2026 11:39:22 +0800, wangruikang@iscas.ac.cn wrote:

> > During SMP boot, the secondary_start_sbi address is passed to the
> > slave core via sbi_hsm_hart_start. In OpenSBI, this address is
> > written to STVEC in sbi_hart_switch_mode.
> 
> This is certainly odd (OpenSBI git blames to initial git commit. No idea
> why this was needed.) but technically permitted by the SBI HSM spec (see
> below).
> 
> * Anup and other opensbi folks: Do you know/remember why it was there?
> It's not really a bug, but some historical context would be appreciated.

Yes, looking forward to responses from Anup and other opensbi folks.

> > According to the privileged specification, the BASE field of STVEC
> > must always be aligned on a 4-byte boundary. However, System.map
> > reveals that secondary_start_sbi is not a 4-byte aligned address.
> >
> > For example, the address of secondary_start_sbi is
> > 0xffffffff80001066, and the disassembly is as follows:
> >
> > Dump of assembler code from 0xffffffff80001052 to 0xffffffff8000107a:
> >    0xffffffff80001052 <_start+4178>:    c.nop   -11
> >    0xffffffff80001054 <_start+4180>:    auipc   gp,0x1a1f
> >    0xffffffff80001058 <_start+4184>:    addi    gp,gp,84
> >    0xffffffff8000105c <_start+4188>:    csrw    satp,a2
> >    0xffffffff80001060 <_start+4192>:    sfence.vma
> >    0xffffffff80001064 <_start+4196>:    ret
> >    0xffffffff80001066 <_start+4198>:    csrw    sie,zero
> >    0xffffffff8000106a <_start+4202>:    csrw    sip,zero
> >    0xffffffff8000106e <_start+4206>:    li      t0,2
> >    0xffffffff80001070 <_start+4208>:    csrw    scounteren,t0
> >    0xffffffff80001074 <_start+4212>:    auipc   gp,0x1a1f
> >    0xffffffff80001078 <_start+4216>:    addi    gp,gp,52
> >
> > When writing to STVEC at address 0xffffffff80001066, the actual
> > write address is 0xffffffff80001064, corresponding to the address
> > of the previous ret instruction.
> Also technically permitted.

Yes, but this is unintended behavior, and we should avoid it.

> > This is unexpected, and if an
> > interrupt occurs at this point, it will cause unpredictable results.
> >
> > However, secondary_start_sbi immediately masks all interrupts and
> > updates STVEC, so no problems occurred.
> 
> An interrupt cannot trap in between secondary_start_sbi starting and the
> interrupts getting masked individually. The only arch state guaranteed
> by HSM is [1]:
> 
> |Register Name | Register Value
> |satp          | 0
> |sstatus.SIE   | 0
> |a0            | hartid
> |a1            | `opaque` parameter
> |All other registers remain in an undefined state.
> 
> Of which sstatus.SIE = 0 means that interrupts are masked in Supervisor
> mode.
> > In summary, it is more reasonable to make secondary_start_sbi
> > satisfy 4-byte alignment.
> 
> Thus, it is unnecessary.
> 
> If this fixes a bug where an interrupt can trap into the wrong stvec for
> you, then your firmware is broken.

This isn't about fixing a bug (as mentioned earlier, it's highly unlikely
to happen), but rather the STVEC check mechanism added to QEMU discovered
this issue. Regarding the problem itself, I would appreciate any clarification
or modifications from OpenSBI, but currently this modification is the simplest
and most feasible.

> Michael's suggestion about SYM_CODE_START still stands, but if you do
> that, that's just a really minor refactoring, and in any case the
> current commit message cannot stand.
> 
> Vivian "dramforever" Wang
> 
> [1]:
> https://github.com/riscv-non-isa/riscv-sbi-doc/blob/v3.0/src/ext-hsm.adoc#function-hart-start-fid-0

Thanks,
Pei

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

      reply	other threads:[~2026-04-15 12:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-13 13:20 [PATCH v2] riscv: smp: Align secondary_start_sbi to 4 bytes cp0613
2026-04-15  2:06 ` Michael Ellerman
2026-04-15 12:37   ` Chen Pei
2026-04-15  3:39 ` Vivian Wang
2026-04-15 12:37   ` Chen Pei [this message]

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=20260415123757.123581-1-cp0613@linux.alibaba.com \
    --to=cp0613@linux.alibaba.com \
    --cc=andrew.jones@oss.qualcomm.com \
    --cc=anup@brainfault.org \
    --cc=guoren@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=opensbi@lists.infradead.org \
    --cc=pjw@kernel.org \
    --cc=wangruikang@iscas.ac.cn \
    /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