Re: [PATCH 2/3] lib/riscv: clear SDT when entering exception handling

From: Andrew Jones <andrew.jones@linux.dev>
To: "Clément Léger" <cleger@rivosinc.com>
Cc: kvm@vger.kernel.org, kvm-riscv@lists.infradead.org,
	 Andrew Jones <ajones@ventanamicro.com>,
	Ved Shanbhogue <ved@rivosinc.com>
Subject: Re: [PATCH 2/3] lib/riscv: clear SDT when entering exception handling
Date: Tue, 3 Jun 2025 10:16:46 +0200	[thread overview]
Message-ID: <20250603-8e4c2aa217e6be3b3ee43972@orel> (raw)
In-Reply-To: <20250523075341.1355755-3-cleger@rivosinc.com>

On Fri, May 23, 2025 at 09:53:09AM +0200, Clément Léger wrote:
> In order to avoid taking double trap once we have entered a trap and
> saved everything, clear SDT at the end of entry. This is not exactly
> required when double trap is disabled (probably most of the time), but
> that's not harmful.

Hmm... I wonder if this shouldn't be left to the handlers. Maybe
we should just provide a couple helpers in processor.h, such as

local_dlbtrp_enable()
local_dlbtrp_disable()

If we do need to manage this at save_context time, then I have
a couple comments below

> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  riscv/cstart.S | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/riscv/cstart.S b/riscv/cstart.S
> index 575f929b..a86f97f0 100644
> --- a/riscv/cstart.S
> +++ b/riscv/cstart.S
> @@ -212,14 +212,15 @@ secondary_entry:
>  	REG_S	t6, PT_T6(a0)			// x31
>  	csrr	a1, CSR_SEPC
>  	REG_S	a1, PT_EPC(a0)
> -	csrr	a1, CSR_SSTATUS
> -	REG_S	a1, PT_STATUS(a0)
>  	csrr	a1, CSR_STVAL
>  	REG_S	a1, PT_BADADDR(a0)
>  	csrr	a1, CSR_SCAUSE
>  	REG_S	a1, PT_CAUSE(a0)
>  	REG_L	a1, PT_ORIG_A0(a0)
>  	REG_S	a1, PT_A0(a0)
> +	li t0, 	SR_SDT
          ^    ^ should not be a tab
          ^ should be tabs

SR_SDT isn't defined until the next patch so this breaks compiling at this
point, which could break bisection. You can do a quick check of a series
for this with

 git rebase -i -x 'make' <base>

> +	csrrc 	a1, CSR_SSTATUS, t0
> +	REG_S	a1, PT_STATUS(a0)
>  .endm
>  
>  /*
> @@ -227,6 +228,8 @@ secondary_entry:
>   * Also restores a0.
>   */
>  .macro restore_context
> +	REG_L	a1, PT_STATUS(a0)
> +	csrw	CSR_SSTATUS, a1
>  	REG_L	ra, PT_RA(a0)			// x1
>  	REG_L	sp, PT_SP(a0)			// x2
>  	REG_L	gp, PT_GP(a0)			// x3
> @@ -260,8 +263,6 @@ secondary_entry:
>  	REG_L	t6, PT_T6(a0)			// x31
>  	REG_L	a1, PT_EPC(a0)
>  	csrw	CSR_SEPC, a1
> -	REG_L	a1, PT_STATUS(a0)
> -	csrw	CSR_SSTATUS, a1
>  	REG_L	a1, PT_BADADDR(a0)
>  	csrw	CSR_STVAL, a1
>  	REG_L	a1, PT_CAUSE(a0)
> -- 
> 2.49.0
>

Thanks,
drew

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