> -----邮件原件----- > 发件人: Sean Christopherson [mailto:sean.j.christopherson@xxxxxxxxx] > 发送时间: 2019年3月15日 22:51 > 收件人: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > 抄送: Li,Rongqing <lirongqing@xxxxxxxxx>; x86@xxxxxxxxxx; > kvm@xxxxxxxxxxxxxxx > 主题: Re: 答复: [PATCH] KVM: svm: merge incomplete IPI emulation handling > > On Fri, Mar 15, 2019 at 10:14:47AM +0100, Vitaly Kuznetsov wrote: > > "Li,Rongqing" <lirongqing@xxxxxxxxx> writes: > > > > >> -----邮件原件----- > > >> 发件人: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > > >> 发送时间: 2019年3月14日 21:23 > > >> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx> > > >> 抄送: x86@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx > > >> 主题: Re: [PATCH] KVM: svm: merge incomplete IPI emulation handling > > >> > > >> Li RongQing <lirongqing@xxxxxxxxx> writes: > > >> > > >> > Invalid int type emulation and target not running emulation have > > >> > same codes, which update APIC ICR high/low registers, and emulate > > >> > sending the IPI. > > >> > > > >> > so fall through this switch cases to reduce duplicate codes > > >> > > > >> > Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx> > > >> > Signed-off-by: Zhang Yu <zhangyu31@xxxxxxxxx> > > >> > --- > > >> > arch/x86/kvm/svm.c | 5 ----- > > >> > 1 file changed, 5 deletions(-) > > >> > > > >> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index > > >> > 276ab8ab6c95..2e0c9cb349d2 100644 > > >> > --- a/arch/x86/kvm/svm.c > > >> > +++ b/arch/x86/kvm/svm.c > > >> > @@ -4508,12 +4508,7 @@ static int > > >> > avic_incomplete_ipi_interception(struct > > >> vcpu_svm *svm) > > >> > * formats are supported. All other IPI types cause > > >> > * a #VMEXIT, which needs to emulated. > > >> > */ > > >> > - kvm_lapic_reg_write(apic, APIC_ICR2, icrh); > > >> > - kvm_lapic_reg_write(apic, APIC_ICR, icrl); > > >> > - break; > > >> > > >> > > >> Doesn't checkpatch.pl complain about missing 'Fall through' comment > > >> here? In any case it's probably worth it adding it. > > >> > > > > > > This place is a empty case block, which does not need the mark "fall > through" > > > So checkpatch.pl did not complain, and gcc did not complain > > > > > > And I have sent patch to remove this unnecessary "fall through" > > > before > > > > > > > (I'm not insisting on the change) > > > > Generally it makes sense to explicitly say 'fall through' in each 'case' > > without a 'break' to assist readers of the code, it's very easy to > > miss this subtle thing otherwise (just IMO). > > How about redoing the comments so that the cases statements are > back-to-back? > I think that would resolve Vitaly's concern but also avoid the unnecessary "fall > through" annotation. It also provides an opportunity to trim a few lines by > widing the comment out to 80 columns. > > E.g.: Thanks, I will send V2 -RongQing > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index > 426039285fd1..71de264c6865 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -4502,31 +4502,21 @@ static int avic_incomplete_ipi_interception(struct > vcpu_svm *svm) > > switch (id) { > case AVIC_IPI_FAILURE_INVALID_INT_TYPE: > + case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING: > /* > - * AVIC hardware handles the generation of > - * IPIs when the specified Message Type is Fixed > - * (also known as fixed delivery mode) and > - * the Trigger Mode is edge-triggered. The hardware > - * also supports self and broadcast delivery modes > - * specified via the Destination Shorthand(DSH) > - * field of the ICRL. Logical and physical APIC ID > - * formats are supported. All other IPI types cause > - * a #VMEXIT, which needs to emulated. > + * AVIC hardware handles the generation of IPIs when the > + * specified Message Type is Fixed (also known as fixed > + * delivery mode) and the Trigger Mode is edge-triggered. > + * The hardware also supports self and broadcast delivery > modes > + * specified via the Destination Shorthand(DSH) field of the > + * ICRL. Logical and physical APIC ID formats are > supported. > + * All other IPI types cause a #VMEXIT, which needs to > + * emulated. AVIC hardware also cannot handle IPIs to > CPUs > + * that are not running, emulate the IPI for that case as > well. > */ > kvm_lapic_reg_write(apic, APIC_ICR2, icrh); > kvm_lapic_reg_write(apic, APIC_ICR, icrl); > break; > - case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING: { > - struct kvm_lapic *apic = svm->vcpu.arch.apic; > - > - /* > - * Update ICR high and low, then emulate sending IPI, > - * which is handled when writing APIC_ICR. > - */ > - kvm_lapic_reg_write(apic, APIC_ICR2, icrh); > - kvm_lapic_reg_write(apic, APIC_ICR, icrl); > - break; > - } > case AVIC_IPI_FAILURE_INVALID_TARGET: > WARN_ONCE(1, "Invalid IPI target: index=%u, vcpu=%d, > icr=%#0x:%#0x\n", > index, svm->vcpu.vcpu_id, icrh, icrl);
