Hi Gleb,
On Wed, Jan 21, 2009 at 02:34:28PM +0200, Gleb Natapov wrote:
> Use this one instead. Adds capabilities checks.
>
> Signed-off-by: Gleb Natapov <gleb@xxxxxxxxxx>
> index 179dcb0..2752016 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -76,12 +76,13 @@ void kvm_pic_clear_isr_ack(struct kvm *kvm)
> /*
> * set irq level. If an edge is detected, then the IRR is set to 1
> */
> -static inline void pic_set_irq1(struct kvm_kpic_state *s, int irq, int level)
> +static inline int pic_set_irq1(struct kvm_kpic_state *s, int irq, int level)
> {
> - int mask;
> + int mask, ret = 1;
> mask = 1 << irq;
> if (s->elcr & mask) /* level triggered */
> if (level) {
> + ret = !(s->irr & mask);
> s->irr |= mask;
> s->last_irr |= mask;
> } else {
> @@ -90,11 +91,15 @@ static inline void pic_set_irq1(struct kvm_kpic_state *s, int irq, int level)
> }
> else /* edge triggered */
> if (level) {
> - if ((s->last_irr & mask) == 0)
> + if ((s->last_irr & mask) == 0) {
> + ret = !(s->irr & mask);
> s->irr |= mask;
> + }
> s->last_irr |= mask;
> } else
> s->last_irr &= ~mask;
> +
> + return (s->imr & mask) ? -1 : ret;
> }
Can you add some documentation, perhaps through definitions, like:
#define KVM_IRQ_LINE_STATUS_MASKED -1
#define KVM_IRQ_LINE_STATUS_FAIL 0
#define KVM_IRQ_LINE_STATUS_INJECTED 1
But with better names. This makes the kernel code more explicit too.
> -void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level)
> +int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level)
> {
> u32 old_irr = ioapic->irr;
> u32 mask = 1 << irq;
> union ioapic_redir_entry entry;
> + int ret = 1;
-1 here ?
And then, in the userspace part, you consider -1 as "injected":
diff --git a/qemu/hw/i8259.c b/qemu/hw/i8259.c
index 6d41a5e..9da4360 100644
--- a/qemu/hw/i8259.c
+++ b/qemu/hw/i8259.c
@@ -186,9 +186,14 @@ static void i8259_set_irq(void *opaque, int irq,
int level)
{
PicState2 *s = opaque;
#ifdef KVM_CAP_IRQCHIP
- if (kvm_enabled())
- if (kvm_set_irq(irq, level))
- return;
+ if (kvm_enabled()) {
+ int pic_ret;
+ if (kvm_set_irq(irq, level, &pic_ret)) {
+ if (pic_ret != 0)
+ apic_set_irq_delivered();
+ return;
+ }
+ }
#endif
Is that what you intended ?
Other than that looks fine to me.
--
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
