Re: [Android-virt] [PATCH v5 05/13] ARM: KVM: Inject IRQs and FIQs from userspace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 11, 2011 at 3:25 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
> On 11 December 2011 20:07, Christoffer Dall
> <christofferdall@xxxxxxxxxxxxxxxxxx> wrote:
>> Well, if it was just "irq & 1", then I hear you, but it would be "(irq
>> >> cpu_idx) & 1" which I don't think is more clear.
>
> Er, what? The fields are [31..1] CPU index and [0] irqtype,
> right? So what you have now is:
>     vcpu_idx = irq_level->irq / 2;
>     irqtype = irq_level->irq % 2;
> and the bitshifting equivalent is:
>     vcpu_idx = irq_level->irq >> 1;
>     irqtype = irq_level->irq & 1;
> surely?
>
> Shifting by the cpuindex is definitely wrong.

actually, that's not how the irq_level field is defined. If you look
in Documentation/virtual/kvm/api.txt:

"ARM uses two types of interrupt lines per CPU, ie. IRQ and FIQ. The
value of the
irq field should be (VCPU_INDEX * 2) for IRQs and ((VCPU_INDEX * 2) + 1) for
FIQs. Level is used to raise/lower the line. See arch/arm/include/asm/kvm.h for
convenience macros."

also, in the kernel code the cpu_index is achieved by a simple integer
division by 2.

as I said, this was the proposal from the last round of reviews after
a lengthy discussion, so I sticked with that.

we should definitely fix either side, and the only sane argument is
that this is an irq_line field, so an index resembling an actual line
seems more semantically in line with the field purpose rather than a
bit encoding, but I am open to arguments and not married to the
current implementation.

> (Incidentally I fixed a bug in your QEMU-side code which wasn't
> feeding this field to the kernel in the way it expects:
>
> http://git.linaro.org/gitweb?p=qemu/qemu-linaro.git;a=commitdiff;h=2502ba067e795e48d346f9816fad45177ca64bca
>
> Sorry, I should have posted that to the list. I'll do that now.)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux