[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] kvm: First step to push iothread lock out of in
Re: [Qemu-devel] [PATCH] kvm: First step to push iothread lock out of inner run loop
Sun, 08 Jul 2012 10:49:10 +0300
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0
On 07/06/2012 09:06 PM, Jan Kiszka wrote:
> On 2012-07-06 19:16, Jan Kiszka wrote:
>> On 2012-06-24 16:08, Jan Kiszka wrote:
>>> On 2012-06-24 10:49, Avi Kivity wrote:
>>>> On 06/23/2012 02:45 PM, Jan Kiszka wrote:
>>>>> Hmm, we may need the iothread lock around cpu_set_apic_tpr for
>>>>> !kvm_irqchip_in_kernel(). And as we are at it, apic_base manipulation
>>>>> can be but there as well.
>>>>> With in-kernel irqchip, there is no such need. Also, no one accesses
>>>>> eflags outside of the vcpu thread, independent of the irqchip mode.
>>>> In fact !kvm_irqchip_in_kernel() is broken wrt the tpr. Interrupt
>>>> injection needs to be done atomically, but currently we check the tpr
>>>> from the injecting thread, which means the cpu thread can race with it.
>>>> We need to move the check to the vcpu thread so that the guest vcpu is
>>> So apic_set_irq basically needs to be deferred to vcpu context, right?
>>> Will have a look.
>> Tried to wrap my head around this, but only found different issues
>> (patches under construction).
>> First of all, a simple run_on_cpu doesn't work as it may drops the BQL
>> at unexpected points inside device models.
>> Then I thought about what could actually race here: The testing of the
>> userspace TPR value under BQL vs. some modification by the CPU while in
>> KVM mode. So we may either inject while the CPU is trying to prevent
>> this - harmless as it happens on real hw as well - or not inject while
> Hmm, this could actually be a problem as the race window is extended
> beyond the instruction that raises TPR. So we may generate unexpected
> spurious interrupts for the guest. And that's because the userspace APIC
> fails to update the CPU interrupt pin properly when the TPR is modified.
> Here is the bug...
To avoid dropping the lock we can do something similar to kvm - queue a
request to reevaluate IRR, then signal that vcpu to exit. No need to
drop the lock. Possibly a check in apic_update_irq() whether we're
running in the vcpu thread; if not, signal and return. That function's
effects are asynchronous if run outside the vcpu, so it's a safe change.
error compiling committee.c: too many arguments to function