[PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly

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 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Janosch Frank 5 years, 11 months ago
For protected VMs status storing is not done by QEMU anymore.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index ed726849114f2f35..5022df8812d406c9 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -25,6 +25,7 @@
 #include "qemu/timer.h"
 #include "qemu/qemu-print.h"
 #include "hw/s390x/ioinst.h"
+#include "hw/s390x/pv.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
 #ifndef CONFIG_USER_ONLY
@@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
     hwaddr len = sizeof(*sa);
     int i;
 
+    /* Storing will occur on next SIE entry for protected VMs */
+    if (s390_is_pv()) {
+        return 0;
+    }
+
     sa = cpu_physical_memory_map(addr, &len, true);
     if (!sa) {
         return -EFAULT;
-- 
2.25.1


Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Christian Borntraeger 5 years, 11 months ago
On 11.03.20 14:21, Janosch Frank wrote:
> For protected VMs status storing is not done by QEMU anymore.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>


> ---
>  target/s390x/helper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index ed726849114f2f35..5022df8812d406c9 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -25,6 +25,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/qemu-print.h"
>  #include "hw/s390x/ioinst.h"
> +#include "hw/s390x/pv.h"
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/runstate.h"
>  #ifndef CONFIG_USER_ONLY
> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>      hwaddr len = sizeof(*sa);
>      int i;
>  
> +    /* Storing will occur on next SIE entry for protected VMs */

Maybe ... next SIE entry of the sending CPU .... 
?

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>


> +    if (s390_is_pv()) {
> +        return 0;
> +    }
> +
>      sa = cpu_physical_memory_map(addr, &len, true);
>      if (!sa) {
>          return -EFAULT;
> 


Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Janosch Frank 5 years, 11 months ago
On 3/12/20 4:51 PM, Christian Borntraeger wrote:
> On 11.03.20 14:21, Janosch Frank wrote:
>> For protected VMs status storing is not done by QEMU anymore.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> 
>> ---
>>  target/s390x/helper.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
>> index ed726849114f2f35..5022df8812d406c9 100644
>> --- a/target/s390x/helper.c
>> +++ b/target/s390x/helper.c
>> @@ -25,6 +25,7 @@
>>  #include "qemu/timer.h"
>>  #include "qemu/qemu-print.h"
>>  #include "hw/s390x/ioinst.h"
>> +#include "hw/s390x/pv.h"
>>  #include "sysemu/hw_accel.h"
>>  #include "sysemu/runstate.h"
>>  #ifndef CONFIG_USER_ONLY
>> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>>      hwaddr len = sizeof(*sa);
>>      int i;
>>  
>> +    /* Storing will occur on next SIE entry for protected VMs */
> 
> Maybe ... next SIE entry of the sending CPU .... 
> ?

Well that would be the current cpu, right?
So:
/* For PVMs storing will occur when this cpu enters SIE again */

> 
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> 
>> +    if (s390_is_pv()) {
>> +        return 0;
>> +    }
>> +
>>      sa = cpu_physical_memory_map(addr, &len, true);
>>      if (!sa) {
>>          return -EFAULT;
>>


Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Christian Borntraeger 5 years, 11 months ago

On 12.03.20 17:13, Janosch Frank wrote:
> On 3/12/20 4:51 PM, Christian Borntraeger wrote:
>> On 11.03.20 14:21, Janosch Frank wrote:
>>> For protected VMs status storing is not done by QEMU anymore.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>
>>
>>> ---
>>>  target/s390x/helper.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
>>> index ed726849114f2f35..5022df8812d406c9 100644
>>> --- a/target/s390x/helper.c
>>> +++ b/target/s390x/helper.c
>>> @@ -25,6 +25,7 @@
>>>  #include "qemu/timer.h"
>>>  #include "qemu/qemu-print.h"
>>>  #include "hw/s390x/ioinst.h"
>>> +#include "hw/s390x/pv.h"
>>>  #include "sysemu/hw_accel.h"
>>>  #include "sysemu/runstate.h"
>>>  #ifndef CONFIG_USER_ONLY
>>> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>>>      hwaddr len = sizeof(*sa);
>>>      int i;
>>>  
>>> +    /* Storing will occur on next SIE entry for protected VMs */
>>
>> Maybe ... next SIE entry of the sending CPU .... 
>> ?
> 
> Well that would be the current cpu, right?
> So:
> /* For PVMs storing will occur when this cpu enters SIE again */

ack.


Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Cornelia Huck 5 years, 10 months ago
On Thu, 12 Mar 2020 17:13:10 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 3/12/20 4:51 PM, Christian Borntraeger wrote:
> > On 11.03.20 14:21, Janosch Frank wrote:  
> >> For protected VMs status storing is not done by QEMU anymore.
> >>
> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >> Reviewed-by: Thomas Huth <thuth@redhat.com>
> >> Reviewed-by: David Hildenbrand <david@redhat.com>  
> > 
> >   
> >> ---
> >>  target/s390x/helper.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> >> index ed726849114f2f35..5022df8812d406c9 100644
> >> --- a/target/s390x/helper.c
> >> +++ b/target/s390x/helper.c
> >> @@ -25,6 +25,7 @@
> >>  #include "qemu/timer.h"
> >>  #include "qemu/qemu-print.h"
> >>  #include "hw/s390x/ioinst.h"
> >> +#include "hw/s390x/pv.h"
> >>  #include "sysemu/hw_accel.h"
> >>  #include "sysemu/runstate.h"
> >>  #ifndef CONFIG_USER_ONLY
> >> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
> >>      hwaddr len = sizeof(*sa);
> >>      int i;
> >>  
> >> +    /* Storing will occur on next SIE entry for protected VMs */  
> > 
> > Maybe ... next SIE entry of the sending CPU .... 
> > ?  
> 
> Well that would be the current cpu, right?
> So:
> /* For PVMs storing will occur when this cpu enters SIE again */

With that comment tweak,
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Christian Borntraeger 5 years, 11 months ago

On 12.03.20 17:13, Janosch Frank wrote:
> On 3/12/20 4:51 PM, Christian Borntraeger wrote:
>> On 11.03.20 14:21, Janosch Frank wrote:
>>> For protected VMs status storing is not done by QEMU anymore.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> Reviewed-by: David Hildenbrand <david@redhat.com>
>>
>>
>>> ---
>>>  target/s390x/helper.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
>>> index ed726849114f2f35..5022df8812d406c9 100644
>>> --- a/target/s390x/helper.c
>>> +++ b/target/s390x/helper.c
>>> @@ -25,6 +25,7 @@
>>>  #include "qemu/timer.h"
>>>  #include "qemu/qemu-print.h"
>>>  #include "hw/s390x/ioinst.h"
>>> +#include "hw/s390x/pv.h"
>>>  #include "sysemu/hw_accel.h"
>>>  #include "sysemu/runstate.h"
>>>  #ifndef CONFIG_USER_ONLY
>>> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>>>      hwaddr len = sizeof(*sa);
>>>      int i;
>>>  
>>> +    /* Storing will occur on next SIE entry for protected VMs */
>>
>> Maybe ... next SIE entry of the sending CPU .... 
>> ?
> 
> Well that would be the current cpu, right?
> So:
> /* For PVMs storing will occur when this cpu enters SIE again */

ack.


Re: [PATCH v9 13/15] s390x: protvirt: Handle SIGP store status correctly
Posted by Claudio Imbrenda 5 years, 11 months ago
On Wed, 11 Mar 2020 09:21:49 -0400
Janosch Frank <frankja@linux.ibm.com> wrote:

> For protected VMs status storing is not done by QEMU anymore.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/helper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index ed726849114f2f35..5022df8812d406c9 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -25,6 +25,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/qemu-print.h"
>  #include "hw/s390x/ioinst.h"
> +#include "hw/s390x/pv.h"
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/runstate.h"
>  #ifndef CONFIG_USER_ONLY
> @@ -246,6 +247,11 @@ int s390_store_status(S390CPU *cpu, hwaddr addr,
> bool store_arch) hwaddr len = sizeof(*sa);
>      int i;
>  
> +    /* Storing will occur on next SIE entry for protected VMs */
> +    if (s390_is_pv()) {
> +        return 0;
> +    }
> +
>      sa = cpu_physical_memory_map(addr, &len, true);
>      if (!sa) {
>          return -EFAULT;

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