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

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: <510cf2fb-1d53-485f-bfd1-3d852378c866@redhat.com>
Date:   Mon, 27 Nov 2017 12:58:14 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Filippo Sironi <sironi@...zon.de>, rkrcmar@...hat.com,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] KVM: x86: Allow userspace to define what's the
 microcode version

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@...zon.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