Re: [PATCH v2 1/3] kvm: x86: Add a hook for arch specific dirty logging emulation

Huang, Kai
2017-05-10T23:23:16Z

On 5/11/2017 3:53 AM, Bandan Das wrote:
> Hi Kai,
>
> "Huang, Kai" <kai.huang@linux.intel.com> writes:
>
>> On 5/6/2017 7:25 AM, Bandan Das wrote:
>>> When KVM updates accessed/dirty bits, this hook can be used
>>> to invoke an arch specific function that implements/emulates
>>> dirty logging such as PML.
>>>
>>> Signed-off-by: Bandan Das <bsd@redhat.com>
>>> ---
>>> arch/x86/include/asm/kvm_host.h | 2 ++
>>> arch/x86/kvm/mmu.c | 15 +++++++++++++++
>>> arch/x86/kvm/mmu.h | 1 +
>>> arch/x86/kvm/paging_tmpl.h | 4 ++++
>>> 4 files changed, 22 insertions(+)
>>>
>>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>>> index f5bddf92..9c761fe 100644
>>> --- a/arch/x86/include/asm/kvm_host.h
>>> +++ b/arch/x86/include/asm/kvm_host.h
>>> @@ -1020,6 +1020,8 @@ struct kvm_x86_ops {
>>> void (*enable_log_dirty_pt_masked)(struct kvm *kvm,
>>> struct kvm_memory_slot *slot,
>>> gfn_t offset, unsigned long mask);
>>> + int (*write_log_dirty)(struct kvm_vcpu *vcpu);
>>
>> Hi,
>>
>> Thanks for adding PML to nested support!
>>
>> IMHO this callback is only used for write L2's dirty gpa to L1's PML
>> buffer, so probably it's better to change the name to something like:
>> nested_write_log_dirty.
>
> The name was meant more to signify what it does: i.e. write dirty log rather
> than where in the hierarchy it's being used :) But I guess, a nested_ prefix
> doesn't hurt either.

Hi Bandan,

I was just suggesting. You and Paolo still make the decision :)

Thanks,
-Kai
>
> Bandan
>
>> Thanks,
>> -Kai
>>
>>> +
>>> /* pmu operations of sub-arch */
>>> const struct kvm_pmu_ops *pmu_ops;
>>>
>>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>>> index 5586765..5d3376f 100644
>>> --- a/arch/x86/kvm/mmu.c
>>> +++ b/arch/x86/kvm/mmu.c
>>> @@ -1498,6 +1498,21 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
>>> kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
>>> }
>>>
>>> +/**
>>> + * kvm_arch_write_log_dirty - emulate dirty page logging
>>> + * @vcpu: Guest mode vcpu
>>> + *
>>> + * Emulate arch specific page modification logging for the
>>> + * nested hypervisor
>>> + */
>>> +int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu)
>>> +{
>>> + if (kvm_x86_ops->write_log_dirty)
>>> + return kvm_x86_ops->write_log_dirty(vcpu);
>>> +
>>> + return 0;
>>> +}
>>
>> kvm_nested_arch_write_log_dirty?
>>
>>> +
>>> bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
>>> struct kvm_memory_slot *slot, u64 gfn)
>>> {
>>> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
>>> index d8ccb32..2797580 100644
>>> --- a/arch/x86/kvm/mmu.h
>>> +++ b/arch/x86/kvm/mmu.h
>>> @@ -202,4 +202,5 @@ void kvm_mmu_gfn_disallow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
>>> void kvm_mmu_gfn_allow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
>>> bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
>>> struct kvm_memory_slot *slot, u64 gfn);
>>> +int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu);
>>> #endif
>>> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
>>> index 314d207..5624174 100644
>>> --- a/arch/x86/kvm/paging_tmpl.h
>>> +++ b/arch/x86/kvm/paging_tmpl.h
>>> @@ -226,6 +226,10 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
>>> if (level == walker->level && write_fault &&
>>> !(pte & PT_GUEST_DIRTY_MASK)) {
>>> trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
>>> +#if PTTYPE == PTTYPE_EPT
>>> + if (kvm_arch_write_log_dirty(vcpu))
>>> + return -EINVAL;
>>> +#endif
>>> pte |= PT_GUEST_DIRTY_MASK;
>>> }
>>> if (pte == orig_pte)
>>>
>