| Message ID | 20220203200957.1434641-5-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: powerpc_excp improvements [6xx] (7/n) | expand |
On Thu, 3 Feb 2022, Fabiano Rosas wrote: > This only applies to the G2s, the other 6xx CPUs will not have this > vector registered. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > target/ppc/excp_helper.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index d855a275ca..e27e1c3c70 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -596,20 +596,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) > > switch (excp) { > case POWERPC_EXCP_CRITICAL: /* Critical input */ > - switch (excp_model) { > - case POWERPC_EXCP_40x: > - srr0 = SPR_40x_SRR2; > - srr1 = SPR_40x_SRR3; > - break; > - case POWERPC_EXCP_BOOKE: > - srr0 = SPR_BOOKE_CSRR0; > - srr1 = SPR_BOOKE_CSRR1; > - break; > - case POWERPC_EXCP_6xx: > - break; > - default: > - goto excp_invalid; It may not be a problem but this seems to change previous behaviour. To keep that you may need to test for G2 here, or rather move this whole case before the default case to avoid goto and be able to just fall through to invalid if CPU is not a G2 (unless we're Ok with an if the default case). Regards, BALATON Zoltan > - } > break; > case POWERPC_EXCP_MCHECK: /* Machine check exception */ > if (msr_me == 0) { > @@ -836,7 +822,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) > powerpc_excp_name(excp)); > break; > default: > - excp_invalid: > cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); > break; > } >
BALATON Zoltan <balaton@eik.bme.hu> writes: > On Thu, 3 Feb 2022, Fabiano Rosas wrote: >> This only applies to the G2s, the other 6xx CPUs will not have this >> vector registered. >> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> >> --- >> target/ppc/excp_helper.c | 15 --------------- >> 1 file changed, 15 deletions(-) >> >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >> index d855a275ca..e27e1c3c70 100644 >> --- a/target/ppc/excp_helper.c >> +++ b/target/ppc/excp_helper.c >> @@ -596,20 +596,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) >> >> switch (excp) { >> case POWERPC_EXCP_CRITICAL: /* Critical input */ >> - switch (excp_model) { >> - case POWERPC_EXCP_40x: >> - srr0 = SPR_40x_SRR2; >> - srr1 = SPR_40x_SRR3; >> - break; >> - case POWERPC_EXCP_BOOKE: >> - srr0 = SPR_BOOKE_CSRR0; >> - srr1 = SPR_BOOKE_CSRR1; >> - break; >> - case POWERPC_EXCP_6xx: >> - break; >> - default: >> - goto excp_invalid; > > It may not be a problem but this seems to change previous behaviour. To > keep that you may need to test for G2 here, or rather move this whole case > before the default case to avoid goto and be able to just fall through to > invalid if CPU is not a G2 (unless we're Ok with an if the default case). I should have been more explicit in the commit message, but that is on purpose. If another 6xx CPU incorrectly registers the CRITICAL exception, then we'll let it crash. This code needs to assume the work done in cpu_init is correct. Otherwise we'd have to check everything twice. This whole exception work is walking towards removing the POWERPC_EXCP identifiers because we have been misusing them as a way to identify individual CPUs.
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index d855a275ca..e27e1c3c70 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -596,20 +596,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) switch (excp) { case POWERPC_EXCP_CRITICAL: /* Critical input */ - switch (excp_model) { - case POWERPC_EXCP_40x: - srr0 = SPR_40x_SRR2; - srr1 = SPR_40x_SRR3; - break; - case POWERPC_EXCP_BOOKE: - srr0 = SPR_BOOKE_CSRR0; - srr1 = SPR_BOOKE_CSRR1; - break; - case POWERPC_EXCP_6xx: - break; - default: - goto excp_invalid; - } break; case POWERPC_EXCP_MCHECK: /* Machine check exception */ if (msr_me == 0) { @@ -836,7 +822,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) powerpc_excp_name(excp)); break; default: - excp_invalid: cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); break; }
This only applies to the G2s, the other 6xx CPUs will not have this vector registered. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- target/ppc/excp_helper.c | 15 --------------- 1 file changed, 15 deletions(-)