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

["Yang, Sheng" <sheng.yang@intel.com>]

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