diff mbox series

[V3] KVM: svm: merge incomplete IPI emulation handling

Message ID 1553076466-29446-1-git-send-email-lirongqing@baidu.com (mailing list archive)
State New, archived
Headers show
Series [V3] KVM: svm: merge incomplete IPI emulation handling | expand

Commit Message

Li,Rongqing March 20, 2019, 10:07 a.m. UTC 
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 and
wide the comment out to 80 columns

Co-developed-by: Zhang Yu <zhangyu31@baidu.com>
Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
v2->v3: use Co-developed-by, instead of Signed-off-by
v1->v2: make cases statements are back-to-back and wide the comment out
        to 80 columns as suggested by Sean Christopherson

 arch/x86/kvm/svm.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

Comments

Sean Christopherson March 20, 2019, 2:17 p.m. UTC | #1 
On Wed, Mar 20, 2019 at 06:07:46PM +0800, Li RongQing wrote:
> 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 and
> wide the comment out to 80 columns
> 
> Co-developed-by: Zhang Yu <zhangyu31@baidu.com>

Zhang Yu's SOB is also needed when using Co-developed-by, e.g.:

Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>


Documentation/process/submitting-patches.rst:

  A Co-developed-by: states that the patch was also created by another developer
  along with the original author.  This is useful at times when multiple people
  work on a single patch.  Note, this person also needs to have a Signed-off-by:
  line in the patch as well.

> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> v2->v3: use Co-developed-by, instead of Signed-off-by
> v1->v2: make cases statements are back-to-back and wide the comment out
>         to 80 columns as suggested by Sean Christopherson
> 
>  arch/x86/kvm/svm.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index c6613d1dfa75..db6946791663 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4498,26 +4498,18 @@ static int avic_incomplete_ipi_interception(struct vcpu_svm *svm)
>  
>  	switch (id) {
>  	case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
> -		/*
> -		 * 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.
> -		 */
> -		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.
> +		 * 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);
> -- 
> 2.16.2
>
Sean Christopherson March 20, 2019, 2:26 p.m. UTC | #2 
On Wed, Mar 20, 2019 at 07:17:45AM -0700, Sean Christopherson wrote:
> On Wed, Mar 20, 2019 at 06:07:46PM +0800, Li RongQing wrote:
> > 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 and
> > wide the comment out to 80 columns
> > 
> > Co-developed-by: Zhang Yu <zhangyu31@baidu.com>
> 
> Zhang Yu's SOB is also needed when using Co-developed-by, e.g.:
> 
> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Co-developed-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> 
> 
> Documentation/process/submitting-patches.rst:
> 
>   A Co-developed-by: states that the patch was also created by another developer
>   along with the original author.  This is useful at times when multiple people
>   work on a single patch.  Note, this person also needs to have a Signed-off-by:
>   line in the patch as well.
> 
> > Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> > v2->v3: use Co-developed-by, instead of Signed-off-by
> > v1->v2: make cases statements are back-to-back and wide the comment out
> >         to 80 columns as suggested by Sean Christopherson
> > 
> >  arch/x86/kvm/svm.c | 28 ++++++++++------------------
> >  1 file changed, 10 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index c6613d1dfa75..db6946791663 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -4498,26 +4498,18 @@ static int avic_incomplete_ipi_interception(struct vcpu_svm *svm)
> >  
> >  	switch (id) {
> >  	case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
> > -		/*
> > -		 * 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.
> > -		 */
> > -		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.
> > +		 * 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);

Unfortunately it would appear this was all for naught as a revert for
commit bb218fbcfaaa ("svm: Fix AVIC incomplete IPI emulation") was
just sent.

http://lkml.kernel.org/r/20190320081158.2442-1-suravee.suthikulpanit@amd.com
diff mbox series

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c6613d1dfa75..db6946791663 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4498,26 +4498,18 @@  static int avic_incomplete_ipi_interception(struct vcpu_svm *svm)
 
 	switch (id) {
 	case AVIC_IPI_FAILURE_INVALID_INT_TYPE:
-		/*
-		 * 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.
-		 */
-		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.
+		 * 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);