Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On Mon, 2012-07-09 at 17:32 +0200, Jan Kiszka wrote:
> On 2012-07-09 00:55, Michael S. Tsirkin wrote:
> > On Mon, Jul 02, 2012 at 11:08:20AM -0600, Alex Williamson wrote:
> >> On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote:
> >>> On 2012-06-11 13:19, Michael S. Tsirkin wrote:
> We can deliver certain interrupts, notably MSIX,
> from atomic context.
> Here's an untested patch to do this (compiled only).
>
> Changes from v2:
> Don't inject broadcast interrupts directly
> Changes from v1:
> Tried to address comments from v1, except unifying
> with kvm_set_irq: passing flags to it looks too ugly.
> Added a comment.
>
> Jan, you said you can test this?
>
>
> Michael S. Tsirkin (2):
> kvm: implement kvm_set_msi_inatomic
> kvm: deliver msix interrupts from irq handler
>
> include/linux/kvm_host.h | 3 ++
> virt/kvm/assigned-dev.c | 31 ++--
> virt/kvm/irq_comm.c | 75
>
> 3 files changed, 102 insertions(+), 7 deletions(-)
>
> >>>
> >>> Finally-tested-by: Jan Kiszka
> >>
> >> Michael, we need either this or the simple oneshot patch to get device
> >> assignment working again for 3.5. Are you planning to push this for
> >> 3.5? Thanks,
> >>
> >> Alex
> >>
> >
> > Avi wants this reworked using an injection cache. I agree with Jan
> > though: oneshot looks too ugly. Just so you can make progress, can't we
> > add a stub handler returning IRQ_WAKE_THREAD unconditionally for now?
> > I.e. like the below (warning: completely untested).
> >
> > Signed-off-by: Michael S. Tsirkin
> >
> > --
> >
> > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > index b1e091a..18cc36e 100644
> > --- a/virt/kvm/assigned-dev.c
> > +++ b/virt/kvm/assigned-dev.c
> > @@ -334,6 +334,11 @@ static int assigned_device_enable_host_intx(struct kvm
> > *kvm,
> > }
> >
> > #ifdef __KVM_HAVE_MSI
> > +static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> > +{
> > + return IRQ_WAKE_THREAD;
> > +}
> > +
> > static int assigned_device_enable_host_msi(struct kvm *kvm,
> >struct kvm_assigned_dev_kernel *dev)
> > {
> > @@ -346,7 +351,7 @@ static int assigned_device_enable_host_msi(struct kvm
> > *kvm,
> > }
> >
> > dev->host_irq = dev->dev->irq;
> > - if (request_threaded_irq(dev->host_irq, NULL,
> > + if (request_threaded_irq(dev->host_irq, kvm_assigned_dev_msi,
> > kvm_assigned_dev_thread_msi, 0,
> > dev->irq_name, dev)) {
> > pci_disable_msi(dev->dev);
> > @@ -358,6 +363,11 @@ static int assigned_device_enable_host_msi(struct kvm
> > *kvm,
> > #endif
> >
> > #ifdef __KVM_HAVE_MSIX
> > +static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> > +{
> > + return IRQ_WAKE_THREAD;
> > +}
> > +
> > static int assigned_device_enable_host_msix(struct kvm *kvm,
> > struct kvm_assigned_dev_kernel *dev)
> > {
> > @@ -374,7 +384,8 @@ static int assigned_device_enable_host_msix(struct kvm
> > *kvm,
> >
> > for (i = 0; i < dev->entries_nr; i++) {
> > r = request_threaded_irq(dev->host_msix_entries[i].vector,
> > -NULL, kvm_assigned_dev_thread_msix,
> > +kvm_assigned_dev_msix,
> > +kvm_assigned_dev_thread_msix,
> > 0, dev->irq_name, dev);
> > if (r)
> > goto err;
> >
>
> Should restore the status quo before 3.5's IRQ layer changes. Looks ok
> to me.
I'll test and post this, Michael is out this week. Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On 2012-07-09 00:55, Michael S. Tsirkin wrote:
> On Mon, Jul 02, 2012 at 11:08:20AM -0600, Alex Williamson wrote:
>> On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote:
>>> On 2012-06-11 13:19, Michael S. Tsirkin wrote:
We can deliver certain interrupts, notably MSIX,
from atomic context.
Here's an untested patch to do this (compiled only).
Changes from v2:
Don't inject broadcast interrupts directly
Changes from v1:
Tried to address comments from v1, except unifying
with kvm_set_irq: passing flags to it looks too ugly.
Added a comment.
Jan, you said you can test this?
Michael S. Tsirkin (2):
kvm: implement kvm_set_msi_inatomic
kvm: deliver msix interrupts from irq handler
include/linux/kvm_host.h | 3 ++
virt/kvm/assigned-dev.c | 31 ++--
virt/kvm/irq_comm.c | 75
3 files changed, 102 insertions(+), 7 deletions(-)
>>>
>>> Finally-tested-by: Jan Kiszka
>>
>> Michael, we need either this or the simple oneshot patch to get device
>> assignment working again for 3.5. Are you planning to push this for
>> 3.5? Thanks,
>>
>> Alex
>>
>
> Avi wants this reworked using an injection cache. I agree with Jan
> though: oneshot looks too ugly. Just so you can make progress, can't we
> add a stub handler returning IRQ_WAKE_THREAD unconditionally for now?
> I.e. like the below (warning: completely untested).
>
> Signed-off-by: Michael S. Tsirkin
>
> --
>
> diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> index b1e091a..18cc36e 100644
> --- a/virt/kvm/assigned-dev.c
> +++ b/virt/kvm/assigned-dev.c
> @@ -334,6 +334,11 @@ static int assigned_device_enable_host_intx(struct kvm
> *kvm,
> }
>
> #ifdef __KVM_HAVE_MSI
> +static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> +{
> + return IRQ_WAKE_THREAD;
> +}
> +
> static int assigned_device_enable_host_msi(struct kvm *kvm,
> struct kvm_assigned_dev_kernel *dev)
> {
> @@ -346,7 +351,7 @@ static int assigned_device_enable_host_msi(struct kvm
> *kvm,
> }
>
> dev->host_irq = dev->dev->irq;
> - if (request_threaded_irq(dev->host_irq, NULL,
> + if (request_threaded_irq(dev->host_irq, kvm_assigned_dev_msi,
>kvm_assigned_dev_thread_msi, 0,
>dev->irq_name, dev)) {
> pci_disable_msi(dev->dev);
> @@ -358,6 +363,11 @@ static int assigned_device_enable_host_msi(struct kvm
> *kvm,
> #endif
>
> #ifdef __KVM_HAVE_MSIX
> +static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> +{
> + return IRQ_WAKE_THREAD;
> +}
> +
> static int assigned_device_enable_host_msix(struct kvm *kvm,
> struct kvm_assigned_dev_kernel *dev)
> {
> @@ -374,7 +384,8 @@ static int assigned_device_enable_host_msix(struct kvm
> *kvm,
>
> for (i = 0; i < dev->entries_nr; i++) {
> r = request_threaded_irq(dev->host_msix_entries[i].vector,
> - NULL, kvm_assigned_dev_thread_msix,
> + kvm_assigned_dev_msix,
> + kvm_assigned_dev_thread_msix,
>0, dev->irq_name, dev);
> if (r)
> goto err;
>
Should restore the status quo before 3.5's IRQ layer changes. Looks ok
to me.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On Mon, Jul 02, 2012 at 11:08:20AM -0600, Alex Williamson wrote:
> On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote:
> > On 2012-06-11 13:19, Michael S. Tsirkin wrote:
> > > We can deliver certain interrupts, notably MSIX,
> > > from atomic context.
> > > Here's an untested patch to do this (compiled only).
> > >
> > > Changes from v2:
> > > Don't inject broadcast interrupts directly
> > > Changes from v1:
> > > Tried to address comments from v1, except unifying
> > > with kvm_set_irq: passing flags to it looks too ugly.
> > > Added a comment.
> > >
> > > Jan, you said you can test this?
> > >
> > >
> > > Michael S. Tsirkin (2):
> > > kvm: implement kvm_set_msi_inatomic
> > > kvm: deliver msix interrupts from irq handler
> > >
> > > include/linux/kvm_host.h | 3 ++
> > > virt/kvm/assigned-dev.c | 31 ++--
> > > virt/kvm/irq_comm.c | 75
> > >
> > > 3 files changed, 102 insertions(+), 7 deletions(-)
> > >
> >
> > Finally-tested-by: Jan Kiszka
>
> Michael, we need either this or the simple oneshot patch to get device
> assignment working again for 3.5. Are you planning to push this for
> 3.5? Thanks,
>
> Alex
>
Avi wants this reworked using an injection cache. I agree with Jan
though: oneshot looks too ugly. Just so you can make progress, can't we
add a stub handler returning IRQ_WAKE_THREAD unconditionally for now?
I.e. like the below (warning: completely untested).
Signed-off-by: Michael S. Tsirkin
--
diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
index b1e091a..18cc36e 100644
--- a/virt/kvm/assigned-dev.c
+++ b/virt/kvm/assigned-dev.c
@@ -334,6 +334,11 @@ static int assigned_device_enable_host_intx(struct kvm
*kvm,
}
#ifdef __KVM_HAVE_MSI
+static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
+{
+ return IRQ_WAKE_THREAD;
+}
+
static int assigned_device_enable_host_msi(struct kvm *kvm,
struct kvm_assigned_dev_kernel *dev)
{
@@ -346,7 +351,7 @@ static int assigned_device_enable_host_msi(struct kvm *kvm,
}
dev->host_irq = dev->dev->irq;
- if (request_threaded_irq(dev->host_irq, NULL,
+ if (request_threaded_irq(dev->host_irq, kvm_assigned_dev_msi,
kvm_assigned_dev_thread_msi, 0,
dev->irq_name, dev)) {
pci_disable_msi(dev->dev);
@@ -358,6 +363,11 @@ static int assigned_device_enable_host_msi(struct kvm *kvm,
#endif
#ifdef __KVM_HAVE_MSIX
+static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
+{
+ return IRQ_WAKE_THREAD;
+}
+
static int assigned_device_enable_host_msix(struct kvm *kvm,
struct kvm_assigned_dev_kernel *dev)
{
@@ -374,7 +384,8 @@ static int assigned_device_enable_host_msix(struct kvm *kvm,
for (i = 0; i < dev->entries_nr; i++) {
r = request_threaded_irq(dev->host_msix_entries[i].vector,
-NULL, kvm_assigned_dev_thread_msix,
+kvm_assigned_dev_msix,
+kvm_assigned_dev_thread_msix,
0, dev->irq_name, dev);
if (r)
goto err;
--
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv3 RFC 0/2] kvm: direct msix injection
Hi, Michael/Alex, do you have progress for device assignment issue fixing? https://bugzilla.kernel.org/show_bug.cgi?id=43328 Thanks, -Xudong > -Original Message- > From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On > Behalf Of Alex Williamson > Sent: Tuesday, July 03, 2012 1:08 AM > To: Jan Kiszka > Cc: Michael S. Tsirkin; kvm@vger.kernel.org > Subject: Re: [PATCHv3 RFC 0/2] kvm: direct msix injection > > On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote: > > On 2012-06-11 13:19, Michael S. Tsirkin wrote: > > > We can deliver certain interrupts, notably MSIX, > > > from atomic context. > > > Here's an untested patch to do this (compiled only). > > > > > > Changes from v2: > > > Don't inject broadcast interrupts directly > > > Changes from v1: > > > Tried to address comments from v1, except unifying > > > with kvm_set_irq: passing flags to it looks too ugly. > > > Added a comment. > > > > > > Jan, you said you can test this? > > > > > > > > > Michael S. Tsirkin (2): > > > kvm: implement kvm_set_msi_inatomic > > > kvm: deliver msix interrupts from irq handler > > > > > > include/linux/kvm_host.h | 3 ++ > > > virt/kvm/assigned-dev.c | 31 ++-- > > > virt/kvm/irq_comm.c | 75 > > > > 3 files changed, 102 insertions(+), 7 deletions(-) > > > > > > > Finally-tested-by: Jan Kiszka > > Michael, we need either this or the simple oneshot patch to get device > assignment working again for 3.5. Are you planning to push this for > 3.5? Thanks, > > Alex > > > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On Mon, 2012-06-25 at 11:32 +0200, Jan Kiszka wrote: > On 2012-06-11 13:19, Michael S. Tsirkin wrote: > > We can deliver certain interrupts, notably MSIX, > > from atomic context. > > Here's an untested patch to do this (compiled only). > > > > Changes from v2: > > Don't inject broadcast interrupts directly > > Changes from v1: > > Tried to address comments from v1, except unifying > > with kvm_set_irq: passing flags to it looks too ugly. > > Added a comment. > > > > Jan, you said you can test this? > > > > > > Michael S. Tsirkin (2): > > kvm: implement kvm_set_msi_inatomic > > kvm: deliver msix interrupts from irq handler > > > > include/linux/kvm_host.h | 3 ++ > > virt/kvm/assigned-dev.c | 31 ++-- > > virt/kvm/irq_comm.c | 75 > > > > 3 files changed, 102 insertions(+), 7 deletions(-) > > > > Finally-tested-by: Jan Kiszka Michael, we need either this or the simple oneshot patch to get device assignment working again for 3.5. Are you planning to push this for 3.5? Thanks, Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On 2012-06-11 13:19, Michael S. Tsirkin wrote: > We can deliver certain interrupts, notably MSIX, > from atomic context. > Here's an untested patch to do this (compiled only). > > Changes from v2: > Don't inject broadcast interrupts directly > Changes from v1: > Tried to address comments from v1, except unifying > with kvm_set_irq: passing flags to it looks too ugly. > Added a comment. > > Jan, you said you can test this? > > > Michael S. Tsirkin (2): > kvm: implement kvm_set_msi_inatomic > kvm: deliver msix interrupts from irq handler > > include/linux/kvm_host.h | 3 ++ > virt/kvm/assigned-dev.c | 31 ++-- > virt/kvm/irq_comm.c | 75 > > 3 files changed, 102 insertions(+), 7 deletions(-) > Finally-tested-by: Jan Kiszka Thanks, Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On 06/13/2012 02:07 AM, Marcelo Tosatti wrote: > On Mon, Jun 11, 2012 at 02:19:17PM +0300, Michael S. Tsirkin wrote: >> We can deliver certain interrupts, notably MSIX, >> from atomic context. >> Here's an untested patch to do this (compiled only). >> >> Changes from v2: >> Don't inject broadcast interrupts directly >> Changes from v1: >> Tried to address comments from v1, except unifying >> with kvm_set_irq: passing flags to it looks too ugly. >> Added a comment. >> >> Jan, you said you can test this? > > Looks fine to me (except the unlikely). Avi/Gleb? I dislike the vcpu loop in atomic context. Some crazies keep increasing the maximum vcpu count. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On Tue, Jun 12, 2012 at 08:07:03PM -0300, Marcelo Tosatti wrote: > On Mon, Jun 11, 2012 at 02:19:17PM +0300, Michael S. Tsirkin wrote: > > We can deliver certain interrupts, notably MSIX, > > from atomic context. > > Here's an untested patch to do this (compiled only). > > > > Changes from v2: > > Don't inject broadcast interrupts directly > > Changes from v1: > > Tried to address comments from v1, except unifying > > with kvm_set_irq: passing flags to it looks too ugly. > > Added a comment. > > > > Jan, you said you can test this? > > Looks fine to me (except the unlikely). Avi/Gleb? Which of the unlikely do you mean? Most of them cover the case where msix is a broadcast or where level is 0 for msi. This never triggered in my testing. So the unlikely seems justified. -- MST -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 RFC 0/2] kvm: direct msix injection
On Mon, Jun 11, 2012 at 02:19:17PM +0300, Michael S. Tsirkin wrote: > We can deliver certain interrupts, notably MSIX, > from atomic context. > Here's an untested patch to do this (compiled only). > > Changes from v2: > Don't inject broadcast interrupts directly > Changes from v1: > Tried to address comments from v1, except unifying > with kvm_set_irq: passing flags to it looks too ugly. > Added a comment. > > Jan, you said you can test this? Looks fine to me (except the unlikely). Avi/Gleb? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
