On Fri, Feb 02, 2018 at 06:02:24PM +0000, David Woodhouse wrote: > On Fri, 2018-02-02 at 12:49 -0500, Konrad Rzeszutek Wilk wrote: > > > @@ -625,7 +629,12 @@ static inline int __do_cpuid_ent(struct > > > kvm_cpuid_entry2 *entry, u32 function, > > > if (!g_phys_as) > > > g_phys_as = phys_as; > > > entry->eax = g_phys_as | (virt_as << 8); > > > - entry->ebx = entry->edx = 0; > > > + entry->edx = 0; > > > + /* IBPB isn't necessarily present in hardware cpuid> */ > > > + if (boot_cpu_has(X86_FEATURE_IBPB)) > > > + entry->ebx |= F(IBPB); > > > + entry->ebx &= kvm_cpuid_8000_0008_ebx_x86_features; > > > + cpuid_mask(&entry->ebx, CPUID_8000_0008_EBX); > > > > It is with x86/pti nowadays. I think you can remove that comment. > > In this code we use the actual CPUID instruction, then filter stuff out > of it (with &= kvm_cpuid_XXX_x86_features and then cpuid_mask() to turn > off any bits which were otherwise present in the hardware and *would* > have been supported by KVM, but which the kernel has decided to pretend > are not present. > > Nothing would *set* the IBPB bit though, since that's a "virtual" bit > on Intel hardware. The comment explains why we have that |= F(IBPB), > and if the comment wasn't true, we wouldn't need that code either. But this seems wrong. That is on Intel CPUs we will advertise on AMD leaf that the IBPB feature is available. Shouldn't we just check to see if the machine is AMD before advertising this bit?
