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;
case KVM_S390_VCPU_RESET:
r = kvm_arch_vcpu_ioctl_reset(vcpu, arg);
break;
>
>> r = kvm_arch_vcpu_ioctl_initial_reset(vcpu);
>> break;
>> + case KVM_S390_VCPU_RESET:
>> + r = kvm_arch_vcpu_ioctl_reset(vcpu, arg);
>> + break;
>> case KVM_SET_ONE_REG:
>> case KVM_GET_ONE_REG: {
>> struct kvm_one_reg reg;
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index f75a051a7705..2846ed5e5dd9 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1496,6 +1496,12 @@ struct kvm_pv_cmd {
>> #define KVM_S390_PV_COMMAND _IOW(KVMIO, 0xc3, struct kvm_pv_cmd)
>> #define KVM_S390_PV_COMMAND_VCPU _IOW(KVMIO, 0xc4, struct kvm_pv_cmd)
>>
>> +#define KVM_S390_VCPU_RESET_NORMAL 0
>> +#define KVM_S390_VCPU_RESET_INITIAL 1
>> +#define KVM_S390_VCPU_RESET_CLEAR 2
>> +
>> +#define KVM_S390_VCPU_RESET _IO(KVMIO, 0xd0)
>
> Why not 0xc5 ?
Fixed
>
> Thomas
>
Attachment:
signature.asc
Description: OpenPGP digital signature
