| Message ID | 20200201185218.24473-44-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Introduce KVM cpu caps | expand |
Sean Christopherson <sean.j.christopherson@intel.com> writes: > Add accessor(s) for KVM cpu caps and use said accessor to detect > hardware support for LA57 instead of manually querying CPUID. > > No functional change intended. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/cpuid.h | 13 +++++++++++++ > arch/x86/kvm/x86.c | 2 +- > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index 7b71ae0ca05e..5ce4219d465f 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -274,6 +274,19 @@ static __always_inline void kvm_cpu_cap_set(unsigned x86_feature) > kvm_cpu_caps[x86_leaf] |= __feature_bit(x86_feature); > } > > +static __always_inline u32 kvm_cpu_cap_get(unsigned x86_feature) > +{ > + unsigned x86_leaf = x86_feature / 32; > + > + reverse_cpuid_check(x86_leaf); > + return kvm_cpu_caps[x86_leaf] & __feature_bit(x86_feature); > +} > + > +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) > +{ > + return kvm_cpu_cap_get(x86_feature); > +} I know this works (and I even checked C99 to make sure that it works not by accident) but I have to admit that explicit '!!' conversion to bool always makes me feel safer :-) > + > static __always_inline void kvm_cpu_cap_check_and_set(unsigned x86_feature) > { > if (boot_cpu_has(x86_feature)) > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c5ed199d6cd9..cb40737187a1 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -912,7 +912,7 @@ static u64 kvm_host_cr4_reserved_bits(struct cpuinfo_x86 *c) > { > u64 reserved_bits = __cr4_reserved_bits(cpu_has, c); > > - if (cpuid_ecx(0x7) & feature_bit(LA57)) > + if (kvm_cpu_cap_has(X86_FEATURE_LA57)) > reserved_bits &= ~X86_CR4_LA57; > > if (kvm_x86_ops->umip_emulated()) Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
On Mon, Feb 24, 2020 at 11:08:30PM +0100, Vitaly Kuznetsov wrote: > Sean Christopherson <sean.j.christopherson@intel.com> writes: > > > Add accessor(s) for KVM cpu caps and use said accessor to detect > > hardware support for LA57 instead of manually querying CPUID. > > > > No functional change intended. > > > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > arch/x86/kvm/cpuid.h | 13 +++++++++++++ > > arch/x86/kvm/x86.c | 2 +- > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > > index 7b71ae0ca05e..5ce4219d465f 100644 > > --- a/arch/x86/kvm/cpuid.h > > +++ b/arch/x86/kvm/cpuid.h > > @@ -274,6 +274,19 @@ static __always_inline void kvm_cpu_cap_set(unsigned x86_feature) > > kvm_cpu_caps[x86_leaf] |= __feature_bit(x86_feature); > > } > > > > +static __always_inline u32 kvm_cpu_cap_get(unsigned x86_feature) > > +{ > > + unsigned x86_leaf = x86_feature / 32; > > + > > + reverse_cpuid_check(x86_leaf); > > + return kvm_cpu_caps[x86_leaf] & __feature_bit(x86_feature); > > +} > > + > > +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) > > +{ > > + return kvm_cpu_cap_get(x86_feature); > > +} > > I know this works (and I even checked C99 to make sure that it works not > by accident) but I have to admit that explicit '!!' conversion to bool > always makes me feel safer :-) Eh, the flip side of blasting it everywhere is that people then forget why the pattern exists in the first place and don't understand when it's truly necessary. > > + > > static __always_inline void kvm_cpu_cap_check_and_set(unsigned x86_feature) > > { > > if (boot_cpu_has(x86_feature)) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index c5ed199d6cd9..cb40737187a1 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -912,7 +912,7 @@ static u64 kvm_host_cr4_reserved_bits(struct cpuinfo_x86 *c) > > { > > u64 reserved_bits = __cr4_reserved_bits(cpu_has, c); > > > > - if (cpuid_ecx(0x7) & feature_bit(LA57)) > > + if (kvm_cpu_cap_has(X86_FEATURE_LA57)) > > reserved_bits &= ~X86_CR4_LA57; > > > > if (kvm_x86_ops->umip_emulated()) > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > -- > Vitaly >
On 24/02/20 23:08, Vitaly Kuznetsov wrote: >> + >> +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) >> +{ >> + return kvm_cpu_cap_get(x86_feature); >> +} > I know this works (and I even checked C99 to make sure that it works not > by accident) but I have to admit that explicit '!!' conversion to bool > always makes me feel safer :-) Same here, I don't really like the automagic bool behavior... Paolo
From: Paolo Bonzini > Sent: 25 February 2020 15:12 > On 24/02/20 23:08, Vitaly Kuznetsov wrote: > >> + > >> +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) > >> +{ > >> + return kvm_cpu_cap_get(x86_feature); > >> +} > > I know this works (and I even checked C99 to make sure that it works not > > by accident) but I have to admit that explicit '!!' conversion to bool > > always makes me feel safer :-) > > Same here, I don't really like the automagic bool behavior... I just dislike 'bool'. Conversion of 0/non-zero to 0/1 isn't completely free. And something has to 'give' when the referenced memory location doesn't contain 0 or 1. One very old version of gcc made a complete hash of: bool_var |= function_returning_bool(); I'm not sure what the standard requires nor what current gcc generates - but you want a 'logical or' instruction. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, Feb 25, 2020 at 04:12:28PM +0100, Paolo Bonzini wrote: > On 24/02/20 23:08, Vitaly Kuznetsov wrote: > >> + > >> +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) > >> +{ > >> + return kvm_cpu_cap_get(x86_feature); > >> +} > > I know this works (and I even checked C99 to make sure that it works not > > by accident) but I have to admit that explicit '!!' conversion to bool > > always makes me feel safer :-) > > Same here, I don't really like the automagic bool behavior... Sounds like I need to add '!!'?
On 25/02/20 22:22, Sean Christopherson wrote: >>> I know this works (and I even checked C99 to make sure that it works not >>> by accident) but I have to admit that explicit '!!' conversion to bool >>> always makes me feel safer :-) >> Same here, I don't really like the automagic bool behavior... > Sounds like I need to add '!!'? > Either that or "!= 0", as you prefer. Paolo
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h index 7b71ae0ca05e..5ce4219d465f 100644 --- a/arch/x86/kvm/cpuid.h +++ b/arch/x86/kvm/cpuid.h @@ -274,6 +274,19 @@ static __always_inline void kvm_cpu_cap_set(unsigned x86_feature) kvm_cpu_caps[x86_leaf] |= __feature_bit(x86_feature); } +static __always_inline u32 kvm_cpu_cap_get(unsigned x86_feature) +{ + unsigned x86_leaf = x86_feature / 32; + + reverse_cpuid_check(x86_leaf); + return kvm_cpu_caps[x86_leaf] & __feature_bit(x86_feature); +} + +static __always_inline bool kvm_cpu_cap_has(unsigned x86_feature) +{ + return kvm_cpu_cap_get(x86_feature); +} + static __always_inline void kvm_cpu_cap_check_and_set(unsigned x86_feature) { if (boot_cpu_has(x86_feature)) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c5ed199d6cd9..cb40737187a1 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -912,7 +912,7 @@ static u64 kvm_host_cr4_reserved_bits(struct cpuinfo_x86 *c) { u64 reserved_bits = __cr4_reserved_bits(cpu_has, c); - if (cpuid_ecx(0x7) & feature_bit(LA57)) + if (kvm_cpu_cap_has(X86_FEATURE_LA57)) reserved_bits &= ~X86_CR4_LA57; if (kvm_x86_ops->umip_emulated())
Add accessor(s) for KVM cpu caps and use said accessor to detect hardware support for LA57 instead of manually querying CPUID. No functional change intended. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/cpuid.h | 13 +++++++++++++ arch/x86/kvm/x86.c | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-)