Re: [PATCH 2/4] KVM: pci device assignment

Yang, Sheng
2008-07-28T07:24:59Z

On Tuesday 22 July 2008 20:13:53 Ben-Ami Yassour wrote:
>
> -int kvm_pic_read_irq(struct kvm_pic *s)
> +int kvm_pic_read_irq(struct kvm *kvm)
> {
> int irq, irq2, intno;
> + struct kvm_pic *s = pic_irqchip(kvm);
>
> irq = pic_get_irq(&s->pics[0]);
> if (irq >= 0) {
> @@ -186,6 +187,8 @@ int kvm_pic_read_irq(struct kvm_pic *s)
> irq = 7;
> intno = s->pics[0].irq_base + irq;
> }
> + kvm_notify_acked_irq(kvm, irq);
> +
> pic_update_irq(s);
>
> return intno;

One coding style suggestion,

struct kvm_pic has *irq_request_opaque which is struct kvm indeed. How
about contain kvm in kvm_pic explicitly? (seems cleaner, though needs
more modification).

Another thing is about host share IRQ: Do we want follow the straight
forward way to add it? That's it, return IRQ_HANDLED from irq handler
and wait for EOI of guest?

I found it's not easy to provide support for MSI without shared IRQ
support. We don't know when guest want to enable MSI or it decided to
shared IRQ, maybe we can intercept the unmask of IOAPIC to know that,
but it's a little tricky.

--
regards
Yang, Sheng