Re: [RFC 33/37] KVM: s390: Introduce VCPU reset IOCTL

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

 



On 15/11/2019 14.06, Janosch Frank wrote:
> On 11/15/19 11:47 AM, Thomas Huth wrote:
>> On 24/10/2019 13.40, Janosch Frank wrote:
>>> With PV we need to do things for all reset types, not only initial...
>>>
>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>> ---
>>>  arch/s390/kvm/kvm-s390.c | 53 ++++++++++++++++++++++++++++++++++++++++
>>>  include/uapi/linux/kvm.h |  6 +++++
>>>  2 files changed, 59 insertions(+)
>>>
>>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>>> index d3fd3ad1d09b..d8ee3a98e961 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -3472,6 +3472,53 @@ static int kvm_arch_vcpu_ioctl_initial_reset(struct kvm_vcpu *vcpu)
>>>  	return 0;
>>>  }
>>>  
>>> +static int kvm_arch_vcpu_ioctl_reset(struct kvm_vcpu *vcpu,
>>> +				     unsigned long type)
>>> +{
>>> +	int rc;
>>> +	u32 ret;
>>> +
>>> +	switch (type) {
>>> +	case KVM_S390_VCPU_RESET_NORMAL:
>>> +		/*
>>> +		 * Only very little is reset, userspace handles the
>>> +		 * non-protected case.
>>> +		 */
>>> +		rc = 0;
>>> +		if (kvm_s390_pv_handle_cpu(vcpu)) {
>>> +			rc = uv_cmd_nodata(kvm_s390_pv_handle_cpu(vcpu),
>>> +					   UVC_CMD_CPU_RESET, &ret);
>>> +			VCPU_EVENT(vcpu, 3, "PROTVIRT RESET NORMAL VCPU: cpu %d rc %x rrc %x",
>>> +				   vcpu->vcpu_id, ret >> 16, ret & 0x0000ffff);
>>> +		}
>>> +		break;
>>> +	case KVM_S390_VCPU_RESET_INITIAL:
>>> +		rc = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
>>> +		if (kvm_s390_pv_handle_cpu(vcpu)) {
>>> +			uv_cmd_nodata(kvm_s390_pv_handle_cpu(vcpu),
>>> +				      UVC_CMD_CPU_RESET_INITIAL,
>>> +				      &ret);
>>> +			VCPU_EVENT(vcpu, 3, "PROTVIRT RESET INITIAL VCPU: cpu %d rc %x rrc %x",
>>> +				   vcpu->vcpu_id, ret >> 16, ret & 0x0000ffff);
>>> +		}
>>> +		break;
>>> +	case KVM_S390_VCPU_RESET_CLEAR:
>>> +		rc = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
>>> +		if (kvm_s390_pv_handle_cpu(vcpu)) {
>>> +			rc = uv_cmd_nodata(kvm_s390_pv_handle_cpu(vcpu),
>>> +					   UVC_CMD_CPU_RESET_CLEAR, &ret);
>>> +			VCPU_EVENT(vcpu, 3, "PROTVIRT RESET CLEAR VCPU: cpu %d rc %x rrc %x",
>>> +				   vcpu->vcpu_id, ret >> 16, ret & 0x0000ffff);
>>> +		}
>>> +		break;
>>> +	default:
>>> +		rc = -EINVAL;
>>> +		break;
>>
>> (nit: you could drop the "break;" here)
>>
>>> +	}
>>> +	return rc;
>>> +}
>>> +
>>> +
>>>  int kvm_arch_vcpu_ioctl_set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>>>  {
>>>  	vcpu_load(vcpu);
>>> @@ -4633,8 +4680,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>>>  		break;
>>>  	}
>>>  	case KVM_S390_INITIAL_RESET:
>>> +		r = -EINVAL;
>>> +		if (kvm_s390_pv_is_protected(vcpu->kvm))
>>> +			break;
>>
>> Wouldn't it be nicer to call
>>
>>   kvm_arch_vcpu_ioctl_reset(vcpu, KVM_S390_VCPU_RESET_INITIAL)
>>
>> in this case instead?
> 
> How about:
>         case KVM_S390_INITIAL_RESET:
> 
> 
>                 arg = KVM_S390_VCPU_RESET_INITIAL;
> 

Add a "/* fallthrough */" comment here...

>         case KVM_S390_VCPU_RESET:
> 
> 
>                 r = kvm_arch_vcpu_ioctl_reset(vcpu, arg);
> 
> 
>                 break;

... then this looks good, yes!

 Thomas

Attachment: signature.asc
Description: OpenPGP digital signature


[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