[PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling

Atish Patra posted 3 patches 8 months, 3 weeks ago
There is a newer version of this series
[PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling
Posted by Atish Patra 8 months, 3 weeks ago
Save stval during exception handling so that it can be decoded to
figure out the details of exception type.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 tools/testing/selftests/kvm/include/riscv/processor.h | 1 +
 tools/testing/selftests/kvm/lib/riscv/handlers.S      | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
index 5f389166338c..f4a7d64fbe9a 100644
--- a/tools/testing/selftests/kvm/include/riscv/processor.h
+++ b/tools/testing/selftests/kvm/include/riscv/processor.h
@@ -95,6 +95,7 @@ struct ex_regs {
 	unsigned long epc;
 	unsigned long status;
 	unsigned long cause;
+	unsigned long stval;
 };
 
 #define NR_VECTORS  2
diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S
index aa0abd3f35bb..2884c1e8939b 100644
--- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
+++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
@@ -45,9 +45,11 @@
 	csrr  s0, CSR_SEPC
 	csrr  s1, CSR_SSTATUS
 	csrr  s2, CSR_SCAUSE
+	csrr  s3, CSR_STVAL
 	sd    s0, 248(sp)
 	sd    s1, 256(sp)
 	sd    s2, 264(sp)
+	sd    s3, 272(sp)
 .endm
 
 .macro restore_context

-- 
2.43.0
Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling
Posted by Andrew Jones 7 months, 3 weeks ago
On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
> Save stval during exception handling so that it can be decoded to
> figure out the details of exception type.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  tools/testing/selftests/kvm/include/riscv/processor.h | 1 +
>  tools/testing/selftests/kvm/lib/riscv/handlers.S      | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
> index 5f389166338c..f4a7d64fbe9a 100644
> --- a/tools/testing/selftests/kvm/include/riscv/processor.h
> +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
> @@ -95,6 +95,7 @@ struct ex_regs {
>  	unsigned long epc;
>  	unsigned long status;
>  	unsigned long cause;
> +	unsigned long stval;
>  };
>  
>  #define NR_VECTORS  2
> diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> index aa0abd3f35bb..2884c1e8939b 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
> +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> @@ -45,9 +45,11 @@
>  	csrr  s0, CSR_SEPC
>  	csrr  s1, CSR_SSTATUS
>  	csrr  s2, CSR_SCAUSE
> +	csrr  s3, CSR_STVAL
>  	sd    s0, 248(sp)
>  	sd    s1, 256(sp)
>  	sd    s2, 264(sp)
> +	sd    s3, 272(sp)

We can't add stval without also changing how much stack we allocate at the
top of this macro, but since we need to keep sp 16-byte aligned in order
to call C code (route_exception()) we'll need to decrement -8*36, not
-8*35. Or, we could just switch struct ex_regs to be the kernel's struct
pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and
the additional long is orig_a0.

>  .endm
>  
>  .macro restore_context

I guess we should restore stval too.

Thanks,
drew

> 
> -- 
> 2.43.0
> 
> 
> -- 
> kvm-riscv mailing list
> kvm-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kvm-riscv
Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling
Posted by Atish Patra 7 months, 3 weeks ago
On 4/25/25 6:50 AM, Andrew Jones wrote:
> On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
>> Save stval during exception handling so that it can be decoded to
>> figure out the details of exception type.
>>
>> Signed-off-by: Atish Patra <atishp@rivosinc.com>
>> ---
>>   tools/testing/selftests/kvm/include/riscv/processor.h | 1 +
>>   tools/testing/selftests/kvm/lib/riscv/handlers.S      | 2 ++
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
>> index 5f389166338c..f4a7d64fbe9a 100644
>> --- a/tools/testing/selftests/kvm/include/riscv/processor.h
>> +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
>> @@ -95,6 +95,7 @@ struct ex_regs {
>>   	unsigned long epc;
>>   	unsigned long status;
>>   	unsigned long cause;
>> +	unsigned long stval;
>>   };
>>   
>>   #define NR_VECTORS  2
>> diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S
>> index aa0abd3f35bb..2884c1e8939b 100644
>> --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
>> +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
>> @@ -45,9 +45,11 @@
>>   	csrr  s0, CSR_SEPC
>>   	csrr  s1, CSR_SSTATUS
>>   	csrr  s2, CSR_SCAUSE
>> +	csrr  s3, CSR_STVAL
>>   	sd    s0, 248(sp)
>>   	sd    s1, 256(sp)
>>   	sd    s2, 264(sp)
>> +	sd    s3, 272(sp)
> We can't add stval without also changing how much stack we allocate at the
> top of this macro, but since we need to keep sp 16-byte aligned in order
> to call C code (route_exception()) we'll need to decrement -8*36, not

Yes. Thanks for catching that.

> -8*35. Or, we could just switch struct ex_regs to be the kernel's struct
> pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and
> the additional long is orig_a0.

I think switching to pt_regs is better in terms of maintainability in 
the future.
I will do that.

>>   .endm
>>   
>>   .macro restore_context
> I guess we should restore stval too.

Do we ?  stval is written by hardware and doesn't contain any state of 
the interrupted program.
Once, the trap handler processes the trap using stval information, there 
is no need to restore it.

Am I missing something ?

> Thanks,
> drew
>
>> -- 
>> 2.43.0
>>
>>
>> -- 
>> kvm-riscv mailing list
>> kvm-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kvm-riscv
Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling
Posted by Andrew Jones 7 months, 3 weeks ago
On Mon, Apr 28, 2025 at 03:47:47PM -0700, Atish Patra wrote:
> 
> On 4/25/25 6:50 AM, Andrew Jones wrote:
> > On Mon, Mar 24, 2025 at 05:40:29PM -0700, Atish Patra wrote:
> > > Save stval during exception handling so that it can be decoded to
> > > figure out the details of exception type.
> > > 
> > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > ---
> > >   tools/testing/selftests/kvm/include/riscv/processor.h | 1 +
> > >   tools/testing/selftests/kvm/lib/riscv/handlers.S      | 2 ++
> > >   2 files changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
> > > index 5f389166338c..f4a7d64fbe9a 100644
> > > --- a/tools/testing/selftests/kvm/include/riscv/processor.h
> > > +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
> > > @@ -95,6 +95,7 @@ struct ex_regs {
> > >   	unsigned long epc;
> > >   	unsigned long status;
> > >   	unsigned long cause;
> > > +	unsigned long stval;
> > >   };
> > >   #define NR_VECTORS  2
> > > diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> > > index aa0abd3f35bb..2884c1e8939b 100644
> > > --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
> > > +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> > > @@ -45,9 +45,11 @@
> > >   	csrr  s0, CSR_SEPC
> > >   	csrr  s1, CSR_SSTATUS
> > >   	csrr  s2, CSR_SCAUSE
> > > +	csrr  s3, CSR_STVAL
> > >   	sd    s0, 248(sp)
> > >   	sd    s1, 256(sp)
> > >   	sd    s2, 264(sp)
> > > +	sd    s3, 272(sp)
> > We can't add stval without also changing how much stack we allocate at the
> > top of this macro, but since we need to keep sp 16-byte aligned in order
> > to call C code (route_exception()) we'll need to decrement -8*36, not
> 
> Yes. Thanks for catching that.
> 
> > -8*35. Or, we could just switch struct ex_regs to be the kernel's struct
> > pt_regs which has 36 unsigned longs. The 'badaddr' member is for stval and
> > the additional long is orig_a0.
> 
> I think switching to pt_regs is better in terms of maintainability in the
> future.
> I will do that.
> 
> > >   .endm
> > >   .macro restore_context
> > I guess we should restore stval too.
> 
> Do we ?  stval is written by hardware and doesn't contain any state of the
> interrupted program.
> Once, the trap handler processes the trap using stval information, there is
> no need to restore it.

True. It just felt unbalanced.

Thanks,
drew

> 
> Am I missing something ?
> 
> > Thanks,
> > drew
> > 
> > > -- 
> > > 2.43.0
> > > 
> > > 
> > > -- 
> > > kvm-riscv mailing list
> > > kvm-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/kvm-riscv
Re: [PATCH 1/3] KVM: riscv: selftests: Add stval to exception handling
Posted by Anup Patel 7 months, 3 weeks ago
On Tue, Mar 25, 2025 at 6:10 AM Atish Patra <atishp@rivosinc.com> wrote:
>
> Save stval during exception handling so that it can be decoded to
> figure out the details of exception type.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

LGTM.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  tools/testing/selftests/kvm/include/riscv/processor.h | 1 +
>  tools/testing/selftests/kvm/lib/riscv/handlers.S      | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
> index 5f389166338c..f4a7d64fbe9a 100644
> --- a/tools/testing/selftests/kvm/include/riscv/processor.h
> +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
> @@ -95,6 +95,7 @@ struct ex_regs {
>         unsigned long epc;
>         unsigned long status;
>         unsigned long cause;
> +       unsigned long stval;
>  };
>
>  #define NR_VECTORS  2
> diff --git a/tools/testing/selftests/kvm/lib/riscv/handlers.S b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> index aa0abd3f35bb..2884c1e8939b 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/handlers.S
> +++ b/tools/testing/selftests/kvm/lib/riscv/handlers.S
> @@ -45,9 +45,11 @@
>         csrr  s0, CSR_SEPC
>         csrr  s1, CSR_SSTATUS
>         csrr  s2, CSR_SCAUSE
> +       csrr  s3, CSR_STVAL
>         sd    s0, 248(sp)
>         sd    s1, 256(sp)
>         sd    s2, 264(sp)
> +       sd    s3, 272(sp)
>  .endm
>
>  .macro restore_context
>
> --
> 2.43.0
>