On Wed, Jun 13, 2018 at 06:17:59PM -0700, Junaid Shahid wrote: > On 06/13/2018 11:28 AM, Sean Christopherson wrote: > > Tangentially related, I think we should expose INVPCID even when PCID > > is not enabled in the guest. vmx_compute_secondary_exec_control() > > disables INVPCID when PCID is disabled, but there is actually a use > > case for INVPCID without PCID, e.g. __native_flush_tlb_global() in > > the Linux kernel uses the global variant instead of toggling CR4.PGE > > regardless of whether PCID is enabled/supported. > > Is it actually legal for a CPU to have support for INVPCID and not have > support for PCIDs? I believe that the code already supports INVPCID if > guest PCIDs are supported but disabled (via CR4.PCIDE=0). But having > CPUID report INVPCID support and not report PCID support would seem > rather strange, even if it is technically legal. It's legal in the sense that there's nothing in the architecture that mandates PCID be supported if INVPCID is supported. Even variants 0 and 1 are usable when PCID is not supported as INVPCID only #GPs in those cases if the target PCID!=0, i.e. INVPCID essentially becomes a superset of INVLPG in that case. A non-buggy kernel should only emit INVPCID with a target PCID!=0 when CR4.PCIDE==1, so exposing INVPCID but no PCID shouldn't introduce kernel instability unless the guest explicitly asserts "!INVPCID || PCID". I agree that it's a strange config, and maybe it doesn't make sense to take the risk of breaking guest kernels for little to no benefit. > > + skip_emulated_instruction(vcpu); > > + return 1; > > These should all be "return kvm_skip_emulated_instruction(vcpu)". > > Thanks for catching that. I failed to notice the change when porting over the patches from an older kernel. > > Thanks, > Junaid
