* Re: [PATCH 3/9] powerpc: Add TIF_ELF2ABI flag.
From: Michael Ellerman @ 2013-11-21 11:55 UTC (permalink / raw)
To: Anton Blanchard; +Cc: rusty, Ulrich.Weigand, paulus, alistair, linuxppc-dev
In-Reply-To: <1384946106-18200-4-git-send-email-anton@samba.org>
On Wed, Nov 20, 2013 at 10:15:00PM +1100, Anton Blanchard wrote:
> From: Rusty Russell <rusty@rustcorp.com.au>
>
> Little endian ppc64 is getting an exciting new ABI. This is reflected
> by the bottom two bits of e_flags in the ELF header:
>
> 0 == legacy binaries (v1 ABI)
> 1 == binaries using the old ABI (compiled with a new toolchain)
> 2 == binaries using the new ABI.
Just to be ridiculously clear for stupid people like me, you refer here
to the "v1 ABI" and "the old ABI" - they are the same thing - right?
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index ba7b197..05a3030 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -107,6 +107,9 @@ static inline struct thread_info *current_thread_info(void)
> #define TIF_EMULATE_STACK_STORE 16 /* Is an instruction emulation
> for stack store? */
> #define TIF_MEMDIE 17 /* is terminating due to OOM killer */
> +#if defined(CONFIG_PPC64)
> +#define TIF_ELF2ABI 18 /* function descriptors must die! */
> +#endif
This is the first TIF flag we #ifdef for 32 vs 64, is that just because
we don't want to waste a flag on 32 bit?
cheers
^ permalink raw reply
* Re: [PATCH 2/9] pseries: Add H_SET_MODE to change exception endianness
From: Michael Ellerman @ 2013-11-21 11:51 UTC (permalink / raw)
To: Anton Blanchard; +Cc: rusty, Ulrich.Weigand, paulus, alistair, linuxppc-dev
In-Reply-To: <1384946106-18200-3-git-send-email-anton@samba.org>
On Wed, Nov 20, 2013 at 10:14:59PM +1100, Anton Blanchard wrote:
> On little endian builds call H_SET_MODE so exceptions have the
> correct endianness. We need to reset the endian during kexec
> so do that in the MMU hashtable clear callback.
>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> ---
> arch/powerpc/include/asm/hvcall.h | 2 ++
> arch/powerpc/include/asm/plpar_wrappers.h | 26 +++++++++++++++++++
> arch/powerpc/platforms/pseries/lpar.c | 17 +++++++++++++
> arch/powerpc/platforms/pseries/setup.c | 42 +++++++++++++++++++++++++++++++
> 4 files changed, 87 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index 0c7f2bf..d8b600b 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -403,6 +403,8 @@ static inline unsigned long cmo_get_page_size(void)
> extern long pSeries_enable_reloc_on_exc(void);
> extern long pSeries_disable_reloc_on_exc(void);
>
> +extern long pseries_big_endian_exceptions(void);
> +
> #else
>
> #define pSeries_enable_reloc_on_exc() do {} while (0)
> diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
> index a63b045..12c32c5 100644
> --- a/arch/powerpc/include/asm/plpar_wrappers.h
> +++ b/arch/powerpc/include/asm/plpar_wrappers.h
> @@ -287,6 +287,32 @@ static inline long disable_reloc_on_exceptions(void) {
> return plpar_set_mode(0, 3, 0, 0);
> }
>
> +/*
> + * Take exceptions in big endian mode on this partition
> + *
> + * Note: this call has a partition wide scope and can take a while to complete.
> + * If it returns H_LONG_BUSY_* it should be retried periodically until it
> + * returns H_SUCCESS.
> + */
> +static inline long enable_big_endian_exceptions(void)
> +{
> + /* mflags = 0: big endian exceptions */
> + return plpar_set_mode(0, 4, 0, 0);
> +}
> +
> +/*
> + * Take exceptions in little endian mode on this partition
> + *
> + * Note: this call has a partition wide scope and can take a while to complete.
> + * If it returns H_LONG_BUSY_* it should be retried periodically until it
> + * returns H_SUCCESS.
> + */
> +static inline long enable_little_endian_exceptions(void)
> +{
> + /* mflags = 1: little endian exceptions */
> + return plpar_set_mode(1, 4, 0, 0);
> +}
> +
...
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 1f97e2b..c1f1908 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -442,6 +442,32 @@ static void pSeries_machine_kexec(struct kimage *image)
> }
> #endif
Nit picking, but I'm not sure the enable_big/little_endian_exceptions()
wrappers buy us much. And they mean you end up with essentially the same
routine twice below:
> +#ifdef __LITTLE_ENDIAN__
> +long pseries_big_endian_exceptions(void)
> +{
> + long rc;
> +
> + while (1) {
> + rc = enable_big_endian_exceptions();
> + if (!H_IS_LONG_BUSY(rc))
> + return rc;
> + mdelay(get_longbusy_msecs(rc));
> + }
> +}
> +
> +static long pseries_little_endian_exceptions(void)
> +{
> + long rc;
> +
> + while (1) {
> + rc = enable_little_endian_exceptions();
> + if (!H_IS_LONG_BUSY(rc))
> + return rc;
> + mdelay(get_longbusy_msecs(rc));
> + }
> +}
> +#endif
Whereas you could do:
static long pseries_big_endian_exceptions(bool big_endian)
{
int mflags;
long rc;
mflags = big_endian ? 0 : 1;
while (1) {
rc = plpar_set_mode(mflags, 4, 0, 0);
if (!H_IS_LONG_BUSY(rc))
return rc;
mdelay(get_longbusy_msecs(rc));
}
}
Perhaps pseries_big_endian_exceptions(false) looks a bit odd though ..
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/signals: Mark VSX not saved with small contexts
From: Michael Ellerman @ 2013-11-21 11:33 UTC (permalink / raw)
To: Michael Neuling
Cc: Carlos O'Donell, Steve Best, linuxppc-dev, Haren Myneni
In-Reply-To: <1384924734-16722-1-git-send-email-mikey@neuling.org>
On Wed, Nov 20, 2013 at 04:18:54PM +1100, Michael Neuling wrote:
> The VSX MSR bit in the user context indicates if the context contains VSX
> state. Currently we set this when the process has touched VSX at any stage.
>
> Unfortunately, if the user has not provided enough space to save the VSX state,
> we can't save it but we currently still set the MSR VSX bit.
>
> This patch changes this to clear the MSR VSX bit when the user doesn't provide
> enough space. This indicates that there is no valid VSX state in the user
> context.
>
> This is needed to support get/set/make/swapcontext for applications that use
> VSX but only provide a small context. For example, getcontext in glibc
> provides a smaller context since the VSX registers don't need to be saved over
> the glibc function call. But since the program calling getcontext may have
> used VSX, the kernel currently says the VSX state is valid when it's not. If
> the returned context is then used in setcontext (ie. a small context without
> VSX but with MSR VSX set), the kernel will refuse the context. This situation
> has been reported by the glibc community.
Broken since forever?
> Tested-by: Haren Myneni <haren@linux.vnet.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> Cc: stable@vger.kernel.org
> ---
> arch/powerpc/kernel/signal_32.c | 10 +++++++++-
What about the 64-bit code? I don't know the code but it appears at a glance to
have the same bug.
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 749778e..1844298 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -457,7 +457,15 @@ static int save_user_regs(struct pt_regs *regs, struct mcontext __user *frame,
> if (copy_vsx_to_user(&frame->mc_vsregs, current))
> return 1;
> msr |= MSR_VSX;
> - }
> + } else if (!ctx_has_vsx_region)
> + /*
> + * With a small context structure we can't hold the VSX
> + * registers, hence clear the MSR value to indicate the state
> + * was not saved.
> + */
> + msr &= ~MSR_VSX;
I think it'd be clearer if this was just the else case. The full context is:
if (current->thread.used_vsr && ctx_has_vsx_region) {
__giveup_vsx(current);
if (copy_vsx_to_user(&frame->mc_vsregs, current))
return 1;
msr |= MSR_VSX;
+ } else if (!ctx_has_vsx_region)
+ /*
+ * With a small context structure we can't hold the VSX
+ * registers, hence clear the MSR value to indicate the state
+ * was not saved.
+ */
+ msr &= ~MSR_VSX;
Which means if current->thread.user_vsr and ctx_has_vsx_region are both false
we potentially leave MSR_VSX set in msr. I think it should be the case that
MSR_VSX is only ever set if current->thread.used_vsr is true, so it should be
OK in pratice, but it seems unnecessarily fragile.
The logic should be "if we write VSX we set MSR_VSX, else we clear MSR_VSX", ie:
if (current->thread.used_vsr && ctx_has_vsx_region) {
__giveup_vsx(current);
if (copy_vsx_to_user(&frame->mc_vsregs, current))
return 1;
msr |= MSR_VSX;
} else
msr &= ~MSR_VSX;
cheers
^ permalink raw reply
* RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
From: Bharat Bhushan @ 2013-11-21 11:20 UTC (permalink / raw)
To: Alex Williamson
Cc: linux-pci@vger.kernel.org, joro@8bytes.org, agraf@suse.de,
Stuart Yoder, Scott Wood, iommu@lists.linux-foundation.org,
bhelgaas@google.com, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1384973243.2879.361.camel@ul30vt.home>
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQWxleCBXaWxsaWFtc29u
IFttYWlsdG86YWxleC53aWxsaWFtc29uQHJlZGhhdC5jb21dDQo+IFNlbnQ6IFRodXJzZGF5LCBO
b3ZlbWJlciAyMSwgMjAxMyAxMjoxNyBBTQ0KPiBUbzogQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+
IENjOiBqb3JvQDhieXRlcy5vcmc7IGJoZWxnYWFzQGdvb2dsZS5jb207IGFncmFmQHN1c2UuZGU7
IFdvb2QgU2NvdHQtQjA3NDIxOw0KPiBZb2RlciBTdHVhcnQtQjA4MjQ4OyBpb21tdUBsaXN0cy5s
aW51eC1mb3VuZGF0aW9uLm9yZzsgbGludXgtDQo+IHBjaUB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4
cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBsaW51eC0NCj4ga2VybmVsQHZnZXIua2VybmVsLm9y
ZzsgQmh1c2hhbiBCaGFyYXQtUjY1Nzc3DQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMC85IHYyXSB2
ZmlvLXBjaTogYWRkIHN1cHBvcnQgZm9yIEZyZWVzY2FsZSBJT01NVSAoUEFNVSkNCj4gDQo+IE9u
IFR1ZSwgMjAxMy0xMS0xOSBhdCAxMDo0NyArMDUzMCwgQmhhcmF0IEJodXNoYW4gd3JvdGU6DQo+
ID4gRnJvbTogQmhhcmF0IEJodXNoYW4gPGJoYXJhdC5iaHVzaGFuQGZyZWVzY2FsZS5jb20+DQo+
ID4NCj4gPiBQQU1VIChGU0wgSU9NTVUpIGhhcyBhIGNvbmNlcHQgb2YgcHJpbWFyeSB3aW5kb3cg
YW5kIHN1YndpbmRvd3MuDQo+ID4gUHJpbWFyeSB3aW5kb3cgY29ycmVzcG9uZHMgdG8gdGhlIGNv
bXBsZXRlIGd1ZXN0IGlvdmEgYWRkcmVzcyBzcGFjZQ0KPiA+IChpbmNsdWRpbmcgTVNJIHNwYWNl
KSwgd2l0aCByZXNwZWN0IHRvIElPTU1VX0FQSSB0aGlzIGlzIHRlcm1lZCBhcw0KPiA+IGdlb21l
dHJ5LiBJT1ZBIEJhc2Ugb2Ygc3Vid2luZG93IGlzIGRldGVybWluZWQgZnJvbSB0aGUgbnVtYmVy
IG9mDQo+ID4gc3Vid2luZG93cyAoY29uZmlndXJhYmxlIHVzaW5nIGlvbW11IEFQSSkuDQo+ID4g
TVNJIEkvTyBwYWdlIG11c3QgYmUgd2l0aGluIHRoZSBnZW9tZXRyeSBhbmQgbWF4aW11bSBzdXBw
b3J0ZWQNCj4gPiBzdWJ3aW5kb3dzLCBzbyBNU0kgSU8tcGFnZSBpcyBzZXR1cCBqdXN0IGFmdGVy
IGd1ZXN0IG1lbW9yeSBpb3ZhIHNwYWNlLg0KPiA+DQo+ID4gU28gcGF0Y2ggMS85LTQvOShpbmNs
dXNpdmUpIGFyZSBmb3IgZGVmaW5pbmcgdGhlIGludGVyZmFjZSB0byBnZXQ6DQo+ID4gICAtIE51
bWJlciBvZiBNU0kgcmVnaW9ucyAod2hpY2ggaXMgbnVtYmVyIG9mIE1TSSBiYW5rcyBmb3IgcG93
ZXJwYykNCj4gPiAgIC0gTVNJLXJlZ2lvbiBhZGRyZXNzIHJhbmdlOiBQaHlzaWNhbCBwYWdlIHdo
aWNoIGhhdmUgdGhlDQo+ID4gICAgIGFkZHJlc3MvYWRkcmVzc2VzIHVzZWQgZm9yIGdlbmVyYXRp
bmcgTVNJIGludGVycnVwdA0KPiA+ICAgICBhbmQgc2l6ZSBvZiB0aGUgcGFnZS4NCj4gPg0KPiA+
IFBhdGNoIDUvOS03LzkoaW5jbHVzaXZlKSBpcyBkZWZpbmluZyB0aGUgaW50ZXJmYWNlIG9mIHNl
dHRpbmcgdXAgTVNJDQo+ID4gaW92YS1iYXNlIGZvciBhIG1zaSByZWdpb24oYmFuaykgZm9yIGEg
ZGV2aWNlLiBzbyB0aGF0IHdoZW4NCj4gPiBtc2ktbWVzc2FnZSB3aWxsIGJlIGNvbXBvc2VkIHRo
ZW4gdGhpcyBjb25maWd1cmVkIGlvdmEgd2lsbCBiZSB1c2VkLg0KPiA+IEVhcmxpZXIgd2Ugd2Vy
ZSB1c2luZyBpb21tdSBpbnRlcmZhY2UgZm9yIGdldHRpbmcgdGhlIGNvbmZpZ3VyZWQgaW92YQ0K
PiA+IHdoaWNoIHdhcyBub3QgY3VycmVjdCBhbmQgQWxleCBXaWxsaWFtc29uIHN1Z2dlZXN0ZWQg
dGhpcyB0eXBlIG9mIGludGVyZmFjZS4NCj4gPg0KPiA+IHBhdGNoIDgvOSBtb3ZlcyBzb21lIGNv
bW1vbiBmdW5jdGlvbnMgaW4gYSBzZXBhcmF0ZSBmaWxlIHNvIHRoYXQgdGhlc2UNCj4gPiBjYW4g
YmUgdXNlZCBieSBGU0xfUEFNVSBpbXBsZW1lbnRhdGlvbiAobmV4dCBwYXRjaCB1c2VzIHRoaXMp
Lg0KPiA+IFRoZXNlIHdpbGwgYmUgdXNlZCBsYXRlciBmb3IgaW9tbXUtbm9uZSBpbXBsZW1lbnRh
dGlvbi4gSSBiZWxpZXZlIHdlDQo+ID4gY2FuIGRvIG1vcmUgb2YgdGhpcyBidXQgd2lsbCB0YWtl
IHN0ZXAgYnkgc3RlcC4NCj4gPg0KPiA+IEZpbmFsbHkgbGFzdCBwYXRjaCBhY3R1YWxseSBhZGRz
IHRoZSBzdXBwb3J0IGZvciBGU0wtUEFNVSA6KQ0KPiANCj4gUGF0Y2hlcyAxLTM6IG1zaV9nZXRf
cmVnaW9uIG5lZWRzIHRvIHJldHVybiBhbiBlcnJvciBhbiBlcnJvciAocHJvYmFibHkNCj4gLUVJ
TlZBTCkgaWYgY2FsbGVkIG9uIGEgcGF0aCB3aGVyZSB0aGVyZSdzIG5vIGJhY2tlbmQgaW1wbGVt
ZW50YXRpb24uDQo+IE90aGVyd2lzZSB0aGUgY2FsbGVyIGRvZXNuJ3Qga25vdyB0aGF0IHRoZSBk
YXRhIGluIHRoZSByZWdpb24gcG9pbnRlciBpc24ndA0KPiB2YWxpZC4NCg0Kd2lsbCBjb3JyZWN0
Lg0KDQo+IA0KPiBQYXRjaGVzIDUmNjogc2FtZSBhcyBhYm92ZSBmb3IgbXNpX3NldF9pb3ZhLCBy
ZXR1cm4gYW4gZXJyb3IgaWYgbm8gYmFja2VuZA0KPiBpbXBsZW1lbnRhdGlvbi4NCg0KT2sNCg0K
PiANCj4gUGF0Y2ggNzogV2h5IGRvZXMgZnNsX21zaV9kZWxfaW92YV9kZXZpY2UgYm90aGVyIHRv
IHJldHVybiBhbnl0aGluZyBpZiBpdCdzDQo+IGFsd2F5cyB6ZXJvPyAgUmV0dXJuIC1FTk9ERVYg
d2hlbiBub3QgZm91bmQ/DQoNCldpbGwgbWFrZSAtRU5PREVWLg0KDQo+IA0KPiBQYXRjaCA5Og0K
PiANCj4gdmZpb19oYW5kbGVfZ2V0X2F0dHIoKSBwYXNzZXMgcmFuZG9tIGtlcm5lbCBkYXRhIGJh
Y2sgdG8gdXNlcnNwYWNlIGluIHRoZSBldmVudA0KPiBvZiBpb21tdV9kb21haW5fZ2V0X2F0dHIo
KSBlcnJvci4NCg0KV2lsbCBjb3JyZWN0Lg0KDQo+IA0KPiB2ZmlvX2hhbmRsZV9zZXRfYXR0cigp
OiBJIGRvbid0IHNlZSBhbnkgZGF0YSB2YWxpZGF0aW9uIGhhcHBlbmluZywgaXMNCj4gaW9tbXVf
ZG9tYWluX3NldF9hdHRyKCkgcmVhbGx5IHRoYXQgc2FmZT8NCg0KV2UgZG8gbm90IG5lZWQgYW55
IGRhdGEgdmFsaWRhdGlvbiBoZXJlIGFuZCBpb21tdSBkcml2ZXIgZG9lcyB3aGF0ZXZlciBuZWVk
ZWQuDQpTbyB5ZXMsICBpb21tdV9kb21haW5fc2V0X2F0dHIoKSBpcyBzYWZlLg0KDQo+IA0KPiBG
b3IgYm90aCBvZiB0aG9zZSwgZHJvcCB0aGUgcHJfZXJyIG9uIHVua25vd24gYXR0cmlidXRlLCBp
dCdzIHN1ZmZpY2llbnQgdG8NCj4gcmV0dXJuIGVycm9yLg0KDQpvaw0KDQo+IA0KPiBJcyBWRklP
X0lPTU1VX1BBTVVfR0VUX01TSV9CQU5LX0NPVU5UIHBlciBhcGVydHVyZSAoaWUuIGVhY2ggdmZp
byB1c2VyIGhhcw0KPiAkQ09VTlQgcmVnaW9ucyBhdCB0aGVpciBkaXNwb3NhbCBleGNsdXNpdmVs
eSk/DQoNCk51bWJlciBvZiBtc2ktYmFuayBjb3VudCBpcyBzeXN0ZW0gd2lkZSBhbmQgbm90IHBl
ciBhcGVydHVyZSwgQnV0IHdpbGwgYmUgc2V0dGluZyB3aW5kb3dzIGZvciBiYW5rcyBpbiB0aGUg
ZGV2aWNlIGFwZXJ0dXJlLg0KU28gc2F5IGlmIHdlIGFyZSBkaXJlY3QgYXNzaWduaW5nIDIgcGNp
IGRldmljZSAoYm90aCBoYXZlIGRpZmZlcmVudCBpb21tdSBncm91cCwgc28gMiBhcGVydHVyZSBp
biBpb21tdSkgdG8gVk0uDQpOb3cgcWVtdSBjYW4gbWFrZSBvbmx5IG9uZSBjYWxsIHRvIGtub3cg
aG93IG1hbnkgbXNpLWJhbmtzIGFyZSB0aGVyZSBidXQgaXQgbXVzdCBzZXQgc3ViLXdpbmRvd3Mg
Zm9yIGFsbCBiYW5rcyBmb3IgYm90aCBwY2kgZGV2aWNlIGluIGl0cyByZXNwZWN0aXZlIGFwZXJ0
dXJlLg0KDQpUaGFua3MNCi1CaGFyYXQNCg0KPiAgVGhhbmtzLA0KPiANCj4gQWxleA0KPiANCj4g
PiB2MS0+djINCj4gPiAgLSBBZGRlZCBpbnRlcmZhY2UgZm9yIHNldHRpbmcgbXNpIGlvdmEgZm9y
IGEgbXNpIHJlZ2lvbiBmb3IgYSBkZXZpY2UuDQo+ID4gICAgRWFybGllciBJIGFkZGVkIGlvbW11
IGludGVyZmFjZSBmb3Igc2FtZSBidXQgYXMgcGVyIGNvbW1lbnQgdGhhdCBpcw0KPiA+ICAgIHJl
bW92ZWQgYW5kIG5vdyBjcmVhdGVkIGEgZGlyZWN0IGludGVyZmFjZSBiZXR3ZWVuIHZmaW8gYW5k
IG1zaS4NCj4gPiAgLSBJbmNvcnBvcmF0ZWQgcmV2aWV3IGNvbW1lbnRzIChkZXRhaWxzIGlzIGlu
IGluZGl2aWR1YWwgcGF0Y2gpDQo+ID4NCj4gPiBCaGFyYXQgQmh1c2hhbiAoOSk6DQo+ID4gICBw
Y2k6bXNpOiBhZGQgd2VhayBmdW5jdGlvbiBmb3IgcmV0dXJuaW5nIG1zaSByZWdpb24gaW5mbw0K
PiA+ICAgcGNpOiBtc2k6IGV4cG9zZSBtc2kgcmVnaW9uIGluZm9ybWF0aW9uIGZ1bmN0aW9ucw0K
PiA+ICAgcG93ZXJwYzogcGNpOiBBZGQgYXJjaCBzcGVjaWZpYyBtc2kgcmVnaW9uIGludGVyZmFj
ZQ0KPiA+ICAgcG93ZXJwYzogbXNpOiBFeHRlbmQgdGhlIG1zaSByZWdpb24gaW50ZXJmYWNlIHRv
IGdldCBpbmZvIGZyb20NCj4gPiAgICAgZnNsX21zaQ0KPiA+ICAgcGNpL21zaTogaW50ZXJmYWNl
IHRvIHNldCBhbiBpb3ZhIGZvciBhIG1zaSByZWdpb24NCj4gPiAgIHBvd2VycGM6IHBjaTogRXh0
ZW5kIG1zaSBpb3ZhIHBhZ2Ugc2V0dXAgdG8gYXJjaCBzcGVjaWZpYw0KPiA+ICAgcGNpOiBtc2k6
IEV4dGVuZCBtc2kgaW92YSBzZXR0aW5nIGludGVyZmFjZSB0byBwb3dlcnBjIGFyY2gNCj4gPiAg
IHZmaW86IG1vdmluZyBzb21lIGZ1bmN0aW9ucyBpbiBjb21tb24gZmlsZQ0KPiA+ICAgdmZpbyBw
Y2k6IEFkZCB2ZmlvIGlvbW11IGltcGxlbWVudGF0aW9uIGZvciBGU0xfUEFNVQ0KPiA+DQo+ID4g
IGFyY2gvcG93ZXJwYy9pbmNsdWRlL2FzbS9tYWNoZGVwLmggfCAgIDEwICsNCj4gPiAgYXJjaC9w
b3dlcnBjL2tlcm5lbC9tc2kuYyAgICAgICAgICB8ICAgMjggKw0KPiA+ICBhcmNoL3Bvd2VycGMv
c3lzZGV2L2ZzbF9tc2kuYyAgICAgIHwgIDEzMiArKysrKy0NCj4gPiAgYXJjaC9wb3dlcnBjL3N5
c2Rldi9mc2xfbXNpLmggICAgICB8ICAgMjUgKy0NCj4gPiAgZHJpdmVycy9wY2kvbXNpLmMgICAg
ICAgICAgICAgICAgICB8ICAgMzUgKysNCj4gPiAgZHJpdmVycy92ZmlvL0tjb25maWcgICAgICAg
ICAgICAgICB8ICAgIDYgKw0KPiA+ICBkcml2ZXJzL3ZmaW8vTWFrZWZpbGUgICAgICAgICAgICAg
IHwgICAgNSArLQ0KPiA+ICBkcml2ZXJzL3ZmaW8vdmZpb19pb21tdV9jb21tb24uYyAgIHwgIDIy
NyArKysrKysrKw0KPiA+ICBkcml2ZXJzL3ZmaW8vdmZpb19pb21tdV9jb21tb24uaCAgIHwgICAy
NyArDQo+ID4gIGRyaXZlcnMvdmZpby92ZmlvX2lvbW11X2ZzbF9wYW11LmMgfCAxMDAzDQo+ICsr
KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKw0KPiA+ICBkcml2ZXJzL3ZmaW8vdmZp
b19pb21tdV90eXBlMS5jICAgIHwgIDIwNiArLS0tLS0tLS0NCj4gPiAgaW5jbHVkZS9saW51eC9t
c2kuaCAgICAgICAgICAgICAgICB8ICAgMTQgKw0KPiA+ICBpbmNsdWRlL2xpbnV4L3BjaS5oICAg
ICAgICAgICAgICAgIHwgICAyMSArDQo+ID4gIGluY2x1ZGUvdWFwaS9saW51eC92ZmlvLmggICAg
ICAgICAgfCAgMTAwICsrKysNCj4gPiAgMTQgZmlsZXMgY2hhbmdlZCwgMTYyMyBpbnNlcnRpb25z
KCspLCAyMTYgZGVsZXRpb25zKC0pICBjcmVhdGUgbW9kZQ0KPiA+IDEwMDY0NCBkcml2ZXJzL3Zm
aW8vdmZpb19pb21tdV9jb21tb24uYyAgY3JlYXRlIG1vZGUgMTAwNjQ0DQo+ID4gZHJpdmVycy92
ZmlvL3ZmaW9faW9tbXVfY29tbW9uLmggIGNyZWF0ZSBtb2RlIDEwMDY0NA0KPiA+IGRyaXZlcnMv
dmZpby92ZmlvX2lvbW11X2ZzbF9wYW11LmMNCj4gPg0KPiA+DQo+ID4gLS0NCj4gPiBUbyB1bnN1
YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUNCj4gPiBs
aW51eC1rZXJuZWwiIGluIHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5r
ZXJuZWwub3JnDQo+ID4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVs
Lm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo+ID4gUGxlYXNlIHJlYWQgdGhlIEZBUSBhdCAgaHR0
cDovL3d3dy50dXgub3JnL2xrbWwvDQo+IA0KPiANCj4gDQoNCg==
^ permalink raw reply
* RE: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU (PAMU)
From: Varun Sethi @ 2013-11-21 11:20 UTC (permalink / raw)
To: Alex Williamson, Bharat Bhushan
Cc: linux-pci@vger.kernel.org, agraf@suse.de, Stuart Yoder,
bhelgaas@google.com, iommu@lists.linux-foundation.org, Scott Wood,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <1384973243.2879.361.camel@ul30vt.home>
> -----Original Message-----
> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> Sent: Thursday, November 21, 2013 12:17 AM
> To: Bhushan Bharat-R65777
> Cc: linux-pci@vger.kernel.org; agraf@suse.de; Yoder Stuart-B08248; Wood
> Scott-B07421; iommu@lists.linux-foundation.org; bhelgaas@google.com;
> linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 0/9 v2] vfio-pci: add support for Freescale IOMMU
> (PAMU)
>=20
> On Tue, 2013-11-19 at 10:47 +0530, Bharat Bhushan wrote:
> > From: Bharat Bhushan <bharat.bhushan@freescale.com>
> >
> > PAMU (FSL IOMMU) has a concept of primary window and subwindows.
> > Primary window corresponds to the complete guest iova address space
> > (including MSI space), with respect to IOMMU_API this is termed as
> > geometry. IOVA Base of subwindow is determined from the number of
> > subwindows (configurable using iommu API).
> > MSI I/O page must be within the geometry and maximum supported
> > subwindows, so MSI IO-page is setup just after guest memory iova space.
> >
> > So patch 1/9-4/9(inclusive) are for defining the interface to get:
> > - Number of MSI regions (which is number of MSI banks for powerpc)
> > - MSI-region address range: Physical page which have the
> > address/addresses used for generating MSI interrupt
> > and size of the page.
> >
> > Patch 5/9-7/9(inclusive) is defining the interface of setting up MSI
> > iova-base for a msi region(bank) for a device. so that when
> > msi-message will be composed then this configured iova will be used.
> > Earlier we were using iommu interface for getting the configured iova
> > which was not currect and Alex Williamson suggeested this type of
> interface.
> >
> > patch 8/9 moves some common functions in a separate file so that these
> > can be used by FSL_PAMU implementation (next patch uses this).
> > These will be used later for iommu-none implementation. I believe we
> > can do more of this but will take step by step.
> >
> > Finally last patch actually adds the support for FSL-PAMU :)
>=20
> Patches 1-3: msi_get_region needs to return an error an error (probably
> -EINVAL) if called on a path where there's no backend implementation.
> Otherwise the caller doesn't know that the data in the region pointer
> isn't valid.
>=20
> Patches 5&6: same as above for msi_set_iova, return an error if no
> backend implementation.
>=20
> Patch 7: Why does fsl_msi_del_iova_device bother to return anything if
> it's always zero? Return -ENODEV when not found?
>=20
> Patch 9:
>=20
> vfio_handle_get_attr() passes random kernel data back to userspace in the
> event of iommu_domain_get_attr() error.
>=20
> vfio_handle_set_attr(): I don't see any data validation happening, is
> iommu_domain_set_attr() really that safe?
[Sethi Varun-B16395] The parameter validation can be left to the lower leve=
l iommu driver. The attribute could be specific to a given hardware.
-Varun
^ permalink raw reply
* Re: [PATCH v2] panic: Make panic_timeout configurable
From: Michael Ellerman @ 2013-11-21 11:16 UTC (permalink / raw)
To: Jason Baron
Cc: Felipe Contreras, linux-kernel@vger.kernel.org,
ralf@linux-mips.org, paulus@samba.org, Andrew Morton,
linuxppc-dev, Ingo Molnar
In-Reply-To: <528BE05E.8090501@akamai.com>
On Tue, Nov 19, 2013 at 05:04:14PM -0500, Jason Baron wrote:
> On 11/19/2013 02:09 AM, Ingo Molnar wrote:
> >
> > * Jason Baron <jbaron@akamai.com> wrote:
> >
> >> On 11/18/2013 05:30 PM, Andrew Morton wrote:
> >>> On Mon, 18 Nov 2013 21:04:36 +0000 (GMT) Jason Baron <jbaron@akamai.com> wrote:
> >>>
> >>>> The panic_timeout value can be set via the command line option 'panic=x', or via
> >>>> /proc/sys/kernel/panic, however that is not sufficient when the panic occurs
> >>>> before we are able to set up these values. Thus, add a CONFIG_PANIC_TIMEOUT
> >>>> so that we can set the desired value from the .config.
> >>>>
> >>>> The default panic_timeout value continues to be 0 - wait forever,
> >>>> except for powerpc and mips, which have been defaulted to 180 and
> >>>> 5 respectively. This is in keeping with the fact that these
> >>>> arches already set panic_timeout in their arch init code.
> >>>> However, I found three exceptions- two in mips and one in powerpc
> >>>> where the settings didn't match these default values. In those
> >>>> cases, I left the arch code so it continues to override, in case
> >>>> the user has not changed from the default. It would nice if these
> >>>> arches had one default value, or if we could determine the
> >>>> correct setting at compile-time.
...
>
> Sure, I can round up all the related patches in this area that make
> sense and re-submit as a series.
>
> Felipe, would the CONFIG_PANIC_TIMEOUT=xx .config parameter work for your
> needs, or would you still like to see the command-line processing moved
> up?
>
> I'd also like to hear from the PowerPC folks about the arch defaults
> there. Now, that mips is ok with CONFIG_PANIC_TIMEOUT, PowerPC is the
> only arch doing specific initialization of 'panic_timeout'.
Hi Jason,
I think we'd like to choose the value at runtime, as we do now. The
powerpc arch supports a wide spread of different hardware, so it's nice
to be able to customise the value based on the platform. Also we build a
single kernel that boots on many platforms, and so we can't pick the
value at compile time.
cheers
^ permalink raw reply
* Re: [PATCH v5 01/17] powerpc/fsl-pci: improve clock API use
From: Gerhard Sittig @ 2013-11-21 9:21 UTC (permalink / raw)
To: Scott Wood
Cc: Mike Turquette, Detlev Zundel, Minghuan Lian, Paul Mackerras,
Anatolij Gustschin, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1384900872.1403.381.camel@snotra.buserror.net>
[ summary: the PCI driver change of mine looks innocent yet
raises questions (not for the current situation, but in the
face of potential future changes); these concerns were not
introduced by me but were "inherited" from the former
implementation, as I understand it
let's drop my patch now, have the Layerscape series show up,
and add proper clock handling to the PCI peripheral driver
later, while in the meantime either nothing needs to be done
(83xx, 85xx, 86xx) or workarounds do their job (512x)
should 8xxx platforms want to introduce CCF support, they can
and may apply the same workaround as 512x ]
On Tue, Nov 19, 2013 at 16:41 -0600, Scott Wood wrote:
>
> On Mon, 2013-11-18 at 00:06 +0100, Gerhard Sittig wrote:
> > make the Freescale PCI driver get, prepare and enable the PCI clock
> > during probe(); the clock gets put upon device shutdown by the devm
> > approach
> >
> > clock lookup is non-fatal as not all platforms may provide clock specs
> > in their device tree or implement a device tree based clock provider,
> > but failure to enable clocks after successful lookup is fatal
> >
> > the driver appears to not have a remove() routine, so no reference to
> > the clock is kept during use, and the clock isn't released (the devm
> > approach will put the clock, but it won't get disabled or unprepared)
> >
> > the 85xx/86xx platforms go through the probe() routine, where clock
> > lookup occurs and the clock gets acquired if one was specified; the
> > 512x/83xx platforms don't pass through probe() but instead directly call
> > the add_bridge() routine at a point in time where the clock provider has
> > not been setup yet even if the platform implements one -- add comments
> > to the code paths as a reminder for the potential need of a workaround
> > in the platform's clock driver, and to keep awareness if code should get
> > re-arranged or moved
> >
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Gerhard Sittig <gsi@denx.de>
> > ---
> > arch/powerpc/sysdev/fsl_pci.c | 52 +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
>
> Please coordinate this change with Minghuan Lian's patchset (posted Oct
> 23) to move the bulk of this driver outside of arch/powerpc.
Ah, you Cc'ed him, good. I spotted an earlier RFC submission,
but somehow missed more recent updates. With the move of the
fsl_pci driver into pcie my comments partially turn into lies as
the driver will grow .remove() support.
What's necessary upon remove is along the lines of b3bfce2b "i2c:
mpc: cleanup clock API use" or 2771399a "fs_enet: cleanup clock
API use" (when clocks were not acquired at all) or something like
180890c7 "mtd: mpc5121_nfc: cleanup clock API use" or 7282bdb2
"USB: fsl-mph-dr-of: cleanup clock API use" (when clocks were
acquired but not fully prepared). With the introduction of
remove support this is just about keeping a reference to the
acquired clock to disable and unprepare it, putting the clock is
done by the devm mechanism.
For the time being I'd suggest to skip this PCI driver clock API
use cleanup patch of mine if the Layerscape PCI patch is more
probable to make it into mainline, and I shall re-submit after
the Layerscape patch was applied.
If my patch is taken before the Layerscape change, then the
latter would have to "postprocess" and update mine. Which would
be straight forward but might be out of the scope of a "move
code" commit.
I'm fine with either approach, dropping my PCI patch now, or
applying it and adding PCI clock release to .remove within the
Layerscape series.
> > diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> > index ccfb50ddfe38..efa0916f61b6 100644
> > --- a/arch/powerpc/sysdev/fsl_pci.c
> > +++ b/arch/powerpc/sysdev/fsl_pci.c
> > @@ -17,6 +17,8 @@
> > * Free Software Foundation; either version 2 of the License, or (at your
> > * option) any later version.
> > */
> > +
> > +#include <linux/clk.h>
> > #include <linux/kernel.h>
> > #include <linux/pci.h>
> > #include <linux/delay.h>
> > @@ -755,6 +757,32 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
> > const int *bus_range;
> > int primary;
> >
> > + /*
> > + * 85xx/86xx platforms take the path through the probe() routine
> > + * as one would expect, PCI related clocks get acquired there if
> > + * specified
> > + *
> > + * 83xx/512x _don't_ pass through probe(), this add_bridge()
> > + * routine instead is called from within .setup_arch() at a
> > + * point in time where clock providers haven't been setup yet;
> > + * so clocks cannot get acquired here -- lookup would always
> > + * fail even on those platforms which implement the provider
> > + *
> > + * there is no counterpart for add_bridge() just like there is
> > + * no remove() counterpart for probe(), so in either case the
> > + * PCI related clock won't get released, and all of the
> > + * 512x/83xx/85xx/86xx platforms behave in identical ways
>
> How is it identical if 85xx/86xx will acquire a clock in probe(), but
> 83xx/512x can't acquire it in add_bridge()?
>
> Could you explain the relevance of releasing clocks here?
The situation I found was that none of the platforms which use
this fsl_pci driver did acquire any PCI related clock, all of
them "just used the hardware" and assumed that "someone" will
have setup and enabled the clock (some boot loader maybe).
The structure of the fsl_pci driver with its attaching but not
detaching from the hardware made things easy. I assumed that
this is because there may be boot media or mass storage connected
to PCI which you don't want to lose and which you don't want to
disconnect from upon shutdown.
So the previous situation for all platforms is
- 512x, 83xx, 85xx, 86xx: don't setup the clock, just use the
hardware, just works
What my PCI driver clock API use cleanup patch does is to add a
clock lookup to the .probe() routine, fail silently if there is
no clock spec or no clock provider, do setup the clock when a
spec was found.
So the intermediate situation for all platforms is
- 512x, 83xx: initialization in .setup_arch(), call to
add_bridge(), don't setup the clock, just use the hardware,
still works
- 85xx, 86xx: initialization in .probe(), clock lookup, silent
failure, don't setup the clock, just use the hardware, still
works
Then CCF support gets added to 512x, including migration support.
The situation then is
- 512x: .setup_arch calls add_bridge, nothing is done to the
clock; CCF provider registers the PCI clock item, pre-enables
the clock item; late init finds the PCI clock "in use"
(pre-enabled) and leaves it enabled; things keep working
- 83xx: .setup_arch and add_bridge, no clock handling, no CCF
provider either, so no "disable unused" step, all the same
- 85xx, 86xx: .probe, no clock handling, no CCF "disable
unused", all the same
Then peripheral drivers get adjusted after CCF has become
available, nothing changes for PCI here.
Then CCF migration support gets removed for 512x. The situation
is then:
- 512x: .setup_arch and add_bridge, no clock related setup; CCF
provider registers PCI clock item, leaves it alone in the
absence of PCI related OF nodes, pre-enables it in the presence
of PCI related OF nodes; CCF "disable unuses" leaves PCI clock
active, PCI still operational
- 83xx, 85xx, 86xx: no change
So before my series, "someone" (a bootloader) sets up the PCI
clock, the driver does nothing with it.
With my series, the CCF provider introduces a PCI clock item, it
gets enabled and remains active, the driver still does nothing to
the clock item. Those platforms without a CCF provider won't
disable the clock item.
Other series may introduce CCF providers for 83xx/85xx/86xx,
these might apply the same workarounds for migration as the 512x
code does.
Yes, I am aware of CCF code for "corenet". But my understanding
is that this only manages a few PLLs and only applies to CPU
cores, not to peripherals and the complete clock tree.
> > + *
> > + * this comment is here to "keep the balance" against the
> > + * probe() routine, and as a reminder to acquire clocks if the
> > + * add_bridge() call should move to some later point in time
> > + *
> > + * until then clock providers are expected to work around the
> > + * peripheral driver's not acquiring the PCI clock on those
> > + * platforms where clock providers exist, while nothing needs to
> > + * be done for those platforms without a clock provider
> > + */
>
> What would be involved in moving 83xx/512x to use .probe() as well?
Oh, that's a completely different can of worms. I dare to judge
what my approach does, as it is minimal and keeps the order of
steps in place.
Rearranging the PCI initialization order is way beyond my
capabilities (both in coding as well as testing). Considering
that I only noticed rather late that 512x won't pass through
probe() and that I might even have broken compilation with an
early submission, I'd rather not start fiddling with this stuff.
My contribution would just not be helpful.
> > is_mpc83xx_pci = 1;
> >
> > if (!of_device_is_available(dev)) {
> > @@ -1086,9 +1114,33 @@ void fsl_pci_assign_primary(void)
> >
> > static int fsl_pci_probe(struct platform_device *pdev)
> > {
> > + struct clk *clk;
> > int ret;
> > struct device_node *node;
> >
> > + /*
> > + * clock lookup is non-fatal since the driver is shared among
> > + * platforms and not all of them provide clocks specs in their
> > + * device tree, but failure to enable a specified clock is
> > + * considered fatal
> > + *
> > + * note that only the 85xx and 86xx platforms pass through this
> > + * probe() routine, while 83xx and 512x directly invoke the
> > + * mpc83xx_add_bridge() routine from within .setup_arch() code
> > + */
> > + clk = devm_clk_get(&pdev->dev, "ipg");
> > + if (!IS_ERR(clk)) {
> > + ret = clk_prepare_enable(clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "Could not enable PCI clock\n");
> > + return ret;
> > + }
> > + /*
> > + * TODO where to store the 'clk' reference? there appears
> > + * to be no remove() routine which undoes what probe() does
> > + */
> > + }
>
> There is a .remove(); this driver just doesn't support it.
>
> As for where to store things, you could turn private_data into a struct
> rather than a direct iomem pointer. Or just replace the comment with a
> non-TODO statement that says we'll never release the clock because the
> PCI controller driver is non-removable.
Re-considering the above, the most appropriate thing to do may be
to drop my patch now, and see how the Layerscape series evolves.
Once there is a PCI driver which works on all the platforms and
has the probe and remove counterparts, adding proper clock
handling to the peripheral driver is straight forward (along the
lines of the above mentioned i2c and fs_enet changes).
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
* [PATCH v10] PPC: POWERNV: move iommu_add_device earlier
From: Alexey Kardashevskiy @ 2013-11-21 6:43 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, Alex Graf, Bharat Bhushan, linux-kernel
The current implementation of IOMMU on sPAPR does not use iommu_ops
and therefore does not call IOMMU API's bus_set_iommu() which
1) sets iommu_ops for a bus
2) registers a bus notifier
Instead, PCI devices are added to IOMMU groups from
subsys_initcall_sync(tce_iommu_init) which does basically the same
thing without using iommu_ops callbacks.
However Freescale PAMU driver (https://lkml.org/lkml/2013/7/1/158)
implements iommu_ops and when tce_iommu_init is called, every PCI device
is already added to some group so there is a conflict.
This patch does 2 things:
1. removes the loop in which PCI devices were added to groups and
adds explicit iommu_add_device() calls to add devices as soon as they get
the iommu_table pointer assigned to them.
2. moves a bus notifier to powernv code in order to avoid conflict with
the notifier from Freescale driver.
iommu_add_device() and iommu_del_device() are public now.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v10:
* fixed linker error when IOMMU_API is not enabled (thanks to Jeremy Kerr)
v9:
* removed "KVM" from the subject as it is not really a KVM patch so
PPC mainainter (hi Ben!) can review/include it into his tree
v8:
* added the check for iommu_group!=NULL before removing device from a group
as suggested by Wei Yang <weiyang@linux.vnet.ibm.com>
v2:
* added a helper - set_iommu_table_base_and_group - which does
set_iommu_table_base() and iommu_add_device()
---
arch/powerpc/include/asm/iommu.h | 26 ++++++++++++++++
arch/powerpc/kernel/iommu.c | 48 +++--------------------------
arch/powerpc/platforms/powernv/pci-ioda.c | 8 ++---
arch/powerpc/platforms/powernv/pci-p5ioc2.c | 2 +-
arch/powerpc/platforms/powernv/pci.c | 33 +++++++++++++++++++-
arch/powerpc/platforms/pseries/iommu.c | 8 +++--
6 files changed, 72 insertions(+), 53 deletions(-)
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index c34656a..774fa27 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -101,8 +101,34 @@ extern void iommu_free_table(struct iommu_table *tbl, const char *node_name);
*/
extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
int nid);
+#ifdef CONFIG_IOMMU_API
extern void iommu_register_group(struct iommu_table *tbl,
int pci_domain_number, unsigned long pe_num);
+extern int iommu_add_device(struct device *dev);
+extern void iommu_del_device(struct device *dev);
+#else
+static inline void iommu_register_group(struct iommu_table *tbl,
+ int pci_domain_number,
+ unsigned long pe_num)
+{
+}
+
+static inline int iommu_add_device(struct device *dev)
+{
+ return 0;
+}
+
+static inline void iommu_del_device(struct device *dev)
+{
+}
+#endif /* !CONFIG_IOMMU_API */
+
+static inline void set_iommu_table_base_and_group(struct device *dev,
+ void *base)
+{
+ set_iommu_table_base(dev, base);
+ iommu_add_device(dev);
+}
extern int iommu_map_sg(struct device *dev, struct iommu_table *tbl,
struct scatterlist *sglist, int nelems,
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 572bb5b..d22abe0 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1105,7 +1105,7 @@ void iommu_release_ownership(struct iommu_table *tbl)
}
EXPORT_SYMBOL_GPL(iommu_release_ownership);
-static int iommu_add_device(struct device *dev)
+int iommu_add_device(struct device *dev)
{
struct iommu_table *tbl;
int ret = 0;
@@ -1134,52 +1134,12 @@ static int iommu_add_device(struct device *dev)
return ret;
}
+EXPORT_SYMBOL_GPL(iommu_add_device);
-static void iommu_del_device(struct device *dev)
+void iommu_del_device(struct device *dev)
{
iommu_group_remove_device(dev);
}
-
-static int iommu_bus_notifier(struct notifier_block *nb,
- unsigned long action, void *data)
-{
- struct device *dev = data;
-
- switch (action) {
- case BUS_NOTIFY_ADD_DEVICE:
- return iommu_add_device(dev);
- case BUS_NOTIFY_DEL_DEVICE:
- iommu_del_device(dev);
- return 0;
- default:
- return 0;
- }
-}
-
-static struct notifier_block tce_iommu_bus_nb = {
- .notifier_call = iommu_bus_notifier,
-};
-
-static int __init tce_iommu_init(void)
-{
- struct pci_dev *pdev = NULL;
-
- BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
-
- for_each_pci_dev(pdev)
- iommu_add_device(&pdev->dev);
-
- bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
- return 0;
-}
-
-subsys_initcall_sync(tce_iommu_init);
-
-#else
-
-void iommu_register_group(struct iommu_table *tbl,
- int pci_domain_number, unsigned long pe_num)
-{
-}
+EXPORT_SYMBOL_GPL(iommu_del_device);
#endif /* CONFIG_IOMMU_API */
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 74a5a57..4daa288 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -440,7 +440,7 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb *phb, struct pci_dev *pdev
return;
pe = &phb->ioda.pe_array[pdn->pe_number];
- set_iommu_table_base(&pdev->dev, &pe->tce32_table);
+ set_iommu_table_base_and_group(&pdev->dev, &pe->tce32_table);
}
static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
@@ -448,7 +448,7 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
struct pci_dev *dev;
list_for_each_entry(dev, &bus->devices, bus_list) {
- set_iommu_table_base(&dev->dev, &pe->tce32_table);
+ set_iommu_table_base_and_group(&dev->dev, &pe->tce32_table);
if (dev->subordinate)
pnv_ioda_setup_bus_dma(pe, dev->subordinate);
}
@@ -611,7 +611,7 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb *phb,
iommu_register_group(tbl, pci_domain_nr(pe->pbus), pe->pe_number);
if (pe->pdev)
- set_iommu_table_base(&pe->pdev->dev, tbl);
+ set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
else
pnv_ioda_setup_bus_dma(pe, pe->pbus);
@@ -687,7 +687,7 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb *phb,
iommu_init_table(tbl, phb->hose->node);
if (pe->pdev)
- set_iommu_table_base(&pe->pdev->dev, tbl);
+ set_iommu_table_base_and_group(&pe->pdev->dev, tbl);
else
pnv_ioda_setup_bus_dma(pe, pe->pbus);
diff --git a/arch/powerpc/platforms/powernv/pci-p5ioc2.c b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
index b68db63..ede341b 100644
--- a/arch/powerpc/platforms/powernv/pci-p5ioc2.c
+++ b/arch/powerpc/platforms/powernv/pci-p5ioc2.c
@@ -92,7 +92,7 @@ static void pnv_pci_p5ioc2_dma_dev_setup(struct pnv_phb *phb,
pci_domain_nr(phb->hose->bus), phb->opal_id);
}
- set_iommu_table_base(&pdev->dev, &phb->p5ioc2.iommu_table);
+ set_iommu_table_base_and_group(&pdev->dev, &phb->p5ioc2.iommu_table);
}
static void __init pnv_pci_init_p5ioc2_phb(struct device_node *np, u64 hub_id,
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index a28d3b5..c005011 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -504,7 +504,7 @@ static void pnv_pci_dma_fallback_setup(struct pci_controller *hose,
pdn->iommu_table = pnv_pci_setup_bml_iommu(hose);
if (!pdn->iommu_table)
return;
- set_iommu_table_base(&pdev->dev, pdn->iommu_table);
+ set_iommu_table_base_and_group(&pdev->dev, pdn->iommu_table);
}
static void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
@@ -623,3 +623,34 @@ void __init pnv_pci_init(void)
ppc_md.teardown_msi_irqs = pnv_teardown_msi_irqs;
#endif
}
+
+static int tce_iommu_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = data;
+
+ switch (action) {
+ case BUS_NOTIFY_ADD_DEVICE:
+ return iommu_add_device(dev);
+ case BUS_NOTIFY_DEL_DEVICE:
+ if (dev->iommu_group)
+ iommu_del_device(dev);
+ return 0;
+ default:
+ return 0;
+ }
+}
+
+static struct notifier_block tce_iommu_bus_nb = {
+ .notifier_call = tce_iommu_bus_notifier,
+};
+
+static int __init tce_iommu_bus_notifier_init(void)
+{
+ BUILD_BUG_ON(PAGE_SIZE < IOMMU_PAGE_SIZE);
+
+ bus_register_notifier(&pci_bus_type, &tce_iommu_bus_nb);
+ return 0;
+}
+
+subsys_initcall_sync(tce_iommu_bus_notifier_init);
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 0307901..d9d0d7a 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -686,7 +686,8 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
iommu_table_setparms(phb, dn, tbl);
PCI_DN(dn)->iommu_table = iommu_init_table(tbl, phb->node);
iommu_register_group(tbl, pci_domain_nr(phb->bus), 0);
- set_iommu_table_base(&dev->dev, PCI_DN(dn)->iommu_table);
+ set_iommu_table_base_and_group(&dev->dev,
+ PCI_DN(dn)->iommu_table);
return;
}
@@ -698,7 +699,8 @@ static void pci_dma_dev_setup_pSeries(struct pci_dev *dev)
dn = dn->parent;
if (dn && PCI_DN(dn))
- set_iommu_table_base(&dev->dev, PCI_DN(dn)->iommu_table);
+ set_iommu_table_base_and_group(&dev->dev,
+ PCI_DN(dn)->iommu_table);
else
printk(KERN_WARNING "iommu: Device %s has no iommu table\n",
pci_name(dev));
@@ -1192,7 +1194,7 @@ static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
pr_debug(" found DMA window, table: %p\n", pci->iommu_table);
}
- set_iommu_table_base(&dev->dev, pci->iommu_table);
+ set_iommu_table_base_and_group(&dev->dev, pci->iommu_table);
}
static int dma_set_mask_pSeriesLP(struct device *dev, u64 dma_mask)
--
1.8.4.rc4
^ permalink raw reply related
* [PATCH] iommu: Add empty stub for iommu_group_get_by_id()
From: Alexey Kardashevskiy @ 2013-11-21 6:41 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Joerg Roedel, linux-kernel
Almost every function in include/linux/iommu.h has an empty stub
but the iommu_group_get_by_id() did not get one by mistake.
This adds an empty stub for iommu_group_get_by_id() for IOMMU_API
disabled config.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
include/linux/iommu.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7ea319e..3c7903d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -245,6 +245,11 @@ static inline struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
return NULL;
}
+static inline struct iommu_group *iommu_group_get_by_id(int id)
+{
+ return NULL;
+}
+
static inline void iommu_domain_free(struct iommu_domain *domain)
{
}
--
1.8.4.rc4
^ permalink raw reply related
* Re: [PATCH v9] PPC: POWERNV: move iommu_add_device earlier
From: Jeremy Kerr @ 2013-11-21 5:41 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
Cc: Bharat Bhushan, Alex Graf, linux-kernel
In-Reply-To: <1384324220-30109-1-git-send-email-aik@ozlabs.ru>
Hi Alexey,
> This patch does 2 things:
> 1. removes the loop in which PCI devices were added to groups and
> adds explicit iommu_add_device() calls to add devices as soon as they get
> the iommu_table pointer assigned to them.
> 2. moves a bus notifier to powernv code in order to avoid conflict with
> the notifier from Freescale driver.
This breaks when building with !IOMMU_API for me, as the
iommu_add_device function is declared but not defined. We'd need
something like the following (on top of your change) to work:
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 426d0ec0..04d2abbe 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -102,10 +102,27 @@ extern void iommu_free_table(struct iommu_table *tbl, cons
*/
extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
int nid);
+
+#ifdef CONFIG_IOMMU_API
extern void iommu_register_group(struct iommu_table *tbl,
int pci_domain_number, unsigned long pe_num);
extern int iommu_add_device(struct device *dev);
extern void iommu_del_device(struct device *dev);
+#else
+static inline void iommu_register_group(struct iommu_table *tbl,
+ int pci_domain_number, unsigned long pe_num)
+{
+}
+
+static inline int iommu_add_device(struct device *dev)
+{
+ return 0;
+}
+
+static inline void iommu_del_device(struct device *dev)
+{
+}
+#endif
static inline void set_iommu_table_base_and_group(struct device *dev,
void *base)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 170b2182..5a02a50f 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -1212,11 +1212,4 @@ void iommu_del_device(struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_del_device);
-#else
-
-void iommu_register_group(struct iommu_table *tbl,
- int pci_domain_number, unsigned long pe_num)
-{
-}
-
#endif /* CONFIG_IOMMU_API */
Cheers,
Jeremy
^ permalink raw reply related
* [PATCH] powerpc: allyesconfig should not select CONFIG_CPU_LITTLE_ENDIAN
From: Anton Blanchard @ 2013-11-21 5:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, paulus, fengguang.wu, Stephen Rothwell,
imunsie
Cc: linux-next, linuxppc-dev, linux-kernel
Stephen reported a failure in an allyesconfig build.
CONFIG_CPU_LITTLE_ENDIAN=y gets set but his toolchain is not
new enough to support little endian. We really want to
default to a big endian build; Ben suggested using a choice
which defaults to CPU_BIG_ENDIAN.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: b/arch/powerpc/platforms/Kconfig.cputype
===================================================================
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -404,13 +404,27 @@ config PPC_DOORBELL
endmenu
-config CPU_LITTLE_ENDIAN
- bool "Build little endian kernel"
- default n
+choice
+ prompt "Endianness selection"
+ default CPU_BIG_ENDIAN
help
This option selects whether a big endian or little endian kernel will
be built.
+config CPU_BIG_ENDIAN
+ bool "Build big endian kernel"
+ help
+ Build a big endian kernel.
+
+ If unsure, select this option.
+
+config CPU_LITTLE_ENDIAN
+ bool "Build little endian kernel"
+ help
+ Build a little endian kernel.
+
Note that if cross compiling a little endian kernel,
CROSS_COMPILE must point to a toolchain capable of targeting
little endian powerpc.
+
+endchoice
^ permalink raw reply
* powerpc: Fix missing includes needed for chroma
From: Michael Neuling @ 2013-11-21 4:40 UTC (permalink / raw)
To: benh; +Cc: Linux PPC dev, Jimi Xenidis
chroma_defconfig is horribly broken currently, so add a bunch of
#includes to fix it.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
So when are we dropping arch/powerpc/platforms/wsp?
diff --git a/arch/powerpc/platforms/wsp/chroma.c b/arch/powerpc/platforms/wsp/chroma.c
index 8ef53bc..aaa46b3 100644
--- a/arch/powerpc/platforms/wsp/chroma.c
+++ b/arch/powerpc/platforms/wsp/chroma.c
@@ -15,6 +15,7 @@
#include <linux/of.h>
#include <linux/smp.h>
#include <linux/time.h>
+#include <linux/of_fdt.h>
#include <asm/machdep.h>
#include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/wsp/h8.c b/arch/powerpc/platforms/wsp/h8.c
index d18e6cc..a3c87f3 100644
--- a/arch/powerpc/platforms/wsp/h8.c
+++ b/arch/powerpc/platforms/wsp/h8.c
@@ -10,6 +10,7 @@
#include <linux/kernel.h>
#include <linux/of.h>
#include <linux/io.h>
+#include <linux/of_address.h>
#include "wsp.h"
diff --git a/arch/powerpc/platforms/wsp/ics.c b/arch/powerpc/platforms/wsp/ics.c
index 2d3b1dd..3b782ce 100644
--- a/arch/powerpc/platforms/wsp/ics.c
+++ b/arch/powerpc/platforms/wsp/ics.c
@@ -18,6 +18,8 @@
#include <linux/smp.h>
#include <linux/spinlock.h>
#include <linux/types.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
#include <asm/io.h>
#include <asm/irq.h>
diff --git a/arch/powerpc/platforms/wsp/opb_pic.c b/arch/powerpc/platforms/wsp/opb_pic.c
index cb565bf..3f67298 100644
--- a/arch/powerpc/platforms/wsp/opb_pic.c
+++ b/arch/powerpc/platforms/wsp/opb_pic.c
@@ -15,6 +15,8 @@
#include <linux/of.h>
#include <linux/slab.h>
#include <linux/time.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
#include <asm/reg_a2.h>
#include <asm/irq.h>
diff --git a/arch/powerpc/platforms/wsp/psr2.c b/arch/powerpc/platforms/wsp/psr2.c
index 508ec82..a87b414 100644
--- a/arch/powerpc/platforms/wsp/psr2.c
+++ b/arch/powerpc/platforms/wsp/psr2.c
@@ -15,6 +15,7 @@
#include <linux/of.h>
#include <linux/smp.h>
#include <linux/time.h>
+#include <linux/of_fdt.h>
#include <asm/machdep.h>
#include <asm/udbg.h>
diff --git a/arch/powerpc/platforms/wsp/scom_wsp.c b/arch/powerpc/platforms/wsp/scom_wsp.c
index 8928507..6538b4d 100644
--- a/arch/powerpc/platforms/wsp/scom_wsp.c
+++ b/arch/powerpc/platforms/wsp/scom_wsp.c
@@ -14,6 +14,7 @@
#include <linux/of.h>
#include <linux/spinlock.h>
#include <linux/types.h>
+#include <linux/of_address.h>
#include <asm/cputhreads.h>
#include <asm/reg_a2.h>
diff --git a/arch/powerpc/platforms/wsp/wsp.c b/arch/powerpc/platforms/wsp/wsp.c
index ddb6efe..58cd1f0 100644
--- a/arch/powerpc/platforms/wsp/wsp.c
+++ b/arch/powerpc/platforms/wsp/wsp.c
@@ -13,6 +13,7 @@
#include <linux/smp.h>
#include <linux/delay.h>
#include <linux/time.h>
+#include <linux/of_address.h>
#include <asm/scom.h>
^ permalink raw reply related
* [PATCH] powerpc: Fix error when cross building TAGS & cscope
From: Michael Neuling @ 2013-11-21 3:59 UTC (permalink / raw)
To: benh; +Cc: Linux PPC dev, Ian Munsie, anton
Currently if I cross build TAGS or cscope from x86 I get this:
% make ARCH=3Dpowerpc TAGS
gcc-4.8.real: error: unrecognized command line option =E2=80=98-mbig-endi=
an=E2=80=99
GEN TAGS
%=20
I'm not setting CROSS_COMPILE=3D as logically I shouldn't need to and I
haven't needed to in the past when building TAGS or cscope. Also, the
above completess correct as the error is not fatal to the build.
This was caused by:
commit d72b08017161ab385d4ae080ea415c9eb7ceef83
Author: Ian Munsie <imunsie@au1.ibm.com>
powerpc: Add ability to build little endian kernels
The below fixes this by testing for the -mbig-endian option before
adding it.
I've not done the same thing in the little endian case as if
-mlittle-endian doesn't exist, we probably want to fail quickly as you
probably have an old big endian compiler.
Signed-off-by: Michael Neuling <mikey@neuling.org>
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 8a24636..273ace7 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -75,8 +75,10 @@ LDEMULATION :=3D lppc
GNUTARGET :=3D powerpcle
MULTIPLEWORD :=3D -mno-multiple
else
+ifeq ($(call cc-option-yn,-mbig-endian),y)
override CC +=3D -mbig-endian
override AS +=3D -mbig-endian
+endif
override LD +=3D -EB
LDEMULATION :=3D ppc
GNUTARGET :=3D powerpc
^ permalink raw reply related
* Re: linux-next: build failure after merge of the final tree (Linus' tree related - powerpc)
From: Anton Blanchard @ 2013-11-21 3:33 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: linux-next, linuxppc-dev, linux-kernel
In-Reply-To: <20131121134622.a853064a1b5a8355e5077737@canb.auug.org.au>
Hi,
> After merging the final tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
>
> <stdin>:1:0: error: -mcall-aixdesc must be big endian
Urgh, allyesconfig is building an LE kernel. Ian: do we need to reverse
the polarity of the option, and call it CONFIG_CPU_BIG_ENDIAN?
Anton
^ permalink raw reply
* linux-next: build failure after merge of the final tree (Linus' tree related - powerpc)
From: Stephen Rothwell @ 2013-11-21 2:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev
Cc: linux-next, linux-kernel, Anton Blanchard
[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]
Hi all,
After merging the final tree, today's linux-next build (powerpc
allyesconfig) failed like this:
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 31: printf: #: invalid number
scripts/gcc-version.sh: line 31: printf: #: invalid number
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 31: printf: #: invalid number
scripts/gcc-version.sh: line 31: printf: #: invalid number
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
<stdin>:1:0: error: -mcall-aixdesc must be big endian
scripts/gcc-version.sh: line 29: printf: #: invalid number
scripts/gcc-version.sh: line 29: printf: #: invalid number
scripts/gcc-version.sh: line 29: printf: #: invalid number
/bin/sh: line 0: test: 0001 6000 0160: integer expression expected
scripts/mod/empty.c:1:0: error: -mcall-aixdesc must be big endian
/* empty file to figure out endianness / word size */
^
scripts/mod/devicetable-offsets.c:1:0: error: -mcall-aixdesc must be big endian
#include <linux/kbuild.h>
^
kernel/bounds.c:1:0: error: -mcall-aixdesc must be big endian
/*
^
Caused by commit 7c105b63bd98 ("powerpc: Add CONFIG_CPU_LITTLE_ENDIAN
kernel config option").
I have reverted that commit for today.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH 2/2] powerpc: mm: change pgtable index size for 64K page
From: Liu Ping Fan @ 2013-11-21 2:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras
In-Reply-To: <1385000275-5988-1-git-send-email-pingfank@linux.vnet.ibm.com>
For 64K page, we waste half of the pte_t page. With this patch, after
changing PGD_INDEX_SIZE from 12 to 11, PTE_INDEX_SIZE from 8 to 9,
we can improve the usage of pte_t page and shrink the continuous phys
size for pgd_t.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/pgtable-ppc64-64k.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable-ppc64-64k.h b/arch/powerpc/include/asm/pgtable-ppc64-64k.h
index a56b82f..f6955ff 100644
--- a/arch/powerpc/include/asm/pgtable-ppc64-64k.h
+++ b/arch/powerpc/include/asm/pgtable-ppc64-64k.h
@@ -4,10 +4,10 @@
#include <asm-generic/pgtable-nopud.h>
-#define PTE_INDEX_SIZE 8
+#define PTE_INDEX_SIZE 9
#define PMD_INDEX_SIZE 10
#define PUD_INDEX_SIZE 0
-#define PGD_INDEX_SIZE 12
+#define PGD_INDEX_SIZE 11
#ifndef __ASSEMBLY__
#define PTE_TABLE_SIZE (sizeof(real_pte_t) << PTE_INDEX_SIZE)
--
1.8.1.4
^ permalink raw reply related
* [PATCH 1/2] powerpc: mm: use macro PGTABLE_EADDR_SIZE instead of digital
From: Liu Ping Fan @ 2013-11-21 2:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras
In case of extending the eaddr in future, use this macro
PGTABLE_EADDR_SIZE to ease the maintenance of the code.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
arch/powerpc/mm/slb_low.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
index 17aa6df..e0b3cf4 100644
--- a/arch/powerpc/mm/slb_low.S
+++ b/arch/powerpc/mm/slb_low.S
@@ -35,7 +35,7 @@ _GLOBAL(slb_allocate_realmode)
* check for bad kernel/user address
* (ea & ~REGION_MASK) >= PGTABLE_RANGE
*/
- rldicr. r9,r3,4,(63 - 46 - 4)
+ rldicr. r9,r3,4,(63 - PGTABLE_EADDR_SIZE - 4)
bne- 8f
srdi r9,r3,60 /* get region */
--
1.8.1.4
^ permalink raw reply related
* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Li Zhong @ 2013-11-21 1:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: PowerPC email list, Russell King - ARM Linux, Paul Mackerras
In-Reply-To: <1384993347.26969.124.camel@pasglop>
On Thu, 2013-11-21 at 11:22 +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2013-11-21 at 00:08 +0000, Russell King - ARM Linux wrote:
> > On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> > > On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > > > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > > > but as I've already said elsewhere, the real fix is to get whatever
> > > > created the struct device to initialise the dev->dma_mask with a
> > > > bus default.
> > > >
> > > > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > > > over the issue - it's fine to do it this way, but someone should still
> > > > fix the broken code creating these devices...
> > >
> > > Ok, they are created by the vio bus core, so it should be doing the
> > > job here of setting the dma_mask pointer to a proper value.
> > >
> > > Li, can you take care of that ? Look at other bus types we have in
> > > there such as the macio bus etc...
> >
> > Oh, hang on a moment, this is the "bus" code.
> >
> > In which case, the question becomes: do vio devices ever need to have
> > a separate streaming DMA mask from a coherent DMA mask? If not, then
> > something like the following is what's needed here, and I should've
> > never have used dma_set_mask_and_coherent().
>
> No, a single mask.
>
> > dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
> > are really supposed to be used by drivers only.
> >
> > arch/powerpc/kernel/vio.c | 3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> > index e7d0c88f621a..d771778f398e 100644
> > --- a/arch/powerpc/kernel/vio.c
> > +++ b/arch/powerpc/kernel/vio.c
> > @@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
> >
> > /* needed to ensure proper operation of coherent allocations
> > * later, in case driver doesn't set it explicitly */
> > - dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> > + viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
> > + viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
> > }
> >
> > /* register with generic device framework */
>
> Right that's exactly what I had in mind. Li, can you test this please ?
Sure, and it works.
Tested-by: Li Zhong <zhong@linux.vnet.ibm.com>
>
> The previous "fix" using dma_coerce_mask_and_coherent() is already on
> its way to Linus, so we'll rework the above patch to undo it but for
> now please test.
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [PATCH v3] KVM: PPC: vfio kvm device: support spapr tce
From: Alexey Kardashevskiy @ 2013-11-21 1:22 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm-ppc, linuxppc-dev, linux-kernel, kvm
In-Reply-To: <1384981031.2879.389.camel@ul30vt.home>
On 11/21/2013 07:57 AM, Alex Williamson wrote:
> On Wed, 2013-11-20 at 16:18 +1100, Alexey Kardashevskiy wrote:
>> In addition to the external VFIO user API, a VFIO KVM device
>> has been introduced recently.
>>
>> sPAPR TCE IOMMU is para-virtualized and the guest does map/unmap
>> via hypercalls which take a logical bus id (LIOBN) as a target IOMMU
>> identifier. LIOBNs are made up and linked to IOMMU groups by the user
>> space. In order to accelerate IOMMU operations in the KVM, we need
>> to tell KVM the information about LIOBN-to-group mapping.
>>
>> For that, a new KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN parameter
>> is added. It accepts a pair of a VFIO group fd and LIOBN.
>>
>> This also adds a new kvm_vfio_find_group_by_liobn() function which
>> receives kvm struct, LIOBN and a callback. As it increases the IOMMU
>> group use counter, the KVMr is required to pass a callback which
>> called when the VFIO group is about to be removed VFIO-KVM tracking so
>> the KVM is able to call iommu_group_put() to release the IOMMU group.
>>
>> The KVM uses kvm_vfio_find_group_by_liobn() once per KVM run and caches
>> the result in kvm_arch. iommu_group_put() for all groups will be called
>> when KVM finishes (in the SPAPR TCE in KVM enablement patch).
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v3:
>> * total rework
>> * added a release callback into kvm_vfio_find_group_by_liobn so now
>> the user of the API can get a notification if the group is about to
>> disappear
>> ---
>> Documentation/virtual/kvm/devices/vfio.txt | 19 ++++-
>> arch/powerpc/kvm/Kconfig | 1 +
>> arch/powerpc/kvm/Makefile | 3 +
>> include/linux/kvm_host.h | 18 +++++
>> include/uapi/linux/kvm.h | 7 ++
>> virt/kvm/vfio.c | 116 ++++++++++++++++++++++++++++-
>> 6 files changed, 161 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/devices/vfio.txt b/Documentation/virtual/kvm/devices/vfio.txt
>> index ef51740..7ecb3b2 100644
>> --- a/Documentation/virtual/kvm/devices/vfio.txt
>> +++ b/Documentation/virtual/kvm/devices/vfio.txt
>> @@ -16,7 +16,22 @@ Groups:
>>
>> KVM_DEV_VFIO_GROUP attributes:
>> KVM_DEV_VFIO_GROUP_ADD: Add a VFIO group to VFIO-KVM device tracking
>> + kvm_device_attr.addr points to an int32_t file descriptor
>> + for the VFIO group.
>> +
>> KVM_DEV_VFIO_GROUP_DEL: Remove a VFIO group from VFIO-KVM device tracking
>> + kvm_device_attr.addr points to an int32_t file descriptor
>> + for the VFIO group.
>> +
>> + KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: sets a liobn for a VFIO group
>> + kvm_device_attr.addr points to a struct:
>> + struct kvm_vfio_spapr_tce_liobn {
>> + __u32 argsz;
>> + __u32 fd;
>
> fds are signed, __s32
>
>> + __u32 liobn;
>> + };
>> + where
>> + @argsz is a struct size;
>> + @fd is a file descriptor for a VFIO group;
>> + @liobn is a logical bus id to be associated with the group.
>>
>> -For each, kvm_device_attr.addr points to an int32_t file descriptor
>> -for the VFIO group.
>> 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..2438d2e 100644
>> --- a/arch/powerpc/kvm/Makefile
>> +++ b/arch/powerpc/kvm/Makefile
>> @@ -87,6 +87,9 @@ kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HV) := \
>> kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>> book3s_xics.o
>>
>> +kvm-book3s_64-objs-$(CONFIG_KVM_VFIO) += \
>> + $(KVM)/vfio.o \
>> +
>> kvm-book3s_64-module-objs := \
>> $(KVM)/kvm_main.o \
>> $(KVM)/eventfd.o \
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index 88ff96a..1d2ad5e 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -1112,5 +1112,23 @@ static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
>> }
>>
>> #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
>> +
>> +typedef void (*kvm_vfio_release_group_callback)(struct kvm *kvm,
>> + unsigned long liobn);
>
> liobn was said to be __u32 in kvm_vfio_spapr_tce_liobn above, here it's
> unsigned long?
PAPR spec says it is 32 bit and kvm_vfio_spapr_tce_liobn is a binary
interface (ABI?) so I want it to be precise.
However kvmppc_get_gpr() (used to parse hypercall parameters such as liobn)
return unsigned long. So inside the kernel I use unsigned long.
So what does make sense to change here?
>
>> +
>> +#if defined(CONFIG_KVM_VFIO) && defined(CONFIG_SPAPR_TCE_IOMMU)
>> +
>> +extern struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> + unsigned long liobn, kvm_vfio_release_group_callback cb);
>> +
>> +#else
>> +
>> +static inline struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> + unsigned long liobn, ikvm_vfio_release_group_callback cb)
>> +{
>> + return NULL;
>> +}
>> +#endif /* CONFIG_KVM_VFIO && CONFIG_SPAPR_TCE_IOMMU */
>> +
>> #endif
>>
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 7c1a349..3d77dde 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -847,6 +847,13 @@ 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_GROUP_SET_SPAPR_TCE_LIOBN 3
>> +
>> +struct kvm_vfio_spapr_tce_liobn {
>> + __u32 argsz;
>> + __u32 fd;
>> + __u32 liobn;
>> +};
>>
>> /*
>> * ioctls for VM fds
>> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
>> index ca4260e..448910d 100644
>> --- a/virt/kvm/vfio.c
>> +++ b/virt/kvm/vfio.c
>> @@ -22,6 +22,12 @@
>> struct kvm_vfio_group {
>> struct list_head node;
>> struct vfio_group *vfio_group;
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + struct {
>> + unsigned long liobn;
>> + kvm_vfio_release_group_callback cb;
>> + } spapr_tce;
>> +#endif
>> };
>>
>> struct kvm_vfio {
>> @@ -59,6 +65,51 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
>> symbol_put(vfio_group_put_external_user);
>> }
>>
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> +struct iommu_group *kvm_vfio_find_group_by_liobn(struct kvm *kvm,
>> + unsigned long liobn, kvm_vfio_release_group_callback cb)
>> +{
>> + struct kvm_vfio_group *kvg;
>> + int group_id;
>> + struct iommu_group *grp;
>> + struct kvm_vfio *kv = NULL;
>> + struct kvm_device *tmp;
>> +
>> + if (!cb)
>> + return NULL;
>
> Is it worthwhile to use ERR_PTR(-EINVAL) here and the caller can use
> IS_ERR()?
>
>> +
>> + /* Find a VFIO KVM device */
>> + list_for_each_entry(tmp, &kvm->devices, vm_node) {
>> + if (tmp->ops != &kvm_vfio_ops)
>> + continue;
>> +
>> + kv = tmp->private;
>> + break;
>> + }
>> +
>> + if (!kv)
>> + return NULL;
>
> ERR_PTR(-EFAULT)? EIO?
>
>> +
>> + /* Find a group */
>
> Still ignoring kv->lock
>
>> + list_for_each_entry(kvg, &kv->group_list, node) {
>> + if (kvg->spapr_tce.liobn != liobn)
>> + continue;
>> +
>> + if (kvg->spapr_tce.cb)
>> + return NULL;
>
> ERR_PTR(-EBUSY)?
>
>> +
>> + kvg->spapr_tce.cb = cb;
>> + group_id = vfio_external_user_iommu_id(kvg->vfio_group);
>> + grp = iommu_group_get_by_id(group_id);
>> +
>> + return grp;
>> + }
>> +
>> + return NULL;
>
> ERR_PTR(-ENODEV)?
>
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_vfio_find_group_by_liobn);
>> +#endif
>> +
>> /*
>> * Groups can use the same or different IOMMU domains. If the same then
>> * adding a new group may change the coherency of groups we've previously
>> @@ -170,6 +221,11 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>> continue;
>>
>> list_del(&kvg->node);
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + if (kvg->spapr_tce.cb)
>> + kvg->spapr_tce.cb(dev->kvm,
>> + kvg->spapr_tce.liobn);
>> +#endif
>> kvm_vfio_group_put_external_user(kvg->vfio_group);
>> kfree(kvg);
>> ret = 0;
>> @@ -183,6 +239,62 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg)
>> kvm_vfio_update_coherency(dev);
>>
>> return ret;
>> +
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN: {
>> + struct kvm_vfio_spapr_tce_liobn param;
>> + unsigned long minsz;
>> + struct kvm_vfio *kv = dev->private;
>> + struct vfio_group *vfio_group;
>> + struct kvm_vfio_group *kvg;
>> + struct fd f;
>> +
>> + minsz = offsetofend(struct kvm_vfio_spapr_tce_liobn, liobn);
>> +
>> + if (copy_from_user(¶m, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (param.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (copy_from_user(¶m, (void __user *)arg, minsz))
>> + return -EFAULT;
>
> copy_from_user twice? Extra copy here?
Oh. All I wanted was to read minsz first but I cut-n-pasted too blindely :)
>
>> +
>> + f = fdget(param.fd);
>> + if (!f.file)
>> + return -EBADF;
>> +
>> + vfio_group = kvm_vfio_group_get_external_user(f.file);
>> + fdput(f);
>> +
>> + if (IS_ERR(vfio_group))
>> + return PTR_ERR(vfio_group);
>> +
>> + ret = -ENOENT;
>> +
>> + mutex_lock(&kv->lock);
>> +
>> + list_for_each_entry(kvg, &kv->group_list, node) {
>> + if (kvg->vfio_group != vfio_group)
>> + continue;
>> +
>> + if (kvg->spapr_tce.liobn) {
>> + ret = -EBUSY;
>> + break;
>> + }
>
> Is zero not an liobn that can be used by userspace?
Good point, thanks. I thought zero cannot be used but by spec -1 is reserved
> Is it intentional
> that there's no way to unset or change the group/liobn mapping? Thanks,
I do not see why we would want to disable once enabled acceleration. Group
removal should clear it though so we are good.
Thanks for the review and patience :)
>
> Alex
>
>> +
>> + kvg->spapr_tce.liobn = param.liobn;
>> + ret = 0;
>> + break;
>> + }
>> +
>> + mutex_unlock(&kv->lock);
>> +
>> + kvm_vfio_group_put_external_user(vfio_group);
>> +
>> + return ret;
>> + }
>> +#endif /* CONFIG_SPAPR_TCE_IOMMU */
>> }
>>
>> return -ENXIO;
>> @@ -207,9 +319,11 @@ static int kvm_vfio_has_attr(struct kvm_device *dev,
>> switch (attr->attr) {
>> case KVM_DEV_VFIO_GROUP_ADD:
>> case KVM_DEV_VFIO_GROUP_DEL:
>> +#ifdef CONFIG_SPAPR_TCE_IOMMU
>> + case KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE_LIOBN:
>> +#endif
>> return 0;
>> }
>> -
>> break;
>> }
>>
>
>
>
--
Alexey
^ permalink raw reply
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-11-21 0:34 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list
Hi Linus !
Since you pulled my previous one in less than 20mn (the LE stuff),
there's no point waiting for tomorrow for these fixes I mentioned
earlier so here they are.
This is a small collection of random bug fixes and a few improvements
of Oops output which I deemed valuable enough to include as well.
The fixes are essentially recent build breakage and regressions,
and a couple of older bugs such as the DTL log duplication, the
EEH issue with PCI_COMMAND_MASTER and the problem with small
contexts passed to get/set_context with VSX enabled.
Cheers,
Ben.
The following changes since commit b4789b8e6be3151a955ade74872822f30e8cd914:
aacraid: prevent invalid pointer dereference (2013-11-19 16:27:39 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
for you to fetch changes up to c13f20ac48328b05cd3b8c19e31ed6c132b44b42:
powerpc/signals: Mark VSX not saved with small contexts (2013-11-21 10:33:45 +1100)
----------------------------------------------------------------
Aneesh Kumar K.V (1):
powerpc: booke: Fix build failures
Anton Blanchard (5):
powerpc: Print DAR and DSISR on machine check oopses
powerpc: Remove a few lines of oops output
powerpc/pseries: Duplicate dtl entries sometimes sent to userspace
powerpc: Only print PACATMSCRATCH in oops when TM is active
powerpc: ppc64 address space capped at 32TB, mmap randomisation disabled
Gavin Shan (2):
powerpc/eeh: Enable PCI_COMMAND_MASTER for PCI bridges
powerpc/eeh: More accurate log
Heiko Carstens (1):
powerpc: Fix __get_user_pages_fast() irq handling
Li Zhong (1):
powerpc/vio: Fix a dma_mask issue of vio
Michael Ellerman (2):
powerpc: Make cpu_to_chip_id() available when SMP=n
powerpc/pseries: Fix SMP=n build of rng.c
Michael Neuling (1):
powerpc/signals: Mark VSX not saved with small contexts
arch/powerpc/include/asm/smp.h | 2 +-
arch/powerpc/kernel/eeh.c | 9 +++++++++
arch/powerpc/kernel/eeh_event.c | 9 +++++++--
arch/powerpc/kernel/process.c | 21 +++++++++++----------
arch/powerpc/kernel/prom.c | 20 ++++++++++++++++++++
arch/powerpc/kernel/signal_32.c | 10 +++++++++-
arch/powerpc/kernel/smp.c | 16 ----------------
arch/powerpc/kernel/time.c | 4 ++--
arch/powerpc/kernel/vio.c | 2 +-
arch/powerpc/mm/gup.c | 5 +++--
arch/powerpc/mm/slice.c | 2 +-
arch/powerpc/platforms/powernv/rng.c | 1 +
arch/powerpc/platforms/pseries/rng.c | 1 +
arch/powerpc/platforms/wsp/chroma.c | 1 +
arch/powerpc/platforms/wsp/h8.c | 1 +
arch/powerpc/platforms/wsp/ics.c | 2 ++
arch/powerpc/platforms/wsp/opb_pic.c | 2 ++
arch/powerpc/platforms/wsp/psr2.c | 1 +
arch/powerpc/platforms/wsp/scom_wsp.c | 1 +
arch/powerpc/platforms/wsp/wsp.c | 1 +
20 files changed, 75 insertions(+), 36 deletions(-)
^ permalink raw reply
* Re: [PATCH] powerpc/gpio: Fix the wrong GPIO input data on MPC8572/MPC8536
From: Scott Wood @ 2013-11-21 0:32 UTC (permalink / raw)
To: Liu Gang; +Cc: linux-gpio, linus.walleij, linuxppc-dev, r61911, b07421
In-Reply-To: <1384916079.16677.26.camel@linux>
On Wed, 2013-11-20 at 10:54 +0800, Liu Gang wrote:
> On Tue, 2013-11-19 at 16:51 -0600, Scott Wood wrote:
> > > @@ -71,6 +71,7 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> > > struct mpc8xxx_gpio_chip *mpc8xxx_gc = to_mpc8xxx_gpio_chip(mm);
> > >
> > > val = in_be32(mm->regs + GPIO_DAT) & ~in_be32(mm->regs + GPIO_DIR);
> > > + mpc8xxx_gc->data &= in_be32(mm->regs + GPIO_DIR);
> > >
> > > return (val | mpc8xxx_gc->data) & mpc8xxx_gpio2mask(gpio);
> > > }
> >
> > It seems odd to update ->data in a function that's supposed to be
> > reading things... Perhaps it would be better to keep ->data in a good
> > state from the beginning.
> >
> > -Scott
>
> Yes, keeping the ->data in a good state from the beginning will be
> better. But this will need more code in different functions to cover
> all the scenarios.
> First, we should check the direct of the pin in the function
> "mpc8xxx_gpio_set", and clean the input bit in ->data after setting
> operation.
For userspace value setting, it looks like gpiolib blocks the write if
the pin if FLAG_IS_OUT is set. This suggests that this is an error
condition for other uses as well. Though, I notice that
mpc8xxx_gpio_dir_out() calls gpio_set() before actually changing the
direction. So it may be useful to avoid races where the wrong value is
output briefly after the direction is changed (especially in open drain
situations, where the signal could have a meaningful default even before
we begin outputting). But that raises the question of how you'd do that
from userspace, and it also renders the to-be-output value as write-only
data (until the direction is actually changed), since a readback would
get the input value instead.
> So maybe it's better to eliminate the effects of the ->data to the
> input pins when reading the status, regardless of the possible changes
> of the pins and the data.
> Do you think so?
Perhaps, but that doesn't require you to modify ->data in the get()
function.
-Scott
^ permalink raw reply
* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Benjamin Herrenschmidt @ 2013-11-21 0:22 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <20131121000819.GU16735@n2100.arm.linux.org.uk>
On Thu, 2013-11-21 at 00:08 +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> > On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > > but as I've already said elsewhere, the real fix is to get whatever
> > > created the struct device to initialise the dev->dma_mask with a
> > > bus default.
> > >
> > > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > > over the issue - it's fine to do it this way, but someone should still
> > > fix the broken code creating these devices...
> >
> > Ok, they are created by the vio bus core, so it should be doing the
> > job here of setting the dma_mask pointer to a proper value.
> >
> > Li, can you take care of that ? Look at other bus types we have in
> > there such as the macio bus etc...
>
> Oh, hang on a moment, this is the "bus" code.
>
> In which case, the question becomes: do vio devices ever need to have
> a separate streaming DMA mask from a coherent DMA mask? If not, then
> something like the following is what's needed here, and I should've
> never have used dma_set_mask_and_coherent().
No, a single mask.
> dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
> are really supposed to be used by drivers only.
>
> arch/powerpc/kernel/vio.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index e7d0c88f621a..d771778f398e 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>
> /* needed to ensure proper operation of coherent allocations
> * later, in case driver doesn't set it explicitly */
> - dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> + viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
> + viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
> }
>
> /* register with generic device framework */
Right that's exactly what I had in mind. Li, can you test this please ?
The previous "fix" using dma_coerce_mask_and_coherent() is already on
its way to Linus, so we'll rework the above patch to undo it but for
now please test.
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Russell King - ARM Linux @ 2013-11-21 0:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <1384992102.26969.120.camel@pasglop>
On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> > Li Zong's patch works around the issue of a failing dma_set_mask(),
> > but as I've already said elsewhere, the real fix is to get whatever
> > created the struct device to initialise the dev->dma_mask with a
> > bus default.
> >
> > Using dma_coerce_xxx() merely makes the problem "go away" papering
> > over the issue - it's fine to do it this way, but someone should still
> > fix the broken code creating these devices...
>
> Ok, they are created by the vio bus core, so it should be doing the
> job here of setting the dma_mask pointer to a proper value.
>
> Li, can you take care of that ? Look at other bus types we have in
> there such as the macio bus etc...
Oh, hang on a moment, this is the "bus" code.
In which case, the question becomes: do vio devices ever need to have
a separate streaming DMA mask from a coherent DMA mask? If not, then
something like the following is what's needed here, and I should've
never have used dma_set_mask_and_coherent().
dma_set_mask_and_coherent() (and the other dma_set_mask() functions)
are really supposed to be used by drivers only.
arch/powerpc/kernel/vio.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index e7d0c88f621a..d771778f398e 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
/* needed to ensure proper operation of coherent allocations
* later, in case driver doesn't set it explicitly */
- dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
+ viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64);
+ viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask;
}
/* register with generic device framework */
^ permalink raw reply related
* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Benjamin Herrenschmidt @ 2013-11-21 0:01 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <20131120232337.GT16735@n2100.arm.linux.org.uk>
On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:
> Li Zong's patch works around the issue of a failing dma_set_mask(),
> but as I've already said elsewhere, the real fix is to get whatever
> created the struct device to initialise the dev->dma_mask with a
> bus default.
>
> Using dma_coerce_xxx() merely makes the problem "go away" papering
> over the issue - it's fine to do it this way, but someone should still
> fix the broken code creating these devices...
Ok, they are created by the vio bus core, so it should be doing the
job here of setting the dma_mask pointer to a proper value.
Li, can you take care of that ? Look at other bus types we have in
there such as the macio bus etc...
Cheers,
Ben.
^ permalink raw reply
* Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Russell King - ARM Linux @ 2013-11-20 23:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Paul Mackerras, PowerPC email list, Li Zhong
In-Reply-To: <1384910882.26969.57.camel@pasglop>
On Wed, Nov 20, 2013 at 12:28:02PM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2013-11-19 at 16:11 +0800, Li Zhong wrote:
> > I encountered following issue:
> > [ 0.283035] ibmvscsi 30000015: couldn't initialize event pool
> > [ 5.688822] ibmvscsi: probe of 30000015 failed with error -1
> >
> > which prevents the storage from being recognized, and the machine from
> > booting.
> >
> > After some digging, it seems that it is caused by commit 4886c399da
> >
> > as dma_mask pointer in viodev->dev is not set, so in
> > dma_set_mask_and_coherent(), dma_set_coherent_mask() is not called
> > because dma_set_mask(), which is dma_set_mask_pSeriesLP() returned EIO.
> > While before the commit, dma_set_coherent_mask() is always called.
> >
> > I tried to replace dma_set_mask_and_coherent() with
> > dma_coerce_mask_and_coherent(), and the machine could boot again.
> >
> > But I'm not sure whether this is the correct fix...
>
> Russell, care to chime in ? I can't make sense of the semantics...
>
> The original commit was fairly clear:
>
> <<
> Replace the following sequence:
>
> dma_set_mask(dev, mask);
> dma_set_coherent_mask(dev, mask);
>
> with a call to the new helper dma_set_mask_and_coherent().
> >>
>
> It all makes sense so far ... but doesn't work for some odd reason,
> and the "fix" uses a function whose name doesn't make much sense to
> me ... what is the difference between "setting" and "coercing"
> the mask ? And why doe replacing two "set" with a "set both" doesn't
> work and require a coerce ?
>
> I'm asking because I'm worried about breakage elsewhere...
I'd expect that the reason it doesn't work is that the dma_set_mask()
is failing, which means we don't go on to set the coherent mask.
Li Zong's patch works around the issue of a failing dma_set_mask(),
but as I've already said elsewhere, the real fix is to get whatever
created the struct device to initialise the dev->dma_mask with a
bus default.
Using dma_coerce_xxx() merely makes the problem "go away" papering
over the issue - it's fine to do it this way, but someone should still
fix the broken code creating these devices...
^ 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