Kernel KVM virtualization development
 help / color / mirror / Atom feed
* Process for doing early internal type review on the list
@ 2026-08-26 23:59 Edgecombe, Rick P
  2026-08-27  0:54 ` Edgecombe, Rick P
  0 siblings, 1 reply; 4+ messages in thread
From: Edgecombe, Rick P @ 2026-08-26 23:59 UTC (permalink / raw)
  To: kas@kernel.og, pbonzini@redhat.com, Hansen, Dave,
	seanjc@google.com
  Cc: kvm@vger.kernel.org, x86@kernel.org

Sean has long been pushing TDX developers to review less internally and capture
everything on the list. For several reasons:
 - Avoid thrash by hashing out things internally that are just overturned later.
 - Capture all the design decisions in lore history.
 - Help developers grow by getting comfortable working and making mistakes in   
   public.
 - A strong personal conviction that through some confluence of intangibles,
   working in public produces better outcomes for the kernel?

At this point we pretty much do all TDX KVM patch review externally. But there
were a couple straggling cases we ran into recently where not working in public
produced snags:
1. Far out things that are in POC stages (e.g. DICE, and next migration)
2. Things that are held internally to not overwhelm the list with too much TDX
stuff at once, while also continuing to make some progress on them. (TDX huge
pages)

In an off-list discussion, the idea came up to have something like RFC, but to
denote that the posting was for early "get it ready" type review. That it was
being shared only to capture discussion, but not garner attention. It could be
used for these straggling categories to do the review in public, but just off to
the side. It would be an optional thing instead of internal review. Not
required, just encouraged when possible. That kind of idea.


Sean suggested a "FUTURE" tag like "[PATCH FUTURE]". He didn't like "PREP"
because it could be confused for a patch that was preparatory for another patch.
I don't love FUTURE because is time focused, rather than about the purpose of
sending the patches. How about "PREVIEW"?

Dave, I'm wondering if you would want to have a similar process for tip targeted
TDX series?

Thanks,

Rick

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for doing early internal type review on the list
  2026-08-26 23:59 Process for doing early internal type review on the list Edgecombe, Rick P
@ 2026-08-27  0:54 ` Edgecombe, Rick P
  2026-08-27 19:02   ` Sean Christopherson
  0 siblings, 1 reply; 4+ messages in thread
From: Edgecombe, Rick P @ 2026-08-27  0:54 UTC (permalink / raw)
  To: pbonzini@redhat.com, Hansen, Dave, seanjc@google.com,
	kas@kernel.org
  Cc: kvm@vger.kernel.org, x86@kernel.org

Arg, fix Kiryl's email address.

On Wed, 2026-08-26 at 16:59 -0700, Rick Edgecombe wrote:
> Sean has long been pushing TDX developers to review less internally and capture
> everything on the list. For several reasons:
>  - Avoid thrash by hashing out things internally that are just overturned later.
>  - Capture all the design decisions in lore history.
>  - Help developers grow by getting comfortable working and making mistakes in   
>    public.
>  - A strong personal conviction that through some confluence of intangibles,
>    working in public produces better outcomes for the kernel?
> 
> At this point we pretty much do all TDX KVM patch review externally. But there
> were a couple straggling cases we ran into recently where not working in public
> produced snags:
> 1. Far out things that are in POC stages (e.g. DICE, and next migration)
> 2. Things that are held internally to not overwhelm the list with too much TDX
> stuff at once, while also continuing to make some progress on them. (TDX huge
> pages)
> 
> In an off-list discussion, the idea came up to have something like RFC, but to
> denote that the posting was for early "get it ready" type review. That it was
> being shared only to capture discussion, but not garner attention. It could be
> used for these straggling categories to do the review in public, but just off to
> the side. It would be an optional thing instead of internal review. Not
> required, just encouraged when possible. That kind of idea.
> 
> 
> Sean suggested a "FUTURE" tag like "[PATCH FUTURE]". He didn't like "PREP"
> because it could be confused for a patch that was preparatory for another patch.
> I don't love FUTURE because is time focused, rather than about the purpose of
> sending the patches. How about "PREVIEW"?
> 
> Dave, I'm wondering if you would want to have a similar process for tip targeted
> TDX series?
> 
> Thanks,
> 
> Rick


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for doing early internal type review on the list
  2026-08-27  0:54 ` Edgecombe, Rick P
@ 2026-08-27 19:02   ` Sean Christopherson
  2026-08-27 19:09     ` Borislav Petkov
  0 siblings, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2026-08-27 19:02 UTC (permalink / raw)
  To: Rick P Edgecombe
  Cc: pbonzini@redhat.com, Dave Hansen, kas@kernel.org,
	kvm@vger.kernel.org, x86@kernel.org

On Thu, Aug 27, 2026, Rick P Edgecombe wrote:
> Arg, fix Kiryl's email address.
> 
> On Wed, 2026-08-26 at 16:59 -0700, Rick Edgecombe wrote:
> > Sean has long been pushing TDX developers to review less internally and capture
> > everything on the list. For several reasons:
> >  - Avoid thrash by hashing out things internally that are just overturned later.
> >  - Capture all the design decisions in lore history.
> >  - Help developers grow by getting comfortable working and making mistakes in   
> >    public.
> >  - A strong personal conviction that through some confluence of intangibles,
> >    working in public produces better outcomes for the kernel?

As evidence, I submit the myriad historical KVM commits with godawful changelogs
and no meaningful on-list review/discussion.  When things go wrong, knowing the
intent and being able to retroactively assess the review process that yielded the
final commit is extremely valuable.

E.g. KVM added 600+ lines of code to (very poorly) virtualize MTRR memtypes, and
carried it for more than 15 years, all because of misdiagnosed #MC issues that
showed up in early EPT + VTd enabling.

Even with the few breadcrumbs provided by on-list review, it took me for freaking
ever to piece together what happened[*].  If all of the review had been done in
private, and all we saw was the final result, then it's entirely possible we'd
still be carrying the MTRR virtualization horrors.

Another way to look at it: would anyone (seriously) argue that changelogs are a
waste of time?  People can read the code, why do we need to explain what it does
and why?  Because anytime future developers are forced to make assumptions or
guesses due to lack of information, the probability of a mistake being and/or a
bug being introduced goes up.  And has been proven a hilarious number of times
this year by AI, humans make a lot of mistakes, and the more information that's
available, the easier it is to decipher and remedy those mistakes.

"Documenting" mistakes is also extremely valuable for helping others avoid those
same mistakes.  E.g. look at all the bajlilion edge cases and wrinkles Sashiko is
pointing out.  Having a public record of exactly what can go wrong, or what sharp
edges exist, is a huge benefit to future developers.  How many times have folks
ended up on Stack Overflow trying to figure out how to do some obsurce thing with
e.g. inline asm?  This is the same thing, just specific to the kernel.  It's a
public, searchable record of potential/common issues and in many cases, the cures
for those issues.

[*] https://lore.kernel.org/all/20240309010929.1403984-2-seanjc@google.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Process for doing early internal type review on the list
  2026-08-27 19:02   ` Sean Christopherson
@ 2026-08-27 19:09     ` Borislav Petkov
  0 siblings, 0 replies; 4+ messages in thread
From: Borislav Petkov @ 2026-08-27 19:09 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Rick P Edgecombe, pbonzini@redhat.com, Dave Hansen,
	kas@kernel.org, kvm@vger.kernel.org, x86@kernel.org

On Thu, Aug 27, 2026 at 12:02:52PM -0700, Sean Christopherson wrote:
> Another way to look at it: would anyone (seriously) argue that changelogs are a
> waste of time?  People can read the code, why do we need to explain what it does
> and why?  Because anytime future developers are forced to make assumptions or
> guesses due to lack of information, the probability of a mistake being and/or a
> bug being introduced goes up.

Now I'm sooo happy that someone else says exactly what I've been saying all
this time. Here's my canned response I had written down because I had to paste
it over and over and over again:

|"Please, do not talk about *what* the patch is doing in the commit message
|- that should be obvious from the diff itself. Rather, concentrate on the
|*why* it needs to be done and why your patch exists.
| 
|Imagine one fine day you're doing git archeology, you find the place in the
|code about which you want to find out why it was changed the way it is now.
| 
|You do git annotate <filename> ... find the line, see the commit id and you
|do:
| 
|git show <commit id>
| 
|You read the commit message and there's just gibberish and nothing's
|explaining *why* that change was done. And you start scratching your head,
|trying to figure out why. Because the damn commit message is not worth the
|electrons used to display it with.
| 
|This happens to us maintainers at least once a week."

oh, and there's more:

|"Yeah, the goal is for our commit messages to be as clear to humans as
|possible, even for people who do not have intimate knowledge of the matter.
|
|And, more importantly, when we start doing git archeology months, years from
|now, it should be perfectly clear why a commit was done.
|
|So don't worry if the official documentation is lacking/lagging. That doesn't
|mean we can't do a better job and explain exactly what and why we're doing it.
|
|So please try to explain the issue in a clear and detailed way.
|
|People and you yourself will be thankful for it, trust me."

So preach brother!

:-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-27 19:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 23:59 Process for doing early internal type review on the list Edgecombe, Rick P
2026-08-27  0:54 ` Edgecombe, Rick P
2026-08-27 19:02   ` Sean Christopherson
2026-08-27 19:09     ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox