On Fri, 2018-02-02 at 14:56 -0500, Konrad Rzeszutek Wilk wrote: > 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? No. The AMD feature bits give us more fine-grained support for exposing IBPB or IBRS alone, so we expose those bits on Intel too.
Attachment:
smime.p7s
Description: S/MIME cryptographic signature