[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZiJ5LrjddiLticvR@chao-email>
Date: Fri, 19 Apr 2024 22:01:18 +0800
From: Chao Gao <chao.gao@...el.com>
To: Xin Li <xin3.li@...el.com>
CC: <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
<linux-doc@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<seanjc@...gle.com>, <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>,
<shuah@...nel.org>, <vkuznets@...hat.com>, <peterz@...radead.org>,
<ravi.v.shankar@...el.com>, <xin@...or.com>
Subject: Re: [PATCH v2 08/25] KVM: VMX: Initialize VMCS FRED fields
On Wed, Feb 07, 2024 at 09:26:28AM -0800, Xin Li wrote:
>Initialize host VMCS FRED fields with host FRED MSRs' value and
>guest VMCS FRED fields to 0.
>
>FRED CPU states are managed in 9 new FRED MSRs, as well as a few
>existing CPU registers and MSRs, e.g., CR4.FRED. To support FRED
>context management, new VMCS fields corresponding to most of FRED
>CPU state MSRs are added to both the host-state and guest-state
>areas of VMCS.
>
>Specifically no VMCS fields are added for FRED RSP0 and SSP0 MSRs,
>because the 2 FRED MSRs are used during ring 3 event delivery only,
>thus KVM, running on ring 0, can run safely even with guest FRED
>RSP0 and SSP0. It can be deferred to load host FRED RSP0 and SSP0
>until before returning to user level.
>
>Signed-off-by: Xin Li <xin3.li@...el.com>
>Tested-by: Shan Kang <shan.kang@...el.com>
>---
>
>Changes since v1:
>* Use kvm_cpu_cap_has() instead of cpu_feature_enabled() to decouple
> KVM's capability to virtualize a feature and host's enabling of a
> feature (Chao Gao).
>* Move guest FRED states init into __vmx_vcpu_reset() (Chao Gao).
>---
> arch/x86/include/asm/vmx.h | 16 ++++++++++++++++
> arch/x86/kvm/vmx/vmx.c | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
>diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
>index cb14f7e315f5..4889754415b5 100644
>--- a/arch/x86/include/asm/vmx.h
>+++ b/arch/x86/include/asm/vmx.h
>@@ -280,12 +280,28 @@ enum vmcs_field {
> GUEST_BNDCFGS_HIGH = 0x00002813,
> GUEST_IA32_RTIT_CTL = 0x00002814,
> GUEST_IA32_RTIT_CTL_HIGH = 0x00002815,
>+ GUEST_IA32_FRED_CONFIG = 0x0000281a,
>+ GUEST_IA32_FRED_RSP1 = 0x0000281c,
>+ GUEST_IA32_FRED_RSP2 = 0x0000281e,
>+ GUEST_IA32_FRED_RSP3 = 0x00002820,
>+ GUEST_IA32_FRED_STKLVLS = 0x00002822,
>+ GUEST_IA32_FRED_SSP1 = 0x00002824,
>+ GUEST_IA32_FRED_SSP2 = 0x00002826,
>+ GUEST_IA32_FRED_SSP3 = 0x00002828,
> HOST_IA32_PAT = 0x00002c00,
> HOST_IA32_PAT_HIGH = 0x00002c01,
> HOST_IA32_EFER = 0x00002c02,
> HOST_IA32_EFER_HIGH = 0x00002c03,
> HOST_IA32_PERF_GLOBAL_CTRL = 0x00002c04,
> HOST_IA32_PERF_GLOBAL_CTRL_HIGH = 0x00002c05,
>+ HOST_IA32_FRED_CONFIG = 0x00002c08,
>+ HOST_IA32_FRED_RSP1 = 0x00002c0a,
>+ HOST_IA32_FRED_RSP2 = 0x00002c0c,
>+ HOST_IA32_FRED_RSP3 = 0x00002c0e,
>+ HOST_IA32_FRED_STKLVLS = 0x00002c10,
>+ HOST_IA32_FRED_SSP1 = 0x00002c12,
>+ HOST_IA32_FRED_SSP2 = 0x00002c14,
>+ HOST_IA32_FRED_SSP3 = 0x00002c16,
> PIN_BASED_VM_EXEC_CONTROL = 0x00004000,
> CPU_BASED_VM_EXEC_CONTROL = 0x00004002,
> EXCEPTION_BITMAP = 0x00004004,
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index d58ed2d3d379..b7b772183ee4 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -1470,6 +1470,18 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
> (unsigned long)(cpu_entry_stack(cpu) + 1));
> }
>
>+#ifdef CONFIG_X86_64
is this #ifdeffery neccesary?
I assume kvm_cpu_cap_has(X86_FEATURE_FRED) is always false for !CONFIG_X86_64.
Looks most of FRED changes in core kernel don't have such #ifdeffery.
>+ /* Per-CPU FRED MSRs */
Please explain why these six MSRs are updated here and why only they are updated in this
comment.
>+ if (kvm_cpu_cap_has(X86_FEATURE_FRED)) {
>+ vmcs_write64(HOST_IA32_FRED_RSP1, read_msr(MSR_IA32_FRED_RSP1));
>+ vmcs_write64(HOST_IA32_FRED_RSP2, read_msr(MSR_IA32_FRED_RSP2));
>+ vmcs_write64(HOST_IA32_FRED_RSP3, read_msr(MSR_IA32_FRED_RSP3));
>+ vmcs_write64(HOST_IA32_FRED_SSP1, read_msr(MSR_IA32_FRED_SSP1));
>+ vmcs_write64(HOST_IA32_FRED_SSP2, read_msr(MSR_IA32_FRED_SSP2));
>+ vmcs_write64(HOST_IA32_FRED_SSP3, read_msr(MSR_IA32_FRED_SSP3));
>+ }
>+#endif
>+
> vmx->loaded_vmcs->cpu = cpu;
> }
> }
>@@ -4321,6 +4333,15 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
> */
> vmcs_write16(HOST_DS_SELECTOR, 0);
> vmcs_write16(HOST_ES_SELECTOR, 0);
>+
>+ /*
>+ * FRED MSRs are per-cpu, however FRED CONFIG and STKLVLS MSRs
>+ * are the same on all CPUs, thus they are initialized here.
>+ */
>+ if (kvm_cpu_cap_has(X86_FEATURE_FRED)) {
>+ vmcs_write64(HOST_IA32_FRED_CONFIG, read_msr(MSR_IA32_FRED_CONFIG));
>+ vmcs_write64(HOST_IA32_FRED_STKLVLS, read_msr(MSR_IA32_FRED_STKLVLS));
>+ }
> #else
> vmcs_write16(HOST_DS_SELECTOR, __KERNEL_DS); /* 22.2.4 */
> vmcs_write16(HOST_ES_SELECTOR, __KERNEL_DS); /* 22.2.4 */
>@@ -4865,6 +4886,19 @@ static void __vmx_vcpu_reset(struct kvm_vcpu *vcpu)
> */
> vmx->pi_desc.nv = POSTED_INTR_VECTOR;
> vmx->pi_desc.sn = 1;
>+
>+#ifdef CONFIG_X86_64
ditto
>+ if (kvm_cpu_cap_has(X86_FEATURE_FRED)) {
>+ vmcs_write64(GUEST_IA32_FRED_CONFIG, 0);
>+ vmcs_write64(GUEST_IA32_FRED_RSP1, 0);
>+ vmcs_write64(GUEST_IA32_FRED_RSP2, 0);
>+ vmcs_write64(GUEST_IA32_FRED_RSP3, 0);
>+ vmcs_write64(GUEST_IA32_FRED_STKLVLS, 0);
>+ vmcs_write64(GUEST_IA32_FRED_SSP1, 0);
>+ vmcs_write64(GUEST_IA32_FRED_SSP2, 0);
>+ vmcs_write64(GUEST_IA32_FRED_SSP3, 0);
>+ }
>+#endif
> }
>
> static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>--
>2.43.0
>
>
Powered by blists - more mailing lists