[13/15] s390x: protvirt: Move diag 308 data over SIDAD
diff mbox series

Message ID 20191120114334.2287-14-frankja@linux.ibm.com
State New
Headers show
Series
  • s390x: Protected Virtualization support
Related show

Commit Message

Janosch Frank Nov. 20, 2019, 11:43 a.m. UTC
For protected guests the IPIB is written/read to/from the sattelite
block, so we need to make those accesses virtual to make them go
through KBM mem ops.

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

Comments

Thomas Huth Nov. 28, 2019, 2:40 p.m. UTC | #1
On 20/11/2019 12.43, Janosch Frank wrote:
> For protected guests the IPIB is written/read to/from the sattelite

satellite

> block, so we need to make those accesses virtual to make them go
> through KBM mem ops.

What's KBM ?

> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  target/s390x/diag.c | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
> index db6d79cef3..d96d8bdc6c 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,27 @@ 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_virt_mem_read(cpu, 0, 0, iplb, sizeof(iplb->len));
> +            s390_cpu_virt_mem_handle_exc(cpu, ra);

I'm looking forward to protected virt support in TCG ;-)

> +        }
> +
>          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_virt_mem_read(cpu, 0, 0, iplb, be32_to_cpu(iplb->len));
> +            s390_cpu_virt_mem_handle_exc(cpu, ra);
> +        }
> +
>  
>          if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) &&
>              !(iplb_valid_se(iplb) && s390_ipl_pv_check_comp(iplb) >= 0)) {
> @@ -149,7 +163,13 @@ out:
>              iplb = s390_ipl_get_iplb();
>          }
>          if (iplb) {
> -            cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
> +            if (!env->pv) {
> +                cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
> +            } else {
> +                s390_cpu_virt_mem_write(cpu, 0, 0, iplb,
> +                                        be32_to_cpu(iplb->len));
> +                s390_cpu_virt_mem_handle_exc(cpu, ra);
> +            }
>              env->regs[r1 + 1] = DIAG_308_RC_OK;
>          } else {
>              env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
> 

I wonder whether we maybe want to have some wrapper functions for these
reads and writes, something like:

void s390_cpu_physical_memory_write(...)
{
    if (!env->pv) {
        cpu_physical_memory_write(...);
    } else {
        s390_cpu_virt_mem_write(...);
    }
}

?

 Thomas
Janosch Frank Nov. 28, 2019, 4:08 p.m. UTC | #2
On 11/28/19 3:40 PM, Thomas Huth wrote:
> On 20/11/2019 12.43, Janosch Frank wrote:
>> For protected guests the IPIB is written/read to/from the sattelite
> 
> satellite
> 
>> block, so we need to make those accesses virtual to make them go
>> through KBM mem ops.
> 
> What's KBM ?

A totally new hypervisor that's much faster than KVM because b comes
before v in the alphabet.

> 
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  target/s390x/diag.c | 26 +++++++++++++++++++++++---
>>  1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index db6d79cef3..d96d8bdc6c 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,27 @@ 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_virt_mem_read(cpu, 0, 0, iplb, sizeof(iplb->len));
>> +            s390_cpu_virt_mem_handle_exc(cpu, ra);
> 
> I'm looking forward to protected virt support in TCG ;-)

Who doesn't?

> 
>> +        }
>> +
>>          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_virt_mem_read(cpu, 0, 0, iplb, be32_to_cpu(iplb->len));
>> +            s390_cpu_virt_mem_handle_exc(cpu, ra);
>> +        }
>> +
>>  
>>          if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) &&
>>              !(iplb_valid_se(iplb) && s390_ipl_pv_check_comp(iplb) >= 0)) {
>> @@ -149,7 +163,13 @@ out:
>>              iplb = s390_ipl_get_iplb();
>>          }
>>          if (iplb) {
>> -            cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
>> +            if (!env->pv) {
>> +                cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
>> +            } else {
>> +                s390_cpu_virt_mem_write(cpu, 0, 0, iplb,
>> +                                        be32_to_cpu(iplb->len));
>> +                s390_cpu_virt_mem_handle_exc(cpu, ra);
>> +            }
>>              env->regs[r1 + 1] = DIAG_308_RC_OK;
>>          } else {
>>              env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
>>
> 
> I wonder whether we maybe want to have some wrapper functions for these
> reads and writes, something like:

diag308 is currently the only instruction where we would need it.

sclp needs a new handler, because we can skip lots of checks if pv is
enabled.
IO instructions are already using logical addressing

> 
> void s390_cpu_physical_memory_write(...)
> {
>     if (!env->pv) {
>         cpu_physical_memory_write(...);
>     } else {
>         s390_cpu_virt_mem_write(...);
>     }
> }
> 
> ?
> 
>  Thomas
>
David Hildenbrand Nov. 28, 2019, 4:14 p.m. UTC | #3
On 28.11.19 17:08, Janosch Frank wrote:
> On 11/28/19 3:40 PM, Thomas Huth wrote:
>> On 20/11/2019 12.43, Janosch Frank wrote:
>>> For protected guests the IPIB is written/read to/from the sattelite
>>
>> satellite
>>
>>> block, so we need to make those accesses virtual to make them go
>>> through KBM mem ops.
>>
>> What's KBM ?
> 
> A totally new hypervisor that's much faster than KVM because b comes
> before v in the alphabet.


Sounds more lime IBM is slowly taking over KVM :D

Patch
diff mbox series

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index db6d79cef3..d96d8bdc6c 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,27 @@  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_virt_mem_read(cpu, 0, 0, iplb, sizeof(iplb->len));
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
+        }
+
         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_virt_mem_read(cpu, 0, 0, iplb, be32_to_cpu(iplb->len));
+            s390_cpu_virt_mem_handle_exc(cpu, ra);
+        }
+
 
         if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) &&
             !(iplb_valid_se(iplb) && s390_ipl_pv_check_comp(iplb) >= 0)) {
@@ -149,7 +163,13 @@  out:
             iplb = s390_ipl_get_iplb();
         }
         if (iplb) {
-            cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
+            if (!env->pv) {
+                cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
+            } else {
+                s390_cpu_virt_mem_write(cpu, 0, 0, iplb,
+                                        be32_to_cpu(iplb->len));
+                s390_cpu_virt_mem_handle_exc(cpu, ra);
+            }
             env->regs[r1 + 1] = DIAG_308_RC_OK;
         } else {
             env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;