Re: [PATCH v4 0/6] Nonatomic interrupt injection

2010-09-19 Thread Avi Kivity

 On 09/17/2010 09:12 PM, Marcelo Tosatti wrote:

  This is now merged, with the change pointed out by Marcelo.  Windows
  XP x64 fails installation without

  (vmx.c handle_cr())
   case 8: {
   u8 cr8_prev = kvm_get_cr8(vcpu);
   u8 cr8 = kvm_register_read(vcpu, reg);
   kvm_set_cr8(vcpu, cr8);
   skip_emulated_instruction(vcpu);
   if (irqchip_in_kernel(vcpu-kvm))
   return 1;
  -if (cr8_prev= cr8)
  -return 1;
   vcpu-run-exit_reason = KVM_EXIT_SET_TPR;
   return 0;
   }

  Which doesn't make any sense (anyone?).  The failure is present even
  without the patchset, and is fixed by the same hack, so a regression
  was not introduced.

If userspace does not have an uptodate TPR value, it can signal an
interrupt that is now blocked? Say:

- cr8 write 0
- cr8 write 5
- no exit to userspace
- userspace signals interrupt with priority
4 because it knows about tpr == 0.



To signal an interrupt, userspace needs to force an exit.  The exit will 
sync cr8.


However, it may be that the decision to inject the interrupt is taken 
before the exit, so the interrupt is injected even though it shouldn't be.


Let's assume that this is so (I'll check).  Is this a bug in the kernel 
or userspace?


My feeling is that this is a kernel bug, and the optimization should be 
removed.


--
error compiling committee.c: too many arguments to function

--
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 v4 0/6] Nonatomic interrupt injection

2010-09-19 Thread Avi Kivity

 On 09/19/2010 11:25 AM, Avi Kivity wrote:


Let's assume that this is so (I'll check).


It's trivially so.  If a completion causes an interrupt to be raised, 
the vcpu's apic code is executed in the iothread context.



--
error compiling committee.c: too many arguments to function

--
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 v4 0/6] Nonatomic interrupt injection

2010-09-19 Thread Avi Kivity

 On 09/19/2010 11:28 AM, Avi Kivity wrote:

 On 09/19/2010 11:25 AM, Avi Kivity wrote:


Let's assume that this is so (I'll check).


It's trivially so.  If a completion causes an interrupt to be raised, 
the vcpu's apic code is executed in the iothread context.





However, that's a bug even with the optimization removed:

iothreadvcpu
ioapic interrrupt
apic: set isr
mov $large_value, cr8
on_vcpu(inject)
inject interrupt even though cr8 inhibits it

Setting the isr and injecting the interrupt should be an atomic 
operation (easily done be running the apic code on the vcpu thread).


--
error compiling committee.c: too many arguments to function

--
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 v4 0/6] Nonatomic interrupt injection

2010-09-17 Thread Marcelo Tosatti
On Thu, Sep 16, 2010 at 03:35:19PM +0200, Avi Kivity wrote:
  On 08/30/2010 02:36 PM, Avi Kivity wrote:
 This patchset changes interrupt injection to be done from normal process
 context instead of interrupts disabled context.  This is useful for real
 mode interrupt injection on Intel without the current hacks (injecting as
 a software interrupt of a vm86 task), reducing latencies, and later, for
 allowing nested virtualization code to use kvm_read_guest()/kvm_write_guest()
 instead of kmap() to access the guest vmcb/vmcs.
 
 Seems to survive a hack that cancels every 16th entry, after injection has
 already taken place.
 
 With the PIC reset fix posted earlier, this passes autotest on both AMD and
 Intel, with in-kernel irqchip.  I'll run -no-kvm-irqchip tests shortly.
 
 Please review carefully, esp. the first patch.  Any missing 
 kvm_make_request()
 there may result in a hung guest.
 
 
 This is now merged, with the change pointed out by Marcelo.  Windows
 XP x64 fails installation without
 
 (vmx.c handle_cr())
  case 8: {
  u8 cr8_prev = kvm_get_cr8(vcpu);
  u8 cr8 = kvm_register_read(vcpu, reg);
  kvm_set_cr8(vcpu, cr8);
  skip_emulated_instruction(vcpu);
  if (irqchip_in_kernel(vcpu-kvm))
  return 1;
 -if (cr8_prev = cr8)
 -return 1;
  vcpu-run-exit_reason = KVM_EXIT_SET_TPR;
  return 0;
  }
 
 Which doesn't make any sense (anyone?).  The failure is present even
 without the patchset, and is fixed by the same hack, so a regression
 was not introduced.

If userspace does not have an uptodate TPR value, it can signal an
interrupt that is now blocked? Say:

- cr8 write 0
- cr8 write 5
- no exit to userspace
- userspace signals interrupt with priority 
4 because it knows about tpr == 0.

--
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 v4 0/6] Nonatomic interrupt injection

2010-09-16 Thread Avi Kivity

 On 08/30/2010 02:36 PM, Avi Kivity wrote:

This patchset changes interrupt injection to be done from normal process
context instead of interrupts disabled context.  This is useful for real
mode interrupt injection on Intel without the current hacks (injecting as
a software interrupt of a vm86 task), reducing latencies, and later, for
allowing nested virtualization code to use kvm_read_guest()/kvm_write_guest()
instead of kmap() to access the guest vmcb/vmcs.

Seems to survive a hack that cancels every 16th entry, after injection has
already taken place.

With the PIC reset fix posted earlier, this passes autotest on both AMD and
Intel, with in-kernel irqchip.  I'll run -no-kvm-irqchip tests shortly.

Please review carefully, esp. the first patch.  Any missing kvm_make_request()
there may result in a hung guest.



This is now merged, with the change pointed out by Marcelo.  Windows XP 
x64 fails installation without


(vmx.c handle_cr())
 case 8: {
 u8 cr8_prev = kvm_get_cr8(vcpu);
 u8 cr8 = kvm_register_read(vcpu, reg);
 kvm_set_cr8(vcpu, cr8);
 skip_emulated_instruction(vcpu);
 if (irqchip_in_kernel(vcpu-kvm))
 return 1;
-if (cr8_prev = cr8)
-return 1;
 vcpu-run-exit_reason = KVM_EXIT_SET_TPR;
 return 0;
 }

Which doesn't make any sense (anyone?).  The failure is present even 
without the patchset, and is fixed by the same hack, so a regression was 
not introduced.


--
error compiling committee.c: too many arguments to function

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


[PATCH v4 0/6] Nonatomic interrupt injection

2010-08-30 Thread Avi Kivity
This patchset changes interrupt injection to be done from normal process
context instead of interrupts disabled context.  This is useful for real
mode interrupt injection on Intel without the current hacks (injecting as
a software interrupt of a vm86 task), reducing latencies, and later, for
allowing nested virtualization code to use kvm_read_guest()/kvm_write_guest()
instead of kmap() to access the guest vmcb/vmcs.

Seems to survive a hack that cancels every 16th entry, after injection has
already taken place.

With the PIC reset fix posted earlier, this passes autotest on both AMD and
Intel, with in-kernel irqchip.  I'll run -no-kvm-irqchip tests shortly.

Please review carefully, esp. the first patch.  Any missing kvm_make_request()
there may result in a hung guest.

v4: add KVM_REQ_EVENT after lapic restore

v3: close new race between injection and entry
fix Intel real-mode injection cancellation

v2: svm support (easier than expected)
fix silly vmx warning

Avi Kivity (6):
  KVM: Check for pending events before attempting injection
  KVM: VMX: Split up vmx_complete_interrupts()
  KVM: VMX: Move real-mode interrupt injection fixup to
vmx_complete_interrupts()
  KVM: VMX: Parameterize vmx_complete_interrupts() for both exit and
entry
  KVM: Non-atomic interrupt injection
  KVM: VMX: Move fixup_rmode_irq() to avoid forward declaration

 arch/x86/include/asm/kvm_host.h |1 +
 arch/x86/kvm/i8259.c|1 +
 arch/x86/kvm/lapic.c|   13 -
 arch/x86/kvm/svm.c  |   20 ++-
 arch/x86/kvm/vmx.c  |  116 ++
 arch/x86/kvm/x86.c  |   44 ++
 include/linux/kvm_host.h|1 +
 7 files changed, 143 insertions(+), 53 deletions(-)

--
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 v4 0/6] Nonatomic interrupt injection

2010-08-30 Thread Avi Kivity

 On 08/30/2010 02:36 PM, Avi Kivity wrote:

I'll run -no-kvm-irqchip tests shortly.


That needed a KVM_REQ_EVENT in the KVM_INTERRUPT path, but otherwise 
it's running smoothly.


--
error compiling committee.c: too many arguments to function

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