On 1/8/20 3:28 PM, Christian Borntraeger wrote:
>
>
> On 05.12.19 13:09, Janosch Frank wrote:
> [...]
>> +4.123 KVM_S390_CLEAR_RESET
>> +
>> +Capability: KVM_CAP_S390_VCPU_RESETS
>> +Architectures: s390
>> +Type: vcpu ioctl
>> +Parameters: none
>> +Returns: 0
>> +
>> +This ioctl resets VCPU registers and control structures that QEMU
>> +can't access via the kvm_run structure. The clear reset is a superset
>> +of the initial reset and additionally clears general, access, floating
>> +and vector registers.
>
> As Thomas outlined, make it more obvious that userspace does the remaining
> parts. I do not think that we want the kernel to do the things (unless it
> helps you in some way for the ultravisor guests)
Ok, will do
>
> [...]
>>
>> +static int kvm_arch_vcpu_ioctl_normal_reset(struct kvm_vcpu *vcpu)
>> +{
>> + kvm_clear_async_pf_completion_queue(vcpu);
>> + kvm_s390_clear_local_irqs(vcpu);
>> + return 0;
>
> Shouldnt we also do
> if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
> kvm_s390_vcpu_stop(vcpu);
>
> here?
Isn't userspace cpu state ctrl basically a prereq anyway for getting
those new ioctls?
>
>
>> +}
>> +
>> static int kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
>> {
>> kvm_s390_vcpu_initial_reset(vcpu);
>> @@ -4363,9 +4371,15 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>> r = kvm_arch_vcpu_ioctl_set_initial_psw(vcpu, psw);
>> break;
>> }
>> +
>> + case KVM_S390_CLEAR_RESET:
>> + /* fallthrough */
>> case KVM_S390_INITIAL_RESET:
>> r = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
>> break;
>
> Then we could also do a fallthrough here and do:
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d9e6bf3..c715ae3 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2867,10 +2867,6 @@ static void kvm_s390_vcpu_initial_reset(struct kvm_vcpu *vcpu)
> vcpu->arch.sie_block->pp = 0;
> vcpu->arch.sie_block->fpf &= ~FPF_BPBC;
> vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
> - kvm_clear_async_pf_completion_queue(vcpu);
> - if (!kvm_s390_user_cpu_state_ctrl(vcpu->kvm))
> - kvm_s390_vcpu_stop(vcpu);
> - kvm_s390_clear_local_irqs(vcpu);
> }
>
> void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
>
>
>
>
>> + case KVM_S390_NORMAL_RESET:
>> + r = kvm_arch_vcpu_ioctl_normal_reset(vcpu);
>> + break;
>> case KVM_SET_ONE_REG:
>> case KVM_GET_ONE_REG: {
Attachment:
signature.asc
Description: OpenPGP digital signature
