| Message ID | 20210225134652.2127648-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | KVM: PPC: Book3S: C-ify the P9 entry/exit code | expand |
Hi Nick, > The va argument is not used in the function or set by its asm caller, > so remove it to be safe. Huh, so it isn't. I tracked the original implementation down to commit a8606e20e41a ("KVM: PPC: Handle some PAPR hcalls in the kernel") where paulus first added the ability to handle it in the kernel - there it takes a va argument but even then doesn't do anything with it. ajd also pointed out that we don't pass a va when linux is running as a guest, and LoPAR does not mention va as an argument. One small nit: checkpatch is complaining about spaces vs tabs: ERROR: code indent should use tabs where possible #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: + unsigned long pte_index, unsigned long avpn);$ WARNING: please, no spaces at the start of a line #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: + unsigned long pte_index, unsigned long avpn);$ Once that is resolved, Reviewed-by: Daniel Axtens <dja@axtens.net> Kind regards, Daniel Axtens > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/include/asm/kvm_ppc.h | 3 +-- > arch/powerpc/kvm/book3s_hv_rm_mmu.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h > index 8aacd76bb702..9531b1c1b190 100644 > --- a/arch/powerpc/include/asm/kvm_ppc.h > +++ b/arch/powerpc/include/asm/kvm_ppc.h > @@ -767,8 +767,7 @@ long kvmppc_h_remove(struct kvm_vcpu *vcpu, unsigned long flags, > unsigned long pte_index, unsigned long avpn); > long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu); > long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, > - unsigned long pte_index, unsigned long avpn, > - unsigned long va); > + unsigned long pte_index, unsigned long avpn); > long kvmppc_h_read(struct kvm_vcpu *vcpu, unsigned long flags, > unsigned long pte_index); > long kvmppc_h_clear_ref(struct kvm_vcpu *vcpu, unsigned long flags, > diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > index 88da2764c1bb..7af7c70f1468 100644 > --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c > +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c > @@ -673,8 +673,7 @@ long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu) > } > > long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, > - unsigned long pte_index, unsigned long avpn, > - unsigned long va) > + unsigned long pte_index, unsigned long avpn) > { > struct kvm *kvm = vcpu->kvm; > __be64 *hpte; > -- > 2.23.0
Excerpts from Daniel Axtens's message of February 26, 2021 3:01 pm: > Hi Nick, > >> The va argument is not used in the function or set by its asm caller, >> so remove it to be safe. > > Huh, so it isn't. I tracked the original implementation down to commit > a8606e20e41a ("KVM: PPC: Handle some PAPR hcalls in the kernel") where > paulus first added the ability to handle it in the kernel - there it > takes a va argument but even then doesn't do anything with it. > > ajd also pointed out that we don't pass a va when linux is running as a > guest, and LoPAR does not mention va as an argument. Yeah interesting, maybe it was from a pre-release version of PAPR? Who knows. > One small nit: checkpatch is complaining about spaces vs tabs: > ERROR: code indent should use tabs where possible > #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: > + unsigned long pte_index, unsigned long avpn);$ > > WARNING: please, no spaces at the start of a line > #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: > + unsigned long pte_index, unsigned long avpn);$ All the declarations are using the same style in this file so I think I'll leave it for someone to do a cleanup patch on. Okay? > > Once that is resolved, > Reviewed-by: Daniel Axtens <dja@axtens.net> Thanks, Nick > > Kind regards, > Daniel Axtens > >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> arch/powerpc/include/asm/kvm_ppc.h | 3 +-- >> arch/powerpc/kvm/book3s_hv_rm_mmu.c | 3 +-- >> 2 files changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h >> index 8aacd76bb702..9531b1c1b190 100644 >> --- a/arch/powerpc/include/asm/kvm_ppc.h >> +++ b/arch/powerpc/include/asm/kvm_ppc.h >> @@ -767,8 +767,7 @@ long kvmppc_h_remove(struct kvm_vcpu *vcpu, unsigned long flags, >> unsigned long pte_index, unsigned long avpn); >> long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu); >> long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, >> - unsigned long pte_index, unsigned long avpn, >> - unsigned long va); >> + unsigned long pte_index, unsigned long avpn); >> long kvmppc_h_read(struct kvm_vcpu *vcpu, unsigned long flags, >> unsigned long pte_index); >> long kvmppc_h_clear_ref(struct kvm_vcpu *vcpu, unsigned long flags, >> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c >> index 88da2764c1bb..7af7c70f1468 100644 >> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c >> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c >> @@ -673,8 +673,7 @@ long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu) >> } >> >> long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, >> - unsigned long pte_index, unsigned long avpn, >> - unsigned long va) >> + unsigned long pte_index, unsigned long avpn) >> { >> struct kvm *kvm = vcpu->kvm; >> __be64 *hpte; >> -- >> 2.23.0 >
Hi Nick, >> ERROR: code indent should use tabs where possible >> #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: >> + unsigned long pte_index, unsigned long avpn);$ >> >> WARNING: please, no spaces at the start of a line >> #25: FILE: arch/powerpc/include/asm/kvm_ppc.h:770: >> + unsigned long pte_index, unsigned long avpn);$ > > All the declarations are using the same style in this file so I think > I'll leave it for someone to do a cleanup patch on. Okay? Huh, right you are. In that case: Reviewed-by: Daniel Axtens <dja@axtens.net> Kind regards, Daniel
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index 8aacd76bb702..9531b1c1b190 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -767,8 +767,7 @@ long kvmppc_h_remove(struct kvm_vcpu *vcpu, unsigned long flags, unsigned long pte_index, unsigned long avpn); long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu); long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, - unsigned long pte_index, unsigned long avpn, - unsigned long va); + unsigned long pte_index, unsigned long avpn); long kvmppc_h_read(struct kvm_vcpu *vcpu, unsigned long flags, unsigned long pte_index); long kvmppc_h_clear_ref(struct kvm_vcpu *vcpu, unsigned long flags, diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 88da2764c1bb..7af7c70f1468 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c @@ -673,8 +673,7 @@ long kvmppc_h_bulk_remove(struct kvm_vcpu *vcpu) } long kvmppc_h_protect(struct kvm_vcpu *vcpu, unsigned long flags, - unsigned long pte_index, unsigned long avpn, - unsigned long va) + unsigned long pte_index, unsigned long avpn) { struct kvm *kvm = vcpu->kvm; __be64 *hpte;
The va argument is not used in the function or set by its asm caller, so remove it to be safe. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/kvm_ppc.h | 3 +-- arch/powerpc/kvm/book3s_hv_rm_mmu.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)