Xin, Xiaohui
2007-Sep-30 07:29 UTC
[Xen-devel] [VTD][PATCH] a time out mechanism for the shared interrupt issue for vtd
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@intel.com Signed-off-by: Kevin Tian <kevin.tian@intel.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Sep-30 08:41 UTC
Re: [Xen-devel] [VTD][PATCH] a time out mechanism for the shared interrupt 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@intel.com> 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@intel.com > Signed-off-by: Kevin Tian <kevin.tian@intel.com> > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kay, Allen M
2007-Oct-02 00:07 UTC
RE: [Xen-devel] [VTD][PATCH] a time out mechanism for the sharedinterrupt 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@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser Sent: Sunday, September 30, 2007 1:41 AM To: Xin, Xiaohui; xen-devel@lists.xensource.com 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@intel.com> 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@intel.com Signed-off-by: Kevin Tian <kevin.tian@intel.com> ________________________________ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kay, Allen M
2007-Oct-02 23:56 UTC
RE: [Xen-devel] [VTD][PATCH] a time out mechanism for thesharedinterrupt issue for vtd
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@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Kay, Allen M Sent: Monday, October 01, 2007 5:07 PM To: Keir Fraser; Xin, Xiaohui; xen-devel@lists.xensource.com 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@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser Sent: Sunday, September 30, 2007 1:41 AM To: Xin, Xiaohui; xen-devel@lists.xensource.com 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@intel.com> 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@intel.com Signed-off-by: Kevin Tian <kevin.tian@intel.com> ________________________________ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Oct-03 08:27 UTC
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@intel.com> 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@lists.xensource.com >> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Kay, Allen M >> Sent: Monday, October 01, 2007 5:07 PM >> To: Keir Fraser; Xin, Xiaohui; xen-devel@lists.xensource.com >> 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@lists.xensource.com >>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser >>> Sent: Sunday, September 30, 2007 1:41 AM >>> To: Xin, Xiaohui; xen-devel@lists.xensource.com >>> 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@intel.com> 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@intel.com >>>> Signed-off-by: Kevin Tian <kevin.tian@intel.com> >>>> >>>> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Xen-devel mailing list >>>> Xen-devel@lists.xensource.com >>>> http://lists.xensource.com/xen-devel >>> >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Xin, Xiaohui
2007-Oct-09 05:26 UTC
RE: [Xen-devel] [VTD][PATCH] a time out mechanism for the shared interrupt issue for vtd
Keir, How about the status of the patch now or any other comments? Thanks Xiaohui ________________________________ From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk] Sent: 2007年10月3日 16:27 To: Kay, Allen M; Xin, Xiaohui; xen-devel@lists.xensource.com 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@intel.com> 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@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] <mailto:xen-devel-bounces@lists.xensource.com%5d> On Behalf Of Kay, Allen M Sent: Monday, October 01, 2007 5:07 PM To: Keir Fraser; Xin, Xiaohui; xen-devel@lists.xensource.com 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@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] <mailto:xen-devel-bounces@lists.xensource.com%5d> On Behalf Of Keir Fraser Sent: Sunday, September 30, 2007 1:41 AM To: Xin, Xiaohui; xen-devel@lists.xensource.com 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@intel.com> 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@intel.com Signed-off-by: Kevin Tian <kevin.tian@intel.com> ________________________________ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Oct-11 14:19 UTC
Re: [Xen-devel] [VTD][PATCH] a time out mechanism for the shared interrupt issue for vtd
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@intel.com> wrote:
> Keir,
> How about the status of the patch now or any other comments?
>
> Thanks
> Xiaohui
>
>
>
> From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
> Sent: 2007年10月3日 16:27
> To: Kay, Allen M; Xin, Xiaohui; xen-devel@lists.xensource.com
> 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@intel.com>
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@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com]
> <mailto:xen-devel-bounces@lists.xensource.com%5d> On Behalf Of Kay,
Allen M
> Sent: Monday, October 01, 2007 5:07 PM
> To: Keir Fraser; Xin, Xiaohui; xen-devel@lists.xensource.com
> 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@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com]
> <mailto:xen-devel-bounces@lists.xensource.com%5d> On Behalf Of Keir
Fraser
> Sent: Sunday, September 30, 2007 1:41 AM
> To: Xin, Xiaohui; xen-devel@lists.xensource.com
> 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@intel.com>
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@intel.com
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
>
>
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel