[PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD

Janosch Frank posted 15 patches 5 years, 11 months ago
Maintainers: Halil Pasic <pasic@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, Richard Henderson <rth@twiddle.net>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Janosch Frank 5 years, 11 months ago
For protected guests, we need to put the STSI emulation results into
the SIDA, so SIE will write them into the guest at the next entry.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/kvm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index cdcd538b4f7fb318..8085d5030e7c6454 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -50,6 +50,7 @@
 #include "exec/memattrs.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 #include "hw/s390x/s390-virtio-hcall.h"
+#include "hw/s390x/pv.h"
 
 #ifndef DEBUG_KVM
 #define DEBUG_KVM  0
@@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
     SysIB_322 sysib;
     int del;
 
-    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
+    if (s390_is_pv()) {
+        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
+    } else if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
         return;
     }
     /* Shift the stack of Extended Names to prepare for our own data */
@@ -1840,7 +1843,11 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
     /* Insert UUID */
     memcpy(sysib.vm[0].uuid, &qemu_uuid, sizeof(sysib.vm[0].uuid));
 
-    s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
+    if (s390_is_pv()) {
+        s390_cpu_pv_mem_write(cpu, 0, &sysib, sizeof(sysib));
+    } else {
+        s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
+    }
 }
 
 static int handle_stsi(S390CPU *cpu)
-- 
2.25.1


Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Claudio Imbrenda 5 years, 11 months ago
On Wed, 11 Mar 2020 09:21:43 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> For protected guests, we need to put the STSI emulation results into
> the SIDA, so SIE will write them into the guest at the next entry.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index cdcd538b4f7fb318..8085d5030e7c6454 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -50,6 +50,7 @@
>  #include "exec/memattrs.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
>  #include "hw/s390x/s390-virtio-hcall.h"
> +#include "hw/s390x/pv.h"
>  
>  #ifndef DEBUG_KVM
>  #define DEBUG_KVM  0
> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu,
> __u64 addr, uint8_t ar) SysIB_322 sysib;
>      int del;
>  
> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib,
> sizeof(sysib))) {
> +    if (s390_is_pv()) {
> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
> +    } else if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib,
> sizeof(sysib))) { return;
>      }
>      /* Shift the stack of Extended Names to prepare for our own data
> */ @@ -1840,7 +1843,11 @@ static void insert_stsi_3_2_2(S390CPU *cpu,
> __u64 addr, uint8_t ar) /* Insert UUID */
>      memcpy(sysib.vm[0].uuid, &qemu_uuid, sizeof(sysib.vm[0].uuid));
>  
> -    s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
> +    if (s390_is_pv()) {
> +        s390_cpu_pv_mem_write(cpu, 0, &sysib, sizeof(sysib));
> +    } else {
> +        s390_cpu_virt_mem_write(cpu, addr, ar, &sysib,
> sizeof(sysib));
> +    }
>  }
>  
>  static int handle_stsi(S390CPU *cpu)

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>


Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Christian Borntraeger 5 years, 11 months ago

On 11.03.20 14:21, Janosch Frank wrote:
> For protected guests, we need to put the STSI emulation results into
> the SIDA, so SIE will write them into the guest at the next entry.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/kvm.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index cdcd538b4f7fb318..8085d5030e7c6454 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -50,6 +50,7 @@
>  #include "exec/memattrs.h"
>  #include "hw/s390x/s390-virtio-ccw.h"
>  #include "hw/s390x/s390-virtio-hcall.h"
> +#include "hw/s390x/pv.h"
>  
>  #ifndef DEBUG_KVM
>  #define DEBUG_KVM  0
> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>      SysIB_322 sysib;
>      int del;
>  
> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
> +    if (s390_is_pv()) {
> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));

Not strictly necessary, but do we also want to do an early exit if the pv case fails?




Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Janosch Frank 5 years, 11 months ago
On 3/12/20 11:42 AM, Christian Borntraeger wrote:
> 
> 
> On 11.03.20 14:21, Janosch Frank wrote:
>> For protected guests, we need to put the STSI emulation results into
>> the SIDA, so SIE will write them into the guest at the next entry.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
>> ---
>>  target/s390x/kvm.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index cdcd538b4f7fb318..8085d5030e7c6454 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -50,6 +50,7 @@
>>  #include "exec/memattrs.h"
>>  #include "hw/s390x/s390-virtio-ccw.h"
>>  #include "hw/s390x/s390-virtio-hcall.h"
>> +#include "hw/s390x/pv.h"
>>  
>>  #ifndef DEBUG_KVM
>>  #define DEBUG_KVM  0
>> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>>      SysIB_322 sysib;
>>      int del;
>>  
>> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
>> +    if (s390_is_pv()) {
>> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
> 
> Not strictly necessary, but do we also want to do an early exit if the pv case fails?
> 

I'd rather do an early exit for the SIDA read/write ioctl itself

Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Cornelia Huck 5 years, 10 months ago
On Thu, 12 Mar 2020 12:20:25 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 3/12/20 11:42 AM, Christian Borntraeger wrote:
> > 
> > 
> > On 11.03.20 14:21, Janosch Frank wrote:  
> >> For protected guests, we need to put the STSI emulation results into
> >> the SIDA, so SIE will write them into the guest at the next entry.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> Reviewed-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>  target/s390x/kvm.c | 11 +++++++++--
> >>  1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> >> index cdcd538b4f7fb318..8085d5030e7c6454 100644
> >> --- a/target/s390x/kvm.c
> >> +++ b/target/s390x/kvm.c
> >> @@ -50,6 +50,7 @@
> >>  #include "exec/memattrs.h"
> >>  #include "hw/s390x/s390-virtio-ccw.h"
> >>  #include "hw/s390x/s390-virtio-hcall.h"
> >> +#include "hw/s390x/pv.h"
> >>  
> >>  #ifndef DEBUG_KVM
> >>  #define DEBUG_KVM  0
> >> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
> >>      SysIB_322 sysib;
> >>      int del;
> >>  
> >> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
> >> +    if (s390_is_pv()) {
> >> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));  
> > 
> > Not strictly necessary, but do we also want to do an early exit if the pv case fails?
> >   
> 
> I'd rather do an early exit for the SIDA read/write ioctl itself

Early exit in what respect? Abort?

If not, checking the return code here and returning looks like
something we want.
Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Janosch Frank 5 years, 10 months ago
On 3/17/20 11:28 AM, Cornelia Huck wrote:
> On Thu, 12 Mar 2020 12:20:25 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> On 3/12/20 11:42 AM, Christian Borntraeger wrote:
>>>
>>>
>>> On 11.03.20 14:21, Janosch Frank wrote:  
>>>> For protected guests, we need to put the STSI emulation results into
>>>> the SIDA, so SIE will write them into the guest at the next entry.
>>>>
>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>>  target/s390x/kvm.c | 11 +++++++++--
>>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>>> index cdcd538b4f7fb318..8085d5030e7c6454 100644
>>>> --- a/target/s390x/kvm.c
>>>> +++ b/target/s390x/kvm.c
>>>> @@ -50,6 +50,7 @@
>>>>  #include "exec/memattrs.h"
>>>>  #include "hw/s390x/s390-virtio-ccw.h"
>>>>  #include "hw/s390x/s390-virtio-hcall.h"
>>>> +#include "hw/s390x/pv.h"
>>>>  
>>>>  #ifndef DEBUG_KVM
>>>>  #define DEBUG_KVM  0
>>>> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
>>>>      SysIB_322 sysib;
>>>>      int del;
>>>>  
>>>> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
>>>> +    if (s390_is_pv()) {
>>>> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));  
>>>
>>> Not strictly necessary, but do we also want to do an early exit if the pv case fails?
>>>   
>>
>> I'd rather do an early exit for the SIDA read/write ioctl itself
> 
> Early exit in what respect? Abort?

Yes, abort
If a write fails we most likely will not succeed on the continuation
check and if a read fails we will error out somewhere in qemu anyway

> 
> If not, checking the return code here and returning looks like
> something we want.
> 


Re: [PATCH v9 07/15] s390x: protvirt: Move STSI data over SIDAD
Posted by Cornelia Huck 5 years, 10 months ago
On Tue, 17 Mar 2020 11:32:03 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 3/17/20 11:28 AM, Cornelia Huck wrote:
> > On Thu, 12 Mar 2020 12:20:25 +0100
> > Janosch Frank <frankja@linux.ibm.com> wrote:
> >   
> >> On 3/12/20 11:42 AM, Christian Borntraeger wrote:  
> >>>
> >>>
> >>> On 11.03.20 14:21, Janosch Frank wrote:    
> >>>> For protected guests, we need to put the STSI emulation results into
> >>>> the SIDA, so SIE will write them into the guest at the next entry.
> >>>>
> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>>> Reviewed-by: David Hildenbrand <david@redhat.com>
> >>>> ---
> >>>>  target/s390x/kvm.c | 11 +++++++++--
> >>>>  1 file changed, 9 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> >>>> index cdcd538b4f7fb318..8085d5030e7c6454 100644
> >>>> --- a/target/s390x/kvm.c
> >>>> +++ b/target/s390x/kvm.c
> >>>> @@ -50,6 +50,7 @@
> >>>>  #include "exec/memattrs.h"
> >>>>  #include "hw/s390x/s390-virtio-ccw.h"
> >>>>  #include "hw/s390x/s390-virtio-hcall.h"
> >>>> +#include "hw/s390x/pv.h"
> >>>>  
> >>>>  #ifndef DEBUG_KVM
> >>>>  #define DEBUG_KVM  0
> >>>> @@ -1800,7 +1801,9 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
> >>>>      SysIB_322 sysib;
> >>>>      int del;
> >>>>  
> >>>> -    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
> >>>> +    if (s390_is_pv()) {
> >>>> +        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));    
> >>>
> >>> Not strictly necessary, but do we also want to do an early exit if the pv case fails?
> >>>     
> >>
> >> I'd rather do an early exit for the SIDA read/write ioctl itself  
> > 
> > Early exit in what respect? Abort?  
> 
> Yes, abort
> If a write fails we most likely will not succeed on the continuation
> check and if a read fails we will error out somewhere in qemu anyway

Ok, so this will go into the previous patch, I guess?