From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Gregory Haskins" Subject: Re: irqdevice INTR example Date: Sun, 15 Apr 2007 19:32:07 -0400 Message-ID: <46227DA4.BA47.005A.0@novell.com> References: <461D7702.BA47.005A.0@novell.com> <461DE791.1040707@qumranet.com> <461DE5C9.BA47.005A.0@novell.com> <461E2AD5.7070905@qumranet.com> <461DFF1C.BA47.005A.0@novell.com> <461E3EDB.3080002@qumranet.com> <461E1F73.BA47.005A.0@novell.com> <4620E56A.7040207@qumranet.com> <46226FBC.BA47.005A.0@novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__Part5E7937E7.0__=" To: Return-path: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org --=__Part5E7937E7.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline >>> On Sun, Apr 15, 2007 at 6:32 PM, in message <46226FBC.BA47.005A.0-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org>, Gregory Haskins wrote: > However, you do highlight a bug in this code that could cause the interrupt > to be deferred indefinitely until a second interrupt came in if the processor > happens to halt after this VMEXIT. I.e., since a signal isn't being delivered while > in irq.guest_mode = 1, we do need to handle this at this layer by looping back > into the CPU. I think I misspoke. I inspected the code after writing this and realized that I think it was fine the way it was. The point of confusion I think was the irq.pending flag, which really wasn't doing anything useful in this context. But other than the superfluous variable, I think the design would work as it was written. Basically, the calls to pending/read_vector would indicate the appropriate vector to inject...so all we really need is a "kick" via the IPI to knock the task out of GUEST mode (if applicable). Once that happens, the normal processing will pick up on the posted vector, even if a HLT were in progress. I removed the irq.pending flag for clarity. If I truly need it downstream in my patch series I will add it there. Attached is an updated patch incorporating the latest feedback. Regards, -Greg --=__Part5E7937E7.0__= Content-Type: text/plain; name="preemptible-cpu-5.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="preemptible-cpu-5.patch" KVM: Preemptible VCPU From: <> This adds support for interrupting an executing CPU Signed-off-by: Gregory Haskins --- drivers/kvm/kvm.h | 13 +++++++++ drivers/kvm/kvm_main.c | 68 ++++++++++++++++++++++++++++++++++++++++++++= ---- drivers/kvm/svm.c | 51 ++++++++++++++++++++++++++++++++++++ drivers/kvm/vmx.c | 55 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 179 insertions(+), 8 deletions(-) diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h index 58966d9..87357cc 100644 --- a/drivers/kvm/kvm.h +++ b/drivers/kvm/kvm.h @@ -271,6 +271,18 @@ void kvm_io_bus_register_dev(struct kvm_io_bus *bus, =20 #define NR_IRQ_WORDS KVM_IRQ_BITMAP_SIZE(unsigned long) =20 +#define KVM_SIGNAL_VIRTUAL_INTERRUPT 33 /* Hardcoded for now */ + +/* + * structure for maintaining info for interrupting an executing VCPU + */ +struct kvm_vcpu_irq { + spinlock_t lock; + struct task_struct *task; + int signo; + int guest_mode; +}; + struct kvm_vcpu { struct kvm *kvm; union { @@ -284,6 +296,7 @@ struct kvm_vcpu { struct kvm_run *run; int interrupt_window_open; struct kvm_irqdevice irq_dev; + struct kvm_vcpu_irq irq; unsigned long regs[NR_VCPU_REGS]; /* for rsp: vcpu_load_rsp_rip() = */ unsigned long rip; /* needs vcpu_load_rsp_rip() */ =20 diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c index 7e00412..6d7178b 100644 --- a/drivers/kvm/kvm_main.c +++ b/drivers/kvm/kvm_main.c @@ -299,6 +299,14 @@ static struct kvm *kvm_create_vm(void) struct kvm_vcpu *vcpu =3D &kvm->vcpus[i]; =20 mutex_init(&vcpu->mutex); + + memset(&vcpu->irq, 0, sizeof(vcpu->irq)); + spin_lock_init(&vcpu->irq.lock); + /* + * This should be settable by userspace someday + */ + vcpu->irq.signo =3D KVM_SIGNAL_VIRTUAL_INTERRUPT; + vcpu->cpu =3D -1; vcpu->kvm =3D kvm; vcpu->mmu.root_hpa =3D INVALID_PAGE; @@ -1838,6 +1846,7 @@ static int kvm_vcpu_ioctl_run(struct kvm_vcpu *vcpu, = struct kvm_run *kvm_run) { int r; sigset_t sigsaved; + unsigned long irqsaved; =20 vcpu_load(vcpu); =20 @@ -1866,6 +1875,10 @@ static int kvm_vcpu_ioctl_run(struct kvm_vcpu = *vcpu, struct kvm_run *kvm_run) kvm_arch_ops->decache_regs(vcpu); } =20 + spin_lock_irqsave(&vcpu->irq.lock, irqsaved); + vcpu->irq.task =3D current; + spin_unlock_irqrestore(&vcpu->irq.lock, irqsaved); +=09 r =3D kvm_arch_ops->run(vcpu, kvm_run); =20 out: @@ -2310,6 +2323,20 @@ out1: } =20 /* + * This function is invoked whenever we want to interrupt a vcpu that is + * currently executing in guest-mode. It currently is a no-op because + * the simple delivery of the IPI to execute this function accomplishes = our + * goal: To cause a VMEXIT. We pass the vcpu (which contains the=20 + * vcpu->irq.task, etc) for future use + */ +static void kvm_vcpu_guest_intr(void *info) +{ +#ifdef NOT_YET + struct kvm_vcpu *vcpu =3D (struct kvm_vcpu*)info; +#endif +} + +/* * This function will be invoked whenever the vcpu->irq_dev raises its = INTR=20 * line */ @@ -2320,13 +2347,42 @@ static void kvm_vcpu_intr(struct kvm_irqsink = *this, * Our irq device is requesting to interrupt the vcpu. If it is * currently running, we should inject a host IPI to force a = VMEXIT=20 */ + struct kvm_vcpu *vcpu =3D (struct kvm_vcpu*)this->private; + unsigned long flags; + + spin_lock_irqsave(&vcpu->irq.lock, flags); =09 - /* - * FIXME: Implement this or the CPU wont notice the interrupt = until - * the next natural VMEXIT. Note that this is how the system - * has always worked, so nothing is broken here. This is a future - * enhancement - */ + if (vcpu->irq.task && (vcpu->irq.task !=3D current)) { + if (vcpu->irq.guest_mode) { + /* + * If we are in guest mode, we can optimize the = IPI + * by executing a function on the owning processor.= + */ + int cpu; + + /* + * Not sure if disabling preemption is needed + * since we are in a spin-lock anyway? The + * kick_process() code does this so I copied it + */ + preempt_disable(); + cpu =3D task_cpu(vcpu->irq.task); + BUG_ON(cpu =3D=3D smp_processor_id()); + smp_call_function_single(cpu,=20 + kvm_vcpu_guest_intr, + vcpu, 0, 0);=20 + preempt_enable(); + } else + /* + * If we are not in guest mode, we must assume = that + * we could be blocked anywhere, including = userspace. + * Send a signal to give everyone a chance to get + * notification + */ + send_sig(vcpu->irq.signo, vcpu->irq.task, 0); + } +=09 + spin_unlock_irqrestore(&vcpu->irq.lock, flags); } =20 static void kvm_vcpu_irqsink_init(struct kvm_vcpu *vcpu) diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c index e59a548..f5cdffe 100644 --- a/drivers/kvm/svm.c +++ b/drivers/kvm/svm.c @@ -1461,11 +1461,48 @@ static int svm_vcpu_run(struct kvm_vcpu *vcpu, = struct kvm_run *kvm_run) u16 gs_selector; u16 ldt_selector; int r; + unsigned long irq_flags; =20 again: + /* + * We disable interrupts until the next VMEXIT to eliminate a = race=20 + * condition for delivery of virtual interrutps. Note that this = is + * probably not as bad as it sounds, as interrupts will still = invoke + * a VMEXIT once transitioned to GUEST mode (and thus exit this = lock=20 + * scope) even if they are disabled. + * + * FIXME: Do we need to do anything additional to mask IPI/NMIs?=20= + */ + local_irq_save(irq_flags); + + spin_lock(&vcpu->irq.lock); + + /* + * If there are any signals pending (virtual interrupt related = or=20 + * otherwise), don't even bother trying to enter guest mode... + */ + if (signal_pending(current)) { + kvm_run->exit_reason =3D KVM_EXIT_INTR; + spin_unlock(&vcpu->irq.lock); + local_irq_restore(irq_flags); + return -EINTR; + } + + /* + * There are optimizations we can make when signaling interrupts + * if we know the VCPU is in GUEST mode, so mark that here + */ + vcpu->irq.guest_mode =3D 1; + + /* + * We also must inject interrupts (if any) while the irq_lock + * is held + */ if (!vcpu->mmio_read_completed) do_interrupt_requests(vcpu, kvm_run); =20 + spin_unlock(&vcpu->irq.lock); + clgi(); =20 pre_svm_run(vcpu); @@ -1597,6 +1634,13 @@ again: #endif : "cc", "memory" ); =20 + /* + * FIXME: We'd like to turn on interrupts ASAP, but is this so = early + * that we will mess up the state of the CPU before we fully=20 + * transition from guest to host? + */ + local_irq_restore(irq_flags); + fx_save(vcpu->guest_fx_image); fx_restore(vcpu->host_fx_image); =20 @@ -1617,6 +1661,13 @@ again: reload_tss(vcpu); =20 /* + * Signal that we have transitioned back to host mode=20 + */ + spin_lock_irqsave(&vcpu->irq.lock, irq_flags); + vcpu->irq.guest_mode =3D 0; + spin_unlock_irqrestore(&vcpu->irq.lock, irq_flags); + + /* * Profile KVM exit RIPs: */ if (unlikely(prof_on =3D=3D KVM_PROFILING)) diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c index a0fdf02..654c4d6 100644 --- a/drivers/kvm/vmx.c +++ b/drivers/kvm/vmx.c @@ -1722,6 +1722,7 @@ static int vmx_vcpu_run(struct kvm_vcpu *vcpu, = struct kvm_run *kvm_run) u16 fs_sel, gs_sel, ldt_sel; int fs_gs_ldt_reload_needed; int r; + unsigned long irq_flags; =20 again: /* @@ -1748,11 +1749,47 @@ again: vmcs_writel(HOST_GS_BASE, segment_base(gs_sel)); #endif =20 + if (vcpu->guest_debug.enabled) + kvm_guest_debug_pre(vcpu); + + /* + * We disable interrupts until the next VMEXIT to eliminate a = race=20 + * condition for delivery of virtual interrutps. Note that this = is + * probably not as bad as it sounds, as interrupts will still = invoke + * a VMEXIT once transitioned to GUEST mode (and thus exit this = lock=20 + * scope) even if they are disabled. + * + * FIXME: Do we need to do anything additional to mask IPI/NMIs?=20= + */ + local_irq_save(irq_flags); + + spin_lock(&vcpu->irq.lock); +=09 + /* + * If there are any signals pending (virtual interrupt related = or=20 + * otherwise), don't even bother trying to enter guest mode... + */ + if (signal_pending(current)) { + kvm_run->exit_reason =3D KVM_EXIT_INTR; + spin_unlock(&vcpu->irq.lock); + local_irq_restore(irq_flags); + return -EINTR; + } + + /* + * There are optimizations we can make when signaling interrupts + * if we know the VCPU is in GUEST mode, so mark that here + */ + vcpu->irq.guest_mode =3D 1; +=09 + /* + * We also must inject interrupts (if any) while the irq.lock + * is held + */ if (!vcpu->mmio_read_completed) do_interrupt_requests(vcpu, kvm_run); =20 - if (vcpu->guest_debug.enabled) - kvm_guest_debug_pre(vcpu); + spin_unlock(&vcpu->irq.lock); =20 fx_save(vcpu->host_fx_image); fx_restore(vcpu->guest_fx_image); @@ -1880,6 +1917,13 @@ again: : "cc", "memory" ); =20 /* + * FIXME: We'd like to turn on interrupts ASAP, but is this so = early + * that we will mess up the state of the CPU before we fully=20 + * transition from guest to host? + */ + local_irq_restore(irq_flags); + + /* * Reload segment selectors ASAP. (it's needed for a functional * kernel: x86 relies on having __KERNEL_PDA in %fs and x86_64 * relies on having 0 in %gs for the CPU PDA to work.) @@ -1911,6 +1955,13 @@ again: =20 asm ("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS)); =20 + /* + * Signal that we have transitioned back to host mode=20 + */ + spin_lock_irqsave(&vcpu->irq.lock, irq_flags); + vcpu->irq.guest_mode =3D 0; + spin_unlock_irqrestore(&vcpu->irq.lock, irq_flags); + if (fail) { kvm_run->exit_reason =3D KVM_EXIT_FAIL_ENTRY; kvm_run->fail_entry.hardware_entry_failure_reason --=__Part5E7937E7.0__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ --=__Part5E7937E7.0__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kvm-devel mailing list kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/kvm-devel --=__Part5E7937E7.0__=--