Nice catch. You are right. I was a bit confused after looking at current
xvisor and KVM port. They are delegating S mode interrupts to VS mode, as
per my understanding after looking at
https://github.com/kvm-riscv/linux/blob/riscv_kvm_master/arch/riscv/kvm/main.c
line no. 34. I will see if there is a way to decline a patch.
Thanks for pointing that out.
Regards,
Rajnesh
On Sun, Feb 23, 2020 at 8:40 PM Jose Martins <josemartin...@gmail.com>
wrote:
> No problem. But I'm failing to see what you mean. My reasoning was:
> the specification mandates that only VS mode interrupt bits are
> writable in hideleg, all the others must be hardwired to zero. This
> means the hypervisor can't really delegate S mode interrupts as you
> are saying. So, if this is implemented correctly, you will never get
> inside that if condition because of an HS interrupt. And all
> delegatable asynchronous exception values must be decremented. So,
> checking if this is an async exception should do the job.
>
> Jose
>
> On Sun, 23 Feb 2020 at 15:10, Rajnesh Kanwal <rajnesh.kanwa...@gmail.com>
> wrote:
> >
> > Hello Jose,
> >
> > Sorry I didn't see that as it hadn't became a part of the port. I don't
> know how
> > they proceed with same patches.
> >
> > Just to add, there is a minor problem with your patch. The cause value
> should
> > only be decremented by one for VS mode interrupts. In case if hypervisor
> has
> > delegated S mode interrupts then we should not decrement cause for those
> > interrupts.
> >
> > Regards,
> > Rajnesh
> >
> >
> > On Sun, Feb 23, 2020 at 7:41 PM Jose Martins <josemartin...@gmail.com>
> wrote:
> >>
> >> Hello rajnesh,
> >>
> >> I had already submitted almost this exact patch a few weeks ago.
> >>
> >> Jose
> >>
> >> On Sun, 23 Feb 2020 at 13:51, <rajnesh.kanwa...@gmail.com> wrote:
> >> >
> >> > From: Rajnesh Kanwal <rajnesh.kanwa...@gmail.com>
> >> >
> >> > Currently riscv_cpu_local_irq_pending is used to find out pending
> >> > interrupt and VS mode interrupts are being shifted to represent
> >> > S mode interrupts in this function. So when the cause returned by
> >> > this function is passed to riscv_cpu_do_interrupt to actually
> >> > forward the interrupt, the VS mode forwarding check does not work
> >> > as intended and interrupt is actually forwarded to hypervisor. This
> >> > patch fixes this issue.
> >> >
> >> > Signed-off-by: Rajnesh Kanwal <rajnesh.kanwa...@gmail.com>
> >> > ---
> >> > target/riscv/cpu_helper.c | 9 ++++++++-
> >> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> > index b9e90dfd9a..59535ecba6 100644
> >> > --- a/target/riscv/cpu_helper.c
> >> > +++ b/target/riscv/cpu_helper.c
> >> > @@ -46,7 +46,7 @@ static int
> riscv_cpu_local_irq_pending(CPURISCVState *env)
> >> > target_ulong pending = env->mip & env->mie &
> >> > ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
> >> > target_ulong vspending = (env->mip & env->mie &
> >> > - (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP))
> >> 1;
> >> > + (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));
> >> >
> >> > target_ulong mie = env->priv < PRV_M ||
> >> > (env->priv == PRV_M && mstatus_mie);
> >> > @@ -900,6 +900,13 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >> >
> >> > if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) &
> 1) &&
> >> > !force_hs_execp) {
> >> > + /*
> >> > + * See if we need to adjust cause. Yes if its VS
> mode interrupt
> >> > + * no if hypervisor has delegated one of hs mode's
> interrupt
> >> > + */
> >> > + if (cause == IRQ_VS_TIMER || cause == IRQ_VS_SOFT ||
> >> > + cause == IRQ_VS_EXT)
> >> > + cause = cause - 1;
> >> > /* Trap to VS mode */
> >> > } else if (riscv_cpu_virt_enabled(env)) {
> >> > /* Trap into HS mode, from virt */
> >> > --
> >> > 2.17.1
> >> >
> >> >
>
