qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] s390x: Beautify diag308 handling


From: David Hildenbrand
Subject: Re: [PATCH 3/4] s390x: Beautify diag308 handling
Date: Fri, 22 Nov 2019 11:56:59 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1

On 22.11.19 08:52, Janosch Frank wrote:
Let's improve readability by:
* Using constants for the subcodes
* Moving parameter checking into a function
* Removing subcode > 6 check as the default case catches that

Signed-off-by: Janosch Frank <address@hidden>
Reviewed-by: Cornelia Huck <address@hidden>
Reviewed-by: Thomas Huth <address@hidden>
---
  target/s390x/diag.c | 54 +++++++++++++++++++++++++++------------------
  1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 53c2f81f2a..18d33c8492 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -53,6 +53,29 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, 
uint64_t r3)
  #define DIAG_308_RC_NO_CONF         0x0102
  #define DIAG_308_RC_INVALID         0x0402
+#define DIAG308_RESET_MOD_CLR 0
+#define DIAG308_RESET_LOAD_NORM                1
+#define DIAG308_LOAD_CLEAR             3
+#define DIAG308_LOAD_NORMAL_DUMP       4
+#define DIAG308_SET                    5
+#define DIAG308_STORE                  6
+
+static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
+                              uintptr_t ra, bool write)
+{
+    if ((r1 & 1) || (addr & ~TARGET_PAGE_MASK)) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        return -1;
+    }
+    if (!address_space_access_valid(&address_space_memory, addr,
+                                    sizeof(IplParameterBlock), write,
+                                    MEMTXATTRS_UNSPECIFIED)) {
+        s390_program_interrupt(env, PGM_ADDRESSING, ra);
+        return -1;
+    }
+    return 0;
+}
+
  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t 
ra)
  {
      CPUState *cs = env_cpu(env);
@@ -65,30 +88,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, 
uint64_t r3, uintptr_t ra)
          return;
      }
- if ((subcode & ~0x0ffffULL) || (subcode > 6)) {
+    if (subcode & ~0x0ffffULL) {
          s390_program_interrupt(env, PGM_SPECIFICATION, ra);
          return;
      }
switch (subcode) {
-    case 0:
+    case DIAG308_RESET_MOD_CLR:
          s390_ipl_reset_request(cs, S390_RESET_MODIFIED_CLEAR);
          break;
-    case 1:
+    case DIAG308_RESET_LOAD_NORM:
          s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
          break;
-    case 3:
+    case DIAG308_LOAD_CLEAR:
+        /* Well we still lack the clearing bit... */
          s390_ipl_reset_request(cs, S390_RESET_REIPL);
          break;
-    case 5:
-        if ((r1 & 1) || (addr & 0x0fffULL)) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-            return;
-        }
-        if (!address_space_access_valid(&address_space_memory, addr,
-                                        sizeof(IplParameterBlock), false,
-                                        MEMTXATTRS_UNSPECIFIED)) {
-            s390_program_interrupt(env, PGM_ADDRESSING, ra);
+    case DIAG308_SET:
+        if (diag308_parm_check(env, r1, addr, ra, false)) {
              return;
          }
          iplb = g_new0(IplParameterBlock, 1);
@@ -110,15 +127,8 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, 
uint64_t r3, uintptr_t ra)
  out:
          g_free(iplb);
          return;
-    case 6:
-        if ((r1 & 1) || (addr & 0x0fffULL)) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-            return;
-        }
-        if (!address_space_access_valid(&address_space_memory, addr,
-                                        sizeof(IplParameterBlock), true,
-                                        MEMTXATTRS_UNSPECIFIED)) {
-            s390_program_interrupt(env, PGM_ADDRESSING, ra);
+    case DIAG308_STORE:
+        if (diag308_parm_check(env, r1, addr, ra, true)) {
              return;
          }
          iplb = s390_ipl_get_iplb();


Reviewed-by: David Hildenbrand <address@hidden>

--

Thanks,

David / dhildenb




reply via email to

[Prev in Thread] Current Thread [Next in Thread]