diff mbox series

lib: sbi_trap: Fix hstatus.SPVP update in sbi_trap_redirect()

Message ID 4446ff53a13d4feca0d17f84efd06b22@MSX-L201.msx.ad.zih.tu-dresden.de
State Accepted
Headers show
Series lib: sbi_trap: Fix hstatus.SPVP update in sbi_trap_redirect() | expand

Commit Message

Georg Kotheimer Aug. 14, 2020, 9:01 p.m. UTC
When redirecting from VS/VU-mode to HS-mode, hstatus.SPVP was set
to the value of mstatus.SPP, as according to the specification both
flags should be set to the same value.
However, the assignment of SPVP takes place before SPP itself is
updated, which results in SPVP having an outdated value.

Signed-off-by: Georg Kotheimer <georg.kotheimer@tu-dresden.de>
---
 lib/sbi/sbi_trap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Anup Patel Aug. 15, 2020, 5:08 p.m. UTC | #1
> -----Original Message-----
> From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of Georg
> Kotheimer
> Sent: 15 August 2020 02:31
> To: opensbi@lists.infradead.org
> Cc: Georg Kotheimer <georg.kotheimer@tu-dresden.de>
> Subject: [PATCH] lib: sbi_trap: Fix hstatus.SPVP update in sbi_trap_redirect()
> 
> When redirecting from VS/VU-mode to HS-mode, hstatus.SPVP was set to
> the value of mstatus.SPP, as according to the specification both flags should
> be set to the same value.
> However, the assignment of SPVP takes place before SPP itself is updated,
> which results in SPVP having an outdated value.
> 
> Signed-off-by: Georg Kotheimer <georg.kotheimer@tu-dresden.de>
> ---
>  lib/sbi/sbi_trap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c index 930119d..c2bd061
> 100644
> --- a/lib/sbi/sbi_trap.c
> +++ b/lib/sbi/sbi_trap.c
> @@ -123,7 +123,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>  		/* Update HSTATUS SPVP and SPV bits */
>  		hstatus = csr_read(CSR_HSTATUS);
>  		hstatus &= ~HSTATUS_SPVP;
> -		hstatus |= (regs->mstatus & MSTATUS_SPP) ?
> HSTATUS_SPVP : 0;
> +		hstatus |= (prev_mode == PRV_S) ? HSTATUS_SPVP : 0;
>  		hstatus &= ~HSTATUS_SPV;
>  		hstatus |= (prev_virt) ? HSTATUS_SPV : 0;
>  		csr_write(CSR_HSTATUS, hstatus);
> --
> 2.25.1
> 
> 
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Nice catch. Looks good to me.

Reviewed-by: Anup Patel <anup.patel@wdc.com>

Regards,
Anup
Anup Patel Aug. 17, 2020, 1:02 p.m. UTC | #2
> -----Original Message-----
> From: Anup Patel
> Sent: 15 August 2020 22:38
> To: Georg Kotheimer <georg.kotheimer@tu-dresden.de>;
> opensbi@lists.infradead.org
> Subject: RE: [PATCH] lib: sbi_trap: Fix hstatus.SPVP update in
> sbi_trap_redirect()
> 
> 
> 
> > -----Original Message-----
> > From: opensbi <opensbi-bounces@lists.infradead.org> On Behalf Of Georg
> > Kotheimer
> > Sent: 15 August 2020 02:31
> > To: opensbi@lists.infradead.org
> > Cc: Georg Kotheimer <georg.kotheimer@tu-dresden.de>
> > Subject: [PATCH] lib: sbi_trap: Fix hstatus.SPVP update in
> > sbi_trap_redirect()
> >
> > When redirecting from VS/VU-mode to HS-mode, hstatus.SPVP was set to
> > the value of mstatus.SPP, as according to the specification both flags
> > should be set to the same value.
> > However, the assignment of SPVP takes place before SPP itself is
> > updated, which results in SPVP having an outdated value.
> >
> > Signed-off-by: Georg Kotheimer <georg.kotheimer@tu-dresden.de>
> > ---
> >  lib/sbi/sbi_trap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c index
> > 930119d..c2bd061
> > 100644
> > --- a/lib/sbi/sbi_trap.c
> > +++ b/lib/sbi/sbi_trap.c
> > @@ -123,7 +123,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
> >  		/* Update HSTATUS SPVP and SPV bits */
> >  		hstatus = csr_read(CSR_HSTATUS);
> >  		hstatus &= ~HSTATUS_SPVP;
> > -		hstatus |= (regs->mstatus & MSTATUS_SPP) ?
> > HSTATUS_SPVP : 0;
> > +		hstatus |= (prev_mode == PRV_S) ? HSTATUS_SPVP : 0;
> >  		hstatus &= ~HSTATUS_SPV;
> >  		hstatus |= (prev_virt) ? HSTATUS_SPV : 0;
> >  		csr_write(CSR_HSTATUS, hstatus);
> > --
> > 2.25.1
> >
> >
> > --
> > opensbi mailing list
> > opensbi@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/opensbi
> 
> Nice catch. Looks good to me.
> 
> Reviewed-by: Anup Patel <anup.patel@wdc.com>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup
diff mbox series

Patch

diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 930119d..c2bd061 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -123,7 +123,7 @@  int sbi_trap_redirect(struct sbi_trap_regs *regs,
 		/* Update HSTATUS SPVP and SPV bits */
 		hstatus = csr_read(CSR_HSTATUS);
 		hstatus &= ~HSTATUS_SPVP;
-		hstatus |= (regs->mstatus & MSTATUS_SPP) ? HSTATUS_SPVP : 0;
+		hstatus |= (prev_mode == PRV_S) ? HSTATUS_SPVP : 0;
 		hstatus &= ~HSTATUS_SPV;
 		hstatus |= (prev_virt) ? HSTATUS_SPV : 0;
 		csr_write(CSR_HSTATUS, hstatus);