On Tue, Mar 03, 2020 at 11:14:22AM +0100, Paolo Bonzini wrote: > On 03/03/20 10:48, Jan Kiszka wrote: > >> > >> I don't think this is a particularly useful change. Yes, it's not > >> intuitive but is it more than a matter of documentation (and possibly > >> moving the check_cr_write snippet into a separate function)? > > > > Besides the non obvious return value of the current function, this > > approach also avoids leaving cpuid traces for querying maxphyaddr, which > > is also not very intuitive IMHO. > > There are already other cases where we leave CPUID traces. We can just > stop tracing if check_limit (which should be renamed to from_guest) is > true; there are other internal cases which call ctxt->ops->get_cpuid, > such as vendor_intel, and those should also use check_limit==true and > check the return value of ctxt->ops->get_cpuid. No, the vendor checks that use get_cpuid() shouldn't do check_limit=true, they're looking for an exact match on the vendor. Not that it matters. @check_limit only comes into play on a vendor check if CPUID.0 doesn't exist, and @check_limit only effects the output if CPUID.0 _does_ exist. I.e. the output for CPUID.0 is unaffected by @check_limit.