diff mbox series

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

Message ID 20200304114231.23493-13-frankja@linux.ibm.com
State New
Headers show
Series s390x: Protected Virtualization support | expand

Commit Message

Janosch Frank March 4, 2020, 11:42 a.m. UTC
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().

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 target/s390x/diag.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

Comments

David Hildenbrand March 4, 2020, 5:54 p.m. UTC | #1
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.
diff mbox series

Patch

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index a6cd1ea260..444c88ef15 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -87,6 +87,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,13 +119,22 @@  void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
             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))) {
@@ -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;
     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();