On 01/08/2017 13:18, David Hildenbrand wrote: > >>> Can't we rewrite that a little bit, avoiding that "best" handling >>> (introducing guest_cpuid_disable_invpcid() and guest_cpuid_has_invpcid()) >>> >>> bool invpcid_enabled = guest_cpuid_has_pcid(vcpu) && >>> guest_cpuid_has_invpcid(); >>> >>> if (!invpcid_enabled) { >>> secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID; >>> /* make sure there is no no INVPCID without PCID */ >>> guest_cpuid_disable_invpcid(vcpu); >>> } >> >> I don't know... if you need a comment, it means the different structure >> of the code doesn't spare many doubts from the reader. And the code >> doesn't become much simpler since you have to handle "nested" anyway. >> What I tried to do was to mimic as much as possible the rdtscp case, but >> it cannot be exactly the same due to the interaction between PCID and >> INVPCID. > > It's more about the handling of best here, which can be avoided quite > easily as I showed (by encapsulating how cpuids are looked up/modified). Yeah, I don't like either option. :) Luckily there is a second maintainer! Paolo