Re: [PATCH] s390x: Clear RAM on diag308 subcode 3 reset

[Thomas Huth <thuth@redhat.com>]

On 11/11/2025 09.43, Christian Borntraeger wrote:
> Am 29.04.25 um 07:20 schrieb Nicholas Miehlbradt:
>> The reset performed by subcode 3 of the diag308 instruction specifies
>> that system memory should be reset. This patch implements that
>> behaviour.
>>
>> Introduce S390_RESET_REIPL_CLEAR to differentiate between subcode 3 and
>> subcode 4 resets.
>>
>> When doing a clear reset, discard the ramblock containing the system
>> ram.
>>
>> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
>> ---
>>   hw/s390x/ipl.h             | 1 +
>>   hw/s390x/s390-virtio-ccw.c | 6 ++++++
>>   target/s390x/diag.c        | 3 +--
>>   target/s390x/kvm/kvm.c     | 6 +++++-
>>   4 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
>> index cb55101f06..9c38946363 100644
>> --- a/hw/s390x/ipl.h
>> +++ b/hw/s390x/ipl.h
>> @@ -38,6 +38,7 @@ enum s390_reset {
>>       /* default is a reset not triggered by a CPU e.g. issued by QMP */
>>       S390_RESET_EXTERNAL = 0,
>>       S390_RESET_REIPL,
>> +    S390_RESET_REIPL_CLEAR,
>>       S390_RESET_MODIFIED_CLEAR,
>>       S390_RESET_LOAD_NORMAL,
>>       S390_RESET_PV,
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 94edd42dd2..bc07158b16 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -455,6 +455,7 @@ static void s390_machine_reset(MachineState *machine, 
>> ResetType type)
>>       enum s390_reset reset_type;
>>       CPUState *cs, *t;
>>       S390CPU *cpu;
>> +    RAMBlock *rb = machine->ram->ram_block;
>>       /*
>>        * Temporarily drop the record/replay mutex to let rr_cpu_thread_fn()
>> @@ -479,6 +480,7 @@ static void s390_machine_reset(MachineState *machine, 
>> ResetType type)
>>       switch (reset_type) {
>>       case S390_RESET_EXTERNAL:
>>       case S390_RESET_REIPL:
>> +    case S390_RESET_REIPL_CLEAR:
>>           /*
>>            * Reset the subsystem which includes a AP reset. If a PV
>>            * guest had APQNs attached the AP reset is a prerequisite to
>> @@ -489,6 +491,10 @@ static void s390_machine_reset(MachineState *machine, 
>> ResetType type)
>>               s390_machine_unprotect(ms);
>>           }
>> +        if (reset_type == S390_RESET_REIPL_CLEAR) {
>> +            ram_block_discard_range(rb, 0 , qemu_ram_get_used_length(rb));
>> +        }
>> +
>>           /*
>>            * Device reset includes CPU clear resets so this has to be
>>            * done AFTER the unprotect call above.
>> diff --git a/target/s390x/diag.c b/target/s390x/diag.c
>> index da44b0133e..cff9fbc4b0 100644
>> --- a/target/s390x/diag.c
>> +++ b/target/s390x/diag.c
>> @@ -105,8 +105,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, 
>> uint64_t r3, uintptr_t ra)
>>           s390_ipl_reset_request(cs, S390_RESET_LOAD_NORMAL);
>>           break;      case DIAG308_LOAD_CLEAR:
>> -        /* Well we still lack the clearing bit... */
>> -        s390_ipl_reset_request(cs, S390_RESET_REIPL);
>> +        s390_ipl_reset_request(cs, S390_RESET_REIPL_CLEAR);
>>           break;
>>       case DIAG308_SET:
>>       case DIAG308_PV_SET:
>> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
>> index b9f1422197..f2d5f7ddc0 100644
>> --- a/target/s390x/kvm/kvm.c
>> +++ b/target/s390x/kvm/kvm.c
>> @@ -1915,7 +1915,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct 
>> kvm_run *run)
>>               ret = handle_intercept(cpu);
>>               break;
>>           case KVM_EXIT_S390_RESET:
>> -            s390_ipl_reset_request(cs, S390_RESET_REIPL);
>> +            if (run->s390_reset_flags & KVM_S390_RESET_CLEAR) {
>> +                s390_ipl_reset_request(cs, S390_RESET_REIPL_CLEAR);
>> +            } else {
>> +                s390_ipl_reset_request(cs, S390_RESET_REIPL);
>> +            }
>>               break;
>>           case KVM_EXIT_S390_TSCH:
>>               ret = handle_tsch(cpu);
> 
> 
> 
> Do I see that right that this patch never made it into qemu master? IIRC 
> Matt has clarified all concerns?

I was hoping to see a reply from David that he's fine with the patch now... 
David?

  Thomas