Re: [PATCH v4 1/9] KVM: x86: Intercept CR4.LAM_SUP when LAM feature is enabled in guest

[Sean Christopherson <seanjc@google.com>]

On Fri, Feb 10, 2023, Robert Hoo wrote:
> On Fri, 2023-02-10 at 11:29 +0800, Yang, Weijiang wrote:
> > On 2/9/2023 10:40 AM, Robert Hoo wrote:
> > > Remove CR4.LAM_SUP (bit 28) from default CR4_RESERVED_BITS, while
> > > reserve
> > > it in __cr4_reserved_bits() by feature testing.
> > > 
> > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
> > 
> > As Sean pointed out in[*], this Reviewed-by is for other purpose,
> > please 
> > remove all of
> > 
> > them in this series.
> 
> No. Sean meant another thing.

Correct, what I object to is Intel _requiring_ a Reviewed-by before posting.

And while I'm certainly not going to refuse patches that have been reviewed
internally, I _strongly_ prefer reviews be on-list so that they are public and
recorded.  Being able to go back and look at the history and evolution of patches
is valuable, and the discussion itself is often beneficial to non-participants,
e.g. people that are new-ish to KVM and/or aren't familiar with the feature being
enabled can often learn new things and avoid similar pitfalls of their own.

Rather than spend cycles getting through internal review, I would much prefer
developers spend their time writing tests and validating their code before posting.
Obviously there's a risk that foregoing internal review will result in low quality
submissions, but I think the LASS series proves that mandatory reviews doesn't
necessarily help on that front.  On the other hand, writing and running tests
naturally enforces a minimum level of quality.

I am happy to help with changelogs, comments, naming, etc.  E.g. I don't get
frustrated when someone who is new to kernel development or for whom English is
not their first language writes an imperfect changelog.  I get frustrated when
there's seemingly no attempt to justify _why_ a patch/KVM does something, and I
get really grumpy when blatantly buggy code is posted with no tests.