diff mbox series

[v4,2/5] s390x: switch pv and subsystem reset ordering on reboot

Message ID 20230823142219.1046522-3-seiden@linux.ibm.com
State New
Headers show
Series s390: Enable AP instructions for pv-guests | expand

Commit Message

Steffen Eiden Aug. 23, 2023, 2:22 p.m. UTC
From: Janosch Frank <frankja@linux.ibm.com>

Bound APQNs have to be reset before tearing down the secure config via
s390_machine_unprotect(). Otherwise the Ultravisor will return a error
code.

So let's switch the ordering around to make that happen.

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Marc Hartmayer Aug. 31, 2023, 4:21 p.m. UTC | #1
On Wed, Aug 23, 2023 at 04:22 PM +0200, Steffen Eiden <seiden@linux.ibm.com> wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
>
> Bound APQNs have to be reset before tearing down the secure config via
> s390_machine_unprotect(). Otherwise the Ultravisor will return a error
> code.
>
> So let's switch the ordering around to make that happen.
>
> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4b36c9970e..795dd53d68 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -442,13 +442,13 @@ static void s390_machine_reset(MachineState *machine, ShutdownCause reason)
>      switch (reset_type) {
>      case S390_RESET_EXTERNAL:
>      case S390_RESET_REIPL:
> +        qemu_devices_reset(reason);
> +        s390_crypto_reset();
> +
>          if (s390_is_pv()) {
>              s390_machine_unprotect(ms);
>          }
>  
> -        qemu_devices_reset(reason);
> -        s390_crypto_reset();
> -
>          /* configure and start the ipl CPU only */
>          run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
>          break;
> -- 
> 2.41.0
>

Unfortunately, this breaks things for me. You can reproduce the problem
easily… Start an SE guest via direct kernel boot and reboot the guest
after the guest has booted.

e.g.

$ sudo qemu-system-s390x -smp 2 --machine accel=kvm,aes-key-wrap=off -kernel /var/lib/libvirt/images/hades/vmlinux-s390x.se.img -m 2048 -nographic -nodefaults -chardev stdio,id=c1,mux=on  -device sclpconsole,chardev=c1 -append "earlyprintk"
…
#  reboot  # (console in the guest)
…
[    3.966496] systemd-shutdown[1]: Syncing filesystems and block devices.
[    3.966606] systemd-shutdown[1]: Rebooting.
qemu-system-s390x: CPU reset failed on CPU 0 type aec4
qemu-system-s390x: CPU reset failed on CPU 1 type aec4
qemu-system-s390x: KVM PV command 4 (KVM_PV_VERIFY) failed: header rc 102 rrc 1b IOCTL rc: -22
Protected boot has failed: 0xa02
Janosch Frank Sept. 1, 2023, 6:31 a.m. UTC | #2
On 8/31/23 18:21, Marc Hartmayer wrote:
> On Wed, Aug 23, 2023 at 04:22 PM +0200, Steffen Eiden <seiden@linux.ibm.com> wrote:
>> From: Janosch Frank <frankja@linux.ibm.com>
>>
>> Bound APQNs have to be reset before tearing down the secure config via
>> s390_machine_unprotect(). Otherwise the Ultravisor will return a error
>> code.
>>
>> So let's switch the ordering around to make that happen.
>>
>> Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>   hw/s390x/s390-virtio-ccw.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 4b36c9970e..795dd53d68 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -442,13 +442,13 @@ static void s390_machine_reset(MachineState *machine, ShutdownCause reason)
>>       switch (reset_type) {
>>       case S390_RESET_EXTERNAL:
>>       case S390_RESET_REIPL:
>> +        qemu_devices_reset(reason);
>> +        s390_crypto_reset();
>> +
>>           if (s390_is_pv()) {
>>               s390_machine_unprotect(ms);
>>           }
>>   
>> -        qemu_devices_reset(reason);
>> -        s390_crypto_reset();
>> -
>>           /* configure and start the ipl CPU only */
>>           run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
>>           break;
>> -- 
>> 2.41.0
>>
> 
> Unfortunately, this breaks things for me. You can reproduce the problem
> easily… Start an SE guest via direct kernel boot and reboot the guest
> after the guest has booted.

Seems like we didn't test a direct kernel boot reboot...
I'm looking into it.
diff mbox series

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4b36c9970e..795dd53d68 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -442,13 +442,13 @@  static void s390_machine_reset(MachineState *machine, ShutdownCause reason)
     switch (reset_type) {
     case S390_RESET_EXTERNAL:
     case S390_RESET_REIPL:
+        qemu_devices_reset(reason);
+        s390_crypto_reset();
+
         if (s390_is_pv()) {
             s390_machine_unprotect(ms);
         }
 
-        qemu_devices_reset(reason);
-        s390_crypto_reset();
-
         /* configure and start the ipl CPU only */
         run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
         break;