Patchwork KVM: svm: merge incomplete IPI emulation handling

login
register
mail settings
Submitter Li RongQing
Date March 14, 2019, 2:19 a.m.
Message ID <1552529944-24864-1-git-send-email-lirongqing@baidu.com>
Download mbox | patch
Permalink /patch/748501/
State New
Headers show

Comments

Li RongQing - March 14, 2019, 2:19 a.m.
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@baidu.com>
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
---
 arch/x86/kvm/svm.c | 5 -----
 1 file changed, 5 deletions(-)
Vitaly Kuznetsov - March 14, 2019, 1:22 p.m.
Li RongQing <lirongqing@baidu.com> 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@baidu.com>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> ---
>  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.

>  	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.
Li RongQing - March 15, 2019, 1:13 a.m.
> -----邮件原件-----

> 发件人: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]

> 发送时间: 2019年3月14日 21:23

> 收件人: Li,Rongqing <lirongqing@baidu.com>

> 抄送: x86@kernel.org; kvm@vger.kernel.org

> 主题: Re: [PATCH] KVM: svm: merge incomplete IPI emulation handling

> 

> Li RongQing <lirongqing@baidu.com> 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@baidu.com>

> > Signed-off-by: Zhang Yu <zhangyu31@baidu.com>

> > ---

> >  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

Thanks

-RongQing

> >  	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.

> 

> --

> Vitaly
Vitaly Kuznetsov - March 15, 2019, 9:14 a.m.
"Li,Rongqing" <lirongqing@baidu.com> writes:

>> -----邮件原件-----
>> 发件人: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
>> 发送时间: 2019年3月14日 21:23
>> 收件人: Li,Rongqing <lirongqing@baidu.com>
>> 抄送: x86@kernel.org; kvm@vger.kernel.org
>> 主题: Re: [PATCH] KVM: svm: merge incomplete IPI emulation handling
>> 
>> Li RongQing <lirongqing@baidu.com> 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@baidu.com>
>> > Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
>> > ---
>> >  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).

Patch

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;
 	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.