[PATCH] KVM: LAPIC: Do not mask the local interrupts when LAPIC is sw disabled

[PATCH] KVM: LAPIC: Do not mask the local interrupts when LAPIC is sw disabled

[Luwei Kang]

The current code will mask all the local interrupts in the local
vector table when the LAPIC is disabled by SVR (Spurious-Interrupt
Vector Register) "APIC Software Enable/Disable" flag (bit8).
This may block local interrupt be delivered to target vCPU
even if LAPIC is enabled by set SVR (bit8 == 1) after.

For example, reset vCPU will mask all the local interrupts and
set the SVR to default value FFH (LAPIC is disabled because
SVR[bit8] == 0). Guest may try to enable some local interrupts
(e.g. LVTPC) by clear bit16 of LVT entry before enable LAPIC.
But bit16 can't be cleared when LAPIC is "software disabled"
and this local interrupt still disabled after LAPIC "software
enabled".

This patch will not mask the local interrupts when LAPIC
is "software disabled" and add LAPIC "software enabled" checking
before deliver local interrupt.

Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/kvm/lapic.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index fcf42a3..a199f47 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1892,15 +1892,6 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 			mask |= APIC_SPIV_DIRECTED_EOI;
 		apic_set_spiv(apic, val & mask);
 		if (!(val & APIC_SPIV_APIC_ENABLED)) {
-			int i;
-			u32 lvt_val;
-
-			for (i = 0; i < KVM_APIC_LVT_NUM; i++) {
-				lvt_val = kvm_lapic_get_reg(apic,
-						       APIC_LVTT + 0x10 * i);
-				kvm_lapic_set_reg(apic, APIC_LVTT + 0x10 * i,
-					     lvt_val | APIC_LVT_MASKED);
-			}
 			apic_update_lvtt(apic);
 			atomic_set(&apic->lapic_timer.pending, 0);
 
@@ -1926,18 +1917,12 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 	case APIC_LVTPC:
 	case APIC_LVT1:
 	case APIC_LVTERR:
-		/* TODO: Check vector */
-		if (!kvm_apic_sw_enabled(apic))
-			val |= APIC_LVT_MASKED;
-
 		val &= apic_lvt_mask[(reg - APIC_LVTT) >> 4];
 		kvm_lapic_set_reg(apic, reg, val);
 
 		break;
 
 	case APIC_LVTT:
-		if (!kvm_apic_sw_enabled(apic))
-			val |= APIC_LVT_MASKED;
 		val &= (apic_lvt_mask[0] | apic->lapic_timer.timer_mode_mask);
 		kvm_lapic_set_reg(apic, APIC_LVTT, val);
 		apic_update_lvtt(apic);
@@ -2260,7 +2245,7 @@ int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
 	u32 reg = kvm_lapic_get_reg(apic, lvt_type);
 	int vector, mode, trig_mode;
 
-	if (kvm_apic_hw_enabled(apic) && !(reg & APIC_LVT_MASKED)) {
+	if (apic_enabled(apic) && !(reg & APIC_LVT_MASKED)) {
 		vector = reg & APIC_VECTOR_MASK;
 		mode = reg & APIC_MODE_MASK;
 		trig_mode = reg & APIC_LVT_LEVEL_TRIGGER;
@@ -2363,7 +2348,7 @@ int kvm_apic_accept_pic_intr(struct kvm_vcpu *vcpu)
 	u32 lvt0 = kvm_lapic_get_reg(vcpu->arch.apic, APIC_LVT0);
 	int r = 0;
 
-	if (!kvm_apic_hw_enabled(vcpu->arch.apic))
+	if (!apic_enabled(vcpu->arch.apic))
 		r = 1;
 	if ((lvt0 & APIC_LVT_MASKED) == 0 &&
 	    GET_APIC_DELIVERY_MODE(lvt0) == APIC_MODE_EXTINT)
-- 
1.8.3.1

Re: [PATCH] KVM: LAPIC: Do not mask the local interrupts when LAPIC is sw disabled

[Sean Christopherson]

On Tue, May 21, 2019 at 06:44:15PM +0800, Luwei Kang wrote:
> The current code will mask all the local interrupts in the local
> vector table when the LAPIC is disabled by SVR (Spurious-Interrupt
> Vector Register) "APIC Software Enable/Disable" flag (bit8).
> This may block local interrupt be delivered to target vCPU
> even if LAPIC is enabled by set SVR (bit8 == 1) after.

The current code aligns with the SDM, which states:

  Local APIC State After It Has Been Software Disabled

  When the APIC software enable/disable flag in the spurious interrupt
  vector register has been explicitly cleared (as opposed to being cleared
  during a power up or reset), the local APIC is temporarily disabled.
  The operation and response of a local APIC while in this software-
  disabled state is as follows:

    - The mask bits for all the LVT entries are set. Attempts to reset
      these bits will be ignored.

RE: [PATCH] KVM: LAPIC: Do not mask the local interrupts when LAPIC is sw disabled

[Kang, Luwei]

> -----Original Message-----
> From: Christopherson, Sean J
> Sent: Friday, May 31, 2019 2:46 AM
> To: Kang, Luwei <luwei.kang@intel.com>
> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; pbonzini@redhat.com; rkrcmar@redhat.com; tglx@linutronix.de;
> mingo@redhat.com; bp@alien8.de; hpa@zytor.com; x86@kernel.org
> Subject: Re: [PATCH] KVM: LAPIC: Do not mask the local interrupts when LAPIC is sw disabled
> 
> On Tue, May 21, 2019 at 06:44:15PM +0800, Luwei Kang wrote:
> > The current code will mask all the local interrupts in the local
> > vector table when the LAPIC is disabled by SVR (Spurious-Interrupt
> > Vector Register) "APIC Software Enable/Disable" flag (bit8).
> > This may block local interrupt be delivered to target vCPU even if
> > LAPIC is enabled by set SVR (bit8 == 1) after.
> 
> The current code aligns with the SDM, which states:
> 
>   Local APIC State After It Has Been Software Disabled
> 
>   When the APIC software enable/disable flag in the spurious interrupt
>   vector register has been explicitly cleared (as opposed to being cleared
>   during a power up or reset), the local APIC is temporarily disabled.
>   The operation and response of a local APIC while in this software-
>   disabled state is as follows:
> 
>     - The mask bits for all the LVT entries are set. Attempts to reset
>       these bits will be ignored.

Thanks for Sean's reminder. 
I make this patch because I found the PMI from Intel PT can't be inject to target vCPU when there have multi vCPU in guest and the Intel PT interrupt happened on not the first vCPU (i.e. not vCPU0).  The interrupt blocked in kvm_apic_local_deliver() function and can't pass the APIC_LVT_MASKED flag check (LVTPC is masked from start to end). The KVM Guest will enabled the LVTPC during LAPIC is software disabled and enabled LAPIC after during VM bootup, but LVTPC is still disabled. Guest PT driver didn't enabled LVTPC before enable PT as well. But the Guest performance monitor counter driver will enabled LVTPC in each time before using PMU. I will do more check on this. Thank you.

Luwei Kang