Gregory Haskins wrote:
>> It's really subtle.
>>
>> With respect to interrupts, VT^H^Hthe hardware provides an override over
>> IF. If an interrupt happens while this override is enabled, we exit
>> guest mode regardless of the state of IF. In effect interrupts are
>> enabled but _delivery_ is disabled. Once we exit guest mode, interrupts
>> become disabled again until we set IF explicitly.
>>
>
> Heres a version that implements that idea. Note that its still a POC, as the
> kick_process still needs to be exported and handled in extern-module-compat
> (or whatever its called)
>
Hopefully Ingo will ack such a patch, as he suggested kick_process()
originally.
> @@ -1866,6 +1874,19 @@ static int kvm_vcpu_ioctl_run(struct kvm_vcpu *vcpu,
> struct kvm_run *kvm_run)
> kvm_arch_ops->decache_regs(vcpu);
> }
>
> + /*
> + * Make sure the current task is accurately recorded. Most of the
> + * time, it will be so we first check if its necessary without taking
> + * the lock. If there is a mismatch, we must aquire the lock and
> + * check again to eliminate races
> + */
> + if (unlikely(vcpu->irq.task != current)) {
> + spin_lock(&vcpu->irq.lock);
> + if (vcpu->irq.task != current)
> + vcpu->irq.task = current;
> + spin_unlock(&vcpu->irq.lock);
> + }
> +
>
Isn't this equivalent to
vcpu->irq.task = current
?
btw, these double-check things are broken, see
http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html.
It will usually work on x86, and can be made to work elsewhere with
memory barriers, but it's less simple than it appears.
> + * FIXME: Do we need to do anything additional to mask IPI/NMIs?
>
I'm fairly certain you don't. I keep hearing about the deadlockability
of sending ipis with interrupts disabled.
> + */
> + local_irq_save(irq_flags);
> +
> + spin_lock(&vcpu->irq.lock);
> +
> + /*
> + * 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 also must inject interrupts (if any) while the irq_lock
> + * is held
> + */
> if (!vcpu->mmio_read_completed)
> do_interrupt_requests(vcpu, kvm_run);
>
> + spin_unlock(&vcpu->irq.lock);
> +
> clgi();
>
stgi() / clgi() is what implements interrupt handling under svm. I
think you got this right, but please tell me you read the relevant
sections in the manual a few times.
> @@ -1617,6 +1650,16 @@ again:
> reload_tss(vcpu);
>
> /*
> + * Signal that we have transitioned back to host mode
> + */
> + spin_lock(&vcpu->irq.lock);
> +
> + vcpu->irq.guest_mode = 0;
> + vcpu->irq.pending = 0;
> +
> + spin_unlock(&vcpu->irq.lock);
> +
>
If an interrupt happened just before here, then we need to check it,
otherwise it's lost. Also an opportunity to see ->read_vector in
action, which was the purpose of the exercise IIRC.
>
> asm ("mov %0, %%ds; mov %0, %%es" : : "r"(__USER_DS));
>
> + /*
> + * Signal that we have transitioned back to host mode
> + */
> + spin_lock(&vcpu->irq.lock);
> +
> + vcpu->irq.guest_mode = 0;
> + vcpu->irq.pending = 0;
> +
> + spin_unlock(&vcpu->irq.lock);
> +
> if (fail) {
> kvm_run->exit_reason = KVM_EXIT_FAIL_ENTRY;
> kvm_run->fail_entry.hardware_entry_failure_reason
>
Ditto.
You also want spin_lock_irq() when interrupts are enabled, since some
callers will lock it in interrupt context.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
-------------------------------------------------------------------------
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
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/kvm-devel