linux-kernel - Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency hypercalls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170203170336.GA15128@potion>
Date:   Fri, 3 Feb 2017 18:03:37 +0100
From:   Radim Krcmar <rkrcmar@...hat.com>
To:     Marcelo Tosatti <mtosatti@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Viresh Kumar <viresh.kumar@...aro.org>
Subject: Re: [patch 2/3] KVM: x86: introduce ioctl to allow frequency
 hypercalls

2017-02-02 15:47-0200, Marcelo Tosatti:
> For most VMs, modifying the host frequency is an undesired 
> operation. Introduce ioctl to enable the guest to 
> modify host CPU frequency.
> 
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@...hat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    2 ++
>  arch/x86/include/uapi/asm/kvm.h |    5 +++++
>  arch/x86/kvm/x86.c              |   20 ++++++++++++++++++++
>  include/uapi/linux/kvm.h        |    3 +++
>  virt/kvm/kvm_main.c             |    2 ++
>  5 files changed, 32 insertions(+)
> 
> Index: kvm-pvfreq/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/kvm/x86.c	2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/kvm/x86.c	2017-01-31 10:34:25.443618639 -0200
> @@ -3665,6 +3665,26 @@
>  		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
>  		break;
>  	}
> +	case KVM_SET_VCPU_ALLOW_FREQ_HC: {

Just enable the frequency hypercalls with KVM_ENABLE_CAP ioctl and get
rid of this ioctl.
(I don't think that we want to allow disabling this capability.)

> +		struct kvm_vcpu_allow_freq freq;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&freq, argp, sizeof(freq)))
> +			goto out;
> +		vcpu->arch.allow_freq_hypercall = freq.enable;

Enabling the capability should also set a bit in KVM_CPUID_FEATURES,
a well-behaved guest won't use it otherwise.

> +		r = 0;
> +		break;
> +	}
> +	case KVM_GET_VCPU_ALLOW_FREQ_HC: {

And this ioctl has no use, so it should be omitted.

(Userspace doesn't learn anything it doesn't already know.
 The feature is unsafe, so the default must be 0.)

> +		struct kvm_vcpu_allow_freq freq;
> +
> +		memset(&freq, 0, sizeof(struct kvm_vcpu_allow_freq));
> +		r = -EFAULT;
> +		if (copy_to_user(&freq, argp, sizeof(freq)))
> +			break;
> +		r = 0;
> +		break;
> +	}
>  	default:
>  		r = -EINVAL;
>  	}
> Index: kvm-pvfreq/include/uapi/linux/kvm.h
> ===================================================================
> --- kvm-pvfreq.orig/include/uapi/linux/kvm.h	2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/include/uapi/linux/kvm.h	2017-01-31 10:32:38.000389402 -0200
> @@ -871,6 +871,7 @@
>  #define KVM_CAP_S390_USER_INSTR0 130
>  #define KVM_CAP_MSI_DEVID 131
>  #define KVM_CAP_PPC_HTM 132
> +#define KVM_CAP_ALLOW_FREQ_HC 133
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -1281,6 +1282,8 @@
>  #define KVM_S390_GET_IRQ_STATE	  _IOW(KVMIO, 0xb6, struct kvm_s390_irq_state)
>  /* Available with KVM_CAP_X86_SMM */
>  #define KVM_SMI                   _IO(KVMIO,   0xb7)
> +#define KVM_SET_VCPU_ALLOW_FREQ_HC   _IO(KVMIO,   0xb8)
> +#define KVM_GET_VCPU_ALLOW_FREQ_HC   _IO(KVMIO,   0xb9)
>  
>  #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
>  #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
> Index: kvm-pvfreq/arch/x86/include/uapi/asm/kvm.h
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/include/uapi/asm/kvm.h	2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/include/uapi/asm/kvm.h	2017-01-31 10:32:38.000389402 -0200
> @@ -357,4 +357,9 @@
>  #define KVM_X86_QUIRK_LINT0_REENABLED	(1 << 0)
>  #define KVM_X86_QUIRK_CD_NW_CLEARED	(1 << 1)
>  
> +struct kvm_vcpu_allow_freq {
> +	__u16 enable;
> +	__u16 pad[7];
> +};
> +
>  #endif /* _ASM_X86_KVM_H */
> Index: kvm-pvfreq/virt/kvm/kvm_main.c
> ===================================================================
> --- kvm-pvfreq.orig/virt/kvm/kvm_main.c	2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/virt/kvm/kvm_main.c	2017-01-31 10:32:38.001389404 -0200
> @@ -2938,6 +2938,8 @@
>  #endif
>  	case KVM_CAP_MAX_VCPU_ID:
>  		return KVM_MAX_VCPU_ID;
> +	case KVM_CAP_ALLOW_FREQ_HC:

This can share an existing return 1 and would be elsewhere with
KVM_ENABLE_CAP.

> +		return 1;
>  	default:
>  		break;
>  	}
> Index: kvm-pvfreq/arch/x86/include/asm/kvm_host.h
> ===================================================================
> --- kvm-pvfreq.orig/arch/x86/include/asm/kvm_host.h	2017-01-31 10:32:33.023378783 -0200
> +++ kvm-pvfreq/arch/x86/include/asm/kvm_host.h	2017-01-31 10:32:38.001389404 -0200
> @@ -678,6 +678,8 @@
>  
>  	/* GPA available (AMD only) */
>  	bool gpa_available;
> +
> +	bool allow_freq_hypercall;
>  };
>  
>  struct kvm_lpage_info {
> 
>