* Re: [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters [not found] ` <20200623131418.31473-2-tianjia.zhang@linux.alibaba.com> @ 2020-06-23 15:31 ` Christian Borntraeger 2020-06-24 2:39 ` Tianjia Zhang 2020-07-10 7:48 ` Paolo Bonzini 0 siblings, 2 replies; 7+ messages in thread From: Christian Borntraeger @ 2020-06-23 15:31 UTC (permalink / raw) To: Tianjia Zhang, pbonzini, tsbogend, paulus, mpe, benh, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel On 23.06.20 15:14, Tianjia Zhang wrote: > In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' > structure. For historical reasons, many kvm-related function parameters > retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This > patch does a unified cleanup of these remaining redundant parameters. > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/s390/kvm/kvm-s390.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) Tinajia, I have trouble seeing value in this particular patch. We add LOCs without providing any noticable benefit. All other patches in this series at least reduce the amount of code. So I would defer this to Paolo if he prefers to have this way across all architectures. > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index d47c19718615..f5f96dc33712 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4175,8 +4175,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu) > return rc; > } > > -static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > +static void sync_regs_fmt2(struct kvm_vcpu *vcpu) > { > + struct kvm_run *kvm_run = vcpu->run; > struct runtime_instr_cb *riccb; > struct gs_cb *gscb; > > @@ -4242,8 +4243,10 @@ static void sync_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > /* SIE will load etoken directly from SDNX and therefore kvm_run */ > } > > -static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > +static void sync_regs(struct kvm_vcpu *vcpu) > { > + struct kvm_run *kvm_run = vcpu->run; > + > if (kvm_run->kvm_dirty_regs & KVM_SYNC_PREFIX) > kvm_s390_set_prefix(vcpu, kvm_run->s.regs.prefix); > if (kvm_run->kvm_dirty_regs & KVM_SYNC_CRS) { > @@ -4272,7 +4275,7 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > > /* Sync fmt2 only data */ > if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) { > - sync_regs_fmt2(vcpu, kvm_run); > + sync_regs_fmt2(vcpu); > } else { > /* > * In several places we have to modify our internal view to > @@ -4291,8 +4294,10 @@ static void sync_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > kvm_run->kvm_dirty_regs = 0; > } > > -static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > +static void store_regs_fmt2(struct kvm_vcpu *vcpu) > { > + struct kvm_run *kvm_run = vcpu->run; > + > kvm_run->s.regs.todpr = vcpu->arch.sie_block->todpr; > kvm_run->s.regs.pp = vcpu->arch.sie_block->pp; > kvm_run->s.regs.gbea = vcpu->arch.sie_block->gbea; > @@ -4312,8 +4317,10 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > /* SIE will save etoken directly into SDNX and therefore kvm_run */ > } > > -static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > +static void store_regs(struct kvm_vcpu *vcpu) > { > + struct kvm_run *kvm_run = vcpu->run; > + > kvm_run->psw_mask = vcpu->arch.sie_block->gpsw.mask; > kvm_run->psw_addr = vcpu->arch.sie_block->gpsw.addr; > kvm_run->s.regs.prefix = kvm_s390_get_prefix(vcpu); > @@ -4332,7 +4339,7 @@ static void store_regs(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) > current->thread.fpu.fpc = vcpu->arch.host_fpregs.fpc; > current->thread.fpu.regs = vcpu->arch.host_fpregs.regs; > if (likely(!kvm_s390_pv_cpu_is_protected(vcpu))) > - store_regs_fmt2(vcpu, kvm_run); > + store_regs_fmt2(vcpu); > } > > int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > @@ -4370,7 +4377,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > goto out; > } > > - sync_regs(vcpu, kvm_run); > + sync_regs(vcpu); > enable_cpu_timer_accounting(vcpu); > > might_fault(); > @@ -4392,7 +4399,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > } > > disable_cpu_timer_accounting(vcpu); > - store_regs(vcpu, kvm_run); > + store_regs(vcpu); > > kvm_sigset_deactivate(vcpu); > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters 2020-06-23 15:31 ` [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters Christian Borntraeger @ 2020-06-24 2:39 ` Tianjia Zhang 2020-07-10 7:48 ` Paolo Bonzini 1 sibling, 0 replies; 7+ messages in thread From: Tianjia Zhang @ 2020-06-24 2:39 UTC (permalink / raw) To: Christian Borntraeger, pbonzini, tsbogend, paulus, mpe, benh, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel On 2020/6/23 23:31, Christian Borntraeger wrote: > > > On 23.06.20 15:14, Tianjia Zhang wrote: >> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' >> structure. For historical reasons, many kvm-related function parameters >> retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This >> patch does a unified cleanup of these remaining redundant parameters. >> >> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/s390/kvm/kvm-s390.c | 23 +++++++++++++++-------- >> 1 file changed, 15 insertions(+), 8 deletions(-) > > Tinajia, > > I have trouble seeing value in this particular patch. We add LOCs > without providing any noticable benefit. All other patches in this series at > least reduce the amount of code. So I would defer this to Paolo if he prefers > to have this way across all architectures. Yes, this is a full architecture optimization. Some of the architecture optimization has been merged into the mainline. I think it is necessary to unify this optimization. This is also the meaning of Paolo. You can refer to the email of the previous version: https://lkml.org/lkml/2020/4/27/16 Thanks, Tianjia _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters 2020-06-23 15:31 ` [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters Christian Borntraeger 2020-06-24 2:39 ` Tianjia Zhang @ 2020-07-10 7:48 ` Paolo Bonzini 1 sibling, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2020-07-10 7:48 UTC (permalink / raw) To: Christian Borntraeger, Tianjia Zhang, tsbogend, paulus, mpe, benh, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel On 23/06/20 17:31, Christian Borntraeger wrote: > > I have trouble seeing value in this particular patch. We add LOCs > without providing any noticable benefit. All other patches in this series at > least reduce the amount of code. So I would defer this to Paolo if he prefers > to have this way across all architectures. Yes, it adds lines of code but they're just + struct kvm_run *kvm_run = vcpu->run; You could avoid the LoC increase by repeating vcpu->run over and over, but I think the code overall is clearer. Paolo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/5] clean up redundant 'kvm_run' parameters [not found] <20200623131418.31473-1-tianjia.zhang@linux.alibaba.com> [not found] ` <20200623131418.31473-2-tianjia.zhang@linux.alibaba.com> @ 2020-07-10 7:32 ` Tianjia Zhang 2020-07-10 8:06 ` Paolo Bonzini [not found] ` <20200623131418.31473-4-tianjia.zhang@linux.alibaba.com> 2 siblings, 1 reply; 7+ messages in thread From: Tianjia Zhang @ 2020-07-10 7:32 UTC (permalink / raw) To: pbonzini, tsbogend, paulus, mpe, benh, borntraeger, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel Hi Paolo, Any opinion on this series patches? Can I help with this patchset ? Thanks and best, Tianjia On 2020/6/23 21:14, Tianjia Zhang wrote: > In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' > structure. For historical reasons, many kvm-related function parameters > retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This > patch does a unified cleanup of these remaining redundant parameters. > > This series of patches has completely cleaned the architecture of > arm64, mips, ppc, and s390 (no such redundant code on x86). Due to > the large number of modified codes, a separate patch is made for each > platform. On the ppc platform, there is also a redundant structure > pointer of 'kvm_run' in 'vcpu_arch', which has also been cleaned > separately. > > --- > v6 changes: > Rearrange patch sets, only keep the unmerged patch. > rebase on mainline. > > v5 change: > ppc: fix for review. > > v4 change: > mips: fixes two errors in entry.c. > > v3 change: > Keep the existing `vcpu->run` in the function body unchanged. > > v2 change: > s390 retains the original variable name and minimizes modification. > > Tianjia Zhang (5): > KVM: s390: clean up redundant 'kvm_run' parameters > KVM: arm64: clean up redundant 'kvm_run' parameters > KVM: PPC: clean up redundant kvm_run parameters in assembly > KVM: MIPS: clean up redundant 'kvm_run' parameters > KVM: MIPS: clean up redundant kvm_run parameters in assembly > > arch/arm64/include/asm/kvm_coproc.h | 12 +-- > arch/arm64/include/asm/kvm_host.h | 11 +-- > arch/arm64/include/asm/kvm_mmu.h | 2 +- > arch/arm64/kvm/arm.c | 6 +- > arch/arm64/kvm/handle_exit.c | 36 ++++---- > arch/arm64/kvm/mmio.c | 11 +-- > arch/arm64/kvm/mmu.c | 5 +- > arch/arm64/kvm/sys_regs.c | 13 ++- > arch/mips/include/asm/kvm_host.h | 32 ++------ > arch/mips/kvm/emulate.c | 59 +++++-------- > arch/mips/kvm/entry.c | 21 ++--- > arch/mips/kvm/mips.c | 14 ++-- > arch/mips/kvm/trap_emul.c | 114 +++++++++++--------------- > arch/mips/kvm/vz.c | 26 +++--- > arch/powerpc/include/asm/kvm_ppc.h | 2 +- > arch/powerpc/kvm/book3s_interrupts.S | 22 +++-- > arch/powerpc/kvm/book3s_pr.c | 9 +- > arch/powerpc/kvm/booke.c | 9 +- > arch/powerpc/kvm/booke_interrupts.S | 9 +- > arch/powerpc/kvm/bookehv_interrupts.S | 10 +-- > arch/s390/kvm/kvm-s390.c | 23 ++++-- > 21 files changed, 188 insertions(+), 258 deletions(-) > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 0/5] clean up redundant 'kvm_run' parameters 2020-07-10 7:32 ` [PATCH v6 0/5] " Tianjia Zhang @ 2020-07-10 8:06 ` Paolo Bonzini 0 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2020-07-10 8:06 UTC (permalink / raw) To: Tianjia Zhang, tsbogend, paulus, mpe, benh, borntraeger, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel On 10/07/20 09:32, Tianjia Zhang wrote: > Hi Paolo, > > Any opinion on this series patches? Can I help with this patchset ? I was hoping to have some Tested-by, for now I'm queuing patches 1 and 2. Thanks, Paolo > Thanks and best, > Tianjia > > On 2020/6/23 21:14, Tianjia Zhang wrote: >> In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' >> structure. For historical reasons, many kvm-related function parameters >> retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This >> patch does a unified cleanup of these remaining redundant parameters. >> >> This series of patches has completely cleaned the architecture of >> arm64, mips, ppc, and s390 (no such redundant code on x86). Due to >> the large number of modified codes, a separate patch is made for each >> platform. On the ppc platform, there is also a redundant structure >> pointer of 'kvm_run' in 'vcpu_arch', which has also been cleaned >> separately. >> >> --- >> v6 changes: >> Rearrange patch sets, only keep the unmerged patch. >> rebase on mainline. >> >> v5 change: >> ppc: fix for review. >> >> v4 change: >> mips: fixes two errors in entry.c. >> >> v3 change: >> Keep the existing `vcpu->run` in the function body unchanged. >> >> v2 change: >> s390 retains the original variable name and minimizes modification. >> >> Tianjia Zhang (5): >> KVM: s390: clean up redundant 'kvm_run' parameters >> KVM: arm64: clean up redundant 'kvm_run' parameters >> KVM: PPC: clean up redundant kvm_run parameters in assembly >> KVM: MIPS: clean up redundant 'kvm_run' parameters >> KVM: MIPS: clean up redundant kvm_run parameters in assembly >> >> arch/arm64/include/asm/kvm_coproc.h | 12 +-- >> arch/arm64/include/asm/kvm_host.h | 11 +-- >> arch/arm64/include/asm/kvm_mmu.h | 2 +- >> arch/arm64/kvm/arm.c | 6 +- >> arch/arm64/kvm/handle_exit.c | 36 ++++---- >> arch/arm64/kvm/mmio.c | 11 +-- >> arch/arm64/kvm/mmu.c | 5 +- >> arch/arm64/kvm/sys_regs.c | 13 ++- >> arch/mips/include/asm/kvm_host.h | 32 ++------ >> arch/mips/kvm/emulate.c | 59 +++++-------- >> arch/mips/kvm/entry.c | 21 ++--- >> arch/mips/kvm/mips.c | 14 ++-- >> arch/mips/kvm/trap_emul.c | 114 +++++++++++--------------- >> arch/mips/kvm/vz.c | 26 +++--- >> arch/powerpc/include/asm/kvm_ppc.h | 2 +- >> arch/powerpc/kvm/book3s_interrupts.S | 22 +++-- >> arch/powerpc/kvm/book3s_pr.c | 9 +- >> arch/powerpc/kvm/booke.c | 9 +- >> arch/powerpc/kvm/booke_interrupts.S | 9 +- >> arch/powerpc/kvm/bookehv_interrupts.S | 10 +-- >> arch/s390/kvm/kvm-s390.c | 23 ++++-- >> 21 files changed, 188 insertions(+), 258 deletions(-) >> > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20200623131418.31473-4-tianjia.zhang@linux.alibaba.com>]
* Re: [PATCH v6 3/5] KVM: PPC: clean up redundant kvm_run parameters in assembly [not found] ` <20200623131418.31473-4-tianjia.zhang@linux.alibaba.com> @ 2020-07-10 7:57 ` Paolo Bonzini 2020-07-23 6:19 ` Paul Mackerras 1 sibling, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2020-07-10 7:57 UTC (permalink / raw) To: Tianjia Zhang, tsbogend, paulus, mpe, benh, borntraeger, frankja, david, cohuck, heiko.carstens, gor, sean.j.christopherson, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp, x86, hpa, maz, james.morse, julien.thierry.kdev, suzuki.poulose, christoffer.dall, peterx, thuth, chenhuacai Cc: linux-s390, kvm, linux-mips, kvm-ppc, linux-kernel, linuxppc-dev, kvmarm, linux-arm-kernel On 23/06/20 15:14, Tianjia Zhang wrote: > > /* Load non-volatile guest state from the vcpu */ > - VCPU_LOAD_NVGPRS(r4) > + VCPU_LOAD_NVGPRS(r3) > > kvm_start_lightweight: > /* Copy registers into shadow vcpu so we can access them in real mode */ > - mr r3, r4 > bl FUNC(kvmppc_copy_to_svcpu) > nop > - REST_GPR(4, r1) > + REST_GPR(3, r1) > > #ifdef CONFIG_PPC_BOOK3S_64 > /* Get the dcbz32 flag */ > @@ -146,7 +144,7 @@ after_sprg3_load: Below, there are a bunch of references to r3 and r4 left rldicl r3, r3, 0, 63 /* r3 &= 1 */ stb r3, HSTATE_RESTORE_HID5(r13) /* Load up guest SPRG3 value, since it's user readable */ lwz r3, VCPU_SHAREDBE(r4) <<< cmpwi r3, 0 ld r5, VCPU_SHARED(r4) <<< where r3 is also destroyed. So I'd rather have three patches: - one that is as small as possible, changing the prototypes and adding mr r4, r3 - one that entirely swaps out r3 and r4. This would be the hard one to review! - one that cleans up the prologue and epilogue But overall I think it's simplest to just leave out this patch. Paolo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 3/5] KVM: PPC: clean up redundant kvm_run parameters in assembly [not found] ` <20200623131418.31473-4-tianjia.zhang@linux.alibaba.com> 2020-07-10 7:57 ` [PATCH v6 3/5] KVM: PPC: clean up redundant kvm_run parameters in assembly Paolo Bonzini @ 2020-07-23 6:19 ` Paul Mackerras 1 sibling, 0 replies; 7+ messages in thread From: Paul Mackerras @ 2020-07-23 6:19 UTC (permalink / raw) To: Tianjia Zhang Cc: wanpengli, kvm, david, benh, heiko.carstens, peterx, linux-mips, hpa, kvmarm, linux-s390, frankja, chenhuacai, maz, joro, x86, borntraeger, mingo, julien.thierry.kdev, thuth, gor, suzuki.poulose, kvm-ppc, bp, tglx, linux-arm-kernel, jmattson, tsbogend, cohuck, christoffer.dall, sean.j.christopherson, linux-kernel, james.morse, mpe, pbonzini, vkuznets, linuxppc-dev On Tue, Jun 23, 2020 at 09:14:16PM +0800, Tianjia Zhang wrote: > In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' > structure. For historical reasons, many kvm-related function parameters > retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This > patch does a unified cleanup of these remaining redundant parameters. Thanks, patch applied to my kvm-ppc-next branch, with fixes. Paul. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-23 6:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200623131418.31473-1-tianjia.zhang@linux.alibaba.com>
[not found] ` <20200623131418.31473-2-tianjia.zhang@linux.alibaba.com>
2020-06-23 15:31 ` [PATCH v6 1/5] KVM: s390: clean up redundant 'kvm_run' parameters Christian Borntraeger
2020-06-24 2:39 ` Tianjia Zhang
2020-07-10 7:48 ` Paolo Bonzini
2020-07-10 7:32 ` [PATCH v6 0/5] " Tianjia Zhang
2020-07-10 8:06 ` Paolo Bonzini
[not found] ` <20200623131418.31473-4-tianjia.zhang@linux.alibaba.com>
2020-07-10 7:57 ` [PATCH v6 3/5] KVM: PPC: clean up redundant kvm_run parameters in assembly Paolo Bonzini
2020-07-23 6:19 ` Paul Mackerras
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).