Re: regression: nested: L1 3.15+ fails to load kvm-intel on L0 <3.15

Stefan Bader
2015-03-18T09:59:28Z

[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]

On 18.03.2015 10:18, Paolo Bonzini wrote:
>
>
> On 18/03/2015 09:46, Stefan Bader wrote:
>>
>> Regardless of that, I wonder whether the below (this version untested) sound
>> acceptable for upstream? At least it would make debugging much simpler. :)
>>
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -2953,8 +2953,11 @@ static __init int adjust_vmx_controls(u32 ctl_min, u32 ct
>> ctl |= vmx_msr_low; /* bit == 1 in low word ==> must be one */
>>
>> /* Ensure minimum (required) set of control bits are supported. */
>> - if (ctl_min & ~ctl)
>> + if (ctl_min & ~ctl) {
>> + printk(KERN_ERR "vmx: msr(%08x) does not match requirements. "
>> + "req=%08x cur=%08x\n", msr, ctl_min, ctl);
>> return -EIO;
>> + }
>>
>> *result = ctl;
>> return 0;
>
> Yes, this is nice. Maybe -ENODEV.

Maybe, though I did not change that. Just added to give some kind of hint when
the module would otherwise fail with just an IO error.

>
> Also, a minimal patch for Ubuntu would probably be:
>
> @@ -2850,7 +2851,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
> vmx_capability.ept, vmx_capability.vpid);
> }
>
> - min = 0;
> + min = VM_EXIT_SAVE_DEBUG_CONTROLS;
> #ifdef CONFIG_X86_64
> min |= VM_EXIT_HOST_ADDR_SPACE_SIZE;
> #endif
>
> but I don't think it's a good idea to add it to stable kernels.

Why is that? Because it has a risk of causing the module failing to load on L0
where it did work before? Which would be something I would rather avoid.
Generally I think it would be good to have something that can be generally
applied. Given the speed that cloud service providers tend to move forward (ok
they may not actively push the ability to go nested).

-Stefan
>
> Paolo