[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aTLpQjsSsjQbHl3y@google.com>
Date: Fri, 5 Dec 2025 06:16:34 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Juergen Gross <jgross@...e.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, x86@...nel.org,
linux-coco@...ts.linux.dev, Paolo Bonzini <pbonzini@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
Kiryl Shutsemau <kas@...nel.org>, Rick Edgecombe <rick.p.edgecombe@...el.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>, David Woodhouse <dwmw2@...radead.org>, Paul Durrant <paul@....org>
Subject: Re: [PATCH 00/10] KVM: Avoid literal numbers as return values
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().
Powered by blists - more mailing lists