Re: [PATCH 2/2] KVM: x86: Allow userspace to define what's the microcode version

[Paolo Bonzini <pbonzini@redhat.com>]

On 26/11/2017 17:41, Filippo Sironi wrote:
> ... that the guest should see.
> Guest operating systems may check the microcode version to decide whether
> to disable certain features that are known to be buggy up to certain
> microcode versions.  Address the issue by making the microcode version
> that the guest should see settable.
> The rationale for having userspace specifying the microcode version, rather
> than having the kernel picking it, is to ensure consistency for live-migrated
> instances; we don't want them to see a microcode version increase without a
> reset.
> 
> Signed-off-by: Filippo Sironi <sironi@amazon.de>
> ---
>  arch/x86/kvm/x86.c       | 23 +++++++++++++++++++++++
>  include/uapi/linux/kvm.h |  3 +++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 925c3e29cad3..741588f27ebc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4033,6 +4033,29 @@ long kvm_arch_vm_ioctl(struct file *filp,
>  	} u;
>  
>  	switch (ioctl) {
> +	case KVM_GET_MICROCODE_VERSION: {
> +		r = -EFAULT;
> +		if (copy_to_user(argp,
> +				 &kvm->arch.microcode_version,
> +				 sizeof(kvm->arch.microcode_version)))
> +			goto out;
> +		break;
> +	}
> +	case KVM_SET_MICROCODE_VERSION: {
> +		u32 microcode_version;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&microcode_version,
> +				   argp,
> +				   sizeof(microcode_version)))
> +			goto out;
> +		r = -EINVAL;
> +		if (!microcode_version)
> +			goto out;
> +		kvm->arch.microcode_version = microcode_version;
> +		r = 0;
> +		break;
> +	}

Also, there's no need to define new ioctls, instead you can just place
it in the vcpu and use KVM_GET_MSR/KVM_SET_MSR.  I'd agree that's
slightly less polished, but it matches what we do already for e.g.
nested VMX model specific registers.  And it spares you for writing the
documentation that you didn't include in this patch. :)

Paolo