Re: [PATCH v2] riscv: smp: Align secondary_start_sbi to 4 bytes

[Chen Pei]

On Wed, 15 Apr 2026 11:39:22 +0800, wangruikang@xxxxxxxxxxx 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