diff mbox

[v1,11/11] target/s390x: use program_interrupt() in per_check_exception()

Message ID 20170830170601.15855-12-david@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Hildenbrand Aug. 30, 2017, 5:06 p.m. UTC
I am not sure if we are handling ilen the right way here. ilen should
always match the instruction triggering the exception. This is relevant
for per exceptions triggered via EXECUTE instructions. The ilen to be
indicated has to match the EXECUTE instruction.

Clean it up for now but leave ilen as is, we can fix that later.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/misc_helper.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Thomas Huth Aug. 31, 2017, 9:37 a.m. UTC | #1
On 30.08.2017 19:06, David Hildenbrand wrote:
> I am not sure if we are handling ilen the right way here. ilen should
> always match the instruction triggering the exception. This is relevant
> for per exceptions triggered via EXECUTE instructions. The ilen to be
> indicated has to match the EXECUTE instruction.
> 
> Clean it up for now but leave ilen as is, we can fix that later.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/misc_helper.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index eb7accc0ce..ac9657f23f 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -445,14 +445,11 @@ 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);
> +        ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> +        program_interrupt(env, PGM_PER, ilen);
>      }
>  }

The changes basically look fine to me, but may I suggest to
1) Add a comment to the code about your concerns with ilen
2) Change the patch description to focus on the work that is actually
done here instead of only talking about your ilen concerns?

 Thanks,
  Thomas
diff mbox

Patch

diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index eb7accc0ce..ac9657f23f 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -445,14 +445,11 @@  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);
+        ilen = get_ilen(cpu_ldub_code(env, env->per_address));
+        program_interrupt(env, PGM_PER, ilen);
     }
 }