On Tue, Dec 13, 2011 at 4:37 AM, Avi Kivity <a...@redhat.com> wrote:
> On 12/12/2011 09:36 PM, Christoffer Dall wrote:
>> >>
>> >> something like this:
>> >>
>> >> if (irq_level->level) {
>> >> set_bit(&vcpu->arch.irq_lines, bit_nr);
>> >> smp_mb();
>> >> wake_up_interruptible(&vcpu->wq);
>> >
>> > or, smp_send_reschedule(). See kvm_vcpu_kick().
>> >
>> > An optimization: do a cmpxchg() and don't wakeup if the operation raised
>> > IRQ file FIQ was set (assuming that FIQ has a higher priority than IRQ).
>> >
>>
>> forgive my lack of experience here:
>>
>> how would you use cmpxchg exactly?
>
> cmpxchg is used as a fallback for general test-and-change which doesn't
> fit any other atomic operation. In this case:
>
> do {
> virt_intr = ACCESS_ONCE(vcpu->virt_intr)
> new_virt_intr = virt_intr | mask
>
> if (new_virt_intr == virt_intr)
> return; /* no change */
>
> } while (!cmpxchg(vcpu->virt_intr, virt_intr, new_virt_intr)
>
> /* at this point, we know the old and new values, and we know the
> update was atomic */
>
heh, nice. thanks.
> if (new_virt_intr == IRQ | FIQ && virt_intr == FIQ) {
> /* IRQ raised, FIQ already set */
> return;
> }
>
hmm, so what you want to avoid here is sending an IPI to the other CPU
in case we already have FIQ raised? But I think we have to do this
anyhow. If a guest is servicing a raised FIQ and have FIQs masked, but
the GIC hasn't lowered the FIQ line yet, and now comes an IRQ, if the
IRQ is unmasked we want to change the hypervisor virtual IRQ register
right away as to signal an IRQ immediately and if the guest masks IRQ
we still want to change the hypervisor virtual register so that the
moment the guest unmasks the IRQ, an exception is raised. The only way
to set the hypervisor register for another CPU would be to have it
take a world-switch round.
So, I think if we simply change either of these lines, we need to
signal the other CPU.
Marc, can you confirm?
>>
>> yes, FIQ takes priority over IRQ, but we need to raise the IRQ flag anyway.
>>
>> I can see that we don't have to call the wake_up function unless the
>> old value was 0 (no interrupts pending == raising the first one). Is
>> this what you mean? If so, I can't seem to wrap my head around how to
>> accomplish this elegantly with cmpxchg...?
>>
>> Then, another issue. I am going to need to two fields and then a
>> memcpy, since I don't want to be reading an atomic value from my
>> world-switch code. (I would need this copy even with a spinlock).
>> Agree?
>
> Don't follow. Why a memcpy? why don't you want to read an atomic value
> during world switch?
>
if it was declared as an atomic_t, since that's theoretically opaque,
could change to 64-bit and so on, I thought it would be ugly to read
that in the world-switch assembler code. But using atomic bitops on a
standard long field, we should be fine.
>> But, if I am doing atomic bitops on an unsigned long field, how do I
>> read that (or test two bits at once) atomically?
>
> Reads (and writes) are always atomic. Its read-modify-write that needs
> special treatment.
>
embarrassing. I actually thought read/writes of a word could be
partial to the byte between SMP nodes, but it turns out, and as you
say, they cannot. Got it.
Thanks again!
--
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
