| Message ID | 20200318143047.2335-10-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On Wed, 18 Mar 2020 10:30:40 -0400 Janosch Frank <frankja@linux.ibm.com> wrote: > SCLP for a protected guest is done over the SIDAD, so we need to use > the s390_cpu_pv_mem_* functions to access the SIDAD instead of guest > memory when reading/writing SCBs. > > To not confuse the sclp emulation, we set 0x4000 as the SCCB address, > since the function that injects the sclp external interrupt would > reject a zero sccb address. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > hw/s390x/sclp.c | 65 ++++++++++++++++++++++++++++++++++------- > include/hw/s390x/sclp.h | 2 ++ > target/s390x/kvm.c | 25 ++++++++++++---- > 3 files changed, 76 insertions(+), 16 deletions(-) > > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > index af0bfbc2eca74767..6486890fecea4b3c 100644 > --- a/hw/s390x/sclp.c > +++ b/hw/s390x/sclp.c > @@ -33,6 +33,22 @@ static inline SCLPDevice *get_sclp_device(void) > return sclp; > } > > +static inline bool sclp_command_code_valid(uint32_t code) > +{ > + 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: > + case SCLP_CMDW_CONFIGURE_IOA: > + case SCLP_CMDW_DECONFIGURE_IOA: > + case SCLP_CMD_READ_EVENT_DATA: > + case SCLP_CMD_WRITE_EVENT_DATA: > + case SCLP_CMD_WRITE_EVENT_MASK: > + return true; > + } > + return false; > +} > + > static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count) > { > MachineState *ms = MACHINE(qdev_get_machine()); > @@ -193,6 +209,43 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code) > } > } > > +/* > + * We only need the address to have something valid for the > + * service_interrupt call. > + */ > +#define SCLP_PV_DUMMY_ADDR 0x4000 > +int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, > + uint32_t code) > +{ > + SCLPDevice *sclp = get_sclp_device(); > + SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); > + SCCB work_sccb; > + hwaddr sccb_len = sizeof(SCCB); > + > + /* > + * Only a very limited number of calls is permitted by the > + * Ultravisor and we support all of them, so we don't check for This is no longer true, no? > + * them. All other specification exceptions are also interpreted > + * by the Ultravisor and hence never cause an exit we need to > + * handle. > + * > + * Setting the CC is also done by the Ultravisor. > + */ > + s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len); > + > + if (!sclp_command_code_valid(code)) { > + work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); > + goto out_write; > + } > + > + sclp_c->execute(sclp, &work_sccb, code); > +out_write: > + s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, > + be16_to_cpu(work_sccb.h.length)); > + sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR); > + return 0; > +} > + > int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) > { > SCLPDevice *sclp = get_sclp_device();
On 3/19/20 1:09 PM, Cornelia Huck wrote: > On Wed, 18 Mar 2020 10:30:40 -0400 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> SCLP for a protected guest is done over the SIDAD, so we need to use >> the s390_cpu_pv_mem_* functions to access the SIDAD instead of guest >> memory when reading/writing SCBs. >> >> To not confuse the sclp emulation, we set 0x4000 as the SCCB address, >> since the function that injects the sclp external interrupt would >> reject a zero sccb address. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> >> --- >> hw/s390x/sclp.c | 65 ++++++++++++++++++++++++++++++++++------- >> include/hw/s390x/sclp.h | 2 ++ >> target/s390x/kvm.c | 25 ++++++++++++---- >> 3 files changed, 76 insertions(+), 16 deletions(-) >> >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c >> index af0bfbc2eca74767..6486890fecea4b3c 100644 >> --- a/hw/s390x/sclp.c >> +++ b/hw/s390x/sclp.c >> @@ -33,6 +33,22 @@ static inline SCLPDevice *get_sclp_device(void) >> return sclp; >> } >> >> +static inline bool sclp_command_code_valid(uint32_t code) >> +{ >> + 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: >> + case SCLP_CMDW_CONFIGURE_IOA: >> + case SCLP_CMDW_DECONFIGURE_IOA: >> + case SCLP_CMD_READ_EVENT_DATA: >> + case SCLP_CMD_WRITE_EVENT_DATA: >> + case SCLP_CMD_WRITE_EVENT_MASK: >> + return true; >> + } >> + return false; >> +} >> + >> static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count) >> { >> MachineState *ms = MACHINE(qdev_get_machine()); >> @@ -193,6 +209,43 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code) >> } >> } >> >> +/* >> + * We only need the address to have something valid for the >> + * service_interrupt call. >> + */ >> +#define SCLP_PV_DUMMY_ADDR 0x4000 >> +int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, >> + uint32_t code) >> +{ >> + SCLPDevice *sclp = get_sclp_device(); >> + SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); >> + SCCB work_sccb; >> + hwaddr sccb_len = sizeof(SCCB); >> + >> + /* >> + * Only a very limited number of calls is permitted by the >> + * Ultravisor and we support all of them, so we don't check for > > This is no longer true, no? Right, I just removed the comment. > >> + * them. All other specification exceptions are also interpreted >> + * by the Ultravisor and hence never cause an exit we need to >> + * handle. >> + * >> + * Setting the CC is also done by the Ultravisor. >> + */ >> + s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len); >> + >> + if (!sclp_command_code_valid(code)) { >> + work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); >> + goto out_write; >> + } >> + >> + sclp_c->execute(sclp, &work_sccb, code); >> +out_write: >> + s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, >> + be16_to_cpu(work_sccb.h.length)); >> + sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR); >> + return 0; >> +} >> + >> int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) >> { >> SCLPDevice *sclp = get_sclp_device(); >
On Thu, 19 Mar 2020 13:46:46 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 3/19/20 1:09 PM, Cornelia Huck wrote: > > On Wed, 18 Mar 2020 10:30:40 -0400 > > Janosch Frank <frankja@linux.ibm.com> wrote: > > > >> SCLP for a protected guest is done over the SIDAD, so we need to use > >> the s390_cpu_pv_mem_* functions to access the SIDAD instead of guest > >> memory when reading/writing SCBs. > >> > >> To not confuse the sclp emulation, we set 0x4000 as the SCCB address, > >> since the function that injects the sclp external interrupt would > >> reject a zero sccb address. > >> > >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >> Reviewed-by: David Hildenbrand <david@redhat.com> > >> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > >> --- > >> hw/s390x/sclp.c | 65 ++++++++++++++++++++++++++++++++++------- > >> include/hw/s390x/sclp.h | 2 ++ > >> target/s390x/kvm.c | 25 ++++++++++++---- > >> 3 files changed, 76 insertions(+), 16 deletions(-) > >> > >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c > >> index af0bfbc2eca74767..6486890fecea4b3c 100644 > >> --- a/hw/s390x/sclp.c > >> +++ b/hw/s390x/sclp.c > >> @@ -33,6 +33,22 @@ static inline SCLPDevice *get_sclp_device(void) > >> return sclp; > >> } > >> > >> +static inline bool sclp_command_code_valid(uint32_t code) > >> +{ > >> + 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: > >> + case SCLP_CMDW_CONFIGURE_IOA: > >> + case SCLP_CMDW_DECONFIGURE_IOA: > >> + case SCLP_CMD_READ_EVENT_DATA: > >> + case SCLP_CMD_WRITE_EVENT_DATA: > >> + case SCLP_CMD_WRITE_EVENT_MASK: > >> + return true; > >> + } > >> + return false; > >> +} > >> + > >> static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count) > >> { > >> MachineState *ms = MACHINE(qdev_get_machine()); > >> @@ -193,6 +209,43 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code) > >> } > >> } > >> > >> +/* > >> + * We only need the address to have something valid for the > >> + * service_interrupt call. > >> + */ > >> +#define SCLP_PV_DUMMY_ADDR 0x4000 > >> +int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, > >> + uint32_t code) > >> +{ > >> + SCLPDevice *sclp = get_sclp_device(); > >> + SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); > >> + SCCB work_sccb; > >> + hwaddr sccb_len = sizeof(SCCB); > >> + > >> + /* > >> + * Only a very limited number of calls is permitted by the > >> + * Ultravisor and we support all of them, so we don't check for > > > > This is no longer true, no? > > Right, I just removed the comment. With that, Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c index af0bfbc2eca74767..6486890fecea4b3c 100644 --- a/hw/s390x/sclp.c +++ b/hw/s390x/sclp.c @@ -33,6 +33,22 @@ static inline SCLPDevice *get_sclp_device(void) return sclp; } +static inline bool sclp_command_code_valid(uint32_t code) +{ + 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: + case SCLP_CMDW_CONFIGURE_IOA: + case SCLP_CMDW_DECONFIGURE_IOA: + case SCLP_CMD_READ_EVENT_DATA: + case SCLP_CMD_WRITE_EVENT_DATA: + case SCLP_CMD_WRITE_EVENT_MASK: + return true; + } + return false; +} + static void prepare_cpu_entries(SCLPDevice *sclp, CPUEntry *entry, int *count) { MachineState *ms = MACHINE(qdev_get_machine()); @@ -193,6 +209,43 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code) } } +/* + * We only need the address to have something valid for the + * service_interrupt call. + */ +#define SCLP_PV_DUMMY_ADDR 0x4000 +int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, + uint32_t code) +{ + SCLPDevice *sclp = get_sclp_device(); + SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp); + SCCB work_sccb; + hwaddr sccb_len = sizeof(SCCB); + + /* + * Only a very limited number of calls is permitted by the + * Ultravisor and we support all of them, so we don't check for + * them. All other specification exceptions are also interpreted + * by the Ultravisor and hence never cause an exit we need to + * handle. + * + * Setting the CC is also done by the Ultravisor. + */ + s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len); + + if (!sclp_command_code_valid(code)) { + work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); + goto out_write; + } + + sclp_c->execute(sclp, &work_sccb, code); +out_write: + s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb, + be16_to_cpu(work_sccb.h.length)); + sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR); + return 0; +} + int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) { SCLPDevice *sclp = get_sclp_device(); @@ -225,17 +278,7 @@ int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code) return -PGM_SPECIFICATION; } - 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: - case SCLP_CMDW_CONFIGURE_IOA: - case SCLP_CMDW_DECONFIGURE_IOA: - case SCLP_CMD_READ_EVENT_DATA: - case SCLP_CMD_WRITE_EVENT_DATA: - case SCLP_CMD_WRITE_EVENT_MASK: - break; - default: + if (!sclp_command_code_valid(code)) { work_sccb.h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND); goto out_write; } diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h index cd7b24359f28deb4..822eff4396ff5dde 100644 --- a/include/hw/s390x/sclp.h +++ b/include/hw/s390x/sclp.h @@ -217,5 +217,7 @@ void s390_sclp_init(void); void sclp_service_interrupt(uint32_t sccb); void raise_irq_cpu_hotplug(void); int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code); +int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb, + uint32_t code); #endif diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 462a1d70ee78104c..6b7819d2c51a111c 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -1233,12 +1233,27 @@ static void kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run, sccb = env->regs[ipbh0 & 0xf]; code = env->regs[(ipbh0 & 0xf0) >> 4]; - r = sclp_service_call(env, sccb, code); - if (r < 0) { - kvm_s390_program_interrupt(cpu, -r); - return; + switch (run->s390_sieic.icptcode) { + case ICPT_PV_INSTR_NOTIFICATION: + g_assert(s390_is_pv()); + /* The notification intercepts are currently handled by KVM */ + error_report("unexpected SCLP PV notification"); + exit(1); + break; + case ICPT_PV_INSTR: + g_assert(s390_is_pv()); + sclp_service_call_protected(env, sccb, code); + /* Setting the CC is done by the Ultravisor. */ + break; + case ICPT_INSTRUCTION: + g_assert(!s390_is_pv()); + r = sclp_service_call(env, sccb, code); + if (r < 0) { + kvm_s390_program_interrupt(cpu, -r); + return; + } + setcc(cpu, r); } - setcc(cpu, r); } static int handle_b2(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)