On Wed Jun 21, 2023 at 6:31 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 6/20/23 18:43, Nicholas Piggin wrote:
> > System call interrupts in ISA v3.1 CPUs add a LEV indication in SRR1
> > that corresponds with the LEV field of the instruction that caused the
> > interrupt.
> >
>
> Did we encounter any issue without this patch leading to this fix?
No, I just noticed it.
> If so, it will be great to talk about it in short if possible.
>
> > Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> > ---
> > This is unchanged, just taken out of the bigger series since it is
> > independent.
> >
> > Thanks,
> > Nick
> >
> > target/ppc/excp_helper.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 77bfc18734..c7550fea13 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -1591,6 +1591,10 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int
> > excp)
> > vhc->hypercall(cpu->vhyp, cpu);
> > return;
> > }
> > + if (env->insns_flags2 & PPC2_ISA310) {
> > + /* ISAv3.1 puts LEV into SRR1 */
> > + msr |= lev << 20;
>
> Since LEV values greater than 2 are reserved, should we do:
> msr |= (lev <= 2) ? lev << 20 : 0;
Oh good point. Actually I think we want this which would be a
separate patch going before this one, as it is a fix for sc
even for pre-v3.1:
Thanks
Nick
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 15a00bd4fa..297159280e 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4424,7 +4424,18 @@ static void gen_sc(DisasContext *ctx)
{
uint32_t lev;
- lev = (ctx->opcode >> 5) & 0x7F;
+ /*
+ * LEV is a 7-bit field, but the top 6 bits are treated as a reserved
+ * field (i.e., ignored). ISA v3.1 changes that to 5 bits, but that
+ * is for Ultravisor is not not supported, so keep using 6 and 1.
+ */
+ lev = (ctx->opcode >> 5) & 0x1;
gen_exception_err(ctx, POWERPC_SYSCALL, lev);
}
