Re: [PATCH v3] KVM: s390: Add new reset vcpu API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 08.01.20 15:35, Janosch Frank wrote:
> 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

I changed my mind (see my other mail) but I would like Thomas, Conny or David
to ack/nack.

> 
>>
>> [...]
>>>  
>>> +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?

I do not see a reason why we should mandate userspace cpu state for the normal
reset. Using this if also enables the fallthrough below - which I like.

> 
>>
>>
>>> +}
>>> +
>>>  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: {
> 
> 




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux