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?
> 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 ?
Thomas
