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

Huang, Kai
2017-05-10T10:49:13Z

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.

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