[PATCH v3 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

[PATCH v3 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

[Collin Walling]

Changelog:

    v3

    • Device IOCTLs removed
        - diag 318 info is now communicated via sync_regs

    • Reset code removed
        - this is now handled in KVM
        - diag318_info is stored within the CPU reset portion of the
            S390CPUState

    • Various cleanups for ELS preliminary patches

    v2

    • QEMU now handles the instruction call
        - as such, the "enable diag 318" IOCTL has been removed

    • patch #1 now changes the read scp/cpu info functions to
      retrieve the machine state once
        - as such, I have not added any ack's or r-bs since this
          patch differs from the previous version

    • patch #3 introduces a new "get_read_scp_info_data_len"
      function in order clean-up the variable data length assignment
      in patch #7
        - a comment above this function should help clarify what's
          going on to make things a bit easier to read

    • other misc clean ups and fixes
        - s/diag318/diag_318 in order to keep the naming scheme
          consistent with Linux and other diag-related code
        - s/byte_134/fac134 to align naming scheme with Linux

-----------------------------------------------------------------------

This patch series introduces two features for an s390 KVM quest:
    - Extended-Length SCCB (els) for the Read SCP/CPU Info SCLP 
        commands
    - DIAGNOSE 0x318 (diag_318) enabling / migration handling

The diag 318 feature depends on els and KVM support.

The els feature is handled entirely with QEMU, and does not require 
KVM support.

Both features are made available starting with the zEC12-full model.

These patches are introduced together for two main reasons:
    - els allows diag 318 to exist while retaining the original 248 
        VCPU max
    - diag 318 is presented to show how els is useful

Full els support is dependant on the Linux kernel, which must react
to the SCLP response code and set an appropriate-length SCCB. 

A user should take care when tuning the CPU model for a VM.
If a user defines a VM with els support and specifies 248 CPUs, but
the guest Linux kernel cannot react to the SCLP response code, then
the guest will crash immediately upon kernel startup.

Collin L. Walling (8):
  s390/sclp: get machine once during read scp/cpu info
  s390/sclp: check sccb len before filling in data
  s390/sclp: rework sclp boundary and length checks
  s390/sclp: read sccb from mem based on sccb length
  s390/sclp: use cpu offset to locate cpu entries
  s390/sclp: add extended-length sccb support for kvm guest
  s390/kvm: header sync for diag318
  s390: guest support for diagnose 0x318

 hw/s390x/sclp.c                     | 117 ++++++++++++++++++++++------
 include/hw/s390x/sclp.h             |   4 +
 linux-headers/asm-s390/kvm.h        |   5 +-
 linux-headers/linux/kvm.h           |   1 +
 target/s390x/cpu.h                  |   3 +-
 target/s390x/cpu_features.h         |   1 +
 target/s390x/cpu_features_def.inc.h |   4 +
 target/s390x/cpu_models.c           |   1 +
 target/s390x/gen-features.c         |   2 +
 target/s390x/kvm.c                  |  39 ++++++++++
 target/s390x/machine.c              |  17 ++++
 11 files changed, 167 insertions(+), 27 deletions(-)

-- 
2.21.3

[PATCH v3 1/8] s390/sclp: get machine once during read scp/cpu info

[Collin Walling]

Functions within read scp/cpu info will need access to the machine
state. Let's make a call to retrieve the machine state once and
pass the appropriate data to the respective functions.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/s390x/sclp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 20aca30ac4..7875334037 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -49,9 +49,8 @@ static inline bool sclp_command_code_valid(uint32_t code)
     return false;
 }
 
-static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count)
+static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
 {
-    MachineState *ms = MACHINE(qdev_get_machine());
     uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
     int i;
 
@@ -77,7 +76,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     IplParameterBlock *ipib = s390_ipl_get_iplb();
 
     /* CPU information */
-    prepare_cpu_entries(sclp, read_info->entries, &cpu_count);
+    prepare_cpu_entries(machine, read_info->entries, &cpu_count);
     read_info->entries_cpu = cpu_to_be16(cpu_count);
     read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
     read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
@@ -132,10 +131,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 /* Provide information about the CPU */
 static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
 {
+    MachineState *machine = MACHINE(qdev_get_machine());
     ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
     int cpu_count;
 
-    prepare_cpu_entries(sclp, cpu_info->entries, &cpu_count);
+    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
     cpu_info->nr_configured = cpu_to_be16(cpu_count);
     cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
     cpu_info->nr_standby = cpu_to_be16(0);
-- 
2.21.3

[PATCH v3 2/8] s390/sclp: check sccb len before filling in data

[Collin Walling]

The SCCB must be checked for a sufficient length before it is filled
with any data. If the length is insufficient, then the SCLP command
is suppressed and the proper response code is set in the SCCB header.

Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/sclp.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 7875334037..181ce04007 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -75,6 +75,12 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     int rnsize, rnmax;
     IplParameterBlock *ipib = s390_ipl_get_iplb();
 
+    if (be16_to_cpu(sccb->h.length) <
+          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return;
+    }
+
     /* CPU information */
     prepare_cpu_entries(machine, read_info->entries, &cpu_count);
     read_info->entries_cpu = cpu_to_be16(cpu_count);
@@ -83,12 +89,6 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 
     read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
 
-    if (be16_to_cpu(sccb->h.length) <
-            (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
-        return;
-    }
-
     /* Configuration Characteristic (Extension) */
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
                          read_info->conf_char);
@@ -135,17 +135,17 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
     ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
     int cpu_count;
 
-    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
-    cpu_info->nr_configured = cpu_to_be16(cpu_count);
-    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
-    cpu_info->nr_standby = cpu_to_be16(0);
-
     if (be16_to_cpu(sccb->h.length) <
-            (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
+          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
         return;
     }
 
+    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
+    cpu_info->nr_configured = cpu_to_be16(cpu_count);
+    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
+    cpu_info->nr_standby = cpu_to_be16(0);
+
     /* The standby offset is 16-byte for each CPU */
     cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
         + cpu_info->nr_configured*sizeof(CPUEntry));
-- 
2.21.3

[PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Collin Walling]

Rework the SCLP boundary check to account for different SCLP commands
(eventually) allowing different boundary sizes.

Move the length check code into a separate function, and introduce a
new function to determine the length of the read SCP data (i.e. the size
from the start of the struct to where the CPU entries should begin).

The format of read CPU info is unlikely to change in the future,
so we do not require a separate function to calculate its length.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 181ce04007..0710138f91 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code)
     return false;
 }
 
+static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
+                                    SCCBHeader *header)
+{
+    uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1;
+    uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
+
+    switch (code & SCLP_CMD_CODE_MASK) {
+    default:
+        if (sccb_max_addr < sccb_boundary) {
+            return true;
+        }
+    }
+    header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
+    return false;
+}
+
+/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */
+static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
+{
+    int required_len = data_len + num_cpus * sizeof(CPUEntry);
+
+    if (be16_to_cpu(sccb->h.length) < required_len) {
+        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        return false;
+    }
+    return true;
+}
+
 static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
 {
     uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
@@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
     }
 }
 
+/*
+ * The data length denotes the start of the struct to where the first
+ * CPU entry is to be allocated. This value also denotes the offset_cpu
+ * field.
+ */
+static inline int get_read_scp_info_data_len(void)
+{
+    return offsetof(ReadInfo, entries);
+}
+
 /* Provide information about the configuration, CPUs and storage */
 static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 {
@@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     int cpu_count;
     int rnsize, rnmax;
     IplParameterBlock *ipib = s390_ipl_get_iplb();
+    int data_len = get_read_scp_info_data_len();
 
-    if (be16_to_cpu(sccb->h.length) <
-          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
         return;
     }
 
     /* CPU information */
     prepare_cpu_entries(machine, read_info->entries, &cpu_count);
     read_info->entries_cpu = cpu_to_be16(cpu_count);
-    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
+    read_info->offset_cpu = cpu_to_be16(data_len);
     read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
 
     read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
@@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
 {
     MachineState *machine = MACHINE(qdev_get_machine());
     ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
+    int data_len = offsetof(ReadCpuInfo, entries);
     int cpu_count;
 
-    if (be16_to_cpu(sccb->h.length) <
-          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
-        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
         return;
     }
 
     prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
     cpu_info->nr_configured = cpu_to_be16(cpu_count);
-    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
+    cpu_info->offset_configured = cpu_to_be16(data_len);
     cpu_info->nr_standby = cpu_to_be16(0);
 
     /* The standby offset is 16-byte for each CPU */
@@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
         goto out_write;
     }
 
+    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
+        goto out_write;
+    }
+
     sclp_c->execute(sclp, &work_sccb, code);
 out_write:
     s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
@@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
         goto out_write;
     }
 
-    if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
-        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
+    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
         goto out_write;
     }
 
-- 
2.21.3

[PATCH v3 4/8] s390/sclp: read sccb from mem based on sccb length

[Collin Walling]

The header of the SCCB contains the actual length of the SCCB. Instead
of using a static 4K size, let's allow for a variable size determined
by the value set in the header. The proper checks are already in place
to ensure the SCCB length is sufficent to store a full response, and
that the length does not cross any explicitly-set boundaries.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/s390x/sclp.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 0710138f91..772b7b3b01 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -256,9 +256,8 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
     SCLPDevice *sclp = get_sclp_device();
     SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
     SCCB work_sccb;
-    hwaddr sccb_len = sizeof(SCCB);
 
-    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len);
+    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sizeof(SCCBHeader));
 
     if (!sclp_command_code_valid(code)) {
         work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
@@ -269,6 +268,9 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
         goto out_write;
     }
 
+    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb,
+                         be16_to_cpu(work_sccb.h.length));
+
     sclp_c->execute(sclp, &work_sccb, code);
 out_write:
     s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
@@ -283,8 +285,6 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
     SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
     SCCB work_sccb;
 
-    hwaddr sccb_len = sizeof(SCCB);
-
     /* first some basic checks on program checks */
     if (env->psw.mask & PSW_MASK_PSTATE) {
         return -PGM_PRIVILEGED;
@@ -302,7 +302,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
      * from playing dirty tricks by modifying the memory content after
      * the host has checked the values
      */
-    cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
+    cpu_physical_memory_read(sccb, &work_sccb, sizeof(SCCBHeader));
 
     /* Valid sccb sizes */
     if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
@@ -318,6 +318,9 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
         goto out_write;
     }
 
+    /* the header contains the actual length of the sccb */
+    cpu_physical_memory_read(sccb, &work_sccb, be16_to_cpu(work_sccb.h.length));
+
     sclp_c->execute(sclp, &work_sccb, code);
 out_write:
     cpu_physical_memory_write(sccb, &work_sccb,
-- 
2.21.3

[PATCH v3 5/8] s390/sclp: use cpu offset to locate cpu entries

[Collin Walling]

The start of the CPU entry region in the Read SCP Info response data is
denoted by the offset_cpu field. As such, QEMU needs to begin creating
entries at this address. Note that the length of the Read SCP Info data
(data_len) denotes the same value as the cpu offset.

This is in preparation of when Read SCP Info inevitably introduces new
bytes that push the start of the CPUEntry field further away.

Read CPU Info is unlikely to ever change, so let's not bother
accounting for the offset there.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/s390x/sclp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 772b7b3b01..0dfbe6e5ec 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -113,13 +113,14 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     int rnsize, rnmax;
     IplParameterBlock *ipib = s390_ipl_get_iplb();
     int data_len = get_read_scp_info_data_len();
+    CPUEntry *entries_start = (void *)sccb + data_len;
 
     if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
         return;
     }
 
     /* CPU information */
-    prepare_cpu_entries(machine, read_info->entries, &cpu_count);
+    prepare_cpu_entries(machine, entries_start, &cpu_count);
     read_info->entries_cpu = cpu_to_be16(cpu_count);
     read_info->offset_cpu = cpu_to_be16(data_len);
     read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
-- 
2.21.3

[PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Collin Walling]

As more features and facilities are added to the Read SCP Info (RSCPI)
response, more space is required to store them. The space used to store
these new features intrudes on the space originally used to store CPU
entries. This means as more features and facilities are added to the
RSCPI response, less space can be used to store CPU entries.

With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
the RSCPI command and determine if the SCCB is large enough to store a
complete reponse. If it is not large enough, then the required length
will be set in the SCCB header.

The caller of the SCLP command is responsible for creating a
large-enough SCCB to store a complete response. Proper checking should
be in place, and the caller should execute the command once-more with
the large-enough SCCB.

This facility also enables an extended SCCB for the Read CPU Info
(RCPUI) command.

When this facility is enabled, the boundary violation response cannot
be a result from the RSCPI, RSCPI Forced, or RCPUI commands.

In order to tolerate kernels that do not yet have full support for this
feature, a "fixed" offset to the start of the CPU Entries within the
Read SCP Info struct is set to allow for the original 248 max entries
when this feature is disabled.

Additionally, this is introduced as a CPU feature to protect the guest
from migrating to a machine that does not support storing an extended
SCCB. This could otherwise hinder the VM from being able to read all
available CPU entries after migration (such as during re-ipl).

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
 include/hw/s390x/sclp.h             |  1 +
 target/s390x/cpu_features_def.inc.h |  1 +
 target/s390x/gen-features.c         |  1 +
 target/s390x/kvm.c                  |  8 ++++++++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 0dfbe6e5ec..f7c49e339e 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
     uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
 
     switch (code & SCLP_CMD_CODE_MASK) {
+    case SCLP_CMDW_READ_SCP_INFO:
+    case SCLP_CMDW_READ_SCP_INFO_FORCED:
+    case SCLP_CMDW_READ_CPU_INFO:
+        /*
+         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
+         * is allowed to exceed the 4k boundary. The respective commands will
+         * set the length field to the required length if an insufficient
+         * SCCB length is provided.
+         */
+        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
+            return true;
+        }
     default:
         if (sccb_max_addr < sccb_boundary) {
             return true;
@@ -72,6 +84,10 @@ static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
 
     if (be16_to_cpu(sccb->h.length) < required_len) {
         sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) &&
+            sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
+            sccb->h.length = required_len;
+        }
         return false;
     }
     return true;
@@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
  */
 static inline int get_read_scp_info_data_len(void)
 {
-    return offsetof(ReadInfo, entries);
+    return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
+           offsetof(ReadInfo, entries) :
+           SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
 }
 
 /* Provide information about the configuration, CPUs and storage */
@@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     CPUEntry *entries_start = (void *)sccb + data_len;
 
     if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
+        warn_report("insufficient sccb size to store read scp info response");
         return;
     }
 
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 822eff4396..ef2d63eae9 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -110,6 +110,7 @@ typedef struct CPUEntry {
     uint8_t reserved1;
 } QEMU_PACKED CPUEntry;
 
+#define SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET     128
 typedef struct ReadInfo {
     SCCBHeader h;
     uint16_t rnmax;
diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
index 5942f81f16..1c04cc18f4 100644
--- a/target/s390x/cpu_features_def.inc.h
+++ b/target/s390x/cpu_features_def.inc.h
@@ -97,6 +97,7 @@ DEF_FEAT(GUARDED_STORAGE, "gs", STFL, 133, "Guarded-storage facility")
 DEF_FEAT(VECTOR_PACKED_DECIMAL, "vxpd", STFL, 134, "Vector packed decimal facility")
 DEF_FEAT(VECTOR_ENH, "vxeh", STFL, 135, "Vector enhancements facility")
 DEF_FEAT(MULTIPLE_EPOCH, "mepoch", STFL, 139, "Multiple-epoch facility")
+DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility")
 DEF_FEAT(TEST_PENDING_EXT_INTERRUPTION, "tpei", STFL, 144, "Test-pending-external-interruption facility")
 DEF_FEAT(INSERT_REFERENCE_BITS_MULT, "irbm", STFL, 145, "Insert-reference-bits-multiple facility")
 DEF_FEAT(MSA_EXT_8, "msa8-base", STFL, 146, "Message-security-assist-extension-8 facility (excluding subfunctions)")
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 8ddeebc544..6857f657fb 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -522,6 +522,7 @@ static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL,
     S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_AP,
+    S390_FEAT_EXTENDED_LENGTH_SCCB,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index f2f75d2a57..a2d5ad78f6 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2456,6 +2456,14 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
         set_bit(S390_FEAT_AP, model->features);
     }
+
+    /*
+     * Extended-Length SCCB is handled entirely within QEMU.
+     * For PV guests this is completely fenced by the Ultravisor, as Service
+     * Call error checking and STFLE interpretation are handled via SIE.
+     */
+    set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features);
+
     /* strip of features that are not part of the maximum model */
     bitmap_and(model->features, model->features, model->def->full_feat,
                S390_FEAT_MAX);
-- 
2.21.3

[PATCH v3 7/8] s390/kvm: header sync for diag318

[Collin Walling]

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 5 ++++-
 linux-headers/linux/kvm.h    | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 0138ccb0d8..98665dff19 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -231,11 +231,13 @@ struct kvm_guest_debug_arch {
 #define KVM_SYNC_GSCB   (1UL << 9)
 #define KVM_SYNC_BPBC   (1UL << 10)
 #define KVM_SYNC_ETOKEN (1UL << 11)
+#define KVM_SYNC_DIAG318 (1UL << 12)
 
 #define KVM_SYNC_S390_VALID_FIELDS \
 	(KVM_SYNC_PREFIX | KVM_SYNC_GPRS | KVM_SYNC_ACRS | KVM_SYNC_CRS | \
 	 KVM_SYNC_ARCH0 | KVM_SYNC_PFAULT | KVM_SYNC_VRS | KVM_SYNC_RICCB | \
-	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN)
+	 KVM_SYNC_FPRS | KVM_SYNC_GSCB | KVM_SYNC_BPBC | KVM_SYNC_ETOKEN | \
+	 KVM_SYNC_DIAG318)
 
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8
@@ -254,6 +256,7 @@ struct kvm_sync_regs {
 	__u64 pft;	/* pfault token [PFAULT] */
 	__u64 pfs;	/* pfault select [PFAULT] */
 	__u64 pfc;	/* pfault compare [PFAULT] */
+	__u64 diag318;	/* diagnose 0x318 info */
 	union {
 		__u64 vrs[32][2];	/* vector registers (KVM_SYNC_VRS) */
 		__u64 fprs[16];		/* fp registers (KVM_SYNC_FPRS) */
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 9804495a46..444fdd977f 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1017,6 +1017,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_VCPU_RESETS 179
 #define KVM_CAP_S390_PROTECTED 180
 #define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_S390_DIAG318 184
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
2.21.3

[PATCH v3 8/8] s390: guest support for diagnose 0x318

[Collin Walling]

DIAGNOSE 0x318 (diag318) is an s390 instruction that allows the storage
of diagnostic information that is collected by the firmware in the case
of hardware/firmware service events.

QEMU handles the instruction by storing the info in the CPU state. A
subsequent register sync will communicate the data to the hypervisor.

QEMU handles the migration via a VM State Description.

This feature depends on the Extended-Length SCCB (els) feature. If
els is not present, then a warning will be printed and the SCLP bit
that allows the Linux kernel to execute the instruction will not be
set.

Availability of this instruction is determined by byte 134 (aka fac134)
bit 0 of the SCLP Read Info block. This coincidentally expands into the
space used for CPU entries, which means VMs running with the diag318
capability may not be able to read information regarding all CPUs
unless the guest kernel supports an extended-length SCCB.

This feature is not supported in protected virtualization mode.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 hw/s390x/sclp.c                     |  5 +++++
 include/hw/s390x/sclp.h             |  3 +++
 target/s390x/cpu.h                  |  3 ++-
 target/s390x/cpu_features.h         |  1 +
 target/s390x/cpu_features_def.inc.h |  3 +++
 target/s390x/cpu_models.c           |  1 +
 target/s390x/gen-features.c         |  1 +
 target/s390x/kvm.c                  | 31 +++++++++++++++++++++++++++++
 target/s390x/machine.c              | 17 ++++++++++++++++
 9 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index f7c49e339e..78dbfbe427 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -152,6 +152,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
                          read_info->conf_char_ext);
 
+    if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
+        s390_get_feat_block(S390_FEAT_TYPE_SCLP_FAC134,
+                            &read_info->fac134);
+    }
+
     read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
                                         SCLP_HAS_IOA_RECONFIG);
 
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index ef2d63eae9..ccb9f0a676 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -133,6 +133,9 @@ typedef struct ReadInfo {
     uint16_t highest_cpu;
     uint8_t  _reserved5[124 - 122];     /* 122-123 */
     uint32_t hmfai;
+    uint8_t  _reserved7[134 - 128];     /* 128-133 */
+    uint8_t  fac134;
+    uint8_t  _reserved8[144 - 135];     /* 135-143 */
     struct CPUEntry entries[];
 } QEMU_PACKED ReadInfo;
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 035427521c..52765961cf 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -112,6 +112,8 @@ struct CPUS390XState {
     uint16_t external_call_addr;
     DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
 
+    uint64_t diag318_info;
+
     /* Fields up to this point are cleared by a CPU reset */
     struct {} end_reset_fields;
 
@@ -136,7 +138,6 @@ struct CPUS390XState {
 
     /* currently processed sigp order */
     uint8_t sigp_order;
-
 };
 
 static inline uint64_t *get_freg(CPUS390XState *cs, int nr)
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index da695a8346..f74f7fc3a1 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -23,6 +23,7 @@ typedef enum {
     S390_FEAT_TYPE_STFL,
     S390_FEAT_TYPE_SCLP_CONF_CHAR,
     S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
+    S390_FEAT_TYPE_SCLP_FAC134,
     S390_FEAT_TYPE_SCLP_CPU,
     S390_FEAT_TYPE_MISC,
     S390_FEAT_TYPE_PLO,
diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
index 1c04cc18f4..f82b4b5ec1 100644
--- a/target/s390x/cpu_features_def.inc.h
+++ b/target/s390x/cpu_features_def.inc.h
@@ -122,6 +122,9 @@ DEF_FEAT(SIE_CMMA, "cmma", SCLP_CONF_CHAR_EXT, 1, "SIE: Collaborative-memory-man
 DEF_FEAT(SIE_PFMFI, "pfmfi", SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility")
 DEF_FEAT(SIE_IBS, "ibs", SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility")
 
+/* Features exposed via SCLP SCCB Facilities byte 134 (bit numbers relative to byte-134) */
+DEF_FEAT(DIAG_318, "diag318", SCLP_FAC134, 0, "Control program name and version codes")
+
 /* Features exposed via SCLP CPU info. */
 DEF_FEAT(SIE_F2, "sief2", SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)")
 DEF_FEAT(SIE_SKEY, "skey", SCLP_CPU, 5, "SIE: Storage-key facility")
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2fa609bffe..034673be54 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -827,6 +827,7 @@ static void check_consistency(const S390CPUModel *model)
         { S390_FEAT_PTFF_STOE, S390_FEAT_MULTIPLE_EPOCH },
         { S390_FEAT_PTFF_STOUE, S390_FEAT_MULTIPLE_EPOCH },
         { S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, S390_FEAT_AP },
+        { S390_FEAT_DIAG_318, S390_FEAT_EXTENDED_LENGTH_SCCB },
     };
     int i;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 6857f657fb..a1f0a6f3c6 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -523,6 +523,7 @@ static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_AP,
     S390_FEAT_EXTENDED_LENGTH_SCCB,
+    S390_FEAT_DIAG_318,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index a2d5ad78f6..b79feeba9f 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -105,6 +105,7 @@
 
 #define DIAG_TIMEREVENT                 0x288
 #define DIAG_IPL                        0x308
+#define DIAG_SET_CONTROL_PROGRAM_CODES  0x318
 #define DIAG_KVM_HYPERCALL              0x500
 #define DIAG_KVM_BREAKPOINT             0x501
 
@@ -602,6 +603,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
     }
 
+    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
+        cs->kvm_run->s.regs.diag318 = env->diag318_info;
+        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
+    }
+
     /* Finally the prefix */
     if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
         cs->kvm_run->s.regs.prefix = env->psa;
@@ -741,6 +747,10 @@ int kvm_arch_get_registers(CPUState *cs)
         }
     }
 
+    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
+        env->diag318_info = cs->kvm_run->s.regs.diag318;
+    }
+
     return 0;
 }
 
@@ -1601,6 +1611,19 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct kvm_run *run)
     return -ENOENT;
 }
 
+static void handle_diag_318(S390CPU *cpu, struct kvm_run *run)
+{
+    uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4;
+    uint64_t diag318_info = run->s.regs.gprs[reg];
+
+    cpu->env.diag318_info = diag318_info;
+
+    if (can_sync_regs(CPU(cpu), KVM_SYNC_DIAG318)) {
+        run->s.regs.diag318 = diag318_info;
+        run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
+    }
+}
+
 #define DIAG_KVM_CODE_MASK 0x000000000000ffff
 
 static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
@@ -1620,6 +1643,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
     case DIAG_IPL:
         kvm_handle_diag_308(cpu, run);
         break;
+    case DIAG_SET_CONTROL_PROGRAM_CODES:
+        handle_diag_318(cpu, run);
+        break;
     case DIAG_KVM_HYPERCALL:
         r = handle_hypercall(cpu, run);
         break;
@@ -2464,6 +2490,11 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
      */
     set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features);
 
+    /* DIAGNOSE 0x318 is not supported under protected virtualization */
+    if (!s390_is_pv() && kvm_check_extension(kvm_state, KVM_CAP_S390_DIAG318)) {
+        set_bit(S390_FEAT_DIAG_318, model->features);
+    }
+
     /* strip of features that are not part of the maximum model */
     bitmap_and(model->features, model->features, model->def->full_feat,
                S390_FEAT_MAX);
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 549bb6c280..5b4e82f1ab 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -234,6 +234,22 @@ const VMStateDescription vmstate_etoken = {
     }
 };
 
+static bool diag318_needed(void *opaque)
+{
+    return s390_has_feat(S390_FEAT_DIAG_318);
+}
+
+const VMStateDescription vmstate_diag318 = {
+    .name = "cpu/diag318",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = diag318_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(env.diag318_info, S390CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_s390_cpu = {
     .name = "cpu",
     .post_load = cpu_post_load,
@@ -270,6 +286,7 @@ const VMStateDescription vmstate_s390_cpu = {
         &vmstate_gscb,
         &vmstate_bpbc,
         &vmstate_etoken,
+        &vmstate_diag318,
         NULL
     },
 };
-- 
2.21.3

Re: [PATCH v3 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200618222258.23287-1-walling@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v3 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318
Type: series
Message-id: 20200618222258.23287-1-walling@linux.ibm.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20200618222258.23287-1-walling@linux.ibm.com -> patchew/20200618222258.23287-1-walling@linux.ibm.com
Switched to a new branch 'test'
7c099e1 s390: guest support for diagnose 0x318
11cf174 s390/kvm: header sync for diag318
9858413 s390/sclp: add extended-length sccb support for kvm guest
a8e6274 s390/sclp: use cpu offset to locate cpu entries
b839587 s390/sclp: read sccb from mem based on sccb length
2d754fb s390/sclp: rework sclp boundary and length checks
a7dce9a s390/sclp: check sccb len before filling in data
ea700d4 s390/sclp: get machine once during read scp/cpu info

=== OUTPUT BEGIN ===
1/8 Checking commit ea700d4d8a47 (s390/sclp: get machine once during read scp/cpu info)
2/8 Checking commit a7dce9a1bf29 (s390/sclp: check sccb len before filling in data)
3/8 Checking commit 2d754fbb29f6 (s390/sclp: rework sclp boundary and length checks)
4/8 Checking commit b839587fc52a (s390/sclp: read sccb from mem based on sccb length)
5/8 Checking commit a8e6274f786b (s390/sclp: use cpu offset to locate cpu entries)
6/8 Checking commit 985841391b37 (s390/sclp: add extended-length sccb support for kvm guest)
WARNING: line over 80 characters
#115: FILE: target/s390x/cpu_features_def.inc.h:100:
+DEF_FEAT(EXTENDED_LENGTH_SCCB, "els", STFL, 140, "Extended-length SCCB facility")

total: 0 errors, 1 warnings, 80 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/8 Checking commit 11cf174b3455 (s390/kvm: header sync for diag318)
8/8 Checking commit 7c099e11b2ba (s390: guest support for diagnose 0x318)
ERROR: line over 90 characters
#103: FILE: target/s390x/cpu_features_def.inc.h:125:
+/* Features exposed via SCLP SCCB Facilities byte 134 (bit numbers relative to byte-134) */

WARNING: line over 80 characters
#104: FILE: target/s390x/cpu_features_def.inc.h:126:
+DEF_FEAT(DIAG_318, "diag318", SCLP_FAC134, 0, "Control program name and version codes")

total: 1 errors, 1 warnings, 161 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200618222258.23287-1-walling@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 0/8] s390: Extended-Length SCCB & DIAGNOSE 0x318

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200618222258.23287-1-walling@linux.ibm.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  LINK    elf2dmp
  AR      libqemuutil.a
  CC      qemu-img.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  AR      libvhost-user.a
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  GEN     docs/interop/qemu-ga-ref.html
  GEN     docs/interop/qemu-ga-ref.txt
  GEN     docs/interop/qemu-ga-ref.7
  LINK    qemu-keymap
  LINK    ivshmem-client
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    ivshmem-server
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-nbd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-storage-daemon
  LINK    qemu-io
  LINK    qemu-edid
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    fsdev/virtfs-proxy-helper
  LINK    scsi/qemu-pr-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-bridge-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    virtiofsd
  LINK    vhost-user-input
  LINK    qemu-ga
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  LINK    qemu-img
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
  CC      pc-bios/optionrom/linuxboot_dma.o
  AS      pc-bios/optionrom/multiboot.o
  AS      pc-bios/optionrom/linuxboot.o
---
  CC      x86_64-softmmu/gdbstub-xml.o
  CC      x86_64-softmmu/trace/generated-helpers.o
  LINK    x86_64-softmmu/qemu-system-x86_64
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
common.rc: line 50: test: check: binary operator expected
(printf '#define QEMU_PKGVERSION ""\n'; printf '#define QEMU_FULL_VERSION "5.0.50"\n'; ) > qemu-version.h.tmp
make -C /tmp/qemu-test/src/slirp BUILD_DIR="/tmp/qemu-test/build/slirp" PKG_CONFIG="pkg-config" CC="clang" AR="ar"      LD="ld" RANLIB="ranlib" CFLAGS="-I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong   -I/usr/include/p11-kit-1   -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -g " LDFLAGS="-Wl,--warn-common -fsanitize=undefined -fsanitize=address -Wl,-z,relro -Wl,-z,now -pie -m64  -fstack-protector-strong"
---
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong -I/usr/include/p11-kit-1 -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80  -Wno-strict-prototypes -Wno-unknown-pragmas -Wno-uninitialized -Wno-missing-prototypes -Wno-return-type -Wno-unused-function -Wno-error -MMD -MP -MT uint128_inline.o -MF ./uint128_inline.d -g   -c -o uint128_inline.o /tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source/uint128_inline.c
clang -iquote /tmp/qemu-test/build/tests/qtest -iquote tests/qtest -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/usr/include/pixman-1   -Werror -fsanitize=undefined -fsanitize=address  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong   -I/usr/include/p11-kit-1   -DSTRUCT_IOVEC_DEFINED  -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -I/tmp/qemu-test/src/tests/qtest -MMD -MP -MT tests/qtest/hd-geo-test.o -MF tests/qtest/hd-geo-test.d -g   -c -o tests/qtest/hd-geo-test.o /tmp/qemu-test/src/tests/qtest/hd-geo-test.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong -I/usr/include/p11-kit-1 -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80  -Wno-strict-prototypes -Wno-unknown-pragmas -Wno-uninitialized -Wno-missing-prototypes -Wno-return-type -Wno-unused-function -Wno-error -MMD -MP -MT uint128.o -MF ./uint128.d -g   -c -o uint128.o /tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source/uint128.c
/tmp/qemu-test/src/tests/qht-bench.c:287:29: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
        *threshold = rate * UINT64_MAX;
                          ~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
---
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: tests/qht-bench.o] Error 1
make: *** Waiting for unfinished jobs....
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong -I/usr/include/p11-kit-1 -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80  -Wno-strict-prototypes -Wno-unknown-pragmas -Wno-uninitialized -Wno-missing-prototypes -Wno-return-type -Wno-unused-function -Wno-error -MMD -MP -MT fail.o -MF ./fail.d -g   -c -o fail.o /tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source/fail.c
clang -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg/i386 -isystem /tmp/qemu-test/src/linux-headers -isystem /tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -I/tmp/qemu-test/src/tests/fp -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/include -I/tmp/qemu-test/src/tests/fp/berkeley-softfloat-3/source/8086-SSE -I/tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source -I/usr/include/pixman-1 -Werror -fsanitize=undefined -fsanitize=address -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -fstack-protector-strong -I/usr/include/p11-kit-1 -DSTRUCT_IOVEC_DEFINED -I/usr/include/libpng16 -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pixman-1 -DHW_POISON_H -DTARGET_ARM  -DSOFTFLOAT_ROUND_ODD -DINLINE_LEVEL=5 -DSOFTFLOAT_FAST_DIV32TO16 -DSOFTFLOAT_FAST_DIV64TO32 -DSOFTFLOAT_FAST_INT64  -DFLOAT16 -DFLOAT64 -DEXTFLOAT80 -DFLOAT128 -DFLOAT_ROUND_ODD -DLONG_DOUBLE_IS_EXTFLOAT80  -Wno-strict-prototypes -Wno-unknown-pragmas -Wno-uninitialized -Wno-missing-prototypes -Wno-return-type -Wno-unused-function -Wno-error -MMD -MP -MT functions_common.o -MF ./functions_common.d -g   -c -o functions_common.o /tmp/qemu-test/src/tests/fp/berkeley-testfloat-3/source/functions_common.c
---
rm -f libtestfloat.a && ar rcs libtestfloat.a uint128_inline.o uint128.o fail.o functions_common.o functionInfos.o standardFunctionInfos.o random.o genCases_common.o genCases_ui32.o genCases_ui64.o genCases_i32.o genCases_i64.o genCases_f16.o genCases_f32.o genCases_f64.o genCases_extF80.o genCases_f128.o genCases_writeTestsTotal.o verCases_inline.o verCases_common.o verCases_writeFunctionName.o readHex.o writeHex.o writeCase_a_ui32.o writeCase_a_ui64.o writeCase_a_f16.o writeCase_ab_f16.o writeCase_abc_f16.o writeCase_a_f32.o writeCase_ab_f32.o writeCase_abc_f32.o writeCase_a_f64.o writeCase_ab_f64.o writeCase_abc_f64.o writeCase_a_extF80M.o writeCase_ab_extF80M.o writeCase_a_f128M.o writeCase_ab_f128M.o writeCase_abc_f128M.o writeCase_z_bool.o writeCase_z_ui32.o writeCase_z_ui64.o writeCase_z_f16.o writeCase_z_f32.o writeCase_z_f64.o writeCase_z_extF80M.o writeCase_z_f128M.o testLoops_common.o test_a_ui32_z_f16.o test_a_ui32_z_f32.o test_a_ui32_z_f64.o test_a_ui32_z_extF80.o test_a_ui32_z_f128.o test_a_ui64_z_f16.o test_a_ui64_z_f32.o test_a_ui64_z_f64.o test_a_ui64_z_extF80.o test_a_ui64_z_f128.o test_a_i32_z_f16.o test_a_i32_z_f32.o test_a_i32_z_f64.o test_a_i32_z_extF80.o test_a_i32_z_f128.o test_a_i64_z_f16.o test_a_i64_z_f32.o test_a_i64_z_f64.o test_a_i64_z_extF80.o test_a_i64_z_f128.o test_a_f16_z_ui32_rx.o test_a_f16_z_ui64_rx.o test_a_f16_z_i32_rx.o test_a_f16_z_i64_rx.o test_a_f16_z_ui32_x.o test_a_f16_z_ui64_x.o test_a_f16_z_i32_x.o test_a_f16_z_i64_x.o test_a_f16_z_f32.o test_a_f16_z_f64.o test_a_f16_z_extF80.o test_a_f16_z_f128.o test_az_f16.o test_az_f16_rx.o test_abz_f16.o test_abcz_f16.o test_ab_f16_z_bool.o test_a_f32_z_ui32_rx.o test_a_f32_z_ui64_rx.o test_a_f32_z_i32_rx.o test_a_f32_z_i64_rx.o test_a_f32_z_ui32_x.o test_a_f32_z_ui64_x.o test_a_f32_z_i32_x.o test_a_f32_z_i64_x.o test_a_f32_z_f16.o test_a_f32_z_f64.o test_a_f32_z_extF80.o test_a_f32_z_f128.o test_az_f32.o test_az_f32_rx.o test_abz_f32.o test_abcz_f32.o test_ab_f32_z_bool.o test_a_f64_z_ui32_rx.o test_a_f64_z_ui64_rx.o test_a_f64_z_i32_rx.o test_a_f64_z_i64_rx.o test_a_f64_z_ui32_x.o test_a_f64_z_ui64_x.o test_a_f64_z_i32_x.o test_a_f64_z_i64_x.o test_a_f64_z_f16.o test_a_f64_z_f32.o test_a_f64_z_extF80.o test_a_f64_z_f128.o test_az_f64.o test_az_f64_rx.o test_abz_f64.o test_abcz_f64.o test_ab_f64_z_bool.o test_a_extF80_z_ui32_rx.o test_a_extF80_z_ui64_rx.o test_a_extF80_z_i32_rx.o test_a_extF80_z_i64_rx.o test_a_extF80_z_ui32_x.o test_a_extF80_z_ui64_x.o test_a_extF80_z_i32_x.o test_a_extF80_z_i64_x.o test_a_extF80_z_f16.o test_a_extF80_z_f32.o test_a_extF80_z_f64.o test_a_extF80_z_f128.o test_az_extF80.o test_az_extF80_rx.o test_abz_extF80.o test_ab_extF80_z_bool.o test_a_f128_z_ui32_rx.o test_a_f128_z_ui64_rx.o test_a_f128_z_i32_rx.o test_a_f128_z_i64_rx.o test_a_f128_z_ui32_x.o test_a_f128_z_ui64_x.o test_a_f128_z_i32_x.o test_a_f128_z_i64_x.o test_a_f128_z_f16.o test_a_f128_z_f32.o test_a_f128_z_f64.o test_a_f128_z_extF80.o test_az_f128.o test_az_f128_rx.o test_abz_f128.o test_abcz_f128.o test_ab_f128_z_bool.o
rm -f libsoftfloat.a && ar rcs libsoftfloat.a s_eq128.o s_le128.o s_lt128.o s_shortShiftLeft128.o s_shortShiftRight128.o s_shortShiftRightJam64.o s_shortShiftRightJam64Extra.o s_shortShiftRightJam128.o s_shortShiftRightJam128Extra.o s_shiftRightJam32.o s_shiftRightJam64.o s_shiftRightJam64Extra.o s_shiftRightJam128.o s_shiftRightJam128Extra.o s_shiftRightJam256M.o s_countLeadingZeros8.o s_countLeadingZeros16.o s_countLeadingZeros32.o s_countLeadingZeros64.o s_add128.o s_add256M.o s_sub128.o s_sub256M.o s_mul64ByShifted32To128.o s_mul64To128.o s_mul128By32.o s_mul128To256M.o s_approxRecip_1Ks.o s_approxRecip32_1.o s_approxRecipSqrt_1Ks.o s_approxRecipSqrt32_1.o s_roundToUI32.o s_roundToUI64.o s_roundToI32.o s_roundToI64.o s_normSubnormalF16Sig.o s_roundPackToF16.o s_normRoundPackToF16.o s_addMagsF16.o s_subMagsF16.o s_mulAddF16.o s_normSubnormalF32Sig.o s_roundPackToF32.o s_normRoundPackToF32.o s_addMagsF32.o s_subMagsF32.o s_mulAddF32.o s_normSubnormalF64Sig.o s_roundPackToF64.o s_normRoundPackToF64.o s_addMagsF64.o s_subMagsF64.o s_mulAddF64.o s_normSubnormalExtF80Sig.o s_roundPackToExtF80.o s_normRoundPackToExtF80.o s_addMagsExtF80.o s_subMagsExtF80.o s_normSubnormalF128Sig.o s_roundPackToF128.o s_normRoundPackToF128.o s_addMagsF128.o s_subMagsF128.o s_mulAddF128.o softfloat_state.o ui32_to_f16.o ui32_to_f32.o ui32_to_f64.o ui32_to_extF80.o ui32_to_extF80M.o ui32_to_f128.o ui32_to_f128M.o ui64_to_f16.o ui64_to_f32.o ui64_to_f64.o ui64_to_extF80.o ui64_to_extF80M.o ui64_to_f128.o ui64_to_f128M.o i32_to_f16.o i32_to_f32.o i32_to_f64.o i32_to_extF80.o i32_to_extF80M.o i32_to_f128.o i32_to_f128M.o i64_to_f16.o i64_to_f32.o i64_to_f64.o i64_to_extF80.o i64_to_extF80M.o i64_to_f128.o i64_to_f128M.o f16_to_ui32.o f16_to_ui64.o f16_to_i32.o f16_to_i64.o f16_to_ui32_r_minMag.o f16_to_ui64_r_minMag.o f16_to_i32_r_minMag.o f16_to_i64_r_minMag.o f16_to_f32.o f16_to_f64.o f16_to_extF80.o f16_to_extF80M.o f16_to_f128.o f16_to_f128M.o f16_roundToInt.o f16_add.o f16_sub.o f16_mul.o f16_mulAdd.o f16_div.o f16_rem.o f16_sqrt.o f16_eq.o f16_le.o f16_lt.o f16_eq_signaling.o f16_le_quiet.o f16_lt_quiet.o f16_isSignalingNaN.o f32_to_ui32.o f32_to_ui64.o f32_to_i32.o f32_to_i64.o f32_to_ui32_r_minMag.o f32_to_ui64_r_minMag.o f32_to_i32_r_minMag.o f32_to_i64_r_minMag.o f32_to_f16.o f32_to_f64.o f32_to_extF80.o f32_to_extF80M.o f32_to_f128.o f32_to_f128M.o f32_roundToInt.o f32_add.o f32_sub.o f32_mul.o f32_mulAdd.o f32_div.o f32_rem.o f32_sqrt.o f32_eq.o f32_le.o f32_lt.o f32_eq_signaling.o f32_le_quiet.o f32_lt_quiet.o f32_isSignalingNaN.o f64_to_ui32.o f64_to_ui64.o f64_to_i32.o f64_to_i64.o f64_to_ui32_r_minMag.o f64_to_ui64_r_minMag.o f64_to_i32_r_minMag.o f64_to_i64_r_minMag.o f64_to_f16.o f64_to_f32.o f64_to_extF80.o f64_to_extF80M.o f64_to_f128.o f64_to_f128M.o f64_roundToInt.o f64_add.o f64_sub.o f64_mul.o f64_mulAdd.o f64_div.o f64_rem.o f64_sqrt.o f64_eq.o f64_le.o f64_lt.o f64_eq_signaling.o f64_le_quiet.o f64_lt_quiet.o f64_isSignalingNaN.o extF80_to_ui32.o extF80_to_ui64.o extF80_to_i32.o extF80_to_i64.o extF80_to_ui32_r_minMag.o extF80_to_ui64_r_minMag.o extF80_to_i32_r_minMag.o extF80_to_i64_r_minMag.o extF80_to_f16.o extF80_to_f32.o extF80_to_f64.o extF80_to_f128.o extF80_roundToInt.o extF80_add.o extF80_sub.o extF80_mul.o extF80_div.o extF80_rem.o extF80_sqrt.o extF80_eq.o extF80_le.o extF80_lt.o extF80_eq_signaling.o extF80_le_quiet.o extF80_lt_quiet.o extF80_isSignalingNaN.o extF80M_to_ui32.o extF80M_to_ui64.o extF80M_to_i32.o extF80M_to_i64.o extF80M_to_ui32_r_minMag.o extF80M_to_ui64_r_minMag.o extF80M_to_i32_r_minMag.o extF80M_to_i64_r_minMag.o extF80M_to_f16.o extF80M_to_f32.o extF80M_to_f64.o extF80M_to_f128M.o extF80M_roundToInt.o extF80M_add.o extF80M_sub.o extF80M_mul.o extF80M_div.o extF80M_rem.o extF80M_sqrt.o extF80M_eq.o extF80M_le.o extF80M_lt.o extF80M_eq_signaling.o extF80M_le_quiet.o extF80M_lt_quiet.o f128_to_ui32.o f128_to_ui64.o f128_to_i32.o f128_to_i64.o f128_to_ui32_r_minMag.o f128_to_ui64_r_minMag.o f128_to_i32_r_minMag.o f128_to_i64_r_minMag.o f128_to_f16.o f128_to_f32.o f128_to_extF80.o f128_to_f64.o f128_roundToInt.o f128_add.o f128_sub.o f128_mul.o f128_mulAdd.o f128_div.o f128_rem.o f128_sqrt.o f128_eq.o f128_le.o f128_lt.o f128_eq_signaling.o f128_le_quiet.o f128_lt_quiet.o f128_isSignalingNaN.o f128M_to_ui32.o f128M_to_ui64.o f128M_to_i32.o f128M_to_i64.o f128M_to_ui32_r_minMag.o f128M_to_ui64_r_minMag.o f128M_to_i32_r_minMag.o f128M_to_i64_r_minMag.o f128M_to_f16.o f128M_to_f32.o f128M_to_extF80M.o f128M_to_f64.o f128M_roundToInt.o f128M_add.o f128M_sub.o f128M_mul.o f128M_mulAdd.o f128M_div.o f128M_rem.o f128M_sqrt.o f128M_eq.o f128M_le.o f128M_lt.o f128M_eq_signaling.o f128M_le_quiet.o f128M_lt_quiet.o softfloat_raiseFlags.o s_f16UIToCommonNaN.o s_commonNaNToF16UI.o s_propagateNaNF16UI.o s_f32UIToCommonNaN.o s_commonNaNToF32UI.o s_propagateNaNF32UI.o s_f64UIToCommonNaN.o s_commonNaNToF64UI.o s_propagateNaNF64UI.o extF80M_isSignalingNaN.o s_extF80UIToCommonNaN.o s_commonNaNToExtF80UI.o s_propagateNaNExtF80UI.o f128M_isSignalingNaN.o s_f128UIToCommonNaN.o s_commonNaNToF128UI.o s_propagateNaNF128UI.o
clang++ -g  -Wl,--warn-common -fsanitize=undefined -fsanitize=address -Wl,-z,relro -Wl,-z,now -pie -m64  -fstack-protector-strong -o fp-test fp-test.o slowfloat.o softfloat.o  libtestfloat.a libsoftfloat.a /tmp/qemu-test/build/libqemuutil.a   -lm -lz  -lgthread-2.0 -pthread -lglib-2.0  -lnettle  -lgnutls  -lzstd   -lrt
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
make[1]: Leaving directory '/tmp/qemu-test/build/tests/fp'
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ddb61b8d3116440182ccd7058ba70083', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-d55f2f3i/src/docker-src.2020-06-18-18.46.42.28017:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=ddb61b8d3116440182ccd7058ba70083
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-d55f2f3i/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    4m44.501s
user    0m8.824s


The full log is available at
http://patchew.org/logs/20200618222258.23287-1-walling@linux.ibm.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH v3 1/8] s390/sclp: get machine once during read scp/cpu info

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 2285 bytes --]

On 6/19/20 12:22 AM, Collin Walling wrote:
> Functions within read scp/cpu info will need access to the machine
> state. Let's make a call to retrieve the machine state once and
> pass the appropriate data to the respective functions.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 20aca30ac4..7875334037 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -49,9 +49,8 @@ static inline bool sclp_command_code_valid(uint32_t code)
>      return false;
>  }
>  
> -static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count)
> +static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>  {
> -    MachineState *ms = MACHINE(qdev_get_machine());
>      uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
>      int i;
>  
> @@ -77,7 +76,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>  
>      /* CPU information */
> -    prepare_cpu_entries(sclp, read_info->entries, &cpu_count);
> +    prepare_cpu_entries(machine, read_info->entries, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
>      read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
>      read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
> @@ -132,10 +131,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  /* Provide information about the CPU */
>  static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>  {
> +    MachineState *machine = MACHINE(qdev_get_machine());
>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
>      int cpu_count;
>  
> -    prepare_cpu_entries(sclp, cpu_info->entries, &cpu_count);
> +    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
>      cpu_info->nr_configured = cpu_to_be16(cpu_count);
>      cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
>      cpu_info->nr_standby = cpu_to_be16(0);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 4/8] s390/sclp: read sccb from mem based on sccb length

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 3185 bytes --]

On 6/19/20 12:22 AM, Collin Walling wrote:
> The header of the SCCB contains the actual length of the SCCB. Instead
> of using a static 4K size, let's allow for a variable size determined
> by the value set in the header. The proper checks are already in place
> to ensure the SCCB length is sufficent to store a full response, and
> that the length does not cross any explicitly-set boundaries.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

I wonder if this brings a measureable performance penalty with it for
protected guests. It's another ioctl to move the remaining bytes from
KVM to QEMU. On the other hand it's only sclp...

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 0710138f91..772b7b3b01 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -256,9 +256,8 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
>      SCLPDevice *sclp = get_sclp_device();
>      SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
>      SCCB work_sccb;
> -    hwaddr sccb_len = sizeof(SCCB);
>  
> -    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len);
> +    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sizeof(SCCBHeader));
>  
>      if (!sclp_command_code_valid(code)) {
>          work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> @@ -269,6 +268,9 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
>          goto out_write;
>      }
>  
> +    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb,
> +                         be16_to_cpu(work_sccb.h.length));
> +
>      sclp_c->execute(sclp, &work_sccb, code);
>  out_write:
>      s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
> @@ -283,8 +285,6 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>      SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
>      SCCB work_sccb;
>  
> -    hwaddr sccb_len = sizeof(SCCB);
> -
>      /* first some basic checks on program checks */
>      if (env->psw.mask & PSW_MASK_PSTATE) {
>          return -PGM_PRIVILEGED;
> @@ -302,7 +302,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>       * from playing dirty tricks by modifying the memory content after
>       * the host has checked the values
>       */
> -    cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
> +    cpu_physical_memory_read(sccb, &work_sccb, sizeof(SCCBHeader));
>  
>      /* Valid sccb sizes */
>      if (be16_to_cpu(work_sccb.h.length) < sizeof(SCCBHeader)) {
> @@ -318,6 +318,9 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>          goto out_write;
>      }
>  
> +    /* the header contains the actual length of the sccb */
> +    cpu_physical_memory_read(sccb, &work_sccb, be16_to_cpu(work_sccb.h.length));
> +
>      sclp_c->execute(sclp, &work_sccb, code);
>  out_write:
>      cpu_physical_memory_write(sccb, &work_sccb,
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 5/8] s390/sclp: use cpu offset to locate cpu entries

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 1719 bytes --]

On 6/19/20 12:22 AM, Collin Walling wrote:
> The start of the CPU entry region in the Read SCP Info response data is
> denoted by the offset_cpu field. As such, QEMU needs to begin creating
> entries at this address. Note that the length of the Read SCP Info data
> (data_len) denotes the same value as the cpu offset.
> 
> This is in preparation of when Read SCP Info inevitably introduces new
> bytes that push the start of the CPUEntry field further away.
> 
> Read CPU Info is unlikely to ever change, so let's not bother
> accounting for the offset there.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 772b7b3b01..0dfbe6e5ec 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -113,13 +113,14 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>      int data_len = get_read_scp_info_data_len();
> +    CPUEntry *entries_start = (void *)sccb + data_len;
>  
>      if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>          return;
>      }
>  
>      /* CPU information */
> -    prepare_cpu_entries(machine, read_info->entries, &cpu_count);
> +    prepare_cpu_entries(machine, entries_start, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
>      read_info->offset_cpu = cpu_to_be16(data_len);
>      read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 8/8] s390: guest support for diagnose 0x318

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 9831 bytes --]

On 6/19/20 12:22 AM, Collin Walling wrote:
> DIAGNOSE 0x318 (diag318) is an s390 instruction that allows the storage
> of diagnostic information that is collected by the firmware in the case
> of hardware/firmware service events.
> 
> QEMU handles the instruction by storing the info in the CPU state. A
> subsequent register sync will communicate the data to the hypervisor.
> 
> QEMU handles the migration via a VM State Description.
> 
> This feature depends on the Extended-Length SCCB (els) feature. If
> els is not present, then a warning will be printed and the SCLP bit
> that allows the Linux kernel to execute the instruction will not be
> set.
> 
> Availability of this instruction is determined by byte 134 (aka fac134)
> bit 0 of the SCLP Read Info block. This coincidentally expands into the
> space used for CPU entries, which means VMs running with the diag318
> capability may not be able to read information regarding all CPUs
> unless the guest kernel supports an extended-length SCCB.
> 
> This feature is not supported in protected virtualization mode.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>

Acked-by: Janosch Frank <frankja@linux.ibm.com>

> ---
>  hw/s390x/sclp.c                     |  5 +++++
>  include/hw/s390x/sclp.h             |  3 +++
>  target/s390x/cpu.h                  |  3 ++-
>  target/s390x/cpu_features.h         |  1 +
>  target/s390x/cpu_features_def.inc.h |  3 +++
>  target/s390x/cpu_models.c           |  1 +
>  target/s390x/gen-features.c         |  1 +
>  target/s390x/kvm.c                  | 31 +++++++++++++++++++++++++++++
>  target/s390x/machine.c              | 17 ++++++++++++++++
>  9 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index f7c49e339e..78dbfbe427 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -152,6 +152,11 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>                           read_info->conf_char_ext);
>  
> +    if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> +        s390_get_feat_block(S390_FEAT_TYPE_SCLP_FAC134,
> +                            &read_info->fac134);
> +    }
> +
>      read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>                                          SCLP_HAS_IOA_RECONFIG);
>  
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index ef2d63eae9..ccb9f0a676 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -133,6 +133,9 @@ typedef struct ReadInfo {
>      uint16_t highest_cpu;
>      uint8_t  _reserved5[124 - 122];     /* 122-123 */
>      uint32_t hmfai;
> +    uint8_t  _reserved7[134 - 128];     /* 128-133 */
> +    uint8_t  fac134;
> +    uint8_t  _reserved8[144 - 135];     /* 135-143 */
>      struct CPUEntry entries[];
>  } QEMU_PACKED ReadInfo;
>  
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 035427521c..52765961cf 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -112,6 +112,8 @@ struct CPUS390XState {
>      uint16_t external_call_addr;
>      DECLARE_BITMAP(emergency_signals, S390_MAX_CPUS);
>  
> +    uint64_t diag318_info;
> +
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;
>  
> @@ -136,7 +138,6 @@ struct CPUS390XState {
>  
>      /* currently processed sigp order */
>      uint8_t sigp_order;
> -

Whitespace damage

>  };
>  
>  static inline uint64_t *get_freg(CPUS390XState *cs, int nr)
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index da695a8346..f74f7fc3a1 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
>      S390_FEAT_TYPE_STFL,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +    S390_FEAT_TYPE_SCLP_FAC134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.inc.h b/target/s390x/cpu_features_def.inc.h
> index 1c04cc18f4..f82b4b5ec1 100644
> --- a/target/s390x/cpu_features_def.inc.h
> +++ b/target/s390x/cpu_features_def.inc.h
> @@ -122,6 +122,9 @@ DEF_FEAT(SIE_CMMA, "cmma", SCLP_CONF_CHAR_EXT, 1, "SIE: Collaborative-memory-man
>  DEF_FEAT(SIE_PFMFI, "pfmfi", SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility")
>  DEF_FEAT(SIE_IBS, "ibs", SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility")
>  
> +/* Features exposed via SCLP SCCB Facilities byte 134 (bit numbers relative to byte-134) */
> +DEF_FEAT(DIAG_318, "diag318", SCLP_FAC134, 0, "Control program name and version codes")
> +
>  /* Features exposed via SCLP CPU info. */
>  DEF_FEAT(SIE_F2, "sief2", SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)")
>  DEF_FEAT(SIE_SKEY, "skey", SCLP_CPU, 5, "SIE: Storage-key facility")
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 2fa609bffe..034673be54 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -827,6 +827,7 @@ static void check_consistency(const S390CPUModel *model)
>          { S390_FEAT_PTFF_STOE, S390_FEAT_MULTIPLE_EPOCH },
>          { S390_FEAT_PTFF_STOUE, S390_FEAT_MULTIPLE_EPOCH },
>          { S390_FEAT_AP_QUEUE_INTERRUPT_CONTROL, S390_FEAT_AP },
> +        { S390_FEAT_DIAG_318, S390_FEAT_EXTENDED_LENGTH_SCCB },
>      };
>      int i;
>  
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 6857f657fb..a1f0a6f3c6 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -523,6 +523,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
>      S390_FEAT_EXTENDED_LENGTH_SCCB,
> +    S390_FEAT_DIAG_318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index a2d5ad78f6..b79feeba9f 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -105,6 +105,7 @@
>  
>  #define DIAG_TIMEREVENT                 0x288
>  #define DIAG_IPL                        0x308
> +#define DIAG_SET_CONTROL_PROGRAM_CODES  0x318
>  #define DIAG_KVM_HYPERCALL              0x500
>  #define DIAG_KVM_BREAKPOINT             0x501
>  
> @@ -602,6 +603,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>          cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_ETOKEN;
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
> +        cs->kvm_run->s.regs.diag318 = env->diag318_info;
> +        cs->kvm_run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
> +    }
> +
>      /* Finally the prefix */
>      if (can_sync_regs(cs, KVM_SYNC_PREFIX)) {
>          cs->kvm_run->s.regs.prefix = env->psa;
> @@ -741,6 +747,10 @@ int kvm_arch_get_registers(CPUState *cs)
>          }
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_DIAG318)) {
> +        env->diag318_info = cs->kvm_run->s.regs.diag318;
> +    }
> +
>      return 0;
>  }
>  
> @@ -1601,6 +1611,19 @@ static int handle_sw_breakpoint(S390CPU *cpu, struct kvm_run *run)
>      return -ENOENT;
>  }
>  
> +static void handle_diag_318(S390CPU *cpu, struct kvm_run *run)
> +{
> +    uint64_t reg = (run->s390_sieic.ipa & 0x00f0) >> 4;
> +    uint64_t diag318_info = run->s.regs.gprs[reg];
> +
> +    cpu->env.diag318_info = diag318_info;
> +
> +    if (can_sync_regs(CPU(cpu), KVM_SYNC_DIAG318)) {
> +        run->s.regs.diag318 = diag318_info;
> +        run->kvm_dirty_regs |= KVM_SYNC_DIAG318;
> +    }
> +}
> +
>  #define DIAG_KVM_CODE_MASK 0x000000000000ffff
>  
>  static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
> @@ -1620,6 +1643,9 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
>      case DIAG_IPL:
>          kvm_handle_diag_308(cpu, run);
>          break;
> +    case DIAG_SET_CONTROL_PROGRAM_CODES:
> +        handle_diag_318(cpu, run);
> +        break;
>      case DIAG_KVM_HYPERCALL:
>          r = handle_hypercall(cpu, run);
>          break;
> @@ -2464,6 +2490,11 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>       */
>      set_bit(S390_FEAT_EXTENDED_LENGTH_SCCB, model->features);
>  
> +    /* DIAGNOSE 0x318 is not supported under protected virtualization */
> +    if (!s390_is_pv() && kvm_check_extension(kvm_state, KVM_CAP_S390_DIAG318)) {
> +        set_bit(S390_FEAT_DIAG_318, model->features);
> +    }
> +
>      /* strip of features that are not part of the maximum model */
>      bitmap_and(model->features, model->features, model->def->full_feat,
>                 S390_FEAT_MAX);
> diff --git a/target/s390x/machine.c b/target/s390x/machine.c
> index 549bb6c280..5b4e82f1ab 100644
> --- a/target/s390x/machine.c
> +++ b/target/s390x/machine.c
> @@ -234,6 +234,22 @@ const VMStateDescription vmstate_etoken = {
>      }
>  };
>  
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG_318);
> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "cpu/diag318",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(env.diag318_info, S390CPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_s390_cpu = {
>      .name = "cpu",
>      .post_load = cpu_post_load,
> @@ -270,6 +286,7 @@ const VMStateDescription vmstate_s390_cpu = {
>          &vmstate_gscb,
>          &vmstate_bpbc,
>          &vmstate_etoken,
> +        &vmstate_diag318,
>          NULL
>      },
>  };
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 3948 bytes --]

On 6/19/20 12:22 AM, Collin Walling wrote:
> Rework the SCLP boundary check to account for different SCLP commands
> (eventually) allowing different boundary sizes.
> 
> Move the length check code into a separate function, and introduce a
> new function to determine the length of the read SCP data (i.e. the size
> from the start of the struct to where the CPU entries should begin).
> 
> The format of read CPU info is unlikely to change in the future,
> so we do not require a separate function to calculate its length.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>

Acked-by: Janosch Frank <frankja@linux.ibm.com>

> ---
[...]
> +/*
> + * The data length denotes the start of the struct to where the first
> + * CPU entry is to be allocated. This value also denotes the offset_cpu
> + * field.
> + */
> +static inline int get_read_scp_info_data_len(void)
> +{
> +    return offsetof(ReadInfo, entries);
> +}
> +
>  /* Provide information about the configuration, CPUs and storage */
>  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  {
> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int cpu_count;
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
> +    int data_len = get_read_scp_info_data_len();

Does somebody have a better name than data_len at hand?

>  
> -    if (be16_to_cpu(sccb->h.length) <
> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>          return;
>      }
>  
>      /* CPU information */
>      prepare_cpu_entries(machine, read_info->entries, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
> -    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
> +    read_info->offset_cpu = cpu_to_be16(data_len);
>      read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
>  
>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
> @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
> +    int data_len = offsetof(ReadCpuInfo, entries);
>      int cpu_count;
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>          return;
>      }
>  
>      prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
>      cpu_info->nr_configured = cpu_to_be16(cpu_count);
> -    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
> +    cpu_info->offset_configured = cpu_to_be16(data_len);
>      cpu_info->nr_standby = cpu_to_be16(0);
>  
>      /* The standby offset is 16-byte for each CPU */
> @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
>          goto out_write;
>      }
>  
> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
> +        goto out_write;
> +    }
> +
>      sclp_c->execute(sclp, &work_sccb, code);
>  out_write:
>      s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
> @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>          goto out_write;
>      }
>  
> -    if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
> -        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
>          goto out_write;
>      }
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/8] s390/sclp: check sccb len before filling in data

[David Hildenbrand]

On 19.06.20 00:22, Collin Walling wrote:
> The SCCB must be checked for a sufficient length before it is filled
> with any data. If the length is insufficient, then the SCLP command
> is suppressed and the proper response code is set in the SCCB header.
> 
> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/sclp.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 7875334037..181ce04007 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -75,6 +75,12 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>  
> +    if (be16_to_cpu(sccb->h.length) <
> +          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return;
> +    }
> +
>      /* CPU information */
>      prepare_cpu_entries(machine, read_info->entries, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
> @@ -83,12 +89,6 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  
>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -            (sizeof(ReadInfo) + cpu_count * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> -        return;
> -    }
> -
>      /* Configuration Characteristic (Extension) */
>      s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR,
>                           read_info->conf_char);
> @@ -135,17 +135,17 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
>      int cpu_count;
>  
> -    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
> -    cpu_info->nr_configured = cpu_to_be16(cpu_count);
> -    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
> -    cpu_info->nr_standby = cpu_to_be16(0);
> -
>      if (be16_to_cpu(sccb->h.length) <
> -            (sizeof(ReadCpuInfo) + cpu_count * sizeof(CPUEntry))) {
> +          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
>          sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>          return;
>      }
>  
> +    prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
> +    cpu_info->nr_configured = cpu_to_be16(cpu_count);
> +    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
> +    cpu_info->nr_standby = cpu_to_be16(0);
> +
>      /* The standby offset is 16-byte for each CPU */
>      cpu_info->offset_standby = cpu_to_be16(cpu_info->offset_configured
>          + cpu_info->nr_configured*sizeof(CPUEntry));
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 1/8] s390/sclp: get machine once during read scp/cpu info

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:51 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> Functions within read scp/cpu info will need access to the machine
> state. Let's make a call to retrieve the machine state once and
> pass the appropriate data to the respective functions.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/s390x/sclp.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 2/8] s390/sclp: check sccb len before filling in data

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:52 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> The SCCB must be checked for a sufficient length before it is filled
> with any data. If the length is insufficient, then the SCLP command
> is suppressed and the proper response code is set in the SCCB header.
> 
> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/sclp.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Cornelia Huck]

[-- Attachment #1: Type: text/plain, Size: 1669 bytes --]

On Fri, 19 Jun 2020 12:50:11 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 6/19/20 12:22 AM, Collin Walling wrote:
> > Rework the SCLP boundary check to account for different SCLP commands
> > (eventually) allowing different boundary sizes.
> > 
> > Move the length check code into a separate function, and introduce a
> > new function to determine the length of the read SCP data (i.e. the size
> > from the start of the struct to where the CPU entries should begin).
> > 
> > The format of read CPU info is unlikely to change in the future,
> > so we do not require a separate function to calculate its length.
> > 
> > Signed-off-by: Collin Walling <walling@linux.ibm.com>  
> 
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> 
> > ---  
> [...]
> > +/*
> > + * The data length denotes the start of the struct to where the first
> > + * CPU entry is to be allocated. This value also denotes the offset_cpu
> > + * field.
> > + */
> > +static inline int get_read_scp_info_data_len(void)
> > +{
> > +    return offsetof(ReadInfo, entries);
> > +}
> > +
> >  /* Provide information about the configuration, CPUs and storage */
> >  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
> >  {
> > @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
> >      int cpu_count;
> >      int rnsize, rnmax;
> >      IplParameterBlock *ipib = s390_ipl_get_iplb();
> > +    int data_len = get_read_scp_info_data_len();  
> 
> Does somebody have a better name than data_len at hand?

I could not come up with anything better, either :(

(...)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 4/8] s390/sclp: read sccb from mem based on sccb length

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:54 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> The header of the SCCB contains the actual length of the SCCB. Instead
> of using a static 4K size, let's allow for a variable size determined
> by the value set in the header. The proper checks are already in place
> to ensure the SCCB length is sufficent to store a full response, and
> that the length does not cross any explicitly-set boundaries.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/s390x/sclp.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 5/8] s390/sclp: use cpu offset to locate cpu entries

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:55 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> The start of the CPU entry region in the Read SCP Info response data is
> denoted by the offset_cpu field. As such, QEMU needs to begin creating
> entries at this address. Note that the length of the Read SCP Info data
> (data_len) denotes the same value as the cpu offset.
> 
> This is in preparation of when Read SCP Info inevitably introduces new
> bytes that push the start of the CPUEntry field further away.
> 
> Read CPU Info is unlikely to ever change, so let's not bother
> accounting for the offset there.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/s390x/sclp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Christian Borntraeger]

On 19.06.20 12:50, Janosch Frank wrote:
> On 6/19/20 12:22 AM, Collin Walling wrote:
>> Rework the SCLP boundary check to account for different SCLP commands
>> (eventually) allowing different boundary sizes.
>>
>> Move the length check code into a separate function, and introduce a
>> new function to determine the length of the read SCP data (i.e. the size
>> from the start of the struct to where the CPU entries should begin).
>>
>> The format of read CPU info is unlikely to change in the future,
>> so we do not require a separate function to calculate its length.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> 
> Acked-by: Janosch Frank <frankja@linux.ibm.com>
> 
>> ---
> [...]
>> +/*
>> + * The data length denotes the start of the struct to where the first
>> + * CPU entry is to be allocated. This value also denotes the offset_cpu
>> + * field.
>> + */
>> +static inline int get_read_scp_info_data_len(void)
>> +{
>> +    return offsetof(ReadInfo, entries);
>> +}
>> +
>>  /* Provide information about the configuration, CPUs and storage */
>>  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>  {
>> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>      int cpu_count;
>>      int rnsize, rnmax;
>>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>> +    int data_len = get_read_scp_info_data_len();
> 
> Does somebody have a better name than data_len at hand?

I stumbled over the same (also for the function). What about cpu_offset instead of data_len?

Re: [PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Christian Borntraeger]

On 19.06.20 00:22, Collin Walling wrote:
> Rework the SCLP boundary check to account for different SCLP commands
> (eventually) allowing different boundary sizes.
> 
> Move the length check code into a separate function, and introduce a
> new function to determine the length of the read SCP data (i.e. the size
> from the start of the struct to where the CPU entries should begin).
> 
> The format of read CPU info is unlikely to change in the future,
> so we do not require a separate function to calculate its length.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 49 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 181ce04007..0710138f91 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code)
>      return false;
>  }
>  
> +static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
> +                                    SCCBHeader *header)

As you write to the sccb in case of error, mabye
sccb_verify_boundary instead of has_valid. has_valid feels like a read-only function.

> +{
> +    uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1;
> +    uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
> +
> +    switch (code & SCLP_CMD_CODE_MASK) {
> +    default:
> +        if (sccb_max_addr < sccb_boundary) {
> +            return true;
> +        }
> +    }
> +    header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> +    return false;
> +}
> +
> +/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */
> +static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)

same here, maybe sccb_verify_length

> +{
> +    int required_len = data_len + num_cpus * sizeof(CPUEntry);
> +
> +    if (be16_to_cpu(sccb->h.length) < required_len) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        return false;
> +    }
> +    return true;
> +}
> +
>  static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>  {
>      uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
> @@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>      }
>  }
>  
> +/*
> + * The data length denotes the start of the struct to where the first
> + * CPU entry is to be allocated. This value also denotes the offset_cpu
> + * field.
> + */
> +static inline int get_read_scp_info_data_len(void)
> +{
> +    return offsetof(ReadInfo, entries);
> +}
> +
>  /* Provide information about the configuration, CPUs and storage */
>  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>  {
> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      int cpu_count;
>      int rnsize, rnmax;
>      IplParameterBlock *ipib = s390_ipl_get_iplb();
> +    int data_len = get_read_scp_info_data_len();
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>          return;
>      }
>  
>      /* CPU information */
>      prepare_cpu_entries(machine, read_info->entries, &cpu_count);
>      read_info->entries_cpu = cpu_to_be16(cpu_count);
> -    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
> +    read_info->offset_cpu = cpu_to_be16(data_len);
>      read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
>  
>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
> @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>  {
>      MachineState *machine = MACHINE(qdev_get_machine());
>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
> +    int data_len = offsetof(ReadCpuInfo, entries);
>      int cpu_count;
>  
> -    if (be16_to_cpu(sccb->h.length) <
> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>          return;
>      }
>  
>      prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
>      cpu_info->nr_configured = cpu_to_be16(cpu_count);
> -    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
> +    cpu_info->offset_configured = cpu_to_be16(data_len);
>      cpu_info->nr_standby = cpu_to_be16(0);
>  
>      /* The standby offset is 16-byte for each CPU */
> @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
>          goto out_write;
>      }
>  
> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
> +        goto out_write;
> +    }
> +
>      sclp_c->execute(sclp, &work_sccb, code);
>  out_write:
>      s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
> @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>          goto out_write;
>      }
>  
> -    if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
> -        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
>          goto out_write;
>      }
>  
> 

Re: [PATCH v3 3/8] s390/sclp: rework sclp boundary and length checks

[Collin Walling]

On 6/22/20 11:22 AM, Christian Borntraeger wrote:
> 
> 
> On 19.06.20 00:22, Collin Walling wrote:
>> Rework the SCLP boundary check to account for different SCLP commands
>> (eventually) allowing different boundary sizes.
>>
>> Move the length check code into a separate function, and introduce a
>> new function to determine the length of the read SCP data (i.e. the size
>> from the start of the struct to where the CPU entries should begin).
>>
>> The format of read CPU info is unlikely to change in the future,
>> so we do not require a separate function to calculate its length.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>  hw/s390x/sclp.c | 59 ++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 49 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 181ce04007..0710138f91 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -49,6 +49,34 @@ static inline bool sclp_command_code_valid(uint32_t code)
>>      return false;
>>  }
>>  
>> +static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
>> +                                    SCCBHeader *header)
> 
> As you write to the sccb in case of error, mabye
> sccb_verify_boundary instead of has_valid. has_valid feels like a read-only function.
> 
>> +{
>> +    uint64_t sccb_max_addr = sccb_addr + be16_to_cpu(header->length) - 1;
>> +    uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
>> +
>> +    switch (code & SCLP_CMD_CODE_MASK) {
>> +    default:
>> +        if (sccb_max_addr < sccb_boundary) {
>> +            return true;
>> +        }
>> +    }
>> +    header->response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>> +    return false;
>> +}
>> +
>> +/* Calculates sufficient SCCB length to store a full Read SCP/CPU response */
>> +static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
> 
> same here, maybe sccb_verify_length

Sounds good. I was struggling with a decent naming scheme for these as
well :)

> 
>> +{
>> +    int required_len = data_len + num_cpus * sizeof(CPUEntry);
>> +
>> +    if (be16_to_cpu(sccb->h.length) < required_len) {
>> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>> +        return false;
>> +    }
>> +    return true;
>> +}
>> +
>>  static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>>  {
>>      uint8_t features[SCCB_CPU_FEATURE_LEN] = { 0 };
>> @@ -66,6 +94,16 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>>      }
>>  }
>>  
>> +/*
>> + * The data length denotes the start of the struct to where the first
>> + * CPU entry is to be allocated. This value also denotes the offset_cpu
>> + * field.
>> + */
>> +static inline int get_read_scp_info_data_len(void)
>> +{
>> +    return offsetof(ReadInfo, entries);
>> +}
>> +
>>  /* Provide information about the configuration, CPUs and storage */
>>  static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>  {
>> @@ -74,17 +112,16 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>      int cpu_count;
>>      int rnsize, rnmax;
>>      IplParameterBlock *ipib = s390_ipl_get_iplb();
>> +    int data_len = get_read_scp_info_data_len();
>>  
>> -    if (be16_to_cpu(sccb->h.length) <
>> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
>> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>>          return;
>>      }
>>  
>>      /* CPU information */
>>      prepare_cpu_entries(machine, read_info->entries, &cpu_count);
>>      read_info->entries_cpu = cpu_to_be16(cpu_count);
>> -    read_info->offset_cpu = cpu_to_be16(offsetof(ReadInfo, entries));
>> +    read_info->offset_cpu = cpu_to_be16(data_len);
>>      read_info->highest_cpu = cpu_to_be16(machine->smp.max_cpus - 1);
>>  
>>      read_info->ibc_val = cpu_to_be32(s390_get_ibc_val());
>> @@ -133,17 +170,16 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>>  {
>>      MachineState *machine = MACHINE(qdev_get_machine());
>>      ReadCpuInfo *cpu_info = (ReadCpuInfo *) sccb;
>> +    int data_len = offsetof(ReadCpuInfo, entries);
>>      int cpu_count;
>>  
>> -    if (be16_to_cpu(sccb->h.length) <
>> -          (sizeof(ReadInfo) + machine->possible_cpus->len * sizeof(CPUEntry))) {
>> -        sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>> +    if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>>          return;
>>      }
>>  
>>      prepare_cpu_entries(machine, cpu_info->entries, &cpu_count);
>>      cpu_info->nr_configured = cpu_to_be16(cpu_count);
>> -    cpu_info->offset_configured = cpu_to_be16(offsetof(ReadCpuInfo, entries));
>> +    cpu_info->offset_configured = cpu_to_be16(data_len);
>>      cpu_info->nr_standby = cpu_to_be16(0);
>>  
>>      /* The standby offset is 16-byte for each CPU */
>> @@ -229,6 +265,10 @@ int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
>>          goto out_write;
>>      }
>>  
>> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
>> +        goto out_write;
>> +    }
>> +
>>      sclp_c->execute(sclp, &work_sccb, code);
>>  out_write:
>>      s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
>> @@ -274,8 +314,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
>>          goto out_write;
>>      }
>>  
>> -    if ((sccb + be16_to_cpu(work_sccb.h.length)) > ((sccb & PAGE_MASK) + PAGE_SIZE)) {
>> -        work_sccb.h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>> +    if (!sccb_has_valid_boundary(sccb, code, &work_sccb.h)) {
>>          goto out_write;
>>      }
>>  
>>


-- 
Regards,
Collin

Stay safe and stay healthy

Re: [PATCH v3 2/8] s390/sclp: check sccb len before filling in data

[Thomas Huth]

On 19/06/2020 00.22, Collin Walling wrote:
> The SCCB must be checked for a sufficient length before it is filled
> with any data. If the length is insufficient, then the SCLP command
> is suppressed and the proper response code is set in the SCCB header.
> 
> Fixes: 832be0d8a3bb ("s390x: sclp: Report insufficient SCCB length")
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   hw/s390x/sclp.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:56 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> As more features and facilities are added to the Read SCP Info (RSCPI)
> response, more space is required to store them. The space used to store
> these new features intrudes on the space originally used to store CPU
> entries. This means as more features and facilities are added to the
> RSCPI response, less space can be used to store CPU entries.
> 
> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
> the RSCPI command and determine if the SCCB is large enough to store a
> complete reponse. If it is not large enough, then the required length
> will be set in the SCCB header.
> 
> The caller of the SCLP command is responsible for creating a
> large-enough SCCB to store a complete response. Proper checking should
> be in place, and the caller should execute the command once-more with
> the large-enough SCCB.
> 
> This facility also enables an extended SCCB for the Read CPU Info
> (RCPUI) command.
> 
> When this facility is enabled, the boundary violation response cannot
> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
> 
> In order to tolerate kernels that do not yet have full support for this
> feature, a "fixed" offset to the start of the CPU Entries within the
> Read SCP Info struct is set to allow for the original 248 max entries
> when this feature is disabled.
> 
> Additionally, this is introduced as a CPU feature to protect the guest
> from migrating to a machine that does not support storing an extended
> SCCB. This could otherwise hinder the VM from being able to read all
> available CPU entries after migration (such as during re-ipl).
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
>  include/hw/s390x/sclp.h             |  1 +
>  target/s390x/cpu_features_def.inc.h |  1 +
>  target/s390x/gen-features.c         |  1 +
>  target/s390x/kvm.c                  |  8 ++++++++
>  5 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 0dfbe6e5ec..f7c49e339e 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
>      uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
>  
>      switch (code & SCLP_CMD_CODE_MASK) {
> +    case SCLP_CMDW_READ_SCP_INFO:
> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> +    case SCLP_CMDW_READ_CPU_INFO:
> +        /*
> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
> +         * is allowed to exceed the 4k boundary. The respective commands will
> +         * set the length field to the required length if an insufficient
> +         * SCCB length is provided.
> +         */
> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> +            return true;
> +        }

Add a fallthrough annotation?

>      default:
>          if (sccb_max_addr < sccb_boundary) {
>              return true;
> @@ -72,6 +84,10 @@ static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
>  
>      if (be16_to_cpu(sccb->h.length) < required_len) {
>          sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) &&
> +            sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
> +            sccb->h.length = required_len;
> +        }
>          return false;
>      }
>      return true;
> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>   */
>  static inline int get_read_scp_info_data_len(void)
>  {
> -    return offsetof(ReadInfo, entries);
> +    return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
> +           offsetof(ReadInfo, entries) :
> +           SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
>  }
>  
>  /* Provide information about the configuration, CPUs and storage */
> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>      CPUEntry *entries_start = (void *)sccb + data_len;
>  
>      if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
> +        warn_report("insufficient sccb size to store read scp info response");

Hm, this warning is triggered by a guest action, isn't it? Not sure how
helpful it is.

>          return;
>      }
>  

(...)

Otherwise, looks good to me.

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Thomas Huth]

On 24/06/2020 14.36, Cornelia Huck wrote:
> On Thu, 18 Jun 2020 18:22:56 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> As more features and facilities are added to the Read SCP Info (RSCPI)
>> response, more space is required to store them. The space used to store
>> these new features intrudes on the space originally used to store CPU
>> entries. This means as more features and facilities are added to the
>> RSCPI response, less space can be used to store CPU entries.
>>
>> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
>> the RSCPI command and determine if the SCCB is large enough to store a
>> complete reponse. If it is not large enough, then the required length
>> will be set in the SCCB header.
>>
>> The caller of the SCLP command is responsible for creating a
>> large-enough SCCB to store a complete response. Proper checking should
>> be in place, and the caller should execute the command once-more with
>> the large-enough SCCB.
>>
>> This facility also enables an extended SCCB for the Read CPU Info
>> (RCPUI) command.
>>
>> When this facility is enabled, the boundary violation response cannot
>> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
>>
>> In order to tolerate kernels that do not yet have full support for this
>> feature, a "fixed" offset to the start of the CPU Entries within the
>> Read SCP Info struct is set to allow for the original 248 max entries
>> when this feature is disabled.
>>
>> Additionally, this is introduced as a CPU feature to protect the guest
>> from migrating to a machine that does not support storing an extended
>> SCCB. This could otherwise hinder the VM from being able to read all
>> available CPU entries after migration (such as during re-ipl).
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>   hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
>>   include/hw/s390x/sclp.h             |  1 +
>>   target/s390x/cpu_features_def.inc.h |  1 +
>>   target/s390x/gen-features.c         |  1 +
>>   target/s390x/kvm.c                  |  8 ++++++++
>>   5 files changed, 31 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 0dfbe6e5ec..f7c49e339e 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
>>       uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
>>   
>>       switch (code & SCLP_CMD_CODE_MASK) {
>> +    case SCLP_CMDW_READ_SCP_INFO:
>> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
>> +    case SCLP_CMDW_READ_CPU_INFO:
>> +        /*
>> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
>> +         * is allowed to exceed the 4k boundary. The respective commands will
>> +         * set the length field to the required length if an insufficient
>> +         * SCCB length is provided.
>> +         */
>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
>> +            return true;
>> +        }
> 
> Add a fallthrough annotation?

... otherwise Coverity and friends will complain later.

>>       default:
>>           if (sccb_max_addr < sccb_boundary) {
>>               return true;
>> @@ -72,6 +84,10 @@ static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
>>   
>>       if (be16_to_cpu(sccb->h.length) < required_len) {
>>           sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) &&
>> +            sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
>> +            sccb->h.length = required_len;
>> +        }
>>           return false;
>>       }
>>       return true;
>> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>>    */
>>   static inline int get_read_scp_info_data_len(void)
>>   {
>> -    return offsetof(ReadInfo, entries);
>> +    return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
>> +           offsetof(ReadInfo, entries) :
>> +           SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
>>   }
>>   
>>   /* Provide information about the configuration, CPUs and storage */
>> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>       CPUEntry *entries_start = (void *)sccb + data_len;
>>   
>>       if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>> +        warn_report("insufficient sccb size to store read scp info response");
> 
> Hm, this warning is triggered by a guest action, isn't it? Not sure how
> helpful it is.

I think this should be qemu_log_mask(LOG_GUEST_ERROR, ...) instead?

  Thomas

Re: [PATCH v3 8/8] s390: guest support for diagnose 0x318

[Cornelia Huck]

On Thu, 18 Jun 2020 18:22:58 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> DIAGNOSE 0x318 (diag318) is an s390 instruction that allows the storage
> of diagnostic information that is collected by the firmware in the case
> of hardware/firmware service events.
> 
> QEMU handles the instruction by storing the info in the CPU state. A
> subsequent register sync will communicate the data to the hypervisor.
> 
> QEMU handles the migration via a VM State Description.
> 
> This feature depends on the Extended-Length SCCB (els) feature. If
> els is not present, then a warning will be printed and the SCLP bit
> that allows the Linux kernel to execute the instruction will not be
> set.
> 
> Availability of this instruction is determined by byte 134 (aka fac134)
> bit 0 of the SCLP Read Info block. This coincidentally expands into the
> space used for CPU entries, which means VMs running with the diag318
> capability may not be able to read information regarding all CPUs
> unless the guest kernel supports an extended-length SCCB.

We cannot do anything about that, I guess.

> 
> This feature is not supported in protected virtualization mode.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  hw/s390x/sclp.c                     |  5 +++++
>  include/hw/s390x/sclp.h             |  3 +++
>  target/s390x/cpu.h                  |  3 ++-
>  target/s390x/cpu_features.h         |  1 +
>  target/s390x/cpu_features_def.inc.h |  3 +++
>  target/s390x/cpu_models.c           |  1 +
>  target/s390x/gen-features.c         |  1 +
>  target/s390x/kvm.c                  | 31 +++++++++++++++++++++++++++++
>  target/s390x/machine.c              | 17 ++++++++++++++++
>  9 files changed, 64 insertions(+), 1 deletion(-)

LGTM.

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Cornelia Huck]

On Wed, 24 Jun 2020 14:40:58 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 24/06/2020 14.36, Cornelia Huck wrote:
> > On Thu, 18 Jun 2020 18:22:56 -0400
> > Collin Walling <walling@linux.ibm.com> wrote:
> >   
> >> As more features and facilities are added to the Read SCP Info (RSCPI)
> >> response, more space is required to store them. The space used to store
> >> these new features intrudes on the space originally used to store CPU
> >> entries. This means as more features and facilities are added to the
> >> RSCPI response, less space can be used to store CPU entries.
> >>
> >> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
> >> the RSCPI command and determine if the SCCB is large enough to store a
> >> complete reponse. If it is not large enough, then the required length
> >> will be set in the SCCB header.
> >>
> >> The caller of the SCLP command is responsible for creating a
> >> large-enough SCCB to store a complete response. Proper checking should
> >> be in place, and the caller should execute the command once-more with
> >> the large-enough SCCB.
> >>
> >> This facility also enables an extended SCCB for the Read CPU Info
> >> (RCPUI) command.
> >>
> >> When this facility is enabled, the boundary violation response cannot
> >> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
> >>
> >> In order to tolerate kernels that do not yet have full support for this
> >> feature, a "fixed" offset to the start of the CPU Entries within the
> >> Read SCP Info struct is set to allow for the original 248 max entries
> >> when this feature is disabled.
> >>
> >> Additionally, this is introduced as a CPU feature to protect the guest
> >> from migrating to a machine that does not support storing an extended
> >> SCCB. This could otherwise hinder the VM from being able to read all
> >> available CPU entries after migration (such as during re-ipl).
> >>
> >> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> >> ---
> >>   hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
> >>   include/hw/s390x/sclp.h             |  1 +
> >>   target/s390x/cpu_features_def.inc.h |  1 +
> >>   target/s390x/gen-features.c         |  1 +
> >>   target/s390x/kvm.c                  |  8 ++++++++
> >>   5 files changed, 31 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> >> index 0dfbe6e5ec..f7c49e339e 100644
> >> --- a/hw/s390x/sclp.c
> >> +++ b/hw/s390x/sclp.c
> >> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
> >>       uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
> >>   
> >>       switch (code & SCLP_CMD_CODE_MASK) {
> >> +    case SCLP_CMDW_READ_SCP_INFO:
> >> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> >> +    case SCLP_CMDW_READ_CPU_INFO:
> >> +        /*
> >> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
> >> +         * is allowed to exceed the 4k boundary. The respective commands will
> >> +         * set the length field to the required length if an insufficient
> >> +         * SCCB length is provided.
> >> +         */
> >> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> >> +            return true;
> >> +        }  
> > 
> > Add a fallthrough annotation?  
> 
> ... otherwise Coverity and friends will complain later.

Nod.

> 
> >>       default:
> >>           if (sccb_max_addr < sccb_boundary) {
> >>               return true;
> >> @@ -72,6 +84,10 @@ static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
> >>   
> >>       if (be16_to_cpu(sccb->h.length) < required_len) {
> >>           sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> >> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) &&
> >> +            sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
> >> +            sccb->h.length = required_len;
> >> +        }
> >>           return false;
> >>       }
> >>       return true;
> >> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
> >>    */
> >>   static inline int get_read_scp_info_data_len(void)
> >>   {
> >> -    return offsetof(ReadInfo, entries);
> >> +    return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
> >> +           offsetof(ReadInfo, entries) :
> >> +           SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
> >>   }
> >>   
> >>   /* Provide information about the configuration, CPUs and storage */
> >> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
> >>       CPUEntry *entries_start = (void *)sccb + data_len;
> >>   
> >>       if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
> >> +        warn_report("insufficient sccb size to store read scp info response");  
> > 
> > Hm, this warning is triggered by a guest action, isn't it? Not sure how
> > helpful it is.  
> 
> I think this should be qemu_log_mask(LOG_GUEST_ERROR, ...) instead?

Yes, that sounds better.

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Collin Walling]

On 6/24/20 8:55 AM, Cornelia Huck wrote:
> On Wed, 24 Jun 2020 14:40:58 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 24/06/2020 14.36, Cornelia Huck wrote:
>>> On Thu, 18 Jun 2020 18:22:56 -0400
>>> Collin Walling <walling@linux.ibm.com> wrote:
>>>   
>>>> As more features and facilities are added to the Read SCP Info (RSCPI)
>>>> response, more space is required to store them. The space used to store
>>>> these new features intrudes on the space originally used to store CPU
>>>> entries. This means as more features and facilities are added to the
>>>> RSCPI response, less space can be used to store CPU entries.
>>>>
>>>> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
>>>> the RSCPI command and determine if the SCCB is large enough to store a
>>>> complete reponse. If it is not large enough, then the required length
>>>> will be set in the SCCB header.
>>>>
>>>> The caller of the SCLP command is responsible for creating a
>>>> large-enough SCCB to store a complete response. Proper checking should
>>>> be in place, and the caller should execute the command once-more with
>>>> the large-enough SCCB.
>>>>
>>>> This facility also enables an extended SCCB for the Read CPU Info
>>>> (RCPUI) command.
>>>>
>>>> When this facility is enabled, the boundary violation response cannot
>>>> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
>>>>
>>>> In order to tolerate kernels that do not yet have full support for this
>>>> feature, a "fixed" offset to the start of the CPU Entries within the
>>>> Read SCP Info struct is set to allow for the original 248 max entries
>>>> when this feature is disabled.
>>>>
>>>> Additionally, this is introduced as a CPU feature to protect the guest
>>>> from migrating to a machine that does not support storing an extended
>>>> SCCB. This could otherwise hinder the VM from being able to read all
>>>> available CPU entries after migration (such as during re-ipl).
>>>>
>>>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>>>> ---
>>>>   hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
>>>>   include/hw/s390x/sclp.h             |  1 +
>>>>   target/s390x/cpu_features_def.inc.h |  1 +
>>>>   target/s390x/gen-features.c         |  1 +
>>>>   target/s390x/kvm.c                  |  8 ++++++++
>>>>   5 files changed, 31 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>>> index 0dfbe6e5ec..f7c49e339e 100644
>>>> --- a/hw/s390x/sclp.c
>>>> +++ b/hw/s390x/sclp.c
>>>> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
>>>>       uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
>>>>   
>>>>       switch (code & SCLP_CMD_CODE_MASK) {
>>>> +    case SCLP_CMDW_READ_SCP_INFO:
>>>> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
>>>> +    case SCLP_CMDW_READ_CPU_INFO:
>>>> +        /*
>>>> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
>>>> +         * is allowed to exceed the 4k boundary. The respective commands will
>>>> +         * set the length field to the required length if an insufficient
>>>> +         * SCCB length is provided.
>>>> +         */
>>>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
>>>> +            return true;
>>>> +        }  
>>>
>>> Add a fallthrough annotation?  
>>
>> ... otherwise Coverity and friends will complain later.
> 
> Nod.
> 

Something simple like...

/* without this feature, these commands must respect the 4k boundary */

?

>>
>>>>       default:
>>>>           if (sccb_max_addr < sccb_boundary) {
>>>>               return true;
>>>> @@ -72,6 +84,10 @@ static bool sccb_sufficient_len(SCCB *sccb, int num_cpus, int data_len)
>>>>   
>>>>       if (be16_to_cpu(sccb->h.length) < required_len) {
>>>>           sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>>>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) &&
>>>> +            sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
>>>> +            sccb->h.length = required_len;
>>>> +        }
>>>>           return false;
>>>>       }
>>>>       return true;
>>>> @@ -101,7 +117,9 @@ static void prepare_cpu_entries(MachineState *ms, CPUEntry *entry, int *count)
>>>>    */
>>>>   static inline int get_read_scp_info_data_len(void)
>>>>   {
>>>> -    return offsetof(ReadInfo, entries);
>>>> +    return s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB) ?
>>>> +           offsetof(ReadInfo, entries) :
>>>> +           SCLP_READ_SCP_INFO_FIXED_CPU_OFFSET;
>>>>   }
>>>>   
>>>>   /* Provide information about the configuration, CPUs and storage */
>>>> @@ -116,6 +134,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
>>>>       CPUEntry *entries_start = (void *)sccb + data_len;
>>>>   
>>>>       if (!sccb_sufficient_len(sccb, machine->possible_cpus->len, data_len)) {
>>>> +        warn_report("insufficient sccb size to store read scp info response");  
>>>
>>> Hm, this warning is triggered by a guest action, isn't it? Not sure how
>>> helpful it is.  
>>
>> I think this should be qemu_log_mask(LOG_GUEST_ERROR, ...) instead?
> 
> Yes, that sounds better.
> 
> 

Sure, sounds good.

-- 
Regards,
Collin

Stay safe and stay healthy

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Cornelia Huck]

On Wed, 24 Jun 2020 10:49:57 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> On 6/24/20 8:55 AM, Cornelia Huck wrote:
> > On Wed, 24 Jun 2020 14:40:58 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >   
> >> On 24/06/2020 14.36, Cornelia Huck wrote:  
> >>> On Thu, 18 Jun 2020 18:22:56 -0400
> >>> Collin Walling <walling@linux.ibm.com> wrote:
> >>>     
> >>>> As more features and facilities are added to the Read SCP Info (RSCPI)
> >>>> response, more space is required to store them. The space used to store
> >>>> these new features intrudes on the space originally used to store CPU
> >>>> entries. This means as more features and facilities are added to the
> >>>> RSCPI response, less space can be used to store CPU entries.
> >>>>
> >>>> With the Extended-Length SCCB (ELS) facility, a KVM guest can execute
> >>>> the RSCPI command and determine if the SCCB is large enough to store a
> >>>> complete reponse. If it is not large enough, then the required length
> >>>> will be set in the SCCB header.
> >>>>
> >>>> The caller of the SCLP command is responsible for creating a
> >>>> large-enough SCCB to store a complete response. Proper checking should
> >>>> be in place, and the caller should execute the command once-more with
> >>>> the large-enough SCCB.
> >>>>
> >>>> This facility also enables an extended SCCB for the Read CPU Info
> >>>> (RCPUI) command.
> >>>>
> >>>> When this facility is enabled, the boundary violation response cannot
> >>>> be a result from the RSCPI, RSCPI Forced, or RCPUI commands.
> >>>>
> >>>> In order to tolerate kernels that do not yet have full support for this
> >>>> feature, a "fixed" offset to the start of the CPU Entries within the
> >>>> Read SCP Info struct is set to allow for the original 248 max entries
> >>>> when this feature is disabled.
> >>>>
> >>>> Additionally, this is introduced as a CPU feature to protect the guest
> >>>> from migrating to a machine that does not support storing an extended
> >>>> SCCB. This could otherwise hinder the VM from being able to read all
> >>>> available CPU entries after migration (such as during re-ipl).
> >>>>
> >>>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> >>>> ---
> >>>>   hw/s390x/sclp.c                     | 21 ++++++++++++++++++++-
> >>>>   include/hw/s390x/sclp.h             |  1 +
> >>>>   target/s390x/cpu_features_def.inc.h |  1 +
> >>>>   target/s390x/gen-features.c         |  1 +
> >>>>   target/s390x/kvm.c                  |  8 ++++++++
> >>>>   5 files changed, 31 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> >>>> index 0dfbe6e5ec..f7c49e339e 100644
> >>>> --- a/hw/s390x/sclp.c
> >>>> +++ b/hw/s390x/sclp.c
> >>>> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
> >>>>       uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
> >>>>   
> >>>>       switch (code & SCLP_CMD_CODE_MASK) {
> >>>> +    case SCLP_CMDW_READ_SCP_INFO:
> >>>> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
> >>>> +    case SCLP_CMDW_READ_CPU_INFO:
> >>>> +        /*
> >>>> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
> >>>> +         * is allowed to exceed the 4k boundary. The respective commands will
> >>>> +         * set the length field to the required length if an insufficient
> >>>> +         * SCCB length is provided.
> >>>> +         */
> >>>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
> >>>> +            return true;
> >>>> +        }    
> >>>
> >>> Add a fallthrough annotation?    
> >>
> >> ... otherwise Coverity and friends will complain later.  
> > 
> > Nod.
> >   
> 
> Something simple like...
> 
> /* without this feature, these commands must respect the 4k boundary */
> 
> ?

No, I meant something that is parsed by static checkers (/* fallthrough */
seems to be the common marker for that in QEMU). I think what the
fallthrough does is already clear enough to humans.

> 
> >>  
> >>>>       default:
> >>>>           if (sccb_max_addr < sccb_boundary) {
> >>>>               return true;

Re: [PATCH v3 6/8] s390/sclp: add extended-length sccb support for kvm guest

[Thomas Huth]

On 24/06/2020 16.57, Cornelia Huck wrote:
> On Wed, 24 Jun 2020 10:49:57 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> On 6/24/20 8:55 AM, Cornelia Huck wrote:
>>> On Wed, 24 Jun 2020 14:40:58 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>    
>>>> On 24/06/2020 14.36, Cornelia Huck wrote:
>>>>> On Thu, 18 Jun 2020 18:22:56 -0400
>>>>> Collin Walling <walling@linux.ibm.com> wrote:
[...]
>>>>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>>>>> index 0dfbe6e5ec..f7c49e339e 100644
>>>>>> --- a/hw/s390x/sclp.c
>>>>>> +++ b/hw/s390x/sclp.c
>>>>>> @@ -56,6 +56,18 @@ static bool sccb_has_valid_boundary(uint64_t sccb_addr, uint32_t code,
>>>>>>        uint64_t sccb_boundary = (sccb_addr & PAGE_MASK) + PAGE_SIZE;
>>>>>>    
>>>>>>        switch (code & SCLP_CMD_CODE_MASK) {
>>>>>> +    case SCLP_CMDW_READ_SCP_INFO:
>>>>>> +    case SCLP_CMDW_READ_SCP_INFO_FORCED:
>>>>>> +    case SCLP_CMDW_READ_CPU_INFO:
>>>>>> +        /*
>>>>>> +         * An extended-length SCCB is only allowed for Read SCP/CPU Info and
>>>>>> +         * is allowed to exceed the 4k boundary. The respective commands will
>>>>>> +         * set the length field to the required length if an insufficient
>>>>>> +         * SCCB length is provided.
>>>>>> +         */
>>>>>> +        if (s390_has_feat(S390_FEAT_EXTENDED_LENGTH_SCCB)) {
>>>>>> +            return true;
>>>>>> +        }
>>>>>
>>>>> Add a fallthrough annotation?
>>>>
>>>> ... otherwise Coverity and friends will complain later.
>>>
>>> Nod.
>>>    
>>
>> Something simple like...
>>
>> /* without this feature, these commands must respect the 4k boundary */
>>
>> ?
> 
> No, I meant something that is parsed by static checkers (/* fallthrough */
> seems to be the common marker for that in QEMU). I think what the
> fallthrough does is already clear enough to humans.

See also the "-Wimplicit-fallthrough" compiler option ... which we do 
not have enabled for QEMU yet, but maybe will be enabled one day. It can 
e.g. check for "/* fallthrough */" comments.

  Thomas