Stefano Stabellini
2011-Nov-18 11:13 UTC
[PATCH v2] x86: re-inject emulated level pirqs in PV on HVM guests if still asserted
This patch is a backport of CS 24007 for xen-4.1-testing. PV on HVM guests can loose level interrupts coming from emulated devices if they have been remapped onto event channels. The reason is that we are missing the code to inject a pirq again in the guest when the guest EOIs it, if it corresponds to an emulated level interrupt and the interrupt is still asserted. Fix this issue and also return error when the guest tries to get the irq_status of a non-existing pirq. Changes in v2: - move the spinlock afterward to cover the new code only. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: Jan Beulich <JBeulich@suse.com> diff -r e73ada19a69d xen/arch/x86/physdev.c --- a/xen/arch/x86/physdev.c Thu Nov 17 09:13:25 2011 +0000 +++ b/xen/arch/x86/physdev.c Fri Nov 18 09:42:03 2011 +0000 @@ -268,6 +268,20 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H ret = pirq_guest_eoi(v->domain, eoi.irq); else ret = 0; + spin_lock(&v->domain->event_lock); + if ( is_hvm_domain(v->domain) && + domain_pirq_to_emuirq(v->domain, eoi.irq) > 0 ) + { + struct hvm_irq *hvm_irq = &v->domain->arch.hvm_domain.irq; + int gsi = domain_pirq_to_emuirq(v->domain, eoi.irq); + + /* if this is a level irq and count > 0, send another + * notification */ + if ( gsi >= NR_ISAIRQS /* ISA irqs are edge triggered */ + && hvm_irq->gsi_assert_count[gsi] ) + send_guest_pirq(v->domain, eoi.irq); + } + spin_unlock(&v->domain->event_lock); break; } @@ -323,9 +337,10 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H break; irq_status_query.flags = 0; if ( is_hvm_domain(v->domain) && - domain_pirq_to_irq(v->domain, irq) <= 0 ) + domain_pirq_to_irq(v->domain, irq) <= 0 && + domain_pirq_to_emuirq(v->domain, irq) == IRQ_UNBOUND ) { - ret = copy_to_guest(arg, &irq_status_query, 1) ? -EFAULT : 0; + ret = -EINVAL; break; }
Pasi Kärkkäinen
2011-Nov-18 12:41 UTC
Re: [PATCH v2] x86: re-inject emulated level pirqs in PV on HVM guests if still asserted
On Fri, Nov 18, 2011 at 11:13:13AM +0000, Stefano Stabellini wrote:> This patch is a backport of CS 24007 for xen-4.1-testing. > > PV on HVM guests can loose level interrupts coming from emulated > devices if they have been remapped onto event channels. The reason is > that we are missing the code to inject a pirq again in the guest when > the guest EOIs it, if it corresponds to an emulated level interrupt > and the interrupt is still asserted. >Btw this issue also happens with pure HVM guests using emulated devices. I was seeing this bug with Linux F16 HVM guest with IDE disk and realtek emulated NIC. The emulated realtek nic didn''t work before applying this patch. -- Pasi> Fix this issue and also return error when the guest tries to get the > irq_status of a non-existing pirq. > > > Changes in v2: > > - move the spinlock afterward to cover the new code only. > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > > > diff -r e73ada19a69d xen/arch/x86/physdev.c > --- a/xen/arch/x86/physdev.c Thu Nov 17 09:13:25 2011 +0000 > +++ b/xen/arch/x86/physdev.c Fri Nov 18 09:42:03 2011 +0000 > @@ -268,6 +268,20 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H > ret = pirq_guest_eoi(v->domain, eoi.irq); > else > ret = 0; > + spin_lock(&v->domain->event_lock); > + if ( is_hvm_domain(v->domain) && > + domain_pirq_to_emuirq(v->domain, eoi.irq) > 0 ) > + { > + struct hvm_irq *hvm_irq = &v->domain->arch.hvm_domain.irq; > + int gsi = domain_pirq_to_emuirq(v->domain, eoi.irq); > + > + /* if this is a level irq and count > 0, send another > + * notification */ > + if ( gsi >= NR_ISAIRQS /* ISA irqs are edge triggered */ > + && hvm_irq->gsi_assert_count[gsi] ) > + send_guest_pirq(v->domain, eoi.irq); > + } > + spin_unlock(&v->domain->event_lock); > break; > } > > @@ -323,9 +337,10 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H > break; > irq_status_query.flags = 0; > if ( is_hvm_domain(v->domain) && > - domain_pirq_to_irq(v->domain, irq) <= 0 ) > + domain_pirq_to_irq(v->domain, irq) <= 0 && > + domain_pirq_to_emuirq(v->domain, irq) == IRQ_UNBOUND ) > { > - ret = copy_to_guest(arg, &irq_status_query, 1) ? -EFAULT : 0; > + ret = -EINVAL; > break; > } > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel
Jan Beulich
2011-Nov-21 10:53 UTC
Re: [PATCH v2] x86: re-inject emulated level pirqs in PV on HVM guests if still asserted
>>> On 18.11.11 at 12:13, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> This patch is a backport of CS 24007 for xen-4.1-testing. > > PV on HVM guests can loose level interrupts coming from emulated > devices if they have been remapped onto event channels. The reason is > that we are missing the code to inject a pirq again in the guest when > the guest EOIs it, if it corresponds to an emulated level interrupt > and the interrupt is still asserted. > > Fix this issue and also return error when the guest tries to get the > irq_status of a non-existing pirq. > > > Changes in v2: > > - move the spinlock afterward to cover the new code only.This, as expected, doesn''t hang anymore with kernels making use of the PIRQ EOI map. Jan> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > > > diff -r e73ada19a69d xen/arch/x86/physdev.c > --- a/xen/arch/x86/physdev.c Thu Nov 17 09:13:25 2011 +0000 > +++ b/xen/arch/x86/physdev.c Fri Nov 18 09:42:03 2011 +0000 > @@ -268,6 +268,20 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H > ret = pirq_guest_eoi(v->domain, eoi.irq); > else > ret = 0; > + spin_lock(&v->domain->event_lock); > + if ( is_hvm_domain(v->domain) && > + domain_pirq_to_emuirq(v->domain, eoi.irq) > 0 ) > + { > + struct hvm_irq *hvm_irq = &v->domain->arch.hvm_domain.irq; > + int gsi = domain_pirq_to_emuirq(v->domain, eoi.irq); > + > + /* if this is a level irq and count > 0, send another > + * notification */ > + if ( gsi >= NR_ISAIRQS /* ISA irqs are edge triggered */ > + && hvm_irq->gsi_assert_count[gsi] ) > + send_guest_pirq(v->domain, eoi.irq); > + } > + spin_unlock(&v->domain->event_lock); > break; > } > > @@ -323,9 +337,10 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H > break; > irq_status_query.flags = 0; > if ( is_hvm_domain(v->domain) && > - domain_pirq_to_irq(v->domain, irq) <= 0 ) > + domain_pirq_to_irq(v->domain, irq) <= 0 && > + domain_pirq_to_emuirq(v->domain, irq) == IRQ_UNBOUND ) > { > - ret = copy_to_guest(arg, &irq_status_query, 1) ? -EFAULT : 0; > + ret = -EINVAL; > break; > } >
Stefano Stabellini
2011-Nov-21 11:06 UTC
Re: [PATCH v2] x86: re-inject emulated level pirqs in PV on HVM guests if still asserted
On Mon, 21 Nov 2011, Jan Beulich wrote:> >>> On 18.11.11 at 12:13, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > This patch is a backport of CS 24007 for xen-4.1-testing. > > > > PV on HVM guests can loose level interrupts coming from emulated > > devices if they have been remapped onto event channels. The reason is > > that we are missing the code to inject a pirq again in the guest when > > the guest EOIs it, if it corresponds to an emulated level interrupt > > and the interrupt is still asserted. > > > > Fix this issue and also return error when the guest tries to get the > > irq_status of a non-existing pirq. > > > > > > Changes in v2: > > > > - move the spinlock afterward to cover the new code only. > > This, as expected, doesn''t hang anymore with kernels making use of > the PIRQ EOI map.Thanks for testing! Keir, are you happy with the patch?
Keir Fraser
2011-Nov-21 11:54 UTC
Re: [PATCH v2] x86: re-inject emulated level pirqs in PV on HVM guests if still asserted
On 21/11/2011 11:06, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com> wrote:> On Mon, 21 Nov 2011, Jan Beulich wrote: >>>>> On 18.11.11 at 12:13, Stefano Stabellini >>>>> <stefano.stabellini@eu.citrix.com> >> wrote: >>> This patch is a backport of CS 24007 for xen-4.1-testing. >>> >>> PV on HVM guests can loose level interrupts coming from emulated >>> devices if they have been remapped onto event channels. The reason is >>> that we are missing the code to inject a pirq again in the guest when >>> the guest EOIs it, if it corresponds to an emulated level interrupt >>> and the interrupt is still asserted. >>> >>> Fix this issue and also return error when the guest tries to get the >>> irq_status of a non-existing pirq. >>> >>> >>> Changes in v2: >>> >>> - move the spinlock afterward to cover the new code only. >> >> This, as expected, doesn''t hang anymore with kernels making use of >> the PIRQ EOI map. > > Thanks for testing! > Keir, are you happy with the patch?Yes, I applied it already. -- Keir