| Message ID | 20200214151636.8764-14-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On Fri, 14 Feb 2020 10:16:32 -0500 Janosch Frank <frankja@linux.ibm.com> wrote: > For protected guests the IPIB is written/read to/from the satellite > block, so we need to make those accesses virtual to make them go > through KVM mem ops. Confused. What does 'make those accesses virtual' mean? > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > target/s390x/diag.c | 32 +++++++++++++++++++++++++------- > 1 file changed, 25 insertions(+), 7 deletions(-) > > diff --git a/target/s390x/diag.c b/target/s390x/diag.c > index 6aaeef6029..59ae122e82 100644 > --- a/target/s390x/diag.c > +++ b/target/s390x/diag.c > @@ -88,6 +88,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr, > void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) > { > CPUState *cs = env_cpu(env); > + S390CPU *cpu = S390_CPU(cs); > uint64_t addr = env->regs[r1]; > uint64_t subcode = env->regs[r3]; > IplParameterBlock *iplb; > @@ -118,14 +119,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) > if (diag308_parm_check(env, r1, addr, ra, false)) { > return; > } > + Whitespace. > iplb = g_new0(IplParameterBlock, 1); > - cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); > + if (!env->pv) { > + cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); > + } else { > + s390_cpu_pv_mem_read(cpu, 0, iplb, sizeof(iplb->len)); > + } > + > if (!iplb_valid_len(iplb)) { > env->regs[r1 + 1] = DIAG_308_RC_INVALID; > goto out; > } > > - cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); > + if (!env->pv) { > + cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); > + } else { > + s390_cpu_pv_mem_read(cpu, 0, iplb, be32_to_cpu(iplb->len)); > + } > > if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) && > !(iplb_valid_pv(iplb) && s390_ipl_pv_check_components(iplb) >= 0)) { > @@ -137,23 +148,30 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) > env->regs[r1 + 1] = DIAG_308_RC_OK; > out: > g_free(iplb); > - return; > + break; > case DIAG308_STORE: > case DIAG308_PV_STORE: > if (diag308_parm_check(env, r1, addr, ra, true)) { > return; > } > + Whitespace. > if (subcode == DIAG308_PV_STORE) { > iplb = s390_ipl_get_iplb_secure(); > } else { > iplb = s390_ipl_get_iplb(); > } > - if (iplb) { > - cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); > - env->regs[r1 + 1] = DIAG_308_RC_OK; > - } else { > + if (!iplb) { > env->regs[r1 + 1] = DIAG_308_RC_NO_CONF; > + return; > } > + > + if (!env->pv) { > + cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); > + } else { > + s390_cpu_pv_mem_write(cpu, 0, iplb, be32_to_cpu(iplb->len)); > + } > + > + env->regs[r1 + 1] = DIAG_308_RC_OK; > break; > case DIAG308_PV_START: > iplb = s390_ipl_get_iplb_secure();
On 2/20/20 12:00 PM, Cornelia Huck wrote: > On Fri, 14 Feb 2020 10:16:32 -0500 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> For protected guests the IPIB is written/read to/from the satellite >> block, so we need to make those accesses virtual to make them go >> through KVM mem ops. ..we need those accesses to go through s390_cpu_pv_mem_read/write() so we use the memop IOCTL to access the sattelite block. > > Confused. What does 'make those accesses virtual' mean? I need to update the patch description. Virtual memory access was needed when I overloaded the normal memops to access the SIDA. Real access would have accessed guest memory directly which results in a guest crash for protected VMs. > >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> target/s390x/diag.c | 32 +++++++++++++++++++++++++------- >> 1 file changed, 25 insertions(+), 7 deletions(-) >> >> diff --git a/target/s390x/diag.c b/target/s390x/diag.c >> index 6aaeef6029..59ae122e82 100644 >> --- a/target/s390x/diag.c >> +++ b/target/s390x/diag.c >> @@ -88,6 +88,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr, >> void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) >> { >> CPUState *cs = env_cpu(env); >> + S390CPU *cpu = S390_CPU(cs); >> uint64_t addr = env->regs[r1]; >> uint64_t subcode = env->regs[r3]; >> IplParameterBlock *iplb; >> @@ -118,14 +119,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) >> if (diag308_parm_check(env, r1, addr, ra, false)) { >> return; >> } >> + > > Whitespace. Ack. > >> iplb = g_new0(IplParameterBlock, 1); >> - cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); >> + if (!env->pv) { >> + cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); >> + } else { >> + s390_cpu_pv_mem_read(cpu, 0, iplb, sizeof(iplb->len)); >> + } >> + >> if (!iplb_valid_len(iplb)) { >> env->regs[r1 + 1] = DIAG_308_RC_INVALID; >> goto out; >> } >> >> - cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); >> + if (!env->pv) { >> + cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); >> + } else { >> + s390_cpu_pv_mem_read(cpu, 0, iplb, be32_to_cpu(iplb->len)); >> + } >> >> if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) && >> !(iplb_valid_pv(iplb) && s390_ipl_pv_check_components(iplb) >= 0)) { >> @@ -137,23 +148,30 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) >> env->regs[r1 + 1] = DIAG_308_RC_OK; >> out: >> g_free(iplb); >> - return; >> + break; >> case DIAG308_STORE: >> case DIAG308_PV_STORE: >> if (diag308_parm_check(env, r1, addr, ra, true)) { >> return; >> } >> + > > Whitespace. Ack. > >> if (subcode == DIAG308_PV_STORE) { >> iplb = s390_ipl_get_iplb_secure(); >> } else { >> iplb = s390_ipl_get_iplb(); >> } >> - if (iplb) { >> - cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); >> - env->regs[r1 + 1] = DIAG_308_RC_OK; >> - } else { >> + if (!iplb) { >> env->regs[r1 + 1] = DIAG_308_RC_NO_CONF; >> + return; >> } >> + >> + if (!env->pv) { >> + cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); >> + } else { >> + s390_cpu_pv_mem_write(cpu, 0, iplb, be32_to_cpu(iplb->len)); >> + } >> + >> + env->regs[r1 + 1] = DIAG_308_RC_OK; >> break; >> case DIAG308_PV_START: >> iplb = s390_ipl_get_iplb_secure(); > >
diff --git a/target/s390x/diag.c b/target/s390x/diag.c index 6aaeef6029..59ae122e82 100644 --- a/target/s390x/diag.c +++ b/target/s390x/diag.c @@ -88,6 +88,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr, void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) { CPUState *cs = env_cpu(env); + S390CPU *cpu = S390_CPU(cs); uint64_t addr = env->regs[r1]; uint64_t subcode = env->regs[r3]; IplParameterBlock *iplb; @@ -118,14 +119,24 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) if (diag308_parm_check(env, r1, addr, ra, false)) { return; } + iplb = g_new0(IplParameterBlock, 1); - cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); + if (!env->pv) { + cpu_physical_memory_read(addr, iplb, sizeof(iplb->len)); + } else { + s390_cpu_pv_mem_read(cpu, 0, iplb, sizeof(iplb->len)); + } + if (!iplb_valid_len(iplb)) { env->regs[r1 + 1] = DIAG_308_RC_INVALID; goto out; } - cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); + if (!env->pv) { + cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len)); + } else { + s390_cpu_pv_mem_read(cpu, 0, iplb, be32_to_cpu(iplb->len)); + } if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) && !(iplb_valid_pv(iplb) && s390_ipl_pv_check_components(iplb) >= 0)) { @@ -137,23 +148,30 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra) env->regs[r1 + 1] = DIAG_308_RC_OK; out: g_free(iplb); - return; + break; case DIAG308_STORE: case DIAG308_PV_STORE: if (diag308_parm_check(env, r1, addr, ra, true)) { return; } + if (subcode == DIAG308_PV_STORE) { iplb = s390_ipl_get_iplb_secure(); } else { iplb = s390_ipl_get_iplb(); } - if (iplb) { - cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); - env->regs[r1 + 1] = DIAG_308_RC_OK; - } else { + if (!iplb) { env->regs[r1 + 1] = DIAG_308_RC_NO_CONF; + return; } + + if (!env->pv) { + cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len)); + } else { + s390_cpu_pv_mem_write(cpu, 0, iplb, be32_to_cpu(iplb->len)); + } + + env->regs[r1 + 1] = DIAG_308_RC_OK; break; case DIAG308_PV_START: iplb = s390_ipl_get_iplb_secure();
For protected guests the IPIB is written/read to/from the satellite block, so we need to make those accesses virtual to make them go through KVM mem ops. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- target/s390x/diag.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)