[RFC,v1,5/5] Call irqbypass update routine when updating irqfd
diff mbox

Message ID 1436497207-4786-6-git-send-email-feng.wu@intel.com
State New
Headers show

Commit Message

Wu, Feng July 10, 2015, 3 a.m. UTC
Call update routine when updating irqfd, this can update the
IRTE for Intel posted-interrupts.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 virt/kvm/eventfd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Alex Williamson July 10, 2015, 3:26 a.m. UTC | #1
On Fri, 2015-07-10 at 11:00 +0800, Feng Wu wrote:
> Call update routine when updating irqfd, this can update the
> IRTE for Intel posted-interrupts.
> 
> Signed-off-by: Feng Wu <feng.wu@intel.com>
> ---
>  virt/kvm/eventfd.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index a32cf6c..1226835 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -570,8 +570,10 @@ void kvm_irq_routing_update(struct kvm *kvm)
>  
>  	spin_lock_irq(&kvm->irqfds.lock);
>  
> -	list_for_each_entry(irqfd, &kvm->irqfds.items, list)
> +	list_for_each_entry(irqfd, &kvm->irqfds.items, list) {
>  		irqfd_update(kvm, irqfd);
> +		irqfd->consumer.update(&irqfd->consumer);
> +	}
>  
>  	spin_unlock_irq(&kvm->irqfds.lock);
>  }

I don't understand why the irq bypass manager needs to know about this
update callback.  We could just as easily make it be a function pointer
on the irqfd structure or maybe just open code it.  It's defined by the
consumer and called by the consumer, the irq bypass manager shouldn't
know about it.  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Feng July 10, 2015, 4:12 a.m. UTC | #2
> -----Original Message-----

> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org] On

> Behalf Of Alex Williamson

> Sent: Friday, July 10, 2015 11:26 AM

> To: Wu, Feng

> Cc: kvm@vger.kernel.org; pbonzini@redhat.com; joro@8bytes.org;

> avi.kivity@gmail.com; eric.auger@linaro.org

> Subject: Re: [RFC v1 5/5] Call irqbypass update routine when updating irqfd

> 

> On Fri, 2015-07-10 at 11:00 +0800, Feng Wu wrote:

> > Call update routine when updating irqfd, this can update the

> > IRTE for Intel posted-interrupts.

> >

> > Signed-off-by: Feng Wu <feng.wu@intel.com>

> > ---

> >  virt/kvm/eventfd.c | 4 +++-

> >  1 file changed, 3 insertions(+), 1 deletion(-)

> >

> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c

> > index a32cf6c..1226835 100644

> > --- a/virt/kvm/eventfd.c

> > +++ b/virt/kvm/eventfd.c

> > @@ -570,8 +570,10 @@ void kvm_irq_routing_update(struct kvm *kvm)

> >

> >  	spin_lock_irq(&kvm->irqfds.lock);

> >

> > -	list_for_each_entry(irqfd, &kvm->irqfds.items, list)

> > +	list_for_each_entry(irqfd, &kvm->irqfds.items, list) {

> >  		irqfd_update(kvm, irqfd);

> > +		irqfd->consumer.update(&irqfd->consumer);

> > +	}

> >

> >  	spin_unlock_irq(&kvm->irqfds.lock);

> >  }

> 

> I don't understand why the irq bypass manager needs to know about this

> update callback.  We could just as easily make it be a function pointer

> on the irqfd structure or maybe just open code it.  It's defined by the

> consumer and called by the consumer, the irq bypass manager shouldn't

> know about it.  Thanks,


Yes, you are right. All we need is the producer information which has been
passed in the register routine. And we can easily make this update logic
inside the consumer. Thanks for your comments!

Thanks,
Feng

> 

> Alex

> 

> --

> To unsubscribe from this list: send the line "unsubscribe kvm" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Feng July 10, 2015, 8:28 a.m. UTC | #3
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV3UsIEZlbmcNCj4gU2Vu
dDogRnJpZGF5LCBKdWx5IDEwLCAyMDE1IDEyOjEzIFBNDQo+IFRvOiBBbGV4IFdpbGxpYW1zb24N
Cj4gQ2M6IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IHBib256aW5pQHJlZGhhdC5jb207IGpvcm9AOGJ5
dGVzLm9yZzsNCj4gYXZpLmtpdml0eUBnbWFpbC5jb207IGVyaWMuYXVnZXJAbGluYXJvLm9yZzsg
V3UsIEZlbmcNCj4gU3ViamVjdDogUkU6IFtSRkMgdjEgNS81XSBDYWxsIGlycWJ5cGFzcyB1cGRh
dGUgcm91dGluZSB3aGVuIHVwZGF0aW5nIGlycWZkDQo+IA0KPiANCj4gDQo+ID4gLS0tLS1Pcmln
aW5hbCBNZXNzYWdlLS0tLS0NCj4gPiBGcm9tOiBrdm0tb3duZXJAdmdlci5rZXJuZWwub3JnIFtt
YWlsdG86a3ZtLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24NCj4gPiBCZWhhbGYgT2YgQWxleCBX
aWxsaWFtc29uDQo+ID4gU2VudDogRnJpZGF5LCBKdWx5IDEwLCAyMDE1IDExOjI2IEFNDQo+ID4g
VG86IFd1LCBGZW5nDQo+ID4gQ2M6IGt2bUB2Z2VyLmtlcm5lbC5vcmc7IHBib256aW5pQHJlZGhh
dC5jb207IGpvcm9AOGJ5dGVzLm9yZzsNCj4gPiBhdmkua2l2aXR5QGdtYWlsLmNvbTsgZXJpYy5h
dWdlckBsaW5hcm8ub3JnDQo+ID4gU3ViamVjdDogUmU6IFtSRkMgdjEgNS81XSBDYWxsIGlycWJ5
cGFzcyB1cGRhdGUgcm91dGluZSB3aGVuIHVwZGF0aW5nIGlycWZkDQo+ID4NCj4gPiBPbiBGcmks
IDIwMTUtMDctMTAgYXQgMTE6MDAgKzA4MDAsIEZlbmcgV3Ugd3JvdGU6DQo+ID4gPiBDYWxsIHVw
ZGF0ZSByb3V0aW5lIHdoZW4gdXBkYXRpbmcgaXJxZmQsIHRoaXMgY2FuIHVwZGF0ZSB0aGUNCj4g
PiA+IElSVEUgZm9yIEludGVsIHBvc3RlZC1pbnRlcnJ1cHRzLg0KPiA+ID4NCj4gPiA+IFNpZ25l
ZC1vZmYtYnk6IEZlbmcgV3UgPGZlbmcud3VAaW50ZWwuY29tPg0KPiA+ID4gLS0tDQo+ID4gPiAg
dmlydC9rdm0vZXZlbnRmZC5jIHwgNCArKystDQo+ID4gPiAgMSBmaWxlIGNoYW5nZWQsIDMgaW5z
ZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQ0KPiA+ID4NCj4gPiA+IGRpZmYgLS1naXQgYS92aXJ0
L2t2bS9ldmVudGZkLmMgYi92aXJ0L2t2bS9ldmVudGZkLmMNCj4gPiA+IGluZGV4IGEzMmNmNmMu
LjEyMjY4MzUgMTAwNjQ0DQo+ID4gPiAtLS0gYS92aXJ0L2t2bS9ldmVudGZkLmMNCj4gPiA+ICsr
KyBiL3ZpcnQva3ZtL2V2ZW50ZmQuYw0KPiA+ID4gQEAgLTU3MCw4ICs1NzAsMTAgQEAgdm9pZCBr
dm1faXJxX3JvdXRpbmdfdXBkYXRlKHN0cnVjdCBrdm0gKmt2bSkNCj4gPiA+DQo+ID4gPiAgCXNw
aW5fbG9ja19pcnEoJmt2bS0+aXJxZmRzLmxvY2spOw0KPiA+ID4NCj4gPiA+IC0JbGlzdF9mb3Jf
ZWFjaF9lbnRyeShpcnFmZCwgJmt2bS0+aXJxZmRzLml0ZW1zLCBsaXN0KQ0KPiA+ID4gKwlsaXN0
X2Zvcl9lYWNoX2VudHJ5KGlycWZkLCAma3ZtLT5pcnFmZHMuaXRlbXMsIGxpc3QpIHsNCj4gPiA+
ICAJCWlycWZkX3VwZGF0ZShrdm0sIGlycWZkKTsNCj4gPiA+ICsJCWlycWZkLT5jb25zdW1lci51
cGRhdGUoJmlycWZkLT5jb25zdW1lcik7DQo+ID4gPiArCX0NCj4gPiA+DQo+ID4gPiAgCXNwaW5f
dW5sb2NrX2lycSgma3ZtLT5pcnFmZHMubG9jayk7DQo+ID4gPiAgfQ0KPiA+DQo+ID4gSSBkb24n
dCB1bmRlcnN0YW5kIHdoeSB0aGUgaXJxIGJ5cGFzcyBtYW5hZ2VyIG5lZWRzIHRvIGtub3cgYWJv
dXQgdGhpcw0KPiA+IHVwZGF0ZSBjYWxsYmFjay4gIFdlIGNvdWxkIGp1c3QgYXMgZWFzaWx5IG1h
a2UgaXQgYmUgYSBmdW5jdGlvbiBwb2ludGVyDQo+ID4gb24gdGhlIGlycWZkIHN0cnVjdHVyZSBv
ciBtYXliZSBqdXN0IG9wZW4gY29kZSBpdC4gIEl0J3MgZGVmaW5lZCBieSB0aGUNCj4gPiBjb25z
dW1lciBhbmQgY2FsbGVkIGJ5IHRoZSBjb25zdW1lciwgdGhlIGlycSBieXBhc3MgbWFuYWdlciBz
aG91bGRuJ3QNCj4gPiBrbm93IGFib3V0IGl0LiAgVGhhbmtzLA0KPiANCj4gWWVzLCB5b3UgYXJl
IHJpZ2h0LiBBbGwgd2UgbmVlZCBpcyB0aGUgcHJvZHVjZXIgaW5mb3JtYXRpb24gd2hpY2ggaGFz
IGJlZW4NCj4gcGFzc2VkIGluIHRoZSByZWdpc3RlciByb3V0aW5lLiBBbmQgd2UgY2FuIGVhc2ls
eSBtYWtlIHRoaXMgdXBkYXRlIGxvZ2ljDQo+IGluc2lkZSB0aGUgY29uc3VtZXIuIFRoYW5rcyBm
b3IgeW91ciBjb21tZW50cyENCj4gDQo+IFRoYW5rcywNCj4gRmVuZw0KDQpCVFcsIFBhb2xvICYg
QWxleCwgaW4gVkZJTyBmcmFtZXdvcmssIGhvdyBjYW4gd2Uga25vdyBhIHZDUFUgb3IgYSBndWVz
dA0KaGFzIGFzc2lnbmVkIGRldmljZXMgdG8gaXQ/DQoNClRoYW5rcywNCkZlbmcNCg0KPiANCj4g
Pg0KPiA+IEFsZXgNCj4gPg0KPiA+IC0tDQo+ID4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxp
c3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGt2bSIgaW4NCj4gPiB0aGUgYm9keSBvZiBh
IG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiA+IE1vcmUgbWFqb3Jkb21v
IGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini July 10, 2015, 12:47 p.m. UTC | #4
On 10/07/2015 10:28, Wu, Feng wrote:
> > Yes, you are right. All we need is the producer information which has been
> > passed in the register routine. And we can easily make this update logic
> > inside the consumer. Thanks for your comments!
> 
> BTW, Paolo & Alex, in VFIO framework, how can we know a vCPU or a guest
> has assigned devices to it?

See here:
http://article.gmane.org/gmane.comp.emulators.kvm.devel/137930/raw

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Williamson July 10, 2015, 2:11 p.m. UTC | #5
On Fri, 2015-07-10 at 14:47 +0200, Paolo Bonzini wrote:
> 
> On 10/07/2015 10:28, Wu, Feng wrote:
> > > Yes, you are right. All we need is the producer information which has been
> > > passed in the register routine. And we can easily make this update logic
> > > inside the consumer. Thanks for your comments!
> > 
> > BTW, Paolo & Alex, in VFIO framework, how can we know a vCPU or a guest
> > has assigned devices to it?
> 
> See here:
> http://article.gmane.org/gmane.comp.emulators.kvm.devel/137930/raw


In general, VFIO has zero visibility into KVM.  VFIO doesn't know or
care what the userspace driver is, whether it's QEMU/KVM, a set of ruby
bindings for VFIO, a DPDK library, etc.  As Paolo points out, KVM does
have ways to be told about assigned devices from userspace and probe
some properties, like whether the IOMMU allows non-coherent DMA.  These
are handled by the KVM-VFIO pseudo device (virt/kvm/vfio.c).  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index a32cf6c..1226835 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -570,8 +570,10 @@  void kvm_irq_routing_update(struct kvm *kvm)
 
 	spin_lock_irq(&kvm->irqfds.lock);
 
-	list_for_each_entry(irqfd, &kvm->irqfds.items, list)
+	list_for_each_entry(irqfd, &kvm->irqfds.items, list) {
 		irqfd_update(kvm, irqfd);
+		irqfd->consumer.update(&irqfd->consumer);
+	}
 
 	spin_unlock_irq(&kvm->irqfds.lock);
 }