Re: [PATCH 2/3] KVM: x86/mmu: Fix pf_fixed count in tdp_mmu_map_handle_target_level()

[Kai Huang <kai.huang@intel.com>]

On Thu, 2021-05-06 at 15:29 +0000, Sean Christopherson wrote:
> On Thu, May 06, 2021, Kai Huang wrote:
> > On Wed, 2021-05-05 at 09:11 -0700, Ben Gardon wrote:
> > > It would probably also be worth putting a comment on pf_fixed so that
> > > people in the future know what it's supposed to mean and we don't get
> > > into archeology, reverse engineering the meaning of the stat again.
> > 
> > It seems the legacy MMU code path is a better place to add the comment to explain when
> > pf_fixed should be increased.  However I am not sure whether it is necessary for this
> > patch (and I confess I found it's hard to explain why to increase pf_fixed in case of
> > emulation :)).  Or perhaps Sean can write a patch to add comment to legacy MMU :)
> 
> Ya, I think it makes sense to hold off on documenting the existing behavior in
> the TDP MMU.  As is often the case in KVM, just because KVM has always done
> something one way, doesn't mean it's correct/ideal.  But, bikeshedding over what
> faults exactly should count towards pf_fixed is best left to a separate patch.
> 
> > I ended up with  below, by adding a comment in TDP MMU saying "to make it consistent with
> > legacy MMU...", and in the commit message, I put a lore link of this discussion, since I
> > found Sean's explanation is quite useful. When people are interested in, they can do a git
> > blame and find the commit msg of this change -- although it is not as straightforward as
> > having comment directly.
> > 
> > Is this OK to you?
> > 
> > And Sean?
> 
> Yep, works for me.

Thanks!