This series is the first part of replacing the use of literal numbers
(0 and 1) as return values with either true/false or with defines.

This work is a prelude of getting rid of the magic value "1" for
"return to guest". I started in x86 KVM host code doing that and soon
stumbled over lots of other use cases of the magic "1" as return value,
especially in MSR emulation where a comment even implied this "1" was
due to the "return to guest" semantics.

A detailed analysis of all related code paths revealed that there was
indeed a rather clean interface between the functions using the MSR
emulation "1" and those using the "return to guest" "1". 

A few functions just using "0" and "1" instead of bool are changed,
tooi (patches 1-4).

The rest of the series is cleaning up the MSR emulation code by using
new proper defines for return values 0 and 1.

The whole series should not result in any functional change.

Juergen Gross (10):
  KVM: Switch coalesced_mmio_in_range() to return bool
  KVM/x86: Use bool for the err parameter of kvm_complete_insn_gp()
  KVM/x86: Let x86_emulate_ops.set_cr() return a bool
  KVM/x86: Let x86_emulate_ops.set_dr() return a bool
  KVM/x86: Add KVM_MSR_RET_* defines for values 0 and 1
  KVM/x86: Use defines for APIC related MSR emulation
  KVM/x86: Use defines for Hyper-V related MSR emulation
  KVM/x86: Use defines for VMX related MSR emulation
  KVM/x86: Use defines for SVM related MSR emulation
  KVM/x86: Use defines for common related MSR emulation

 arch/x86/include/asm/kvm_host.h |  14 +-
 arch/x86/kvm/emulate.c          |   2 +-
 arch/x86/kvm/hyperv.c           | 110 +++++++-------
 arch/x86/kvm/kvm_emulate.h      |   4 +-
 arch/x86/kvm/lapic.c            |  48 +++----
 arch/x86/kvm/mtrr.c             |  12 +-
 arch/x86/kvm/pmu.c              |  12 +-
 arch/x86/kvm/smm.c              |   2 +-
 arch/x86/kvm/svm/pmu.c          |  12 +-
 arch/x86/kvm/svm/svm.c          |  54 +++----
 arch/x86/kvm/vmx/main.c         |   2 +-
 arch/x86/kvm/vmx/nested.c       |  18 +--
 arch/x86/kvm/vmx/pmu_intel.c    |  20 +--
 arch/x86/kvm/vmx/tdx.c          |  18 +--
 arch/x86/kvm/vmx/tdx.h          |   2 +-
 arch/x86/kvm/vmx/vmx.c          | 122 ++++++++--------
 arch/x86/kvm/x86.c              | 246 ++++++++++++++++----------------
 arch/x86/kvm/x86.h              |  10 +-
 arch/x86/kvm/xen.c              |  14 +-
 virt/kvm/coalesced_mmio.c       |  14 +-
 20 files changed, 372 insertions(+), 364 deletions(-)

-- 
2.51.0

On Fri, Dec 05, 2025, Juergen Gross wrote:
> This series is the first part of replacing the use of literal numbers
> (0 and 1) as return values with either true/false or with defines.

Sorry, but NAK to using true/false.  IMO, it's far worse than 0/1.  At least 0/1
draws from the kernel's 0/-errno approach.  With booleans, the polarity is often
hard to discern without a priori knowledge of the pattern, and even then it can
be confusing.  E.g. for me, returning "true" when .set_{c,d}r() fails is unexpected,
and results in unintuitive code like this:

                if (!kvm_dr6_valid(val))
			return true;

For isolated APIs whose values aren't intented to be propagated back up to the
.handle_exit() call site, I would much rather return 0/-EINVAL.

Do you have a sketch of what the end goal/result will look like?  IIRC, last time
anyone looked at doing this (which was a few years ago, but I don't think KVM has
changed _that_ much), we backed off because a partial conversion would leave KVM
in an unwieldy and somewhat scary state.

> This work is a prelude of getting rid of the magic value "1" for
> "return to guest". I started in x86 KVM host code doing that and soon
> stumbled over lots of other use cases of the magic "1" as return value,
> especially in MSR emulation where a comment even implied this "1" was
> due to the "return to guest" semantics.
> 
> A detailed analysis of all related code paths revealed that there was
> indeed a rather clean interface between the functions using the MSR
> emulation "1" and those using the "return to guest" "1". 

Ya, we've started chipping away at the MSR stuff.  The big challenge is avoiding
subtle ABI changes related to the fixups done by kvm_do_msr_access().

On 05.12.25 15:16, Sean Christopherson wrote:
> On Fri, Dec 05, 2025, Juergen Gross wrote:
>> This series is the first part of replacing the use of literal numbers
>> (0 and 1) as return values with either true/false or with defines.
> 
> Sorry, but NAK to using true/false.  IMO, it's far worse than 0/1.  At least 0/1
> draws from the kernel's 0/-errno approach.  With booleans, the polarity is often
> hard to discern without a priori knowledge of the pattern, and even then it can
> be confusing.  E.g. for me, returning "true" when .set_{c,d}r() fails is unexpected,
> and results in unintuitive code like this:
> 
>                  if (!kvm_dr6_valid(val))
> 			return true;

I don't see "return 1;" being much better here.

> For isolated APIs whose values aren't intented to be propagated back up to the
> .handle_exit() call site, I would much rather return 0/-EINVAL.

Fine with me (I agree this would be more readable).

> Do you have a sketch of what the end goal/result will look like?  IIRC, last time
> anyone looked at doing this (which was a few years ago, but I don't think KVM has
> changed _that_ much), we backed off because a partial conversion would leave KVM
> in an unwieldy and somewhat scary state.

In the end I'd like to get rid of most "return 1;" and several "return 0;"
instances in KVM.

The main reason is that it is sometimes very hard to determine what the
current "return 1" is meant to say ("error" or "return to guest" or just
"okay"). This is especially true in some of the low level MSR emulation
code, e.g. in kvm_pmu_get_msr(): only after examining the call paths I was
sure the "return 0" wasn't meant to return to qemu, but to indicate success.

I have already started to replace the "return 1;" instances in the exit
handlers with "return KVM_RET_GUEST;", but the MSR emulation code convinced
me to analyze it first and to clear it up before changing any of its "1"
return values by accident to "KVM_RET_GUEST".

In the end my plan is to cover all archs to replace the literal "1"s with
"KVM_RET_GUEST" where appropriate, and as many other literal "1"s as possible
with more meaningful defines.

I hoped to get this done much earlier and faster, but this is quite a yak to
shave. :-)

I realized that pushing out patches as soon as possible is the only way to
get this finished at all, as this is a moving target with all the work of
others which might interfere. So my revised plan is to do one arch after
the other and in each arch to cover stuff like the MSR emulation first in
order not to mix things up again.

>> This work is a prelude of getting rid of the magic value "1" for
>> "return to guest". I started in x86 KVM host code doing that and soon
>> stumbled over lots of other use cases of the magic "1" as return value,
>> especially in MSR emulation where a comment even implied this "1" was
>> due to the "return to guest" semantics.
>>
>> A detailed analysis of all related code paths revealed that there was
>> indeed a rather clean interface between the functions using the MSR
>> emulation "1" and those using the "return to guest" "1".
> 
> Ya, we've started chipping away at the MSR stuff.  The big challenge is avoiding
> subtle ABI changes related to the fixups done by kvm_do_msr_access().

Right.

This whole work was triggered by my accidental "fix" of kvm_mmu_page_fault()
replacing a "1" with "RET_PF_RETRY", which you stopped from hitting upstream.


Juergen