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

Commit Message

Li RongQing March 18, 2019, 12:56 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

Signed-off-by: Li RongQing <[e-mail]>
Signed-off-by: Zhang Yu <[e-mail]>
---
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

Borislav Petkov March 18, 2019, 7:02 a.m. UTC | #1

On Mon, Mar 18, 2019 at 08:56:53AM +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
> 
> Signed-off-by: Li RongQing <[e-mail]>
> Signed-off-by: Zhang Yu <[e-mail]>

This SOB chain is wrong. See

  11) Sign your work - the Developer's Certificate of Origin

in Documentation/process/submitting-patches.rst for details.

Li RongQing March 18, 2019, 7:53 a.m. UTC | #2

> -----邮件原件-----
> 发件人: [e-mail] [mailto:[e-mail]] 代
> 表 Borislav Petkov
> 发送时间: 2019年3月18日 15:03
> 收件人: Li,Rongqing <[e-mail]>
> 抄送: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> 主题: Re: [PATCH][V2] KVM: svm: merge incomplete IPI emulation handling
> 
> On Mon, Mar 18, 2019 at 08:56:53AM +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
> >
> > Signed-off-by: Li RongQing <[e-mail]>
> > Signed-off-by: Zhang Yu <[e-mail]>
> 
> This SOB chain is wrong. See
> 
>   11) Sign your work - the Developer's Certificate of Origin
> 

Do you means I should add sign-off-by: Sean Christopherson? 
Should I send V3? or Is it oK with the below ?


Signed-off-by: Sean Christopherson <[e-mail]>

-RongQing


> in Documentation/process/submitting-patches.rst for details.
> 
> --
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

Borislav Petkov March 18, 2019, 8:09 a.m. UTC | #3

On Mon, Mar 18, 2019 at 07:53:24AM +0000, Li,Rongqing wrote:
> Do you means I should add sign-off-by: Sean Christopherson?
> Should I send V3? or Is it oK with the below ?
> 
> 
> Signed-off-by: Sean Christopherson <[e-mail]>
> 
> -RongQing
> 
> 
> > in Documentation/process/submitting-patches.rst for details.

No, read this doc to understand first that sprinkling SOBs is not what
we do and those things have meaning.

Li RongQing March 20, 2019, 8:45 a.m. UTC | #4

> -----邮件原件-----
> 发件人: [e-mail] [mailto:[e-mail]] 代
> 表 Borislav Petkov
> 发送时间: 2019年3月18日 16:10
> 收件人: Li,Rongqing <[e-mail]>
> 抄送: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> 主题: Re: 答复: [PATCH][V2] KVM: svm: merge incomplete IPI emulation
> handling
> 
> On Mon, Mar 18, 2019 at 07:53:24AM +0000, Li,Rongqing wrote:
> > Do you means I should add sign-off-by: Sean Christopherson?
> > Should I send V3? or Is it oK with the below ?
> >
> >
> > Signed-off-by: Sean Christopherson <[e-mail]>
> >
> > -RongQing
> >
> >
> > > in Documentation/process/submitting-patches.rst for details.
> 
> No, read this doc to understand first that sprinkling SOBs is not what we do and
> those things have meaning.
> 


Read, but no clear where are wrong

-RongQing

> --
> Regards/Gruss,
>     Boris.
> 
> Good mailing practices for 400: avoid top-posting and trim the reply.

Borislav Petkov March 20, 2019, 9:05 a.m. UTC | #5

On Wed, Mar 20, 2019 at 08:45:21AM +0000, Li,Rongqing wrote:
> Read, but no clear where are wrong

What is the purpose of the other Signed-off-by's you've added? What are
they supposed to express?

When you answer those questions to yourself, read

12) When to use Acked-by:, Cc:, and Co-developed-by:

and

13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

sections and see if that rings any bells.

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