linux-kernel - Re: [PATCH v3 25/27] KVM: nVMX: Add FRED VMCS fields

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] [day] [month] [year] [list]

Message-ID: <538c630c-0de3-4807-9e9f-6af02dd18d0e@zytor.com>
Date: Tue, 29 Oct 2024 10:40:03 -0700
From: Xin Li <xin@...or.com>
To: Sean Christopherson <seanjc@...gle.com>, Chao Gao <chao.gao@...el.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, pbonzini@...hat.com, corbet@....net,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
        luto@...nel.org, peterz@...radead.org, andrew.cooper3@...rix.com
Subject: Re: [PATCH v3 25/27] KVM: nVMX: Add FRED VMCS fields

On 10/28/2024 11:27 AM, Sean Christopherson wrote:
> On Mon, Oct 28, 2024, Chao Gao wrote:
>> On Fri, Oct 25, 2024 at 12:25:45AM -0700, Xin Li wrote:
>>>>> static void nested_vmx_setup_cr_fixed(struct nested_vmx_msrs *msrs)
>>>>> diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
>>>>> index 2c296b6abb8c..5272f617fcef 100644
>>>>> --- a/arch/x86/kvm/vmx/nested.h
>>>>> +++ b/arch/x86/kvm/vmx/nested.h
>>>>> @@ -251,6 +251,14 @@ static inline bool nested_cpu_has_encls_exit(struct vmcs12 *vmcs12)
>>>>> 	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENCLS_EXITING);
>>>>> }
>>>>>
>>>>> +static inline bool nested_cpu_has_fred(struct vmcs12 *vmcs12)
>>>>> +{
>>>>> +	return vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_FRED &&
>>>>> +	       vmcs12->vm_exit_controls & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS &&
>>>>> +	       vmcs12->secondary_vm_exit_controls & SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
>>>>> +	       vmcs12->secondary_vm_exit_controls & SECONDARY_VM_EXIT_LOAD_IA32_FRED;
>>>>
>>>> Is it a requirement in the SDM that the VMM should enable all FRED controls or
>>>> none? If not, the VMM is allowed to enable only one or two of them. This means
>>>> KVM would need to emulate FRED controls for the L1 VMM as three separate
>>>> features.
>>>
>>> The SDM doesn't say that.  But FRED states are used during and
>>> immediately after VM entry and exit, I don't see a good reason for a VMM
>>> to enable only one or two of the 3 save/load configs.
> 
> Not KVM's concern.
> 
>>> Say if VM_ENTRY_LOAD_IA32_FRED is not set, it means a VMM needs to
>>> switch to guest FRED states before it does a VM entry, which is
>>> absolutely a big mess.
> 
> Again, not KVM's concern.
> 
>> If the VMM doesn't enable FRED, it's fine to load guest FRED states before VM
>> entry, right?
> 
> Yep.  Or if L1 is simply broken and elects to manually load FRED state before
> VM-Enter instead of using VM_ENTRY_LOAD_IA32_FRED, then any badness that happens
> is 100% L1's problem to deal with.  KVM's responsiblity is to emulate the
> architectural behavior, what L1 may or may not do is irrelevant.

Damn, obviously I COMPLETELY missed this point.

Let me think how should KVM as L0 handle it.

> 
>> The key is to emulate hardware behavior accurately without making assumptions
>> about guests.
> 
> +1000
> 
>> If some combinations of controls cannot be emulated properly, KVM
>> should report internal errors at some point.

Yeah, only if CANNOT.  Otherwise a broken VMM will behave differently on
real hardware and KVM, even if it crashes in a way which it never knows
about, right?