On 12/12/2011 04:38 PM, Christoffer Dall wrote:
> >
> > Why don't they match? The assignment of lines to actual pins differs,
> > but essentially it's the same thing (otherwise we'd use a different ioctl).
> >
>
> because there is no notion of gsi and irq_source_id on ARM.
gsi = number of irq line, just a bad name, but you do have it on ARM.
irq_source_id really shouldn't have been in kvm_set_irq(), it's an
implementation detail rather than an architectural feature; just ignore it.
> What's the
> harm of this additional tracepoint?
If we get tools that use them, they have an additional difference to
consider. It's a weak argument but it's there.
> If I should re-use the existing one, should I simply move it outside
> of __KVM_HAVE_IOAPIC?
Protect it with __KVM_HAVE_IRQ_LINE so we don't leak unused tracepoints
for other archs.
> >> + trace_kvm_irq_line(irq_level->irq % 2, irq_level->level, vcpu_idx);
> >> +
> >> + if (irq_level->level) {
> >> + vcpu->arch.virt_irq |= mask;
> >
> > racy - this is a vm ioctl, and so can (and usually is) invoked from
> > outside the vcpu thread.
> >
>
> this is taken care of in SMP host patch, but will be moved down the
> patches for next revision.
Yes please. It's hard to review this way. Fold all the smp stuff into
the patches which introduce the functionality.
>
> >> + vcpu->arch.wait_for_interrupts = 0;
> >
> > Need an actual wakeup here (see x86's kvm_vcpu_kick() - should really be
> > common code; it takes care of both the 'vcpu sleeping and needs a
> > wakeup' and 'vcpu is in guest mode and needs to go to the host to
> > evaluate interrupt state').
> >
>
> the wakeup - same as above. Good point that we need to signal the
> other CPU. Will come, maybe not next revision, but the one after that.
Ok. I think you can reuse x86 concepts and even code. I'll accept
patches to make code arch independent ahead of the merge if it helps.
--
error compiling committee.c: too many arguments to function
