| Message ID | 20130704134029.4db2736a.yoshikawa_takuya_b1@lab.ntt.co.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 04, 2013 at 01:40:29PM +0900, Takuya Yoshikawa wrote: > This is called right after the memslots is updated, i.e. when the result > of update_memslots() gets installed in install_new_memslots(). Since > the memslots needs to be updated twice when we delete or move a memslot, > kvm_arch_commit_memory_region() does not correspond to this exactly. > > In the following patch, x86 will use this new API to check if the mmio > generation has reached its maximum value, in which case mmio sptes need > to be flushed out. > > Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> > --- > Removed the trailing space after "return old_memslots;" at this chance. > > arch/arm/kvm/arm.c | 4 ++++ > arch/ia64/kvm/kvm-ia64.c | 4 ++++ > arch/mips/kvm/kvm_mips.c | 4 ++++ > arch/powerpc/kvm/powerpc.c | 4 ++++ > arch/s390/kvm/kvm-s390.c | 4 ++++ > arch/x86/kvm/x86.c | 4 ++++ > include/linux/kvm_host.h | 1 + > virt/kvm/kvm_main.c | 5 ++++- > 8 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > index 327a1fb..51c3659 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -219,6 +219,10 @@ long kvm_arch_dev_ioctl(struct file *filp, > return -EINVAL; > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c > index 5b2dc0d..bdfd878 100644 > --- a/arch/ia64/kvm/kvm-ia64.c > +++ b/arch/ia64/kvm/kvm-ia64.c > @@ -1560,6 +1560,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) > return 0; > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c > index e0dad02..daa32ea 100644 > --- a/arch/mips/kvm/kvm_mips.c > +++ b/arch/mips/kvm/kvm_mips.c > @@ -208,6 +208,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) > return 0; > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 6316ee3..ae63ae4 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -420,6 +420,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) > return kvmppc_core_create_memslot(slot, npages); > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index ba694d2..a3d797b 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -1056,6 +1056,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) > return 0; > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > /* Section: memory related */ > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 7d71c0f..71912c9 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -7020,6 +7020,10 @@ out_free: > return -ENOMEM; > } > > +void kvm_arch_memslots_updated(struct kvm *kvm) > +{ > +} > + > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index e3aae6d..1c1e9de 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -498,6 +498,7 @@ int __kvm_set_memory_region(struct kvm *kvm, > void kvm_arch_free_memslot(struct kvm_memory_slot *free, > struct kvm_memory_slot *dont); > int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages); > +void kvm_arch_memslots_updated(struct kvm *kvm); We can define empty function here like this: #ifdef __KVM_HAVE_MEMSLOT_UPDATE void kvm_arch_memslots_updated(struct kvm *kvm); #else static void kvm_arch_memslots_updated(struct kvm *kvm) { } #endif and make x86.c define __KVM_HAVE_MEMSLOT_UPDATE. But I am fine with your approach too. Do other arch maintainers have any preferences here? > int kvm_arch_prepare_memory_region(struct kvm *kvm, > struct kvm_memory_slot *memslot, > struct kvm_userspace_memory_region *mem, > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 1580dd4..4ed9d89 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -731,7 +731,10 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm, > update_memslots(slots, new, kvm->memslots->generation); > rcu_assign_pointer(kvm->memslots, slots); > synchronize_srcu_expedited(&kvm->srcu); > - return old_memslots; > + > + kvm_arch_memslots_updated(kvm); > + > + return old_memslots; > } > > /* > -- > 1.7.9.5 -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04.07.2013, at 11:53, Gleb Natapov wrote: > On Thu, Jul 04, 2013 at 01:40:29PM +0900, Takuya Yoshikawa wrote: >> This is called right after the memslots is updated, i.e. when the result >> of update_memslots() gets installed in install_new_memslots(). Since >> the memslots needs to be updated twice when we delete or move a memslot, >> kvm_arch_commit_memory_region() does not correspond to this exactly. >> >> In the following patch, x86 will use this new API to check if the mmio >> generation has reached its maximum value, in which case mmio sptes need >> to be flushed out. >> >> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> >> --- >> Removed the trailing space after "return old_memslots;" at this chance. >> >> arch/arm/kvm/arm.c | 4 ++++ >> arch/ia64/kvm/kvm-ia64.c | 4 ++++ >> arch/mips/kvm/kvm_mips.c | 4 ++++ >> arch/powerpc/kvm/powerpc.c | 4 ++++ >> arch/s390/kvm/kvm-s390.c | 4 ++++ >> arch/x86/kvm/x86.c | 4 ++++ >> include/linux/kvm_host.h | 1 + >> virt/kvm/kvm_main.c | 5 ++++- >> 8 files changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c >> index 327a1fb..51c3659 100644 >> --- a/arch/arm/kvm/arm.c >> +++ b/arch/arm/kvm/arm.c >> @@ -219,6 +219,10 @@ long kvm_arch_dev_ioctl(struct file *filp, >> return -EINVAL; >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> struct kvm_userspace_memory_region *mem, >> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c >> index 5b2dc0d..bdfd878 100644 >> --- a/arch/ia64/kvm/kvm-ia64.c >> +++ b/arch/ia64/kvm/kvm-ia64.c >> @@ -1560,6 +1560,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) >> return 0; >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> struct kvm_userspace_memory_region *mem, >> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c >> index e0dad02..daa32ea 100644 >> --- a/arch/mips/kvm/kvm_mips.c >> +++ b/arch/mips/kvm/kvm_mips.c >> @@ -208,6 +208,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) >> return 0; >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> struct kvm_userspace_memory_region *mem, >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> index 6316ee3..ae63ae4 100644 >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -420,6 +420,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) >> return kvmppc_core_create_memslot(slot, npages); >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> struct kvm_userspace_memory_region *mem, >> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c >> index ba694d2..a3d797b 100644 >> --- a/arch/s390/kvm/kvm-s390.c >> +++ b/arch/s390/kvm/kvm-s390.c >> @@ -1056,6 +1056,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) >> return 0; >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> /* Section: memory related */ >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 7d71c0f..71912c9 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -7020,6 +7020,10 @@ out_free: >> return -ENOMEM; >> } >> >> +void kvm_arch_memslots_updated(struct kvm *kvm) >> +{ >> +} >> + >> int kvm_arch_prepare_memory_region(struct kvm *kvm, >> struct kvm_memory_slot *memslot, >> struct kvm_userspace_memory_region *mem, >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index e3aae6d..1c1e9de 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -498,6 +498,7 @@ int __kvm_set_memory_region(struct kvm *kvm, >> void kvm_arch_free_memslot(struct kvm_memory_slot *free, >> struct kvm_memory_slot *dont); >> int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages); >> +void kvm_arch_memslots_updated(struct kvm *kvm); > We can define empty function here like this: > #ifdef __KVM_HAVE_MEMSLOT_UPDATE > void kvm_arch_memslots_updated(struct kvm *kvm); > #else > static void kvm_arch_memslots_updated(struct kvm *kvm) > { > } > #endif > > and make x86.c define __KVM_HAVE_MEMSLOT_UPDATE. > > But I am fine with your approach too. Do other arch maintainers have any > preferences here? I think this is common infrastructure, so the current approach is a better fit that anything conditional on __KVM_HAVE. If we really want to make use of this on the PPC side, we'll have to move it into the respective host MMU source file, but I'd be more than happy to leave that as a future work item. So for now: Acked-by: Alexander Graf <agraf@suse.de> Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/04/2013 12:40 PM, Takuya Yoshikawa wrote: > This is called right after the memslots is updated, i.e. when the result > of update_memslots() gets installed in install_new_memslots(). Since > the memslots needs to be updated twice when we delete or move a memslot, > kvm_arch_commit_memory_region() does not correspond to this exactly. > > In the following patch, x86 will use this new API to check if the mmio > generation has reached its maximum value, in which case mmio sptes need > to be flushed out. Reviewed-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 4 Jul 2013 12:53:15 +0300 Gleb Natapov <gleb@redhat.com> wrote: > On Thu, Jul 04, 2013 at 01:40:29PM +0900, Takuya Yoshikawa wrote: > > This is called right after the memslots is updated, i.e. when the result > > of update_memslots() gets installed in install_new_memslots(). Since > > the memslots needs to be updated twice when we delete or move a memslot, > > kvm_arch_commit_memory_region() does not correspond to this exactly. > > > > In the following patch, x86 will use this new API to check if the mmio > > generation has reached its maximum value, in which case mmio sptes need > > to be flushed out. > > > > Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> > > --- > > Removed the trailing space after "return old_memslots;" at this chance. > > > > arch/arm/kvm/arm.c | 4 ++++ > > arch/ia64/kvm/kvm-ia64.c | 4 ++++ > > arch/mips/kvm/kvm_mips.c | 4 ++++ > > arch/powerpc/kvm/powerpc.c | 4 ++++ > > arch/s390/kvm/kvm-s390.c | 4 ++++ > > arch/x86/kvm/x86.c | 4 ++++ > > include/linux/kvm_host.h | 1 + > > virt/kvm/kvm_main.c | 5 ++++- > > 8 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > index e3aae6d..1c1e9de 100644 > > --- a/include/linux/kvm_host.h > > +++ b/include/linux/kvm_host.h > > @@ -498,6 +498,7 @@ int __kvm_set_memory_region(struct kvm *kvm, > > void kvm_arch_free_memslot(struct kvm_memory_slot *free, > > struct kvm_memory_slot *dont); > > int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages); > > +void kvm_arch_memslots_updated(struct kvm *kvm); > We can define empty function here like this: > #ifdef __KVM_HAVE_MEMSLOT_UPDATE > void kvm_arch_memslots_updated(struct kvm *kvm); > #else > static void kvm_arch_memslots_updated(struct kvm *kvm) > { > } > #endif > > and make x86.c define __KVM_HAVE_MEMSLOT_UPDATE. > > But I am fine with your approach too. Do other arch maintainers have any > preferences here? I don't really have a strong preference either way, so Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com> for the current approach. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 327a1fb..51c3659 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -219,6 +219,10 @@ long kvm_arch_dev_ioctl(struct file *filp, return -EINVAL; } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c index 5b2dc0d..bdfd878 100644 --- a/arch/ia64/kvm/kvm-ia64.c +++ b/arch/ia64/kvm/kvm-ia64.c @@ -1560,6 +1560,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) return 0; } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c index e0dad02..daa32ea 100644 --- a/arch/mips/kvm/kvm_mips.c +++ b/arch/mips/kvm/kvm_mips.c @@ -208,6 +208,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) return 0; } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 6316ee3..ae63ae4 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -420,6 +420,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) return kvmppc_core_create_memslot(slot, npages); } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index ba694d2..a3d797b 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -1056,6 +1056,10 @@ int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages) return 0; } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + /* Section: memory related */ int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7d71c0f..71912c9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7020,6 +7020,10 @@ out_free: return -ENOMEM; } +void kvm_arch_memslots_updated(struct kvm *kvm) +{ +} + int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index e3aae6d..1c1e9de 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -498,6 +498,7 @@ int __kvm_set_memory_region(struct kvm *kvm, void kvm_arch_free_memslot(struct kvm_memory_slot *free, struct kvm_memory_slot *dont); int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages); +void kvm_arch_memslots_updated(struct kvm *kvm); int kvm_arch_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *memslot, struct kvm_userspace_memory_region *mem, diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 1580dd4..4ed9d89 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -731,7 +731,10 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm, update_memslots(slots, new, kvm->memslots->generation); rcu_assign_pointer(kvm->memslots, slots); synchronize_srcu_expedited(&kvm->srcu); - return old_memslots; + + kvm_arch_memslots_updated(kvm); + + return old_memslots; } /*
This is called right after the memslots is updated, i.e. when the result of update_memslots() gets installed in install_new_memslots(). Since the memslots needs to be updated twice when we delete or move a memslot, kvm_arch_commit_memory_region() does not correspond to this exactly. In the following patch, x86 will use this new API to check if the mmio generation has reached its maximum value, in which case mmio sptes need to be flushed out. Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp> --- Removed the trailing space after "return old_memslots;" at this chance. arch/arm/kvm/arm.c | 4 ++++ arch/ia64/kvm/kvm-ia64.c | 4 ++++ arch/mips/kvm/kvm_mips.c | 4 ++++ arch/powerpc/kvm/powerpc.c | 4 ++++ arch/s390/kvm/kvm-s390.c | 4 ++++ arch/x86/kvm/x86.c | 4 ++++ include/linux/kvm_host.h | 1 + virt/kvm/kvm_main.c | 5 ++++- 8 files changed, 29 insertions(+), 1 deletion(-)