Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 03/09/2015 07:18, Nakajima, Jun wrote:
> On Wed, Sep 2, 2015 at 3:38 PM, Steve Rutherford  
> wrote:
>> On Thu, Aug 13, 2015 at 09:31:48AM +0200, Paolo Bonzini wrote:
>> Pinging this thread.
>>
>> Should I put together a patch to make split irqchip work properly with the 
>> old TMR behavior?
> 
> Yes, please.
> 
> Intel® 64 and IA-32 Architectures Software Developer’s Manual:
> 
> 24.11.4 Software Access to Related Structures
> 
> In addition to data in the VMCS region itself, VMX non-root operation
> can be controlled by data structures that are
> referenced by pointers in a VMCS (for example, the I/O bitmaps).

The SDM does not list these data structures however.  It also does not
say that, whenever a page is pointed to by the VMCS, *the whole page*
counts as a control data structure.

In http://article.gmane.org/gmane.linux.kernel/2011131 I explained my
reading of the manual and why the vTMR is IMO not part of the control
data structures.  In a nutshell, the vISR, vIRR, vTPR, vPPR etc. are
control data structures, but the other fields look to me like they are
just data.  Jun, can you find anything wrong in the reasoning?

Next week I'll write test cases for it, which are worthwhile anyway.  In
the meanwhile, if Steve wants to prepare a patch that injects level
interrupts (those that have to set the vTMR to 1) while the VCPU is not
running, that would also work great for me as I was going to look into
that anyway.

Paolo

 While
> the pointers to these data structures are
> parts of the VMCS, the data structures themselves are not. They are
> not accessible using VMREAD and VMWRITE
> but by ordinary memory writes.
> Software should ensure that each such data structure is modified only
> when no logical processor with a current
> VMCS that references it is in VMX non-root operation. Doing otherwise
> may lead to unpredictable behavior
> (including behaviors identified in Section 24.11.1)
> 
> 
> 29.6 POSTED-INTERRUPT PROCESSING
> ...
> Use of the posted-interrupt descriptor differs from that of other data
> structures that are referenced by pointers in
> a VMCS. There is a general requirement that software ensure that each
> such data structure is modified only when
> no logical processor with a current VMCS that references it is in VMX
> non-root operation. That requirement does
> not apply to the posted-interrupt descriptor. There is a requirement,
> however, that such modifications be done
> using locked read-modify-write instructions.
> 
> 
>>
>>>
>>>
>>> On 13/08/2015 08:35, Zhang, Yang Z wrote:
> You may be right. It is safe if no future hardware plans to use
> it. Let me check with our hardware team to see whether it will be
> used or not in future.

 After checking with Jun, there is no guarantee that the guest running
 on another CPU will operate properly if hypervisor modify the vTMR
 from another CPU. So the hypervisor should not to do it.
>>>
>>> I guess I can cause a vmexit on level-triggered interrupts, it's not a
>>> big deal, but no weasel words, please.
>>>
>>> What's going to break, and where is it documented?
>>>
>>> Paolo
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Nakajima, Jun]

On Wed, Sep 2, 2015 at 3:38 PM, Steve Rutherford  wrote:
> On Thu, Aug 13, 2015 at 09:31:48AM +0200, Paolo Bonzini wrote:
> Pinging this thread.
>
> Should I put together a patch to make split irqchip work properly with the 
> old TMR behavior?

Yes, please.

Intel® 64 and IA-32 Architectures Software Developer’s Manual:

24.11.4 Software Access to Related Structures

In addition to data in the VMCS region itself, VMX non-root operation
can be controlled by data structures that are
referenced by pointers in a VMCS (for example, the I/O bitmaps). While
the pointers to these data structures are
parts of the VMCS, the data structures themselves are not. They are
not accessible using VMREAD and VMWRITE
but by ordinary memory writes.
Software should ensure that each such data structure is modified only
when no logical processor with a current
VMCS that references it is in VMX non-root operation. Doing otherwise
may lead to unpredictable behavior
(including behaviors identified in Section 24.11.1)


29.6 POSTED-INTERRUPT PROCESSING
...
Use of the posted-interrupt descriptor differs from that of other data
structures that are referenced by pointers in
a VMCS. There is a general requirement that software ensure that each
such data structure is modified only when
no logical processor with a current VMCS that references it is in VMX
non-root operation. That requirement does
not apply to the posted-interrupt descriptor. There is a requirement,
however, that such modifications be done
using locked read-modify-write instructions.


>
>>
>>
>> On 13/08/2015 08:35, Zhang, Yang Z wrote:
>> >> You may be right. It is safe if no future hardware plans to use
>> >> it. Let me check with our hardware team to see whether it will be
>> >> used or not in future.
>> >
>> > After checking with Jun, there is no guarantee that the guest running
>> > on another CPU will operate properly if hypervisor modify the vTMR
>> > from another CPU. So the hypervisor should not to do it.
>>
>> I guess I can cause a vmexit on level-triggered interrupts, it's not a
>> big deal, but no weasel words, please.
>>
>> What's going to break, and where is it documented?
>>
>> Paolo
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Jun
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Steve Rutherford]

On Thu, Aug 13, 2015 at 09:31:48AM +0200, Paolo Bonzini wrote:
Pinging this thread.

Should I put together a patch to make split irqchip work properly with the old 
TMR behavior?

> 
> 
> On 13/08/2015 08:35, Zhang, Yang Z wrote:
> >> You may be right. It is safe if no future hardware plans to use
> >> it. Let me check with our hardware team to see whether it will be
> >> used or not in future.
> > 
> > After checking with Jun, there is no guarantee that the guest running
> > on another CPU will operate properly if hypervisor modify the vTMR
> > from another CPU. So the hypervisor should not to do it.
> 
> I guess I can cause a vmexit on level-triggered interrupts, it's not a
> big deal, but no weasel words, please.
> 
> What's going to break, and where is it documented?
> 
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Zhang, Yang Z wrote on 2015-08-04:
 Paolo Bonzini wrote on 2015-08-04:
 
 
 On 04/08/2015 02:46, Zhang, Yang Z wrote:
 It is a problem for split irqchip, where the EOI exit bitmap can
 be inferred from the IOAPIC routes but the TMR cannot.  The
 hardware behavior on the other hand can be implemented purely within the 
 LAPIC.
 
 So updating the TMR within LAPIC is the only solution to handle it?
 
 It's the simplest and the one that makes most sense.  Considering
 that TMR is a pretty obscure feature, it's unlikely that it will be
 accelerated in the future.
 
 You may be right. It is safe if no future hardware plans to use it.
 Let me check with our hardware team to see whether it will be used or not in 
 future.

After checking with Jun, there is no guarantee that the guest running on 
another CPU will operate properly if hypervisor modify the vTMR from another 
CPU. So the hypervisor should not to do it.

 
 
 Paolo
 
 
 Best regards,
 Yang



Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 13/08/2015 08:35, Zhang, Yang Z wrote:
 You may be right. It is safe if no future hardware plans to use
 it. Let me check with our hardware team to see whether it will be
 used or not in future.
 
 After checking with Jun, there is no guarantee that the guest running
 on another CPU will operate properly if hypervisor modify the vTMR
 from another CPU. So the hypervisor should not to do it.

I guess I can cause a vmexit on level-triggered interrupts, it's not a
big deal, but no weasel words, please.

What's going to break, and where is it documented?

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 04/08/2015 02:46, Zhang, Yang Z wrote:
  It is a problem for split irqchip, where the EOI exit bitmap can be
  inferred from the IOAPIC routes but the TMR cannot.  The hardware
  behavior on the other hand can be implemented purely within the LAPIC.
 
 So updating the TMR within LAPIC is the only solution to handle it?

It's the simplest and the one that makes most sense.  Considering that
TMR is a pretty obscure feature, it's unlikely that it will be
accelerated in the future.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-08-04:
 
 
 On 04/08/2015 02:46, Zhang, Yang Z wrote:
 It is a problem for split irqchip, where the EOI exit bitmap can be
 inferred from the IOAPIC routes but the TMR cannot.  The hardware
 behavior on the other hand can be implemented purely within the LAPIC.
 
 So updating the TMR within LAPIC is the only solution to handle it?
 
 It's the simplest and the one that makes most sense.  Considering that
 TMR is a pretty obscure feature, it's unlikely that it will be
 accelerated in the future.

You may be right. It is safe if no future hardware plans to use it. Let me 
check with our hardware team to see whether it will be used or not in future. 

 
 Paolo


Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-08-03:
 
 
 On 03/08/2015 12:23, Zhang, Yang Z wrote:
 In any case, the TMR behavior introduced by the APICv patches is
 completely different from the hardware behavior, so it has to be fixed.
 
 But any real problem with it?
 
 It is a problem for split irqchip, where the EOI exit bitmap can be
 inferred from the IOAPIC routes but the TMR cannot.  The hardware
 behavior on the other hand can be implemented purely within the LAPIC.

So updating the TMR within LAPIC is the only solution to handle it?

 
  The alternative is to inject level-triggered interrupts
 synchronously, without using posted interrupts.
 
 I'll write some testcases to understand the functioning of TMR in
 the virtual-APIC page, but the manual seems clear to me.
 
 Currently, no existing hardware will use TMR and will not cause any
 problem.(That's the reason why we leave it in Xen).But we don't know
 whether future hardware will use it or not(SDM always keeps changing
 :)).
 
 But that would be covered by a different execution control (for
 backwards compatibility).  We'll get there when such a feature is introduced.

Yes, we can leave it in future. But one concern is that it may hard to handle 
it at that time if someone also develops feature which rely on it (like current 
patch to split irqchip). 

 
 And per 24.11.4's description, the perfect solution is don't modify
 it. btw, IIRC, only TMR doesn't follow the rule. All other VMCS
 accesses are issued in right VMCS context.
 
 Yes, that's correct.  It's just the TMR.
 
 Paolo


Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-08-03:
 
 
 On 03/08/2015 04:37, Zhang, Yang Z wrote:
 Only virtualized APIC register reads use the virtual TMR
 registers (SDM
 29.4.2 or 29.5), but these just read data from the corresponding
 field in the virtual APIC page.
 
 24.11.4 Software Access to Related Structures In addition to data in
 the VMCS region itself, VMX non-root operation can be controlled by
 data structures that are referenced by pointers in a VMCS (for
 example, the I/O bitmaps). While the pointers to these data
 structures are parts of the VMCS, the data structures themselves are
 not. They are not accessible using VMREAD and VMWRITE but by
 ordinary memory writes.
 
 I don't think the TMR fields of the virtual-APIC page are among the
 data structures that _controls_ VMX non-root operations, because:
 
 * it is not part of the virtualized APIC state is listed in 29.1.1
 
 * read accesses from the APIC-access page simply return data from the
 corresponding page offset on the virtual-APIC page using the memory
 access type stored in IA32_VMX_BASIC_MSR.  I think this explicitly
 says that the effects of 24.11.1 (especially non-deterministic
 behavior after a write) do not apply here.
 
 In any case, the TMR behavior introduced by the APICv patches is
 completely different from the hardware behavior, so it has to be fixed.

But any real problem with it?

  The alternative is to inject level-triggered interrupts
 synchronously, without using posted interrupts.
 
 I'll write some testcases to understand the functioning of TMR in the
 virtual-APIC page, but the manual seems clear to me.

Currently, no existing hardware will use TMR and will not cause any 
problem.(That's the reason why we leave it in Xen).But we don't know whether 
future hardware will use it or not(SDM always keeps changing :)).And per 
24.11.4's description, the perfect solution is don't modify it. 
btw, IIRC, only TMR doesn't follow the rule. All other VMCS accesses are issued 
in right VMCS context.

 
 Paolo


Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 03/08/2015 04:37, Zhang, Yang Z wrote:
  Only virtualized APIC register reads use the virtual TMR registers (SDM
  29.4.2 or 29.5), but these just read data from the corresponding field
  in the virtual APIC page.
 
 24.11.4 Software Access to Related Structures
 In addition to data in the VMCS region itself, VMX non-root operation
 can be controlled by data structures that are referenced by pointers
 in a VMCS (for example, the I/O bitmaps). While the pointers to these
 data structures are parts of the VMCS, the data structures themselves
 are not. They are not accessible using VMREAD and VMWRITE but by
 ordinary memory writes.

I don't think the TMR fields of the virtual-APIC page are among the data
structures that _controls_ VMX non-root operations, because:

* it is not part of the virtualized APIC state is listed in 29.1.1

* read accesses from the APIC-access page simply return data from the
corresponding page offset on the virtual-APIC page using the memory
access type stored in IA32_VMX_BASIC_MSR.  I think this explicitly says
that the effects of 24.11.1 (especially non-deterministic behavior after
a write) do not apply here.

In any case, the TMR behavior introduced by the APICv patches is
completely different from the hardware behavior, so it has to be fixed.
 The alternative is to inject level-triggered interrupts synchronously,
without using posted interrupts.

I'll write some testcases to understand the functioning of TMR in the
virtual-APIC page, but the manual seems clear to me.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 03/08/2015 12:23, Zhang, Yang Z wrote:
  In any case, the TMR behavior introduced by the APICv patches is
  completely different from the hardware behavior, so it has to be fixed.
 
 But any real problem with it?

It is a problem for split irqchip, where the EOI exit bitmap can be
inferred from the IOAPIC routes but the TMR cannot.  The hardware
behavior on the other hand can be implemented purely within the LAPIC.

   The alternative is to inject level-triggered interrupts
  synchronously, without using posted interrupts.
  
  I'll write some testcases to understand the functioning of TMR in the
  virtual-APIC page, but the manual seems clear to me.
 
 Currently, no existing hardware will use TMR and will not cause any
 problem.(That's the reason why we leave it in Xen).But we don't know
 whether future hardware will use it or not(SDM always keeps changing
 :)).

But that would be covered by a different execution control (for
backwards compatibility).  We'll get there when such a feature is
introduced.

 And per 24.11.4's description, the perfect solution is don't
 modify it. btw, IIRC, only TMR doesn't follow the rule. All other
 VMCS accesses are issued in right VMCS context.

Yes, that's correct.  It's just the TMR.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-07-31:
 
 
 On 31/07/2015 01:26, Zhang, Yang Z wrote:
 Do not compute TMR in advance.  Instead, set the TMR just before
 the interrupt is accepted into the IRR.  This limits the coupling
 between IOAPIC and LAPIC.
 
 Uh.., it back to original way which is wrong. You cannot modify the
 apic page(here is the TMR reg) directly when the corresponding VMCS
 may be used at same time.
 
 Where is this documented?  The TMR is not part of the set of virtualized
 APIC registers (TPR, PPR, EOI, ISR, IRR, ICR+ICR2; SDM 29.1.1).
 
 Only virtualized APIC register reads use the virtual TMR registers (SDM
 29.4.2 or 29.5), but these just read data from the corresponding field
 in the virtual APIC page.

It's not only for virtual apic page. All data structures that is used by 
VMCS(either directly inside in VMCS or referenced by pointer in a VMCS) need 
follow this rule:

24.11.4 Software Access to Related Structures
In addition to data in the VMCS region itself, VMX non-root operation can be 
controlled by data structures that are
referenced by pointers in a VMCS (for example, the I/O bitmaps). While the 
pointers to these data structures are
parts of the VMCS, the data structures themselves are not. They are not 
accessible using VMREAD and VMWRITE
but by ordinary memory writes.
Software should ensure that each such data structure is modified only when no 
logical processor with a current
VMCS that references it is in VMX non-root operation. Doing otherwise may lead 
to unpredictable behavior
(including behaviors identified in Section 24.11.1).

 
 Paolo


Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-07-31:
 
 
 On 31/07/2015 04:49, Steve Rutherford wrote:
 Oh... Yeah. That's a damn good point, given that the interrupt can be
 injected from another thread while one is in that guest vcpu.
 
 Easiest time to update the TMR should be on guest entry through
 vcpu_scan_ioapic, as before.
 
 The best way to go is probably to ditch the new per vcpu EOI exit
 bitmap, and just update/use the TMR. There's no reason to duplicate
 that data in the representation of the apic (I suspect that the
 duplication was inspired by my mistaken notion of the TMR). The
 IOAPIC exit check can use the TMR instead.
 
 Based upon my reading of the SDM, the only reason that the eoi exit
 bitmaps are not the exact same as the TMR is that it is possible to
 have virtual-interrupt delivery enabled /without/ an apic access page
 (Note: V-ID = EOI exit bitmap enabled).
 
 Yang, do you happen to know if that is the case?
 
 [Note: Just looked back at the old code for updating the EOI exit
 bitmaps, which for some reason was configured to trigger EOI exits
 for all IOAPIC interrupts, not just level-triggered IOAPIC
 interrupts. Which is weird, and I believe totally unecessary.]
 
 The RTC interrupt needs to trigger an EOI exit with the in-kernel
 IOAPIC, in order to detect coalesced interrupts.  This is necessary to
 avoid severe time drift in Windows guest.

Correct! EOI exit bitmap is not absolutely same with TMR. Some interrupts 
(especially timer interrupt) which is edge trigger but need a notification when 
it got injected into guest. 

 
 Paolo


Best regards,
Yang


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 31/07/2015 04:49, Steve Rutherford wrote:
 Oh... Yeah. That's a damn good point, given that the interrupt can be
 injected from another thread while one is in that guest vcpu.
 
 Easiest time to update the TMR should be on guest entry through
 vcpu_scan_ioapic, as before.
 
 The best way to go is probably to ditch the new per vcpu EOI exit
 bitmap, and just update/use the TMR. There's no reason to duplicate
 that data in the representation of the apic (I suspect that the
 duplication was inspired by my mistaken notion of the TMR). The
 IOAPIC exit check can use the TMR instead.
 
 Based upon my reading of the SDM, the only reason that the eoi exit
 bitmaps are not the exact same as the TMR is that it is possible to
 have virtual-interrupt delivery enabled /without/ an apic access page
 (Note: V-ID = EOI exit bitmap enabled).
 
 Yang, do you happen to know if that is the case?
 
 [Note: Just looked back at the old code for updating the EOI exit
 bitmaps, which for some reason was configured to trigger EOI exits
 for all IOAPIC interrupts, not just level-triggered IOAPIC
 interrupts. Which is weird, and I believe totally unecessary.]

The RTC interrupt needs to trigger an EOI exit with the in-kernel
IOAPIC, in order to detect coalesced interrupts.  This is necessary to
avoid severe time drift in Windows guest.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

On 31/07/2015 01:26, Zhang, Yang Z wrote:
 Do not compute TMR in advance.  Instead, set the TMR just before
 the interrupt is accepted into the IRR.  This limits the coupling
 between IOAPIC and LAPIC.
 
 Uh.., it back to original way which is wrong. You cannot modify the
 apic page(here is the TMR reg) directly when the corresponding VMCS
 may be used at same time.

Where is this documented?  The TMR is not part of the set of virtualized
APIC registers (TPR, PPR, EOI, ISR, IRR, ICR+ICR2; SDM 29.1.1).

Only virtualized APIC register reads use the virtual TMR registers (SDM
29.4.2 or 29.5), but these just read data from the corresponding field
in the virtual APIC page.

Paolo
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Zhang, Yang Z]

Paolo Bonzini wrote on 2015-07-29:
 Do not compute TMR in advance.  Instead, set the TMR just before the
 interrupt is accepted into the IRR.  This limits the coupling between
 IOAPIC and LAPIC.
 

Uh.., it back to original way which is wrong. You cannot modify the apic 
page(here is the TMR reg) directly when the corresponding VMCS may be used at 
same time.


 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/ioapic.c |  9 ++---
  arch/x86/kvm/ioapic.h |  3 +--
  arch/x86/kvm/lapic.c  | 19 ++-
  arch/x86/kvm/lapic.h  |  1 -
  arch/x86/kvm/x86.c|  5 +
  5 files changed, 14 insertions(+), 23 deletions(-)
 diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
 index 856f79105bb5..eaf4ec38d980 100644
 --- a/arch/x86/kvm/ioapic.c
 +++ b/arch/x86/kvm/ioapic.c
 @@ -246,8 +246,7 @@ static void update_handled_vectors(struct kvm_ioapic
 *ioapic)
   smp_wmb();
  }
 -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
 - u32 *tmr)
 +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
  {
   struct kvm_ioapic *ioapic = vcpu-kvm-arch.vioapic;
   union kvm_ioapic_redirect_entry *e;
 @@ -260,13 +259,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu,
 u64 *eoi_exit_bitmap,
   kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC, 
 index) ||
   index == RTC_GSI) { if 
 (kvm_apic_match_dest(vcpu, NULL, 0,
 - e-fields.dest_id, e-fields.dest_mode)) {
 + e-fields.dest_id, e-fields.dest_mode))
   __set_bit(e-fields.vector,
   (unsigned long *)eoi_exit_bitmap);
 - if (e-fields.trig_mode == IOAPIC_LEVEL_TRIG)
 - __set_bit(e-fields.vector, -   
 (unsigned long *)tmr); -  
   }
   }
   }
   spin_unlock(ioapic-lock);
 diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
 index ca0b0b4e6256..3dbd0e2aac4e 100644
 --- a/arch/x86/kvm/ioapic.h
 +++ b/arch/x86/kvm/ioapic.h
 @@ -120,7 +120,6 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
 kvm_lapic *src,
   struct kvm_lapic_irq *irq, unsigned long *dest_map);
  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
 - u32 *tmr);
 +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
 
  #endif
 diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
 index 2a5ca97c263b..9be64c77d6db 100644
 --- a/arch/x86/kvm/lapic.c
 +++ b/arch/x86/kvm/lapic.c
 @@ -551,15 +551,6 @@ static void pv_eoi_clr_pending(struct kvm_vcpu
 *vcpu)
   __clear_bit(KVM_APIC_PV_EOI_PENDING, vcpu-arch.apic_attention);
  }
 -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
 -{
 - struct kvm_lapic *apic = vcpu-arch.apic;
 - int i;
 -
 - for (i = 0; i  8; i++)
 - apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
 -}
 -
  static void apic_update_ppr(struct kvm_lapic *apic)
  {
   u32 tpr, isrv, ppr, old_ppr;
 @@ -781,6 +772,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int
 delivery_mode,
   case APIC_DM_LOWEST:
   vcpu-arch.apic_arb_prio++;
   case APIC_DM_FIXED:
 + if (unlikely(trig_mode  !level))
 + break;
 +
   /* FIXME add logic for vcpu on reset */
   if (unlikely(!apic_enabled(apic)))
   break;
 @@ -790,6 +784,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
 int delivery_mode,
   if (dest_map)
   __set_bit(vcpu-vcpu_id, dest_map);
 + if (apic_test_vector(vector, apic-regs + APIC_TMR) != 
 !!trig_mode) {
 + if (trig_mode)
 + apic_set_vector(vector, apic-regs + APIC_TMR);
 + else
 + apic_clear_vector(vector, apic-regs + 
 APIC_TMR);
 + }
 +
   if (kvm_x86_ops-deliver_posted_interrupt)
   kvm_x86_ops-deliver_posted_interrupt(vcpu, vector);
   else {
 diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
 index 764037991d26..eb46d6bcaa75 100644
 --- a/arch/x86/kvm/lapic.h
 +++ b/arch/x86/kvm/lapic.h
 @@ -57,7 +57,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64
 value);
  u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu);
  void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
  void __kvm_apic_update_irr(u32 *pir, void *regs);
  void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
  int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct 

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Steve Rutherford]

On Thu, Jul 30, 2015 at 11:26:28PM +, Zhang, Yang Z wrote:
 Paolo Bonzini wrote on 2015-07-29:
  Do not compute TMR in advance.  Instead, set the TMR just before the
  interrupt is accepted into the IRR.  This limits the coupling between
  IOAPIC and LAPIC.
  
 
 Uh.., it back to original way which is wrong. You cannot modify the apic 
 page(here is the TMR reg) directly when the corresponding VMCS may be used at 
 same time.
Oh... Yeah. That's a damn good point, given that the interrupt can be injected 
from another thread while one is in that guest vcpu. 

Easiest time to update the TMR should be on guest entry through 
vcpu_scan_ioapic, as before. 

The best way to go is probably to ditch the new per vcpu EOI exit bitmap, and 
just update/use the TMR. There's no reason to duplicate that data in the 
representation of the apic (I suspect that the duplication was inspired by my 
mistaken notion of the TMR). The IOAPIC exit check can use the TMR instead. 

Based upon my reading of the SDM, the only reason that the eoi exit bitmaps are 
not the exact same as the TMR is that it is possible to have virtual-interrupt 
delivery enabled /without/ an apic access page (Note: V-ID = EOI exit bitmap 
enabled).

Yang, do you happen to know if that is the case?

[Note: Just looked back at the old code for updating the EOI exit bitmaps, 
which for some reason was configured to trigger EOI exits for all IOAPIC 
interrupts, not just level-triggered IOAPIC interrupts. Which is weird, and I 
believe totally unecessary.]


 
 
  Signed-off-by: Paolo Bonzini pbonz...@redhat.com
  ---
   arch/x86/kvm/ioapic.c |  9 ++---
   arch/x86/kvm/ioapic.h |  3 +--
   arch/x86/kvm/lapic.c  | 19 ++-
   arch/x86/kvm/lapic.h  |  1 -
   arch/x86/kvm/x86.c|  5 +
   5 files changed, 14 insertions(+), 23 deletions(-)
  diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
  index 856f79105bb5..eaf4ec38d980 100644
  --- a/arch/x86/kvm/ioapic.c
  +++ b/arch/x86/kvm/ioapic.c
  @@ -246,8 +246,7 @@ static void update_handled_vectors(struct kvm_ioapic
  *ioapic)
  smp_wmb();
   }
  -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
  -   u32 *tmr)
  +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
   {
  struct kvm_ioapic *ioapic = vcpu-kvm-arch.vioapic;
  union kvm_ioapic_redirect_entry *e;
  @@ -260,13 +259,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu,
  u64 *eoi_exit_bitmap,
  kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC, 
  index) ||
  index == RTC_GSI) { if 
  (kvm_apic_match_dest(vcpu, NULL, 0,
  -   e-fields.dest_id, e-fields.dest_mode)) {
  +   e-fields.dest_id, e-fields.dest_mode))
  __set_bit(e-fields.vector,
  (unsigned long *)eoi_exit_bitmap);
  -   if (e-fields.trig_mode == IOAPIC_LEVEL_TRIG)
  -   __set_bit(e-fields.vector, -   
  (unsigned long *)tmr); -
  }
  }
  }
  spin_unlock(ioapic-lock);
  diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
  index ca0b0b4e6256..3dbd0e2aac4e 100644
  --- a/arch/x86/kvm/ioapic.h
  +++ b/arch/x86/kvm/ioapic.h
  @@ -120,7 +120,6 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct
  kvm_lapic *src,
  struct kvm_lapic_irq *irq, unsigned long *dest_map);
   int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
   int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
  -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
  -   u32 *tmr);
  +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
  
   #endif
  diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
  index 2a5ca97c263b..9be64c77d6db 100644
  --- a/arch/x86/kvm/lapic.c
  +++ b/arch/x86/kvm/lapic.c
  @@ -551,15 +551,6 @@ static void pv_eoi_clr_pending(struct kvm_vcpu
  *vcpu)
  __clear_bit(KVM_APIC_PV_EOI_PENDING, vcpu-arch.apic_attention);
   }
  -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
  -{
  -   struct kvm_lapic *apic = vcpu-arch.apic;
  -   int i;
  -
  -   for (i = 0; i  8; i++)
  -   apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
  -}
  -
   static void apic_update_ppr(struct kvm_lapic *apic)
   {
  u32 tpr, isrv, ppr, old_ppr;
  @@ -781,6 +772,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int
  delivery_mode,
  case APIC_DM_LOWEST:
  vcpu-arch.apic_arb_prio++;
  case APIC_DM_FIXED:
  +   if (unlikely(trig_mode  !level))
  +   break;
  +
  /* FIXME add logic for vcpu on reset */
  if (unlikely(!apic_enabled(apic)))
  break;
  @@ -790,6 +784,13 @@ static int 

Re: [PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Steve Rutherford]

On Wed, Jul 29, 2015 at 03:37:34PM +0200, Paolo Bonzini wrote:
 Do not compute TMR in advance.  Instead, set the TMR just before the interrupt
 is accepted into the IRR.  This limits the coupling between IOAPIC and LAPIC.
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  arch/x86/kvm/ioapic.c |  9 ++---
  arch/x86/kvm/ioapic.h |  3 +--
  arch/x86/kvm/lapic.c  | 19 ++-
  arch/x86/kvm/lapic.h  |  1 -
  arch/x86/kvm/x86.c|  5 +
  5 files changed, 14 insertions(+), 23 deletions(-)
 
 diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
 index 856f79105bb5..eaf4ec38d980 100644
 --- a/arch/x86/kvm/ioapic.c
 +++ b/arch/x86/kvm/ioapic.c
 @@ -246,8 +246,7 @@ static void update_handled_vectors(struct kvm_ioapic 
 *ioapic)
   smp_wmb();
  }
  
 -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
 - u32 *tmr)
 +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
  {
   struct kvm_ioapic *ioapic = vcpu-kvm-arch.vioapic;
   union kvm_ioapic_redirect_entry *e;
 @@ -260,13 +259,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 
 *eoi_exit_bitmap,
   kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC, 
 index) ||
   index == RTC_GSI) {
   if (kvm_apic_match_dest(vcpu, NULL, 0,
 - e-fields.dest_id, e-fields.dest_mode)) {
 + e-fields.dest_id, e-fields.dest_mode))
   __set_bit(e-fields.vector,
   (unsigned long *)eoi_exit_bitmap);
 - if (e-fields.trig_mode == IOAPIC_LEVEL_TRIG)
 - __set_bit(e-fields.vector,
 - (unsigned long *)tmr);
 - }
   }
   }
   spin_unlock(ioapic-lock);
 diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
 index ca0b0b4e6256..3dbd0e2aac4e 100644
 --- a/arch/x86/kvm/ioapic.h
 +++ b/arch/x86/kvm/ioapic.h
 @@ -120,7 +120,6 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct 
 kvm_lapic *src,
   struct kvm_lapic_irq *irq, unsigned long *dest_map);
  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 -void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
 - u32 *tmr);
 +void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
  
  #endif
 diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
 index 2a5ca97c263b..9be64c77d6db 100644
 --- a/arch/x86/kvm/lapic.c
 +++ b/arch/x86/kvm/lapic.c
 @@ -551,15 +551,6 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
   __clear_bit(KVM_APIC_PV_EOI_PENDING, vcpu-arch.apic_attention);
  }
  
 -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
 -{
 - struct kvm_lapic *apic = vcpu-arch.apic;
 - int i;
 -
 - for (i = 0; i  8; i++)
 - apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
 -}
 -
  static void apic_update_ppr(struct kvm_lapic *apic)
  {
   u32 tpr, isrv, ppr, old_ppr;
 @@ -781,6 +772,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
 delivery_mode,
   case APIC_DM_LOWEST:
   vcpu-arch.apic_arb_prio++;
   case APIC_DM_FIXED:
 + if (unlikely(trig_mode  !level))
 + break;
 +
   /* FIXME add logic for vcpu on reset */
   if (unlikely(!apic_enabled(apic)))
   break;
 @@ -790,6 +784,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
 delivery_mode,
   if (dest_map)
   __set_bit(vcpu-vcpu_id, dest_map);
  
 + if (apic_test_vector(vector, apic-regs + APIC_TMR) != 
 !!trig_mode) {
 + if (trig_mode)
 + apic_set_vector(vector, apic-regs + APIC_TMR);
 + else
 + apic_clear_vector(vector, apic-regs + 
 APIC_TMR);
 + }
 +
   if (kvm_x86_ops-deliver_posted_interrupt)
   kvm_x86_ops-deliver_posted_interrupt(vcpu, vector);
   else {
 diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
 index 764037991d26..eb46d6bcaa75 100644
 --- a/arch/x86/kvm/lapic.h
 +++ b/arch/x86/kvm/lapic.h
 @@ -57,7 +57,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value);
  u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu);
  void kvm_apic_set_version(struct kvm_vcpu *vcpu);
  
 -void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
  void __kvm_apic_update_irr(u32 *pir, void *regs);
  void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
  int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
 index 23e47a0b054b..48dc954542db 100644
 --- 

[PATCH 1/2] KVM: x86: set TMR when the interrupt is accepted

[Paolo Bonzini]

Do not compute TMR in advance.  Instead, set the TMR just before the interrupt
is accepted into the IRR.  This limits the coupling between IOAPIC and LAPIC.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 arch/x86/kvm/ioapic.c |  9 ++---
 arch/x86/kvm/ioapic.h |  3 +--
 arch/x86/kvm/lapic.c  | 19 ++-
 arch/x86/kvm/lapic.h  |  1 -
 arch/x86/kvm/x86.c|  5 +
 5 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 856f79105bb5..eaf4ec38d980 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -246,8 +246,7 @@ static void update_handled_vectors(struct kvm_ioapic 
*ioapic)
smp_wmb();
 }
 
-void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
-   u32 *tmr)
+void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
 {
struct kvm_ioapic *ioapic = vcpu-kvm-arch.vioapic;
union kvm_ioapic_redirect_entry *e;
@@ -260,13 +259,9 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 
*eoi_exit_bitmap,
kvm_irq_has_notifier(ioapic-kvm, KVM_IRQCHIP_IOAPIC, 
index) ||
index == RTC_GSI) {
if (kvm_apic_match_dest(vcpu, NULL, 0,
-   e-fields.dest_id, e-fields.dest_mode)) {
+   e-fields.dest_id, e-fields.dest_mode))
__set_bit(e-fields.vector,
(unsigned long *)eoi_exit_bitmap);
-   if (e-fields.trig_mode == IOAPIC_LEVEL_TRIG)
-   __set_bit(e-fields.vector,
-   (unsigned long *)tmr);
-   }
}
}
spin_unlock(ioapic-lock);
diff --git a/arch/x86/kvm/ioapic.h b/arch/x86/kvm/ioapic.h
index ca0b0b4e6256..3dbd0e2aac4e 100644
--- a/arch/x86/kvm/ioapic.h
+++ b/arch/x86/kvm/ioapic.h
@@ -120,7 +120,6 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct 
kvm_lapic *src,
struct kvm_lapic_irq *irq, unsigned long *dest_map);
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
-void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap,
-   u32 *tmr);
+void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
 
 #endif
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 2a5ca97c263b..9be64c77d6db 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -551,15 +551,6 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
__clear_bit(KVM_APIC_PV_EOI_PENDING, vcpu-arch.apic_attention);
 }
 
-void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr)
-{
-   struct kvm_lapic *apic = vcpu-arch.apic;
-   int i;
-
-   for (i = 0; i  8; i++)
-   apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);
-}
-
 static void apic_update_ppr(struct kvm_lapic *apic)
 {
u32 tpr, isrv, ppr, old_ppr;
@@ -781,6 +772,9 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
case APIC_DM_LOWEST:
vcpu-arch.apic_arb_prio++;
case APIC_DM_FIXED:
+   if (unlikely(trig_mode  !level))
+   break;
+
/* FIXME add logic for vcpu on reset */
if (unlikely(!apic_enabled(apic)))
break;
@@ -790,6 +784,13 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int 
delivery_mode,
if (dest_map)
__set_bit(vcpu-vcpu_id, dest_map);
 
+   if (apic_test_vector(vector, apic-regs + APIC_TMR) != 
!!trig_mode) {
+   if (trig_mode)
+   apic_set_vector(vector, apic-regs + APIC_TMR);
+   else
+   apic_clear_vector(vector, apic-regs + 
APIC_TMR);
+   }
+
if (kvm_x86_ops-deliver_posted_interrupt)
kvm_x86_ops-deliver_posted_interrupt(vcpu, vector);
else {
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 764037991d26..eb46d6bcaa75 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -57,7 +57,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value);
 u64 kvm_lapic_get_base(struct kvm_vcpu *vcpu);
 void kvm_apic_set_version(struct kvm_vcpu *vcpu);
 
-void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
 void __kvm_apic_update_irr(u32 *pir, void *regs);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
 int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 23e47a0b054b..48dc954542db 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6149,17 +6149,14 @@ static void process_smi(struct kvm_vcpu