On Tue, Feb 18, 2025 at 12:57 PM Deepak Gupta <de...@rivosinc.com> wrote:
>
> Commit f06bfe3dc38c ("target/riscv: implement zicfiss instructions") adds
> `ssamoswap` instruction. `ssamoswap` takes the code-point from existing
> reserved encoding (and not a zimop like other shadow stack instructions).
> If shadow stack is not enabled (via xenvcfg.SSE), then `ssamoswap` must
> result in an illegal instruction exception. However there is a slightly
> modified behavior for M-mode.
>
> Shadow stack are not available in M-mode and all shadow stack instructions
> in M-mode exhibit zimop behavior. However, `ssamoswap` can still succeed
> if MPRV=1 and MPP is non-zero (see section 2.7 of zicfiss specification).
> This patch corrects that behavior for `ssamoswap`.Section "22.2.3. Shadow Stack Memory Protection " of the latest priv spec [1] seems to say: "When the effective privilege mode is M, any memory access by an SSAMOSWAP.W/D instruction will result in a store/AMO access-fault exception." 1: https://github.com/riscv/riscv-isa-manual/releases/tag/riscv-isa-release-9cfaf37-2025-03-06 Alistair > > Fixes: f06bfe3dc38c ("target/riscv: implement zicfiss instructions") > > Reported-by: Ved Shanbhogue <v...@rivosinc.com> > Signed-off-by: Deepak Gupta <de...@rivosinc.com> > --- > target/riscv/insn_trans/trans_rvzicfiss.c.inc | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/insn_trans/trans_rvzicfiss.c.inc > b/target/riscv/insn_trans/trans_rvzicfiss.c.inc > index e3ebc4977c..ec016cd70f 100644 > --- a/target/riscv/insn_trans/trans_rvzicfiss.c.inc > +++ b/target/riscv/insn_trans/trans_rvzicfiss.c.inc > @@ -15,6 +15,13 @@ > * You should have received a copy of the GNU General Public License along > with > * this program. If not, see <http://www.gnu.org/licenses/>. > */ > + > + #define REQUIRE_ZICFISS(ctx) do { \ > + if (!ctx->cfg_ptr->ext_zicfiss) { \ > + return false; \ > + } \ > +} while (0) > + > static bool trans_sspopchk(DisasContext *ctx, arg_sspopchk *a) > { > if (!ctx->bcfi_enabled) { > @@ -77,7 +84,8 @@ static bool trans_ssrdp(DisasContext *ctx, arg_ssrdp *a) > static bool trans_ssamoswap_w(DisasContext *ctx, arg_amoswap_w *a) > { > REQUIRE_A_OR_ZAAMO(ctx); > - if (!ctx->bcfi_enabled) { > + REQUIRE_ZICFISS(ctx); > + if ((ctx->priv != PRV_M) && !ctx->bcfi_enabled) { > return false; > } > > @@ -97,7 +105,8 @@ static bool trans_ssamoswap_d(DisasContext *ctx, > arg_amoswap_w *a) > { > REQUIRE_64BIT(ctx); > REQUIRE_A_OR_ZAAMO(ctx); > - if (!ctx->bcfi_enabled) { > + REQUIRE_ZICFISS(ctx); > + if ((ctx->priv != PRV_M) && !ctx->bcfi_enabled) { > return false; > } > > -- > 2.34.1 > >
