* [PATCH] KVM: VMX: simplify NMI mask management
@ 2011-03-23 13:12 Avi Kivity
2011-03-23 14:54 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2011-03-23 13:12 UTC (permalink / raw)
To: Marcelo Tosatti, kvm, Jan Kiszka
Use vmx_set_nmi_mask() instead of open-coding management of
the hardware bit and the software hint (nmi_known_unmasked).
There's a slight change of behaviour when running without
hardware virtual NMI support - we now clear the NMI mask if
NMI delivery faulted in that case as well. This improves
emulation accuracy.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/kvm/vmx.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1bdb49d..2b99ae7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3532,11 +3532,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
switch (type) {
case INTR_TYPE_NMI_INTR:
vcpu->arch.nmi_injected = false;
- if (cpu_has_virtual_nmis()) {
- vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
- GUEST_INTR_STATE_NMI);
- vmx->nmi_known_unmasked = false;
- }
+ vmx_set_nmi_mask(vcpu, true);
break;
case INTR_TYPE_EXT_INTR:
case INTR_TYPE_SOFT_INTR:
@@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
* Clear bit "block by NMI" before VM entry if a NMI
* delivery faulted.
*/
- vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
- GUEST_INTR_STATE_NMI);
- vmx->nmi_known_unmasked = true;
+ vmx_set_nmi_mask(&vmx->vcpu, false);
break;
case INTR_TYPE_SOFT_EXCEPTION:
vmx->vcpu.arch.event_exit_inst_len =
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: VMX: simplify NMI mask management
2011-03-23 13:12 [PATCH] KVM: VMX: simplify NMI mask management Avi Kivity
@ 2011-03-23 14:54 ` Jan Kiszka
2011-03-23 14:58 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-03-23 14:54 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 2011-03-23 14:12, Avi Kivity wrote:
> Use vmx_set_nmi_mask() instead of open-coding management of
> the hardware bit and the software hint (nmi_known_unmasked).
>
> There's a slight change of behaviour when running without
> hardware virtual NMI support - we now clear the NMI mask if
> NMI delivery faulted in that case as well. This improves
> emulation accuracy.
>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> arch/x86/kvm/vmx.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 1bdb49d..2b99ae7 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3532,11 +3532,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu)
> switch (type) {
> case INTR_TYPE_NMI_INTR:
> vcpu->arch.nmi_injected = false;
> - if (cpu_has_virtual_nmis()) {
> - vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
> - GUEST_INTR_STATE_NMI);
> - vmx->nmi_known_unmasked = false;
> - }
> + vmx_set_nmi_mask(vcpu, true);
> break;
> case INTR_TYPE_EXT_INTR:
> case INTR_TYPE_SOFT_INTR:
> @@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
> * Clear bit "block by NMI" before VM entry if a NMI
> * delivery faulted.
> */
> - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
> - GUEST_INTR_STATE_NMI);
> - vmx->nmi_known_unmasked = true;
> + vmx_set_nmi_mask(&vmx->vcpu, false);
> break;
> case INTR_TYPE_SOFT_EXCEPTION:
> vmx->vcpu.arch.event_exit_inst_len =
Looks consistent to me, but maybe you should update the comment above
that's now also applied to the emulated mask.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: VMX: simplify NMI mask management
2011-03-23 14:54 ` Jan Kiszka
@ 2011-03-23 14:58 ` Avi Kivity
2011-03-23 15:10 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2011-03-23 14:58 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 03/23/2011 04:54 PM, Jan Kiszka wrote:
> > @@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
> > * Clear bit "block by NMI" before VM entry if a NMI
> > * delivery faulted.
> > */
> > - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
> > - GUEST_INTR_STATE_NMI);
> > - vmx->nmi_known_unmasked = true;
> > + vmx_set_nmi_mask(&vmx->vcpu, false);
> > break;
> > case INTR_TYPE_SOFT_EXCEPTION:
> > vmx->vcpu.arch.event_exit_inst_len =
>
> Looks consistent to me, but maybe you should update the comment above
> that's now also applied to the emulated mask.
In what way?
/*
* SDM 3: 27.7.1.2 (September 2008)
* Clear bit "block by NMI" before VM entry if a NMI
* delivery faulted.
*/
Seems pretty descriptive.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: VMX: simplify NMI mask management
2011-03-23 14:58 ` Avi Kivity
@ 2011-03-23 15:10 ` Jan Kiszka
2011-03-23 16:28 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-03-23 15:10 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 2011-03-23 15:58, Avi Kivity wrote:
> On 03/23/2011 04:54 PM, Jan Kiszka wrote:
>>> @@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
>>> * Clear bit "block by NMI" before VM entry if a NMI
>>> * delivery faulted.
>>> */
>>> - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
>>> - GUEST_INTR_STATE_NMI);
>>> - vmx->nmi_known_unmasked = true;
>>> + vmx_set_nmi_mask(&vmx->vcpu, false);
>>> break;
>>> case INTR_TYPE_SOFT_EXCEPTION:
>>> vmx->vcpu.arch.event_exit_inst_len =
>>
>> Looks consistent to me, but maybe you should update the comment above
>> that's now also applied to the emulated mask.
>
> In what way?
>
> /*
> * SDM 3: 27.7.1.2 (September 2008)
> * Clear bit "block by NMI" before VM entry if a NMI
> * delivery faulted.
> */
>
> Seems pretty descriptive.
IIRC, the above only applies to CPUs that support VNMI and is
meaningless for older ones. But vmx_set_nmi_mask is not void for those
old CPUs.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: VMX: simplify NMI mask management
2011-03-23 15:10 ` Jan Kiszka
@ 2011-03-23 16:28 ` Avi Kivity
2011-03-23 17:14 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2011-03-23 16:28 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 03/23/2011 05:10 PM, Jan Kiszka wrote:
> On 2011-03-23 15:58, Avi Kivity wrote:
> > On 03/23/2011 04:54 PM, Jan Kiszka wrote:
> >>> @@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
> >>> * Clear bit "block by NMI" before VM entry if a NMI
> >>> * delivery faulted.
> >>> */
> >>> - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
> >>> - GUEST_INTR_STATE_NMI);
> >>> - vmx->nmi_known_unmasked = true;
> >>> + vmx_set_nmi_mask(&vmx->vcpu, false);
> >>> break;
> >>> case INTR_TYPE_SOFT_EXCEPTION:
> >>> vmx->vcpu.arch.event_exit_inst_len =
> >>
> >> Looks consistent to me, but maybe you should update the comment above
> >> that's now also applied to the emulated mask.
> >
> > In what way?
> >
> > /*
> > * SDM 3: 27.7.1.2 (September 2008)
> > * Clear bit "block by NMI" before VM entry if a NMI
> > * delivery faulted.
> > */
> >
> > Seems pretty descriptive.
>
> IIRC, the above only applies to CPUs that support VNMI and is
> meaningless for older ones. But vmx_set_nmi_mask is not void for those
> old CPUs.
That's what I pointed out in the second paragraph of the commit log -
that it's desired for non-vnmi capable cpus:
static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
if (!cpu_has_virtual_nmis()) {
/*
* Tracking the NMI-blocked state in software is built upon
* finding the next open IRQ window. This, in turn, depends on
* well-behaving guests: They have to keep IRQs disabled at
* least as long as the NMI handler runs. Otherwise we may
* cause NMI nesting, maybe breaking the guest. But as this is
* highly unlikely, we can live with the residual risk.
*/
vmx->soft_vnmi_blocked = 1;
vmx->vnmi_blocked_time = 0;
}
We block nmi here, and if we later take a fault, nmi remains both
undelivered and blocked.
The SDM refers to the fact the the cpu blocks NMI first and checks for
exceptions during delivery later. That happens to match the soft-vnmi
implementation, so the comment remains valid.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: VMX: simplify NMI mask management
2011-03-23 16:28 ` Avi Kivity
@ 2011-03-23 17:14 ` Jan Kiszka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-03-23 17:14 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm@vger.kernel.org
On 2011-03-23 17:28, Avi Kivity wrote:
> On 03/23/2011 05:10 PM, Jan Kiszka wrote:
>> On 2011-03-23 15:58, Avi Kivity wrote:
>>> On 03/23/2011 04:54 PM, Jan Kiszka wrote:
>>>>> @@ -3991,9 +3987,7 @@ static void __vmx_complete_interrupts(struct vcpu_vmx *vmx,
>>>>> * Clear bit "block by NMI" before VM entry if a NMI
>>>>> * delivery faulted.
>>>>> */
>>>>> - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO,
>>>>> - GUEST_INTR_STATE_NMI);
>>>>> - vmx->nmi_known_unmasked = true;
>>>>> + vmx_set_nmi_mask(&vmx->vcpu, false);
>>>>> break;
>>>>> case INTR_TYPE_SOFT_EXCEPTION:
>>>>> vmx->vcpu.arch.event_exit_inst_len =
>>>>
>>>> Looks consistent to me, but maybe you should update the comment above
>>>> that's now also applied to the emulated mask.
>>>
>>> In what way?
>>>
>>> /*
>>> * SDM 3: 27.7.1.2 (September 2008)
>>> * Clear bit "block by NMI" before VM entry if a NMI
>>> * delivery faulted.
>>> */
>>>
>>> Seems pretty descriptive.
>>
>> IIRC, the above only applies to CPUs that support VNMI and is
>> meaningless for older ones. But vmx_set_nmi_mask is not void for those
>> old CPUs.
>
> That's what I pointed out in the second paragraph of the commit log -
> that it's desired for non-vnmi capable cpus:
>
> static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> if (!cpu_has_virtual_nmis()) {
> /*
> * Tracking the NMI-blocked state in software is built upon
> * finding the next open IRQ window. This, in turn, depends on
> * well-behaving guests: They have to keep IRQs disabled at
> * least as long as the NMI handler runs. Otherwise we may
> * cause NMI nesting, maybe breaking the guest. But as this is
> * highly unlikely, we can live with the residual risk.
> */
> vmx->soft_vnmi_blocked = 1;
> vmx->vnmi_blocked_time = 0;
> }
>
> We block nmi here, and if we later take a fault, nmi remains both
> undelivered and blocked.
>
> The SDM refers to the fact the the cpu blocks NMI first and checks for
> exceptions during delivery later. That happens to match the soft-vnmi
> implementation, so the comment remains valid.
>
Makes sense now.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-23 17:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-23 13:12 [PATCH] KVM: VMX: simplify NMI mask management Avi Kivity
2011-03-23 14:54 ` Jan Kiszka
2011-03-23 14:58 ` Avi Kivity
2011-03-23 15:10 ` Jan Kiszka
2011-03-23 16:28 ` Avi Kivity
2011-03-23 17:14 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.