Setup the global vmcs_config for FRED:
1) Add VM_ENTRY_LOAD_IA32_FRED to KVM_OPTIONAL_VMX_VM_ENTRY_CONTROLS to
have a FRED CPU load guest FRED MSRs from VMCS upon VM entry.
2) Add SECONDARY_VM_EXIT_SAVE_IA32_FRED to
KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU save
guest FRED MSRs to VMCS during VM exit.
3) add SECONDARY_VM_EXIT_LOAD_IA32_FRED to
KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU load
host FRED MSRs from VMCS during VM exit.
Also add sanity checks to make sure FRED VM entry/exit controls can be
set on a FRED CPU.
Tested-by: Shan Kang <shan.kang@intel.com>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
arch/x86/include/asm/vmx.h | 3 +++
arch/x86/kvm/vmx/vmx.c | 19 ++++++++++++++++++-
arch/x86/kvm/vmx/vmx.h | 7 +++++--
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 4d4177ec802c..41796a733bc9 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -106,6 +106,8 @@
#define VM_EXIT_PT_CONCEAL_PIP 0x01000000
#define VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000
#define VM_EXIT_ACTIVATE_SECONDARY_CONTROLS 0x80000000
+#define SECONDARY_VM_EXIT_SAVE_IA32_FRED 0x00000001
+#define SECONDARY_VM_EXIT_LOAD_IA32_FRED 0x00000002
#define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR 0x00036dff
@@ -119,6 +121,7 @@
#define VM_ENTRY_LOAD_BNDCFGS 0x00010000
#define VM_ENTRY_PT_CONCEAL_PIP 0x00020000
#define VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000
+#define VM_ENTRY_LOAD_IA32_FRED 0x00800000
#define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR 0x000011ff
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index df769207cbe0..9186f41974ab 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2694,10 +2694,27 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
_vmexit_control &= ~x_ctrl;
}
- if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
+ if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
_secondary_vmexit_control =
adjust_vmx_controls64(KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS,
MSR_IA32_VMX_EXIT_CTLS2);
+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
+ !(_secondary_vmexit_control & SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
+ _secondary_vmexit_control & SECONDARY_VM_EXIT_LOAD_IA32_FRED)) {
+ pr_warn_once("FRED enabled but no VMX VM-Exit {SAVE,LOAD}_IA32_FRED controls: %llx\n",
+ _secondary_vmexit_control);
+ if (error_on_inconsistent_vmcs_config)
+ return -EIO;
+ }
+ }
+
+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
+ !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
+ pr_warn_once("FRED enabled but no VMX VM-Entry LOAD_IA32_FRED control: %x\n",
+ _vmentry_control);
+ if (error_on_inconsistent_vmcs_config)
+ return -EIO;
+ }
rdmsrl(MSR_IA32_VMX_BASIC, basic_msr);
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 99a0f6783085..f8c02bd37069 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -480,7 +480,8 @@ static inline u8 vmx_get_rvi(void)
VM_ENTRY_LOAD_IA32_EFER | \
VM_ENTRY_LOAD_BNDCFGS | \
VM_ENTRY_PT_CONCEAL_PIP | \
- VM_ENTRY_LOAD_IA32_RTIT_CTL)
+ VM_ENTRY_LOAD_IA32_RTIT_CTL | \
+ VM_ENTRY_LOAD_IA32_FRED)
#define __KVM_REQUIRED_VMX_VM_EXIT_CONTROLS \
(VM_EXIT_SAVE_DEBUG_CONTROLS | \
@@ -506,7 +507,9 @@ static inline u8 vmx_get_rvi(void)
VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
#define KVM_REQUIRED_VMX_SECONDARY_VM_EXIT_CONTROLS (0)
-#define KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS (0)
+#define KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS \
+ (SECONDARY_VM_EXIT_SAVE_IA32_FRED | \
+ SECONDARY_VM_EXIT_LOAD_IA32_FRED)
#define KVM_REQUIRED_VMX_PIN_BASED_VM_EXEC_CONTROL \
(PIN_BASED_EXT_INTR_MASK | \
--
2.42.0
On 8.11.23 г. 20:29 ч., Xin Li wrote:
> Setup the global vmcs_config for FRED:
> 1) Add VM_ENTRY_LOAD_IA32_FRED to KVM_OPTIONAL_VMX_VM_ENTRY_CONTROLS to
> have a FRED CPU load guest FRED MSRs from VMCS upon VM entry.
> 2) Add SECONDARY_VM_EXIT_SAVE_IA32_FRED to
> KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU save
> guest FRED MSRs to VMCS during VM exit.
> 3) add SECONDARY_VM_EXIT_LOAD_IA32_FRED to
> KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU load
> host FRED MSRs from VMCS during VM exit.
>
> Also add sanity checks to make sure FRED VM entry/exit controls can be
> set on a FRED CPU.
>
> Tested-by: Shan Kang <shan.kang@intel.com>
> Signed-off-by: Xin Li <xin3.li@intel.com>
> ---
> arch/x86/include/asm/vmx.h | 3 +++
> arch/x86/kvm/vmx/vmx.c | 19 ++++++++++++++++++-
> arch/x86/kvm/vmx/vmx.h | 7 +++++--
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 4d4177ec802c..41796a733bc9 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -106,6 +106,8 @@
> #define VM_EXIT_PT_CONCEAL_PIP 0x01000000
> #define VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000
> #define VM_EXIT_ACTIVATE_SECONDARY_CONTROLS 0x80000000
> +#define SECONDARY_VM_EXIT_SAVE_IA32_FRED 0x00000001
> +#define SECONDARY_VM_EXIT_LOAD_IA32_FRED 0x00000002
>
> #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR 0x00036dff
>
> @@ -119,6 +121,7 @@
> #define VM_ENTRY_LOAD_BNDCFGS 0x00010000
> #define VM_ENTRY_PT_CONCEAL_PIP 0x00020000
> #define VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000
> +#define VM_ENTRY_LOAD_IA32_FRED 0x00800000
>
> #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR 0x000011ff
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index df769207cbe0..9186f41974ab 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2694,10 +2694,27 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> _vmexit_control &= ~x_ctrl;
> }
>
> - if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
> + if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
> _secondary_vmexit_control =
> adjust_vmx_controls64(KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS,
> MSR_IA32_VMX_EXIT_CTLS2);
> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> + !(_secondary_vmexit_control & SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
> + _secondary_vmexit_control & SECONDARY_VM_EXIT_LOAD_IA32_FRED)) {
Can those checks actually trigger? I.e if FEATURE_FRED is set it means
the CPU implements the FRED spec. According to the spec it's guaranteed
that VMX_EXIT_CTLS2 will contain those bits set to 1. So aren't those
checks superfluous?
> + pr_warn_once("FRED enabled but no VMX VM-Exit {SAVE,LOAD}_IA32_FRED controls: %llx\n",
> + _secondary_vmexit_control);
> + if (error_on_inconsistent_vmcs_config)
> + return -EIO;
> + }
> + }
> +
> + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> + !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
DITTO
<snip>
> > - if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
> > + if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
> > _secondary_vmexit_control =
> >
> adjust_vmx_controls64(KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CO
> NTROLS,
> > MSR_IA32_VMX_EXIT_CTLS2);
> > + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> > + !(_secondary_vmexit_control &
> SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
> > + _secondary_vmexit_control &
> SECONDARY_VM_EXIT_LOAD_IA32_FRED)) {
>
> Can those checks actually trigger? I.e if FEATURE_FRED is set it means
> the CPU implements the FRED spec. According to the spec it's guaranteed
> that VMX_EXIT_CTLS2 will contain those bits set to 1. So aren't those
> checks superfluous?
Such checks are for nVMX FRED.
> > + if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> > + !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
> DITTO
>
> <snip>
On Wed, Nov 08, 2023 at 10:29:45AM -0800, Xin Li wrote:
>Setup the global vmcs_config for FRED:
>1) Add VM_ENTRY_LOAD_IA32_FRED to KVM_OPTIONAL_VMX_VM_ENTRY_CONTROLS to
> have a FRED CPU load guest FRED MSRs from VMCS upon VM entry.
>2) Add SECONDARY_VM_EXIT_SAVE_IA32_FRED to
> KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU save
> guest FRED MSRs to VMCS during VM exit.
>3) add SECONDARY_VM_EXIT_LOAD_IA32_FRED to
> KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU load
> host FRED MSRs from VMCS during VM exit.
>
>Also add sanity checks to make sure FRED VM entry/exit controls can be
>set on a FRED CPU.
>
>Tested-by: Shan Kang <shan.kang@intel.com>
>Signed-off-by: Xin Li <xin3.li@intel.com>
>---
> arch/x86/include/asm/vmx.h | 3 +++
> arch/x86/kvm/vmx/vmx.c | 19 ++++++++++++++++++-
> arch/x86/kvm/vmx/vmx.h | 7 +++++--
> 3 files changed, 26 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
>index 4d4177ec802c..41796a733bc9 100644
>--- a/arch/x86/include/asm/vmx.h
>+++ b/arch/x86/include/asm/vmx.h
>@@ -106,6 +106,8 @@
> #define VM_EXIT_PT_CONCEAL_PIP 0x01000000
> #define VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000
> #define VM_EXIT_ACTIVATE_SECONDARY_CONTROLS 0x80000000
>+#define SECONDARY_VM_EXIT_SAVE_IA32_FRED 0x00000001
>+#define SECONDARY_VM_EXIT_LOAD_IA32_FRED 0x00000002
>
> #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR 0x00036dff
>
>@@ -119,6 +121,7 @@
> #define VM_ENTRY_LOAD_BNDCFGS 0x00010000
> #define VM_ENTRY_PT_CONCEAL_PIP 0x00020000
> #define VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000
>+#define VM_ENTRY_LOAD_IA32_FRED 0x00800000
>
> #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR 0x000011ff
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index df769207cbe0..9186f41974ab 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -2694,10 +2694,27 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> _vmexit_control &= ~x_ctrl;
> }
>
>- if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
>+ if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
> _secondary_vmexit_control =
> adjust_vmx_controls64(KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS,
> MSR_IA32_VMX_EXIT_CTLS2);
>+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
>+ !(_secondary_vmexit_control & SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
>+ _secondary_vmexit_control & SECONDARY_VM_EXIT_LOAD_IA32_FRED)) {
>+ pr_warn_once("FRED enabled but no VMX VM-Exit {SAVE,LOAD}_IA32_FRED controls: %llx\n",
>+ _secondary_vmexit_control);
if there is no VM_EXIT_ACTIVATE_SECONDARY_CONTROLS, shouldn't we also emit this
warning?
>+ if (error_on_inconsistent_vmcs_config)
>+ return -EIO;
>+ }
>+ }
>+
>+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
>+ !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
>+ pr_warn_once("FRED enabled but no VMX VM-Entry LOAD_IA32_FRED control: %x\n",
>+ _vmentry_control);
Can we just hide FRED from guests like what KVM does for other features which
have similar dependencies? see vmx_set_cpu_caps().
On Thu, Nov 09, 2023, Chao Gao wrote:
> On Wed, Nov 08, 2023 at 10:29:45AM -0800, Xin Li wrote:
> >Setup the global vmcs_config for FRED:
> >1) Add VM_ENTRY_LOAD_IA32_FRED to KVM_OPTIONAL_VMX_VM_ENTRY_CONTROLS to
> > have a FRED CPU load guest FRED MSRs from VMCS upon VM entry.
> >2) Add SECONDARY_VM_EXIT_SAVE_IA32_FRED to
> > KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU save
> > guest FRED MSRs to VMCS during VM exit.
> >3) add SECONDARY_VM_EXIT_LOAD_IA32_FRED to
> > KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS to have a FRED CPU load
> > host FRED MSRs from VMCS during VM exit.
> >
> >Also add sanity checks to make sure FRED VM entry/exit controls can be
> >set on a FRED CPU.
> >
> >Tested-by: Shan Kang <shan.kang@intel.com>
> >Signed-off-by: Xin Li <xin3.li@intel.com>
> >---
> > arch/x86/include/asm/vmx.h | 3 +++
> > arch/x86/kvm/vmx/vmx.c | 19 ++++++++++++++++++-
> > arch/x86/kvm/vmx/vmx.h | 7 +++++--
> > 3 files changed, 26 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> >index 4d4177ec802c..41796a733bc9 100644
> >--- a/arch/x86/include/asm/vmx.h
> >+++ b/arch/x86/include/asm/vmx.h
> >@@ -106,6 +106,8 @@
> > #define VM_EXIT_PT_CONCEAL_PIP 0x01000000
> > #define VM_EXIT_CLEAR_IA32_RTIT_CTL 0x02000000
> > #define VM_EXIT_ACTIVATE_SECONDARY_CONTROLS 0x80000000
> >+#define SECONDARY_VM_EXIT_SAVE_IA32_FRED 0x00000001
> >+#define SECONDARY_VM_EXIT_LOAD_IA32_FRED 0x00000002
> >
> > #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR 0x00036dff
> >
> >@@ -119,6 +121,7 @@
> > #define VM_ENTRY_LOAD_BNDCFGS 0x00010000
> > #define VM_ENTRY_PT_CONCEAL_PIP 0x00020000
> > #define VM_ENTRY_LOAD_IA32_RTIT_CTL 0x00040000
> >+#define VM_ENTRY_LOAD_IA32_FRED 0x00800000
> >
> > #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR 0x000011ff
> >
> >diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> >index df769207cbe0..9186f41974ab 100644
> >--- a/arch/x86/kvm/vmx/vmx.c
> >+++ b/arch/x86/kvm/vmx/vmx.c
> >@@ -2694,10 +2694,27 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> > _vmexit_control &= ~x_ctrl;
> > }
> >
> >- if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS)
> >+ if (_vmexit_control & VM_EXIT_ACTIVATE_SECONDARY_CONTROLS) {
> > _secondary_vmexit_control =
> > adjust_vmx_controls64(KVM_OPTIONAL_VMX_SECONDARY_VM_EXIT_CONTROLS,
> > MSR_IA32_VMX_EXIT_CTLS2);
> >+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> >+ !(_secondary_vmexit_control & SECONDARY_VM_EXIT_SAVE_IA32_FRED &&
> >+ _secondary_vmexit_control & SECONDARY_VM_EXIT_LOAD_IA32_FRED)) {
> >+ pr_warn_once("FRED enabled but no VMX VM-Exit {SAVE,LOAD}_IA32_FRED controls: %llx\n",
> >+ _secondary_vmexit_control);
>
> if there is no VM_EXIT_ACTIVATE_SECONDARY_CONTROLS, shouldn't we also emit this
> warning?
>
> >+ if (error_on_inconsistent_vmcs_config)
> >+ return -EIO;
> >+ }
> >+ }
> >+
> >+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> >+ !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
> >+ pr_warn_once("FRED enabled but no VMX VM-Entry LOAD_IA32_FRED control: %x\n",
> >+ _vmentry_control);
>
> Can we just hide FRED from guests like what KVM does for other features which
> have similar dependencies? see vmx_set_cpu_caps().
Both of these warnings should simply be dropped. The error_on_inconsistent_vmcs_config
stuff is for inconsistencies within the allowed VMCS fields. Having a feature
that is supported in bare metal but not virtualized is perfectly legal, if
uncommon.
What *is* needed is for KVM to refuse to virtualize FRED if the entry/exit controls
aren't consistent. E.g. if at least one control is present, and at least one
control is missing. I.e. KVM needs a version of vmcs_entry_exit_pairs that can
deal with SECONDAY_VM_EXIT controls. I'll circle back to this when I give the
series a proper review, which is going to be 3+ weeks.
> > >+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> > >+ !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
> > >+ pr_warn_once("FRED enabled but no VMX VM-Entry
> LOAD_IA32_FRED control: %x\n",
> > >+ _vmentry_control);
> >
> > Can we just hide FRED from guests like what KVM does for other
> > features which have similar dependencies? see vmx_set_cpu_caps().
>
> Both of these warnings should simply be dropped. The
> error_on_inconsistent_vmcs_config stuff is for inconsistencies within the allowed
> VMCS fields. Having a feature that is supported in bare metal but not virtualized
> is perfectly legal, if uncommon.
I deliberately keep it, at least for now, because these checks are helpful
during the development of nVMX FRED. It will be helpful for other VMMs
being developed/tested on KVM.
> What *is* needed is for KVM to refuse to virtualize FRED if the entry/exit controls
> aren't consistent. E.g. if at least one control is present, and at least one
> control is missing. I.e. KVM needs a version of vmcs_entry_exit_pairs that can
> deal with SECONDAY_VM_EXIT controls.
I agree there are better ways. But maybe after or before VMX FRED.
> I'll circle back to this when I give the
> series a proper review, which is going to be 3+ weeks.
The traffic in KVM mailing list is surprisingly high recently. So that is
totally expected.
On Fri, Nov 10, 2023, Xin3 Li wrote:
> > > >+ if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> > > >+ !(_vmentry_control & VM_ENTRY_LOAD_IA32_FRED)) {
> > > >+ pr_warn_once("FRED enabled but no VMX VM-Entry
> > LOAD_IA32_FRED control: %x\n",
> > > >+ _vmentry_control);
> > >
> > > Can we just hide FRED from guests like what KVM does for other
> > > features which have similar dependencies? see vmx_set_cpu_caps().
> >
> > Both of these warnings should simply be dropped. The
> > error_on_inconsistent_vmcs_config stuff is for inconsistencies within the allowed
> > VMCS fields. Having a feature that is supported in bare metal but not virtualized
> > is perfectly legal, if uncommon.
>
> I deliberately keep it, at least for now, because these checks are helpful
> during the development of nVMX FRED. It will be helpful for other VMMs
> being developed/tested on KVM.
No, remove it. It's architecturally legal for a CPU to support a feature in bare
metal but not provide virtualization support.
> > What *is* needed is for KVM to refuse to virtualize FRED if the entry/exit controls
> > aren't consistent. E.g. if at least one control is present, and at least one
> > control is missing. I.e. KVM needs a version of vmcs_entry_exit_pairs that can
> > deal with SECONDAY_VM_EXIT controls.
>
> I agree there are better ways. But maybe after or before VMX FRED.
Uh, no. This is not optional. FRED is the first feature that uses SECONDAY_VM_EXIT
controls, so FRED gets the honor of adding the necessary infrastructure support.
> > > > Can we just hide FRED from guests like what KVM does for other
> > > > features which have similar dependencies? see vmx_set_cpu_caps().
> > >
> > > Both of these warnings should simply be dropped. The
> > > error_on_inconsistent_vmcs_config stuff is for inconsistencies within the
> allowed
> > > VMCS fields. Having a feature that is supported in bare metal but not
> virtualized
> > > is perfectly legal, if uncommon.
> >
> > I deliberately keep it, at least for now, because these checks are helpful
> > during the development of nVMX FRED. It will be helpful for other VMMs
> > being developed/tested on KVM.
>
> No, remove it. It's architecturally legal for a CPU to support a feature in bare
> metal but not provide virtualization support.
Like the stage when native Linux has FRED support while KVM not yet?
> > > What *is* needed is for KVM to refuse to virtualize FRED if the entry/exit
> controls
> > > aren't consistent. E.g. if at least one control is present, and at least one
> > > control is missing. I.e. KVM needs a version of vmcs_entry_exit_pairs that can
> > > deal with SECONDAY_VM_EXIT controls.
> >
> > I agree there are better ways. But maybe after or before VMX FRED.
>
> Uh, no. This is not optional. FRED is the first feature that uses
> SECONDAY_VM_EXIT
> controls, so FRED gets the honor of adding the necessary infrastructure support.
The 2nd VM exit controls is a must for FRED, so I should do it.
I think you mean the consistency checks can be done in a better way (which
is not just for FRED controls consistency checks). No?
Thanks!
Xin
© 2016 - 2025 Red Hat, Inc.