Re: [PATCH v6 12/18] s390x: protvirt: Move diag 308 data over SIDAD

[David Hildenbrand]

On 04.03.20 12:42, Janosch Frank wrote:
> For protected guests the IPIB is written/read to/from the satellite
> block, so we need those accesses to go through
> s390_cpu_pv_mem_read/write().

Maybe mention SIDAD instead if "satellite block", similar to the other
patch descriptions.

[...]

> -        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))) {
> @@ -136,7 +146,7 @@ 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;

Why that change? Unrelated?

>      case DIAG308_STORE:
>      case DIAG308_PV_STORE:
>          if (diag308_parm_check(env, r1, addr, ra, true)) {
> @@ -147,12 +157,18 @@ out:
>          } 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();
> 

I really do wonder if you should squash in patch #2 and leave it in this
position of the series.

-- 
Thanks,

David / dhildenb