>>> static void kvm_vcpu_irqsink_init(struct kvm_vcpu *vcpu)
>>>diff -- git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
>>>index 4c03881..91546ae 100644
>>>--- a/drivers/kvm/svm.c
>>>+++ b/drivers/kvm/svm.c
>>>@@ - 1542,11 +1542,40 @@ 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;
>>>
>>> again:
>>>+ /*
>>>+ * We disable interrupts until the next VMEXIT to eliminate a
>> race
>>>+ * 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
>>>+ * scope) even if they are disabled.
>>>+ *
>>>+ * FIXME: Do we need to do anything additional to mask IPI/NMIs?
>>>+ */
>>>+ local_irq_save(irq_flags);
>>>+
>>> spin_lock(&vcpu- >irq.lock);
>>>
>>> /*
>>>+ * If there are any signals pending (virtual interrupt related
>> or
>>>+ * otherwise), don't even bother trying to enter guest mode...
>>>+ */
>>>+ if (signal_pending(current)) {
>>>+ kvm_run- >exit_reason = KVM_EXIT_INTR;
>>>+ spin_unlock(&vcpu- >irq.lock);
>>>+ local_irq_restore(irq_flags);
>>>+ return - EINTR;
Instead of returning you should 'goto out;' since the post_kvm_run_save
is called there + minimize return paths.
>>>+ }
>>
>>
>> A possible optimization would be to check if we have an irq to
inject.
>> If we have, then ignore the signal and enter guest mode.
>> Since an irq is already pending, the signal would not be resulting in
>> another irq injection.
>> What do you think?
>
>I am a little fuzzy on whether this is necessary myself. The
motivation
>for this code block originally was really more for the cases where
signals
>are sent that *aren't* related to interrupts. For instance, what if
QEMU
>was trying to force the vCPU to exit for some reason (e.g. an AIO
>completion event that has to be decoded by userspace before a IRQ is
>generated)? If the signal is queued beforehand AND linux doesn't
already
>reschedule things for us, I think there is a race against the VCPU
being
>stuck in guest mode until the next (unrelated) VMEXIT since the IPI
would
>have already happened. This code (I believe) closes the window on that
>scenario. I am just not sure if its a realistic one. Perhaps someone
with
>more linux signal handling knowledge than myself can chime in and
confirm?
>(And I am not saying that person isn't you, Dor. I am only saying it
isn't
>me ;)
You're right that there is a slight chance for a qemu signal such as AIO
completion to reach right after we enter the kernel. My intention was to
unit the signal pending checks - there is another one after the guest
exits vmx mode. Then once we're back on host mode we can optimize the
path by checking whether we need to re-inject in-kernel-apic pending irq
or a case where a physical irq prevented us from injecting the virq.
The optimization is not specific to apic and can take place even now.
>
>>
>>>+
>>>+ /*
>>>+ * 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 = 1;
>>>+
>>>+ /*
>>> * We must inject interrupts (if any) while the irq_lock
>>> * is held
>>> */
>>>@@ - 1688,6 +1717,13 @@ again:
>>> #endif
>>> : "cc", "memory" );
>>>
>>>+ /*
>>>+ * 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
>>>+ * transition from guest to host?
>>>+ */
>>
>> The guest_mode can be assigned here, thus eliminating the cli- sti
below.
>
>Yeah....I was concerned about putting too much logic before the
guest/host
>state transfer because I didn't know enough about it to know if I would
>stomp on some register that still hadn't been saved. If you say its
safe
>to do that here, then I agree that is the optimal way to do it.
There is nothing to worry about since the ipi just caused the cpu to
leave guest mode, the vcpu locks are still held and after getting the
ipi (that you wrote and does nothing) the host will continue the same
execution path.
Note that today the code has irq enabled all the exit path too.
-------------------------------------------------------------------------
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/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
