On Thu, 7 Sep 2017 15:52:19 +0200
David Hildenbrand <da...@redhat.com> wrote:
> On 07.09.2017 15:32, Cornelia Huck wrote:
> > On Mon, 4 Sep 2017 17:43:07 +0200
> > David Hildenbrand <da...@redhat.com> wrote:
> >
> >> Clean it up by reusing program_interrupt(). Add a concern regarding
> >> ilen.
> >>
> >> Signed-off-by: David Hildenbrand <da...@redhat.com>
> >> ---
> >> target/s390x/misc_helper.c | 15 +++++++++------
> >> 1 file changed, 9 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> >> index 57c02ddf1b..5096286157 100644
> >> --- a/target/s390x/misc_helper.c
> >> +++ b/target/s390x/misc_helper.c
> >> @@ -446,14 +446,17 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
> >> #ifndef CONFIG_USER_ONLY
> >> void HELPER(per_check_exception)(CPUS390XState *env)
> >> {
> >> - CPUState *cs = CPU(s390_env_get_cpu(env));
> >> + uint32_t ilen;
> >>
> >> if (env->per_perc_atmid) {
> >> - env->int_pgm_code = PGM_PER;
> >> - env->int_pgm_ilen = get_ilen(cpu_ldub_code(env,
> >> env->per_address));
> >> -
> >> - cs->exception_index = EXCP_PGM;
> >> - cpu_loop_exit(cs);
> >> + /*
> >> + * FIXME: ILEN_AUTO is most probably the right thing to use. ilen
> >> + * always has to match the instruction referenced in the PSW. E.g.
> >> + * if a PER interrupt is triggered via EXECUTE, we have to use
> >> ilen
> >> + * of EXECUTE, while per_address contains the target of EXECUTE.
> >> + */
> >> + ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> >> + program_interrupt(env, PGM_PER, ilen);
> >> }
> >> }
> >>
> >
> > Wrapping my head around it: You preserve the current behavior, which
> > should probably be changed to use ILEN_AUTO to handle cases like
> > EXECUTE correctly?
>
> Exactly, e.g. if EXECUTE executes instruction Y, and Y is to generate an
> IFETCH event, AFAIK
>
> a) per_address points at Y
> b) old PGM PSW points at instruction after EXECUTE (ignoring nullifying
> for now)
> c) PGM ilen has to match EXECUTE (so the PSW can properly be rewound)
>
> The case where per_address == (PGM PSW - ilen) (ignoring nullification)
> should happen without EXECUTE being involved. That's why PER currently
> works just fine with Linux (e.g. uprobe smoke tests when kernel boots up).
>
> Execute handling with PER is just nasty. And one first has to find out
> how that plays together with EXECUTE handling in TCG. Therefore, no easy
> fix (although ILEN_AUTO might most probably really be the right thing to
> do).
That really sounds like a lot of fun...
>
> >
> > Do you plan to do a follow up? (Should I spend some cycles on it?)
> >
>
> Somewhere on my list. The next thing (in this area) I want do is write
> kvm-unit-tests for PER, because also KVM could benefit from that
> (EXECUTE handling is just nasty).
>
> So sure, go ahead and have a look at it if you have some spare cycles :)
Spare cycles? Me? :)
I think we can live with the current state a bit longer. Adding the
FIXME is actually an improvement...
