* [PATCH] powerpc: Added __cmpdi2 for signed 64bit comparision
From: Bharat Bhushan @ 2013-10-09 5:11 UTC (permalink / raw)
To: scottwood, linuxppc-dev, benh; +Cc: Bharat Bhushan
This was missing on powerpc and I am getting compilation error
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/kernel/misc_32.S | 14 ++++++++++++++
arch/powerpc/kernel/ppc_ksyms.c | 2 ++
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 777d999..7c0eec2 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
blr
/*
+ * 64-bit comparison: __cmpdi2(s64 a, s64 b)
+ * Returns 0 if a < b, 1 if a == b, 2 if a > b.
+ */
+_GLOBAL(__cmpdi2)
+ cmpw r3,r5
+ li r3,1
+ bne 1f
+ cmplw r4,r6
+ beqlr
+1: li r3,0
+ bltlr
+ li r3,2
+ blr
+/*
* 64-bit comparison: __ucmpdi2(u64 a, u64 b)
* Returns 0 if a < b, 1 if a == b, 2 if a > b.
*/
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 21646db..5674c00 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3);
EXPORT_SYMBOL(__lshrdi3);
int __ucmpdi2(unsigned long long, unsigned long long);
EXPORT_SYMBOL(__ucmpdi2);
+int __cmpdi2(long long, long long);
+EXPORT_SYMBOL(__cmpdi2);
#endif
long long __bswapdi2(long long);
EXPORT_SYMBOL(__bswapdi2);
--
1.7.0.4
^ permalink raw reply related
* RE: [PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision
From: Bhushan Bharat-R65777 @ 2013-10-09 5:17 UTC (permalink / raw)
To: Bhushan Bharat-R65777, Wood Scott-B07421,
linuxppc-dev@lists.ozlabs.org, benh@kernel.crashing.org
In-Reply-To: <1381295322-7197-1-git-send-email-Bharat.Bhushan@freescale.com>
Oops it came as 1/4,
I am sorry, please ignore this
Thanks
-Bharat
> -----Original Message-----
> From: Bhushan Bharat-R65777
> Sent: Wednesday, October 09, 2013 10:39 AM
> To: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; benh@kernel.crashin=
g.org
> Cc: Bhushan Bharat-R65777; Bhushan Bharat-R65777
> Subject: [PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision
>=20
> This was missing on powerpc and I am getting compilation error
> drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
> drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
>=20
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> arch/powerpc/kernel/misc_32.S | 14 ++++++++++++++
> arch/powerpc/kernel/ppc_ksyms.c | 2 ++
> 2 files changed, 16 insertions(+), 0 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.=
S index
> 777d999..7c0eec2 100644
> --- a/arch/powerpc/kernel/misc_32.S
> +++ b/arch/powerpc/kernel/misc_32.S
> @@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
> blr
>=20
> /*
> + * 64-bit comparison: __cmpdi2(s64 a, s64 b)
> + * Returns 0 if a < b, 1 if a =3D=3D b, 2 if a > b.
> + */
> +_GLOBAL(__cmpdi2)
> + cmpw r3,r5
> + li r3,1
> + bne 1f
> + cmplw r4,r6
> + beqlr
> +1: li r3,0
> + bltlr
> + li r3,2
> + blr
> +/*
> * 64-bit comparison: __ucmpdi2(u64 a, u64 b)
> * Returns 0 if a < b, 1 if a =3D=3D b, 2 if a > b.
> */
> diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ks=
yms.c
> index 21646db..5674c00 100644
> --- a/arch/powerpc/kernel/ppc_ksyms.c
> +++ b/arch/powerpc/kernel/ppc_ksyms.c
> @@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3); EXPORT_SYMBOL(__lshrdi3); =
int
> __ucmpdi2(unsigned long long, unsigned long long); EXPORT_SYMBOL(__ucmpd=
i2);
> +int __cmpdi2(long long, long long);
> +EXPORT_SYMBOL(__cmpdi2);
> #endif
> long long __bswapdi2(long long);
> EXPORT_SYMBOL(__bswapdi2);
> --
> 1.7.0.4
^ permalink raw reply
* [PATCH 1/4] powerpc: Added __cmpdi2 for signed 64bit comparision
From: Bharat Bhushan @ 2013-10-09 5:08 UTC (permalink / raw)
To: scottwood, linuxppc-dev, benh; +Cc: Bharat Bhushan
This was missing on powerpc and I am getting compilation error
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
drivers/vfio/pci/vfio_pci_rdwr.c:193: undefined reference to `__cmpdi2'
Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
---
arch/powerpc/kernel/misc_32.S | 14 ++++++++++++++
arch/powerpc/kernel/ppc_ksyms.c | 2 ++
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index 777d999..7c0eec2 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -644,6 +644,20 @@ _GLOBAL(__lshrdi3)
blr
/*
+ * 64-bit comparison: __cmpdi2(s64 a, s64 b)
+ * Returns 0 if a < b, 1 if a == b, 2 if a > b.
+ */
+_GLOBAL(__cmpdi2)
+ cmpw r3,r5
+ li r3,1
+ bne 1f
+ cmplw r4,r6
+ beqlr
+1: li r3,0
+ bltlr
+ li r3,2
+ blr
+/*
* 64-bit comparison: __ucmpdi2(u64 a, u64 b)
* Returns 0 if a < b, 1 if a == b, 2 if a > b.
*/
diff --git a/arch/powerpc/kernel/ppc_ksyms.c b/arch/powerpc/kernel/ppc_ksyms.c
index 21646db..5674c00 100644
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -143,6 +143,8 @@ EXPORT_SYMBOL(__ashldi3);
EXPORT_SYMBOL(__lshrdi3);
int __ucmpdi2(unsigned long long, unsigned long long);
EXPORT_SYMBOL(__ucmpdi2);
+int __cmpdi2(long long, long long);
+EXPORT_SYMBOL(__cmpdi2);
#endif
long long __bswapdi2(long long);
EXPORT_SYMBOL(__bswapdi2);
--
1.7.0.4
^ permalink raw reply related
* RE: [PATCH 1/7] powerpc: Add interface to get msi region information
From: Bhushan Bharat-R65777 @ 2013-10-09 4:47 UTC (permalink / raw)
To: Wood Scott-B07421
Cc: agraf@suse.de, joro@8bytes.org, linux-kernel@vger.kernel.org,
iommu@lists.linux-foundation.org, alex.williamson@redhat.com,
linux-pci@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1381273037.7979.298.camel@snotra.buserror.net>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogV2VkbmVzZGF5LCBPY3RvYmVyIDA5LCAyMDEzIDQ6MjcgQU0NCj4gVG86IEJo
dXNoYW4gQmhhcmF0LVI2NTc3Nw0KPiBDYzogYWxleC53aWxsaWFtc29uQHJlZGhhdC5jb207IGpv
cm9AOGJ5dGVzLm9yZzsgYmVuaEBrZXJuZWwuY3Jhc2hpbmcub3JnOw0KPiBnYWxha0BrZXJuZWwu
Y3Jhc2hpbmcub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBsaW51eHBwYy0NCj4g
ZGV2QGxpc3RzLm96bGFicy5vcmc7IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGFncmFmQHN1
c2UuZGU7DQo+IGlvbW11QGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnOyBCaHVzaGFuIEJoYXJh
dC1SNjU3NzcNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAxLzddIHBvd2VycGM6IEFkZCBpbnRlcmZh
Y2UgdG8gZ2V0IG1zaSByZWdpb24gaW5mb3JtYXRpb24NCj4gDQo+IE9uIFRodSwgMjAxMy0wOS0x
OSBhdCAxMjo1OSArMDUzMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+ID4gQEAgLTM3Niw2ICs0
MDUsNyBAQCBzdGF0aWMgaW50IGZzbF9vZl9tc2lfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2Rldmlj
ZSAqZGV2KQ0KPiA+ICAJaW50IGxlbjsNCj4gPiAgCXUzMiBvZmZzZXQ7DQo+ID4gIAlzdGF0aWMg
Y29uc3QgdTMyIGFsbF9hdmFpbFtdID0geyAwLCBOUl9NU0lfSVJRUyB9Ow0KPiA+ICsJc3RhdGlj
IGludCBiYW5rX2luZGV4Ow0KPiA+DQo+ID4gIAltYXRjaCA9IG9mX21hdGNoX2RldmljZShmc2xf
b2ZfbXNpX2lkcywgJmRldi0+ZGV2KTsNCj4gPiAgCWlmICghbWF0Y2gpDQo+ID4gQEAgLTQxOSw4
ICs0NDksOCBAQCBzdGF0aWMgaW50IGZzbF9vZl9tc2lfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2Rl
dmljZSAqZGV2KQ0KPiA+ICAJCQkJZGV2LT5kZXYub2Zfbm9kZS0+ZnVsbF9uYW1lKTsNCj4gPiAg
CQkJZ290byBlcnJvcl9vdXQ7DQo+ID4gIAkJfQ0KPiA+IC0JCW1zaS0+bXNpaXJfb2Zmc2V0ID0N
Cj4gPiAtCQkJZmVhdHVyZXMtPm1zaWlyX29mZnNldCArIChyZXMuc3RhcnQgJiAweGZmZmZmKTsN
Cj4gPiArCQltc2ktPm1zaWlyID0gcmVzLnN0YXJ0ICsgZmVhdHVyZXMtPm1zaWlyX29mZnNldDsN
Cj4gPiArCQlwcmludGsoIm1zaS0+bXNpaXIgPSAlbGx4XG4iLCBtc2ktPm1zaWlyKTsNCj4gDQo+
IGRldl9kYmcgb3IgcmVtb3ZlDQoNCk9vcHMsIHNvcnJ5IGl0IHdhcyBsZWZ0b3ZlciBvZiBkZWJ1
Z2dpbmcgOigNCg0KPiANCj4gPiAgCX0NCj4gPg0KPiA+ICAJbXNpLT5mZWF0dXJlID0gZmVhdHVy
ZXMtPmZzbF9waWNfaXA7IEBAIC00NzAsNiArNTAwLDcgQEAgc3RhdGljIGludA0KPiA+IGZzbF9v
Zl9tc2lfcHJvYmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqZGV2KQ0KPiA+ICAJCX0NCj4gPiAg
CX0NCj4gPg0KPiA+ICsJbXNpLT5iYW5rX2luZGV4ID0gYmFua19pbmRleCsrOw0KPiANCj4gV2hh
dCBpZiBtdWx0aXBsZSBNU0lzIGFyZSBib2luZyBwcm9iZWQgaW4gcGFyYWxsZWw/DQoNCk9oaCwg
SSBoYXZlIG5vdCB0aG91Z2h0IHRoYXQgaXQgY2FuIGJlIGNhbGxlZCBpbiBwYXJhbGxlbA0KDQo+
ICBiYW5rX2luZGV4IGlzIG5vdCBhdG9taWMuDQoNCldpbGwgZGVjbGFyZSBiYW5rX2ludGV4IGFz
IGF0b21pY190IGFuZCB1c2UgYXRvbWljX2luY19yZXR1cm4oJmJhbmtfaW5kZXgpDQoNCj4gDQo+
ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9zeXNkZXYvZnNsX21zaS5oDQo+ID4gYi9hcmNo
L3Bvd2VycGMvc3lzZGV2L2ZzbF9tc2kuaCBpbmRleCA4MjI1Zjg2Li42YmQ1Y2ZjIDEwMDY0NA0K
PiA+IC0tLSBhL2FyY2gvcG93ZXJwYy9zeXNkZXYvZnNsX21zaS5oDQo+ID4gKysrIGIvYXJjaC9w
b3dlcnBjL3N5c2Rldi9mc2xfbXNpLmgNCj4gPiBAQCAtMjksMTIgKzI5LDE5IEBAIHN0cnVjdCBm
c2xfbXNpIHsNCj4gPiAgCXN0cnVjdCBpcnFfZG9tYWluICppcnFob3N0Ow0KPiA+DQo+ID4gIAl1
bnNpZ25lZCBsb25nIGNhc2NhZGVfaXJxOw0KPiA+IC0NCj4gPiAtCXUzMiBtc2lpcl9vZmZzZXQ7
IC8qIE9mZnNldCBvZiBNU0lJUiwgcmVsYXRpdmUgdG8gc3RhcnQgb2YgQ0NTUiAqLw0KPiA+ICsJ
ZG1hX2FkZHJfdCBtc2lpcjsgLyogTVNJSVIgQWRkcmVzcyBpbiBDQ1NSICovDQo+IA0KPiBBcmUg
eW91IHN1cmUgZG1hX2FkZHJfdCBpcyByaWdodCBoZXJlLCB2ZXJzdXMgcGh5c19hZGRyX3Q/ICBJ
dCBpbXBsaWVzIHRoYXQgaXQncw0KPiB0aGUgb3V0cHV0IG9mIHRoZSBETUEgQVBJLCBidXQgSSBk
b24ndCB0aGluayB0aGUgRE1BIEFQSSBpcyB1c2VkIGluIHRoZSBNU0kNCj4gZHJpdmVyLiAgUGVy
aGFwcyBpdCBzaG91bGQgYmUsIGJ1dCB3ZSBzdGlsbCB3YW50IHRoZSByYXcgcGh5c2ljYWwgYWRk
cmVzcyB0bw0KPiBwYXNzIG9uIHRvIFZGSU8uDQoNCkxvb2tpbmcgdGhyb3VnaCB0aGUgY29udmVy
c2F0aW9uIEkgd2lsbCBtYWtlIHRoaXMgcGh5c19hZGRyX3QNCg0KPiANCj4gPiAgCXZvaWQgX19p
b21lbSAqbXNpX3JlZ3M7DQo+ID4gIAl1MzIgZmVhdHVyZTsNCj4gPiAgCWludCBtc2lfdmlycXNb
TlJfTVNJX1JFR107DQo+ID4NCj4gPiArCS8qDQo+ID4gKwkgKiBEdXJpbmcgcHJvYmUgZWFjaCBi
YW5rIGlzIGFzc2lnbmVkIGEgaW5kZXggbnVtYmVyLg0KPiA+ICsJICogaW5kZXggbnVtYmVyIHJh
bmdlcyBmcm9tIDAgdG8gMl4zMi4NCj4gPiArCSAqIEV4YW1wbGUgIE1TSSBiYW5rIDEgPSAwDQo+
ID4gKwkgKiBNU0kgYmFuayAyID0gMSwgYW5kIHNvIG9uLg0KPiA+ICsJICovDQo+ID4gKwlpbnQg
YmFua19pbmRleDsNCj4gDQo+IDJeMzIgZG9lc24ndCBmaXQgaW4gImludCIgKG5vciBkb2VzIDJe
MzIgLSAxKS4NCg0KUmlnaHQgOigNCg0KPiANCj4gSnVzdCBzYXkgdGhhdCBpbmRpY2VzIHN0YXJ0
IGF0IDAuDQoNCldpbGwgY29ycmVjdCB0aGlzDQoNClRoYW5rcw0KLUJoYXJhdA0KDQo+IA0KPiAt
U2NvdHQNCj4gDQoNCg==
^ permalink raw reply
* Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts
From: Anshuman Khandual @ 2013-10-09 4:46 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mikey
In-Reply-To: <20131009012130.GA23780@concordia>
On 10/09/2013 06:51 AM, Michael Ellerman wrote:
> On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
>> On 10/08/2013 09:51 AM, Michael Ellerman wrote:
>>> On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
>>>> 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.
>>>
>>> Patch looks good.
>>>
>>> But it reminds me I have an item in my TODO list:
>>> - "Why can't config_bhrb() be done in compute_mmcr()" ?
>>>
>>
>> compute_mmcr() function deals with generic MMCR* configs for normal PMU
>> events. Even if BHRB config touches MMCRA register, it's configuration
>> does not interfere with the PMU config for general events. So its best
>> to keep them separate.
>
> I'm unconvinced. If they'd been together to begin with this bug never
> would have happened.
This is an ordering of configuration problem. Putting them together in the
same function does not rule out the chances of this ordering problem. Could
you please kindly explain how this could have been avoided ?
>
> And there's the added overhead of another indirect function call.
>
This overhead should be minimal given the fact that we already call so
many PMU specific indirect calls. BHRB is a different part of the PMU
hardware, so a separate call for this purpose is not a bad idea. AFAIK,
X86 does that too for LBR. But yes it is debatable.
>> Besides, we can always look at these code consolidation
>> issues in future.
>
> The future is now.
What I meant was functional correctness has always more priority than
code consolidation efforts. Yes I will look into this after book3s
software branch filtering code has been merged.
>
>> But this patch solves a problem which is happening right now.
>
> Sure, I'm not saying we shouldn't merge it as a fix. But I think we
> should do the cleanup to move it into compute_mmcr() for 3.13.
yeah that sounds reasonable.
Regards
Anshuman
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Benjamin Herrenschmidt @ 2013-10-09 4:24 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86,
Alexander Gordeev, iss_storagedev, linux-driver, Tejun Heo,
Bjorn Helgaas, Dan Williams, Jon Mason, Ingo Molnar,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Martin Schwidefsky, linux390, linuxppc-dev
In-Reply-To: <5254D397.9030307@zytor.com>
On Tue, 2013-10-08 at 20:55 -0700, H. Peter Anvin wrote:
> Why not add a minimum number to pci_enable_msix(), i.e.:
>
> pci_enable_msix(pdev, msix_entries, nvec, minvec)
>
> ... which means "nvec" is the number of interrupts *requested*, and
> "minvec" is the minimum acceptable number (otherwise fail).
Which is exactly what Ben (the other Ben :-) suggested and that I
supports...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Add a debugfs file to read the firmware console
From: Benjamin Herrenschmidt @ 2013-10-09 4:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20131009032314.GD23780@concordia>
On Wed, 2013-10-09 at 14:23 +1100, Michael Ellerman wrote:
> On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote:
> > With OPALv3, the firmware can provide the address of it's internal console
> > to Linux, which we can then display using debugfs. This is handy for
> > diagnostics and debugging.
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> >
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index 2911abe..10d7894 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -17,6 +17,8 @@
> > #include <linux/interrupt.h>
> > #include <linux/notifier.h>
> > #include <linux/slab.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/uaccess.h>
> > #include <asm/opal.h>
> > #include <asm/firmware.h>
> >
> > @@ -27,6 +29,21 @@ struct opal {
> > u64 entry;
> > } opal;
> >
> > +/* OPAL in-memory console */
>
> It might be nice to point out that the format of the struct is defined
> by OPAL and must be in sync with what OPAL is using.
Yes, we could move the structure definition to opal.h...
> > +struct memcons {
> > + uint64_t magic;
>
> u64 ?
Who cares ? Especially if it goes into opal.h it should stick to the
types used in that file.
> > +#define MEMCONS_MAGIC 0x6630696567726173
> > + uint64_t obuf_phys;
> > + uint64_t ibuf_phys;
> > + uint32_t obuf_size;
> > + uint32_t ibuf_size;
> > + uint32_t out_pos;
> > +#define MEMCONS_OUT_POS_WRAP 0x80000000u
> > +#define MEMCONS_OUT_POS_MASK 0x00ffffffu
>
> Where does this come from?
My a** :-) I made it up as I wrote the OPAL side one, why ?
> > + uint32_t in_prod;
> > + uint32_t in_cons;
> > +};
>
> Should it be packed?
Nope, no need. It's all nice and naturally aligned.
> > @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data)
> > return IRQ_HANDLED;
> > }
> >
> > +#ifdef CONFIG_DEBUG_FS
> > +static ssize_t opal_memcons_read(struct file *file, char __user *to,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct memcons *mc = file->private_data;
> > + size_t available, ret, chunk0, chunk1, lcount;
> > + const char *start, *conbuf = __va(mc->obuf_phys);
> > + loff_t opos, pos;
> > +
> > + /*
> > + * Find out how much is in the buffer. If it has wrapped
> > + * the whole buffer, else just the beginning. It has wrapped
> > + * if the next character is not \0
> > + */
> > + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) {
> > + available = mc->obuf_size;
> > + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK;
> > + start = conbuf + chunk1;
> > + chunk0 = mc->obuf_size - chunk1;
> > + } else {
> > + available = mc->out_pos;
> > + start = conbuf;
> > + chunk0 = available;
> > + chunk1 = 0;
> > + }
>
> Surely simple_read_from_buffer() could make some of this simpler?
If you can find a way to make it deal with a ring buffer...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: H. Peter Anvin @ 2013-10-09 3:55 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: <cover.1380703262.git.agordeev@redhat.com>
On 10/02/2013 03:29 AM, Alexander Gordeev wrote:
>
> As result, device drivers will cease to use the overcomplicated
> repeated fallbacks technique and resort to a straightforward
> pattern - determine the number of MSI/MSI-X interrupts required
> before calling pci_enable_msi_block() and pci_enable_msix()
> interfaces:
>
>
> rc = pci_msix_table_size(adapter->pdev);
> if (rc < 0)
> return rc;
>
> nvec = min(nvec, rc);
> if (nvec < FOO_DRIVER_MINIMUM_NVEC) {
> return -ENOSPC;
>
> for (i = 0; i < nvec; i++)
> adapter->msix_entries[i].entry = i;
>
> rc = pci_enable_msix(adapter->pdev,
> adapter->msix_entries, nvec);
> return rc;
>
Why not add a minimum number to pci_enable_msix(), i.e.:
pci_enable_msix(pdev, msix_entries, nvec, minvec)
... which means "nvec" is the number of interrupts *requested*, and
"minvec" is the minimum acceptable number (otherwise fail).
-hpa
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Add a debugfs file to read the firmware console
From: Michael Ellerman @ 2013-10-09 3:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1381218400.645.204.camel@pasglop>
On Tue, Oct 08, 2013 at 06:46:40PM +1100, Benjamin Herrenschmidt wrote:
> With OPALv3, the firmware can provide the address of it's internal console
> to Linux, which we can then display using debugfs. This is handy for
> diagnostics and debugging.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 2911abe..10d7894 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -17,6 +17,8 @@
> #include <linux/interrupt.h>
> #include <linux/notifier.h>
> #include <linux/slab.h>
> +#include <linux/debugfs.h>
> +#include <linux/uaccess.h>
> #include <asm/opal.h>
> #include <asm/firmware.h>
>
> @@ -27,6 +29,21 @@ struct opal {
> u64 entry;
> } opal;
>
> +/* OPAL in-memory console */
It might be nice to point out that the format of the struct is defined
by OPAL and must be in sync with what OPAL is using.
> +struct memcons {
> + uint64_t magic;
u64 ?
> +#define MEMCONS_MAGIC 0x6630696567726173
> + uint64_t obuf_phys;
> + uint64_t ibuf_phys;
> + uint32_t obuf_size;
> + uint32_t ibuf_size;
> + uint32_t out_pos;
> +#define MEMCONS_OUT_POS_WRAP 0x80000000u
> +#define MEMCONS_OUT_POS_MASK 0x00ffffffu
Where does this come from?
> + uint32_t in_prod;
> + uint32_t in_cons;
> +};
Should it be packed?
> @@ -369,6 +386,90 @@ static irqreturn_t opal_interrupt(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +#ifdef CONFIG_DEBUG_FS
> +static ssize_t opal_memcons_read(struct file *file, char __user *to,
> + size_t count, loff_t *ppos)
> +{
> + struct memcons *mc = file->private_data;
> + size_t available, ret, chunk0, chunk1, lcount;
> + const char *start, *conbuf = __va(mc->obuf_phys);
> + loff_t opos, pos;
> +
> + /*
> + * Find out how much is in the buffer. If it has wrapped
> + * the whole buffer, else just the beginning. It has wrapped
> + * if the next character is not \0
> + */
> + if (mc->out_pos & MEMCONS_OUT_POS_WRAP) {
> + available = mc->obuf_size;
> + chunk1 = mc->out_pos & MEMCONS_OUT_POS_MASK;
> + start = conbuf + chunk1;
> + chunk0 = mc->obuf_size - chunk1;
> + } else {
> + available = mc->out_pos;
> + start = conbuf;
> + chunk0 = available;
> + chunk1 = 0;
> + }
Surely simple_read_from_buffer() could make some of this simpler?
cheers
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Mark Lord @ 2013-10-09 1:55 UTC (permalink / raw)
To: Alexander Gordeev, linux-kernel
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Linus Torvalds,
Ingo Molnar, linux-pci, iss_storagedev, linux-driver, Tejun Heo,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, Ralf Baechle, e1000-devel,
Martin Schwidefsky, linux390, linuxppc-dev
In-Reply-To: <cover.1380703262.git.agordeev@redhat.com>
On 13-10-02 06:29 AM, 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.
Rather than silently break dozens of drivers in mysterious ways,
please invent new function names for the replacements to the
existing pci_enable_msix() and pci_enable_msi_block() functions.
That way, both in-tree and out-of-tree drivers will notice the API change,
rather than having it go unseen and just failing for unknown reasons.
Thanks.
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Michael Ellerman @ 2013-10-09 1:34 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: <20131008073301.GC10669@dhcp-26-207.brq.redhat.com>
On Tue, Oct 08, 2013 at 09:33:02AM +0200, Alexander Gordeev wrote:
> On Tue, Oct 08, 2013 at 03:33:30PM +1100, Michael Ellerman wrote:
> > On Wed, Oct 02, 2013 at 12:29:04PM +0200, Alexander Gordeev wrote:
> > > This technique proved to be confusing and error-prone. Vast share
> > > of device drivers simply fail to follow the described guidelines.
> >
> > To clarify "Vast share of device drivers":
> >
> > - 58 drivers call pci_enable_msix()
> > - 24 try a single allocation and then fallback to MSI/LSI
> > - 19 use the loop style allocation as above
> > - 14 try an allocation, and if it fails retry once
> > - 1 incorrectly continues when pci_enable_msix() returns > 0
> >
> > So 33 drivers (> 50%) successfully make use of the "confusing and
> > error-prone" return value.
>
> Ok, you caught me - 'vast share' is incorrect and is a subject to
> rewording. But out of 19/58 how many drivers tested fallbacks on the
> real hardware? IOW, which drivers are affected by the pSeries quota?
It's not 19/58, it's 33/58.
As to how many we care about on powerpc I can't say, so you have a point
there. But I still think the interface is not actually that terrible.
cheers
^ permalink raw reply
* Re: [PATCH 5/9][v5] powerpc: implement is_instr_load_store().
From: Michael Ellerman @ 2013-10-09 1:27 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linux-kernel, Stephane Eranian, linuxppc-dev, Paul Mackerras,
Arnaldo Carvalho de Melo, Anshuman Khandual
In-Reply-To: <1381280599.15322.1.camel@concordia>
On Wed, Oct 09, 2013 at 12:03:19PM +1100, Michael Ellerman wrote:
> On Tue, 2013-10-08 at 12:31 -0700, Sukadev Bhattiprolu wrote:
> > Michael Ellerman [michael@ellerman.id.au] wrote:
> > | bool is_load_store(int ext_opcode)
> > | {
> > | upper = ext_opcode >> 5;
> > | lower = ext_opcode & 0x1f;
> > |
> > | /* Short circuit as many misses as we can */
> > | if (lower < 3 || lower > 23)
> > | return false;
> >
> > I see some loads/stores like these which are not covered by
> > the above check. Is it ok to ignore them ?
> >
> > lower == 29: ldepx, stdepx, eviddepx, evstddepx
> >
> > lower == 31: lwepx, lbepx, lfdepx, stfdepx,
>
> Those are the external process ID instructions, which I've never heard
> of anyone using, I think we can ignore them.
>
> > Looking through the opcode maps, I also see these for primary
> > op code 4:
> >
> > evldd, evlddx, evldwx, evldw, evldh, evldhx.
> >
> > Should we include those also ?
>
> Yes I think so. I didn't check any of the other opcodes for you.
Paul points out these are for the SPE extension, which we also don't
care about. So ignore those as well.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts
From: Michael Ellerman @ 2013-10-09 1:21 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: linuxppc-dev, mikey
In-Reply-To: <5253B26E.3020800@linux.vnet.ibm.com>
On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
> On 10/08/2013 09:51 AM, Michael Ellerman wrote:
> > On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
> >> 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.
> >
> > Patch looks good.
> >
> > But it reminds me I have an item in my TODO list:
> > - "Why can't config_bhrb() be done in compute_mmcr()" ?
> >
>
> compute_mmcr() function deals with generic MMCR* configs for normal PMU
> events. Even if BHRB config touches MMCRA register, it's configuration
> does not interfere with the PMU config for general events. So its best
> to keep them separate.
I'm unconvinced. If they'd been together to begin with this bug never
would have happened.
And there's the added overhead of another indirect function call.
> Besides, we can always look at these code consolidation
> issues in future.
The future is now.
> But this patch solves a problem which is happening right now.
Sure, I'm not saying we shouldn't merge it as a fix. But I think we
should do the cleanup to move it into compute_mmcr() for 3.13.
cheers
^ permalink raw reply
* Re: [PATCH 5/9][v5] powerpc: implement is_instr_load_store().
From: Michael Ellerman @ 2013-10-09 1:03 UTC (permalink / raw)
To: Sukadev Bhattiprolu
Cc: linux-kernel, Stephane Eranian, linuxppc-dev, Paul Mackerras,
Arnaldo Carvalho de Melo, Anshuman Khandual
In-Reply-To: <20131008193117.GA699@us.ibm.com>
On Tue, 2013-10-08 at 12:31 -0700, Sukadev Bhattiprolu wrote:
> Michael Ellerman [michael@ellerman.id.au] wrote:
> | bool is_load_store(int ext_opcode)
> | {
> | upper = ext_opcode >> 5;
> | lower = ext_opcode & 0x1f;
> |
> | /* Short circuit as many misses as we can */
> | if (lower < 3 || lower > 23)
> | return false;
>
> I see some loads/stores like these which are not covered by
> the above check. Is it ok to ignore them ?
>
> lower == 29: ldepx, stdepx, eviddepx, evstddepx
>
> lower == 31: lwepx, lbepx, lfdepx, stfdepx,
Those are the external process ID instructions, which I've never heard
of anyone using, I think we can ignore them.
> Looking through the opcode maps, I also see these for primary
> op code 4:
>
> evldd, evlddx, evldwx, evldw, evldh, evldhx.
>
> Should we include those also ?
Yes I think so. I didn't check any of the other opcodes for you.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: Reduce panic timeout from 180s to 10s
From: Anton Blanchard @ 2013-10-08 23:56 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, paulus
In-Reply-To: <1381269153.7979.281.camel@snotra.buserror.net>
> > > We made this change to pseries in 2011 and I think it makes
> > > sense to do the same on powernv.
> >
> > I'd vote we set it to 10s for all 64-bit machines in
> > arch/powerpc/kernel/setup_64.c.
>
> Why is 64-bit relevant? And wouldn't such a short delay be a problem
> if the crash is displayed on a monitor?
That is why we made it pseries specific in the past. Almost all our
boxes are on a virtual console and the 3 minutes of pausing just hurt
our uptimes.
If other platform maintainers prefer to keep the 3 minute pause, then
we just change the PowerNV platform.
Anton
^ permalink raw reply
* Re: [PATCH] powerpc: fix e500 SPE float to integer and fixed-point conversions
From: Joseph S. Myers @ 2013-10-08 23:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Shan Hai, linux-kernel
In-Reply-To: <Pine.LNX.4.64.1310082337080.23637@digraph.polyomino.org.uk>
On Tue, 8 Oct 2013, Joseph S. Myers wrote:
> I'll send as a followup the testcase I used for verifying that the
> instructions (other than the theoretical conversions to 64-bit
> integers) produce the correct results. In addition, this has been
> tested with the glibc testsuite (with the e500 port as posted at
> <https://sourceware.org/ml/libc-alpha/2013-10/msg00195.html>, where it
> improves the libm test results.
Here is that testcase.
#include <stdio.h>
#include <stdlib.h>
#define INFF __builtin_inff ()
#define INFD __builtin_inf ()
#define NANF __builtin_nanf ("")
#define NAND __builtin_nan ("")
/* e500 rounding modes: 0 = nearest, 1 = zero, 2 = up, 3 = down. */
static inline void
set_rm (unsigned int mode)
{
unsigned int spefscr;
asm volatile ("mfspefscr %0" : "=r" (spefscr));
spefscr = (spefscr & ~3) | mode;
asm volatile ("mtspefscr %0" : : "r" (spefscr));
}
static int success_count, failure_count;
struct float_test_data
{
float input;
unsigned int expected[4];
};
struct double_test_data
{
double input;
unsigned int expected[4];
};
typedef float vfloat __attribute__ ((vector_size (8)));
typedef unsigned int vuint __attribute__ ((vector_size (8)));
union vfloat_union
{
vfloat vf;
float f[2];
};
union vuint_union
{
vuint vui;
unsigned int ui[2];
};
#define T(A, B, C, D, E) { (A), { (B), (C), (D), (E) } }
#define TZ(A, B) T (A, B, B, B, B)
static void
check_result (const char *insn, double input, unsigned int rm,
unsigned int expected, unsigned int res)
{
if (res == expected)
success_count++;
else
{
failure_count++;
printf ("%s %a mode %u expected 0x%x (%d) got 0x%x (%d)\n",
insn, input, rm, expected, (int) expected, res, (int) res);
}
}
#define RUN_FLOAT_TESTS(INSN) \
static void \
test_##INSN (void) \
{ \
size_t i; \
for (i = 0; \
i < sizeof (INSN##_test_data) / sizeof (INSN##_test_data[0]); \
i++) \
{ \
unsigned int rm; \
for (rm = 0; rm <= 3; rm++) \
{ \
set_rm (rm); \
unsigned int res; \
asm volatile (#INSN " %0, %1" \
: "=&r" (res) \
: "r" (INSN##_test_data[i].input)); \
check_result (#INSN, INSN##_test_data[i].input, rm, \
INSN##_test_data[i].expected[rm], res); \
} \
} \
}
#define RUN_VFLOAT_TESTS(INSN, TINSN) \
static void \
test_##INSN (void) \
{ \
size_t i; \
for (i = 0; \
i < sizeof (TINSN##_test_data) / sizeof (TINSN##_test_data[0]); \
i++) \
{ \
unsigned int rm; \
for (rm = 0; rm <= 3; rm++) \
{ \
set_rm (rm); \
union vfloat_union varg; \
union vuint_union vres; \
varg.f[0] = TINSN##_test_data[i].input; \
varg.f[1] = 0; \
asm volatile (#INSN " %0, %1" \
: "=&r" (vres.vui) \
: "r" (varg.vf)); \
check_result (#INSN " (high)", TINSN##_test_data[i].input, \
rm, TINSN##_test_data[i].expected[rm], \
vres.ui[0]); \
check_result (#INSN " (low 0)", TINSN##_test_data[i].input, \
rm, 0, vres.ui[1]); \
varg.f[1] = TINSN##_test_data[i].input; \
varg.f[0] = 0; \
asm volatile (#INSN " %0, %1" \
: "=&r" (vres.vui) \
: "r" (varg.vf)); \
check_result (#INSN " (low)", TINSN##_test_data[i].input, \
rm, TINSN##_test_data[i].expected[rm], \
vres.ui[1]); \
check_result (#INSN " (high 0)", TINSN##_test_data[i].input, \
rm, 0, vres.ui[0]); \
} \
} \
}
static const struct float_test_data efsctsiz_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0x7fffffff),
TZ (0x1.fffffep127f, 0x7fffffff),
TZ (0x1p31f, 0x7fffffff),
TZ (0x1.fffffep30f, 0x7fffff80),
TZ (1.6f, 1),
TZ (1.5f, 1),
TZ (1.4f, 1),
TZ (1.0f, 1),
TZ (0.6f, 0),
TZ (0.5f, 0),
TZ (0.4f, 0),
TZ (0x1p-149f, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
TZ (-0x1p-149f, 0),
TZ (-0.4f, 0),
TZ (-0.5f, 0),
TZ (-0.6f, 0),
TZ (-1.0f, -1),
TZ (-1.4f, -1),
TZ (-1.5f, -1),
TZ (-1.6f, -1),
TZ (-0x1.fffffep30f, 0x80000080),
TZ (-0x1p31f, 0x80000000),
TZ (-0x1.fffffep127f, 0x80000000),
TZ (-INFF, 0x80000000),
TZ (-NANF, 0),
};
static const struct float_test_data efsctuiz_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0xffffffff),
TZ (0x1.fffffep127f, 0xffffffff),
TZ (0x1p32f, 0xffffffff),
TZ (0x1.fffffep31f, 0xffffff00),
TZ (1.6f, 1),
TZ (1.5f, 1),
TZ (1.4f, 1),
TZ (1.0f, 1),
TZ (0.6f, 0),
TZ (0.5f, 0),
TZ (0.4f, 0),
TZ (0x1p-149f, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
TZ (-0x1p-149f, 0),
TZ (-0.4f, 0),
TZ (-0.5f, 0),
TZ (-0.6f, 0),
TZ (-1.0f, 0),
TZ (-1.4f, 0),
TZ (-1.5f, 0),
TZ (-1.6f, 0),
TZ (-0x1.fffffep127f, 0),
TZ (-INFF, 0),
TZ (-NANF, 0),
};
static const struct double_test_data efdctsiz_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0x7fffffff),
TZ (0x1.fffffffffffffp1023, 0x7fffffff),
TZ (0x1.0000000000001p31, 0x7fffffff),
TZ (0x1p31, 0x7fffffff),
TZ (0x1.fffffffffffffp30, 0x7fffffff),
TZ (0x1.fffffffcp30, 0x7fffffff),
TZ (1.6, 1),
TZ (1.5, 1),
TZ (1.4, 1),
TZ (1.0, 1),
TZ (0.6, 0),
TZ (0.5, 0),
TZ (0.4, 0),
TZ (0x1p-1074, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
TZ (-0x1p-1074, 0),
TZ (-0.4, 0),
TZ (-0.5, 0),
TZ (-0.6, 0),
TZ (-1.0, -1),
TZ (-1.4, -1),
TZ (-1.5, -1),
TZ (-1.6, -1),
TZ (-0x1.fffffffcp30, 0x80000001),
TZ (-0x1.fffffffffffffp30, 0x80000001),
TZ (-0x1p31, 0x80000000),
TZ (-0x1.0000000000001p31, 0x80000000),
TZ (-0x1.fffffffffffffp1023, 0x80000000),
TZ (-INFD, 0x80000000),
TZ (-NAND, 0),
};
static const struct double_test_data efdctuiz_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0xffffffff),
TZ (0x1.fffffffffffffp1023, 0xffffffff),
TZ (0x1.0000000000001p32, 0xffffffff),
TZ (0x1p32, 0xffffffff),
TZ (0x1.fffffffffffffp31, 0xffffffff),
TZ (1.6, 1),
TZ (1.5, 1),
TZ (1.4, 1),
TZ (1.0, 1),
TZ (0.6, 0),
TZ (0.5, 0),
TZ (0.4, 0),
TZ (0x1p-1074, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
TZ (-0x1p-1074, 0),
TZ (-0.4, 0),
TZ (-0.5, 0),
TZ (-0.6, 0),
TZ (-1.0, 0),
TZ (-1.4, 0),
TZ (-1.5, 0),
TZ (-1.6, 0),
TZ (-0x1.fffffffffffffp1023, 0),
TZ (-INFD, 0),
TZ (-NAND, 0),
};
static const struct float_test_data efsctsi_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0x7fffffff),
TZ (0x1.fffffep127f, 0x7fffffff),
TZ (0x1p31f, 0x7fffffff),
TZ (0x1.fffffep30f, 0x7fffff80),
T (1.6f, 2, 1, 2, 1),
T (1.5f, 2, 1, 2, 1),
T (1.4f, 1, 1, 2, 1),
TZ (1.0f, 1),
T (0.6f, 1, 0, 1, 0),
T (0.5f, 0, 0, 1, 0),
T (0.4f, 0, 0, 1, 0),
T (0x1p-149f, 0, 0, 1, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
T (-0x1p-149f, 0, 0, 0, -1),
T (-0.4f, 0, 0, 0, -1),
T (-0.5f, 0, 0, 0, -1),
T (-0.6f, -1, 0, 0, -1),
TZ (-1.0f, -1),
T (-1.4f, -1, -1, -1, -2),
T (-1.5f, -2, -1, -1, -2),
T (-1.6f, -2, -1, -1, -2),
TZ (-0x1.fffffep30f, 0x80000080),
TZ (-0x1p31f, 0x80000000),
TZ (-0x1.fffffep127f, 0x80000000),
TZ (-INFF, 0x80000000),
TZ (-NANF, 0),
};
static const struct float_test_data efsctui_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0xffffffff),
TZ (0x1.fffffep127f, 0xffffffff),
TZ (0x1p32f, 0xffffffff),
TZ (0x1.fffffep31f, 0xffffff00),
T (1.6f, 2, 1, 2, 1),
T (1.5f, 2, 1, 2, 1),
T (1.4f, 1, 1, 2, 1),
TZ (1.0f, 1),
T (0.6f, 1, 0, 1, 0),
T (0.5f, 0, 0, 1, 0),
T (0.4f, 0, 0, 1, 0),
T (0x1p-149f, 0, 0, 1, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
TZ (-0x1p-149f, 0),
TZ (-0.4f, 0),
TZ (-0.5f, 0),
TZ (-0.6f, 0),
TZ (-1.0f, 0),
TZ (-1.4f, 0),
TZ (-1.5f, 0),
TZ (-1.6f, 0),
TZ (-0x1.fffffep127f, 0),
TZ (-INFF, 0),
TZ (-NANF, 0),
};
static const struct double_test_data efdctsi_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0x7fffffff),
TZ (0x1.fffffffffffffp1023, 0x7fffffff),
TZ (0x1.0000000000001p31, 0x7fffffff),
TZ (0x1p31, 0x7fffffff),
TZ (0x1.fffffffffffffp30, 0x7fffffff),
TZ (0x1.fffffffcp30, 0x7fffffff),
T (1.6, 2, 1, 2, 1),
T (1.5, 2, 1, 2, 1),
T (1.4, 1, 1, 2, 1),
TZ (1.0, 1),
T (0.6, 1, 0, 1, 0),
T (0.5, 0, 0, 1, 0),
T (0.4, 0, 0, 1, 0),
T (0x1p-1074, 0, 0, 1, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
T (-0x1p-1074, 0, 0, 0, -1),
T (-0.4, 0, 0, 0, -1),
T (-0.5, 0, 0, 0, -1),
T (-0.6, -1, 0, 0, -1),
TZ (-1.0, -1),
T (-1.4, -1, -1, -1, -2),
T (-1.5, -2, -1, -1, -2),
T (-1.6, -2, -1, -1, -2),
TZ (-0x1.fffffffcp30, 0x80000001),
T (-0x1.fffffffffffffp30, 0x80000000, 0x80000001, 0x80000001, 0x80000000),
TZ (-0x1p31, 0x80000000),
TZ (-0x1.0000000000001p31, 0x80000000),
TZ (-0x1.fffffffffffffp1023, 0x80000000),
TZ (-INFD, 0x80000000),
TZ (-NAND, 0),
};
static const struct double_test_data efdctui_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0xffffffff),
TZ (0x1.fffffffffffffp1023, 0xffffffff),
TZ (0x1.0000000000001p32, 0xffffffff),
TZ (0x1p32, 0xffffffff),
TZ (0x1.fffffffffffffp31, 0xffffffff),
T (1.6, 2, 1, 2, 1),
T (1.5, 2, 1, 2, 1),
T (1.4, 1, 1, 2, 1),
TZ (1.0, 1),
T (0.6, 1, 0, 1, 0),
T (0.5, 0, 0, 1, 0),
T (0.4, 0, 0, 1, 0),
T (0x1p-1074, 0, 0, 1, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
TZ (-0x1p-1074, 0),
TZ (-0.4, 0),
TZ (-0.5, 0),
TZ (-0.6, 0),
TZ (-1.0, 0),
TZ (-1.4, 0),
TZ (-1.5, 0),
TZ (-1.6, 0),
TZ (-0x1.fffffffffffffp1023, 0),
TZ (-INFD, 0),
TZ (-NAND, 0),
};
static const struct float_test_data efsctsf_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0x7fffffff),
TZ (0x1.fffffep127f, 0x7fffffff),
TZ (0x1.000002p0f, 0x7fffffff),
TZ (1.0f, 0x7fffffff),
TZ (0x1.fffffep-1f, 0x7fffff80),
TZ (0xffffff.0p-31f, 0xffffff),
T (0x7fffff.8p-31f, 0x800000, 0x7fffff, 0x800000, 0x7fffff),
T (0x7ffffe.8p-31f, 0x7ffffe, 0x7ffffe, 0x7fffff, 0x7ffffe),
T (0x1.9p-31f, 2, 1, 2, 1),
T (0x1.8p-31f, 2, 1, 2, 1),
T (0x1.7p-31f, 1, 1, 2, 1),
TZ (0x1p-31f, 1),
T (0x0.9p-31f, 1, 0, 1, 0),
T (0x0.8p-31f, 0, 0, 1, 0),
T (0x0.7p-31f, 0, 0, 1, 0),
T (0x1p-149f, 0, 0, 1, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
T (-0x1p-149f, 0, 0, 0, -1),
T (-0x0.7p-31f, 0, 0, 0, -1),
T (-0x0.8p-31f, 0, 0, 0, -1),
T (-0x0.9p-31f, -1, 0, 0, -1),
TZ (-0x1p-31f, -1),
T (-0x1.7p-31f, -1, -1, -1, -2),
T (-0x1.8p-31f, -2, -1, -1, -2),
T (-0x1.9p-31f, -2, -1, -1, -2),
T (-0x7ffffe.8p-31f, -0x7ffffe, -0x7ffffe, -0x7ffffe, -0x7fffff),
T (-0x7fffff.8p-31f, -0x800000, -0x7fffff, -0x7fffff, -0x800000),
TZ (-0xffffff.0p-31f, -0xffffff),
TZ (-0x1.fffffep-1f, -0x7fffff80),
TZ (-1.0f, 0x80000000),
TZ (-0x1.000002p0f, 0x80000000),
TZ (-0x1.fffffep127f, 0x80000000),
TZ (-INFF, 0x80000000),
TZ (-NANF, 0),
};
static const struct float_test_data efsctuf_test_data[] =
{
TZ (NANF, 0),
TZ (INFF, 0xffffffff),
TZ (0x1.fffffep127f, 0xffffffff),
TZ (0x1.000002p0f, 0xffffffff),
TZ (1.0f, 0xffffffff),
TZ (0x1.fffffep-1f, 0xffffff00),
TZ (0xffffff.0p-32f, 0xffffff),
T (0x7fffff.8p-32f, 0x800000, 0x7fffff, 0x800000, 0x7fffff),
T (0x7ffffe.8p-32f, 0x7ffffe, 0x7ffffe, 0x7fffff, 0x7ffffe),
T (0x1.9p-32f, 2, 1, 2, 1),
T (0x1.8p-32f, 2, 1, 2, 1),
T (0x1.7p-32f, 1, 1, 2, 1),
TZ (0x1p-32f, 1),
T (0x0.9p-32f, 1, 0, 1, 0),
T (0x0.8p-32f, 0, 0, 1, 0),
T (0x0.7p-32f, 0, 0, 1, 0),
T (0x1p-149f, 0, 0, 1, 0),
TZ (0.0f, 0),
TZ (-0.0f, 0),
TZ (-0x1p-149f, 0),
TZ (-0x0.7p-32f, 0),
TZ (-0x0.8p-32f, 0),
TZ (-0x0.9p-32f, 0),
TZ (-0x1p-32f, 0),
TZ (-0x1.7p-32f, 0),
TZ (-0x1.8p-32f, 0),
TZ (-0x1.9p-32f, 0),
TZ (-0x7ffffe.8p-32f, 0),
TZ (-0x7fffff.8p-32f, 0),
TZ (-0xffffff.0p-32f, 0),
TZ (-0x1.fffffep-1f, 0),
TZ (-1.0f, 0),
TZ (-0x1.000002p0f, 0),
TZ (-0x1.fffffep127f, 0),
TZ (-INFF, 0),
TZ (-NANF, 0),
};
static const struct double_test_data efdctsf_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0x7fffffff),
TZ (0x1.fffffffffffffp1023, 0x7fffffff),
TZ (0x1.0000000000001p0, 0x7fffffff),
TZ (1.0, 0x7fffffff),
TZ (0x7fffffffp-31, 0x7fffffff),
T (0x7fffff.8p-31, 0x800000, 0x7fffff, 0x800000, 0x7fffff),
T (0x7ffffe.8p-31, 0x7ffffe, 0x7ffffe, 0x7fffff, 0x7ffffe),
T (0x1.9p-31, 2, 1, 2, 1),
T (0x1.8p-31, 2, 1, 2, 1),
T (0x1.7p-31, 1, 1, 2, 1),
TZ (0x1p-31, 1),
T (0x0.9p-31, 1, 0, 1, 0),
T (0x0.8p-31, 0, 0, 1, 0),
T (0x0.7p-31, 0, 0, 1, 0),
T (0x1p-1074, 0, 0, 1, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
T (-0x1p-1074, 0, 0, 0, -1),
T (-0x0.7p-31, 0, 0, 0, -1),
T (-0x0.8p-31, 0, 0, 0, -1),
T (-0x0.9p-31, -1, 0, 0, -1),
TZ (-0x1p-31, -1),
T (-0x1.7p-31, -1, -1, -1, -2),
T (-0x1.8p-31, -2, -1, -1, -2),
T (-0x1.9p-31, -2, -1, -1, -2),
T (-0x7ffffe.8p-31, -0x7ffffe, -0x7ffffe, -0x7ffffe, -0x7fffff),
T (-0x7fffff.8p-31, -0x800000, -0x7fffff, -0x7fffff, -0x800000),
TZ (-0x7fffffffp-31, -0x7fffffff),
TZ (-1.0, 0x80000000),
TZ (-0x1.0000000000001p0, 0x80000000),
TZ (-0x1.fffffffffffffp1023, 0x80000000),
TZ (-INFD, 0x80000000),
TZ (-NAND, 0),
};
static const struct double_test_data efdctuf_test_data[] =
{
TZ (NAND, 0),
TZ (INFD, 0xffffffff),
TZ (0x1.fffffffffffffp1023, 0xffffffff),
TZ (0x1.0000000000001p0, 0xffffffff),
TZ (1.0, 0xffffffff),
TZ (0xffffffffp-32, 0xffffffff),
T (0xfffffffe.9p-32, 0xffffffff, 0xfffffffe, 0xffffffff, 0xfffffffe),
T (0xfffffffe.8p-32, 0xfffffffe, 0xfffffffe, 0xffffffff, 0xfffffffe),
T (0xfffffffe.7p-32, 0xfffffffe, 0xfffffffe, 0xffffffff, 0xfffffffe),
T (0xfffffffd.9p-32, 0xfffffffe, 0xfffffffd, 0xfffffffe, 0xfffffffd),
T (0xfffffffd.8p-32, 0xfffffffe, 0xfffffffd, 0xfffffffe, 0xfffffffd),
T (0xfffffffd.7p-32, 0xfffffffd, 0xfffffffd, 0xfffffffe, 0xfffffffd),
T (0x7fffff.8p-32, 0x800000, 0x7fffff, 0x800000, 0x7fffff),
T (0x7ffffe.8p-32, 0x7ffffe, 0x7ffffe, 0x7fffff, 0x7ffffe),
T (0x1.9p-32, 2, 1, 2, 1),
T (0x1.8p-32, 2, 1, 2, 1),
T (0x1.7p-32, 1, 1, 2, 1),
TZ (0x1p-32, 1),
T (0x0.9p-32, 1, 0, 1, 0),
T (0x0.8p-32, 0, 0, 1, 0),
T (0x0.7p-32, 0, 0, 1, 0),
T (0x1p-1074, 0, 0, 1, 0),
TZ (0.0, 0),
TZ (-0.0, 0),
TZ (-0x1p-1074, 0),
TZ (-0x0.7p-32, 0),
TZ (-0x0.8p-32, 0),
TZ (-0x0.9p-32, 0),
TZ (-0x1p-32, 0),
TZ (-0x1.7p-32, 0),
TZ (-0x1.8p-32, 0),
TZ (-0x1.9p-32, 0),
TZ (-0x7ffffe.8p-32, 0),
TZ (-0x7fffff.8p-32, 0),
TZ (-0xfffffffd.7p-32, 0),
TZ (-0xfffffffd.8p-32, 0),
TZ (-0xfffffffd.9p-32, 0),
TZ (-0xfffffffe.7p-32, 0),
TZ (-0xfffffffe.8p-32, 0),
TZ (-0xfffffffe.9p-32, 0),
TZ (-0xffffffffp-32, 0),
TZ (-1.0, 0),
TZ (-0x1.0000000000001p0, 0),
TZ (-0x1.fffffffffffffp1023, 0),
TZ (-INFD, 0),
TZ (-NAND, 0),
};
RUN_FLOAT_TESTS (efsctsiz)
RUN_VFLOAT_TESTS (evfsctsiz, efsctsiz)
RUN_FLOAT_TESTS (efsctuiz)
RUN_VFLOAT_TESTS (evfsctuiz, efsctuiz)
RUN_FLOAT_TESTS (efdctsiz)
RUN_FLOAT_TESTS (efdctuiz)
RUN_FLOAT_TESTS (efsctsi)
RUN_VFLOAT_TESTS (evfsctsi, efsctsi)
RUN_FLOAT_TESTS (efsctui)
RUN_VFLOAT_TESTS (evfsctui, efsctui)
RUN_FLOAT_TESTS (efdctsi)
RUN_FLOAT_TESTS (efdctui)
RUN_FLOAT_TESTS (efsctsf)
RUN_VFLOAT_TESTS (evfsctsf, efsctsf)
RUN_FLOAT_TESTS (efsctuf)
RUN_VFLOAT_TESTS (evfsctuf, efsctuf)
RUN_FLOAT_TESTS (efdctsf)
RUN_FLOAT_TESTS (efdctuf)
int
main (void)
{
test_efsctsiz ();
test_evfsctsiz ();
test_efsctuiz ();
test_evfsctuiz ();
test_efdctsiz ();
test_efdctuiz ();
test_efsctsi ();
test_evfsctsi ();
test_efsctui ();
test_evfsctui ();
test_efdctsi ();
test_efdctui ();
test_efsctsf ();
test_evfsctsf ();
test_efsctuf ();
test_evfsctuf ();
test_efdctsf ();
test_efdctuf ();
printf ("%d tests passed, %d tests failed\n", success_count, failure_count);
exit (failure_count != 0 ? EXIT_FAILURE : EXIT_SUCCESS);
}
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply
* [PATCH] powerpc: fix e500 SPE float to integer and fixed-point conversions
From: Joseph S. Myers @ 2013-10-08 23:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Shan Hai, linux-kernel
From: Joseph Myers <joseph@codesourcery.com>
The e500 SPE floating-point emulation code has several problems in how
it handles conversions to integer and fixed-point fractional types.
There are the following 20 relevant instructions. These can convert
to signed or unsigned 32-bit integers, either rounding towards zero
(as correct for C casts from floating-point to integer) or according
to the current rounding mode, or to signed or unsigned 32-bit
fixed-point values (values in the range [-1, 1) or [0, 1)). For
conversion from double precision there are also instructions to
convert to 64-bit integers, rounding towards zero, although as far as
I know those instructions are completely theoretical (they are only
defined for implementations that support both SPE and classic 64-bit,
and I'm not aware of any such hardware even though the architecture
definition permits that combination).
#define EFSCTUI 0x2d4
#define EFSCTSI 0x2d5
#define EFSCTUF 0x2d6
#define EFSCTSF 0x2d7
#define EFSCTUIZ 0x2d8
#define EFSCTSIZ 0x2da
#define EVFSCTUI 0x294
#define EVFSCTSI 0x295
#define EVFSCTUF 0x296
#define EVFSCTSF 0x297
#define EVFSCTUIZ 0x298
#define EVFSCTSIZ 0x29a
#define EFDCTUIDZ 0x2ea
#define EFDCTSIDZ 0x2eb
#define EFDCTUI 0x2f4
#define EFDCTSI 0x2f5
#define EFDCTUF 0x2f6
#define EFDCTSF 0x2f7
#define EFDCTUIZ 0x2f8
#define EFDCTSIZ 0x2fa
The emulation code, for the instructions that come in variants
rounding either towards zero or according to the current rounding
direction, uses "if (func & 0x4)" as a condition for using _FP_ROUND
(otherwise _FP_ROUND_ZERO is used). The condition is correct, but the
code it controls isn't. Whether _FP_ROUND or _FP_ROUND_ZERO is used
makes no difference, as the effect of those soft-fp macros is to round
an intermediate floating-point result using the low three bits (the
last one sticky) of the working format. As these operations are
dealing with a freshly unpacked floating-point input, those low bits
are zero and no rounding occurs. The emulation code then uses the
FP_TO_INT_* macros for the actual integer conversion, with the effect
of always rounding towards zero; for rounding according to the current
rounding direction, it should be using FP_TO_INT_ROUND_*.
The instructions in question have semantics defined (in the Power ISA
documents) for out-of-range values and NaNs: out-of-range values
saturate and NaNs are converted to zero. The emulation does nothing
to follow those semantics for NaNs (the soft-fp handling is to treat
them as infinities), and messes up the saturation semantics. For
single-precision conversion to integers, (((func & 0x3) != 0) || SB_s)
is the condition used for doing a signed conversion. The first part
is correct, but the second isn't: negative numbers should result in
saturation to 0 when converted to unsigned. Double-precision
conversion to 64-bit integers correctly uses ((func & 0x1) == 0).
Double-precision conversion to 32-bit integers uses (((func & 0x3) !=
0) || DB_s), with correct first part and incorrect second part. And
vector float conversion to integers uses (((func & 0x3) != 0) ||
SB0_s) (and similar for the other vector element), where the sign bit
check is again wrong.
The incorrect handling of negative numbers converted to unsigned was
introduced in commit afc0a07d4a283599ac3a6a31d7454e9baaeccca0. The
rationale given there was a C testcase with cast from float to
unsigned int. Conversion of out-of-range floating-point numbers to
integer types in C is undefined behavior in the base standard, defined
in Annex F to produce an unspecified value. That is, the C testcase
used to justify that patch is incorrect - there is no ISO C
requirement for a particular value resulting from this conversion -
and in any case, the correct semantics for such emulation are the
semantics for the instruction (unsigned saturation, which is what it
does in hardware when the emulation is disabled).
The conversion to fixed-point values has its own problems. That code
doesn't try to do a full emulation; it relies on the trap handler only
being called for arguments that are infinities, NaNs, subnormal or out
of range. That's fine, but the logic ((vb.wp[1] >> 23) == 0xff &&
((vb.wp[1] & 0x7fffff) > 0)) for NaN detection won't detect negative
NaNs as being NaNs (the same applies for the double-precision case),
and subnormals are mapped to 0 rather than respecting the rounding
mode; the code should also explicitly raise the "invalid" exception.
The code for vectors works by executing the scalar float instruction
with the trapping disabled, meaning at least subnormals won't be
handled correctly.
As well as all those problems in the main emulation code, the rounding
handler - used to emulate rounding upward and downward when not
supported in hardware and when no higher priority exception occurred -
has its own problems.
* It gets called in some cases even for the instructions rounding to
zero, and then acts according to the current rounding mode when it
should just leave alone the truncated result provided by hardware.
* It presumes that the result is a single-precision, double-precision
or single-precision vector as appropriate for the instruction type,
determines the sign of the result accordingly, and then adjusts the
result based on that sign and the rounding mode.
- In the single-precision cases at least the sign determination for
an integer result is the same as for a floating-point result; in
the double-precision case, converted to 32-bit integer or fixed
point, the sign of a double-precision value is in the high part of
the register but it's the low part of the register that has the
result of the conversion.
- If the result is unsigned fixed-point, its sign may be wrongly
determined as negative (does not actually cause problems, because
inexact unsigned fixed-point results with the high bit set can
only appear when converting from double, in which case the sign
determination is instead wrongly using the high part of the
register).
- If the sign of the result is correctly determined as negative, any
adjustment required to change the truncated result to one correct
for the rounding mode should be in the opposite direction for
two's-complement integers as for sign-magnitude floating-point
values.
- And if the integer result is zero, the correct sign can only be
determined by examining the original operand, and not at all (as
far as I can tell) if the operand and result are the same
register.
This patch fixes all these problems (as far as possible, given the
inability to determine the correct sign in the rounding handler when
the truncated result is 0, the conversion is to a signed type and the
truncated result has overwritten the original operand). Conversion to
fixed-point now uses full emulation, and does not use "asm" in the
vector case; the semantics are exactly those of converting to integer
according to the current rounding direction, once the exponent has
been adjusted, so the code makes such an adjustment then uses the
FP_TO_INT_ROUND macros.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
I'll send as a followup the testcase I used for verifying that the
instructions (other than the theoretical conversions to 64-bit
integers) produce the correct results. In addition, this has been
tested with the glibc testsuite (with the e500 port as posted at
<https://sourceware.org/ml/libc-alpha/2013-10/msg00195.html>, where it
improves the libm test results.
The patch depends on my previous patch
<http://lkml.org/lkml/2013/10/4/497> to fix inexactness detection in
the rounding handler. It does not depend on
<http://lkml.org/lkml/2013/10/4/495> (fix exception clearing),
<http://lkml.org/lkml/2013/10/8/694> (math-emu: fix floating-point to
integer unsigned saturation) or <http://lkml.org/lkml/2013/10/8/700>
(math-emu: fix floating-point to integer overflow detection), in that
I believe it can be applied independently of those other patches
without causing problems, but my testing has been in conjunction with
all those other patches and it may not fully fix all the affected
cases unless they are applied as well.
diff --git a/arch/powerpc/math-emu/math_efp.c b/arch/powerpc/math-emu/math_efp.c
index ecdf35d..01a0abb 100644
--- a/arch/powerpc/math-emu/math_efp.c
+++ b/arch/powerpc/math-emu/math_efp.c
@@ -275,21 +275,13 @@ int do_spe_mathemu(struct pt_regs *regs)
case EFSCTSF:
case EFSCTUF:
- if (!((vb.wp[1] >> 23) == 0xff && ((vb.wp[1] & 0x7fffff) > 0))) {
- /* NaN */
- if (((vb.wp[1] >> 23) & 0xff) == 0) {
- /* denorm */
- vc.wp[1] = 0x0;
- } else if ((vb.wp[1] >> 31) == 0) {
- /* positive normal */
- vc.wp[1] = (func == EFSCTSF) ?
- 0x7fffffff : 0xffffffff;
- } else { /* negative normal */
- vc.wp[1] = (func == EFSCTSF) ?
- 0x80000000 : 0x0;
- }
- } else { /* rB is NaN */
- vc.wp[1] = 0x0;
+ if (SB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ SB_e += (func == EFSCTSF ? 31 : 32);
+ FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
+ (func == EFSCTSF));
}
goto update_regs;
@@ -306,16 +298,25 @@ int do_spe_mathemu(struct pt_regs *regs)
}
case EFSCTSI:
- case EFSCTSIZ:
case EFSCTUI:
+ if (SB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_ROUND_S(vc.wp[1], SB, 32,
+ ((func & 0x3) != 0));
+ }
+ goto update_regs;
+
+ case EFSCTSIZ:
case EFSCTUIZ:
- if (func & 0x4) {
- _FP_ROUND(1, SB);
+ if (SB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
- _FP_ROUND_ZERO(1, SB);
+ FP_TO_INT_S(vc.wp[1], SB, 32,
+ ((func & 0x3) != 0));
}
- FP_TO_INT_S(vc.wp[1], SB, 32,
- (((func & 0x3) != 0) || SB_s));
goto update_regs;
default:
@@ -404,22 +405,13 @@ cmp_s:
case EFDCTSF:
case EFDCTUF:
- if (!((vb.wp[0] >> 20) == 0x7ff &&
- ((vb.wp[0] & 0xfffff) > 0 || (vb.wp[1] > 0)))) {
- /* not a NaN */
- if (((vb.wp[0] >> 20) & 0x7ff) == 0) {
- /* denorm */
- vc.wp[1] = 0x0;
- } else if ((vb.wp[0] >> 31) == 0) {
- /* positive normal */
- vc.wp[1] = (func == EFDCTSF) ?
- 0x7fffffff : 0xffffffff;
- } else { /* negative normal */
- vc.wp[1] = (func == EFDCTSF) ?
- 0x80000000 : 0x0;
- }
- } else { /* NaN */
- vc.wp[1] = 0x0;
+ if (DB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ DB_e += (func == EFDCTSF ? 31 : 32);
+ FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
+ (func == EFDCTSF));
}
goto update_regs;
@@ -437,21 +429,35 @@ cmp_s:
case EFDCTUIDZ:
case EFDCTSIDZ:
- _FP_ROUND_ZERO(2, DB);
- FP_TO_INT_D(vc.dp[0], DB, 64, ((func & 0x1) == 0));
+ if (DB_c == FP_CLS_NAN) {
+ vc.dp[0] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_D(vc.dp[0], DB, 64,
+ ((func & 0x1) == 0));
+ }
goto update_regs;
case EFDCTUI:
case EFDCTSI:
+ if (DB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_ROUND_D(vc.wp[1], DB, 32,
+ ((func & 0x3) != 0));
+ }
+ goto update_regs;
+
case EFDCTUIZ:
case EFDCTSIZ:
- if (func & 0x4) {
- _FP_ROUND(2, DB);
+ if (DB_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
- _FP_ROUND_ZERO(2, DB);
+ FP_TO_INT_D(vc.wp[1], DB, 32,
+ ((func & 0x3) != 0));
}
- FP_TO_INT_D(vc.wp[1], DB, 32,
- (((func & 0x3) != 0) || DB_s));
goto update_regs;
default:
@@ -556,37 +562,60 @@ cmp_d:
cmp = -1;
goto cmp_vs;
- case EVFSCTSF:
- __asm__ __volatile__ ("mtspr 512, %4\n"
- "efsctsf %0, %2\n"
- "efsctsf %1, %3\n"
- : "=r" (vc.wp[0]), "=r" (vc.wp[1])
- : "r" (vb.wp[0]), "r" (vb.wp[1]), "r" (0));
- goto update_regs;
-
case EVFSCTUF:
- __asm__ __volatile__ ("mtspr 512, %4\n"
- "efsctuf %0, %2\n"
- "efsctuf %1, %3\n"
- : "=r" (vc.wp[0]), "=r" (vc.wp[1])
- : "r" (vb.wp[0]), "r" (vb.wp[1]), "r" (0));
+ case EVFSCTSF:
+ if (SB0_c == FP_CLS_NAN) {
+ vc.wp[0] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ SB0_e += (func == EVFSCTSF ? 31 : 32);
+ FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
+ (func == EVFSCTSF));
+ }
+ if (SB1_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ SB1_e += (func == EVFSCTSF ? 31 : 32);
+ FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
+ (func == EVFSCTSF));
+ }
goto update_regs;
case EVFSCTUI:
case EVFSCTSI:
+ if (SB0_c == FP_CLS_NAN) {
+ vc.wp[0] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_ROUND_S(vc.wp[0], SB0, 32,
+ ((func & 0x3) != 0));
+ }
+ if (SB1_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_ROUND_S(vc.wp[1], SB1, 32,
+ ((func & 0x3) != 0));
+ }
+ goto update_regs;
+
case EVFSCTUIZ:
case EVFSCTSIZ:
- if (func & 0x4) {
- _FP_ROUND(1, SB0);
- _FP_ROUND(1, SB1);
+ if (SB0_c == FP_CLS_NAN) {
+ vc.wp[0] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
} else {
- _FP_ROUND_ZERO(1, SB0);
- _FP_ROUND_ZERO(1, SB1);
+ FP_TO_INT_S(vc.wp[0], SB0, 32,
+ ((func & 0x3) != 0));
+ }
+ if (SB1_c == FP_CLS_NAN) {
+ vc.wp[1] = 0;
+ FP_SET_EXCEPTION(FP_EX_INVALID);
+ } else {
+ FP_TO_INT_S(vc.wp[1], SB1, 32,
+ ((func & 0x3) != 0));
}
- FP_TO_INT_S(vc.wp[0], SB0, 32,
- (((func & 0x3) != 0) || SB0_s));
- FP_TO_INT_S(vc.wp[1], SB1, 32,
- (((func & 0x3) != 0) || SB1_s));
goto update_regs;
default:
@@ -681,14 +710,16 @@ int speround_handler(struct pt_regs *regs)
union dw_union fgpr;
int s_lo, s_hi;
int lo_inexact, hi_inexact;
- unsigned long speinsn, type, fc, fptype;
+ int fp_result;
+ unsigned long speinsn, type, fb, fc, fptype, func;
if (get_user(speinsn, (unsigned int __user *) regs->nip))
return -EFAULT;
if ((speinsn >> 26) != 4)
return -EINVAL; /* not an spe instruction */
- type = insn_type(speinsn & 0x7ff);
+ func = speinsn & 0x7ff;
+ type = insn_type(func);
if (type == XCR) return -ENOSYS;
__FPU_FPSCR = mfspr(SPRN_SPEFSCR);
@@ -708,6 +739,65 @@ int speround_handler(struct pt_regs *regs)
fgpr.wp[0] = current->thread.evr[fc];
fgpr.wp[1] = regs->gpr[fc];
+ fb = (speinsn >> 11) & 0x1f;
+ switch (func) {
+ case EFSCTUIZ:
+ case EFSCTSIZ:
+ case EVFSCTUIZ:
+ case EVFSCTSIZ:
+ case EFDCTUIDZ:
+ case EFDCTSIDZ:
+ case EFDCTUIZ:
+ case EFDCTSIZ:
+ /*
+ * These instructions always round to zero,
+ * independent of the rounding mode.
+ */
+ return 0;
+
+ case EFSCTUI:
+ case EFSCTUF:
+ case EVFSCTUI:
+ case EVFSCTUF:
+ case EFDCTUI:
+ case EFDCTUF:
+ fp_result = 0;
+ s_lo = 0;
+ s_hi = 0;
+ break;
+
+ case EFSCTSI:
+ case EFSCTSF:
+ fp_result = 0;
+ /* Recover the sign of a zero result if possible. */
+ if (fgpr.wp[1] == 0)
+ s_lo = regs->gpr[fb] & SIGN_BIT_S;
+ break;
+
+ case EVFSCTSI:
+ case EVFSCTSF:
+ fp_result = 0;
+ /* Recover the sign of a zero result if possible. */
+ if (fgpr.wp[1] == 0)
+ s_lo = regs->gpr[fb] & SIGN_BIT_S;
+ if (fgpr.wp[0] == 0)
+ s_hi = current->thread.evr[fb] & SIGN_BIT_S;
+ break;
+
+ case EFDCTSI:
+ case EFDCTSF:
+ fp_result = 0;
+ s_hi = s_lo;
+ /* Recover the sign of a zero result if possible. */
+ if (fgpr.wp[1] == 0)
+ s_hi = current->thread.evr[fb] & SIGN_BIT_S;
+ break;
+
+ default:
+ fp_result = 1;
+ break;
+ }
+
pr_debug("round fgpr: %08x %08x\n", fgpr.wp[0], fgpr.wp[1]);
switch (fptype) {
@@ -719,15 +809,30 @@ int speround_handler(struct pt_regs *regs)
if ((FP_ROUNDMODE) == FP_RND_PINF) {
if (!s_lo) fgpr.wp[1]++; /* Z > 0, choose Z1 */
} else { /* round to -Inf */
- if (s_lo) fgpr.wp[1]++; /* Z < 0, choose Z2 */
+ if (s_lo) {
+ if (fp_result)
+ fgpr.wp[1]++; /* Z < 0, choose Z2 */
+ else
+ fgpr.wp[1]--; /* Z < 0, choose Z2 */
+ }
}
break;
case DPFP:
if (FP_ROUNDMODE == FP_RND_PINF) {
- if (!s_hi) fgpr.dp[0]++; /* Z > 0, choose Z1 */
+ if (!s_hi) {
+ if (fp_result)
+ fgpr.dp[0]++; /* Z > 0, choose Z1 */
+ else
+ fgpr.wp[1]++; /* Z > 0, choose Z1 */
+ }
} else { /* round to -Inf */
- if (s_hi) fgpr.dp[0]++; /* Z < 0, choose Z2 */
+ if (s_hi) {
+ if (fp_result)
+ fgpr.dp[0]++; /* Z < 0, choose Z2 */
+ else
+ fgpr.wp[1]--; /* Z < 0, choose Z2 */
+ }
}
break;
@@ -738,10 +843,18 @@ int speround_handler(struct pt_regs *regs)
if (hi_inexact && !s_hi)
fgpr.wp[0]++; /* Z_high word > 0, choose Z1 */
} else { /* round to -Inf */
- 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 */
+ if (lo_inexact && s_lo) {
+ if (fp_result)
+ fgpr.wp[1]++; /* Z_low < 0, choose Z2 */
+ else
+ fgpr.wp[1]--; /* Z_low < 0, choose Z2 */
+ }
+ if (hi_inexact && s_hi) {
+ if (fp_result)
+ fgpr.wp[0]++; /* Z_high < 0, choose Z2 */
+ else
+ fgpr.wp[0]--; /* Z_high < 0, choose Z2 */
+ }
}
break;
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* Re: [PATCH 1/7] powerpc: Add interface to get msi region information
From: Scott Wood @ 2013-10-08 23:35 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Alexander Graf, Joerg Roedel, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, open list:INTEL IOMMU (VT-d),
Bharat Bhushan, alex.williamson@redhat.com, Bharat Bhushan,
linuxppc-dev
In-Reply-To: <CAErSpo7+7SHVcOJkHkW5cjb6pN+bqYeRdPBA=MVX0RLz=-pP6Q@mail.gmail.com>
On Tue, 2013-10-08 at 17:25 -0600, Bjorn Helgaas wrote:
> >> - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
> >> + dma_addr_t msiir; /* MSIIR Address in CCSR */
> >
> > Are you sure dma_addr_t is right here, versus phys_addr_t? It implies
> > that it's the output of the DMA API, but I don't think the DMA API is
> > used in the MSI driver. Perhaps it should be, but we still want the raw
> > physical address to pass on to VFIO.
>
> I don't know what "msiir" is used for, but if it's an address you
> program into a PCI device, then it's a dma_addr_t even if you didn't
> get it from the DMA API. Maybe "bus_addr_t" would have been a more
> suggestive name than "dma_addr_t". That said, I have no idea how this
> relates to VFIO.
It's a bit awkward because it gets used both as something to program
into a PCI device (and it's probably a bug that the DMA API doesn't get
used), and also (if I understand the current plans correctly) as a
physical address to give to VFIO to be a destination address in an IOMMU
mapping. So I think the value we keep here should be a phys_addr_t (it
comes straight from the MMIO address in the device tree), which gets
trivially turned into a dma_addr_t by the non-VFIO code path because
there's currently no translation there.
-Scott
^ permalink raw reply
* Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host
From: Benjamin Herrenschmidt @ 2013-10-08 23:31 UTC (permalink / raw)
To: Scott Wood
Cc: linux-pci@vger.kernel.org, Zang Roy-R61911, Minghuan Lian,
Paul Mackerras, Bjorn Helgaas, linuxppc-dev
In-Reply-To: <1381274440.7979.309.camel@snotra.buserror.net>
On Tue, 2013-10-08 at 18:20 -0500, Scott Wood wrote:
> > So I'll apply these given an ack from the powerpc folks.
>
> ACK this patch. The second one I'd like to see broken up into
> digestible chunks so I can better review it.
Bjorn, for such FSL-only stuff, Scott ack is enough, don't wait for
mine :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/7] powerpc: Add interface to get msi region information
From: Bjorn Helgaas @ 2013-10-08 23:25 UTC (permalink / raw)
To: Scott Wood
Cc: Alexander Graf, Joerg Roedel, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, open list:INTEL IOMMU (VT-d),
Bharat Bhushan, alex.williamson@redhat.com, Bharat Bhushan,
linuxppc-dev
In-Reply-To: <1381273037.7979.298.camel@snotra.buserror.net>
>> - u32 msiir_offset; /* Offset of MSIIR, relative to start of CCSR */
>> + dma_addr_t msiir; /* MSIIR Address in CCSR */
>
> Are you sure dma_addr_t is right here, versus phys_addr_t? It implies
> that it's the output of the DMA API, but I don't think the DMA API is
> used in the MSI driver. Perhaps it should be, but we still want the raw
> physical address to pass on to VFIO.
I don't know what "msiir" is used for, but if it's an address you
program into a PCI device, then it's a dma_addr_t even if you didn't
get it from the DMA API. Maybe "bus_addr_t" would have been a more
suggestive name than "dma_addr_t". That said, I have no idea how this
relates to VFIO.
Bjorn
^ permalink raw reply
* [PATCH] math-emu: fix floating-point to integer overflow detection
From: Joseph S. Myers @ 2013-10-08 23:24 UTC (permalink / raw)
To: linux-kernel; +Cc: linuxppc-dev
From: Joseph Myers <joseph@codesourcery.com>
On overflow, the math-emu macro _FP_TO_INT_ROUND tries to saturate its
result (subject to the value of rsigned specifying the desired
overflow semantics). However, if the rounding step has the effect of
increasing the exponent so as to cause overflow (if the rounded result
is 1 larger than the largest positive value with the given number of
bits, allowing for signedness), the overflow does not get detected,
meaning that for unsigned results 0 is produced instead of the maximum
unsigned integer with the give number of bits, without an exception
being raised for overflow, and that for signed results the minimum
(negative) value is produced instead of the maximum (positive) value,
again without an exception. This patch makes the code check for
rounding increasing the exponent and adjusts the exponent value as
needed for the overflow check.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
This macro is not present in the glibc/libgcc version of the code.
This patch is independent of my separate patch
<http://lkml.org/lkml/2013/10/8/694> to fix the results for unsigned
saturation, although you need both patches together to get the correct
results for the affected unsigned overflow case. It remains the case
both before and after this patch that the conversions wrongly treat a
signed result of the most negative integer as an overflow, when
actually only that integer minus 1 or smaller should be an overflow,
although this only means an incorrect exception rather than affecting
the value returned; that was one of the bugs I fixed in the
glibc/libgcc version of this code in 2006 (as part of a major overhaul
of the code including various interface changes, so not trivially
backportable to the kernel version).
diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index 9696a5e..6bdf8c6 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -743,12 +743,17 @@ do { \
} \
else \
{ \
+ int _lz0, _lz1; \
if (X##_e <= -_FP_WORKBITS - 1) \
_FP_FRAC_SET_##wc(X, _FP_MINFRAC_##wc); \
else \
_FP_FRAC_SRS_##wc(X, _FP_FRACBITS_##fs - 1 - X##_e, \
_FP_WFRACBITS_##fs); \
+ _FP_FRAC_CLZ_##wc(_lz0, X); \
_FP_ROUND(wc, X); \
+ _FP_FRAC_CLZ_##wc(_lz1, X); \
+ if (_lz1 < _lz0) \
+ X##_e++; /* For overflow detection. */ \
_FP_FRAC_SRL_##wc(X, _FP_WORKBITS); \
_FP_FRAC_ASSEMBLE_##wc(r, X, rsize); \
} \
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host
From: Scott Wood @ 2013-10-08 23:20 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci@vger.kernel.org, Zang Roy-R61911, Minghuan Lian,
Paul Mackerras, linuxppc-dev
In-Reply-To: <CAErSpo76_w+wK76m73kkdYS1PouhYcGfZus_FYF5E2FHJTA6gQ@mail.gmail.com>
On Tue, 2013-10-08 at 17:09 -0600, Bjorn Helgaas wrote:
> On Tue, Oct 8, 2013 at 4:46 PM, Scott Wood <scottwood@freescale.com> wrote:
> > On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:
> >> [+cc Ben, Paul, linuxppc-dev]
> >>
> >> On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
> >> > The Freescale's Layerscape series processors will use ARM cores.
> >> > The LS1's PCIe controllers is the same as T4240's. So it's better
> >> > the PCIe controller driver can support PowerPC and ARM
> >> > simultaneously. This patch is for this purpose. It derives
> >> > the common functions from arch/powerpc/sysdev/fsl_pci.c to
> >> > drivers/pci/host/pci-fsl-common.c and leaves the architecture
> >> > specific functions which should be implemented in arch related files.
> >> >
> >> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> >>
> >> I cc'd the powerpc maintainers so we can work out which tree this
> >> should go through.
> >>
> >> > ---
> >> > change log:
> >> > v1-v2:
> >> > 1. rename pci.h to pci-common.h
> >> > 2. rename pci-fsl.c to pci-fsl-common.c
> >> >
> >> > Based on upstream master.
> >> > Based on the discussion of RFC version here
> >> > http://patchwork.ozlabs.org/patch/274487/
> >> >
> >> > arch/powerpc/sysdev/fsl_pci.c | 521 +-----------------
> >> > arch/powerpc/sysdev/fsl_pci.h | 89 ----
> >> > .../fsl_pci.c => drivers/pci/host/pci-fsl-common.c | 591 +--------------------
> >> > .../fsl_pci.h => include/linux/fsl/pci-common.h | 45 +-
> >>
> >> Is there any way to avoid putting this file in include/linux? I know
> >> you want to share it beyond PowerPC, and I know there are similar
> >> examples there already, but this is all arch-specific or
> >> chipset-specific stuff that seems like it should be in some
> >> not-so-public place. It doesn't seem scalable to add an include/linux
> >> subdirectory for every chipset that might be shared across
> >> architectures.
> >
> > What specifically is the problem with it, as long as it's properly
> > namespaced?
>
> Well, as I said above, it doesn't seem scalable,
I'm not sure what scaling problems you're picturing, assuming proper
namespacing and organization within include/linux/.
> and it doesn't seem to be the common existing practice.
>
> Possibly this is just because sharing chipsets across arches isn't very common yet.
>
> I hadn't noticed that include/linux/fsl exists already; I thought you
> were adding it. Given that it *does* exist already, I guess I'm OK
> with putting more stuff in it.
I see other existing practice as well. Besides plenty of
"include/linux/fsl*" that ought to be moved to "include/linux/fsl/", I
see things like include/linux/amba/, include/linux/scx200*,
include/linux/clksrc-dbx500-prcmu.h, include/linux/com202020.h, etc.
These are just a few random examples out of many.
> So I'll apply these given an ack from the powerpc folks.
ACK this patch. The second one I'd like to see broken up into
digestible chunks so I can better review it.
-Scott
^ permalink raw reply
* [PATCH] math-emu: fix floating-point to integer unsigned saturation
From: Joseph S. Myers @ 2013-10-08 23:12 UTC (permalink / raw)
To: linux-kernel; +Cc: linuxppc-dev
From: Joseph Myers <joseph@codesourcery.com>
The math-emu macros _FP_TO_INT and _FP_TO_INT_ROUND are supposed to
saturate their results for out-of-range arguments, except in the case
rsigned == 2 (when instead the low bits of the result are taken).
However, in the case rsigned == 0 (converting to unsigned integers),
they mistakenly produce 0 for positive results and the maximum
unsigned integer for negative results, the opposite of correct
unsigned saturation. This patch fixes the logic.
Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
I intend to make the corresponding changes to the glibc/libgcc copy of
this code, given that it would be desirable to resync the Linux and
glibc/libgcc copies (the latter has had many enhancements and bug
fixes since it was copied into Linux), although strictly this
incorrect saturation is only a bug when trying to emulate particular
instruction semantics, not when used in userspace to implement C
operations where the results of out-of-range conversions are
unspecified or undefined.
diff --git a/include/math-emu/op-common.h b/include/math-emu/op-common.h
index 9696a5e..70fe5e9 100644
--- a/include/math-emu/op-common.h
+++ b/include/math-emu/op-common.h
@@ -685,7 +685,7 @@ do { \
else \
{ \
r = 0; \
- if (X##_s) \
+ if (!X##_s) \
r = ~r; \
} \
FP_SET_EXCEPTION(FP_EX_INVALID); \
@@ -762,7 +762,7 @@ do { \
if (!rsigned) \
{ \
r = 0; \
- if (X##_s) \
+ if (!X##_s) \
r = ~r; \
} \
else if (rsigned != 2) \
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply related
* Re: [PATCH 1/2][v2] pci: fsl: derive the common PCI driver to drivers/pci/host
From: Bjorn Helgaas @ 2013-10-08 23:09 UTC (permalink / raw)
To: Scott Wood
Cc: linux-pci@vger.kernel.org, Zang Roy-R61911, Minghuan Lian,
Paul Mackerras, linuxppc-dev
In-Reply-To: <1381272382.7979.292.camel@snotra.buserror.net>
On Tue, Oct 8, 2013 at 4:46 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Tue, 2013-10-08 at 13:13 -0600, Bjorn Helgaas wrote:
>> [+cc Ben, Paul, linuxppc-dev]
>>
>> On Mon, Sep 30, 2013 at 04:52:54PM +0800, Minghuan Lian wrote:
>> > The Freescale's Layerscape series processors will use ARM cores.
>> > The LS1's PCIe controllers is the same as T4240's. So it's better
>> > the PCIe controller driver can support PowerPC and ARM
>> > simultaneously. This patch is for this purpose. It derives
>> > the common functions from arch/powerpc/sysdev/fsl_pci.c to
>> > drivers/pci/host/pci-fsl-common.c and leaves the architecture
>> > specific functions which should be implemented in arch related files.
>> >
>> > Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>>
>> I cc'd the powerpc maintainers so we can work out which tree this
>> should go through.
>>
>> > ---
>> > change log:
>> > v1-v2:
>> > 1. rename pci.h to pci-common.h
>> > 2. rename pci-fsl.c to pci-fsl-common.c
>> >
>> > Based on upstream master.
>> > Based on the discussion of RFC version here
>> > http://patchwork.ozlabs.org/patch/274487/
>> >
>> > arch/powerpc/sysdev/fsl_pci.c | 521 +-----------------
>> > arch/powerpc/sysdev/fsl_pci.h | 89 ----
>> > .../fsl_pci.c => drivers/pci/host/pci-fsl-common.c | 591 +--------------------
>> > .../fsl_pci.h => include/linux/fsl/pci-common.h | 45 +-
>>
>> Is there any way to avoid putting this file in include/linux? I know
>> you want to share it beyond PowerPC, and I know there are similar
>> examples there already, but this is all arch-specific or
>> chipset-specific stuff that seems like it should be in some
>> not-so-public place. It doesn't seem scalable to add an include/linux
>> subdirectory for every chipset that might be shared across
>> architectures.
>
> What specifically is the problem with it, as long as it's properly
> namespaced?
Well, as I said above, it doesn't seem scalable, and it doesn't seem
to be the common existing practice. Possibly this is just because
sharing chipsets across arches isn't very common yet.
I hadn't noticed that include/linux/fsl exists already; I thought you
were adding it. Given that it *does* exist already, I guess I'm OK
with putting more stuff in it.
So I'll apply these given an ack from the powerpc folks.
Bjorn
^ permalink raw reply
* RE: [PATCH RFC 63/77] qlcnic: Update MSI/MSI-X interrupts enablement code
From: Himanshu Madhani @ 2013-10-08 22:46 UTC (permalink / raw)
To: Alexander Gordeev, linux-kernel
Cc: linux-mips@linux-mips.org, VMware, Inc.,
linux-nvme@lists.infradead.org, linux-ide@vger.kernel.org,
linux-s390@vger.kernel.org, Andy King, linux-scsi,
linux-rdma@vger.kernel.org, x86@kernel.org, Ingo Molnar,
linux-pci, iss_storagedev@hp.com, Dept-Eng Linux Driver,
Tejun Heo, Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, Ralf Baechle,
e1000-devel@lists.sourceforge.net, Martin Schwidefsky,
linux390@de.ibm.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <c92efbde96541d08f37510422c096d543bb01279.1380703263.git.agordeev@redhat.com>
> -----Original Message-----
> From: Alexander Gordeev [mailto:agordeev@redhat.com]
> Sent: Wednesday, October 02, 2013 3:49 AM
> To: linux-kernel
> Cc: Alexander Gordeev; Bjorn Helgaas; Ralf Baechle; Michael Ellerman;
> Benjamin Herrenschmidt; Martin Schwidefsky; Ingo Molnar; Tejun Heo; Dan
> Williams; Andy King; Jon Mason; Matt Porter; linux-pci; linux-mips@linux-
> mips.org; linuxppc-dev@lists.ozlabs.org; linux390@de.ibm.com; linux-
> s390@vger.kernel.org; x86@kernel.org; linux-ide@vger.kernel.org;
> iss_storagedev@hp.com; linux-nvme@lists.infradead.org; linux-
> rdma@vger.kernel.org; netdev; e1000-devel@lists.sourceforge.net; Dept-
> Eng Linux Driver; Solarflare linux maintainers; VMware, Inc.; linux-scsi
> Subject: [PATCH RFC 63/77] qlcnic: Update MSI/MSI-X interrupts enablement
> code
>=20
> As result of recent re-design of the MSI/MSI-X interrupts enabling patter=
n
> this driver has to be updated to use the new technique to obtain a optima=
l
> number of MSI/MSI-X interrupts required.
>=20
"We will test this change for the driver and provide feedback."
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
Thanks,
Himanshu
^ 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