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

RE: [PATCH] RE: [Xen-ia64-devel] xencons interrupt problem


  • To: "Alex Williamson" <alex.williamson@xxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Mon, 3 Jul 2006 13:34:15 +0800
  • Cc: xen-ia64-devel <xen-ia64-devel@xxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Sun, 02 Jul 2006 22:34:39 -0700
  • List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
  • Thread-index: AcacdhlwSl4QWPxRTPiB/SpEXID2HgB5oXfw
  • Thread-topic: [PATCH] RE: [Xen-ia64-devel] xencons interrupt problem

>From: Alex Williamson [mailto:alex.williamson@xxxxxx]
>Sent: 2006年7月1日 2:50
>
>   I have something working, please tell me what you think.  Modifying
>assign_irq_vector() to pass in a GSI seemed rather intrusive to the
>guest, so I took a different approach.  Instead I recognize the guest
>trying to overwrite a Xen owned vector, store the guest vector in an
>array and set it up as a "Xen redirect" vector.  These vectors are
>serviced from __do_IRQ after the Xen handler.  There is only one
>redirect vector allowed per Xen vector, but that seems sufficient at
>this point.  This allows full use of both the PCI UART (owned by Xen)
>and the PCI NIC (owned by the guest) sharing a physical RTE.  Let me
>know if you think this approach is acceptable.  Thanks,
>
>       Alex
>
>Signed-off-by: Alex Williamson <alex.williamson@xxxxxx>

Hi, Alex,
        The idea is basically good, but I do see several issues with your 
patch as following:

1.
out:
+       if (guest_vector[irq].enabled) {
+               irq_desc_t *guest_desc = irq_desc + guest_vector[irq].vector;
+
+               if (likely(guest_desc->status & IRQ_GUEST)) {
+                       spin_lock(&guest_desc->lock);
+                       __do_IRQ_guest(guest_vector[irq].vector);
+                       spin_unlock(&guest_desc->lock);
+               }
+       }

Ack() and End() are always been issued before and after above code 
piece, which is not desired for devices owned by guest since 
__do_IRQ_guest only sets pending event bit without wait and guest 
hasn't got chance to handle the irq. EOI is only issued when guest 
hypercalls to notify.

2. pirq_guest_eoi needs to convert guest vector to xen vector and then 
to issue real end method under xen vector. Maybe a reverse array is 
required here for performance

3. Same conversion is also required in pirq_guest_unbind, to flush 
pending irqs.

4. Add your new hw interrupt controller type in pirq_acktype which 
decides whether guest EOI notification is required

5. For change in iosapic_guest_write, we shouldn't warning when 
guest_vector[xen_vector].vector exists since it's possible for guest 
to mask/unmask existing RTE entry. We can only warn when existing 
vector doesn't match new rte value.

6. I'd like to see a comment to emphasize so-called guest vector 
actually occupies a vector in xen vector space, which means such 
xen-guest shared irq line always consumes two xen vectors. Or else 
it's ambiguous a bit.

Thanks
Kevins

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.