Re: [PATCH v4 4/7] KVM: x86: Move nested CPU dirty logging logic to common code

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

On Tue, 2025-10-14 at 14:24 -0700, Sean Christopherson wrote:
> On Tue, Oct 14, 2025, Kai Huang wrote:
> > On Tue, 2025-10-14 at 11:34 +0000, Huang, Kai wrote:
> > > >   
> > > > +static void kvm_vcpu_update_cpu_dirty_logging(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	if (WARN_ON_ONCE(!enable_pml))
> > > > +		return;
> > > 
> > > Nit:  
> > > 
> > > Since kvm_mmu_update_cpu_dirty_logging() checks kvm-
> > > > arch.cpu_dirty_log_size to determine whether PML is enabled, maybe it's
> > > better to check vcpu->kvm.arch.cpu_dirty_log_size here too to make them
> > > consistent.
> > 
> > After second thought, I think we should just change to checking the vcpu-
> > > kvm.arch.cpu_dirty_log_size.
> 
> +1
> 
> > > Anyway, the intention of this patch is moving code out of VMX to x86, so
> > > if needed, perhaps we can do the change in another patch.
> > > 
> > > Btw, now with 'enable_pml' also being moved to x86 common, both
> > > 'enable_pml' and 'kvm->arch.cpu_dirty_log_size' can be used to determine
> > > whether KVM has enabled PML.  It's kinda redundant, but I guess it's fine.
> > 
> > If we change to check cpu_dirty_log_size here, the x86 common code won't
> > use 'enable_pml' anymore and I think we can just get rid of that patch.
> > 
> > Sean, do you have any preference?
> 
> Definitely check cpu_dirty_log_size.  It's more precise (TDX doesn't (yet) support
> PML), avoids the export, and hopefully will yield more consistent code.

Yeah completely agree.  Thanks for the feedback.