* Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Alex Williamson @ 2013-10-04 17:12 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: agraf@suse.de, Wood Scott-B07421, linux-pci@vger.kernel.org,
joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07190F35@039-SN2MPN1-011.039d.mgd.msft.net>
On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote:
>
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Friday, October 04, 2013 9:15 PM
> > To: Bhushan Bharat-R65777
> > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux-
> > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > foundation.org
> > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
> >
> > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote:
> > >
> > > > -----Original Message-----
> > > > From: linux-pci-owner@vger.kernel.org
> > > > [mailto:linux-pci-owner@vger.kernel.org]
> > > > On Behalf Of Alex Williamson
> > > > Sent: Wednesday, September 25, 2013 10:16 PM
> > > > To: Bhushan Bharat-R65777
> > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org;
> > > > Bhushan Bharat-R65777
> > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > device
> > > >
> > > > On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > > > > This api return the iommu domain to which the device is attached.
> > > > > The iommu_domain is required for making API calls related to iommu.
> > > > > Follow up patches which use this API to know iommu maping.
> > > > >
> > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > > > > ---
> > > > > drivers/iommu/iommu.c | 10 ++++++++++
> > > > > include/linux/iommu.h | 7 +++++++
> > > > > 2 files changed, 17 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index
> > > > > fbe9ca7..6ac5f50 100644
> > > > > --- a/drivers/iommu/iommu.c
> > > > > +++ b/drivers/iommu/iommu.c
> > > > > @@ -696,6 +696,16 @@ void iommu_detach_device(struct iommu_domain
> > > > > *domain, struct device *dev) }
> > > > > EXPORT_SYMBOL_GPL(iommu_detach_device);
> > > > >
> > > > > +struct iommu_domain *iommu_get_dev_domain(struct device *dev) {
> > > > > + struct iommu_ops *ops = dev->bus->iommu_ops;
> > > > > +
> > > > > + if (unlikely(ops == NULL || ops->get_dev_iommu_domain == NULL))
> > > > > + return NULL;
> > > > > +
> > > > > + return ops->get_dev_iommu_domain(dev); }
> > > > > +EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
> > > >
> > > > What prevents this from racing iommu_domain_free()? There's no
> > > > references acquired, so there's no reason for the caller to assume the
> > pointer is valid.
> > >
> > > Sorry for late query, somehow this email went into a folder and
> > > escaped;
> > >
> > > Just to be sure, there is not lock at generic "struct iommu_domain", but IP
> > specific structure (link FSL domain) linked in iommu_domain->priv have a lock,
> > so we need to ensure this race in FSL iommu code (say
> > drivers/iommu/fsl_pamu_domain.c), right?
> >
> > No, it's not sufficient to make sure that your use of the interface is race
> > free. The interface itself needs to be designed so that it's difficult to use
> > incorrectly.
>
> So we can define iommu_get_dev_domain()/iommu_put_dev_domain();
> iommu_get_dev_domain() will return domain with the lock held, and
> iommu_put_dev_domain() will release the lock? And
> iommu_get_dev_domain() must always be followed by
> iommu_get_dev_domain().
What lock? get/put are generally used for reference counting, not
locking in the kernel.
> > That's not the case here. This is a backdoor to get the iommu
> > domain from the iommu driver regardless of who is using it or how. The iommu
> > domain is created and managed by vfio, so shouldn't we be looking at how to do
> > this through vfio?
>
> Let me first describe what we are doing here:
> During initialization:-
> - vfio talks to MSI system to know the MSI-page and size
> - vfio then interacts with iommu to map the MSI-page in iommu (IOVA is decided by userspace and physical address is the MSI-page)
> - So the IOVA subwindow mapping is created in iommu and yes VFIO know about this mapping.
>
> Now do SET_IRQ(MSI/MSIX) ioctl:
> - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set MSI address/data in device.
> - So in current implementation (this patchset) msi-subsystem gets the IOVA from iommu via this defined interface.
> - Are you saying that rather than getting this from iommu, we should get this from vfio? What difference does this make?
Yes, you just said above that vfio knows the msi to iova mapping, so why
go outside of vfio to find it later? The difference is one case you can
have a proper reference to data structures to make sure the pointer you
get back actually has meaning at the time you're using it vs the code
here where you're defining an API that returns a meaningless value
because you can't check or enforce that an arbitrary caller is using it
correctly. It's not maintainable. Thanks,
Alex
> > It seems like you'd want to use your device to get a vfio
> > group reference, from which you could do something with the vfio external user
> > interface and get the iommu domain reference. Thanks,
> >
> > Alex
> >
> > > > > /*
> > > > > * IOMMU groups are really the natrual working unit of the IOMMU, but
> > > > > * the IOMMU API works on domains and devices. Bridge that gap
> > > > > by diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > > > > index 7ea319e..fa046bd 100644
> > > > > --- a/include/linux/iommu.h
> > > > > +++ b/include/linux/iommu.h
> > > > > @@ -127,6 +127,7 @@ struct iommu_ops {
> > > > > int (*domain_set_windows)(struct iommu_domain *domain, u32
> > w_count);
> > > > > /* Get the numer of window per domain */
> > > > > u32 (*domain_get_windows)(struct iommu_domain *domain);
> > > > > + struct iommu_domain *(*get_dev_iommu_domain)(struct device
> > > > > +*dev);
> > > > >
> > > > > unsigned long pgsize_bitmap;
> > > > > };
> > > > > @@ -190,6 +191,7 @@ extern int iommu_domain_window_enable(struct
> > > > > iommu_domain
> > > > *domain, u32 wnd_nr,
> > > > > phys_addr_t offset, u64 size,
> > > > > int prot);
> > > > > extern void iommu_domain_window_disable(struct iommu_domain
> > > > > *domain,
> > > > > u32 wnd_nr);
> > > > > +extern struct iommu_domain *iommu_get_dev_domain(struct device
> > > > > +*dev);
> > > > > /**
> > > > > * report_iommu_fault() - report about an IOMMU fault to the IOMMU
> > framework
> > > > > * @domain: the iommu domain where the fault has happened @@
> > > > > -284,6
> > > > > +286,11 @@ static inline void iommu_domain_window_disable(struct
> > > > > iommu_domain *domain, { }
> > > > >
> > > > > +static inline struct iommu_domain *iommu_get_dev_domain(struct
> > > > > +device
> > > > > +*dev) {
> > > > > + return NULL;
> > > > > +}
> > > > > +
> > > > > static inline phys_addr_t iommu_iova_to_phys(struct iommu_domain
> > > > > *domain, dma_addr_t iova) {
> > > > > return 0;
> > > >
> > > >
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> > > > in the body of a message to majordomo@vger.kernel.org More majordomo
> > > > info at http://vger.kernel.org/majordomo-info.html
> > >
> >
> >
> >
>
^ permalink raw reply
* RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Bhushan Bharat-R65777 @ 2013-10-04 17:23 UTC (permalink / raw)
To: Alex Williamson
Cc: agraf@suse.de, Wood Scott-B07421, linux-pci@vger.kernel.org,
joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1380906772.25705.19.camel@ul30vt.home>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u
IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IEZyaWRheSwgT2N0
b2JlciAwNCwgMjAxMyAxMDo0MyBQTQ0KPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+IENj
OiBqb3JvQDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgZ2FsYWtAa2VybmVs
LmNyYXNoaW5nLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBj
LWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0NCj4gcGNpQHZnZXIua2VybmVsLm9yZzsgYWdy
YWZAc3VzZS5kZTsgV29vZCBTY290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiBmb3Vu
ZGF0aW9uLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvN10gaW9tbXU6IGFkZCBhcGkgdG8g
Z2V0IGlvbW11X2RvbWFpbiBvZiBhIGRldmljZQ0KPiANCj4gT24gRnJpLCAyMDEzLTEwLTA0IGF0
IDE2OjQ3ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4NCj4gPiA+IC0t
LS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBBbGV4IFdpbGxpYW1zb24gW21h
aWx0bzphbGV4LndpbGxpYW1zb25AcmVkaGF0LmNvbV0NCj4gPiA+IFNlbnQ6IEZyaWRheSwgT2N0
b2JlciAwNCwgMjAxMyA5OjE1IFBNDQo+ID4gPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+
ID4gPiBDYzogam9yb0A4Ynl0ZXMub3JnOyBiZW5oQGtlcm5lbC5jcmFzaGluZy5vcmc7DQo+ID4g
PiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eC0ga2VybmVsQHZnZXIua2VybmVsLm9y
ZzsNCj4gPiA+IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0gcGNpQHZnZXIu
a2VybmVsLm9yZzsNCj4gPiA+IGFncmFmQHN1c2UuZGU7IFdvb2QgU2NvdHQtQjA3NDIxOyBpb21t
dUBsaXN0cy5saW51eC0gZm91bmRhdGlvbi5vcmcNCj4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
Mi83XSBpb21tdTogYWRkIGFwaSB0byBnZXQgaW9tbXVfZG9tYWluIG9mIGENCj4gPiA+IGRldmlj
ZQ0KPiA+ID4NCj4gPiA+IE9uIEZyaSwgMjAxMy0xMC0wNCBhdCAwOTo1NCArMDAwMCwgQmh1c2hh
biBCaGFyYXQtUjY1Nzc3IHdyb3RlOg0KPiA+ID4gPg0KPiA+ID4gPiA+IC0tLS0tT3JpZ2luYWwg
TWVzc2FnZS0tLS0tDQo+ID4gPiA+ID4gRnJvbTogbGludXgtcGNpLW93bmVyQHZnZXIua2VybmVs
Lm9yZw0KPiA+ID4gPiA+IFttYWlsdG86bGludXgtcGNpLW93bmVyQHZnZXIua2VybmVsLm9yZ10N
Cj4gPiA+ID4gPiBPbiBCZWhhbGYgT2YgQWxleCBXaWxsaWFtc29uDQo+ID4gPiA+ID4gU2VudDog
V2VkbmVzZGF5LCBTZXB0ZW1iZXIgMjUsIDIwMTMgMTA6MTYgUE0NCj4gPiA+ID4gPiBUbzogQmh1
c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gQ2M6IGpvcm9AOGJ5dGVzLm9yZzsgYmVuaEBr
ZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiA+ID4gPiA+IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7
IGxpbnV4LSBrZXJuZWxAdmdlci5rZXJuZWwub3JnOw0KPiA+ID4gPiA+IGxpbnV4cHBjLWRldkBs
aXN0cy5vemxhYnMub3JnOyBsaW51eC0gcGNpQHZnZXIua2VybmVsLm9yZzsNCj4gPiA+ID4gPiBh
Z3JhZkBzdXNlLmRlOyBXb29kIFNjb3R0LUIwNzQyMTsgaW9tbXVAbGlzdHMubGludXgtDQo+ID4g
PiA+ID4gZm91bmRhdGlvbi5vcmc7IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiA+ID4gPiA+IFN1
YmplY3Q6IFJlOiBbUEFUQ0ggMi83XSBpb21tdTogYWRkIGFwaSB0byBnZXQgaW9tbXVfZG9tYWlu
IG9mIGENCj4gPiA+ID4gPiBkZXZpY2UNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IE9uIFRodSwgMjAx
My0wOS0xOSBhdCAxMjo1OSArMDUzMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+ID4gPiA+ID4g
PiBUaGlzIGFwaSByZXR1cm4gdGhlIGlvbW11IGRvbWFpbiB0byB3aGljaCB0aGUgZGV2aWNlIGlz
IGF0dGFjaGVkLg0KPiA+ID4gPiA+ID4gVGhlIGlvbW11X2RvbWFpbiBpcyByZXF1aXJlZCBmb3Ig
bWFraW5nIEFQSSBjYWxscyByZWxhdGVkIHRvIGlvbW11Lg0KPiA+ID4gPiA+ID4gRm9sbG93IHVw
IHBhdGNoZXMgd2hpY2ggdXNlIHRoaXMgQVBJIHRvIGtub3cgaW9tbXUgbWFwaW5nLg0KPiA+ID4g
PiA+ID4NCj4gPiA+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IEJoYXJhdCBCaHVzaGFuIDxiaGFyYXQu
Ymh1c2hhbkBmcmVlc2NhbGUuY29tPg0KPiA+ID4gPiA+ID4gLS0tDQo+ID4gPiA+ID4gPiAgZHJp
dmVycy9pb21tdS9pb21tdS5jIHwgICAxMCArKysrKysrKysrDQo+ID4gPiA+ID4gPiAgaW5jbHVk
ZS9saW51eC9pb21tdS5oIHwgICAgNyArKysrKysrDQo+ID4gPiA+ID4gPiAgMiBmaWxlcyBjaGFu
Z2VkLCAxNyBpbnNlcnRpb25zKCspLCAwIGRlbGV0aW9ucygtKQ0KPiA+ID4gPiA+ID4NCj4gPiA+
ID4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lvbW11L2lvbW11LmMgYi9kcml2ZXJzL2lvbW11
L2lvbW11LmMNCj4gPiA+ID4gPiA+IGluZGV4DQo+ID4gPiA+ID4gPiBmYmU5Y2E3Li42YWM1ZjUw
IDEwMDY0NA0KPiA+ID4gPiA+ID4gLS0tIGEvZHJpdmVycy9pb21tdS9pb21tdS5jDQo+ID4gPiA+
ID4gPiArKysgYi9kcml2ZXJzL2lvbW11L2lvbW11LmMNCj4gPiA+ID4gPiA+IEBAIC02OTYsNiAr
Njk2LDE2IEBAIHZvaWQgaW9tbXVfZGV0YWNoX2RldmljZShzdHJ1Y3QNCj4gPiA+ID4gPiA+IGlv
bW11X2RvbWFpbiAqZG9tYWluLCBzdHJ1Y3QgZGV2aWNlICpkZXYpICB9DQo+ID4gPiA+ID4gPiBF
WFBPUlRfU1lNQk9MX0dQTChpb21tdV9kZXRhY2hfZGV2aWNlKTsNCj4gPiA+ID4gPiA+DQo+ID4g
PiA+ID4gPiArc3RydWN0IGlvbW11X2RvbWFpbiAqaW9tbXVfZ2V0X2Rldl9kb21haW4oc3RydWN0
IGRldmljZSAqZGV2KSB7DQo+ID4gPiA+ID4gPiArCXN0cnVjdCBpb21tdV9vcHMgKm9wcyA9IGRl
di0+YnVzLT5pb21tdV9vcHM7DQo+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4gPiArCWlmICh1bmxp
a2VseShvcHMgPT0gTlVMTCB8fCBvcHMtPmdldF9kZXZfaW9tbXVfZG9tYWluID09IE5VTEwpKQ0K
PiA+ID4gPiA+ID4gKwkJcmV0dXJuIE5VTEw7DQo+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4gPiAr
CXJldHVybiBvcHMtPmdldF9kZXZfaW9tbXVfZG9tYWluKGRldik7IH0NCj4gPiA+ID4gPiA+ICtF
WFBPUlRfU1lNQk9MX0dQTChpb21tdV9nZXRfZGV2X2RvbWFpbik7DQo+ID4gPiA+ID4NCj4gPiA+
ID4gPiBXaGF0IHByZXZlbnRzIHRoaXMgZnJvbSByYWNpbmcgaW9tbXVfZG9tYWluX2ZyZWUoKT8g
IFRoZXJlJ3Mgbm8NCj4gPiA+ID4gPiByZWZlcmVuY2VzIGFjcXVpcmVkLCBzbyB0aGVyZSdzIG5v
IHJlYXNvbiBmb3IgdGhlIGNhbGxlciB0bw0KPiA+ID4gPiA+IGFzc3VtZSB0aGUNCj4gPiA+IHBv
aW50ZXIgaXMgdmFsaWQuDQo+ID4gPiA+DQo+ID4gPiA+IFNvcnJ5IGZvciBsYXRlIHF1ZXJ5LCBz
b21laG93IHRoaXMgZW1haWwgd2VudCBpbnRvIGEgZm9sZGVyIGFuZA0KPiA+ID4gPiBlc2NhcGVk
Ow0KPiA+ID4gPg0KPiA+ID4gPiBKdXN0IHRvIGJlIHN1cmUsIHRoZXJlIGlzIG5vdCBsb2NrIGF0
IGdlbmVyaWMgInN0cnVjdA0KPiA+ID4gPiBpb21tdV9kb21haW4iLCBidXQgSVANCj4gPiA+IHNw
ZWNpZmljIHN0cnVjdHVyZSAobGluayBGU0wgZG9tYWluKSBsaW5rZWQgaW4gaW9tbXVfZG9tYWlu
LT5wcml2DQo+ID4gPiBoYXZlIGEgbG9jaywgc28gd2UgbmVlZCB0byBlbnN1cmUgdGhpcyByYWNl
IGluIEZTTCBpb21tdSBjb2RlIChzYXkNCj4gPiA+IGRyaXZlcnMvaW9tbXUvZnNsX3BhbXVfZG9t
YWluLmMpLCByaWdodD8NCj4gPiA+DQo+ID4gPiBObywgaXQncyBub3Qgc3VmZmljaWVudCB0byBt
YWtlIHN1cmUgdGhhdCB5b3VyIHVzZSBvZiB0aGUgaW50ZXJmYWNlDQo+ID4gPiBpcyByYWNlIGZy
ZWUuICBUaGUgaW50ZXJmYWNlIGl0c2VsZiBuZWVkcyB0byBiZSBkZXNpZ25lZCBzbyB0aGF0DQo+
ID4gPiBpdCdzIGRpZmZpY3VsdCB0byB1c2UgaW5jb3JyZWN0bHkuDQo+ID4NCj4gPiBTbyB3ZSBj
YW4gZGVmaW5lIGlvbW11X2dldF9kZXZfZG9tYWluKCkvaW9tbXVfcHV0X2Rldl9kb21haW4oKTsN
Cj4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpIHdpbGwgcmV0dXJuIGRvbWFpbiB3aXRoIHRoZSBs
b2NrIGhlbGQsIGFuZA0KPiA+IGlvbW11X3B1dF9kZXZfZG9tYWluKCkgd2lsbCByZWxlYXNlIHRo
ZSBsb2NrPyBBbmQNCj4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpIG11c3QgYWx3YXlzIGJlIGZv
bGxvd2VkIGJ5DQo+ID4gaW9tbXVfZ2V0X2Rldl9kb21haW4oKS4NCj4gDQo+IFdoYXQgbG9jaz8g
IGdldC9wdXQgYXJlIGdlbmVyYWxseSB1c2VkIGZvciByZWZlcmVuY2UgY291bnRpbmcsIG5vdCBs
b2NraW5nIGluDQo+IHRoZSBrZXJuZWwuDQo+IA0KPiA+ID4gVGhhdCdzIG5vdCB0aGUgY2FzZSBo
ZXJlLiAgVGhpcyBpcyBhIGJhY2tkb29yIHRvIGdldCB0aGUgaW9tbXUNCj4gPiA+IGRvbWFpbiBm
cm9tIHRoZSBpb21tdSBkcml2ZXIgcmVnYXJkbGVzcyBvZiB3aG8gaXMgdXNpbmcgaXQgb3IgaG93
Lg0KPiA+ID4gVGhlIGlvbW11IGRvbWFpbiBpcyBjcmVhdGVkIGFuZCBtYW5hZ2VkIGJ5IHZmaW8s
IHNvIHNob3VsZG4ndCB3ZSBiZQ0KPiA+ID4gbG9va2luZyBhdCBob3cgdG8gZG8gdGhpcyB0aHJv
dWdoIHZmaW8/DQo+ID4NCj4gPiBMZXQgbWUgZmlyc3QgZGVzY3JpYmUgd2hhdCB3ZSBhcmUgZG9p
bmcgaGVyZToNCj4gPiBEdXJpbmcgaW5pdGlhbGl6YXRpb246LQ0KPiA+ICAtIHZmaW8gdGFsa3Mg
dG8gTVNJIHN5c3RlbSB0byBrbm93IHRoZSBNU0ktcGFnZSBhbmQgc2l6ZQ0KPiA+ICAtIHZmaW8g
dGhlbiBpbnRlcmFjdHMgd2l0aCBpb21tdSB0byBtYXAgdGhlIE1TSS1wYWdlIGluIGlvbW11IChJ
T1ZBDQo+ID4gaXMgZGVjaWRlZCBieSB1c2Vyc3BhY2UgYW5kIHBoeXNpY2FsIGFkZHJlc3MgaXMg
dGhlIE1TSS1wYWdlKQ0KPiA+ICAtIFNvIHRoZSBJT1ZBIHN1YndpbmRvdyBtYXBwaW5nIGlzIGNy
ZWF0ZWQgaW4gaW9tbXUgYW5kIHllcyBWRklPIGtub3cgYWJvdXQNCj4gdGhpcyBtYXBwaW5nLg0K
PiA+DQo+ID4gTm93IGRvIFNFVF9JUlEoTVNJL01TSVgpIGlvY3RsOg0KPiA+ICAtIGNhbGxzIHBj
aV9lbmFibGVfbXNpeCgpL3BjaV9lbmFibGVfbXNpX2Jsb2NrKCk6IHdoaWNoIGlzIHN1cHBvc2Vk
IHRvIHNldA0KPiBNU0kgYWRkcmVzcy9kYXRhIGluIGRldmljZS4NCj4gPiAgLSBTbyBpbiBjdXJy
ZW50IGltcGxlbWVudGF0aW9uICh0aGlzIHBhdGNoc2V0KSBtc2ktc3Vic3lzdGVtIGdldHMgdGhl
IElPVkENCj4gZnJvbSBpb21tdSB2aWEgdGhpcyBkZWZpbmVkIGludGVyZmFjZS4NCj4gPiAgLSBB
cmUgeW91IHNheWluZyB0aGF0IHJhdGhlciB0aGFuIGdldHRpbmcgdGhpcyBmcm9tIGlvbW11LCB3
ZSBzaG91bGQgZ2V0IHRoaXMNCj4gZnJvbSB2ZmlvPyBXaGF0IGRpZmZlcmVuY2UgZG9lcyB0aGlz
IG1ha2U/DQo+IA0KPiBZZXMsIHlvdSBqdXN0IHNhaWQgYWJvdmUgdGhhdCB2ZmlvIGtub3dzIHRo
ZSBtc2kgdG8gaW92YSBtYXBwaW5nLCBzbyB3aHkgZ28NCj4gb3V0c2lkZSBvZiB2ZmlvIHRvIGZp
bmQgaXQgbGF0ZXI/ICBUaGUgZGlmZmVyZW5jZSBpcyBvbmUgY2FzZSB5b3UgY2FuIGhhdmUgYQ0K
PiBwcm9wZXIgcmVmZXJlbmNlIHRvIGRhdGEgc3RydWN0dXJlcyB0byBtYWtlIHN1cmUgdGhlIHBv
aW50ZXIgeW91IGdldCBiYWNrDQo+IGFjdHVhbGx5IGhhcyBtZWFuaW5nIGF0IHRoZSB0aW1lIHlv
dSdyZSB1c2luZyBpdCB2cyB0aGUgY29kZSBoZXJlIHdoZXJlIHlvdSdyZQ0KPiBkZWZpbmluZyBh
biBBUEkgdGhhdCByZXR1cm5zIGEgbWVhbmluZ2xlc3MgdmFsdWUNCg0KV2l0aCBGU0wtUEFNVSB3
ZSB3aWxsIGFsd2F5cyBnZXQgY29uc2lzdGFudCBkYXRhIGZyb20gaW9tbXUgb3IgdmZpby1kYXRh
IHN0cnVjdHVyZS4NCg0KPiBiZWNhdXNlIHlvdSBjYW4ndCBjaGVjayBvcg0KPiBlbmZvcmNlIHRo
YXQgYW4gYXJiaXRyYXJ5IGNhbGxlciBpcyB1c2luZyBpdCBjb3JyZWN0bHkuDQoNCkkgYW0gbm90
IHN1cmUgd2hhdCBpcyBhcmJpdHJhcnkgY2FsbGVyPyBwZGV2IGlzIGtub3duIHRvIHZmaW8sIHNv
IHZmaW8gd2lsbCBvbmx5IG1ha2UgcGNpX2VuYWJsZV9tc2l4KCkvcGNpX2VuYWJsZV9tc2lfYmxv
Y2soKSBmb3IgdGhpcyBwZGV2LiBJZiBhbnlvdGhlciBjb2RlIG1ha2VzIHRoZW4gaXQgaXMgc29t
ZSBvdGhlciB1bmV4cGVjdGVkbHkgdGhpbmcgaGFwcGVuaW5nIGluIHN5c3RlbSwgbm8/DQoNCg0K
PiAgSXQncyBub3QgbWFpbnRhaW5hYmxlLg0KPiBUaGFua3MsDQoNCkkgZG8gbm90IGhhdmUgYW55
IGlzc3VlIHdpdGggdGhpcyBhcyB3ZWxsLCBjYW4geW91IGFsc28gZGVzY3JpYmUgdGhlIHR5cGUg
b2YgQVBJIHlvdSBhcmUgZW52aXNpb25pbmc7DQpJIGNhbiB0aGluayBvZiBkZWZpbmluZyBzb21l
IGZ1bmN0aW9uIGluIHZmaW8uYy92ZmlvX2lvbW11Ki5jLCBtYWtlIHRoZW0gZ2xvYmFsIGFuZCBk
ZWNsYXJlIHRoZW4gaW4gaW5jbHVkZS9MaW51eC92ZmlvLmgNCkFuZCBpbmNsdWRlIDxMaW51eC92
ZmlvLmg+IGluIGNhbGxlciBmaWxlIChhcmNoL3Bvd2VycGMva2VybmVsL21zaS5jKQ0KDQpUaGFu
a3MNCi1CaGFyYXQNCj4gDQo+IEFsZXgNCj4gDQo+ID4gPiBJdCBzZWVtcyBsaWtlIHlvdSdkIHdh
bnQgdG8gdXNlIHlvdXIgZGV2aWNlIHRvIGdldCBhIHZmaW8gZ3JvdXANCj4gPiA+IHJlZmVyZW5j
ZSwgZnJvbSB3aGljaCB5b3UgY291bGQgZG8gc29tZXRoaW5nIHdpdGggdGhlIHZmaW8gZXh0ZXJu
YWwNCj4gPiA+IHVzZXIgaW50ZXJmYWNlIGFuZCBnZXQgdGhlIGlvbW11IGRvbWFpbiByZWZlcmVu
Y2UuICBUaGFua3MsDQo+ID4gPg0KPiA+ID4gQWxleA0KPiA+ID4NCj4gPiA+ID4gPiA+ICAvKg0K
PiA+ID4gPiA+ID4gICAqIElPTU1VIGdyb3VwcyBhcmUgcmVhbGx5IHRoZSBuYXRydWFsIHdvcmtp
bmcgdW5pdCBvZiB0aGUgSU9NTVUsIGJ1dA0KPiA+ID4gPiA+ID4gICAqIHRoZSBJT01NVSBBUEkg
d29ya3Mgb24gZG9tYWlucyBhbmQgZGV2aWNlcy4gIEJyaWRnZSB0aGF0DQo+ID4gPiA+ID4gPiBn
YXAgYnkgZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvaW9tbXUuaA0KPiA+ID4gPiA+ID4gYi9p
bmNsdWRlL2xpbnV4L2lvbW11LmggaW5kZXggN2VhMzE5ZS4uZmEwNDZiZCAxMDA2NDQNCj4gPiA+
ID4gPiA+IC0tLSBhL2luY2x1ZGUvbGludXgvaW9tbXUuaA0KPiA+ID4gPiA+ID4gKysrIGIvaW5j
bHVkZS9saW51eC9pb21tdS5oDQo+ID4gPiA+ID4gPiBAQCAtMTI3LDYgKzEyNyw3IEBAIHN0cnVj
dCBpb21tdV9vcHMgew0KPiA+ID4gPiA+ID4gIAlpbnQgKCpkb21haW5fc2V0X3dpbmRvd3MpKHN0
cnVjdCBpb21tdV9kb21haW4gKmRvbWFpbiwgdTMyDQo+ID4gPiB3X2NvdW50KTsNCj4gPiA+ID4g
PiA+ICAJLyogR2V0IHRoZSBudW1lciBvZiB3aW5kb3cgcGVyIGRvbWFpbiAqLw0KPiA+ID4gPiA+
ID4gIAl1MzIgKCpkb21haW5fZ2V0X3dpbmRvd3MpKHN0cnVjdCBpb21tdV9kb21haW4gKmRvbWFp
bik7DQo+ID4gPiA+ID4gPiArCXN0cnVjdCBpb21tdV9kb21haW4gKigqZ2V0X2Rldl9pb21tdV9k
b21haW4pKHN0cnVjdCBkZXZpY2UNCj4gPiA+ID4gPiA+ICsqZGV2KTsNCj4gPiA+ID4gPiA+DQo+
ID4gPiA+ID4gPiAgCXVuc2lnbmVkIGxvbmcgcGdzaXplX2JpdG1hcDsgIH07IEBAIC0xOTAsNiAr
MTkxLDcgQEAgZXh0ZXJuDQo+ID4gPiA+ID4gPiBpbnQgaW9tbXVfZG9tYWluX3dpbmRvd19lbmFi
bGUoc3RydWN0IGlvbW11X2RvbWFpbg0KPiA+ID4gPiA+ICpkb21haW4sIHUzMiB3bmRfbnIsDQo+
ID4gPiA+ID4gPiAgCQkJCSAgICAgIHBoeXNfYWRkcl90IG9mZnNldCwgdTY0IHNpemUsDQo+ID4g
PiA+ID4gPiAgCQkJCSAgICAgIGludCBwcm90KTsNCj4gPiA+ID4gPiA+ICBleHRlcm4gdm9pZCBp
b21tdV9kb21haW5fd2luZG93X2Rpc2FibGUoc3RydWN0IGlvbW11X2RvbWFpbg0KPiA+ID4gPiA+
ID4gKmRvbWFpbiwNCj4gPiA+ID4gPiA+IHUzMiB3bmRfbnIpOw0KPiA+ID4gPiA+ID4gK2V4dGVy
biBzdHJ1Y3QgaW9tbXVfZG9tYWluICppb21tdV9nZXRfZGV2X2RvbWFpbihzdHJ1Y3QNCj4gPiA+
ID4gPiA+ICtkZXZpY2UgKmRldik7DQo+ID4gPiA+ID4gPiAgLyoqDQo+ID4gPiA+ID4gPiAgICog
cmVwb3J0X2lvbW11X2ZhdWx0KCkgLSByZXBvcnQgYWJvdXQgYW4gSU9NTVUgZmF1bHQgdG8gdGhl
DQo+ID4gPiA+ID4gPiBJT01NVQ0KPiA+ID4gZnJhbWV3b3JrDQo+ID4gPiA+ID4gPiAgICogQGRv
bWFpbjogdGhlIGlvbW11IGRvbWFpbiB3aGVyZSB0aGUgZmF1bHQgaGFzIGhhcHBlbmVkIEBADQo+
ID4gPiA+ID4gPiAtMjg0LDYNCj4gPiA+ID4gPiA+ICsyODYsMTEgQEAgc3RhdGljIGlubGluZSB2
b2lkDQo+ID4gPiA+ID4gPiAraW9tbXVfZG9tYWluX3dpbmRvd19kaXNhYmxlKHN0cnVjdA0KPiA+
ID4gPiA+ID4gaW9tbXVfZG9tYWluICpkb21haW4sICB7ICB9DQo+ID4gPiA+ID4gPg0KPiA+ID4g
PiA+ID4gK3N0YXRpYyBpbmxpbmUgc3RydWN0IGlvbW11X2RvbWFpbg0KPiA+ID4gPiA+ID4gKypp
b21tdV9nZXRfZGV2X2RvbWFpbihzdHJ1Y3QgZGV2aWNlDQo+ID4gPiA+ID4gPiArKmRldikgew0K
PiA+ID4gPiA+ID4gKwlyZXR1cm4gTlVMTDsNCj4gPiA+ID4gPiA+ICt9DQo+ID4gPiA+ID4gPiAr
DQo+ID4gPiA+ID4gPiAgc3RhdGljIGlubGluZSBwaHlzX2FkZHJfdCBpb21tdV9pb3ZhX3RvX3Bo
eXMoc3RydWN0DQo+ID4gPiA+ID4gPiBpb21tdV9kb21haW4gKmRvbWFpbiwgZG1hX2FkZHJfdCBp
b3ZhKSAgew0KPiA+ID4gPiA+ID4gIAlyZXR1cm4gMDsNCj4gPiA+ID4gPg0KPiA+ID4gPiA+DQo+
ID4gPiA+ID4NCj4gPiA+ID4gPiAtLQ0KPiA+ID4gPiA+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhp
cyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1wY2kiDQo+ID4gPiA+ID4g
aW4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcgTW9y
ZQ0KPiA+ID4gPiA+IG1ham9yZG9tbyBpbmZvIGF0IGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq
b3Jkb21vLWluZm8uaHRtbA0KPiA+ID4gPg0KPiA+ID4NCj4gPiA+DQo+ID4gPg0KPiA+DQo+IA0K
PiANCj4gDQoNCg==
^ permalink raw reply
* Re: [PATCH v3 net-next] fix unsafe set_memory_rw from softirq
From: Ingo Molnar @ 2013-10-04 17:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: Heiko Carstens, Paul Mackerras, H. Peter Anvin, sparclinux,
Nicolas Dichtel, Alexei Starovoitov, linux-s390, Russell King,
x86, James Morris, Ingo Molnar, Alexey Kuznetsov,
Paul E. McKenney, Xi Wang, Matt Evans, Thomas Gleixner,
linux-arm-kernel, Stelian Nirlu, Nicolas Schichan,
Hideaki YOSHIFUJI, netdev, linux-kernel, David S. Miller,
Mircea Gherzan, Daniel Borkmann, Martin Schwidefsky, linux390,
linuxppc-dev, Patrick McHardy
In-Reply-To: <CANn89iKkbR0_HaofvC_OVvqRv_Hqj3rATx-Z_4xXeusOasa56g@mail.gmail.com>
* Eric Dumazet <edumazet@google.com> wrote:
> 1)
> >
> > I took a brief look at arch/x86/net/bpf_jit_comp.c while reviewing this
> > patch.
> >
> > You need to split up bpf_jit_compile(), it's an obscenely large, ~600
> > lines long function. We don't do that in modern, maintainable kernel code.
> >
> > 2)
> >
> > This 128 bytes extra padding:
> >
> > /* Most of BPF filters are really small,
> > * but if some of them fill a page, allow at least
> > * 128 extra bytes to insert a random section of int3
> > */
> > sz = round_up(proglen + sizeof(*header) + 128, PAGE_SIZE);
> >
> > why is it done? It's not clear to me from the comment.
> >
>
> commit 314beb9bcabfd6b4542ccbced2402af2c6f6142a
> Author: Eric Dumazet <edumazet@google.com>
> Date: Fri May 17 16:37:03 2013 +0000
>
> x86: bpf_jit_comp: secure bpf jit against spraying attacks
>
> hpa bringed into my attention some security related issues
> with BPF JIT on x86.
>
> This patch makes sure the bpf generated code is marked read only,
> as other kernel text sections.
>
> It also splits the unused space (we vmalloc() and only use a fraction of
> the page) in two parts, so that the generated bpf code not starts at a
> known offset in the page, but a pseudo random one.
Thanks for the explanation - that makes sense.
Ingo
^ permalink raw reply
* Re: [PATCH v3 net-next] fix unsafe set_memory_rw from softirq
From: Alexei Starovoitov @ 2013-10-04 17:49 UTC (permalink / raw)
To: Ingo Molnar
Cc: Heiko Carstens, Eric Dumazet, Paul Mackerras, H. Peter Anvin,
sparclinux, Nicolas Dichtel, linux-s390, Russell King, x86,
James Morris, Ingo Molnar, Alexey Kuznetsov, Paul E. McKenney,
Xi Wang, Matt Evans, Thomas Gleixner, linux-arm-kernel,
Stelian Nirlu, Nicolas Schichan, Hideaki YOSHIFUJI, netdev,
linux-kernel, David S. Miller, Mircea Gherzan, Daniel Borkmann,
Martin Schwidefsky, linux390, linuxppc-dev, Patrick McHardy
In-Reply-To: <20131004075133.GA12313@gmail.com>
On Fri, Oct 4, 2013 at 12:51 AM, Ingo Molnar <mingo@kernel.org> wrote:
>> +static void bpf_jit_free_deferred(struct work_struct *work)
>> +{
>> + struct sk_filter *fp = container_of((void *)work, struct sk_filter,
>> + insns);
>> + unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
>> + struct bpf_binary_header *header = (void *)addr;
>> +
>> + set_memory_rw(addr, header->pages);
>> + module_free(NULL, header);
>> + kfree(fp);
>> +}
>
> Using the data type suggestions I make further below, this could be
> written in a simpler form, as:
>
> struct sk_filter *fp = container_of(work, struct sk_filter, work);
yes. I've made it already as part of V4
> Also, a question, why do you mask with PAGE_MASK here:
>
> unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
>
> ?
>
> AFAICS bpf_func is the module_alloc() result - and module code is page
> aligned. So ->bpf_func is always page aligned here. (The sk_run_filter
> special case cannot happen here.)
randomization of bpf_func start is a prevention of jit spraying
attacks as Eric pointed out.
>> void bpf_jit_free(struct sk_filter *fp)
>> {
>> if (fp->bpf_func != sk_run_filter) {
>> - unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
>> - struct bpf_binary_header *header = (void *)addr;
>> -
>> - set_memory_rw(addr, header->pages);
>> - module_free(NULL, header);
>> + struct work_struct *work = (struct work_struct *)fp->insns;
>> + INIT_WORK(work, bpf_jit_free_deferred);
>
> Missing newline between local variables and statements.
yes. noted and fixed in V4.
>> unsigned int (*bpf_func)(const struct sk_buff *skb,
>> const struct sock_filter *filter);
>> - struct rcu_head rcu;
>> + /* insns start right after bpf_func, so that sk_run_filter() fetches
>> + * first insn from the same cache line that was used to call into
>> + * sk_run_filter()
>> + */
>> struct sock_filter insns[0];
>
> Please use the customary (multi-line) comment style:
>
> /*
> * Comment .....
> * ...... goes here.
> */
>
> specified in Documentation/CodingStyle.
I believe filter.h belongs to networking comment style, that's why
checkpatch didn't complain.
But I removed the comment in V4
>> static inline unsigned int sk_filter_len(const struct sk_filter *fp)
>> {
>> - return fp->len * sizeof(struct sock_filter) + sizeof(*fp);
>> + return max(fp->len * sizeof(struct sock_filter),
>> + sizeof(struct work_struct)) + sizeof(*fp);
>
> So, "sizeof(struct work_struct)) + sizeof(*fp)" is a pattern that repeats
> a couple of times. Might make sense to stick that into a helper of its own
> - but in general this open coded overlay allocation method looks a bit
> fragile and not very obvious in isolation.
>
> So it could be done a bit cleaner, using an anonymous union:
>
> /*
> * These two overlay, the work struct is used during workqueue
> * driven teardown, when the instructions are not used anymore:
> */
> union {
> struct sock_filter insns[0];
> struct work_struct work;
> };
>
> And then all the sizeof() calculations become obvious and sk_filter_len()
> could be eliminated - I've marked the conversions in the code further
> below.
Eric made exactly the same comment. Agreed and fixed in V4
>> #else
>> +#include <linux/slab.h>
>
> Inlines in the middle of header files are generally frowned upon.
>
> The standard pattern is to put them at the top, that way it's easier to
> see the dependencies and there's also less .config dependent inclusion,
> which makes header hell cleanup work easier.
Agree. I only followed the style that is already in filter.h 20 lines above.
#ifdef CONFIG_BPF_JIT
#include <stdarg.h>
#include <linux/linkage.h>
#include <linux/printk.h>
as part of the cleanup can move all of them to the top. In the separate commit?
>> struct sk_filter *fp;
>> unsigned int fsize = sizeof(struct sock_filter) * fprog->len;
>> + unsigned int sk_fsize = max_t(u32, fsize, sizeof(struct work_struct))
>> + + sizeof(*fp);
>
> Using the structure definition I suggested, this could be replaced with
> the more obvious:
>
> unsigned int sk_fsize = max(fsize, sizeof(*fp));
with helper function it's even cleaner. Fixed in V4
> A couple of questions/suggestions:
>
> 1)
>
> I took a brief look at arch/x86/net/bpf_jit_comp.c while reviewing this
> patch.
>
> You need to split up bpf_jit_compile(), it's an obscenely large, ~600
> lines long function. We don't do that in modern, maintainable kernel code.
I had the same thought, therefore in my proposed generalization of bpf:
http://patchwork.ozlabs.org/patch/279280/
It is split into two. do_jit() is still a bit large at 400 lines. Can
split it further.
> 3)
>
> It's nice code altogether! Are there any plans to generalize its
> interfaces, to allow arbitrary bytecode to be used by other kernel
> subsystems as well? In particular tracing filters could make use of it,
> but it would also allow safe probe points.
That was exactly the reasons to generalize bpf as I proposed.
"extended BPF is a set of pseudo instructions that stitch kernel provided
data in the form of bpf_context with kernel provided set of functions in a safe
and deterministic way with minimal performance overhead vs native code"
Not sure what 'tracing filters' you have in mind, but I'm happy to try
them with extended bpf model.
imo existing bpf with two registers is too limiting for performance
and argument passing.
That's why going to 10 made it a lot more flexible and generically usable.
Sorry to hijack the thread.
Thanks
Alexei
^ permalink raw reply
* Re: [PATCH v3 net-next] fix unsafe set_memory_rw from softirq
From: Ingo Molnar @ 2013-10-04 18:00 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Heiko Carstens, Eric Dumazet, Paul Mackerras, H. Peter Anvin,
sparclinux, Nicolas Dichtel, linux-s390, Russell King, x86,
James Morris, Ingo Molnar, Alexey Kuznetsov, Paul E. McKenney,
Xi Wang, Matt Evans, Thomas Gleixner, linux-arm-kernel,
Stelian Nirlu, Nicolas Schichan, Hideaki YOSHIFUJI, netdev,
linux-kernel, David S. Miller, Mircea Gherzan, Daniel Borkmann,
Martin Schwidefsky, linux390, linuxppc-dev, Patrick McHardy
In-Reply-To: <CAMEtUuy0mpdUHJjztK31mHjNVahnoZ8CHo2sLLyJmt=p5+gq0w@mail.gmail.com>
* Alexei Starovoitov <ast@plumgrid.com> wrote:
> >> #else
> >> +#include <linux/slab.h>
> >
> > Inlines in the middle of header files are generally frowned upon.
> >
> > The standard pattern is to put them at the top, that way it's easier to
> > see the dependencies and there's also less .config dependent inclusion,
> > which makes header hell cleanup work easier.
>
> Agree. I only followed the style that is already in filter.h 20 lines above.
>
> #ifdef CONFIG_BPF_JIT
> #include <stdarg.h>
> #include <linux/linkage.h>
> #include <linux/printk.h>
>
> as part of the cleanup can move all of them to the top. In the separate commit?
Yeah, sure, that's fine.
> >> struct sk_filter *fp;
> >> unsigned int fsize = sizeof(struct sock_filter) * fprog->len;
> >> + unsigned int sk_fsize = max_t(u32, fsize, sizeof(struct work_struct))
> >> + + sizeof(*fp);
> >
> > Using the structure definition I suggested, this could be replaced with
> > the more obvious:
> >
> > unsigned int sk_fsize = max(fsize, sizeof(*fp));
>
> with helper function it's even cleaner. Fixed in V4
So my thought was that the helper function is perhaps too trivial and
somewhat obscures the allocation pattern, but yeah. Either way is fine
with me.
> > A couple of questions/suggestions:
> >
> > 1)
> >
> > I took a brief look at arch/x86/net/bpf_jit_comp.c while reviewing this
> > patch.
> >
> > You need to split up bpf_jit_compile(), it's an obscenely large, ~600
> > lines long function. We don't do that in modern, maintainable kernel code.
>
> I had the same thought, therefore in my proposed generalization of bpf:
> http://patchwork.ozlabs.org/patch/279280/
> It is split into two. do_jit() is still a bit large at 400 lines. Can
> split it further.
Yeah, I think as long as you split out the loop iterator into a separate
function it gets much better.
The actual instruction generation code within the iterator looks good in a
single chunk - splitting it up further than that might in fact make it
less readable.
> > 3)
> >
> > It's nice code altogether! Are there any plans to generalize its
> > interfaces, to allow arbitrary bytecode to be used by other kernel
> > subsystems as well? In particular tracing filters could make use of
> > it, but it would also allow safe probe points.
>
> That was exactly the reasons to generalize bpf as I proposed.
Ok, cool :-)
For the x86 bits:
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Alex Williamson @ 2013-10-04 18:12 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: agraf@suse.de, Wood Scott-B07421, linux-pci@vger.kernel.org,
joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07191143@039-SN2MPN1-011.039d.mgd.msft.net>
On Fri, 2013-10-04 at 17:23 +0000, Bhushan Bharat-R65777 wrote:
>
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Friday, October 04, 2013 10:43 PM
> > To: Bhushan Bharat-R65777
> > Cc: joro@8bytes.org; benh@kernel.crashing.org; galak@kernel.crashing.org; linux-
> > kernel@vger.kernel.org; linuxppc-dev@lists.ozlabs.org; linux-
> > pci@vger.kernel.org; agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > foundation.org
> > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a device
> >
> > On Fri, 2013-10-04 at 16:47 +0000, Bhushan Bharat-R65777 wrote:
> > >
> > > > -----Original Message-----
> > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > Sent: Friday, October 04, 2013 9:15 PM
> > > > To: Bhushan Bharat-R65777
> > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux- foundation.org
> > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > device
> > > >
> > > > On Fri, 2013-10-04 at 09:54 +0000, Bhushan Bharat-R65777 wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: linux-pci-owner@vger.kernel.org
> > > > > > [mailto:linux-pci-owner@vger.kernel.org]
> > > > > > On Behalf Of Alex Williamson
> > > > > > Sent: Wednesday, September 25, 2013 10:16 PM
> > > > > > To: Bhushan Bharat-R65777
> > > > > > Cc: joro@8bytes.org; benh@kernel.crashing.org;
> > > > > > galak@kernel.crashing.org; linux- kernel@vger.kernel.org;
> > > > > > linuxppc-dev@lists.ozlabs.org; linux- pci@vger.kernel.org;
> > > > > > agraf@suse.de; Wood Scott-B07421; iommu@lists.linux-
> > > > > > foundation.org; Bhushan Bharat-R65777
> > > > > > Subject: Re: [PATCH 2/7] iommu: add api to get iommu_domain of a
> > > > > > device
> > > > > >
> > > > > > On Thu, 2013-09-19 at 12:59 +0530, Bharat Bhushan wrote:
> > > > > > > This api return the iommu domain to which the device is attached.
> > > > > > > The iommu_domain is required for making API calls related to iommu.
> > > > > > > Follow up patches which use this API to know iommu maping.
> > > > > > >
> > > > > > > Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> > > > > > > ---
> > > > > > > drivers/iommu/iommu.c | 10 ++++++++++
> > > > > > > include/linux/iommu.h | 7 +++++++
> > > > > > > 2 files changed, 17 insertions(+), 0 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > > > > > index
> > > > > > > fbe9ca7..6ac5f50 100644
> > > > > > > --- a/drivers/iommu/iommu.c
> > > > > > > +++ b/drivers/iommu/iommu.c
> > > > > > > @@ -696,6 +696,16 @@ void iommu_detach_device(struct
> > > > > > > iommu_domain *domain, struct device *dev) }
> > > > > > > EXPORT_SYMBOL_GPL(iommu_detach_device);
> > > > > > >
> > > > > > > +struct iommu_domain *iommu_get_dev_domain(struct device *dev) {
> > > > > > > + struct iommu_ops *ops = dev->bus->iommu_ops;
> > > > > > > +
> > > > > > > + if (unlikely(ops == NULL || ops->get_dev_iommu_domain == NULL))
> > > > > > > + return NULL;
> > > > > > > +
> > > > > > > + return ops->get_dev_iommu_domain(dev); }
> > > > > > > +EXPORT_SYMBOL_GPL(iommu_get_dev_domain);
> > > > > >
> > > > > > What prevents this from racing iommu_domain_free()? There's no
> > > > > > references acquired, so there's no reason for the caller to
> > > > > > assume the
> > > > pointer is valid.
> > > > >
> > > > > Sorry for late query, somehow this email went into a folder and
> > > > > escaped;
> > > > >
> > > > > Just to be sure, there is not lock at generic "struct
> > > > > iommu_domain", but IP
> > > > specific structure (link FSL domain) linked in iommu_domain->priv
> > > > have a lock, so we need to ensure this race in FSL iommu code (say
> > > > drivers/iommu/fsl_pamu_domain.c), right?
> > > >
> > > > No, it's not sufficient to make sure that your use of the interface
> > > > is race free. The interface itself needs to be designed so that
> > > > it's difficult to use incorrectly.
> > >
> > > So we can define iommu_get_dev_domain()/iommu_put_dev_domain();
> > > iommu_get_dev_domain() will return domain with the lock held, and
> > > iommu_put_dev_domain() will release the lock? And
> > > iommu_get_dev_domain() must always be followed by
> > > iommu_get_dev_domain().
> >
> > What lock? get/put are generally used for reference counting, not locking in
> > the kernel.
> >
> > > > That's not the case here. This is a backdoor to get the iommu
> > > > domain from the iommu driver regardless of who is using it or how.
> > > > The iommu domain is created and managed by vfio, so shouldn't we be
> > > > looking at how to do this through vfio?
> > >
> > > Let me first describe what we are doing here:
> > > During initialization:-
> > > - vfio talks to MSI system to know the MSI-page and size
> > > - vfio then interacts with iommu to map the MSI-page in iommu (IOVA
> > > is decided by userspace and physical address is the MSI-page)
> > > - So the IOVA subwindow mapping is created in iommu and yes VFIO know about
> > this mapping.
> > >
> > > Now do SET_IRQ(MSI/MSIX) ioctl:
> > > - calls pci_enable_msix()/pci_enable_msi_block(): which is supposed to set
> > MSI address/data in device.
> > > - So in current implementation (this patchset) msi-subsystem gets the IOVA
> > from iommu via this defined interface.
> > > - Are you saying that rather than getting this from iommu, we should get this
> > from vfio? What difference does this make?
> >
> > Yes, you just said above that vfio knows the msi to iova mapping, so why go
> > outside of vfio to find it later? The difference is one case you can have a
> > proper reference to data structures to make sure the pointer you get back
> > actually has meaning at the time you're using it vs the code here where you're
> > defining an API that returns a meaningless value
>
> With FSL-PAMU we will always get consistant data from iommu or vfio-data structure.
Great, but you're trying to add a generic API to the IOMMU subsystem
that's difficult to use correctly. The fact that you use it correctly
does not justify the API.
> > because you can't check or
> > enforce that an arbitrary caller is using it correctly.
>
> I am not sure what is arbitrary caller? pdev is known to vfio, so vfio
> will only make pci_enable_msix()/pci_enable_msi_block() for this pdev.
> If anyother code makes then it is some other unexpectedly thing
> happening in system, no?
What's proposed here is a generic IOMMU API. Anybody can call this.
What if the host SCSI driver decides to go get the iommu domain for it's
device (or any other device)? Does that fit your usage model?
> > It's not maintainable.
> > Thanks,
>
> I do not have any issue with this as well, can you also describe the type of API you are envisioning;
> I can think of defining some function in vfio.c/vfio_iommu*.c, make them global and declare then in include/Linux/vfio.h
> And include <Linux/vfio.h> in caller file (arch/powerpc/kernel/msi.c)
Do you really want module dependencies between vfio and your core kernel
MSI setup? Look at the vfio external user interface that we've already
defined. That allows other components of the kernel to get a proper
reference to a vfio group. From there you can work out how to get what
you want. Another alternative is that vfio could register an MSI to
IOVA mapping with architecture code when the mapping is created. The
MSI setup path could then do a lookup in architecture code for the
mapping. You could even store the MSI to IOVA mapping in VFIO and
create an interface where SET_IRQ passes that mapping into setup code.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Ben Hutchings @ 2013-10-04 21:29 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131004082920.GA4536@dhcp-26-207.brq.redhat.com>
On Fri, 2013-10-04 at 10:29 +0200, Alexander Gordeev wrote:
> On Thu, Oct 03, 2013 at 11:49:45PM +0100, Ben Hutchings wrote:
> > On Wed, 2013-10-02 at 12:48 +0200, Alexander Gordeev wrote:
> > > This update converts pci_enable_msix() and pci_enable_msi_block()
> > > interfaces to canonical kernel functions and makes them return a
> > > error code in case of failure or 0 in case of success.
> > [...]
> >
> > I think this is fundamentally flawed: pci_msix_table_size() and
> > pci_get_msi_cap() can only report the limits of the *device* (which the
> > driver usually already knows), whereas MSI allocation can also be
> > constrained due to *global* limits on the number of distinct IRQs.
>
> Even the current implementation by no means addresses it. Although it
> might seem a case for architectures to report the number of IRQs available
> for a driver to retry, in fact they all just fail. The same applies to
> *any* other type of resource involved: irq_desc's, CPU interrupt vector
> space, msi_desc's etc. No platform cares about it and just bails out once
> a constrain met (please correct me if I am wrong here). Given that Linux
> has been doing well even on embedded I think we should not change it.
>
> The only exception to the above is pSeries platform which takes advantage
> of the current design (to implement MSI quota). There are indications we
> can satisfy pSeries requirements, but the design proposed in this RFC
> is not going to change drastically anyway. The start of the discusstion
> is here: https://lkml.org/lkml/2013/9/5/293
All I can see there is that Tejun didn't think that the global limits
and positive return values were implemented by any architecture. But
you have a counter-example, so I'm not sure what your point is.
It has been quite a while since I saw this happen on x86. But I just
checked on a test system running RHEL 5 i386 (Linux 2.6.18). If I ask
for 16 MSI-X vectors on a device that supports 1024, the return value is
8, and indeed I can then successfully allocate 8.
Now that's going quite a way back, and it may be that global limits
aren't a significant problem any more. With the x86_64 build of RHEL 5
on an identical system, I can allocate 16 or even 32, so this is
apparently not a hardware limit in this case.
> > Currently pci_enable_msix() will report a positive value if it fails due
> > to the global limit. Your patch 7 removes that. pci_enable_msi_block()
> > unfortunately doesn't appear to do this.
>
> pci_enable_msi_block() can do more than one MSI only on x86 (with IOMMU),
> but it does not bother to return positive numbers, indeed.
>
> > It seems to me that a more useful interface would take a minimum and
> > maximum number of vectors from the driver. This wouldn't allow the
> > driver to specify that it could only accept, say, any even number within
> > a certain range, but you could still leave the current functions
> > available for any driver that needs that.
>
> Mmmm.. I am not sure I am getting it. Could you please rephrase?
Most drivers seem to either:
(a) require exactly a certain number of MSI vectors, or
(b) require a minimum number of MSI vectors, usually want to allocate
more, and work with any number in between
We can support drivers in both classes by adding new allocation
functions that allow specifying a minimum (required) and maximum
(wanted) number of MSI vectors. Those in class (a) would just specify
the same value for both. These new functions can take account of any
global limit or allocation policy without any further changes to the
drivers that use them.
The few drivers with more specific requirements would still need to
implement the currently recommended loop, using the old allocation
functions.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] powerpc: fix e500 SPE float rounding inexactness detection
From: Joseph S. Myers @ 2013-10-04 22:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-kernel
From: Joseph Myers <joseph@codesourcery.com>
The e500 SPE floating-point emulation code for the rounding modes
rounding to positive or negative infinity (which may not be
implemented in hardware) tries to avoid emulating rounding if the
result was inexact. However, it tests inexactness using the sticky
bit with the cumulative result of previous operations, rather than
with the non-sticky bits relating to the operation that generated the
interrupt. Furthermore, when a vector operation generates the
interrupt, it's possible that only one of the low and high parts is
inexact, and so only that part should have rounding emulated. This
results in incorrect rounding of exact results in these modes when the
sticky bit is set from a previous operation.
(I'm not sure why the rounding interrupts are generated at all when
the result is exact, but empirically the hardware does generate them.)
This patch checks for inexactness using the correct bits of SPEFSCR,
and ensures that rounding only occurs when the relevant part of the
result was actually inexact.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index a73f088..ecdf35d 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -662,7 +680,8 @@ int speround_handler(struct pt_regs *regs)
{
union dw_union fgpr;
int s_lo, s_hi;
- unsigned long speinsn, type, fc;
+ int lo_inexact, hi_inexact;
+ unsigned long speinsn, type, fc, fptype;
if (get_user(speinsn, (unsigned int __user *) regs->nip))
return -EFAULT;
@@ -675,8 +694,12 @@ int speround_handler(struct pt_regs *regs)
__FPU_FPSCR = mfspr(SPRN_SPEFSCR);
pr_debug("speinsn:%08lx spefscr:%08lx\n", speinsn, __FPU_FPSCR);
+ fptype = (speinsn >> 5) & 0x7;
+
/* No need to round if the result is exact */
- if (!(__FPU_FPSCR & FP_EX_INEXACT))
+ lo_inexact = __FPU_FPSCR & (SPEFSCR_FG | SPEFSCR_FX);
+ hi_inexact = __FPU_FPSCR & (SPEFSCR_FGH | SPEFSCR_FXH);
+ if (!(lo_inexact || (hi_inexact && fptype == VCT)))
return 0;
fc = (speinsn >> 21) & 0x1f;
@@ -687,7 +710,7 @@ int speround_handler(struct pt_regs *regs)
pr_debug("round fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]);
- switch ((speinsn >> 5) & 0x7) {
+ switch (fptype) {
/* Since SPE instructions on E500 core can handle round to nearest
* and round toward zero with IEEE-754 complied, we just need
* to handle round toward +Inf and round toward -Inf by software.
@@ -710,11 +733,15 @@ int speround_handler(struct pt_regs *regs)
case VCT:
if (FP_ROUNDMODE == FP_RND_PINF) {
- if (!s_lo) fgpr.wp[1]++; /* Z_low > 0, choose Z1 */
- if (!s_hi) fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */
+ if (lo_inexact && !s_lo)
+ fgpr.wp[1]++; /* Z_low > 0, choose Z1 */
+ if (hi_inexact && !s_hi)
+ fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */
} else { /* round to -Inf */
- if (s_lo) fgpr.wp[1]++; /* Z_low < 0, choose Z2 */
- if (s_hi) fgpr.wp[0]++; /* Z_high < 0, choose Z2 */
+ if (lo_inexact && s_lo)
+ fgpr.wp[1]++; /* Z_low < 0, choose Z2 */
+ if (hi_inexact && s_hi)
+ fgpr.wp[0]++; /* Z_high < 0, choose Z2 */
}
break;
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* Re: [PATCH] kvm: powerpc: book3s: Fix build break for BOOK3S_32
From: Paul Mackerras @ 2013-10-04 23:45 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, Aneesh Kumar K.V, kvm-ppc, kvm
In-Reply-To: <1945837D-825F-4509-9D7C-D839917863A9@suse.de>
On Fri, Oct 04, 2013 at 03:00:11PM +0200, Alexander Graf wrote:
>
> On 04.10.2013, at 14:35, Paul Mackerras wrote:
>
> > On Fri, Oct 04, 2013 at 02:27:02PM +0200, Alexander Graf wrote:
> >>
> >> On 04.10.2013, at 14:23, Alexander Graf wrote:
> >>
> >>>
> >>> On 03.10.2013, at 06:14, Paul Mackerras wrote:
> >>>
> >>>> On Wed, Oct 02, 2013 at 08:08:44PM +0530, Aneesh Kumar K.V wrote:
> >>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> >>>>>
> >>>>> This was introduced by 85a0d845d8bb5df5d2669416212f56cbe1474c6b
> >>>>
> >>>> It's a good idea to give the headline of the commit as well as the ID.
> >>>> I also like to trim the ID to 10 characters or so. So it should look
> >>>> like this:
> >>>>
> >>>> This was introduced by 85a0d845d8 ("KVM: PPC: Book3S PR: Allocate
> >>>> kvm_vcpu structs from kvm_vcpu_cache").
> >>>>
> >>>>> arch/powerpc/kvm/book3s_pr.c: In function 'kvmppc_core_vcpu_create':
> >>>>> arch/powerpc/kvm/book3s_pr.c:1182:30: error: 'struct kvmppc_vcpu_book3s' has no member named 'shadow_vcpu'
> >>>>> make[1]: *** [arch/powerpc/kvm/book3s_pr.o] Error 1
> >>>>>
> >>>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> >>>>
> >>>> Acked-by: Paul Mackerras <paulus@samba.org>
> >>>
> >>> Would you guys mind if I merge this into the offending patch? It's not trickled into -next yet, so rebasing should work.
> >>>
> >>> If not, please resend with the fixed commit message.
> >>
> >> Eh - I must've missed v2 :). So that leaves only the question on whether you'd be ok to squash the patch instead. It'd help bisectability.
> >
> > I'm OK with that. If you do, why don't you squash the first of the
> > two patches that I just sent into the commit it fixes as well?
>
> Because patch 1/2 spans two separate commits it would have to get squashed into (6aa82e, 70afec) and patch 2/2 doesn't make sense to get squashed anywhere :).
Actually 70afec is fine, if you look at it, it's only 6aa82e that
needs fixing.
Paul.
^ permalink raw reply
* [PATCH] powerpc: fix exception clearing in e500 SPE float emulation
From: Joseph S. Myers @ 2013-10-04 22:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-kernel
From: Joseph Myers <joseph@codesourcery.com>
The e500 SPE floating-point emulation code clears existing exceptions
(__FPU_FPSCR &= ~FP_EX_MASK;) before ORing in the exceptions from the
emulated operation. However, these exception bits are the "sticky",
cumulative exception bits, and should only be cleared by the user
program setting SPEFSCR, not implicitly by any floating-point
instruction (whether executed purely by the hardware or emulated).
The spurious clearing of these bits shows up as missing exceptions in
glibc testing.
Fixing this, however, is not as simple as just not clearing the bits,
because while the bits may be from previous floating-point operations
(in which case they should not be cleared), the processor can also set
the sticky bits itself before the interrupt for an exception occurs,
and this can happen in cases when IEEE 754 semantics are that the
sticky bit should not be set. Specifically, the "invalid" sticky bit
is set in various cases with non-finite operands, where IEEE 754
semantics do not involve raising such an exception, and the
"underflow" sticky bit is set in cases of exact underflow, whereas
IEEE 754 semantics are that this flag is set only for inexact
underflow. Thus, for correct emulation the kernel needs to know the
setting of these two sticky bits before the instruction being
emulated.
When a floating-point operation raises an exception, the kernel can
note the state of the sticky bits immediately afterwards. Some
<fenv.h> functions that affect the state of these bits, such as
fesetenv and feholdexcept, need to use prctl with PR_GET_FPEXC and
PR_SET_FPEXC anyway, and so it is natural to record the state of those
bits during that call into the kernel and so avoid any need for a
separate call into the kernel to inform it of a change to those bits.
Thus, the interface I chose to use (in this patch and the glibc port
on which I am working; the old EGLIBC port, based on Aldy's SPE
add-on, has many deficiencies in its <fenv.h> functions
implementation, so that they do not interact correctly with any Linux
kernel version) is that one of those prctl calls must be made after
any userspace change to those sticky bits, other than through a
floating-point operation that traps into the kernel anyway.
feclearexcept and fesetexceptflag duly make those calls, which would
not be required were it not for this issue.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index ce4de5a..0b02e23 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -237,6 +237,8 @@ struct thread_struct {
unsigned long evr[32]; /* upper 32-bits of SPE regs */
u64 acc; /* Accumulator */
unsigned long spefscr; /* SPE & eFP status */
+ unsigned long spefscr_last; /* SPEFSCR value on last prctl
+ call or trap return */
int used_spe; /* set if process has used spe */
#endif /* CONFIG_SPE */
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
@@ -303,7 +305,9 @@ struct thread_struct {
(_ALIGN_UP(sizeof(init_thread_info), 16) + (unsigned long) &init_stack)
#ifdef CONFIG_SPE
-#define SPEFSCR_INIT .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE,
+#define SPEFSCR_INIT \
+ .spefscr = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE, \
+ .spefscr_last = SPEFSCR_FINVE | SPEFSCR_FDBZE | SPEFSCR_FUNFE | SPEFSCR_FOVFE,
#else
#define SPEFSCR_INIT
#endif
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 96d2fdf..e3b91f1 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1151,6 +1151,7 @@ int set_fpexc_mode(struct task_struct *tsk, unsigned int val)
if (val & PR_FP_EXC_SW_ENABLE) {
#ifdef CONFIG_SPE
if (cpu_has_feature(CPU_FTR_SPE)) {
+ tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR);
tsk->thread.fpexc_mode = val &
(PR_FP_EXC_SW_ENABLE | PR_FP_ALL_EXCEPT);
return 0;
@@ -1182,9 +1183,10 @@ int get_fpexc_mode(struct task_struct *tsk, unsigned long adr)
if (tsk->thread.fpexc_mode & PR_FP_EXC_SW_ENABLE)
#ifdef CONFIG_SPE
- if (cpu_has_feature(CPU_FTR_SPE))
+ if (cpu_has_feature(CPU_FTR_SPE)) {
+ tsk->thread.spefscr_last = mfspr(SPRN_SPEFSCR);
val = tsk->thread.fpexc_mode;
- else
+ } else
return -EINVAL;
#else
return -EINVAL;
diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index a73f088..59835c6 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -630,9 +630,27 @@ update_ccr:
regs->ccr |= (IR << ((7 - ((speinsn >> 23) & 0x7)) << 2));
update_regs:
- __FPU_FPSCR &= ~FP_EX_MASK;
+ /*
+ * If the "invalid" exception sticky bit was set by the
+ * processor for non-finite input, but was not set before the
+ * instruction being emulated, clear it. Likewise for the
+ * "underflow" bit, which may have been set by the processor
+ * for exact underflow, not just inexact underflow when the
+ * flag should be set for IEEE 754 semantics. Other sticky
+ * exceptions will only be set by the processor when they are
+ * correct according to IEEE 754 semantics, and we must not
+ * clear sticky bits that were already set before the emulated
+ * instruction as they represent the user-visible sticky
+ * exception status. "inexact" traps to kernel are not
+ * required for IEEE semantics and are not enabled by default,
+ * so the "inexact" sticky bit may have been set by a previous
+ * instruction without the kernel being aware of it.
+ */
+ __FPU_FPSCR
+ &= ~(FP_EX_INVALID | FP_EX_UNDERFLOW) | current->thread.spefscr_last;
__FPU_FPSCR |= (FP_CUR_EXCEPTIONS & FP_EX_MASK);
mtspr(SPRN_SPEFSCR, __FPU_FPSCR);
+ current->thread.spefscr_last = __FPU_FPSCR;
current->thread.evr[fc] = vc.wp[0];
regs->gpr[fc] = vc.wp[1];
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* Re: [PATCH] kvm: powerpc: book3s: Fix build break for BOOK3S_32
From: Alexander Graf @ 2013-10-04 23:59 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Aneesh Kumar K.V, kvm-ppc, kvm
In-Reply-To: <20131004234509.GB2256@iris.ozlabs.ibm.com>
On 05.10.2013, at 01:45, Paul Mackerras wrote:
> On Fri, Oct 04, 2013 at 03:00:11PM +0200, Alexander Graf wrote:
>>=20
>> On 04.10.2013, at 14:35, Paul Mackerras wrote:
>>=20
>>> On Fri, Oct 04, 2013 at 02:27:02PM +0200, Alexander Graf wrote:
>>>>=20
>>>> On 04.10.2013, at 14:23, Alexander Graf wrote:
>>>>=20
>>>>>=20
>>>>> On 03.10.2013, at 06:14, Paul Mackerras wrote:
>>>>>=20
>>>>>> On Wed, Oct 02, 2013 at 08:08:44PM +0530, Aneesh Kumar K.V wrote:
>>>>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>>>>>=20
>>>>>>> This was introduced by 85a0d845d8bb5df5d2669416212f56cbe1474c6b
>>>>>>=20
>>>>>> It's a good idea to give the headline of the commit as well as =
the ID.
>>>>>> I also like to trim the ID to 10 characters or so. So it should =
look
>>>>>> like this:
>>>>>>=20
>>>>>> This was introduced by 85a0d845d8 ("KVM: PPC: Book3S PR: Allocate
>>>>>> kvm_vcpu structs from kvm_vcpu_cache").
>>>>>>=20
>>>>>>> arch/powerpc/kvm/book3s_pr.c: In function =
'kvmppc_core_vcpu_create':
>>>>>>> arch/powerpc/kvm/book3s_pr.c:1182:30: error: 'struct =
kvmppc_vcpu_book3s' has no member named 'shadow_vcpu'
>>>>>>> make[1]: *** [arch/powerpc/kvm/book3s_pr.o] Error 1
>>>>>>>=20
>>>>>>> Signed-off-by: Aneesh Kumar K.V =
<aneesh.kumar@linux.vnet.ibm.com>
>>>>>>=20
>>>>>> Acked-by: Paul Mackerras <paulus@samba.org>
>>>>>=20
>>>>> Would you guys mind if I merge this into the offending patch? It's =
not trickled into -next yet, so rebasing should work.
>>>>>=20
>>>>> If not, please resend with the fixed commit message.
>>>>=20
>>>> Eh - I must've missed v2 :). So that leaves only the question on =
whether you'd be ok to squash the patch instead. It'd help =
bisectability.
>>>=20
>>> I'm OK with that. If you do, why don't you squash the first of the
>>> two patches that I just sent into the commit it fixes as well?
>>=20
>> Because patch 1/2 spans two separate commits it would have to get =
squashed into (6aa82e, 70afec) and patch 2/2 doesn't make sense to get =
squashed anywhere :).
>=20
> Actually 70afec is fine, if you look at it, it's only 6aa82e that
> needs fixing.
True. Squashed them :).
Alex
^ permalink raw reply
* Re: [RFC PATCH] PPC: KVM: vfio kvm device: support spapr tce
From: Alexey Kardashevskiy @ 2013-10-05 1:52 UTC (permalink / raw)
To: Alex Williamson; +Cc: linuxppc-dev, kvm-ppc, kvm, linux-kernel
In-Reply-To: <1380902702.25705.11.camel@ul30vt.home>
On 05.10.2013 2:05, Alex Williamson wrote:
> On Fri, 2013-10-04 at 22:24 +1000, Alexey Kardashevskiy wrote:
>> This is a very rough change set required for ppc64 to use this KVM device.
>>
>> vfio_rm.c is a piece of code which is going to be called from the realmode (MMU off),
>> and I will put everything spapr-related under #ifdef CONFIG_SPAPR_TCE_IOMMU,
>> it is just friday and I have to run :)
>>
>> This is an RFC but it works.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> arch/powerpc/kvm/Kconfig | 1 +
>> arch/powerpc/kvm/Makefile | 4 ++++
>> include/linux/kvm_host.h | 8 ++++---
>> include/linux/vfio.h | 3 +++
>> include/uapi/linux/kvm.h | 1 +
>> virt/kvm/vfio.c | 46 ++++++++++++++++++++++++++++++++++++++++
>> virt/kvm/vfio_rm.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>> 7 files changed, 114 insertions(+), 3 deletions(-)
>> create mode 100644 virt/kvm/vfio_rm.c
>>
>> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
>> index 61b3535..d1b7f64 100644
>> --- a/arch/powerpc/kvm/Kconfig
>> +++ b/arch/powerpc/kvm/Kconfig
>> @@ -60,6 +60,7 @@ config KVM_BOOK3S_64
>> select KVM_BOOK3S_64_HANDLER
>> select KVM
>> select SPAPR_TCE_IOMMU
>> + select KVM_VFIO
>> ---help---
>> Support running unmodified book3s_64 and book3s_32 guest kernels
>> in virtual machines on book3s_64 host processors.
>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
>> index 6646c95..fc2878b 100644
>> --- a/arch/powerpc/kvm/Makefile
>> +++ b/arch/powerpc/kvm/Makefile
>> @@ -55,6 +55,8 @@ kvm-objs-$(CONFIG_KVM_E500MC) := $(kvm-e500mc-objs)
>>
>> kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_PR) := \
>> $(KVM)/coalesced_mmio.o \
>> + $(KVM)/vfio.o \
>> + $(KVM)/vfio_rm.o \
>> fpu.o \
>> book3s_paired_singles.o \
>> book3s_pr.o \
>> @@ -76,6 +78,7 @@ kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>> kvm-book3s_64-builtin-xics-objs-$(CONFIG_KVM_XICS) := \
>> book3s_hv_rm_xics.o
>> kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>> + $(KVM)/vfio_rm.o \
>> book3s_hv_rmhandlers.o \
>> book3s_hv_rm_mmu.o \
>> book3s_64_vio_hv.o \
>> @@ -89,6 +92,7 @@ kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>>
>> kvm-book3s_64-module-objs := \
>> $(KVM)/kvm_main.o \
>> + $(KVM)/vfio.o \
>> $(KVM)/eventfd.o \
>> powerpc.o \
>> emulate.o \
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index ad2b581..43c0290 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -407,6 +407,8 @@ struct kvm {
>> #endif
>> long tlbs_dirty;
>> struct list_head devices;
>> +
>> + struct kvm_vfio *vfio;
>
>
> <cringe> can't this be on kvm->arch?
It can, I just thought since it is valid for more than just one
platform, it can go here.
>> };
>>
>> #define kvm_err(fmt, ...) \
>> @@ -677,15 +679,15 @@ void kvm_arch_register_noncoherent_dma(struct kvm *kvm);
>> void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm);
>> bool kvm_arch_has_noncoherent_dma(struct kvm *kvm);
>> #else
>> -static inline void kvm_arch_register_noncoherent_dma(void)
>> +static inline void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
>> {
>> }
>>
>> -static inline void kvm_arch_unregister_noncoherent_dma(void)
>> +static inline void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm)
>> {
>> }
>>
>> -static inline bool kvm_arch_has_noncoherent_dma(void)
>> +static inline bool kvm_arch_has_noncoherent_dma(struct kvm *kvm)
>> {
>> return false;
>> }
>
> Will fix in my series.
>
>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>> index 24579a0..681e19b 100644
>> --- a/include/linux/vfio.h
>> +++ b/include/linux/vfio.h
>> @@ -97,4 +97,7 @@ extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
>> extern void vfio_group_put_external_user(struct vfio_group *group);
>> extern int vfio_external_user_iommu_id(struct vfio_group *group);
>>
>> +extern struct iommu_group *vfio_find_group_by_liobn(struct kvm *kvm,
>> + unsigned long liobn);
>> +
>
> Wrong header file.
>
>> #endif /* VFIO_H */
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 7c1a349..a74ad16 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -847,6 +847,7 @@ struct kvm_device_attr {
>> #define KVM_DEV_VFIO_GROUP 1
>> #define KVM_DEV_VFIO_GROUP_ADD 1
>> #define KVM_DEV_VFIO_GROUP_DEL 2
>> +#define KVM_DEV_VFIO_SPAPR_TCE_LIOBN 2
>>
>> /*
>> * ioctls for VM fds
>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>> index 2e336a7..39dea9f 100644
>> --- a/virt/kvm/vfio.c
>> +++ b/virt/kvm/vfio.c
>> @@ -22,6 +22,7 @@
>> struct kvm_vfio_group {
>> struct list_head node;
>> struct vfio_group *vfio_group;
>> + uint64_t liobn; /* sPAPR */
>
> Perhaps an arch pointer or at least a union.
>
>> };
>>
>> struct kvm_vfio {
>> @@ -188,12 +189,52 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>> return -ENXIO;
>> }
>>
>> +static int kvm_vfio_set_spapr_tce_liobn(struct kvm_device *dev,
>> + long attr, u64 arg)
>> +{
>> + struct kvm_vfio *kv = dev->private;
>> + struct vfio_group *vfio_group;
>> + struct kvm_vfio_group *kvg;
>> + void __user *argp = (void __user *)arg;
>> + struct fd f;
>> + int32_t fd;
>> + uint64_t liobn = attr;
>> +
>> + if (get_user(fd, (int32_t __user *)argp))
>> + return -EFAULT;
>> +
>> + f = fdget(fd);
>> + if (!f.file)
>> + return -EBADF;
>> +
>> + vfio_group = kvm_vfio_group_get_external_user(f.file);
>> + fdput(f);
>> +
>> + list_for_each_entry(kvg, &kv->group_list, node) {
>> + if (kvg->vfio_group == vfio_group) {
>> + WARN_ON(kvg->liobn);
>
> Users shouldn't be able to trigger WARN_ON so easily, return -EBUSY,
> allow it to be unset and re-set, or just allow the overwrite.
>
>> + kvg->liobn = liobn;
>> + kvm_vfio_group_put_external_user(vfio_group);
>> + return 0;
>> + }
>> + }
>> +
>> + kvm_vfio_group_put_external_user(vfio_group);
>> +
>> + return -ENXIO;
>> +}
>> +
>> static int kvm_vfio_set_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> switch (attr->group) {
>> case KVM_DEV_VFIO_GROUP:
>> return kvm_vfio_set_group(dev, attr->attr, attr->addr);
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + case KVM_DEV_VFIO_SPAPR_TCE_LIOBN:
>> + return kvm_vfio_set_spapr_tce_liobn(dev, attr->attr,
>> + attr->addr);
>> +#endif
>> }
>>
>> return -ENXIO;
>> @@ -211,6 +252,10 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
>> }
>>
>> break;
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + case KVM_DEV_VFIO_SPAPR_TCE_LIOBN:
>> + return 0;
>> +#endif
>> }
>>
>> return -ENXIO;
>> @@ -250,6 +295,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>> mutex_init(&kv->lock);
>>
>> dev->private = kv;
>> + dev->kvm->vfio = kv;
>>
>> return 0;
>> }
>> diff --git a/virt/kvm/vfio_rm.c b/virt/kvm/vfio_rm.c
>> new file mode 100644
>> index 0000000..ee9fd96
>> --- /dev/null
>> +++ b/virt/kvm/vfio_rm.c
>> @@ -0,0 +1,54 @@
>> +#include <linux/errno.h>
>> +#include <linux/file.h>
>> +#include <linux/kvm_host.h>
>> +#include <linux/list.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/vfio.h>
>> +
>> +struct kvm_vfio_group {
>> + struct list_head node;
>> + struct vfio_group *vfio_group;
>> + uint64_t liobn; /* sPAPR */
>> +};
>> +
>> +struct kvm_vfio {
>> + struct list_head group_list;
>> + struct mutex lock;
>> + bool noncoherent;
>> +};
>> +
>> +struct vfio_group {
>> + struct kref kref;
>> + int minor;
>> + atomic_t container_users;
>> + struct iommu_group *iommu_group;
>> + struct vfio_container *container;
>> + struct list_head device_list;
>> + struct mutex device_lock;
>> + struct device *dev;
>> + struct notifier_block nb;
>> + struct list_head vfio_next;
>> + struct list_head container_next;
>> + atomic_t opened;
>> +};
>> +
>> +struct iommu_group *vfio_find_group_by_liobn(struct kvm *kvm, unsigned long liobn)
>> +{
>> + struct kvm_vfio_group *kvg;
>> +
>> + if (!kvm->vfio)
>> + return NULL;
>> +
>> + list_for_each_entry(kvg, &kvm->vfio->group_list, node) {
>> + if (kvg->liobn == liobn)
>> + return kvg->vfio_group->iommu_group;
>> + }
>> +
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_find_group_by_liobn);
>> +
>> +
>
> You're kidding, right? These are intentionally private data structures
> that are blatantly copied so that you can extract what you want. NACK.
> The iommu_group is available off struct device, do you even need vfio or
> this kvm-vfio device to get from liobn to iommu_group? Thanks,
This is an RFC. I am not saying this is what can go to upstream or
anything. I am not kidding (why everyone assumes that?), I am showing
what API I would like to have in the VFIO KVM device. I need the way to
get iommu_table (which is in a private data of iommu_group) by LIOBN and
the VFIO KVM device is the _only_ entity which will know about this
connection (LIOBN is made up by the qemu and told to the guest) and it
cannot go to the kvm.ko - and the patch like this is the best way to
show it as my english obviously sucks.
--
With best regards
Alexey Kardashevskiy -- icq: 52150396
^ permalink raw reply
* Re: [RFC PATCH] PPC: KVM: vfio kvm device: support spapr tce
From: Alexey Kardashevskiy @ 2013-10-05 3:36 UTC (permalink / raw)
To: Alex Williamson; +Cc: linuxppc-dev, kvm-ppc, kvm, linux-kernel
In-Reply-To: <524F70D0.4050502@ozlabs.ru>
On 10/05/2013 11:52 AM, Alexey Kardashevskiy wrote:
> On 05.10.2013 2:05, Alex Williamson wrote:
>> On Fri, 2013-10-04 at 22:24 +1000, Alexey Kardashevskiy wrote:
>>> This is a very rough change set required for ppc64 to use this KVM device.
>>>
>>> vfio_rm.c is a piece of code which is going to be called from the realmode (MMU off),
>>> and I will put everything spapr-related under #ifdef CONFIG_SPAPR_TCE_IOMMU,
>>> it is just friday and I have to run :)
>>>
>>> This is an RFC but it works.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> arch/powerpc/kvm/Kconfig | 1 +
>>> arch/powerpc/kvm/Makefile | 4 ++++
>>> include/linux/kvm_host.h | 8 ++++---
>>> include/linux/vfio.h | 3 +++
>>> include/uapi/linux/kvm.h | 1 +
>>> virt/kvm/vfio.c | 46 ++++++++++++++++++++++++++++++++++++++++
>>> virt/kvm/vfio_rm.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
>>> 7 files changed, 114 insertions(+), 3 deletions(-)
>>> create mode 100644 virt/kvm/vfio_rm.c
>>>
>>> diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
>>> index 61b3535..d1b7f64 100644
>>> --- a/arch/powerpc/kvm/Kconfig
>>> +++ b/arch/powerpc/kvm/Kconfig
>>> @@ -60,6 +60,7 @@ config KVM_BOOK3S_64
>>> select KVM_BOOK3S_64_HANDLER
>>> select KVM
>>> select SPAPR_TCE_IOMMU
>>> + select KVM_VFIO
>>> ---help---
>>> Support running unmodified book3s_64 and book3s_32 guest kernels
>>> in virtual machines on book3s_64 host processors.
>>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
>>> index 6646c95..fc2878b 100644
>>> --- a/arch/powerpc/kvm/Makefile
>>> +++ b/arch/powerpc/kvm/Makefile
>>> @@ -55,6 +55,8 @@ kvm-objs-$(CONFIG_KVM_E500MC) := $(kvm-e500mc-objs)
>>>
>>> kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_PR) := \
>>> $(KVM)/coalesced_mmio.o \
>>> + $(KVM)/vfio.o \
>>> + $(KVM)/vfio_rm.o \
>>> fpu.o \
>>> book3s_paired_singles.o \
>>> book3s_pr.o \
>>> @@ -76,6 +78,7 @@ kvm-book3s_64-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>>> kvm-book3s_64-builtin-xics-objs-$(CONFIG_KVM_XICS) := \
>>> book3s_hv_rm_xics.o
>>> kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>>> + $(KVM)/vfio_rm.o \
>>> book3s_hv_rmhandlers.o \
>>> book3s_hv_rm_mmu.o \
>>> book3s_64_vio_hv.o \
>>> @@ -89,6 +92,7 @@ kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>>>
>>> kvm-book3s_64-module-objs := \
>>> $(KVM)/kvm_main.o \
>>> + $(KVM)/vfio.o \
>>> $(KVM)/eventfd.o \
>>> powerpc.o \
>>> emulate.o \
>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>> index ad2b581..43c0290 100644
>>> --- a/include/linux/kvm_host.h
>>> +++ b/include/linux/kvm_host.h
>>> @@ -407,6 +407,8 @@ struct kvm {
>>> #endif
>>> long tlbs_dirty;
>>> struct list_head devices;
>>> +
>>> + struct kvm_vfio *vfio;
>>
>>
>> <cringe> can't this be on kvm->arch?
>
> It can, I just thought since it is valid for more than just one
> platform, it can go here.
>
>
>>> };
>>>
>>> #define kvm_err(fmt, ...) \
>>> @@ -677,15 +679,15 @@ void kvm_arch_register_noncoherent_dma(struct kvm *kvm);
>>> void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm);
>>> bool kvm_arch_has_noncoherent_dma(struct kvm *kvm);
>>> #else
>>> -static inline void kvm_arch_register_noncoherent_dma(void)
>>> +static inline void kvm_arch_register_noncoherent_dma(struct kvm *kvm)
>>> {
>>> }
>>>
>>> -static inline void kvm_arch_unregister_noncoherent_dma(void)
>>> +static inline void kvm_arch_unregister_noncoherent_dma(struct kvm *kvm)
>>> {
>>> }
>>>
>>> -static inline bool kvm_arch_has_noncoherent_dma(void)
>>> +static inline bool kvm_arch_has_noncoherent_dma(struct kvm *kvm)
>>> {
>>> return false;
>>> }
>>
>> Will fix in my series.
>>
>>> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>>> index 24579a0..681e19b 100644
>>> --- a/include/linux/vfio.h
>>> +++ b/include/linux/vfio.h
>>> @@ -97,4 +97,7 @@ extern struct vfio_group *vfio_group_get_external_user(struct file *filep);
>>> extern void vfio_group_put_external_user(struct vfio_group *group);
>>> extern int vfio_external_user_iommu_id(struct vfio_group *group);
>>>
>>> +extern struct iommu_group *vfio_find_group_by_liobn(struct kvm *kvm,
>>> + unsigned long liobn);
>>> +
>>
>> Wrong header file.
btw there are two - uapi/linux/vfio.h and linux/vfio.h. The external user
API is in linux/vfio.h but my change should go to uapi/linux/vfio.h, is
that correct? Or we need a third header? Just asking, no kidding, no arguing :)
>>
>>> #endif /* VFIO_H */
>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>> index 7c1a349..a74ad16 100644
>>> --- a/include/uapi/linux/kvm.h
>>> +++ b/include/uapi/linux/kvm.h
>>> @@ -847,6 +847,7 @@ struct kvm_device_attr {
>>> #define KVM_DEV_VFIO_GROUP 1
>>> #define KVM_DEV_VFIO_GROUP_ADD 1
>>> #define KVM_DEV_VFIO_GROUP_DEL 2
>>> +#define KVM_DEV_VFIO_SPAPR_TCE_LIOBN 2
>>>
>>> /*
>>> * ioctls for VM fds
>>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>>> index 2e336a7..39dea9f 100644
>>> --- a/virt/kvm/vfio.c
>>> +++ b/virt/kvm/vfio.c
>>> @@ -22,6 +22,7 @@
>>> struct kvm_vfio_group {
>>> struct list_head node;
>>> struct vfio_group *vfio_group;
>>> + uint64_t liobn; /* sPAPR */
>>
>> Perhaps an arch pointer or at least a union.
>>
>>> };
>>>
>>> struct kvm_vfio {
>>> @@ -188,12 +189,52 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>>> return -ENXIO;
>>> }
>>>
>>> +static int kvm_vfio_set_spapr_tce_liobn(struct kvm_device *dev,
>>> + long attr, u64 arg)
>>> +{
>>> + struct kvm_vfio *kv = dev->private;
>>> + struct vfio_group *vfio_group;
>>> + struct kvm_vfio_group *kvg;
>>> + void __user *argp = (void __user *)arg;
>>> + struct fd f;
>>> + int32_t fd;
>>> + uint64_t liobn = attr;
>>> +
>>> + if (get_user(fd, (int32_t __user *)argp))
>>> + return -EFAULT;
>>> +
>>> + f = fdget(fd);
>>> + if (!f.file)
>>> + return -EBADF;
>>> +
>>> + vfio_group = kvm_vfio_group_get_external_user(f.file);
>>> + fdput(f);
>>> +
>>> + list_for_each_entry(kvg, &kv->group_list, node) {
>>> + if (kvg->vfio_group == vfio_group) {
>>> + WARN_ON(kvg->liobn);
>>
>> Users shouldn't be able to trigger WARN_ON so easily, return -EBUSY,
>> allow it to be unset and re-set, or just allow the overwrite.
>>
>>> + kvg->liobn = liobn;
>>> + kvm_vfio_group_put_external_user(vfio_group);
>>> + return 0;
>>> + }
>>> + }
>>> +
>>> + kvm_vfio_group_put_external_user(vfio_group);
>>> +
>>> + return -ENXIO;
>>> +}
>>> +
>>> static int kvm_vfio_set_attr(struct kvm_device *dev,
>>> struct kvm_device_attr *attr)
>>> {
>>> switch (attr->group) {
>>> case KVM_DEV_VFIO_GROUP:
>>> return kvm_vfio_set_group(dev, attr->attr, attr->addr);
>>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>>> + case KVM_DEV_VFIO_SPAPR_TCE_LIOBN:
>>> + return kvm_vfio_set_spapr_tce_liobn(dev, attr->attr,
>>> + attr->addr);
>>> +#endif
>>> }
>>>
>>> return -ENXIO;
>>> @@ -211,6 +252,10 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
>>> }
>>>
>>> break;
>>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>>> + case KVM_DEV_VFIO_SPAPR_TCE_LIOBN:
>>> + return 0;
>>> +#endif
>>> }
>>>
>>> return -ENXIO;
>>> @@ -250,6 +295,7 @@ static int kvm_vfio_create(struct kvm_device *dev, u32 type)
>>> mutex_init(&kv->lock);
>>>
>>> dev->private = kv;
>>> + dev->kvm->vfio = kv;
>>>
>>> return 0;
>>> }
>>> diff --git a/virt/kvm/vfio_rm.c b/virt/kvm/vfio_rm.c
>>> new file mode 100644
>>> index 0000000..ee9fd96
>>> --- /dev/null
>>> +++ b/virt/kvm/vfio_rm.c
>>> @@ -0,0 +1,54 @@
>>> +#include <linux/errno.h>
>>> +#include <linux/file.h>
>>> +#include <linux/kvm_host.h>
>>> +#include <linux/list.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/uaccess.h>
>>> +#include <linux/vfio.h>
>>> +
>>> +struct kvm_vfio_group {
>>> + struct list_head node;
>>> + struct vfio_group *vfio_group;
>>> + uint64_t liobn; /* sPAPR */
>>> +};
>>> +
>>> +struct kvm_vfio {
>>> + struct list_head group_list;
>>> + struct mutex lock;
>>> + bool noncoherent;
>>> +};
>>> +
>>> +struct vfio_group {
>>> + struct kref kref;
>>> + int minor;
>>> + atomic_t container_users;
>>> + struct iommu_group *iommu_group;
>>> + struct vfio_container *container;
>>> + struct list_head device_list;
>>> + struct mutex device_lock;
>>> + struct device *dev;
>>> + struct notifier_block nb;
>>> + struct list_head vfio_next;
>>> + struct list_head container_next;
>>> + atomic_t opened;
>>> +};
>>> +
>>> +struct iommu_group *vfio_find_group_by_liobn(struct kvm *kvm, unsigned long liobn)
>>> +{
>>> + struct kvm_vfio_group *kvg;
>>> +
>>> + if (!kvm->vfio)
>>> + return NULL;
>>> +
>>> + list_for_each_entry(kvg, &kvm->vfio->group_list, node) {
>>> + if (kvg->liobn == liobn)
>>> + return kvg->vfio_group->iommu_group;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(vfio_find_group_by_liobn);
>>> +
>>> +
>>
>> You're kidding, right? These are intentionally private data structures
>> that are blatantly copied so that you can extract what you want. NACK.
>> The iommu_group is available off struct device, do you even need vfio or
>> this kvm-vfio device to get from liobn to iommu_group? Thanks,
>
>
> This is an RFC. I am not saying this is what can go to upstream or
> anything. I am not kidding (why everyone assumes that?), I am showing
> what API I would like to have in the VFIO KVM device. I need the way to
> get iommu_table (which is in a private data of iommu_group) by LIOBN and
> the VFIO KVM device is the _only_ entity which will know about this
> connection (LIOBN is made up by the qemu and told to the guest) and it
> cannot go to the kvm.ko - and the patch like this is the best way to
> show it as my english obviously sucks.
Oh. I was confused by:
drivers/vfio/vfio.c|67| struct vfio_group {
drivers/vfio/vfio_iommu_type1.c|73| struct vfio_group {
which are two completely different types (confusing).
So. I either need an additional file to compile to the kernel for mmu-off
case (such as vfio_rm.c) and share vfio_group struct via some internal
header or compile vfio.c into the kernel image always, in this case I'll
need to export kvm_vfio_ops symbol. What would you suggest?
--
Alexey
^ permalink raw reply
* Re: [PATCH 6/9][v5] powerpc/perf: Define big-endian version of perf_mem_data_src
From: Sukadev Bhattiprolu @ 2013-10-05 3:53 UTC (permalink / raw)
To: Michael Ellerman
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Arnaldo Carvalho de Melo, Anshuman Khandual
In-Reply-To: <20131003053944.GD17237@concordia>
Michael Ellerman [michael@ellerman.id.au] wrote:
| On Tue, Oct 01, 2013 at 05:15:07PM -0700, Sukadev Bhattiprolu wrote:
| > perf_mem_data_src is an union that is initialized via the ->val field
| > and accessed via the bitmap fields. For this to work on big endian
| > platforms, we also need a big-endian represenation of perf_mem_data_src.
| >
| > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
| > index ca1d90b..846f399 100644
| > --- a/include/uapi/linux/perf_event.h
| > +++ b/include/uapi/linux/perf_event.h
| > @@ -19,6 +19,50 @@
| > #include <asm/byteorder.h>
| >
| > /*
| > + * Kernel and userspace check for endianness in incompatible ways.
| > + * In user space, <endian.h> defines both __BIG_ENDIAN and __LITTLE_ENDIAN
| > + * but sets __BYTE_ORDER to one or the other. So user space uses checks are:
|
|
| Why can't you use __BIG_ENDIAN_BITFIELD ?
So, the perf tool overrides the <asm/byteorder.h> with a local version.
And since this local version is arch neutral, we can't excplicitly include
the endian headers like <asm/byteorder.h> does.
How about we do something like this (both kernel and tool seem to build
on both x86 and power).
Sukadev.
---
include/uapi/linux/perf_event.h | 16 ++++++++++++++++
tools/perf/util/include/asm/byteorder.h | 27 +++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index ca1d90b..dcfa74f 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -695,6 +695,7 @@ enum perf_callchain_context {
#define PERF_FLAG_FD_OUTPUT (1U << 1)
#define PERF_FLAG_PID_CGROUP (1U << 2) /* pid=cgroup id, per-cpu mode only */
+#if defined (__LITTLE_ENDIAN_BITFIELD)
union perf_mem_data_src {
__u64 val;
struct {
@@ -706,6 +707,21 @@ union perf_mem_data_src {
mem_rsvd:31;
};
};
+#elif defined(__BIG_ENDIAN_BITFIELD)
+union perf_mem_data_src {
+ __u64 val;
+ struct {
+ __u64 mem_rsvd:31,
+ mem_dtlb:7, /* tlb access */
+ mem_lock:2, /* lock instr */
+ mem_snoop:5, /* snoop mode */
+ mem_lvl:14, /* memory hierarchy level */
+ mem_op:5; /* type of opcode */
+ };
+};
+#else
+#error "Unknown endianness"
+#endif
/* type of opcode (load/store/prefetch,code) */
#define PERF_MEM_OP_NA 0x01 /* not available */
diff --git a/tools/perf/util/include/asm/byteorder.h b/tools/perf/util/include/asm/byteorder.h
index 2a9bdc0..521a382 100644
--- a/tools/perf/util/include/asm/byteorder.h
+++ b/tools/perf/util/include/asm/byteorder.h
@@ -1,2 +1,29 @@
#include <asm/types.h>
#include "../../../../include/uapi/linux/swab.h"
+#include <endian.h>
+
+/*
+ * __LITTLE_ENDIAN_BITFIELD and __BIG_ENDIAN_BITFIELD are normally picked
+ * from <byteorder.h>. Since we override the default <byteorder.h>, define
+ * them explicitly here
+ */
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+#ifndef __LITTLE_ENDIAN_BITFIELD
+#define __LITTLE_ENDIAN_BITFIELD
+#endif
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+#ifndef __BIG_ENDIAN_BITFIELD
+#define __BIG_ENDIAN_BITFIELD
+#endif
+
+#else
+#error "Unknown endianness"
+#endif
+
+#if defined(__LITTLE_ENDIAN_BITFIELD) && defined(__BIG_ENDIAN_BITFIELD)
+#error Both __LITTLE_ENDIAN_BITFIELD and __BIG_ENDIAN_BITFIELD defined! \
+ Some perf data structures (eg: perf_mem_data_src) will be wrong.
+#endif
--
1.7.1
^ permalink raw reply related
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-05 14:20 UTC (permalink / raw)
To: Ben Hutchings
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <1380922156.3214.49.camel@bwh-desktop.uk.level5networks.com>
On Fri, Oct 04, 2013 at 10:29:16PM +0100, Ben Hutchings wrote:
> On Fri, 2013-10-04 at 10:29 +0200, Alexander Gordeev wrote:
> All I can see there is that Tejun didn't think that the global limits
> and positive return values were implemented by any architecture.
I would say more than just that :) I picked few quotes which in my
reading represent the guys positions:
Tejun Heo: "...do we really
care about possible partial success? This sort of interface is
unnecessarily complex and actively harmful. It forces all users to
wonder what could possibly happen and implement all sorts of nutty
fallback logic which is highly likely to be error-prone on both the
software and hardware side. Seriously, how much testing would such
code path would get both on the driver and firmware sides?"
Bjorn Helgaas: "I agree, that would be much simpler.
I propose that you rework it that way, and at least find out what
(if anything) would break if we do that."
Michael Ellerman: "I really think you're overstating the complexity here.
Functions typically return a boolean -> nothing to see here
This function returns a tristate value -> brain explosion!";
"All a lot of bother for no real gain IMHO."
> But you have a counter-example, so I'm not sure what your point is.
I concur with Tejun. I think we need to get rid of the loop.
As of the counter-example I think we could honour the pSeries quota by
inroducing an interface to interrogate what you call global limits -
pci_get_msix_limit(), i.e.:
rc = pci_msix_table_size(pdev, nvec);
if (rc < 0)
return rc;
nvec = min(rc, nvec);
rc = pci_get_msix_limit(pdev, nvec);
if (rc < 0)
return rc;
nvec = min(rc, nvec);
for (i = 0; i < nvec; i++)
msix_entry[i].entry = i;
rc = pci_enable_msix(pdev, msix_entry, nvec);
if (rc)
return rc;
The latest state of those discussion is somewhere around Michael's:
"We could probably make that work." and Tejun's: "Are we talking about
some limited number of device drivers here? Also, is the quota still
necessary for machines in production today?"
So my point is - drivers should first obtain a number of MSIs they *can*
get, then *derive* a number of MSIs the device is fine with and only then
request that number. Not terribly different from memory or any other type
of resource allocation ;)
> It has been quite a while since I saw this happen on x86. But I just
> checked on a test system running RHEL 5 i386 (Linux 2.6.18). If I ask
> for 16 MSI-X vectors on a device that supports 1024, the return value is
> 8, and indeed I can then successfully allocate 8.
>
> Now that's going quite a way back, and it may be that global limits
> aren't a significant problem any more. With the x86_64 build of RHEL 5
> on an identical system, I can allocate 16 or even 32, so this is
> apparently not a hardware limit in this case.
Well, I do not know how to comment here. 2.6.18 has a significantly
different codebase wrt MSIs. What about a recent version?
> Most drivers seem to either:
> (a) require exactly a certain number of MSI vectors, or
> (b) require a minimum number of MSI vectors, usually want to allocate
> more, and work with any number in between
>
> We can support drivers in both classes by adding new allocation
> functions that allow specifying a minimum (required) and maximum
> (wanted) number of MSI vectors. Those in class (a) would just specify
> the same value for both. These new functions can take account of any
> global limit or allocation policy without any further changes to the
> drivers that use them.
I think such interface is redundant wrt the current pci_enable_msix()
implementation.. and you propose to leave it. IMO it unnecessarily blows
the generic MSI API with no demand from drivers.
> The few drivers with more specific requirements would still need to
> implement the currently recommended loop, using the old allocation
> functions.
Although the classes of drivers you specified indeed exist, I do believe
just a single interface is enough to handle them all.
> Ben.
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH RFC 54/77] ntb: Ensure number of MSIs on SNB is enough for the link interrupt
From: Alexander Gordeev @ 2013-10-05 21:43 UTC (permalink / raw)
To: Jon Mason
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, stable,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar,
linux-pci, iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Solarflare linux maintainers, netdev, linux-kernel,
Ralf Baechle, e1000-devel, Martin Schwidefsky, linux390,
linuxppc-dev
In-Reply-To: <20131003004805.GL6768@jonmason-lab>
On Wed, Oct 02, 2013 at 05:48:05PM -0700, Jon Mason wrote:
> On Wed, Oct 02, 2013 at 12:49:10PM +0200, Alexander Gordeev wrote:
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > ---
> > drivers/ntb/ntb_hw.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
> > index de2062c..eccd5e5 100644
> > --- a/drivers/ntb/ntb_hw.c
> > +++ b/drivers/ntb/ntb_hw.c
> > @@ -1066,7 +1066,7 @@ static int ntb_setup_msix(struct ntb_device *ndev)
> > /* On SNB, the link interrupt is always tied to 4th vector. If
> > * we can't get all 4, then we can't use MSI-X.
> > */
> > - if (ndev->hw_type != BWD_HW) {
> > + if ((rc < SNB_MSIX_CNT) && (ndev->hw_type != BWD_HW)) {
>
> Nack, this check is unnecessary.
If SNB can do more than SNB_MSIX_CNT MSI-Xs then this check is needed
to enable less than maximum MSI-Xs in case the maximum was not allocated.
Otherwise SNB will fallback to single MSI instead of multiple MSI-Xs.
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Benjamin Herrenschmidt @ 2013-10-05 21:46 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ben Hutchings,
Ingo Molnar, iss_storagedev, linux-driver, Martin Schwidefsky,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Tejun Heo, linux390, linuxppc-dev
In-Reply-To: <20131005142054.GA11270@dhcp-26-207.brq.redhat.com>
On Sat, 2013-10-05 at 16:20 +0200, Alexander Gordeev wrote:
> So my point is - drivers should first obtain a number of MSIs they *can*
> get, then *derive* a number of MSIs the device is fine with and only then
> request that number. Not terribly different from memory or any other type
> of resource allocation ;)
What if the limit is for a group of devices ? Your interface is racy in
that case, another driver could have eaten into the limit in between the
calls.
Ben.
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-06 6:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ben Hutchings,
Ingo Molnar, iss_storagedev, linux-driver, Martin Schwidefsky,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Tejun Heo, linux390, linuxppc-dev
In-Reply-To: <1381009586.645.141.camel@pasglop>
On Sun, Oct 06, 2013 at 08:46:26AM +1100, Benjamin Herrenschmidt wrote:
> On Sat, 2013-10-05 at 16:20 +0200, Alexander Gordeev wrote:
> > So my point is - drivers should first obtain a number of MSIs they *can*
> > get, then *derive* a number of MSIs the device is fine with and only then
> > request that number. Not terribly different from memory or any other type
> > of resource allocation ;)
>
> What if the limit is for a group of devices ? Your interface is racy in
> that case, another driver could have eaten into the limit in between the
> calls.
Well, the another driver has had a better karma ;) But seriously, the
current scheme with a loop is not race-safe wrt to any other type of
resource which might exhaust. What makes the quota so special so we
should care about it and should not care i.e. about lack of msi_desc's?
Yeah, I know the quota might hit more likely. But why it is not addressed
right now then? Not a single function in chains...
rtas_msi_check_device() -> msi_quota_for_device() -> traverse_pci_devices()
rtas_setup_msi_irqs() -> msi_quota_for_device() -> traverse_pci_devices()
...is race-safe. So if it has not been bothering anyone until now then
no reason to start worrying now :)
In fact, in the current design to address the quota race decently the
drivers would have to protect the *loop* to prevent the quota change
between a pci_enable_msix() returned a positive number and the the next
call to pci_enable_msix() with that number. Is it doable?
> Ben.
>
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Benjamin Herrenschmidt @ 2013-10-06 6:19 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ben Hutchings,
Ingo Molnar, iss_storagedev, linux-driver, Martin Schwidefsky,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Tejun Heo, linux390, linuxppc-dev
In-Reply-To: <20131006060243.GB28142@dhcp-26-207.brq.redhat.com>
On Sun, 2013-10-06 at 08:02 +0200, Alexander Gordeev wrote:
> On Sun, Oct 06, 2013 at 08:46:26AM +1100, Benjamin Herrenschmidt wrote:
> > On Sat, 2013-10-05 at 16:20 +0200, Alexander Gordeev wrote:
> > > So my point is - drivers should first obtain a number of MSIs they *can*
> > > get, then *derive* a number of MSIs the device is fine with and only then
> > > request that number. Not terribly different from memory or any other type
> > > of resource allocation ;)
> >
> > What if the limit is for a group of devices ? Your interface is racy in
> > that case, another driver could have eaten into the limit in between the
> > calls.
>
> Well, the another driver has had a better karma ;) But seriously, the
> current scheme with a loop is not race-safe wrt to any other type of
> resource which might exhaust. What makes the quota so special so we
> should care about it and should not care i.e. about lack of msi_desc's?
I'm not saying the current scheme is better but I prefer the option of
passing a min,max to the request function.
> Yeah, I know the quota might hit more likely. But why it is not addressed
> right now then? Not a single function in chains...
> rtas_msi_check_device() -> msi_quota_for_device() -> traverse_pci_devices()
> rtas_setup_msi_irqs() -> msi_quota_for_device() -> traverse_pci_devices()
> ...is race-safe. So if it has not been bothering anyone until now then
> no reason to start worrying now :)
>
> In fact, in the current design to address the quota race decently the
> drivers would have to protect the *loop* to prevent the quota change
> between a pci_enable_msix() returned a positive number and the the next
> call to pci_enable_msix() with that number. Is it doable?
I am not advocating for the current design, simply saying that your
proposal doesn't address this issue while Ben's does.
Cheers,
Ben.
> > Ben.
> >
> >
>
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-06 7:10 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ben Hutchings,
Ingo Molnar, iss_storagedev, linux-driver, Martin Schwidefsky,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Tejun Heo, linux390, linuxppc-dev
In-Reply-To: <1381040386.645.143.camel@pasglop>
On Sun, Oct 06, 2013 at 05:19:46PM +1100, Benjamin Herrenschmidt wrote:
> On Sun, 2013-10-06 at 08:02 +0200, Alexander Gordeev wrote:
> > In fact, in the current design to address the quota race decently the
> > drivers would have to protect the *loop* to prevent the quota change
> > between a pci_enable_msix() returned a positive number and the the next
> > call to pci_enable_msix() with that number. Is it doable?
>
> I am not advocating for the current design, simply saying that your
> proposal doesn't address this issue while Ben's does.
There is one major flaw in min-max approach - the generic MSI layer
will have to take decisions on exact number of MSIs to request, not
device drivers.
This will never work for all devices, because there might be specific
requirements which are not covered by the min-max. That is what Ben
described "...say, any even number within a certain range". Ben suggests
to leave the existing loop scheme to cover such devices, which I think is
not right.
What about introducing pci_lock_msi() and pci_unlock_msi() and let device
drivers care about their ranges and specifics in race-safe manner?
I do not call to introduce it right now (since it appears pSeries has not
been hitting the race for years) just as a possible alternative to Ben's
proposal.
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup
From: Gerhard Sittig @ 2013-10-06 10:01 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss,
Dan Williams, Anatolij Gustschin, linuxppc-dev
In-Reply-To: <CAF0T0X6NE=wFYMzVEm4F9CQWXN-Bu201Z8duEH9kevOhB53jCg@mail.gmail.com>
On Thu, Oct 03, 2013 at 18:00 +0400, Alexander Popov wrote:
>
> v2013/7/14 Gerhard Sittig <gsi@denx.de>:
> > this series
> > - introduces slave s/g support (that's support for DMA transfers which
> > involve peripherals in contrast to mem-to-mem transfers)
> > - adds device tree based lookup support for DMA channels
> > - combines floating patches and related feedback which already covered
> > several aspects of what the suggested LPB driver needs, to demonstrate
> > how integration might be done
> > - carries Q&D SD card support to enable another DMA client during test,
> > while this patch needs to get dropped upon pickup
> >
> > changes since v1:
> > - re-order mpc8308 related code paths for improved readability, no
> > change in behaviour, introduction of symbolic channel names here
> > already
> > - squash 'execute() start condition' and 'terminate all' into the
> > introduction of 'slave s/g prep' and 'device control' support; refuse
> > s/g lists with more than one item since slave support is operational
> > yet proper s/g support is missing (can get addressed later)
> > - always start transfers from software on MPC8308 as there are no
> > external request lines for peripheral flow control
> > - drop dt-bindings header file and symbolic channel names in OF nodes
>
> Changes since v2 (RFC v3 was badly formed, excuse me for that):
> Part 1/5:
> - use #define instead of enum since individual channels don't require
> special handling.
> Part 2/5:
> - add a flag "will_access_peripheral" to DMA transfer descriptor
> according recommendations of Gerhard Sittig.
> This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg()
> and is evaluated in mpc_dma_execute() to choose a type of start for
> the transfer.
> - prevent descriptors of transfers which involve peripherals from
> being chained together;
> each of such transfers needs hardware initiated start.
> - add locking while working with struct mpc_dma_chan
> according recommendations of Lars-Peter Clausen.
> - remove default nbytes value. Client kernel modules must set
> src_maxburst and dst_maxburst fields of struct dma_slave_config (dmaengine.h).
> Part 6/8:
> unchanged.
> Part 7/8:
> unchanged.
> Part 8/8:
> unchanged.
>
> These changes are tested on MPC5125
> - with SCLPC driver (transfers between dev and mem work fine).
> - with dmatest module (all 64 DMA channels can perform mem-to-mem transfers
> which can be chained in one DMA transaction).
>
> > known issues:
> > - it's yet to get confirmed whether MPC8308 can use slave support or
> > whether the DMA controller's driver shall actively reject it, the
> > information that's available so far suggests that peripheral transfers
> > to IP bus attached I/O is useful and shall not get blocked right away
I'm not certain whether keeping the "cover letter" in threaded
form is appropriate. But speaking about non-existent parts
(6-8/8), not saying what the current version is, missing stats
certainly isn't right.
Your submission style adds more work to doing review and
providing feedback than what's necessary. You assume that others
would keep the history for you, or would do the archeology and
collect individual pieces from the past, to recover what you fail
to send out.
The messages show up in an unexpected order here (3, 4, 5, 0
first on one thread, 1, 2 then in another thread, with some 2800
messages between them) and carry conflicting subjects or version
numbers. Formatting was broken in transport, patches won't apply.
Please do consider checking what you send out, and how you can
improve the reception's side of the process. Try to help those
people you want to receive help from. Try to support both kinds
of review for people coming new to the subject as well as those
who have seen a former version.
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ permalink raw reply
* Re: [PATCH RFC v4 2/5] dma: mpc512x: add support for peripheral transfers
From: Gerhard Sittig @ 2013-10-06 11:10 UTC (permalink / raw)
To: Alexander Popov
Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss,
Dan Williams, Anatolij Gustschin, linuxppc-dev
In-Reply-To: <1380809202-16192-1-git-send-email-a13xp0p0v88@gmail.com>
On Thu, Oct 03, 2013 at 18:06 +0400, Alexander Popov wrote:
>
> Introduce support for slave s/g transfer preparation and the associated
> device control callback in the MPC512x DMA controller driver, which adds
> support for data transfers between memory and peripheral I/O to the
> previously supported mem-to-mem transfers.
>
> Refuse to prepare chunked transfers (transfers with more than one part)
> as long as proper support for scatter/gather is lacking.
>
> Keep MPC8308 operational by always starting transfers from software,
> this SoC appears to not have request lines for flow control when
> peripherals are involved in transfers.
>
> Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
> ---
> drivers/dma/mpc512x_dma.c | 201 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 193 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
> index f41639f..d0c8950 100644
> --- a/drivers/dma/mpc512x_dma.c
> +++ b/drivers/dma/mpc512x_dma.c
> [ ... ]
> @@ -28,11 +29,6 @@
> * file called COPYING.
> */
>
> -/*
> - * This is initial version of MPC5121 DMA driver. Only memory to memory
> - * transfers are supported (tested using dmatest module).
> - */
> -
> #include <linux/module.h>
> #include <linux/dmaengine.h>
> #include <linux/dma-mapping.h>
You may want to keep the comment in this location after updating
it. It's worth remaining aware of the current status, missing
features and/or known limitations. And there _are_ (acceptable
yet worth tracking) issues at this stage of the implementation.
> @@ -187,6 +183,7 @@ struct mpc_dma_desc {
> dma_addr_t tcd_paddr;
> int error;
> struct list_head node;
> + int will_access_peripheral;
> };
>
> struct mpc_dma_chan {
> @@ -199,6 +196,10 @@ struct mpc_dma_chan {
> struct mpc_dma_tcd *tcd;
> dma_addr_t tcd_paddr;
>
> + /* Settings for access to peripheral FIFO */
> + dma_addr_t per_paddr; /* FIFO address */
> + u32 tcd_nunits;
> +
> /* Lock for this structure */
> spinlock_t lock;
> };
> @@ -247,10 +248,27 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
> struct mpc_dma_desc *first = NULL;
> struct mpc_dma_desc *prev = NULL;
> struct mpc_dma_desc *mdesc;
> + int staffed = 0;
> int cid = mchan->chan.chan_id;
>
> - /* Move all queued descriptors to active list */
> - list_splice_tail_init(&mchan->queued, &mchan->active);
> + /*
> + * Mem-to-mem transfers can be chained
> + * together into one transaction.
> + * But each transfer which involves peripherals
> + * must be executed separately.
> + */
> + while (!staffed) {
> + mdesc = list_first_entry(&mchan->queued,
> + struct mpc_dma_desc, node);
> +
> + if (!mdesc->will_access_peripheral)
> + list_move_tail(&mdesc->node, &mchan->active);
> + else {
> + staffed = 1;
> + if (list_empty(&mchan->active))
> + list_move_tail(&mdesc->node, &mchan->active);
> + }
> + }
>
> /* Chain descriptors into one transaction */
> list_for_each_entry(mdesc, &mchan->active, node) {
Use of 'break' may eliminate the necessity of the 'staffed'
variable, the name of which somehow feels weird, yet I cannot
come up with a better name. Having explicit abort checks in the
collection loop might better reflect the intention.
The loop's logic looks suspicious. I'm afraid daemons start
flying in the absence of peripheral transfers (memory only
transfers won't terminate the loop).
It took me a while to find out what the intension is and whether
the code meets the intention. A comment could help here (grab
either memory or peripheral transfers, don't mix peripheral and
memory transfers within the same 'active' list, grab at most one
peripheral transfer, assumes that the 'queued' list isn't empty).
Some conditions may be documented above the function and thus the
comment is in the file and only "missing" in the patch's context.
The comment might be obsolete if termination conditions before
'break' already carry useful enough information.
> @@ -264,6 +282,8 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan)
>
> prev->tcd->dlast_sga = mdesc->tcd_paddr;
> prev->tcd->e_sg = 1;
> +
> + /* software start for mem-to-mem transfers */
> mdesc->tcd->start = 1;
>
> prev = mdesc;
This is inside _execute() and the code doesn't tell apart memory
or peripheral transfers. So either the code does something wrong
or the comment is (has become?) obsolete or misleading. Needs to
get re-checked.
> @@ -641,6 +672,157 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src,
> return &mdesc->desc;
> }
>
> +static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg(
> + struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction direction,
> + unsigned long flags, void *context)
> +{
> + struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan);
> + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan);
> + struct mpc_dma_desc *mdesc = NULL;
> + dma_addr_t per_paddr;
> + u32 tcd_nunits = 0;
> + struct mpc_dma_tcd *tcd;
> + unsigned long iflags;
> + struct scatterlist *sg;
> + size_t len;
> + int iter, i;
> +
> + if (!list_empty(&mchan->active))
> + return NULL;
What's the intention of this check, and is it actually necessary?
Is _preparing_ a transfer forbidden while the hardware is busy
running another transfer (or postprocessing just has not yet
completed)? I don't think so (but might miss a DMA engine API
requirement).
I feel that the DMA engine driver should happily allow for
preparation and submission of transfers, while the _execute()
logic automatically controls flow of queued transfers to the
hardware execution. It's a useful feature when clients can setup
the next transfer while the current transfer still is in progress
(isn't this the motivation of having DMA at all?).
> +
> + /* currently there is no proper support for scatter/gather */
> + if (sg_len > 1)
> + return NULL;
That's a TODO or known limitation item for the top of the file.
> +
> + for_each_sg(sgl, sg, sg_len, i) {
> + spin_lock_irqsave(&mchan->lock, iflags);
> +
> + mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc,
> + node);
> + if (!mdesc) {
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> + /* try to free completed descriptors */
> + mpc_dma_process_completed(mdma);
> + return NULL;
> + }
> +
> + list_del(&mdesc->node);
>From here on 'mdesc' is neither in the 'free' nor in the
'prepared' lists. Returning when conditions aren't met will leak
the entry. Error handling is required.
> +
> + per_paddr = mchan->per_paddr;
> + tcd_nunits = mchan->tcd_nunits;
> +
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> +
> + mdesc->error = 0;
> + mdesc->will_access_peripheral = 1;
> + tcd = mdesc->tcd;
> +
> + /* Prepare Transfer Control Descriptor for this transaction */
> + memset(tcd, 0, sizeof(struct mpc_dma_tcd));
> +
> + if (!IS_ALIGNED(sg_dma_address(sg), 4))
> + return NULL;
> +
> + if (direction == DMA_DEV_TO_MEM) {
> + tcd->saddr = per_paddr;
> + tcd->daddr = sg_dma_address(sg);
> + tcd->soff = 0;
> + tcd->doff = 4;
> + } else if (direction == DMA_MEM_TO_DEV) {
> + tcd->saddr = sg_dma_address(sg);
> + tcd->daddr = per_paddr;
> + tcd->soff = 4;
> + tcd->doff = 0;
> + } else {
> + return NULL;
> + }
> + tcd->ssize = MPC_DMA_TSIZE_4;
> + tcd->dsize = MPC_DMA_TSIZE_4;
These checks and assignments hardcode a port size and transfer
item address increment of 4 bytes. In the future this may need
to become more flexible, until then it's certainly worth keeping
track of (remaining aware of). If preparation enforces such a
constraint, it may be worth checking/signalling in the setup
phase already as well (the device control routine below).
Remember, use in combination with LPC may be the _motivation_ of
your current work, but this is the DMA controller backend driver
which would be great if it could cooperate with any other
peripheral driver which happens to or tries to use DMA as well
(including future potential UART/SPI/I2S peripheral drivers,
MMC/SD card drivers, or GPIO triggered data transfers with
arbitrary characteristics).
NB: Is there a means to specify peripheral transfers which
_don't_ increment the RAM address (could be useful for SPI, to
send dummy TX bytes or to ignore RX data of arbitrary length
without allocating huge buffers), or peripheral transfers which
_do_ increment the peripheral "port's" address (might apply to
memory mapped flash controllers). It's not essential here, just
crossed my mind ...
> +
> + len = sg_dma_len(sg);
> +
> + if (tcd_nunits)
> + tcd->nbytes = tcd_nunits * 4;
> + else
> + return NULL;
I'm afraid that _insisting_ in all DMA clients to provide the
'nunits' parameter ('maxburst' in device control) might be too
specific a constraint.
Clients may not be aware of the necessity, since the idea is to
abstract away the specifics of the backend (DMA controller).
Please consider whether a useful default can be applied in the
absence of an explicit client request. Make the driver as
interoperable as useful, don't restrict yourself to a few
specific clients or just one client that you currently are
focussed on.
> +
> + if (!IS_ALIGNED(len, tcd->nbytes))
> + return NULL;
> +
> + iter = len / tcd->nbytes;
> + if (iter > ((1 << 15) - 1)) { /* maximum biter */
nit: 'iter >= 1 << 15' is shorter and maybe easier to read (might
better reflect that you want to not exceed the 15bit field)
> + return NULL; /* len is too big */
> + } else {
> + /* citer_linkch contains the high bits of iter */
> + tcd->biter = iter & 0x1ff;
> + tcd->biter_linkch = iter >> 9;
> + tcd->citer = tcd->biter;
> + tcd->citer_linkch = tcd->biter_linkch;
> + }
> +
> + tcd->e_sg = 0;
> + tcd->d_req = 1;
> +
> + /* Place descriptor in prepared list */
> + spin_lock_irqsave(&mchan->lock, iflags);
> + list_add_tail(&mdesc->node, &mchan->prepared);
> + spin_unlock_irqrestore(&mchan->lock, iflags);
> + }
> +
> + return &mdesc->desc;
> +}
> +
> +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct mpc_dma_chan *mchan;
> + struct mpc_dma *mdma;
> + struct dma_slave_config *cfg;
> + unsigned long flags;
> +
> + mchan = dma_chan_to_mpc_dma_chan(chan);
> + switch (cmd) {
> + case DMA_TERMINATE_ALL:
> + /* disable channel requests */
> + mdma = dma_chan_to_mpc_dma(chan);
> +
> + spin_lock_irqsave(&mchan->lock, flags);
> +
> + out_8(&mdma->regs->dmacerq, chan->chan_id);
> + list_splice_tail_init(&mchan->prepared, &mchan->free);
> + list_splice_tail_init(&mchan->queued, &mchan->free);
> + list_splice_tail_init(&mchan->active, &mchan->free);
> +
> + spin_unlock_irqrestore(&mchan->lock, flags);
> +
> + return 0;
> + case DMA_SLAVE_CONFIG:
> + cfg = (void *)arg;
> + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
> + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
> + return -EINVAL;
Ah, the addr_width is getting checked, good. I've missed this
before.
Might as well want to check maxburst values here, and optionally
apply a default?
> +
> + spin_lock_irqsave(&mchan->lock, flags);
> +
> + if (cfg->direction == DMA_DEV_TO_MEM) {
Note that cfg->direction is of a different type than
DMA_DEV_TO_MEM (of different type than the 'direction' parameter
in the preparation routine) -- it's mere coincidence that the
numeric values happen to match at the moment.
> + mchan->per_paddr = cfg->src_addr;
> + mchan->tcd_nunits = cfg->src_maxburst;
> + } else {
> + mchan->per_paddr = cfg->dst_addr;
> + mchan->tcd_nunits = cfg->dst_maxburst;
> + }
> +
> + spin_unlock_irqrestore(&mchan->lock, flags);
> +
> + return 0;
> + default:
> + return -ENOSYS;
> + }
> +
> + return -EINVAL;
> +}
> +
> static int mpc_dma_probe(struct platform_device *op)
> {
> struct device_node *dn = op->dev.of_node;
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
^ permalink raw reply
* Re: [PATCH v4 net-next] fix unsafe set_memory_rw from softirq
From: Eric Dumazet @ 2013-10-06 16:56 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: linux-s390, netdev, Heiko Carstens, Eric Dumazet, Daniel Borkmann,
linuxppc-dev, David S. Miller, linux-arm-kernel
In-Reply-To: <1380870846-3357-1-git-send-email-ast@plumgrid.com>
On Fri, 2013-10-04 at 00:14 -0700, Alexei Starovoitov wrote:
> on x86 system with net.core.bpf_jit_enable = 1
> cannot reuse jited filter memory, since it's readonly,
> so use original bpf insns memory to hold work_struct
>
> defer kfree of sk_filter until jit completed freeing
>
> tested on x86_64 and i386
>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts
From: Anshuman Khandual @ 2013-10-07 4:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mikey
Right now the `config_bhrb` PMU specific call happens after write_mmcr0
which actually enables the PMU for event counting and interrupt. So
there is a small window of time where the PMU and BHRB runs without the
required HW branch filter (if any) enabled in BHRB. This can cause some
of the branch samples to be collected through BHRB without any filter
being applied and hence affecting the correctness of the results. This
patch moves the BHRB config function call before enabling the interrupts.
Here are some data points captured via trace prints which depicts how we
could get PMU interrupts with BHRB filter NOT enabled with a standard
perf record command line (asking for branch record information as well).
perf record -j any_call ls
Before the patch:-
ls-1962 [003] d... 2065.299590: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299603: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299611: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299618: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299625: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299632: .perf_event_interrupt: MMCRA: 40000000000
ls-1962 [003] d... 2065.299639: .perf_event_interrupt: MMCRA: 40000000000
--> All the PMU interrupts before this point did not have the
--> requested HW branch filter enabled in the MMCRA.
ls-1962 [003] d... 2065.299647: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.299662: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.299700: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.299798: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.299956: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.300145: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.300347: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.300556: .perf_event_interrupt: MMCRA: 40040000000
ls-1962 [003] d... 2065.300771: .perf_event_interrupt: MMCRA: 40040000000
After the patch:-
ls-1850 [008] d... 190.311828: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311848: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311856: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311863: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311869: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311876: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311884: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311892: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311907: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.311945: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.312044: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.312206: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.312397: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d..2 190.312626: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.312814: .perf_event_interrupt: MMCRA: 40040000000
ls-1850 [008] d... 190.313004: .perf_event_interrupt: MMCRA: 40040000000
--> All the PMU interrupts have the requested
--> HW BHRB branch filter enabled in MMCRA.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/perf/core-book3s.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29b89e8..7d2f13c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1147,6 +1147,9 @@ static void power_pmu_enable(struct pmu *pmu)
mmcr0 = ebb_switch_in(ebb, cpuhw->mmcr[0]);
mb();
+ if (cpuhw->bhrb_users)
+ ppmu->config_bhrb(cpuhw->bhrb_filter);
+
write_mmcr0(cpuhw, mmcr0);
/*
@@ -1158,8 +1161,6 @@ static void power_pmu_enable(struct pmu *pmu)
}
out:
- if (cpuhw->bhrb_users)
- ppmu->config_bhrb(cpuhw->bhrb_filter);
local_irq_restore(flags);
}
--
1.7.11.7
^ permalink raw reply related
* RE: [PATCH 2/7] iommu: add api to get iommu_domain of a device
From: Bhushan Bharat-R65777 @ 2013-10-07 5:46 UTC (permalink / raw)
To: Alex Williamson
Cc: agraf@suse.de, Wood Scott-B07421, linux-pci@vger.kernel.org,
joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1380910322.25705.56.camel@ul30vt.home>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u
IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IEZyaWRheSwgT2N0
b2JlciAwNCwgMjAxMyAxMTo0MiBQTQ0KPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+IENj
OiBqb3JvQDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsgZ2FsYWtAa2VybmVs
LmNyYXNoaW5nLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4cHBj
LWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0NCj4gcGNpQHZnZXIua2VybmVsLm9yZzsgYWdy
YWZAc3VzZS5kZTsgV29vZCBTY290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiBmb3Vu
ZGF0aW9uLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDIvN10gaW9tbXU6IGFkZCBhcGkgdG8g
Z2V0IGlvbW11X2RvbWFpbiBvZiBhIGRldmljZQ0KPiANCj4gT24gRnJpLCAyMDEzLTEwLTA0IGF0
IDE3OjIzICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3Nzcgd3JvdGU6DQo+ID4NCj4gPiA+IC0t
LS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBBbGV4IFdpbGxpYW1zb24gW21h
aWx0bzphbGV4LndpbGxpYW1zb25AcmVkaGF0LmNvbV0NCj4gPiA+IFNlbnQ6IEZyaWRheSwgT2N0
b2JlciAwNCwgMjAxMyAxMDo0MyBQTQ0KPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K
PiA+ID4gQ2M6IGpvcm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiA+
ID4gZ2FsYWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXgtIGtlcm5lbEB2Z2VyLmtlcm5lbC5v
cmc7DQo+ID4gPiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGludXgtIHBjaUB2Z2Vy
Lmtlcm5lbC5vcmc7DQo+ID4gPiBhZ3JhZkBzdXNlLmRlOyBXb29kIFNjb3R0LUIwNzQyMTsgaW9t
bXVAbGlzdHMubGludXgtIGZvdW5kYXRpb24ub3JnDQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENI
IDIvN10gaW9tbXU6IGFkZCBhcGkgdG8gZ2V0IGlvbW11X2RvbWFpbiBvZiBhDQo+ID4gPiBkZXZp
Y2UNCj4gPiA+DQo+ID4gPiBPbiBGcmksIDIwMTMtMTAtMDQgYXQgMTY6NDcgKzAwMDAsIEJodXNo
YW4gQmhhcmF0LVI2NTc3NyB3cm90ZToNCj4gPiA+ID4NCj4gPiA+ID4gPiAtLS0tLU9yaWdpbmFs
IE1lc3NhZ2UtLS0tLQ0KPiA+ID4gPiA+IEZyb206IEFsZXggV2lsbGlhbXNvbiBbbWFpbHRvOmFs
ZXgud2lsbGlhbXNvbkByZWRoYXQuY29tXQ0KPiA+ID4gPiA+IFNlbnQ6IEZyaWRheSwgT2N0b2Jl
ciAwNCwgMjAxMyA5OjE1IFBNDQo+ID4gPiA+ID4gVG86IEJodXNoYW4gQmhhcmF0LVI2NTc3Nw0K
PiA+ID4gPiA+IENjOiBqb3JvQDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsN
Cj4gPiA+ID4gPiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eC0ga2VybmVsQHZnZXIu
a2VybmVsLm9yZzsNCj4gPiA+ID4gPiBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZzsgbGlu
dXgtIHBjaUB2Z2VyLmtlcm5lbC5vcmc7DQo+ID4gPiA+ID4gYWdyYWZAc3VzZS5kZTsgV29vZCBT
Y290dC1CMDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiA+ID4gPiA+IGZvdW5kYXRpb24ub3Jn
DQo+ID4gPiA+ID4gU3ViamVjdDogUmU6IFtQQVRDSCAyLzddIGlvbW11OiBhZGQgYXBpIHRvIGdl
dCBpb21tdV9kb21haW4gb2YgYQ0KPiA+ID4gPiA+IGRldmljZQ0KPiA+ID4gPiA+DQo+ID4gPiA+
ID4gT24gRnJpLCAyMDEzLTEwLTA0IGF0IDA5OjU0ICswMDAwLCBCaHVzaGFuIEJoYXJhdC1SNjU3
Nzcgd3JvdGU6DQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3Nh
Z2UtLS0tLQ0KPiA+ID4gPiA+ID4gPiBGcm9tOiBsaW51eC1wY2ktb3duZXJAdmdlci5rZXJuZWwu
b3JnDQo+ID4gPiA+ID4gPiA+IFttYWlsdG86bGludXgtcGNpLW93bmVyQHZnZXIua2VybmVsLm9y
Z10NCj4gPiA+ID4gPiA+ID4gT24gQmVoYWxmIE9mIEFsZXggV2lsbGlhbXNvbg0KPiA+ID4gPiA+
ID4gPiBTZW50OiBXZWRuZXNkYXksIFNlcHRlbWJlciAyNSwgMjAxMyAxMDoxNiBQTQ0KPiA+ID4g
PiA+ID4gPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gPiA+IENjOiBqb3Jv
QDhieXRlcy5vcmc7IGJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZzsNCj4gPiA+ID4gPiA+ID4gZ2Fs
YWtAa2VybmVsLmNyYXNoaW5nLm9yZzsgbGludXgtIGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+
ID4gPiA+ID4gPiA+IGxpbnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0gcGNpQHZn
ZXIua2VybmVsLm9yZzsNCj4gPiA+ID4gPiA+ID4gYWdyYWZAc3VzZS5kZTsgV29vZCBTY290dC1C
MDc0MjE7IGlvbW11QGxpc3RzLmxpbnV4LQ0KPiA+ID4gPiA+ID4gPiBmb3VuZGF0aW9uLm9yZzsg
Qmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+ID4gPiA+ID4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
Mi83XSBpb21tdTogYWRkIGFwaSB0byBnZXQgaW9tbXVfZG9tYWluDQo+ID4gPiA+ID4gPiA+IG9m
IGEgZGV2aWNlDQo+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+IE9uIFRodSwgMjAxMy0wOS0x
OSBhdCAxMjo1OSArMDUzMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+ID4gPiA+ID4gPiA+ID4g
VGhpcyBhcGkgcmV0dXJuIHRoZSBpb21tdSBkb21haW4gdG8gd2hpY2ggdGhlIGRldmljZSBpcyBh
dHRhY2hlZC4NCj4gPiA+ID4gPiA+ID4gPiBUaGUgaW9tbXVfZG9tYWluIGlzIHJlcXVpcmVkIGZv
ciBtYWtpbmcgQVBJIGNhbGxzIHJlbGF0ZWQgdG8NCj4gaW9tbXUuDQo+ID4gPiA+ID4gPiA+ID4g
Rm9sbG93IHVwIHBhdGNoZXMgd2hpY2ggdXNlIHRoaXMgQVBJIHRvIGtub3cgaW9tbXUgbWFwaW5n
Lg0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gU2lnbmVkLW9mZi1ieTogQmhhcmF0
IEJodXNoYW4NCj4gPiA+ID4gPiA+ID4gPiA8YmhhcmF0LmJodXNoYW5AZnJlZXNjYWxlLmNvbT4N
Cj4gPiA+ID4gPiA+ID4gPiAtLS0NCj4gPiA+ID4gPiA+ID4gPiAgZHJpdmVycy9pb21tdS9pb21t
dS5jIHwgICAxMCArKysrKysrKysrDQo+ID4gPiA+ID4gPiA+ID4gIGluY2x1ZGUvbGludXgvaW9t
bXUuaCB8ICAgIDcgKysrKysrKw0KPiA+ID4gPiA+ID4gPiA+ICAyIGZpbGVzIGNoYW5nZWQsIDE3
IGluc2VydGlvbnMoKyksIDAgZGVsZXRpb25zKC0pDQo+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4g
PiA+ID4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9pb21tdS9pb21tdS5jIGIvZHJpdmVycy9pb21t
dS9pb21tdS5jDQo+ID4gPiA+ID4gPiA+ID4gaW5kZXgNCj4gPiA+ID4gPiA+ID4gPiBmYmU5Y2E3
Li42YWM1ZjUwIDEwMDY0NA0KPiA+ID4gPiA+ID4gPiA+IC0tLSBhL2RyaXZlcnMvaW9tbXUvaW9t
bXUuYw0KPiA+ID4gPiA+ID4gPiA+ICsrKyBiL2RyaXZlcnMvaW9tbXUvaW9tbXUuYw0KPiA+ID4g
PiA+ID4gPiA+IEBAIC02OTYsNiArNjk2LDE2IEBAIHZvaWQgaW9tbXVfZGV0YWNoX2RldmljZShz
dHJ1Y3QNCj4gPiA+ID4gPiA+ID4gPiBpb21tdV9kb21haW4gKmRvbWFpbiwgc3RydWN0IGRldmlj
ZSAqZGV2KSAgfQ0KPiA+ID4gPiA+ID4gPiA+IEVYUE9SVF9TWU1CT0xfR1BMKGlvbW11X2RldGFj
aF9kZXZpY2UpOw0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+ID4gK3N0cnVjdCBpb21t
dV9kb21haW4gKmlvbW11X2dldF9kZXZfZG9tYWluKHN0cnVjdCBkZXZpY2UgKmRldikgew0KPiA+
ID4gPiA+ID4gPiA+ICsJc3RydWN0IGlvbW11X29wcyAqb3BzID0gZGV2LT5idXMtPmlvbW11X29w
czsNCj4gPiA+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4gPiA+ID4gKwlpZiAodW5saWtlbHkob3Bz
ID09IE5VTEwgfHwgb3BzLT5nZXRfZGV2X2lvbW11X2RvbWFpbiA9PQ0KPiBOVUxMKSkNCj4gPiA+
ID4gPiA+ID4gPiArCQlyZXR1cm4gTlVMTDsNCj4gPiA+ID4gPiA+ID4gPiArDQo+ID4gPiA+ID4g
PiA+ID4gKwlyZXR1cm4gb3BzLT5nZXRfZGV2X2lvbW11X2RvbWFpbihkZXYpOyB9DQo+ID4gPiA+
ID4gPiA+ID4gK0VYUE9SVF9TWU1CT0xfR1BMKGlvbW11X2dldF9kZXZfZG9tYWluKTsNCj4gPiA+
ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gV2hhdCBwcmV2ZW50cyB0aGlzIGZyb20gcmFjaW5nIGlv
bW11X2RvbWFpbl9mcmVlKCk/ICBUaGVyZSdzDQo+ID4gPiA+ID4gPiA+IG5vIHJlZmVyZW5jZXMg
YWNxdWlyZWQsIHNvIHRoZXJlJ3Mgbm8gcmVhc29uIGZvciB0aGUgY2FsbGVyDQo+ID4gPiA+ID4g
PiA+IHRvIGFzc3VtZSB0aGUNCj4gPiA+ID4gPiBwb2ludGVyIGlzIHZhbGlkLg0KPiA+ID4gPiA+
ID4NCj4gPiA+ID4gPiA+IFNvcnJ5IGZvciBsYXRlIHF1ZXJ5LCBzb21laG93IHRoaXMgZW1haWwg
d2VudCBpbnRvIGEgZm9sZGVyDQo+ID4gPiA+ID4gPiBhbmQgZXNjYXBlZDsNCj4gPiA+ID4gPiA+
DQo+ID4gPiA+ID4gPiBKdXN0IHRvIGJlIHN1cmUsIHRoZXJlIGlzIG5vdCBsb2NrIGF0IGdlbmVy
aWMgInN0cnVjdA0KPiA+ID4gPiA+ID4gaW9tbXVfZG9tYWluIiwgYnV0IElQDQo+ID4gPiA+ID4g
c3BlY2lmaWMgc3RydWN0dXJlIChsaW5rIEZTTCBkb21haW4pIGxpbmtlZCBpbg0KPiA+ID4gPiA+
IGlvbW11X2RvbWFpbi0+cHJpdiBoYXZlIGEgbG9jaywgc28gd2UgbmVlZCB0byBlbnN1cmUgdGhp
cyByYWNlDQo+ID4gPiA+ID4gaW4gRlNMIGlvbW11IGNvZGUgKHNheSBkcml2ZXJzL2lvbW11L2Zz
bF9wYW11X2RvbWFpbi5jKSwgcmlnaHQ/DQo+ID4gPiA+ID4NCj4gPiA+ID4gPiBObywgaXQncyBu
b3Qgc3VmZmljaWVudCB0byBtYWtlIHN1cmUgdGhhdCB5b3VyIHVzZSBvZiB0aGUNCj4gPiA+ID4g
PiBpbnRlcmZhY2UgaXMgcmFjZSBmcmVlLiAgVGhlIGludGVyZmFjZSBpdHNlbGYgbmVlZHMgdG8g
YmUNCj4gPiA+ID4gPiBkZXNpZ25lZCBzbyB0aGF0IGl0J3MgZGlmZmljdWx0IHRvIHVzZSBpbmNv
cnJlY3RseS4NCj4gPiA+ID4NCj4gPiA+ID4gU28gd2UgY2FuIGRlZmluZSBpb21tdV9nZXRfZGV2
X2RvbWFpbigpL2lvbW11X3B1dF9kZXZfZG9tYWluKCk7DQo+ID4gPiA+IGlvbW11X2dldF9kZXZf
ZG9tYWluKCkgd2lsbCByZXR1cm4gZG9tYWluIHdpdGggdGhlIGxvY2sgaGVsZCwgYW5kDQo+ID4g
PiA+IGlvbW11X3B1dF9kZXZfZG9tYWluKCkgd2lsbCByZWxlYXNlIHRoZSBsb2NrPyBBbmQNCj4g
PiA+ID4gaW9tbXVfZ2V0X2Rldl9kb21haW4oKSBtdXN0IGFsd2F5cyBiZSBmb2xsb3dlZCBieQ0K
PiA+ID4gPiBpb21tdV9nZXRfZGV2X2RvbWFpbigpLg0KPiA+ID4NCj4gPiA+IFdoYXQgbG9jaz8g
IGdldC9wdXQgYXJlIGdlbmVyYWxseSB1c2VkIGZvciByZWZlcmVuY2UgY291bnRpbmcsIG5vdA0K
PiA+ID4gbG9ja2luZyBpbiB0aGUga2VybmVsLg0KPiA+ID4NCj4gPiA+ID4gPiBUaGF0J3Mgbm90
IHRoZSBjYXNlIGhlcmUuICBUaGlzIGlzIGEgYmFja2Rvb3IgdG8gZ2V0IHRoZSBpb21tdQ0KPiA+
ID4gPiA+IGRvbWFpbiBmcm9tIHRoZSBpb21tdSBkcml2ZXIgcmVnYXJkbGVzcyBvZiB3aG8gaXMg
dXNpbmcgaXQgb3IgaG93Lg0KPiA+ID4gPiA+IFRoZSBpb21tdSBkb21haW4gaXMgY3JlYXRlZCBh
bmQgbWFuYWdlZCBieSB2ZmlvLCBzbyBzaG91bGRuJ3Qgd2UNCj4gPiA+ID4gPiBiZSBsb29raW5n
IGF0IGhvdyB0byBkbyB0aGlzIHRocm91Z2ggdmZpbz8NCj4gPiA+ID4NCj4gPiA+ID4gTGV0IG1l
IGZpcnN0IGRlc2NyaWJlIHdoYXQgd2UgYXJlIGRvaW5nIGhlcmU6DQo+ID4gPiA+IER1cmluZyBp
bml0aWFsaXphdGlvbjotDQo+ID4gPiA+ICAtIHZmaW8gdGFsa3MgdG8gTVNJIHN5c3RlbSB0byBr
bm93IHRoZSBNU0ktcGFnZSBhbmQgc2l6ZQ0KPiA+ID4gPiAgLSB2ZmlvIHRoZW4gaW50ZXJhY3Rz
IHdpdGggaW9tbXUgdG8gbWFwIHRoZSBNU0ktcGFnZSBpbiBpb21tdQ0KPiA+ID4gPiAoSU9WQSBp
cyBkZWNpZGVkIGJ5IHVzZXJzcGFjZSBhbmQgcGh5c2ljYWwgYWRkcmVzcyBpcyB0aGUNCj4gPiA+
ID4gTVNJLXBhZ2UpDQo+ID4gPiA+ICAtIFNvIHRoZSBJT1ZBIHN1YndpbmRvdyBtYXBwaW5nIGlz
IGNyZWF0ZWQgaW4gaW9tbXUgYW5kIHllcyBWRklPDQo+ID4gPiA+IGtub3cgYWJvdXQNCj4gPiA+
IHRoaXMgbWFwcGluZy4NCj4gPiA+ID4NCj4gPiA+ID4gTm93IGRvIFNFVF9JUlEoTVNJL01TSVgp
IGlvY3RsOg0KPiA+ID4gPiAgLSBjYWxscyBwY2lfZW5hYmxlX21zaXgoKS9wY2lfZW5hYmxlX21z
aV9ibG9jaygpOiB3aGljaCBpcw0KPiA+ID4gPiBzdXBwb3NlZCB0byBzZXQNCj4gPiA+IE1TSSBh
ZGRyZXNzL2RhdGEgaW4gZGV2aWNlLg0KPiA+ID4gPiAgLSBTbyBpbiBjdXJyZW50IGltcGxlbWVu
dGF0aW9uICh0aGlzIHBhdGNoc2V0KSBtc2ktc3Vic3lzdGVtIGdldHMNCj4gPiA+ID4gdGhlIElP
VkENCj4gPiA+IGZyb20gaW9tbXUgdmlhIHRoaXMgZGVmaW5lZCBpbnRlcmZhY2UuDQo+ID4gPiA+
ICAtIEFyZSB5b3Ugc2F5aW5nIHRoYXQgcmF0aGVyIHRoYW4gZ2V0dGluZyB0aGlzIGZyb20gaW9t
bXUsIHdlDQo+ID4gPiA+IHNob3VsZCBnZXQgdGhpcw0KPiA+ID4gZnJvbSB2ZmlvPyBXaGF0IGRp
ZmZlcmVuY2UgZG9lcyB0aGlzIG1ha2U/DQo+ID4gPg0KPiA+ID4gWWVzLCB5b3UganVzdCBzYWlk
IGFib3ZlIHRoYXQgdmZpbyBrbm93cyB0aGUgbXNpIHRvIGlvdmEgbWFwcGluZywgc28NCj4gPiA+
IHdoeSBnbyBvdXRzaWRlIG9mIHZmaW8gdG8gZmluZCBpdCBsYXRlcj8gIFRoZSBkaWZmZXJlbmNl
IGlzIG9uZSBjYXNlDQo+ID4gPiB5b3UgY2FuIGhhdmUgYSBwcm9wZXIgcmVmZXJlbmNlIHRvIGRh
dGEgc3RydWN0dXJlcyB0byBtYWtlIHN1cmUgdGhlDQo+ID4gPiBwb2ludGVyIHlvdSBnZXQgYmFj
ayBhY3R1YWxseSBoYXMgbWVhbmluZyBhdCB0aGUgdGltZSB5b3UncmUgdXNpbmcNCj4gPiA+IGl0
IHZzIHRoZSBjb2RlIGhlcmUgd2hlcmUgeW91J3JlIGRlZmluaW5nIGFuIEFQSSB0aGF0IHJldHVy
bnMgYQ0KPiA+ID4gbWVhbmluZ2xlc3MgdmFsdWUNCj4gPg0KPiA+IFdpdGggRlNMLVBBTVUgd2Ug
d2lsbCBhbHdheXMgZ2V0IGNvbnNpc3RhbnQgZGF0YSBmcm9tIGlvbW11IG9yIHZmaW8tZGF0YQ0K
PiBzdHJ1Y3R1cmUuDQo+IA0KPiBHcmVhdCwgYnV0IHlvdSdyZSB0cnlpbmcgdG8gYWRkIGEgZ2Vu
ZXJpYyBBUEkgdG8gdGhlIElPTU1VIHN1YnN5c3RlbSB0aGF0J3MNCj4gZGlmZmljdWx0IHRvIHVz
ZSBjb3JyZWN0bHkuICBUaGUgZmFjdCB0aGF0IHlvdSB1c2UgaXQgY29ycmVjdGx5IGRvZXMgbm90
IGp1c3RpZnkNCj4gdGhlIEFQSS4NCj4gDQo+ID4gPiBiZWNhdXNlIHlvdSBjYW4ndCBjaGVjayBv
cg0KPiA+ID4gZW5mb3JjZSB0aGF0IGFuIGFyYml0cmFyeSBjYWxsZXIgaXMgdXNpbmcgaXQgY29y
cmVjdGx5Lg0KPiA+DQo+ID4gSSBhbSBub3Qgc3VyZSB3aGF0IGlzIGFyYml0cmFyeSBjYWxsZXI/
IHBkZXYgaXMga25vd24gdG8gdmZpbywgc28gdmZpbw0KPiA+IHdpbGwgb25seSBtYWtlIHBjaV9l
bmFibGVfbXNpeCgpL3BjaV9lbmFibGVfbXNpX2Jsb2NrKCkgZm9yIHRoaXMgcGRldi4NCj4gPiBJ
ZiBhbnlvdGhlciBjb2RlIG1ha2VzIHRoZW4gaXQgaXMgc29tZSBvdGhlciB1bmV4cGVjdGVkbHkg
dGhpbmcNCj4gPiBoYXBwZW5pbmcgaW4gc3lzdGVtLCBubz8NCj4gDQo+IFdoYXQncyBwcm9wb3Nl
ZCBoZXJlIGlzIGEgZ2VuZXJpYyBJT01NVSBBUEkuICBBbnlib2R5IGNhbiBjYWxsIHRoaXMuDQo+
IFdoYXQgaWYgdGhlIGhvc3QgU0NTSSBkcml2ZXIgZGVjaWRlcyB0byBnbyBnZXQgdGhlIGlvbW11
IGRvbWFpbiBmb3IgaXQncyBkZXZpY2UNCj4gKG9yIGFueSBvdGhlciBkZXZpY2UpPyAgRG9lcyB0
aGF0IGZpdCB5b3VyIHVzYWdlIG1vZGVsPw0KPiANCj4gPiA+ICBJdCdzIG5vdCBtYWludGFpbmFi
bGUuDQo+ID4gPiBUaGFua3MsDQo+ID4NCj4gPiBJIGRvIG5vdCBoYXZlIGFueSBpc3N1ZSB3aXRo
IHRoaXMgYXMgd2VsbCwgY2FuIHlvdSBhbHNvIGRlc2NyaWJlIHRoZQ0KPiA+IHR5cGUgb2YgQVBJ
IHlvdSBhcmUgZW52aXNpb25pbmc7IEkgY2FuIHRoaW5rIG9mIGRlZmluaW5nIHNvbWUgZnVuY3Rp
b24NCj4gPiBpbiB2ZmlvLmMvdmZpb19pb21tdSouYywgbWFrZSB0aGVtIGdsb2JhbCBhbmQgZGVj
bGFyZSB0aGVuIGluDQo+ID4gaW5jbHVkZS9MaW51eC92ZmlvLmggQW5kIGluY2x1ZGUgPExpbnV4
L3ZmaW8uaD4gaW4gY2FsbGVyIGZpbGUNCj4gPiAoYXJjaC9wb3dlcnBjL2tlcm5lbC9tc2kuYykN
Cj4gDQo+IERvIHlvdSByZWFsbHkgd2FudCBtb2R1bGUgZGVwZW5kZW5jaWVzIGJldHdlZW4gdmZp
byBhbmQgeW91ciBjb3JlIGtlcm5lbCBNU0kNCj4gc2V0dXA/ICBMb29rIGF0IHRoZSB2ZmlvIGV4
dGVybmFsIHVzZXIgaW50ZXJmYWNlIHRoYXQgd2UndmUgYWxyZWFkeSBkZWZpbmVkLg0KPiBUaGF0
IGFsbG93cyBvdGhlciBjb21wb25lbnRzIG9mIHRoZSBrZXJuZWwgdG8gZ2V0IGEgcHJvcGVyIHJl
ZmVyZW5jZSB0byBhIHZmaW8NCj4gZ3JvdXAuICBGcm9tIHRoZXJlIHlvdSBjYW4gd29yayBvdXQg
aG93IHRvIGdldCB3aGF0IHlvdSB3YW50LiAgQW5vdGhlcg0KPiBhbHRlcm5hdGl2ZSBpcyB0aGF0
IHZmaW8gY291bGQgcmVnaXN0ZXIgYW4gTVNJIHRvIElPVkEgbWFwcGluZyB3aXRoIGFyY2hpdGVj
dHVyZQ0KPiBjb2RlIHdoZW4gdGhlIG1hcHBpbmcgaXMgY3JlYXRlZC4gIFRoZSBNU0kgc2V0dXAg
cGF0aCBjb3VsZCB0aGVuIGRvIGEgbG9va3VwIGluDQo+IGFyY2hpdGVjdHVyZSBjb2RlIGZvciB0
aGUgbWFwcGluZy4gIFlvdSBjb3VsZCBldmVuIHN0b3JlIHRoZSBNU0kgdG8gSU9WQSBtYXBwaW5n
DQo+IGluIFZGSU8gYW5kIGNyZWF0ZSBhbiBpbnRlcmZhY2Ugd2hlcmUgU0VUX0lSUSBwYXNzZXMg
dGhhdCBtYXBwaW5nIGludG8gc2V0dXANCj4gY29kZS4NCg0KT2ssIFdoYXQgSSB3YW50IGlzIHRv
IGdldCBJT1ZBIGFzc29jaWF0ZWQgd2l0aCBhIHBoeXNpY2FsIGFkZHJlc3MgKHBoeXNpY2FsIGFk
ZHJlc3Mgb2YgTVNJLWJhbmspLg0KQW5kIGN1cnJlbnRseSBJIGRvIG5vdCBzZWUgYSB3YXkgdG8g
a25vdyBJT1ZBIG9mIGEgcGh5c2ljYWwgYWRkcmVzcyBhbmQgZG9pbmcgYWxsIHRoaXMgZG9tYWlu
IGdldCBhbmQgdGhlbiBzZWFyY2ggdGhyb3VnaCBhbGwgb2YgaW9tbXUtd2luZG93cyBvZiB0aGF0
IGRvbWFpbi4NCg0KV2hhdCBpZiB3ZSBhZGQgYW4gaW9tbXUtQVBJIHdoaWNoIGNhbiByZXR1cm4g
dGhlIElPVkEgbWFwcGluZyBvZiBhIHBoeXNpY2FsIGFkZHJlc3MuIEN1cnJlbnQgdXNlIGNhc2Ug
aXMgc2V0dGluZyB1cCBNU0kncyBmb3IgYXBlcnR1cmUgdHlwZSBvZiBJT01NVSBhbHNvIGdldHRp
bmcgYSBwaHlzX3RvX2lvdmEoKSBtYXBwaW5nIGlzIGluZGVwZW5kZW50IG9mIFZGSU8sIHlvdXIg
dGhvdWdodD8NCg0KVGhhbmtzDQotQmhhcmF0DQoNCj4gVGhhbmtzLA0KPiANCj4gQWxleA0KPiAN
Cg0K
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox