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

["Nikunj A. Dadhania" <nikunj@amd.com>]

On 10/15/2025 3:07 AM, Huang, Kai wrote:
> 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.

I will add this as a pre-patch, does it need a fixes tag ?

>>>>
>>>> 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.

Sure, thanks for the feedback.

Regards
Nikunj