Sean Christopherson <seanjc@xxxxxxxxxx> writes:
> Paul pointed out the error messages when KVM fails to load are unhelpful
> in understanding exactly what went wrong if userspace probes the "wrong"
> module.
>
> Add a mandatory kvm_x86_ops field to track vendor module names, kvm_intel
> and kvm_amd, and use the name for relevant error message when KVM fails
> to load so that the user knows which module failed to load.
>
> Opportunistically tweak the "disabled by bios" error message to clarify
> that _support_ was disabled, not that the module itself was magically
> disabled by BIOS.
>
...
> Suggested-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/svm/svm.c | 2 ++
> arch/x86/kvm/vmx/vmx.c | 2 ++
> arch/x86/kvm/x86.c | 8 +++++---
> 4 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 80f4b8a9233c..b05bfcc72042 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1302,6 +1302,8 @@ static inline u16 kvm_lapic_irq_dest_mode(bool dest_mode_logical)
> }
>
> struct kvm_x86_ops {
> + const char *name;
> +
> int (*hardware_enable)(void);
> void (*hardware_disable)(void);
> void (*hardware_unsetup)(void);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 89077160d463..cee4915d2ce3 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4580,6 +4580,8 @@ static int svm_vm_init(struct kvm *kvm)
> }
>
> static struct kvm_x86_ops svm_x86_ops __initdata = {
> + .name = "kvm_amd",
> +
> .hardware_unsetup = svm_hardware_teardown,
> .hardware_enable = svm_hardware_enable,
> .hardware_disable = svm_hardware_disable,
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 1c8b2b6e7ed9..c147438eaafc 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7568,6 +7568,8 @@ static bool vmx_check_apicv_inhibit_reasons(ulong bit)
> }
>
> static struct kvm_x86_ops vmx_x86_ops __initdata = {
> + .name = "kvm_intel",
> +
> .hardware_unsetup = hardware_unsetup,
>
> .hardware_enable = hardware_enable,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c59b63c56af9..e966e9cdd805 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8539,18 +8539,20 @@ int kvm_arch_init(void *opaque)
> int r;
>
> if (kvm_x86_ops.hardware_enable) {
> - printk(KERN_ERR "kvm: already loaded the other module\n");
> + pr_err("kvm: already loaded vendor module '%s'\n", kvm_x86_ops.name);
> r = -EEXIST;
> goto out;
> }
>
> if (!ops->cpu_has_kvm_support()) {
> - pr_err_ratelimited("kvm: no hardware support\n");
> + pr_err_ratelimited("kvm: no hardware support for '%s'\n",
> + ops->runtime_ops->name);
> r = -EOPNOTSUPP;
> goto out;
> }
> if (ops->disabled_by_bios()) {
> - pr_err_ratelimited("kvm: disabled by bios\n");
> + pr_err_ratelimited("kvm: support for '%s' disabled by bios\n",
> + ops->runtime_ops->name);
I'd suggest we change this to
pr_err_ratelimited("kvm: %s: virtualization disabled in BIOS\n",
ops->runtime_ops->name);
or something like that as generally, it makes little sense to search for
'KVM' in BIOS settings. You need too look for either 'Virtualization' or
VT-x/AMD-v.
> r = -EOPNOTSUPP;
> goto out;
> }
--
Vitaly
