| Message ID | 20231201132618.555031-2-vaibhav@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: PPC: Nested APIv2 : Performance improvements | expand |
Vaibhav Jain <vaibhav@linux.ibm.com> writes: > From: Jordan Niethe <jniethe5@gmail.com> > > An L0 must invalidate the L2's RPT during H_GUEST_DELETE if this has not > already been done. This is a slow operation that means H_GUEST_DELETE > must return H_BUSY multiple times before completing. Invalidating the > tables before deleting the guest so there is less work for the L0 to do. > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > arch/powerpc/include/asm/kvm_book3s.h | 1 + > arch/powerpc/kvm/book3s_hv.c | 6 ++++-- > arch/powerpc/kvm/book3s_hv_nested.c | 2 +- > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h > index 4f527d09c92b..a37736ed3728 100644 > --- a/arch/powerpc/include/asm/kvm_book3s.h > +++ b/arch/powerpc/include/asm/kvm_book3s.h > @@ -302,6 +302,7 @@ void kvmhv_nested_exit(void); > void kvmhv_vm_nested_init(struct kvm *kvm); > long kvmhv_set_partition_table(struct kvm_vcpu *vcpu); > long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu); > +void kvmhv_flush_lpid(u64 lpid); > void kvmhv_set_ptbl_entry(u64 lpid, u64 dw0, u64 dw1); > void kvmhv_release_all_nested(struct kvm *kvm); > long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu); > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index 1ed6ec140701..5543e8490cd9 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -5691,10 +5691,12 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm) > kvmhv_set_ptbl_entry(kvm->arch.lpid, 0, 0); > } > > - if (kvmhv_is_nestedv2()) > + if (kvmhv_is_nestedv2()) { > + kvmhv_flush_lpid(kvm->arch.lpid); > plpar_guest_delete(0, kvm->arch.lpid); > I am not sure I follow the optimization here. I would expect the hypervisor to kill all the translation caches as part of guest_delete. What is the benefit of doing a lpid flush outside the guest delete? > - else > + } else { > kvmppc_free_lpid(kvm->arch.lpid); > + } > > kvmppc_free_pimap(kvm); > } > diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c > index 3b658b8696bc..5c375ec1a3c6 100644 > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -503,7 +503,7 @@ void kvmhv_nested_exit(void) > } > } > > -static void kvmhv_flush_lpid(u64 lpid) > +void kvmhv_flush_lpid(u64 lpid) > { > long rc; > > -- > 2.42.0
Hi Aneesh, Thanks for looking into this patch. My responses inline below: "Aneesh Kumar K.V (IBM)" <aneesh.kumar@kernel.org> writes: > Vaibhav Jain <vaibhav@linux.ibm.com> writes: > >> From: Jordan Niethe <jniethe5@gmail.com> >> >> An L0 must invalidate the L2's RPT during H_GUEST_DELETE if this has not >> already been done. This is a slow operation that means H_GUEST_DELETE >> must return H_BUSY multiple times before completing. Invalidating the >> tables before deleting the guest so there is less work for the L0 to do. >> >> Signed-off-by: Jordan Niethe <jniethe5@gmail.com> >> --- >> arch/powerpc/include/asm/kvm_book3s.h | 1 + >> arch/powerpc/kvm/book3s_hv.c | 6 ++++-- >> arch/powerpc/kvm/book3s_hv_nested.c | 2 +- >> 3 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h >> index 4f527d09c92b..a37736ed3728 100644 >> --- a/arch/powerpc/include/asm/kvm_book3s.h >> +++ b/arch/powerpc/include/asm/kvm_book3s.h >> @@ -302,6 +302,7 @@ void kvmhv_nested_exit(void); >> void kvmhv_vm_nested_init(struct kvm *kvm); >> long kvmhv_set_partition_table(struct kvm_vcpu *vcpu); >> long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu); >> +void kvmhv_flush_lpid(u64 lpid); >> void kvmhv_set_ptbl_entry(u64 lpid, u64 dw0, u64 dw1); >> void kvmhv_release_all_nested(struct kvm *kvm); >> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu); >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index 1ed6ec140701..5543e8490cd9 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -5691,10 +5691,12 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm) >> kvmhv_set_ptbl_entry(kvm->arch.lpid, 0, 0); >> } >> >> - if (kvmhv_is_nestedv2()) >> + if (kvmhv_is_nestedv2()) { >> + kvmhv_flush_lpid(kvm->arch.lpid); >> plpar_guest_delete(0, kvm->arch.lpid); >> > > I am not sure I follow the optimization here. I would expect the > hypervisor to kill all the translation caches as part of guest_delete. > What is the benefit of doing a lpid flush outside the guest delete? > Thats right. However without this optimization the H_GUEST_DELETE hcall in plpar_guest_delete() returns H_BUSY multiple times resulting in multiple hcalls to the hypervisor until it finishes. Flushing the guest translation cache upfront reduces the number of HCALLs L1 guests has to make to delete a L2 guest via H_GUEST_DELETE. >> - else >> + } else { >> kvmppc_free_lpid(kvm->arch.lpid); >> + } >> >> kvmppc_free_pimap(kvm); >> } >> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c >> index 3b658b8696bc..5c375ec1a3c6 100644 >> --- a/arch/powerpc/kvm/book3s_hv_nested.c >> +++ b/arch/powerpc/kvm/book3s_hv_nested.c >> @@ -503,7 +503,7 @@ void kvmhv_nested_exit(void) >> } >> } >> >> -static void kvmhv_flush_lpid(u64 lpid) >> +void kvmhv_flush_lpid(u64 lpid) >> { >> long rc; >> >> -- >> 2.42.0
Vaibhav Jain <vaibhav@linux.ibm.com> writes: > Hi Aneesh, > > Thanks for looking into this patch. My responses inline below: > > "Aneesh Kumar K.V (IBM)" <aneesh.kumar@kernel.org> writes: > >> Vaibhav Jain <vaibhav@linux.ibm.com> writes: >> >>> From: Jordan Niethe <jniethe5@gmail.com> >>> >>> An L0 must invalidate the L2's RPT during H_GUEST_DELETE if this has not >>> already been done. This is a slow operation that means H_GUEST_DELETE >>> must return H_BUSY multiple times before completing. Invalidating the >>> tables before deleting the guest so there is less work for the L0 to do. >>> >>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com> >>> --- >>> arch/powerpc/include/asm/kvm_book3s.h | 1 + >>> arch/powerpc/kvm/book3s_hv.c | 6 ++++-- >>> arch/powerpc/kvm/book3s_hv_nested.c | 2 +- >>> 3 files changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h >>> index 4f527d09c92b..a37736ed3728 100644 >>> --- a/arch/powerpc/include/asm/kvm_book3s.h >>> +++ b/arch/powerpc/include/asm/kvm_book3s.h >>> @@ -302,6 +302,7 @@ void kvmhv_nested_exit(void); >>> void kvmhv_vm_nested_init(struct kvm *kvm); >>> long kvmhv_set_partition_table(struct kvm_vcpu *vcpu); >>> long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu); >>> +void kvmhv_flush_lpid(u64 lpid); >>> void kvmhv_set_ptbl_entry(u64 lpid, u64 dw0, u64 dw1); >>> void kvmhv_release_all_nested(struct kvm *kvm); >>> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu); >>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >>> index 1ed6ec140701..5543e8490cd9 100644 >>> --- a/arch/powerpc/kvm/book3s_hv.c >>> +++ b/arch/powerpc/kvm/book3s_hv.c >>> @@ -5691,10 +5691,12 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm) >>> kvmhv_set_ptbl_entry(kvm->arch.lpid, 0, 0); >>> } >>> >>> - if (kvmhv_is_nestedv2()) >>> + if (kvmhv_is_nestedv2()) { >>> + kvmhv_flush_lpid(kvm->arch.lpid); >>> plpar_guest_delete(0, kvm->arch.lpid); >>> >> >> I am not sure I follow the optimization here. I would expect the >> hypervisor to kill all the translation caches as part of guest_delete. >> What is the benefit of doing a lpid flush outside the guest delete? >> > Thats right. However without this optimization the H_GUEST_DELETE hcall > in plpar_guest_delete() returns H_BUSY multiple times resulting in > multiple hcalls to the hypervisor until it finishes. Flushing the guest > translation cache upfront reduces the number of HCALLs L1 guests has to > make to delete a L2 guest via H_GUEST_DELETE. > can we add that as a comment above that kvmhv_flush_lpid()? -aneesh
"Aneesh Kumar K.V" <aneesh.kumar@kernel.org> writes: > Vaibhav Jain <vaibhav@linux.ibm.com> writes: > >> Hi Aneesh, >> >> Thanks for looking into this patch. My responses inline below: >> >> "Aneesh Kumar K.V (IBM)" <aneesh.kumar@kernel.org> writes: >> >>> Vaibhav Jain <vaibhav@linux.ibm.com> writes: >>> >>>> From: Jordan Niethe <jniethe5@gmail.com> >>>> >>>> An L0 must invalidate the L2's RPT during H_GUEST_DELETE if this has not >>>> already been done. This is a slow operation that means H_GUEST_DELETE >>>> must return H_BUSY multiple times before completing. Invalidating the >>>> tables before deleting the guest so there is less work for the L0 to do. >>>> >>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com> >>>> --- >>>> arch/powerpc/include/asm/kvm_book3s.h | 1 + >>>> arch/powerpc/kvm/book3s_hv.c | 6 ++++-- >>>> arch/powerpc/kvm/book3s_hv_nested.c | 2 +- >>>> 3 files changed, 6 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h >>>> index 4f527d09c92b..a37736ed3728 100644 >>>> --- a/arch/powerpc/include/asm/kvm_book3s.h >>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h >>>> @@ -302,6 +302,7 @@ void kvmhv_nested_exit(void); >>>> void kvmhv_vm_nested_init(struct kvm *kvm); >>>> long kvmhv_set_partition_table(struct kvm_vcpu *vcpu); >>>> long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu); >>>> +void kvmhv_flush_lpid(u64 lpid); >>>> void kvmhv_set_ptbl_entry(u64 lpid, u64 dw0, u64 dw1); >>>> void kvmhv_release_all_nested(struct kvm *kvm); >>>> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu); >>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >>>> index 1ed6ec140701..5543e8490cd9 100644 >>>> --- a/arch/powerpc/kvm/book3s_hv.c >>>> +++ b/arch/powerpc/kvm/book3s_hv.c >>>> @@ -5691,10 +5691,12 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm) >>>> kvmhv_set_ptbl_entry(kvm->arch.lpid, 0, 0); >>>> } >>>> >>>> - if (kvmhv_is_nestedv2()) >>>> + if (kvmhv_is_nestedv2()) { >>>> + kvmhv_flush_lpid(kvm->arch.lpid); >>>> plpar_guest_delete(0, kvm->arch.lpid); >>>> >>> >>> I am not sure I follow the optimization here. I would expect the >>> hypervisor to kill all the translation caches as part of guest_delete. >>> What is the benefit of doing a lpid flush outside the guest delete? >>> >> Thats right. However without this optimization the H_GUEST_DELETE hcall >> in plpar_guest_delete() returns H_BUSY multiple times resulting in >> multiple hcalls to the hypervisor until it finishes. Flushing the guest >> translation cache upfront reduces the number of HCALLs L1 guests has to >> make to delete a L2 guest via H_GUEST_DELETE. >> > > can we add that as a comment above that kvmhv_flush_lpid()? Sure, I will put up a comment with that detail in v2 of the patch series. > > -aneesh
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h index 4f527d09c92b..a37736ed3728 100644 --- a/arch/powerpc/include/asm/kvm_book3s.h +++ b/arch/powerpc/include/asm/kvm_book3s.h @@ -302,6 +302,7 @@ void kvmhv_nested_exit(void); void kvmhv_vm_nested_init(struct kvm *kvm); long kvmhv_set_partition_table(struct kvm_vcpu *vcpu); long kvmhv_copy_tofrom_guest_nested(struct kvm_vcpu *vcpu); +void kvmhv_flush_lpid(u64 lpid); void kvmhv_set_ptbl_entry(u64 lpid, u64 dw0, u64 dw1); void kvmhv_release_all_nested(struct kvm *kvm); long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu); diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 1ed6ec140701..5543e8490cd9 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -5691,10 +5691,12 @@ static void kvmppc_core_destroy_vm_hv(struct kvm *kvm) kvmhv_set_ptbl_entry(kvm->arch.lpid, 0, 0); } - if (kvmhv_is_nestedv2()) + if (kvmhv_is_nestedv2()) { + kvmhv_flush_lpid(kvm->arch.lpid); plpar_guest_delete(0, kvm->arch.lpid); - else + } else { kvmppc_free_lpid(kvm->arch.lpid); + } kvmppc_free_pimap(kvm); } diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c index 3b658b8696bc..5c375ec1a3c6 100644 --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -503,7 +503,7 @@ void kvmhv_nested_exit(void) } } -static void kvmhv_flush_lpid(u64 lpid) +void kvmhv_flush_lpid(u64 lpid) { long rc;