diff mbox series

[01/12] KVM: x86: Add a framework for enabling KVM-governed x86 features

Message ID 20230217231022.816138-2-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: Add "governed" X86_FEATURE framework | expand

Commit Message

Sean Christopherson Feb. 17, 2023, 11:10 p.m. UTC
Introduce yet another X86_FEATURE flag framework to manage and cache KVM
governed features (for lack of a better term).  "Governed" in this case
means that KVM has some level of involvement and/or vested interest in
whether or not an X86_FEATURE can be used by the guest.  The intent of the
framework is twofold: to simplify caching of guest CPUID flags that KVM
needs to frequently query, and to add clarity to such caching, e.g. it
isn't immediately obvious that SVM's bundle of flags for "optional nested]
SVM features" track whether or not a flag is exposed to L1.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/include/asm/kvm_host.h  | 11 +++++++
 arch/x86/kvm/cpuid.c             |  2 ++
 arch/x86/kvm/cpuid.h             | 51 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/governed_features.h |  9 ++++++
 4 files changed, 73 insertions(+)
 create mode 100644 arch/x86/kvm/governed_features.h

Comments

Vitaly Kuznetsov Feb. 21, 2023, 5:12 p.m. UTC | #1
Sean Christopherson <seanjc@google.com> writes:

> Introduce yet another X86_FEATURE flag framework to manage and cache KVM
> governed features (for lack of a better term).  "Governed" in this case
> means that KVM has some level of involvement and/or vested interest in
> whether or not an X86_FEATURE can be used by the guest.  The intent of the
> framework is twofold: to simplify caching of guest CPUID flags that KVM
> needs to frequently query, and to add clarity to such caching, e.g. it
> isn't immediately obvious that SVM's bundle of flags for "optional nested]

Nit: unneeded ']'

> SVM features" track whether or not a flag is exposed to L1.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h  | 11 +++++++
>  arch/x86/kvm/cpuid.c             |  2 ++
>  arch/x86/kvm/cpuid.h             | 51 ++++++++++++++++++++++++++++++++
>  arch/x86/kvm/governed_features.h |  9 ++++++
>  4 files changed, 73 insertions(+)
>  create mode 100644 arch/x86/kvm/governed_features.h
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 792a6037047a..cd660de02f7b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -835,6 +835,17 @@ struct kvm_vcpu_arch {
>  	struct kvm_cpuid_entry2 *cpuid_entries;
>  	struct kvm_hypervisor_cpuid kvm_cpuid;
>  
> +	/*
> +	 * Track whether or not the guest is allowed to use features that are
> +	 * governed by KVM, where "governed" means KVM needs to manage state
> +	 * and/or explicitly enable the feature in hardware.  Typically, but
> +	 * not always, governed features can be used by the guest if and only
> +	 * if both KVM and userspace want to expose the feature to the guest.
> +	 */
> +	struct {
> +		u32 enabled;
> +	} governed_features;
> +
>  	u64 reserved_gpa_bits;
>  	int maxphyaddr;
>  
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 8f8edeaf8177..013fdc27fc8f 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -335,6 +335,8 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
>  	struct kvm_lapic *apic = vcpu->arch.apic;
>  	struct kvm_cpuid_entry2 *best;
>  
> +	vcpu->arch.governed_features.enabled = 0;
> +
>  	best = kvm_find_cpuid_entry(vcpu, 1);
>  	if (best && apic) {
>  		if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index b1658c0de847..f61a2106ba90 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -232,4 +232,55 @@ static __always_inline bool guest_pv_has(struct kvm_vcpu *vcpu,
>  	return vcpu->arch.pv_cpuid.features & (1u << kvm_feature);
>  }
>  
> +enum kvm_governed_features {
> +#define KVM_GOVERNED_FEATURE(x) KVM_GOVERNED_##x,
> +#include "governed_features.h"
> +	KVM_NR_GOVERNED_FEATURES
> +};
> +
> +static __always_inline int kvm_governed_feature_index(unsigned int x86_feature)
> +{
> +	switch (x86_feature) {
> +#define KVM_GOVERNED_FEATURE(x) case x: return KVM_GOVERNED_##x;
> +#include "governed_features.h"
> +	default:
> +		return -1;
> +	}
> +}
> +
> +static __always_inline int kvm_is_governed_feature(unsigned int x86_feature)
> +{
> +	return kvm_governed_feature_index(x86_feature) >= 0;
> +}
> +
> +static __always_inline u32 kvm_governed_feature_bit(unsigned int x86_feature)
> +{
> +	int index = kvm_governed_feature_index(x86_feature);
> +
> +	BUILD_BUG_ON(index < 0);
> +	return BIT(index);
> +}
> +
> +static __always_inline void kvm_governed_feature_set(struct kvm_vcpu *vcpu,
> +						     unsigned int x86_feature)
> +{
> +	BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES >
> +		     sizeof(vcpu->arch.governed_features.enabled) * BITS_PER_BYTE);
> +
> +	vcpu->arch.governed_features.enabled |= kvm_governed_feature_bit(x86_feature);
> +}
> +
> +static __always_inline void kvm_governed_feature_check_and_set(struct kvm_vcpu *vcpu,
> +							       unsigned int x86_feature)
> +{
> +	if (guest_cpuid_has(vcpu, x86_feature))
> +		kvm_governed_feature_set(vcpu, x86_feature);
> +}
> +
> +static __always_inline bool guest_can_use(struct kvm_vcpu *vcpu,
> +					  unsigned int x86_feature)
> +{
> +	return vcpu->arch.governed_features.enabled & kvm_governed_feature_bit(x86_feature);
> +}
> +
>  #endif
> diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
> new file mode 100644
> index 000000000000..40ce8e6608cd
> --- /dev/null
> +++ b/arch/x86/kvm/governed_features.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#if !defined(KVM_GOVERNED_FEATURE) || defined(KVM_GOVERNED_X86_FEATURE)
> +BUILD_BUG()
> +#endif
> +
> +#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x)
> +
> +#undef KVM_GOVERNED_X86_FEATURE
> +#undef KVM_GOVERNED_FEATURE
Binbin Wu June 29, 2023, 2:40 a.m. UTC | #2
On 2/18/2023 7:10 AM, Sean Christopherson wrote:
> Introduce yet another X86_FEATURE flag framework to manage and cache KVM
> governed features (for lack of a better term).  "Governed" in this case
> means that KVM has some level of involvement and/or vested interest in
> whether or not an X86_FEATURE can be used by the guest.  The intent of the
> framework is twofold: to simplify caching of guest CPUID flags that KVM
> needs to frequently query, and to add clarity to such caching, e.g. it
> isn't immediately obvious that SVM's bundle of flags for "optional nested]
spare ]

> SVM features" track whether or not a flag is exposed to L1.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/include/asm/kvm_host.h  | 11 +++++++
>   arch/x86/kvm/cpuid.c             |  2 ++
>   arch/x86/kvm/cpuid.h             | 51 ++++++++++++++++++++++++++++++++
>   arch/x86/kvm/governed_features.h |  9 ++++++
>   4 files changed, 73 insertions(+)
>   create mode 100644 arch/x86/kvm/governed_features.h
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 792a6037047a..cd660de02f7b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -835,6 +835,17 @@ struct kvm_vcpu_arch {
>   	struct kvm_cpuid_entry2 *cpuid_entries;
>   	struct kvm_hypervisor_cpuid kvm_cpuid;
>   
> +	/*
> +	 * Track whether or not the guest is allowed to use features that are
> +	 * governed by KVM, where "governed" means KVM needs to manage state
> +	 * and/or explicitly enable the feature in hardware.  Typically, but
> +	 * not always, governed features can be used by the guest if and only
> +	 * if both KVM and userspace want to expose the feature to the guest.
> +	 */
> +	struct {
> +		u32 enabled;
Although there are some guidances/preconditions of using the framework,
is it possible that u32 will be ran out quickly after people starts to 
use the framework?

Of course, I noticed there is build  bug check on the length, it should 
be OK to increase the length when needed.

> +	} governed_features;
> +
>   	u64 reserved_gpa_bits;
>   	int maxphyaddr;
>   
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 8f8edeaf8177..013fdc27fc8f 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -335,6 +335,8 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
>   	struct kvm_lapic *apic = vcpu->arch.apic;
>   	struct kvm_cpuid_entry2 *best;
>   
> +	vcpu->arch.governed_features.enabled = 0;
> +
>   	best = kvm_find_cpuid_entry(vcpu, 1);
>   	if (best && apic) {
>   		if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index b1658c0de847..f61a2106ba90 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -232,4 +232,55 @@ static __always_inline bool guest_pv_has(struct kvm_vcpu *vcpu,
>   	return vcpu->arch.pv_cpuid.features & (1u << kvm_feature);
>   }
>   
> +enum kvm_governed_features {
> +#define KVM_GOVERNED_FEATURE(x) KVM_GOVERNED_##x,
> +#include "governed_features.h"
> +	KVM_NR_GOVERNED_FEATURES
> +};
> +
> +static __always_inline int kvm_governed_feature_index(unsigned int x86_feature)
> +{
> +	switch (x86_feature) {
> +#define KVM_GOVERNED_FEATURE(x) case x: return KVM_GOVERNED_##x;
> +#include "governed_features.h"
> +	default:
> +		return -1;
> +	}
> +}
> +
> +static __always_inline int kvm_is_governed_feature(unsigned int x86_feature)
Is it better to use bool instead of int?

> +{
> +	return kvm_governed_feature_index(x86_feature) >= 0;
> +}
> +
> +static __always_inline u32 kvm_governed_feature_bit(unsigned int x86_feature)
> +{
> +	int index = kvm_governed_feature_index(x86_feature);
> +
> +	BUILD_BUG_ON(index < 0);
> +	return BIT(index);
> +}
> +
> +static __always_inline void kvm_governed_feature_set(struct kvm_vcpu *vcpu,
> +						     unsigned int x86_feature)
> +{
> +	BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES >
> +		     sizeof(vcpu->arch.governed_features.enabled) * BITS_PER_BYTE);
> +
> +	vcpu->arch.governed_features.enabled |= kvm_governed_feature_bit(x86_feature);
> +}
> +
> +static __always_inline void kvm_governed_feature_check_and_set(struct kvm_vcpu *vcpu,
> +							       unsigned int x86_feature)
> +{
> +	if (guest_cpuid_has(vcpu, x86_feature))
> +		kvm_governed_feature_set(vcpu, x86_feature);
> +}
> +
> +static __always_inline bool guest_can_use(struct kvm_vcpu *vcpu,
> +					  unsigned int x86_feature)
> +{
> +	return vcpu->arch.governed_features.enabled & kvm_governed_feature_bit(x86_feature);
> +}
> +
>   #endif
> diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
> new file mode 100644
> index 000000000000..40ce8e6608cd
> --- /dev/null
> +++ b/arch/x86/kvm/governed_features.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#if !defined(KVM_GOVERNED_FEATURE) || defined(KVM_GOVERNED_X86_FEATURE)
> +BUILD_BUG()
> +#endif
> +
> +#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x)
> +
> +#undef KVM_GOVERNED_X86_FEATURE
> +#undef KVM_GOVERNED_FEATURE
Sean Christopherson June 29, 2023, 4:26 p.m. UTC | #3
On Thu, Jun 29, 2023, Binbin Wu wrote:
> 
> 
> On 2/18/2023 7:10 AM, Sean Christopherson wrote:
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 792a6037047a..cd660de02f7b 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -835,6 +835,17 @@ struct kvm_vcpu_arch {
> >   	struct kvm_cpuid_entry2 *cpuid_entries;
> >   	struct kvm_hypervisor_cpuid kvm_cpuid;
> > +	/*
> > +	 * Track whether or not the guest is allowed to use features that are
> > +	 * governed by KVM, where "governed" means KVM needs to manage state
> > +	 * and/or explicitly enable the feature in hardware.  Typically, but
> > +	 * not always, governed features can be used by the guest if and only
> > +	 * if both KVM and userspace want to expose the feature to the guest.
> > +	 */
> > +	struct {
> > +		u32 enabled;
> Although there are some guidances/preconditions of using the framework,
> is it possible that u32 will be ran out quickly after people starts to use
> the framework?

It's definitely possible.  And there's no reason to limit this to a u32, I really
have no idea why I did that. 

Ah, it's because "struct kvm_vcpu_arch" is defined in arch/x86/include/asm/kvm_host.h,
and I didn't want to expose governed_features.h in arch/x86/include/asm.  Hrm,
that's really annoying.

Aha!  A better workaround for that conudrum would be to define an explicit "max"
and use that, with a FIXME to call out that this really should use
KVM_NR_GOVERNED_FEATURES directly.  I have aspirations of moving kvm_host.h to
arch/<arch>/kvm, at which point this can be cleaned up by declaring "enum
kvm_governed_features" in kvm_host.h (though it'll likely be named something
like kvm_arch.h at that point).

	/*
	 * FIXME: Drop this macro and use KVM_NR_GOVERNED_FEATURES directly
	 * when "struct kvm_vcpu_arch" is no longer defined in an
	 * arch/x86/include/asm header.  The max is mostly arbitrary, i.e.
	 * can be increased as necessary.
	 */
#define KVM_MAX_NR_GOVERNED_FEATURES BITS_PER_LONG

	/*
	 * Track whether or not the guest is allowed to use features that are
	 * governed by KVM, where "governed" means KVM needs to manage state
	 * and/or explicitly enable the feature in hardware.  Typically, but
	 * not always, governed features can be used by the guest if and only
	 * if both KVM and userspace want to expose the feature to the guest.
	 */
	struct {
		DECLARE_BITMAP(enabled, KVM_MAX_NR_GOVERNED_FEATURES);
	} governed_features;


> Of course, I noticed there is build� bug check on the length, it should be
> OK to increase the length when needed.

> > +static __always_inline int kvm_governed_feature_index(unsigned int x86_feature)
> > +{
> > +	switch (x86_feature) {
> > +#define KVM_GOVERNED_FEATURE(x) case x: return KVM_GOVERNED_##x;
> > +#include "governed_features.h"
> > +	default:
> > +		return -1;
> > +	}
> > +}
> > +
> > +static __always_inline int kvm_is_governed_feature(unsigned int x86_feature)
> Is it better to use bool instead of int?

Yes, this definitely should return a bool.

Thanks!
Chao Gao June 30, 2023, 8:01 a.m. UTC | #4
On Fri, Feb 17, 2023 at 03:10:11PM -0800, Sean Christopherson wrote:
>+static __always_inline void kvm_governed_feature_set(struct kvm_vcpu *vcpu,
>+						     unsigned int x86_feature)
>+{
>+	BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES >
>+		     sizeof(vcpu->arch.governed_features.enabled) * BITS_PER_BYTE);
>+
>+	vcpu->arch.governed_features.enabled |= kvm_governed_feature_bit(x86_feature);
>+}
>+
>+static __always_inline void kvm_governed_feature_check_and_set(struct kvm_vcpu *vcpu,
>+							       unsigned int x86_feature)
>+{
>+	if (guest_cpuid_has(vcpu, x86_feature))

Most callers in this series are conditional on either boot_cpu_has() or some
local variables. Can we convert them to kvm_cpu_cap_has() and incorporate them
within this function? i.e.,

	if (kvm_cpu_cap_has(x86_feature) && guest_cpuid_has(vcpu, x86_feature))


The benefits of doing so are
1. callers needn't repeat

	if (kvm_cpu_cap_has(x86_feature))
		kvm_governed_feature_check_and_set(x86_feature)

2. this fits the idea better that guests can use a governed feature only if host
   supports it _and_ QEMU exposes it to the guest.

>+		kvm_governed_feature_set(vcpu, x86_feature);
>+}
>+
Sean Christopherson June 30, 2023, 3:31 p.m. UTC | #5
On Fri, Jun 30, 2023, Chao Gao wrote:
> On Fri, Feb 17, 2023 at 03:10:11PM -0800, Sean Christopherson wrote:
> >+static __always_inline void kvm_governed_feature_set(struct kvm_vcpu *vcpu,
> >+						     unsigned int x86_feature)
> >+{
> >+	BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES >
> >+		     sizeof(vcpu->arch.governed_features.enabled) * BITS_PER_BYTE);
> >+
> >+	vcpu->arch.governed_features.enabled |= kvm_governed_feature_bit(x86_feature);
> >+}
> >+
> >+static __always_inline void kvm_governed_feature_check_and_set(struct kvm_vcpu *vcpu,
> >+							       unsigned int x86_feature)
> >+{
> >+	if (guest_cpuid_has(vcpu, x86_feature))
> 
> Most callers in this series are conditional on either boot_cpu_has() or some
> local variables. Can we convert them to kvm_cpu_cap_has() and incorporate them
> within this function? i.e.,
> 
> 	if (kvm_cpu_cap_has(x86_feature) && guest_cpuid_has(vcpu, x86_feature))

Hmm, I was going to say "no", as most callers don't check kvm_cpu_cap_has() verbatim,
but it doesn't have to be that way.   The majority of SVM features factor in module
params, but KVM should set the kvm_cpu capability if and only if a feature is supported
in hardware *and* enabled by its module param.

And arguably that's kinda sorta a bug fix, because this

	if (lbrv)
		kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_LBRV);

technically should be 

	if (lbrv && nested)
		kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_LBRV);	

Heh, and it's kinda sorta a bug fix for XSAVES on VMX, because this

	if (cpu_has_vmx_xsaves() && boot_cpu_has(X86_FEATURE_XSAVE) &&
	    guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
		kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);

should technically be

	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES) &&
	    boot_cpu_has(X86_FEATURE_XSAVE) &&
	    guest_cpuid_has(vcpu, X86_FEATURE_XSAVE))
		kvm_governed_feature_check_and_set(vcpu, X86_FEATURE_XSAVES);

> The benefits of doing so are
> 1. callers needn't repeat
> 
> 	if (kvm_cpu_cap_has(x86_feature))
> 		kvm_governed_feature_check_and_set(x86_feature)
> 
> 2. this fits the idea better that guests can use a governed feature only if host
>    supports it _and_ QEMU exposes it to the guest.

Agreed, especially since we'll still have kvm_governed_feature_set() for the
extra special cases.

Thanks for the input!
diff mbox series

Patch

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 792a6037047a..cd660de02f7b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -835,6 +835,17 @@  struct kvm_vcpu_arch {
 	struct kvm_cpuid_entry2 *cpuid_entries;
 	struct kvm_hypervisor_cpuid kvm_cpuid;
 
+	/*
+	 * Track whether or not the guest is allowed to use features that are
+	 * governed by KVM, where "governed" means KVM needs to manage state
+	 * and/or explicitly enable the feature in hardware.  Typically, but
+	 * not always, governed features can be used by the guest if and only
+	 * if both KVM and userspace want to expose the feature to the guest.
+	 */
+	struct {
+		u32 enabled;
+	} governed_features;
+
 	u64 reserved_gpa_bits;
 	int maxphyaddr;
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 8f8edeaf8177..013fdc27fc8f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -335,6 +335,8 @@  static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 	struct kvm_cpuid_entry2 *best;
 
+	vcpu->arch.governed_features.enabled = 0;
+
 	best = kvm_find_cpuid_entry(vcpu, 1);
 	if (best && apic) {
 		if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index b1658c0de847..f61a2106ba90 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -232,4 +232,55 @@  static __always_inline bool guest_pv_has(struct kvm_vcpu *vcpu,
 	return vcpu->arch.pv_cpuid.features & (1u << kvm_feature);
 }
 
+enum kvm_governed_features {
+#define KVM_GOVERNED_FEATURE(x) KVM_GOVERNED_##x,
+#include "governed_features.h"
+	KVM_NR_GOVERNED_FEATURES
+};
+
+static __always_inline int kvm_governed_feature_index(unsigned int x86_feature)
+{
+	switch (x86_feature) {
+#define KVM_GOVERNED_FEATURE(x) case x: return KVM_GOVERNED_##x;
+#include "governed_features.h"
+	default:
+		return -1;
+	}
+}
+
+static __always_inline int kvm_is_governed_feature(unsigned int x86_feature)
+{
+	return kvm_governed_feature_index(x86_feature) >= 0;
+}
+
+static __always_inline u32 kvm_governed_feature_bit(unsigned int x86_feature)
+{
+	int index = kvm_governed_feature_index(x86_feature);
+
+	BUILD_BUG_ON(index < 0);
+	return BIT(index);
+}
+
+static __always_inline void kvm_governed_feature_set(struct kvm_vcpu *vcpu,
+						     unsigned int x86_feature)
+{
+	BUILD_BUG_ON(KVM_NR_GOVERNED_FEATURES >
+		     sizeof(vcpu->arch.governed_features.enabled) * BITS_PER_BYTE);
+
+	vcpu->arch.governed_features.enabled |= kvm_governed_feature_bit(x86_feature);
+}
+
+static __always_inline void kvm_governed_feature_check_and_set(struct kvm_vcpu *vcpu,
+							       unsigned int x86_feature)
+{
+	if (guest_cpuid_has(vcpu, x86_feature))
+		kvm_governed_feature_set(vcpu, x86_feature);
+}
+
+static __always_inline bool guest_can_use(struct kvm_vcpu *vcpu,
+					  unsigned int x86_feature)
+{
+	return vcpu->arch.governed_features.enabled & kvm_governed_feature_bit(x86_feature);
+}
+
 #endif
diff --git a/arch/x86/kvm/governed_features.h b/arch/x86/kvm/governed_features.h
new file mode 100644
index 000000000000..40ce8e6608cd
--- /dev/null
+++ b/arch/x86/kvm/governed_features.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#if !defined(KVM_GOVERNED_FEATURE) || defined(KVM_GOVERNED_X86_FEATURE)
+BUILD_BUG()
+#endif
+
+#define KVM_GOVERNED_X86_FEATURE(x) KVM_GOVERNED_FEATURE(X86_FEATURE_##x)
+
+#undef KVM_GOVERNED_X86_FEATURE
+#undef KVM_GOVERNED_FEATURE