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!
