| Message ID | 20200618222258.23287-5-walling@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390: Extended-Length SCCB & DIAGNOSE 0x318 | expand |
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, >
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>
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,