Re: [PATCH] s390x: do a subsystem reset before the unprotect on reboot

2023-09-06 Thread Christian Borntraeger




Am 01.09.23 um 13:48 schrieb Janosch Frank:

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 do a subsystem_reset() which includes a AP reset before the
unprotect call. We'll do a full device_reset() afterwards which will
reset some devices twice. That's ok since we can't move the
device_reset() before the unprotect as it includes a CPU clear reset
which the Ultravisor does not expect at that point in time.

Signed-off-by: Janosch Frank 


Makes sense.
Acked-by: Christian Borntraeger 


---

I'm not able to test this for the PV AP case right new, that has to
wait to early next week. However Marc told me that the non-AP PV test
works fine now.

---
  hw/s390x/s390-virtio-ccw.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3dd0b2372d..2d75f2131f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -438,10 +438,20 @@ static void s390_machine_reset(MachineState *machine, 
ShutdownCause reason)
  switch (reset_type) {
  case S390_RESET_EXTERNAL:
  case S390_RESET_REIPL:
+/*
+ * Reset the subsystem which includes a AP reset. If a PV
+ * guest had APQNs attached the AP reset is a prerequisite to
+ * unprotecting since the UV checks if all APQNs are reset.
+ */
+subsystem_reset();
  if (s390_is_pv()) {
  s390_machine_unprotect(ms);
  }
  
+/*

+ * Device reset includes CPU clear resets so this has to be
+ * done AFTER the unprotect call above.
+ */
  qemu_devices_reset(reason);
  s390_crypto_reset();
  




Re: [PATCH] s390x: do a subsystem reset before the unprotect on reboot

2023-09-06 Thread Viktor Mihajlovski
On Fri, 2023-09-01 at 11:48 +, Janosch Frank wrote:
> 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 do a subsystem_reset() which includes a AP reset before the
> unprotect call. We'll do a full device_reset() afterwards which will
> reset some devices twice. That's ok since we can't move the
> device_reset() before the unprotect as it includes a CPU clear reset
> which the Ultravisor does not expect at that point in time.
> 
> Signed-off-by: Janosch Frank 
> ---
> 
> I'm not able to test this for the PV AP case right new, that has to
> wait to early next week. However Marc told me that the non-AP PV test
> works fine now.
> 
> ---
>  hw/s390x/s390-virtio-ccw.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3dd0b2372d..2d75f2131f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -438,10 +438,20 @@ static void s390_machine_reset(MachineState *machine, 
> ShutdownCause reason)
>  switch (reset_type) {
>  case S390_RESET_EXTERNAL:
>  case S390_RESET_REIPL:
> +/*
> + * Reset the subsystem which includes a AP reset. If a PV
> + * guest had APQNs attached the AP reset is a prerequisite to
> + * unprotecting since the UV checks if all APQNs are reset.
> + */
> +subsystem_reset();
>  if (s390_is_pv()) {
>  s390_machine_unprotect(ms);
>  }
>  
> +/*
> + * Device reset includes CPU clear resets so this has to be
> + * done AFTER the unprotect call above.
> + */
>  qemu_devices_reset(reason);
>  s390_crypto_reset();
>  
I tested this with and without bound/associated APQNs both with booting
from disk and with direct kernel boot. Subsequent reboots are correctly
resetting the APQNs. I also successfully tested the case direct kernel
boot -> chreipl -> disk boot.

If you want you can add
  Tested-by: Viktor Mihajlovski 




[PATCH] s390x: do a subsystem reset before the unprotect on reboot

2023-09-01 Thread Janosch Frank
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 do a subsystem_reset() which includes a AP reset before the
unprotect call. We'll do a full device_reset() afterwards which will
reset some devices twice. That's ok since we can't move the
device_reset() before the unprotect as it includes a CPU clear reset
which the Ultravisor does not expect at that point in time.

Signed-off-by: Janosch Frank 
---

I'm not able to test this for the PV AP case right new, that has to
wait to early next week. However Marc told me that the non-AP PV test
works fine now.

---
 hw/s390x/s390-virtio-ccw.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3dd0b2372d..2d75f2131f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -438,10 +438,20 @@ static void s390_machine_reset(MachineState *machine, 
ShutdownCause reason)
 switch (reset_type) {
 case S390_RESET_EXTERNAL:
 case S390_RESET_REIPL:
+/*
+ * Reset the subsystem which includes a AP reset. If a PV
+ * guest had APQNs attached the AP reset is a prerequisite to
+ * unprotecting since the UV checks if all APQNs are reset.
+ */
+subsystem_reset();
 if (s390_is_pv()) {
 s390_machine_unprotect(ms);
 }
 
+/*
+ * Device reset includes CPU clear resets so this has to be
+ * done AFTER the unprotect call above.
+ */
 qemu_devices_reset(reason);
 s390_crypto_reset();
 
-- 
2.34.1