[Qemu-devel] [PATCH 0/7] s390 fixes

[Qemu-devel] [PATCH 0/7] s390 fixes

[Christian Borntraeger]

Alex,

here is a bunch of fixes/cleanups for s390.

Heinz Graalfs (1):
  s390/sclpconsole: handle char layer busy conditions

Thomas Huth (6):
  s390x/ioinst: Add missing alignment checks for IO instructions
  s390x/ioinst: Throw addressing exception when memory_map failed
  s390x/ioinst: Fixed alignment check in SCHM instruction
  s390x/ioinst: Fixed priority of operand exceptions
  s390x/kvm: Reworked/fixed handling of cc3 in kvm_handle_css_inst()
  s390x/kvm: Remove redundant return code

 hw/char/sclpconsole.c | 18 +-------------
 target-s390x/ioinst.c | 65 ++++++++++++++++++++++++++++++++++-----------------
 target-s390x/kvm.c    | 64 +++++++++++---------------------------------------
 3 files changed, 57 insertions(+), 90 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/7] s390/sclpconsole: handle char layer busy conditions

[Christian Borntraeger]

From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>

Handle busy conditions (errno=EAGAIN) in char layer by using
the new char layer in the sclp console.

Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/char/sclpconsole.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index bcc7893..eb3988c 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -184,8 +184,6 @@ static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
 static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
                                   size_t len)
 {
-    ssize_t ret = 0;
-    const uint8_t *iov_offset;
     SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
 
     if (!scon->chr) {
@@ -193,21 +191,7 @@ static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
         return len;
     }
 
-    iov_offset = buf;
-    while (len > 0) {
-        ret = qemu_chr_fe_write(scon->chr, buf, len);
-        if (ret == 0) {
-            /* a pty doesn't seem to be connected - no error */
-            len = 0;
-        } else if (ret == -EAGAIN || (ret > 0 && ret < len)) {
-            len -= ret;
-            iov_offset += ret;
-        } else {
-            len = 0;
-        }
-    }
-
-    return ret;
+    return qemu_chr_fe_write_all(scon->chr, buf, len);
 }
 
 static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/7] s390x/ioinst: Add missing alignment checks for IO instructions

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

The IO instructions MSCH, SSCH, STSCH, TSCH, STCRW and TPI require
that the second operand address must be aligned on a word boundary.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/ioinst.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 28c508d..91cc41b 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -157,6 +157,10 @@ int ioinst_handle_msch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     trace_ioinst_sch_id("msch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
     schib = s390_cpu_physical_memory_map(env, addr, &len, 0);
     if (!schib || len != sizeof(*schib)) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -228,6 +232,10 @@ int ioinst_handle_ssch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     trace_ioinst_sch_id("ssch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
     orig_orb = s390_cpu_physical_memory_map(env, addr, &len, 0);
     if (!orig_orb || len != sizeof(*orig_orb)) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -272,6 +280,10 @@ int ioinst_handle_stcrw(CPUS390XState *env, uint32_t ipb)
     hwaddr len = sizeof(*crw);
 
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
     crw = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!crw || len != sizeof(*crw)) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -300,6 +312,10 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     trace_ioinst_sch_id("stsch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
     schib = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!schib || len != sizeof(*schib)) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -345,6 +361,10 @@ int ioinst_handle_tsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     trace_ioinst_sch_id("tsch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
     irb = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!irb || len != sizeof(*irb)) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -625,6 +645,11 @@ int ioinst_handle_tpi(CPUS390XState *env, uint32_t ipb)
 
     trace_ioinst("tpi");
     addr = decode_basedisp_s(env, ipb);
+    if (addr & 3) {
+        program_interrupt(env, PGM_SPECIFICATION, 2);
+        return -EIO;
+    }
+
     lowcore = addr ? 0 : 1;
     len = lowcore ? 8 /* two words */ : 12 /* three words */;
     orig_len = len;
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/7] s390x/ioinst: Throw addressing exception when memory_map failed

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

So far, the IO instructions were throwing specification exceptions when
there was a problem with accessing the memory. However, the better way
is to throw an addressing exception instead.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/ioinst.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 91cc41b..0dc258f 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -163,7 +163,7 @@ int ioinst_handle_msch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     schib = s390_cpu_physical_memory_map(env, addr, &len, 0);
     if (!schib || len != sizeof(*schib)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         cc = -EIO;
         goto out;
     }
@@ -238,7 +238,7 @@ int ioinst_handle_ssch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     orig_orb = s390_cpu_physical_memory_map(env, addr, &len, 0);
     if (!orig_orb || len != sizeof(*orig_orb)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         cc = -EIO;
         goto out;
     }
@@ -286,7 +286,7 @@ int ioinst_handle_stcrw(CPUS390XState *env, uint32_t ipb)
     }
     crw = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!crw || len != sizeof(*crw)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         cc = -EIO;
         goto out;
     }
@@ -318,7 +318,7 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     schib = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!schib || len != sizeof(*schib)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         cc = -EIO;
         goto out;
     }
@@ -367,7 +367,7 @@ int ioinst_handle_tsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     }
     irb = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!irb || len != sizeof(*irb)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         cc = -EIO;
         goto out;
     }
@@ -600,7 +600,7 @@ int ioinst_handle_chsc(CPUS390XState *env, uint32_t ipb)
     }
     req = s390_cpu_physical_memory_map(env, addr, &map_size, 1);
     if (!req || map_size != TARGET_PAGE_SIZE) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         ret = -EIO;
         goto out;
     }
@@ -655,7 +655,7 @@ int ioinst_handle_tpi(CPUS390XState *env, uint32_t ipb)
     orig_len = len;
     int_code = s390_cpu_physical_memory_map(env, addr, &len, 1);
     if (!int_code || (len != orig_len)) {
-        program_interrupt(env, PGM_SPECIFICATION, 2);
+        program_interrupt(env, PGM_ADDRESSING, 2);
         ret = -EIO;
         goto out;
     }
-- 
1.8.1.4

[Qemu-devel] [PATCH 4/7] s390x/ioinst: Fixed alignment check in SCHM instruction

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

Register 2 only has to be aligned to a 32-byte boundary, not a
full page boundary.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/ioinst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 0dc258f..098bd8d 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -688,7 +688,7 @@ int ioinst_handle_schm(CPUS390XState *env, uint64_t reg1, uint64_t reg2,
     update = SCHM_REG1_UPD(reg1);
     dct = SCHM_REG1_DCT(reg1);
 
-    if (update && (reg2 & 0x0000000000000fff)) {
+    if (update && (reg2 & 0x000000000000001f)) {
         program_interrupt(env, PGM_OPERAND, 2);
         return -EIO;
     }
-- 
1.8.1.4

[Qemu-devel] [PATCH 5/7] s390x/ioinst: Fixed priority of operand exceptions

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

Operand exceptions have a lower priority than specification and address
exceptions. Thus the checks for operand exceptions must be done later.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/ioinst.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 098bd8d..85fd285 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -151,11 +151,6 @@ int ioinst_handle_msch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     int cc;
     hwaddr len = sizeof(*schib);
 
-    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(env, PGM_OPERAND, 2);
-        return -EIO;
-    }
-    trace_ioinst_sch_id("msch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
     if (addr & 3) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -167,11 +162,13 @@ int ioinst_handle_msch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
         cc = -EIO;
         goto out;
     }
-    if (!ioinst_schib_valid(schib)) {
+    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
+        !ioinst_schib_valid(schib)) {
         program_interrupt(env, PGM_OPERAND, 2);
         cc = -EIO;
         goto out;
     }
+    trace_ioinst_sch_id("msch", cssid, ssid, schid);
     sch = css_find_subch(m, cssid, ssid, schid);
     if (sch && css_subch_visible(sch)) {
         ret = css_do_msch(sch, schib);
@@ -226,11 +223,6 @@ int ioinst_handle_ssch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     int cc;
     hwaddr len = sizeof(*orig_orb);
 
-    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(env, PGM_OPERAND, 2);
-        return -EIO;
-    }
-    trace_ioinst_sch_id("ssch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
     if (addr & 3) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -243,11 +235,13 @@ int ioinst_handle_ssch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
         goto out;
     }
     copy_orb_from_guest(&orb, orig_orb);
-    if (!ioinst_orb_valid(&orb)) {
+    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
+        !ioinst_orb_valid(&orb)) {
         program_interrupt(env, PGM_OPERAND, 2);
         cc = -EIO;
         goto out;
     }
+    trace_ioinst_sch_id("ssch", cssid, ssid, schid);
     sch = css_find_subch(m, cssid, ssid, schid);
     if (sch && css_subch_visible(sch)) {
         ret = css_do_ssch(sch, &orb);
@@ -306,11 +300,6 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
     SCHIB *schib;
     hwaddr len = sizeof(*schib);
 
-    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        program_interrupt(env, PGM_OPERAND, 2);
-        return -EIO;
-    }
-    trace_ioinst_sch_id("stsch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb);
     if (addr & 3) {
         program_interrupt(env, PGM_SPECIFICATION, 2);
@@ -322,6 +311,13 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
         cc = -EIO;
         goto out;
     }
+
+    if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
+        program_interrupt(env, PGM_OPERAND, 2);
+        cc = -EIO;
+        goto out;
+    }
+    trace_ioinst_sch_id("stsch", cssid, ssid, schid);
     sch = css_find_subch(m, cssid, ssid, schid);
     if (sch) {
         if (css_subch_visible(sch)) {
-- 
1.8.1.4

[Qemu-devel] [PATCH 6/7] s390x/kvm: Reworked/fixed handling of cc3 in kvm_handle_css_inst()

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

Consolidated the setting of the condition code in kvm_handle_css_inst().
For the (unhandled) instructions EQBS and SQBS, we have to return
an operation exception instead of cc3. Also removed the is_ioinst()
function to avoid decoding the opcode twice.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c | 59 ++++++++++--------------------------------------------
 1 file changed, 11 insertions(+), 48 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 60e94f8..1b123cf 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -532,50 +532,19 @@ static int kvm_handle_css_inst(S390CPU *cpu, struct kvm_run *run,
         no_cc = 1;
         r = ioinst_handle_sal(env, env->regs[1]);
         break;
-    default:
-        r = -1;
+    case PRIV_SIGA:
+        /* Not provided, set CC = 3 for subchannel not operational */
+        r = 3;
         break;
+    default:
+        return -1;
     }
 
-    if (r >= 0) {
-        if (!no_cc) {
-            setcc(cpu, r);
-        }
-        r = 0;
-    } else if (r < -1) {
-        r = 0;
-    }
-    return r;
-}
-
-static int is_ioinst(uint8_t ipa0, uint8_t ipa1, uint8_t ipb)
-{
-    int ret = 0;
-    uint16_t ipa = (ipa0 << 8) | ipa1;
-
-    switch (ipa) {
-    case IPA0_B2 | PRIV_CSCH:
-    case IPA0_B2 | PRIV_HSCH:
-    case IPA0_B2 | PRIV_MSCH:
-    case IPA0_B2 | PRIV_SSCH:
-    case IPA0_B2 | PRIV_STSCH:
-    case IPA0_B2 | PRIV_TPI:
-    case IPA0_B2 | PRIV_SAL:
-    case IPA0_B2 | PRIV_RSCH:
-    case IPA0_B2 | PRIV_STCRW:
-    case IPA0_B2 | PRIV_STCPS:
-    case IPA0_B2 | PRIV_RCHP:
-    case IPA0_B2 | PRIV_SCHM:
-    case IPA0_B2 | PRIV_CHSC:
-    case IPA0_B2 | PRIV_SIGA:
-    case IPA0_B2 | PRIV_XSCH:
-    case IPA0_B9 | PRIV_EQBS:
-    case IPA0_EB | PRIV_SQBS:
-        ret = 1;
-        break;
+    if (r >= 0 && !no_cc) {
+        setcc(cpu, r);
     }
 
-    return ret;
+    return 0;
 }
 
 static int handle_priv(S390CPU *cpu, struct kvm_run *run,
@@ -591,15 +560,9 @@ static int handle_priv(S390CPU *cpu, struct kvm_run *run,
             r = kvm_sclp_service_call(cpu, run, ipbh0);
             break;
         default:
-            if (is_ioinst(ipa0, ipa1, ipb)) {
-                r = kvm_handle_css_inst(cpu, run, ipa0, ipa1, ipb);
-                if (r == -1) {
-                    setcc(cpu, 3);
-                    r = 0;
-                }
-            } else {
-                dprintf("KVM: unknown PRIV: 0x%x\n", ipa1);
-                r = -1;
+            r = kvm_handle_css_inst(cpu, run, ipa0, ipa1, ipb);
+            if (r == -1) {
+                dprintf("KVM: unhandled PRIV: 0x%x\n", ipa1);
             }
             break;
     }
-- 
1.8.1.4

[Qemu-devel] [PATCH 7/7] s390x/kvm: Remove redundant return code

[Christian Borntraeger]

From: Thomas Huth <thuth@linux.vnet.ibm.com>

Removed the redundant return code statement from handle_instruction() - it
always returned 0 and never reports any errors to its caller, since errors
from the sub-functions are already reported via program exceptions instead.

Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: CHristian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 1b123cf..3dfd19c 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -697,7 +697,7 @@ out:
     return 0;
 }
 
-static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
+static void handle_instruction(S390CPU *cpu, struct kvm_run *run)
 {
     unsigned int ipa0 = (run->s390_sieic.ipa & 0xff00);
     uint8_t ipa1 = run->s390_sieic.ipa & 0x00ff;
@@ -722,7 +722,6 @@ static int handle_instruction(S390CPU *cpu, struct kvm_run *run)
     if (r < 0) {
         enter_pgmcheck(cpu, 0x0001);
     }
-    return 0;
 }
 
 static bool is_special_wait_psw(CPUState *cs)
@@ -742,7 +741,7 @@ static int handle_intercept(S390CPU *cpu)
             (long)cs->kvm_run->psw_addr);
     switch (icpt_code) {
         case ICPT_INSTRUCTION:
-            r = handle_instruction(cpu, run);
+            handle_instruction(cpu, run);
             break;
         case ICPT_WAITPSW:
             /* disabled wait, since enabled wait is handled in kernel */
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/7] s390 fixes

[Christian Borntraeger]

On 15/07/13 21:57, Christian Borntraeger wrote:
> Alex,
> 
> here is a bunch of fixes/cleanups for s390.
> 
> Heinz Graalfs (1):
>   s390/sclpconsole: handle char layer busy conditions
> 
> Thomas Huth (6):
>   s390x/ioinst: Add missing alignment checks for IO instructions
>   s390x/ioinst: Throw addressing exception when memory_map failed
>   s390x/ioinst: Fixed alignment check in SCHM instruction
>   s390x/ioinst: Fixed priority of operand exceptions
>   s390x/kvm: Reworked/fixed handling of cc3 in kvm_handle_css_inst()
>   s390x/kvm: Remove redundant return code
> 
>  hw/char/sclpconsole.c | 18 +-------------
>  target-s390x/ioinst.c | 65 ++++++++++++++++++++++++++++++++++-----------------
>  target-s390x/kvm.c    | 64 +++++++++++---------------------------------------
>  3 files changed, 57 insertions(+), 90 deletions(-)
> 

Ping?