Re: [Xen-devel] [VTD][PATCH] a time out mechanism for the shared interrupt issue for vtd

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

Yes, the patch needs a refresh in the following respects:

 1. Fix it so it applies to xen-unstable tip
 2. Do not change the locking around usage of hvm_dpci_eoi(). There is no need (I’m pretty sure) to release the hvm irq_lock before acquiring the desc->lock.
 3. The TIME_OUT_PERIOD{1,2} definitions are nonsense — set_timer() takes an argument in nanoseconds, not in jiffies. 81920*HZ looks like you want a 81920s timeout, which is probably not what you mean (nor what you actually get). Please fix these.
 4. The change to pci_intx_assert() to return a boolean — I don’t understand precisly what this boolean means. It looks like a hack. As does the new ‘has_timer’ boolean argument to hvm_dpci_eoi(). They need explanation, or removing, or something.

 -- Keir

On 9/10/07 06:26, "Xin, Xiaohui" <xiaohui.xin@xxxxxxxxx> wrote:

Keir,
How about the status of the patch now or any other comments?
 
Thanks
Xiaohui
 

---

From: Keir Fraser [mailto:Keir.Fraser@xxxxxxxxxxxx]
Sent: 2007年10月3日 16:27
To: Kay, Allen M; Xin, Xiaohui; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [VTD][PATCH] a time out mechanism for thesharedinterrupt issue for vtd

I’m thinking about it. I’ll probably put it in in some form later this week.

 -- Keir

On 3/10/07 00:56, "Kay, Allen M" <allen.m.kay@xxxxxxxxx> wrote:
Keir,

Do you have any other issues with this patch?  This patch should fix a lot of shared interrupt related failures our QA team has encountered.

Allen

 

---

From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx  [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] <mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx%5d> On Behalf Of Kay, Allen  M
Sent: Monday, October 01, 2007 5:07 PM
To: Keir Fraser;  Xin, Xiaohui; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: RE: [Xen-devel]  [VTD][PATCH] a time out mechanism for thesharedinterrupt issue for  vtd

 
 
Xiaohui and Kevin will be out for about a week for  national holiday.  I have looked into the issues you  raised:

 
 
1) Looks like irq_lock changes in  vioapic_update_EOI() and hvm_dpci_eoi() are not needed.  You can go ahead  and remove them.

 
 
2) The change for hvm_pci_intx_assert() seems to be  needed by vmx/vmx_dirq_assist().  It is passing the return value of  viopic_irq_positive_edge() to convey info such as whether the interrupt is  masked or not.  In vmx_dpirq_assist(), the return value is used to  determine whether to deassert the interrupt or wait for the interrupt for some  more time.  If the return value is 0, it mean the interrupt is still  masked by the guest - guest is not ready to accept interrupt yet - so it  deasserts the interrupt.

 
 
My test shows it handles shared interrupt cases  including ioapic_ack=new (by temporarily commenting out ioapic_ack_new = 0)  pretty well thus fixes a major deficiency in PCI passthru  functionality.

 
 
Allen


 

 

---

From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx  [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] <mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx%5d> On Behalf Of Keir  Fraser
Sent: Sunday, September 30, 2007 1:41 AM
To: Xin,  Xiaohui; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel]  [VTD][PATCH] a time out mechanism for the sharedinterrupt issue for  vtd

 
Why does the irq_lock need to be released before  taking the desc->lock in pirq_guest_eoi()? What does the new return  boolean from hvm_pci_intx_assert() mean?

 -- Keir

On  30/9/07 08:29, "Xin, Xiaohui" <xiaohui.xin@xxxxxxxxx>  wrote:

 
Attached is a  patch for shared interrupt between dom0 and HVM domain for vtd.
Most of  problem is caused by that we should inject interrupt to both domains and  the
physical interrupt deassertion then may be delayed by the device  assigned to the HVM.
 
The patch adds a timer, and the time out  value is sufficient large to tolerant
the delaying used to wait for the  physical interrupt deassertion.
 
The patch works well with the  situation that SATA disk shares interrupt with PCIe NIC.
And for vtd=1,  the ioapic_ack=new method also works  well.
 
Signed-off-by: Xin,  Xiaohui<xiaohui.xin@xxxxxxxxx
Signed-off-by: Kevin Tian  <kevin.tian@xxxxxxxxx>
 

 

 

---

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

 
 

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