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();
>
>