| Message ID | 20170505192515.27833-2-bsd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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) >
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. 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) >>
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) >>> >
"Huang, Kai" <kai.huang@linux.intel.com> writes: ... > Hi Bandan, > > I was just suggesting. You and Paolo still make the decision :) Sure Kai, I don't mind the name change at all. The maintainer has already picked this up and I don't think the function name change is worth submitting a follow up. Thank you very much for the review! :) Bandan > 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) >>>> >>
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); + /* 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; +} + 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)
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(+)