On 21/11/12 21:46, Christoffer Dall wrote: > On Wed, Nov 21, 2012 at 1:15 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >> While the vgic code doesn't really care about an edge interrupt >> having its line "lowered", doing so makes more sense from a timer >> perspective, and clearly indicates that this is an edge-triggered >> interrupt. >> > > I am extremely confused. > > The architected timers on an A15 is always level interrupt, correct? > We present an A15 to the guest so the guest kernel expects that this > is the case and that's what the emulated cfg register will contain. So > I don't understand your reference to the edge thingy above? The cfg register exposes an edge interrupt to the guest. And for all purpose, the fact that it is treated as an edge interrupt is irrelevant to the guest, as it makes no observable difference. On the host, it makes a hell of a difference (vgic maintenance interrupts). > If I understand the patch correctly, this relates to the fact that the > timer interrupts are active-LOW, and then I get all scared, because I > think I always assumed that when we set level==1 in the kvm_irq_level > struct we mean "raise an interrupt", regardless of wether the line is > active-LOW or active-HIGH. Otherwise we will never inject an interrupt > that is active-LOW when it's active, but always inject that interrupt > when it's inactive. It doesn't matter! We don't really inject a level, we inject an interrupt. An event. The fact that we use the same internal API is pure convenience. >> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx> >> --- >> arch/arm/kvm/timer.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/arch/arm/kvm/timer.c b/arch/arm/kvm/timer.c >> index a241298..b9e573f 100644 >> --- a/arch/arm/kvm/timer.c >> +++ b/arch/arm/kvm/timer.c >> @@ -42,6 +42,9 @@ static void kvm_timer_inject_irq(struct kvm_vcpu *vcpu) >> kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, >> vcpu->arch.timer_cpu.irq->irq, >> vcpu->arch.timer_cpu.irq->level); >> + kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, >> + vcpu->arch.timer_cpu.irq->irq, >> + !vcpu->arch.timer_cpu.irq->level); > > regardless of the above, this looks super screwy and must be explained > with a comment. > > If we start supporting a core where the timer interrupt is edge-based, > than we never raise a timer interrupt, which is just plain weird. You assume we care about the hardware timer. We don't. It is just a convenient and standardized interface to let the guest request an event. The interrupt is handled on the host, where the interrupt polarity matters. What we inject into the guest is a different story, and I don't think it matters. M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
