LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] misc: cxl: sysfs.c:  Remove unused function
From: Rickard Strandqvist @ 2014-12-20 16:13 UTC (permalink / raw)
  To: Ian Munsie, Michael Neuling
  Cc: linuxppc-dev, linux-kernel, Rickard Strandqvist

Remove the function mmio_size_show() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 drivers/misc/cxl/sysfs.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index ce7ec06..0431ec3 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -102,17 +102,6 @@ static struct device_attribute afu_master_attrs[] = {
 
 /*********  AFU attributes  **************************************************/
 
-static ssize_t mmio_size_show(struct device *device,
-			      struct device_attribute *attr,
-			      char *buf)
-{
-	struct cxl_afu *afu = to_cxl_afu(device);
-
-	if (afu->pp_size)
-		return scnprintf(buf, PAGE_SIZE, "%llu\n", afu->pp_size);
-	return scnprintf(buf, PAGE_SIZE, "%llu\n", afu->adapter->ps_size);
-}
-
 static ssize_t reset_store_afu(struct device *device,
 			       struct device_attribute *attr,
 			       const char *buf, size_t count)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH] arch: powerpc: platforms: ps3: repository.c: Remove unused function
From: Rickard Strandqvist @ 2014-12-20 15:00 UTC (permalink / raw)
  To: Geoff Levand, Benjamin Herrenschmidt
  Cc: cbe-oss-dev, Rickard Strandqvist, linux-kernel, Paul Mackerras,
	linuxppc-dev

Remove the function ps3_repository_write_highmem_info() that is not used anywhere.

This was partially found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
 arch/powerpc/platforms/ps3/platform.h   |    2 --
 arch/powerpc/platforms/ps3/repository.c |   10 ----------
 2 files changed, 12 deletions(-)

diff --git a/arch/powerpc/platforms/ps3/platform.h b/arch/powerpc/platforms/ps3/platform.h
index d71329a..f15507c 100644
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -201,8 +201,6 @@ int ps3_repository_write_highmem_base(unsigned int region_index,
 	u64 highmem_base);
 int ps3_repository_write_highmem_size(unsigned int region_index,
 	u64 highmem_size);
-int ps3_repository_write_highmem_info(unsigned int region_index,
-	u64 highmem_base, u64 highmem_size);
 int ps3_repository_delete_highmem_info(unsigned int region_index);
 
 /* repository pme info */
diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
index bfccdc7..2d5e56a 100644
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -1160,16 +1160,6 @@ int ps3_repository_write_highmem_size(unsigned int region_index,
 		highmem_size, 0);
 }
 
-int ps3_repository_write_highmem_info(unsigned int region_index,
-	u64 highmem_base, u64 highmem_size)
-{
-	int result;
-
-	result = ps3_repository_write_highmem_base(region_index, highmem_base);
-	return result ? result
-		: ps3_repository_write_highmem_size(region_index, highmem_size);
-}
-
 static int ps3_repository_delete_highmem_base(unsigned int region_index)
 {
 	return delete_node(
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Scott Wood @ 2014-12-19 23:15 UTC (permalink / raw)
  To: Xie Shaohui-B21989
  Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Medve Emilian-EMMEDVE1, Liberman Igal-B31950
In-Reply-To: <DM2PR0301MB0864AA31CD1A7438F476020FE26B0@DM2PR0301MB0864.namprd03.prod.outlook.com>

On Fri, 2014-12-19 at 01:23 -0600, Xie Shaohui-B21989 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, December 19, 2014 6:01 AM
> > To: Xie Shaohui-B21989
> > Cc: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; Medve Emilian-
> > EMMEDVE1; Liberman Igal-B31950
> > Subject: Re: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
> > 
> > On Thu, 2014-12-18 at 06:53 -0600, Xie Shaohui-B21989 wrote:
> > > Ping.
> > >
> > > Best Regards,
> > > Shaohui Xie
> > 
> > I can't put patches in my -next until the merge window closes.
> > 
> > > > > > +EXAMPLE
> > > > > > +
> > > > > > +Example for FMan v2 external MDIO:
> > > > > > +
> > > > > > +mdio@f1000 {
> > > > > > +	compatible = "fsl,fman-xmdio";
> > > > > > +	reg = <0xf1000 0x1000>;
> > > > > > +	bus-frequency = <20000>;
> > > > > > +};
> > > > >
> > > > > So the bus frequency is only 20 KHz?  Or is the unit supposed to
> > > > > be something other than Hz?
> > > > [S.H] it's only an example, it could be different on real SoCs, but
> > > > they always lower than the standard one, The standard one is 2.5MHz, I have
> > to use Hz for it.
> > 
> > Is there any SoC for which 20 kHz is the right frequency?  I just want to make
> > sure the example is realistic.
> [S.H] the clock divider has a limitation that the MAX value it can get on Fman v2 is 255 (0xff, 8 bits),
> On Fman v3 is 511(0x1ff, 9 bits).
> 
> So the lowest frequency on Fman v2 is: Fman_clock / (2 * 255),
> On Fman v3 is: Fman_clock / ((2 * 511) + 1).
> 
> Take default Fman frequency setting from SDK1.7 as example, the lowest clock used for Fman v2 is 581MHz,
> The lowest clock for Fman v3 is 600MHz.
> 
> Then the lowest bus frequency can get is:
> Fman v2: ~1140KHz
> Fman v3: ~587KHz
> 
> 20KHz is not practice, we don't have a suggested value in errata document.
> For this example, should I post a new version with a value like 1200KHz?

This is different from how you described the problem before.  If the
limitation is on the divider, rather than the absolute bus frequency,
then specifiy the max divider.  Or better, since according to the above
this correlates with fman version, just have the driver know what the
max divider is for each fman version.

-Scott

^ permalink raw reply

* Re: [PATCH] powerpc/powernv: force all CPUs to be bootable
From: Greg Kurz @ 2014-12-19 21:44 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood
In-Reply-To: <20141212113627.12778.47915.stgit@bahia.local>

On Fri, 12 Dec 2014 12:37:40 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> The subcore logic needs all the CPUs declared in the DT to be bootable,
> otherwise the kernel hangs at boot time. Since subcore support starts
> with POWER8, we can keep the current behaviour for older CPUs.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
> 
> Since smt-enabled is still needed by Freescale, the choice was made to fix
> powernv only. I could test that smt-enabled is still honored on POWER7 and
> ignored on POWER8, as expected.
> 

FWIW I get plenty (for nearly all vcpus) the following dump when booting a
PowerKVM guest on a virtio-blk disk with smt-enabled=off:

[   10.535151] kobject '(null)' (c0000007ff260c18): tried to add an uninitialized object, something is seriously wrong.
[   10.535538] CPU: 12 PID: 708 Comm: systemd-udevd Not tainted 3.18.0-rc3+ #3
[   10.535779] Call Trace:
[   10.535886] [c0000000ff0e38f0] [c0000000007a1db8] dump_stack+0x90/0xbc (unreliable)
[   10.536242] [c0000000ff0e3920] [c0000000003c5504] kobject_add+0xc4/0xd0
[   10.536593] [c0000000ff0e39a0] [c00000000039df20] blk_mq_register_hctx+0xd0/0x100
[   10.536930] [c0000000ff0e3a20] [c00000000039ed0c] blk_mq_sysfs_register+0x5c/0xb0
[   10.537257] [c0000000ff0e3a50] [c00000000039b694] blk_mq_queue_reinit_notify+0xc4/0x110
[   10.537594] [c0000000ff0e3a90] [c0000000000c5388] notifier_call_chain+0x88/0x100
[   10.537942] [c0000000ff0e3ae0] [c00000000009e264] cpu_notify+0x34/0x80
[   10.538227] [c0000000ff0e3b00] [c00000000009e794] _cpu_up+0x204/0x210
[   10.538519] [c0000000ff0e3bb0] [c00000000009e8ac] cpu_up+0x10c/0x130
[   10.538796] [c0000000ff0e3c30] [c00000000062121c] cpu_subsys_online+0x5c/0xe0
[   10.539089] [c0000000ff0e3c80] [c0000000004bf0e4] device_online+0xb4/0x120
[   10.539370] [c0000000ff0e3cc0] [c0000000004bf1f4] online_store+0xa4/0xb0
[   10.539651] [c0000000ff0e3d00] [c0000000004bb744] dev_attr_store+0x34/0x60
[   10.539929] [c0000000ff0e3d20] [c0000000003038c4] sysfs_kf_write+0x64/0xa0
[   10.540217] [c0000000ff0e3d40] [c000000000302940] kernfs_fop_write+0x140/0x1d0
[   10.540553] [c0000000ff0e3d90] [c00000000026ac64] vfs_write+0xd4/0x260
[   10.540831] [c0000000ff0e3de0] [c00000000026b780] SyS_write+0x60/0x110
[   10.541131] [c0000000ff0e3e30] [c0000000000091fc] syscall_exit+0x0/0x7c

It seems that split core isn't the only code that gets affected by smt-enabled.
Should we drop cpu_bootable for pseries ?

Cheers.

--
Greg

>  arch/powerpc/platforms/powernv/smp.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index b716f66..5b6e002 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -198,13 +198,24 @@ static void pnv_smp_cpu_kill_self(void)
>  
>  #endif /* CONFIG_HOTPLUG_CPU */
>  
> +static int pnv_cpu_bootable(unsigned int nr)
> +{
> +	/* Starting with POWER8, all CPUs need to be booted to avoid hangs
> +	 * during subcore init.
> +	 */
> +	if (cpu_has_feature(CPU_FTR_ARCH_207S))
> +		return 1;
> +
> +	return smp_generic_cpu_bootable(nr);
> +}
> +
>  static struct smp_ops_t pnv_smp_ops = {
>  	.message_pass	= smp_muxed_ipi_message_pass,
>  	.cause_ipi	= NULL,	/* Filled at runtime by xics_smp_probe() */
>  	.probe		= xics_smp_probe,
>  	.kick_cpu	= pnv_smp_kick_cpu,
>  	.setup_cpu	= pnv_smp_setup_cpu,
> -	.cpu_bootable	= smp_generic_cpu_bootable,
> +	.cpu_bootable	= pnv_cpu_bootable,
>  #ifdef CONFIG_HOTPLUG_CPU
>  	.cpu_disable	= pnv_smp_cpu_disable,
>  	.cpu_die	= generic_cpu_die,
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [V6,1/9] elf: Add new powerpc specifc core note sections
From: Edjunior Barbosa Machado @ 2014-12-19 19:28 UTC (permalink / raw)
  To: Anshuman Khandual, Michael Ellerman, linux-kernel, linuxppc-dev
  Cc: mikey, james.hogan, avagin, Paul.Clothier, peterz, palves,
	Ulrich Weigand, shuahkh, akpm, oleg, dhowells, kirjanov, davej,
	tglx, sukadev, davem, sam.bobroff
In-Reply-To: <548578A8.5020901@linux.vnet.ibm.com>

On 12/08/2014 08:08 AM, Anshuman Khandual wrote:
> On 12/03/2014 12:18 PM, Anshuman Khandual wrote:
>> On 12/03/2014 10:52 AM, Michael Ellerman wrote:
>>> On Tue, 2014-02-12 at 07:56:45 UTC, Anshuman Khandual wrote:
>>>> This patch adds four new ELF core note sections for powerpc
>>>> transactional memory and one new ELF core note section for
>>>> powerpc general miscellaneous debug registers. These addition
>>>> of new ELF core note sections extends the existing ELF ABI
>>>> without affecting it in any manner.
>>>>
>>>> Acked-by: Andrew Morton <akpm@linux-foundation.org>
>>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>> ---
>>>>  include/uapi/linux/elf.h | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
>>>> index ea9bf25..2260fc0 100644
>>>> --- a/include/uapi/linux/elf.h
>>>> +++ b/include/uapi/linux/elf.h
>>>> @@ -379,6 +379,11 @@ typedef struct elf64_shdr {
>>>>  #define NT_PPC_VMX	0x100		/* PowerPC Altivec/VMX registers */
>>>>  #define NT_PPC_SPE	0x101		/* PowerPC SPE/EVR registers */
>>>>  #define NT_PPC_VSX	0x102		/* PowerPC VSX registers */
>>>> +#define NT_PPC_TM_SPR	0x103		/* PowerPC TM special registers */
>>>> +#define NT_PPC_TM_CGPR	0x104		/* PowerpC TM checkpointed GPR */
>>>> +#define NT_PPC_TM_CFPR	0x105		/* PowerPC TM checkpointed FPR */
>>>> +#define NT_PPC_TM_CVMX	0x106		/* PowerPC TM checkpointed VMX */
>>>> +#define NT_PPC_MISC	0x107		/* PowerPC miscellaneous registers */
>>>
>>> This is a really terrible name, "MISC".
>>>
>>> Having said that, I guess it's accurate. We have a whole bunch of regs that
>>> have accrued over recent years that aren't accessible via ptrace.
>>>
>>> It seems to me if we're adding a misc regset we should be adding everything we
>>> might want to it that is currenty architected.
>>
>> But I believe they also need to be part of the thread_struct structure to be
>> accessible from ptrace.
> 
> Currently we dont context save/restore the PMC count registers (PMC1-PMC6)
> during the process context switch. So the values of PMC1..PMC6 are not
> thread specific in the structure. To be able to access them in ptrace
> when the tracee has stopped, we need to context save these counters
> in the thread struct. Shall we do that ? Then we can add them to the
> MISC regset bucket irrespective of whats the value we get in there when
> we probe through ptrace.
> 
> The same goes for MMCRA, CFAR registers as well.
> 
>>  
>>>
>>> But currently you only include the PPR, TAR & DSCR.
>>
>> Yeah, thats what we started with.
>>
>>>
>>> Looking at Power ISA v2.07, I see the following that could be included:
>>>
>>>   MMCR2
>>>   MMCRA
>>>   PMC1
>>>   PMC2
>>>   PMC3
>>>   PMC4
>>>   PMC5
>>>   PMC6
>>>   MMCR0
>>>   EBBHR
>>>   EBBRR
>>>   BESCR
>>>   SIAR
>>>   SDAR
>>>   CFAR?
>>
>> MMCRA, PMC[1..6], EBBHR, BESCR, EBBRR, CFAR are not part of the thread struct.
> 
> Sorry. EBBRR, EBBHR, BESCR registers are part of the thread struct.
> 
>>
>>>
>>> Those are all new in 2.07 except for CFAR.
>>>
>>> There might be more I missed, that was just a quick scan.
>>>
>>> Some are only accessible when EBB is in use, maybe those could be a separate
>>> regset.
>>
>> Yeah we can have one more regset for EBB specific registers.
> 
> Should the new EBB specific regset include only EBBRR, EBBHR, BESCR registers
> or should it also include SIAR, SDAR, SIER, MMCR0, MMCR2 registers as well. I
> was thinking about putting these five registers into the MISC bucket instead.
> But from the perf code, it looks like these five registers are also related to
> the EBB context as well.
> 
> Some clarity on these points would really help.

Hi,

from the provided testcase using ptrace interface, reviewing with the help
of Ulrich, it looks OK from GDB perspective, with the exception of a few
concerns:

The patchset seems to change the "original" ptrace requests (i.e.
PTRACE_GETREGS/GETFPREGS/GETVRREGS...) to return the "transactional" state, and
adds new register sets to return the "checkpointed" state. Considering that
whenever you get a debugger interception inside a transactional block, the
transaction will abort, we're wondering if it wouldn't make more sense to
display the 'checkpointed' state as the normal registers since this is where the
execution will continue from.

Also, we've noticed that the 'misc' regset contains registers from different ISA
versions (dscr and ppr appear in ISA 2.05, tar is from 2.07). I'm not sure if
there is a way to detect presence/validity of such registers, but perhaps it
might be a good idea to separate registers from different ISAs in different
regsets.

Regarding the inclusion of other registers along with the EBB-related ones, I'm
sorry but I'm not familiar with them.

Thanks and regards,
--
Edjunior

^ permalink raw reply

* Re: [PATCH] macintosh: therm_pm72: delete deprecated driver
From: Wolfram Sang @ 2014-12-19 16:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Michael Ellerman
In-Reply-To: <1415267404.4925.15.camel@kernel.crashing.org>

[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]

On Thu, Nov 06, 2014 at 08:50:04PM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2014-11-06 at 10:25 +0100, Wolfram Sang wrote:
> > On Thu, Nov 06, 2014 at 01:19:36PM +1100, Benjamin Herrenschmidt wrote:
> > > On Thu, 2014-11-06 at 02:45 +0100, Wolfram Sang wrote:
> > > > The new driver is around for more than 2 years now, so the old one can
> > > > go. Getting rid of it helps the removal of the legacy .attach_adapter
> > > > callback of the I2C subsystem.
> > > > 
> > > > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> > > 
> > > Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > Thanks! I can take it via my I2C tree, but I'd think it makes more sense
> > if you take it via ppc? What do you prefer?
> 
> Either works. These days Michael Ellerman handles the day to day
> maintainance of the powerpc tree so it's his choice but if you need this
> as a dependency on your subsequent API removal it's probably better that
> you take it.

So, I haven't seen this coming in via ppc. I am going to send another
pull request to Linus tomorrow and will include it unless somebody
objects soon.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: Disabled LocalPlus Controller (LPC) clock on MPC512x
From: Matteo Facchinetti @ 2014-12-19  9:38 UTC (permalink / raw)
  To: alex.popov; +Cc: Gerhard Sittig, linuxppc-dev, linux-kernel
In-Reply-To: <54901ED3.30207@linux.com>

On 16/12/2014 13:00, Alexander Popov wrote:
> 02.12.2014 13:47, Matteo Facchinetti пишет:
>> On 26/11/2014 12:49, Alexander Popov wrote:
>>> So starting Linux with clk_ignore_unused bootparam
>>> or inserting dummy LPB reading to some initcall is a temporary fix.
>
> In fact clk_ignore_unused bootparam helps to avoid disabling NFC clock.
> The board crash is reproduced again if I perform the following steps:
> 1. disable NFC clock in uboot by clearing NFC_EN bit in SCCR1 register,
> 2. boot Linux with clk_ignore_unused,
> 3. touch any LPB address.
>

Could you see the Reset Status Register (RSR) after board crash?
When boad reset, you may stop uboot in console and then print the value 
of this register.

This could be help to see what happen internally to the microcontroller.

^ permalink raw reply

* [git pull] Please pull mpe/linux.git powerpc-3.19-2 tag
From: Michael Ellerman @ 2014-12-19  9:16 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: sukadev, paul, mst, rgb, wsa, linux-kernel, paulus, anton,
	shreyas, neelegup, gkurz, linuxppc-dev, imunsie

[-- Attachment #1: Type: text/plain, Size: 4904 bytes --]

Hi Linus,

Please pull the second batch of powerpc updates for 3.19:

The following changes since commit 56548fc0e86cb9156af7a7e1f15ba78f251dafaf:

  powerpc/powernv: Return to cpu offline loop when finished in KVM guest (2014-12-08 13:16:31 +1100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux.git tags/powerpc-3.19-2

for you to fetch changes up to d70a54e2d08510a99b1f10eceeae6f2f7086e226:

  powerpc/powernv: Ignore smt-enabled on Power8 and later (2014-12-18 19:59:21 +1100)

----------------------------------------------------------------
powerpc updates for 3.19 batch 2

The highlight is the series that reworks the idle management on powernv, which
allows us to use deeper idle states on those machines.

There's the fix from Anton for the "BUG at kernel/smpboot.c:134!" problem.

An i2c driver for powernv. This is acked by Wolfram Sang, and he asked that we
take it through the powerpc tree.

A fix for audit from rgb at Red Hat, acked by Paul Moore who is one of the audit
maintainers.

A patch from Ben to export the symbol map of our OPAL firmware as a sysfs file,
so that tools can use it.

Also some CXL fixes, a couple of powerpc perf fixes, a fix for smt-enabled, and
the patch to add __force to get_user() so we can use bitwise types.
----------------------------------------------------------------

Anton Blanchard (1):
      powerpc: Secondary CPUs must set cpu_callin_map after setting active and online

Benjamin Herrenschmidt (1):
      powerpc/powernv: Expose OPAL firmware symbol map

Greg Kurz (1):
      powerpc/powernv: Ignore smt-enabled on Power8 and later

Ian Munsie (3):
      cxl: Change contexts_lock to a mutex to fix sleep while atomic bug
      cxl: Add timeout to process element commands
      cxl: Unmap MMIO regions when detaching a context

Michael S. Tsirkin (1):
      powerpc/uaccess: Allow get_user() with bitwise types

Neelesh Gupta (1):
      i2c: Driver to expose PowerNV platform i2c busses

Paul Mackerras (1):
      powerpc/powernv: Switch off MMU before entering nap/sleep/rvwinkle mode

Richard Guy Briggs (1):
      powerpc: add little endian flag to syscall_get_arch()

Shreyas B. Prabhu (3):
      powerpc/powernv: Enable Offline CPUs to enter deep idle states
      powernv/cpuidle: Redesign idle states management
      powernv/powerpc: Add winkle support for offline cpus

Sukadev Bhattiprolu (1):
      power/perf/hv-24x7: Use kmem_cache_free() instead of kfree

sukadev@linux.vnet.ibm.com (1):
      powerpc/perf/hv-24x7: Use per-cpu page buffer

 Documentation/devicetree/bindings/i2c/i2c-opal.txt |  37 +++
 arch/powerpc/include/asm/cpuidle.h                 |  20 ++
 arch/powerpc/include/asm/opal.h                    |  42 +++
 arch/powerpc/include/asm/paca.h                    |  10 +
 arch/powerpc/include/asm/ppc-opcode.h              |   2 +
 arch/powerpc/include/asm/processor.h               |   3 +-
 arch/powerpc/include/asm/reg.h                     |   4 +
 arch/powerpc/include/asm/syscall.h                 |   6 +-
 arch/powerpc/include/asm/uaccess.h                 |   6 +-
 arch/powerpc/kernel/asm-offsets.c                  |  11 +
 arch/powerpc/kernel/exceptions-64s.S               |  35 ++-
 arch/powerpc/kernel/idle_power7.S                  | 344 ++++++++++++++++++---
 arch/powerpc/kernel/smp.c                          |   9 +-
 arch/powerpc/perf/hv-24x7.c                        |  23 +-
 arch/powerpc/platforms/powernv/opal-wrappers.S     |  39 +++
 arch/powerpc/platforms/powernv/opal.c              |  50 ++-
 arch/powerpc/platforms/powernv/powernv.h           |   2 +
 arch/powerpc/platforms/powernv/setup.c             | 166 ++++++++++
 arch/powerpc/platforms/powernv/smp.c               |  29 +-
 arch/powerpc/platforms/powernv/subcore.c           |  34 ++
 arch/powerpc/platforms/powernv/subcore.h           |   9 +-
 drivers/cpuidle/cpuidle-powernv.c                  |  10 +-
 drivers/i2c/busses/Kconfig                         |  11 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-opal.c                      | 294 ++++++++++++++++++
 drivers/misc/cxl/context.c                         |  26 +-
 drivers/misc/cxl/cxl.h                             |   9 +-
 drivers/misc/cxl/file.c                            |   6 +-
 drivers/misc/cxl/native.c                          |  12 +-
 drivers/misc/cxl/pci.c                             |   2 +-
 drivers/misc/cxl/sysfs.c                           |  10 +-
 include/uapi/linux/audit.h                         |   2 +
 32 files changed, 1156 insertions(+), 108 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-opal.txt
 create mode 100644 arch/powerpc/include/asm/cpuidle.h
 create mode 100644 drivers/i2c/busses/i2c-opal.c


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* RE: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Shaohui Xie @ 2014-12-19  7:23 UTC (permalink / raw)
  To: Scott Wood
  Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Emilian Medve, Igal.Liberman@freescale.com
In-Reply-To: <1418940078.5581.143.camel@freescale.com>

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0K
PiBTZW50OiBGcmlkYXksIERlY2VtYmVyIDE5LCAyMDE0IDY6MDEgQU0NCj4gVG86IFhpZSBTaGFv
aHVpLUIyMTk4OQ0KPiBDYzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IGRldmljZXRy
ZWVAdmdlci5rZXJuZWwub3JnOyBNZWR2ZSBFbWlsaWFuLQ0KPiBFTU1FRFZFMTsgTGliZXJtYW4g
SWdhbC1CMzE5NTANCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gW3YyXSBwb3dlci9mc2w6IGFkZCBN
RElPIGR0IGJpbmRpbmcgZm9yIEZNYW4NCj4gDQo+IE9uIFRodSwgMjAxNC0xMi0xOCBhdCAwNjo1
MyAtMDYwMCwgWGllIFNoYW9odWktQjIxOTg5IHdyb3RlOg0KPiA+IFBpbmcuDQo+ID4NCj4gPiBC
ZXN0IFJlZ2FyZHMsDQo+ID4gU2hhb2h1aSBYaWUNCj4gDQo+IEkgY2FuJ3QgcHV0IHBhdGNoZXMg
aW4gbXkgLW5leHQgdW50aWwgdGhlIG1lcmdlIHdpbmRvdyBjbG9zZXMuDQo+IA0KPiA+ID4gPiA+
ICtFWEFNUExFDQo+ID4gPiA+ID4gKw0KPiA+ID4gPiA+ICtFeGFtcGxlIGZvciBGTWFuIHYyIGV4
dGVybmFsIE1ESU86DQo+ID4gPiA+ID4gKw0KPiA+ID4gPiA+ICttZGlvQGYxMDAwIHsNCj4gPiA+
ID4gPiArCWNvbXBhdGlibGUgPSAiZnNsLGZtYW4teG1kaW8iOw0KPiA+ID4gPiA+ICsJcmVnID0g
PDB4ZjEwMDAgMHgxMDAwPjsNCj4gPiA+ID4gPiArCWJ1cy1mcmVxdWVuY3kgPSA8MjAwMDA+Ow0K
PiA+ID4gPiA+ICt9Ow0KPiA+ID4gPg0KPiA+ID4gPiBTbyB0aGUgYnVzIGZyZXF1ZW5jeSBpcyBv
bmx5IDIwIEtIej8gIE9yIGlzIHRoZSB1bml0IHN1cHBvc2VkIHRvDQo+ID4gPiA+IGJlIHNvbWV0
aGluZyBvdGhlciB0aGFuIEh6Pw0KPiA+ID4gW1MuSF0gaXQncyBvbmx5IGFuIGV4YW1wbGUsIGl0
IGNvdWxkIGJlIGRpZmZlcmVudCBvbiByZWFsIFNvQ3MsIGJ1dA0KPiA+ID4gdGhleSBhbHdheXMg
bG93ZXIgdGhhbiB0aGUgc3RhbmRhcmQgb25lLCBUaGUgc3RhbmRhcmQgb25lIGlzIDIuNU1Ieiwg
SSBoYXZlDQo+IHRvIHVzZSBIeiBmb3IgaXQuDQo+IA0KPiBJcyB0aGVyZSBhbnkgU29DIGZvciB3
aGljaCAyMCBrSHogaXMgdGhlIHJpZ2h0IGZyZXF1ZW5jeT8gIEkganVzdCB3YW50IHRvIG1ha2UN
Cj4gc3VyZSB0aGUgZXhhbXBsZSBpcyByZWFsaXN0aWMuDQpbUy5IXSB0aGUgY2xvY2sgZGl2aWRl
ciBoYXMgYSBsaW1pdGF0aW9uIHRoYXQgdGhlIE1BWCB2YWx1ZSBpdCBjYW4gZ2V0IG9uIEZtYW4g
djIgaXMgMjU1ICgweGZmLCA4IGJpdHMpLA0KT24gRm1hbiB2MyBpcyA1MTEoMHgxZmYsIDkgYml0
cykuDQoNClNvIHRoZSBsb3dlc3QgZnJlcXVlbmN5IG9uIEZtYW4gdjIgaXM6IEZtYW5fY2xvY2sg
LyAoMiAqIDI1NSksDQpPbiBGbWFuIHYzIGlzOiBGbWFuX2Nsb2NrIC8gKCgyICogNTExKSArIDEp
Lg0KDQpUYWtlIGRlZmF1bHQgRm1hbiBmcmVxdWVuY3kgc2V0dGluZyBmcm9tIFNESzEuNyBhcyBl
eGFtcGxlLCB0aGUgbG93ZXN0IGNsb2NrIHVzZWQgZm9yIEZtYW4gdjIgaXMgNTgxTUh6LA0KVGhl
IGxvd2VzdCBjbG9jayBmb3IgRm1hbiB2MyBpcyA2MDBNSHouDQoNClRoZW4gdGhlIGxvd2VzdCBi
dXMgZnJlcXVlbmN5IGNhbiBnZXQgaXM6DQpGbWFuIHYyOiB+MTE0MEtIeg0KRm1hbiB2MzogfjU4
N0tIeg0KDQoyMEtIeiBpcyBub3QgcHJhY3RpY2UsIHdlIGRvbid0IGhhdmUgYSBzdWdnZXN0ZWQg
dmFsdWUgaW4gZXJyYXRhIGRvY3VtZW50Lg0KRm9yIHRoaXMgZXhhbXBsZSwgc2hvdWxkIEkgcG9z
dCBhIG5ldyB2ZXJzaW9uIHdpdGggYSB2YWx1ZSBsaWtlIDEyMDBLSHo/DQoNClRoYW5rcyENClNo
YW9odWkNCg==

^ permalink raw reply

* Re: [PATCH V3] powerpc, powernv: Add OPAL platform event driver
From: Jeremy Kerr @ 2014-12-19  5:32 UTC (permalink / raw)
  To: Anshuman Khandual, linuxppc-dev; +Cc: mikey
In-Reply-To: <1416921665-15855-1-git-send-email-khandual@linux.vnet.ibm.com>

Hi Anshuman,

> This patch creates a new OPAL platform event character driver
> which will give userspace clients the access to these events
> and process them effectively. Following platforms events are
> currently supported with this platform driver.
> 
> 	(1) Environmental and Power Warning (EPOW)
> 	(2) Delayed Power Off (DPO)
> 
> The user interface for this driver is /dev/opal_event character
> device file where the user space clients can poll and read for
> new opal platform events. The expected sequence of events driven
> from user space should be like the following.
> 
> 	(1) Open the character device file
> 	(2) Poll on the file for POLLIN event
> 	(3) When unblocked, must attempt to read PLAT_EVENT_MAX_SIZE size
> 	(4) Kernel driver will pass at most one opal_plat_event structure
> 	(5) Poll again for more new events

Why is the poll() required? Can't read() just block if there are no
events (and !O_NONBLOCK, of course).

> @@ -281,6 +282,7 @@ enum OpalMessageType {
>  	OPAL_MSG_EPOW,
>  	OPAL_MSG_SHUTDOWN,
>  	OPAL_MSG_HMI_EVT,
> +	OPAL_MSG_DPO,
>  	OPAL_MSG_TYPE_MAX,
>  };

I'd suggest adding the DPO interface definitions as a separate change,
but that's no huge deal.

>  
> @@ -452,6 +454,46 @@ struct opal_msg {
>  	__be64 params[8];
>  };
>  
> +/*
> + * EPOW status sharing (OPAL and the host)
> + *
> + * The host will pass on OPAL, a buffer of length OPAL_SYSEPOW_MAX
> + * with individual elements being 16 bits wide to fetch the system
> + * wide EPOW status. Each element in the buffer will contain the
> + * EPOW status in it's bit representation for a particular EPOW sub
> + * class as defiend here. So multiple detailed EPOW status bits
> + * specific for any sub class can be represented in a single buffer
> + * element as it's bit representation.
> + */

Why do we have this in separate u16s? This just results in a really
sparse bitmask.

I guess the FW interface is already defined though, right?

[Minor grammar nit: "it's" is short for "it is", which isn't what you
want here :)]

> -int64_t opal_get_epow_status(__be64 *status);
> +int64_t opal_get_epow_status(uint16_t *status, uint16_t *length);

Please keep the endian annotations there.

> +int64_t opal_get_dpo_status(int64_t *dpo_timeout);

and __be64 * here.

> +
> +/* EPOW classification */
> +enum epow_condition {
> +	EPOW_SYSPOWER_CHNG	= 0,	/* Power */
> +	EPOW_SYSPOWER_FAIL	= 1,
> +	EPOW_SYSPOWER_INCL	= 2,
> +	EPOW_SYSPOWER_UPS	= 3,
> +	EPOW_SYSTEMP_AMB	= 4,	/* Temperature */
> +	EPOW_SYSTEMP_INT	= 5,
> +	EPOW_SYSTEMP_HMD	= 6,
> +	EPOW_SYSCOOL_INSF	= 7,	/* Cooling */
> +	EPOW_MAX = 8,
> +};
> +
> +/* OPAL EPOW event */
> +struct epow_event {
> +	__u64	epow[EPOW_MAX];		/* Detailed system EPOW status */

Why an array of u64s when you're only using one bit of each? Do we
expect other status details to be set in each of these?

If not, you're converting from one sparse bitmask (the OPAL
interface) to another different sparse bitmask (the kernel interface).
Either keep them the same, or use a more sensible format.

Also, are you expecting a single epow_event to have multiple conditons
set? If not, no need for a bitmask at all.

> +	__u64	timeout;		/* Timeout to shutdown in secs */
> +};
> +
> +/* OPAL DPO event */
> +struct dpo_event {
> +	__u64	orig_timeout;		/* Platform provided timeout in secs */
> +	__u64	remain_timeout;		/* Timeout to shutdown in secs */
> +};

Will all of these events have a timeout? If so, just put a timeout in
struct opal_plat_event.

Why does the DPO timeout have orig/remain timeouts, but not the EPOW one?

See below, but perhaps we want a absolute timeout here, instead of
relative? (which is essentially invalid as soon as you've read it).

> +
> +/* OPAL event */
> +struct opal_plat_event {
> +	__u32	type;			/* Type of OPAL platform event */
> +#define OPAL_PLAT_EVENT_TYPE_EPOW	0
> +#define OPAL_PLAT_EVENT_TYPE_DPO	1
> +#define OPAL_PLAT_EVENT_TYPE_MAX	2
> +	__u32	size;			/* Size of OPAL platform event */
> +	union {
> +		struct epow_event epow;	/* EPOW platform event */
> +		struct dpo_event  dpo;	/* DPO platform event */
> +	};
> +};
> +
> +/*
> + * Suggested read size
> + *
> + * The user space client should attempt to read OPAL_PLAT_EVENT_READ_SIZE
> + * amount of data from the character device file '/dev/opal_event' at any
> + * point of time. The kernel driver will pass an entire opal_plat_event
> + * structure in every read. This ensures that minium data the user space
> + * client gets from the kernel is one opal_plat_event structure.
> + */
> +#define	PLAT_EVENT_MAX_SIZE	4096

-> OPAL_PLAT_EVENT_MAX_SIZE - this it a global header.

> +/*
> + * opal_event_read
> + *
> + * User client needs to attempt to read PLAT_EVENT_MAX_SIZE amount of data
> + * from the file descriptor at a time. The driver will pass a single node
> + * from the list if available at a time and then delete the node from the list.
> + */
> +static ssize_t opal_event_read(struct file *filep,
> +			char __user *buf, size_t len, loff_t *off)
> +{
> +	struct opal_platform_evt *evt;
> +	unsigned long flags;
> +
> +	if (len < sizeof(struct opal_plat_event))
> +		return -EINVAL;

Maybe -EMSGSIZE?

> +
> +	/* Fetch the first node on the list */
> +	spin_lock_irqsave(&opal_plat_evt_spinlock, flags);
> +	if (list_empty(&opal_event_queue)) {
> +		spin_unlock_irqrestore(&opal_plat_evt_spinlock, flags);
> +		return 0;
> +	}
> +
> +	/* Fetch and delete from the list */
> +	evt = list_first_entry(&opal_event_queue,
> +					struct opal_platform_evt, link);
> +	list_del(&evt->link);
> +
> +	/*
> +	 * Update the remaining timeout for DPO event.
> +	 * This can only be updated during the read time.
> +	 */
> +	if (evt->opal_event.type == OPAL_PLAT_EVENT_TYPE_DPO) {
> +		unsigned long timeout;
> +
> +		if (opal_dpo_target &&
> +				evt->opal_event.dpo.orig_timeout) {
> +			timeout = (opal_dpo_target - jiffies) / HZ;
> +			evt->opal_event.dpo.remain_timeout = timeout;
> +		}
> +	}

Would it make more sense to just put one timestamp in here (when we
first see the event from OPAL), of the actual timeout? What's the global
opal_dpo_target for? Doesn't that just mean subsequent DPO events will
invalidate others' timeouts?

Also, do you need the spinlock held here? You've already dequeued the item
from the list. If you keep the opal_dpo_target global, you'll need to do
updates with the lock held too.

> +	spin_unlock_irqrestore(&opal_plat_evt_spinlock, flags);
> +
> +	if (copy_to_user(buf, &evt->opal_event,
> +		sizeof(struct opal_plat_event))) {
> +
> +		/*
> +		 * Copy to user has failed. The event node had
> +		 * been deleted from the list. Lets add it back
> +		 * there.
> +		 */
> +		spin_lock_irqsave(&opal_plat_evt_spinlock, flags);
> +		list_add_tail(&evt->link, &opal_event_queue);
> +		spin_unlock_irqrestore(&opal_plat_evt_spinlock, flags);
> +		return -EFAULT;
> +	}
> +
> +	kfree(evt);
> +	return sizeof(struct opal_plat_event);
> +}
> +
> +/*
> + * opal_event_poll
> + *
> + * Poll is unblocked right away with POLLIN when data is available.
> + * When data is not available, the process will have to block till
> + * it gets waked up and data is available to read.
> + */
> +static unsigned int opal_event_poll(struct file *file, poll_table *wait)
> +{
> +	poll_wait(file, &opal_plat_evt_wait, wait);
> +	if (!list_empty(&opal_event_queue))
> +		return POLLIN;
> +	return 0;
> +}
> +
> +/*
> + * opal_event_open
> + *
> + * This makes sure that only one process can open the
> + * character device file at any point of time. Others
> + * attempting to open the file descriptor will either
> + * get EBUSY (with O_NONBLOCK flag) or wait for the
> + * other process to close the file descriptor.
> + */
> +static int opal_event_open(struct inode *inode, struct file *file)
> +{
> +	int err;
> +
> +	mutex_lock(&opal_plat_evt_mutex);
> +	while (opal_event_open_flag) {
> +		mutex_unlock(&opal_plat_evt_mutex);
> +		if (file->f_flags & O_NONBLOCK)
> +			return -EBUSY;

O_NONBLOCK would signify that all subsequent operations are
non-blocking, not just the open. It happens that this is the case
(because of your requirement on poll()), but it shouldn't be.

Just return -EBUSY, and get rid of the wait queue.

> +/* Process the received EPOW information */
> +void process_epow(__u64 *epow, int16_t *epow_status, int max_epow_class)
> +{
> +	/*
> +	 * Platform might have returned less number of EPOW
> +	 * subclass status than asked for. This situation
> +	 * happens when the platform firmware is older compared
> +	 * to the kernel.
> +	 */
> +
> +	if (!max_epow_class) {
> +		pr_warn("EPOW: OPAL_SYSEPOW_POWER subclass not present\n");
> +		return;
> +	}
> +
> +	/* Power */
> +	max_epow_class--;
> +	if (epow_status[OPAL_SYSEPOW_POWER] & OPAL_SYSPOWER_CHNG) {
> +		pr_info("EPOW: Power configuration changed\n");
> +		epow[EPOW_SYSPOWER_CHNG] = 1;
> +	}
> +
> +	if (epow_status[OPAL_SYSEPOW_POWER] & OPAL_SYSPOWER_FAIL) {
> +		pr_info("EPOW: Impending system power failure\n");
> +		epow[EPOW_SYSPOWER_FAIL] = 1;
> +	}
> +
> +	if (epow_status[OPAL_SYSEPOW_POWER] & OPAL_SYSPOWER_INCL) {
> +		pr_info("EPOW: Incomplete system power\n");
> +		epow[EPOW_SYSPOWER_INCL] = 1;
> +	}
> +
> +	if (epow_status[OPAL_SYSEPOW_POWER] & OPAL_SYSPOWER_UPS) {
> +		pr_info("EPOW: System on UPS power\n");
> +		epow[EPOW_SYSPOWER_UPS] = 1;
> +	}
> +
> +	if (!max_epow_class) {
> +		pr_warn("EPOW: OPAL_SYSEPOW_TEMP subclass not present\n");
> +		return;
> +	}
> +
> +	/* Temperature */
> +	max_epow_class--;
> +	if (epow_status[OPAL_SYSEPOW_TEMP] & OPAL_SYSTEMP_AMB) {
> +		pr_info("EPOW: Over ambient temperature\n");
> +		epow[EPOW_SYSTEMP_AMB] = 1;
> +	}
> +
> +	if (epow_status[OPAL_SYSEPOW_TEMP] & OPAL_SYSTEMP_INT) {
> +		pr_info("EPOW: Over internal temperature\n");
> +		epow[EPOW_SYSTEMP_INT] = 1;
> +	}
> +
> +	if (epow_status[OPAL_SYSEPOW_TEMP] & OPAL_SYSTEMP_HMD) {
> +		pr_info("EPOW: Over internal humidity\n");
> +		epow[EPOW_SYSTEMP_HMD] = 1;
> +	}
> +
> +	if (!max_epow_class) {
> +		pr_warn("EPOW: OPAL_SYSEPOW_COOLING subclass not present\n");
> +		return;
> +	}
> +
> +	/* Cooling */
> +	max_epow_class--;
> +	if (epow_status[OPAL_SYSEPOW_COOLING] & OPAL_SYSCOOL_INSF) {
> +		pr_info("EPOW: Insufficient cooling\n");
> +		epow[EPOW_SYSCOOL_INSF] = 1;
> +	}
> +}

This is fairly verbose, due to the separate u64s in the epow types.

> +
> +/*
> + * fetch_epow_status
> + *
> + * Fetch the system EPOW status through an OPAL call and
> + * validate the number of EPOW sub class status received.
> + */
> +static void fetch_epow_status(int16_t *epow_status, int16_t *n_epow)
> +{
> +	int rc;
> +
> +	memset(epow_status, 0, sizeof(int16_t) * OPAL_SYSEPOW_MAX);
> +	*n_epow = OPAL_SYSEPOW_MAX;
> +	rc = opal_get_epow_status(epow_status, n_epow);
> +	if (rc != OPAL_SUCCESS) {
> +		pr_err("EPOW: OPAL call failed\n");
> +		memset(epow_status, 0, sizeof(int16_t) * OPAL_SYSEPOW_MAX);
> +		*n_epow = 0;
> +		return;
> +	}
> +	if (!(*n_epow))
> +		pr_err("EPOW: No subclass status received\n");
> +}

Endian conversion?

> +
> +/*
> + * fetch_dpo_timeout
> + *
> + * Fetch the system DPO timeout status through an OPAL call.
> + */
> +static void fetch_dpo_timeout(int64_t *dpo_timeout)
> +{
> +	int rc;
> +
> +	rc = opal_get_dpo_status(dpo_timeout);
> +	if (rc == OPAL_WRONG_STATE) {
> +		pr_info("DPO: Not initiated by OPAL\n");
> +		*dpo_timeout = 0;
> +	}
> +}

And here too?

> +
> +/*
> + * valid_epow
> + *
> + * Validate the received EPOW event status. This ensures
> + * that there are valid status for various EPOW sub classes
> + * and their individual events.
> + */
> +static bool valid_epow(int16_t *epow_status, int16_t n_epow)
> +{
> +	int i;
> +
> +	/* EPOW sub classes present */
> +	if (!n_epow)
> +		return false;
> +
> +	/* EPOW events present */
> +	for (i = 0; i < n_epow; i++) {
> +		if (epow_status[i])
> +			return true;
> +	}
> +	return false;
> +}
> +
> +/*
> + * epow_exclude
> + *
> + * XXX: EPOW events on the action exclude list. System shutdown
> + * would not be scheduled for all these platform events. In future
> + * this should be communicated from the platform firmware through
> + * device tree attributes.
> + */

Exclude from what? System shutdown, right? You have the logic between
this and the subsequent function reversed, which makes it a little
complex to red.

Maybe make this epow_event_is_actionable()?

> +/*
> + * opal_event_handle_basic
> + *
> + * Sets up the basic information for an opal platform event,
> + * activates the timer, adds to the list and wakes up waiting
> + * threads on the character device.
> + */
> +static void opal_event_handle_basic(struct opal_platform_evt *evt,
> +				unsigned long type, unsigned long timeout)
> +{
> +	unsigned long flags;
> +
> +	evt->opal_event.type = type;
> +	switch (type) {
> +	case OPAL_PLAT_EVENT_TYPE_EPOW:
> +		evt->opal_event.size = sizeof(struct epow_event);
> +		evt->opal_event.epow.timeout = timeout;
> +		if (actionable_epow(evt->opal_event.epow.epow))
> +			opal_event_start_timer(OPAL_PLAT_EVENT_TYPE_EPOW,
> +							OPAL_EPOW_TIMEOUT);
> +		break;
> +	case  OPAL_PLAT_EVENT_TYPE_DPO:
> +		evt->opal_event.size = sizeof(struct dpo_event);
> +		evt->opal_event.dpo.orig_timeout = timeout;
> +		opal_event_start_timer(OPAL_PLAT_EVENT_TYPE_DPO, timeout);
> +		break;
> +	default:
> +		pr_err("Unknown event type\n");
> +		break;
> +	}
> +	spin_lock_irqsave(&opal_plat_evt_spinlock, flags);
> +	list_add_tail(&evt->link, &opal_event_queue);
> +	spin_unlock_irqrestore(&opal_plat_evt_spinlock, flags);
> +	wake_up_interruptible(&opal_plat_evt_wait);
> +}
> +
> +/*
> + * opal_event_existing_status
> + *
> + * Fetch and process existing opal platform event conditions
> + * present on the system. If events detected, add them to the
> + * list which can be consumed by the user space right away.
> + */
> +static void opal_event_existing_status(void)
> +{
> +	struct opal_platform_evt *evt;
> +	int64_t dpo_timeout;
> +	int16_t	epow_status[OPAL_SYSEPOW_MAX], n_epow;
> +
> +	fetch_epow_status(epow_status, &n_epow);
> +	if (valid_epow(epow_status, n_epow)) {
> +		evt = kzalloc(sizeof(struct opal_platform_evt), GFP_KERNEL);
> +		if (!evt) {
> +			pr_err("EPOW: Memory allocation for event failed\n");
> +			return;
> +		}
> +		process_epow(evt->opal_event.epow.epow, epow_status, n_epow);
> +		opal_event_handle_basic(evt, OPAL_PLAT_EVENT_TYPE_EPOW,
> +							OPAL_EPOW_TIMEOUT);
> +	}
> +
> +	fetch_dpo_timeout(&dpo_timeout);
> +	if (dpo_timeout) {
> +		evt = kzalloc(sizeof(struct opal_platform_evt), GFP_KERNEL);
> +		if (!evt) {
> +			pr_err("DPO: Memory allocation for event failed\n");
> +			return;
> +		}
> +		opal_dpo_target = jiffies + dpo_timeout * HZ;
> +		opal_event_handle_basic(evt, OPAL_PLAT_EVENT_TYPE_DPO,
> +								dpo_timeout);
> +	}
> +}
> +
> +/* Platform EPOW message received */
> +static int opal_epow_event(struct notifier_block *nb,
> +				unsigned long msg_type, void *msg)
> +{
> +	struct opal_platform_evt *evt;
> +	int16_t	epow_status[OPAL_SYSEPOW_MAX], n_epow;
> +
> +	if (msg_type != OPAL_MSG_EPOW)
> +		return 0;
> +
> +	fetch_epow_status(epow_status, &n_epow);
> +	if (!valid_epow(epow_status, n_epow))
> +		return -EINVAL;
> +
> +	pr_debug("EPOW event: Power(%x) Thermal(%x) Cooling(%x)\n",
> +			epow_status[0], epow_status[1], epow_status[2]);
> +	evt = kzalloc(sizeof(struct opal_platform_evt), GFP_KERNEL);
> +	if (!evt) {
> +		pr_err("EPOW: Memory allocation for event failed\n");
> +		return -ENOMEM;
> +	}
> +	process_epow(evt->opal_event.epow.epow, epow_status, n_epow);
> +	opal_event_handle_basic(evt,
> +				OPAL_PLAT_EVENT_TYPE_EPOW, OPAL_EPOW_TIMEOUT);
> +	return 0;
> +}
> +
> +/* Platform DPO message received */
> +static int opal_dpo_event(struct notifier_block *nb,
> +				unsigned long msg_type, void *msg)
> +{
> +	struct opal_platform_evt *evt;
> +	int64_t dpo_timeout;
> +
> +	if (msg_type != OPAL_MSG_DPO)
> +		return 0;
> +
> +	fetch_dpo_timeout(&dpo_timeout);
> +	if (!dpo_timeout)
> +		return -EINVAL;
> +
> +	pr_debug("DPO event: Timeout:%llu\n", dpo_timeout);
> +	evt = kzalloc(sizeof(struct opal_platform_evt), GFP_KERNEL);
> +	if (!evt) {
> +		pr_err("DPO: Memory allocation for event failed\n");
> +		return -ENOMEM;
> +	}
> +	opal_dpo_target = jiffies + dpo_timeout * HZ;
> +	opal_event_handle_basic(evt, OPAL_PLAT_EVENT_TYPE_DPO, dpo_timeout);
> +	return 0;
> +}

Okay, these are a little messy:

1) an inocoming EPOW/DPO event will invoke both notifier callbacks

2) the notifier callback will check if it's the right handler for the
   msg_type, and handle the event, so we get separate code paths
   for EPOW & DPO.

3) both callbacks then converge on a single code path, by calling
   opal_event_basic with a specific type argument. Here we get
   a single code path for EPOW & DPO.

4) opal_event_basic then checks the type, and performs separate
   functions depending on EPOW vs. DPO

- and then the logic (and code) is repeated for the
opal_existing_event_status function

Can you unify this, so there's less duplication and conditional code?

Also, are you sure you can kzalloc(GFP_KERNEL) here?


> +/* Platform driver probe */
> +static int opal_event_probe(struct platform_device *pdev)
> +{
> +	struct device *dev;
> +	int ret;
> +
> +	if (opal_event_probe_finished) {
> +		pr_err("%s getting called once again\n", __func__);
> +		return 0;
> +	}
> +	opal_event_probe_finished = true;
> +
> +	init_timer(&opal_event_timer);
> +	opal_event_timer.function = opal_event_timeout;
> +	opal_event_open_flag = false;
> +	opal_dpo_target = 0;
> +
> +	ret = alloc_chrdev_region(&opal_event_dev, 0,
> +					OPAL_EVENT_MAX_DEVS, "opal_event");
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "aloc_chrdev_region failed\n");
> +		return ret;
> +	}
> +
> +	opal_event_class = class_create(THIS_MODULE, "opal_event");
> +	if (IS_ERR(opal_event_class)) {
> +		ret = PTR_ERR(opal_event_class);
> +		dev_err(&pdev->dev, "class_create failed with %d\n", ret);
> +		goto fail_chrdev;
> +	}
> +
> +	dev = device_create(opal_event_class, &pdev->dev,
> +					opal_event_dev, NULL, "opal_event");
> +	if (IS_ERR(dev)) {
> +		ret = PTR_ERR(dev);
> +		dev_err(&pdev->dev, "device_create failed with %d\n", ret);
> +		goto fail_class;
> +	}
> +
> +	cdev_init(&opal_event_cdev, &fops);
> +	ret = cdev_add(&opal_event_cdev, opal_event_dev, OPAL_EVENT_MAX_DEVS);
> +	if (ret < 0) {
> +		dev_err(dev, "cdev_add failed\n");
> +		ret = -EINVAL;
> +		goto fail_device;
> +	}
>

You can probably avoid a lot of this by using a miscdev. Do we need a
whole chardev class here?

> +static int __init opal_platform_event_init(void)
> +{
> +	opal_event_probe_finished = false;
> +	return platform_driver_register(&opal_event_driver);
> +}
> +
> +static void __exit opal_platform_event_exit(void)
> +{
> +	platform_driver_unregister(&opal_event_driver);
> +}
> +module_init(opal_platform_event_init);
> +module_exit(opal_platform_event_exit);


You can replace all of this with:

module_platform_driver(opal_event_driver);

And your probe function should only be called once. Are you sure you need
that opal_event_probe_finished check?

Regards,


Jeremy

^ permalink raw reply

* net: ucc: tbi phy detection broken by 058112c7efc9ef43bb511c137293dddbe6e42908
From: Lennart Sorensen @ 2014-12-19  3:49 UTC (permalink / raw)
  To: Len Sorensen
  Cc: Florian Fainelli, netdev, linux-kernel, linuxppc-dev,
	David S. Miller, Sebastian Hesselbarth

I have been trying to move an 8360 based system from a 3.0 kernel to a
3.12 (on the way to 3.14 with ipipe/xenomai) kernel and encountered an
oops in the ucc_geth driver when using RTBI mode on one of the ucc
ports.  I haven't managed to find any commits to of_mdio or ucc_geth or
fsl_pq_mdio that would appear to address this problem, so I believe it
is still present in the latest kernel, but have not confirmed that with
testing yet.

Commit 058112c7efc9ef43bb511c137293dddbe6e42908 appears to have broken
ucc support for tbi phy detection.

With the patch in place, I am unable to get the mdio bus to create phy
devices for the tbi phy in the ucc on an 8360e, and the ucc_geth driver
causes a kernel oops, while with the patch reverted, it does create them
and the driver comes up and works.

The tbi phy is needed when using a ucc in RTBI, TBI or SGMII mode.

I am not convinced that the tbi phy really behaves quite like a real phy,
which may be why get_phy_device does not work with it.  Perhaps there
is a better way to deal with the tbi phy on the ucc for this purpose.

Certainly as it is, this patch has caused a regression though, although
probably not very many systems with ucc ports actually use one of the
affected modes so the damage isn't that great.

-- 
Len Sorensen

^ permalink raw reply

* [PATCH v3] powerpc/book3s: Fix flush_tlb cpu_spec hook to take a generic argument.
From: Mahesh J Salgaonkar @ 2014-12-19  3:11 UTC (permalink / raw)
  To: linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt; +Cc: Paul Mackerras

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

The flush_tlb hook in cpu_spec was introduced as a generic function hook
to invalidate TLBs. But the current implementation of flush_tlb hook
takes IS (invalidation selector) as an argument which is architecture
dependent. Hence, It is not right to have a generic routine where caller
has to pass non-generic argument.

This patch fixes this and makes flush_tlb hook as high level API.

Reported-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
Changes in V3:
- Changed TLB flush action #defines into enum.

Changes in V2:
- Moved TLB flush action #defines to cputable.h
- Added missing ptesyncs to before and after TLB flush.
- Moved switch case to flush_tlb_206() function as suggested by
  Michael Ellerman.
---
 arch/powerpc/include/asm/cputable.h   |    8 ++++-
 arch/powerpc/include/asm/mmu-hash64.h |    1 +
 arch/powerpc/kernel/cpu_setup_power.S |   10 +-----
 arch/powerpc/kernel/cputable.c        |    4 +-
 arch/powerpc/kernel/mce_power.c       |   53 ++++++++++++++++++++++++++++++++-
 arch/powerpc/kvm/book3s_hv_ras.c      |    4 +-
 6 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index daa5af9..493af2d 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -100,7 +100,7 @@ struct cpu_spec {
 	/*
 	 * Processor specific routine to flush tlbs.
 	 */
-	void		(*flush_tlb)(unsigned long inval_selector);
+	void		(*flush_tlb)(unsigned int action);
 
 };
 
@@ -114,6 +114,12 @@ extern void do_feature_fixups(unsigned long value, void *fixup_start,
 
 extern const char *powerpc_base_platform;
 
+/* TLB flush actions. Used as argument to cpu_spec.flush_tlb() hook */
+enum {
+	TLB_INVAL_SCOPE_GLOBAL = 0,	/* invalidate all TLBs */
+	TLB_INVAL_SCOPE_LPID = 1,	/* invalidate TLBs for current LPID */
+};
+
 #endif /* __ASSEMBLY__ */
 
 /* CPU kernel features */
diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
index aeebc94..4f50db7 100644
--- a/arch/powerpc/include/asm/mmu-hash64.h
+++ b/arch/powerpc/include/asm/mmu-hash64.h
@@ -112,6 +112,7 @@
 #define TLBIEL_INVAL_SET_SHIFT	12
 
 #define POWER7_TLB_SETS		128	/* # sets in POWER7 TLB */
+#define POWER8_TLB_SETS		512	/* # sets in POWER8 TLB */
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/powerpc/kernel/cpu_setup_power.S b/arch/powerpc/kernel/cpu_setup_power.S
index 4673353..9c9b741 100644
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ b/arch/powerpc/kernel/cpu_setup_power.S
@@ -137,15 +137,11 @@ __init_HFSCR:
 /*
  * Clear the TLB using the specified IS form of tlbiel instruction
  * (invalidate by congruence class). P7 has 128 CCs., P8 has 512.
- *
- * r3 = IS field
  */
 __init_tlb_power7:
-	li	r3,0xc00	/* IS field = 0b11 */
-_GLOBAL(__flush_tlb_power7)
 	li	r6,128
 	mtctr	r6
-	mr	r7,r3		/* IS field */
+	li	r7,0xc00	/* IS field = 0b11 */
 	ptesync
 2:	tlbiel	r7
 	addi	r7,r7,0x1000
@@ -154,11 +150,9 @@ _GLOBAL(__flush_tlb_power7)
 1:	blr
 
 __init_tlb_power8:
-	li	r3,0xc00	/* IS field = 0b11 */
-_GLOBAL(__flush_tlb_power8)
 	li	r6,512
 	mtctr	r6
-	mr	r7,r3		/* IS field */
+	li	r7,0xc00	/* IS field = 0b11 */
 	ptesync
 2:	tlbiel	r7
 	addi	r7,r7,0x1000
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 8084059..81c1ba5 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -71,8 +71,8 @@ extern void __restore_cpu_power7(void);
 extern void __setup_cpu_power8(unsigned long offset, struct cpu_spec* spec);
 extern void __restore_cpu_power8(void);
 extern void __restore_cpu_a2(void);
-extern void __flush_tlb_power7(unsigned long inval_selector);
-extern void __flush_tlb_power8(unsigned long inval_selector);
+extern void __flush_tlb_power7(unsigned int action);
+extern void __flush_tlb_power8(unsigned int action);
 extern long __machine_check_early_realmode_p7(struct pt_regs *regs);
 extern long __machine_check_early_realmode_p8(struct pt_regs *regs);
 #endif /* CONFIG_PPC64 */
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index b6f123a..2c647b1 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -28,6 +28,55 @@
 #include <asm/mce.h>
 #include <asm/machdep.h>
 
+static void flush_tlb_206(unsigned int num_sets, unsigned int action)
+{
+	unsigned long rb;
+	unsigned int i;
+
+	switch (action) {
+	case TLB_INVAL_SCOPE_GLOBAL:
+		rb = TLBIEL_INVAL_SET;
+		break;
+	case TLB_INVAL_SCOPE_LPID:
+		rb = TLBIEL_INVAL_SET_LPID;
+		break;
+	default:
+		BUG();
+		break;
+	}
+
+	asm volatile("ptesync" : : : "memory");
+	for (i = 0; i < num_sets; i++) {
+		asm volatile("tlbiel %0" : : "r" (rb));
+		rb += 1 << TLBIEL_INVAL_SET_SHIFT;
+	}
+	asm volatile("ptesync" : : : "memory");
+}
+
+/*
+ * Generic routine to flush TLB on power7. This routine is used as
+ * flush_tlb hook in cpu_spec for Power7 processor.
+ *
+ * action => TLB_INVAL_SCOPE_GLOBAL:  Invalidate all TLBs.
+ *	     TLB_INVAL_SCOPE_LPID: Invalidate TLB for current LPID.
+ */
+void __flush_tlb_power7(unsigned int action)
+{
+	flush_tlb_206(POWER7_TLB_SETS, action);
+}
+
+/*
+ * Generic routine to flush TLB on power8. This routine is used as
+ * flush_tlb hook in cpu_spec for power8 processor.
+ *
+ * action => TLB_INVAL_SCOPE_GLOBAL:  Invalidate all TLBs.
+ *	     TLB_INVAL_SCOPE_LPID: Invalidate TLB for current LPID.
+ */
+void __flush_tlb_power8(unsigned int action)
+{
+	flush_tlb_206(POWER8_TLB_SETS, action);
+}
+
 /* flush SLBs and reload */
 static void flush_and_reload_slb(void)
 {
@@ -79,7 +128,7 @@ static long mce_handle_derror(uint64_t dsisr, uint64_t slb_error_bits)
 	}
 	if (dsisr & P7_DSISR_MC_TLB_MULTIHIT_MFTLB) {
 		if (cur_cpu_spec && cur_cpu_spec->flush_tlb)
-			cur_cpu_spec->flush_tlb(TLBIEL_INVAL_SET);
+			cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_GLOBAL);
 		/* reset error bits */
 		dsisr &= ~P7_DSISR_MC_TLB_MULTIHIT_MFTLB;
 	}
@@ -110,7 +159,7 @@ static long mce_handle_common_ierror(uint64_t srr1)
 		break;
 	case P7_SRR1_MC_IFETCH_TLB_MULTIHIT:
 		if (cur_cpu_spec && cur_cpu_spec->flush_tlb) {
-			cur_cpu_spec->flush_tlb(TLBIEL_INVAL_SET);
+			cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_GLOBAL);
 			handled = 1;
 		}
 		break;
diff --git a/arch/powerpc/kvm/book3s_hv_ras.c b/arch/powerpc/kvm/book3s_hv_ras.c
index d562c8e..d10dd57 100644
--- a/arch/powerpc/kvm/book3s_hv_ras.c
+++ b/arch/powerpc/kvm/book3s_hv_ras.c
@@ -84,7 +84,7 @@ static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
 		}
 		if (dsisr & DSISR_MC_TLB_MULTI) {
 			if (cur_cpu_spec && cur_cpu_spec->flush_tlb)
-				cur_cpu_spec->flush_tlb(TLBIEL_INVAL_SET_LPID);
+				cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_LPID);
 			dsisr &= ~DSISR_MC_TLB_MULTI;
 		}
 		/* Any other errors we don't understand? */
@@ -102,7 +102,7 @@ static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
 		break;
 	case SRR1_MC_IFETCH_TLBMULTI:
 		if (cur_cpu_spec && cur_cpu_spec->flush_tlb)
-			cur_cpu_spec->flush_tlb(TLBIEL_INVAL_SET_LPID);
+			cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_LPID);
 		break;
 	default:
 		handled = 0;

^ permalink raw reply related

* RE: [RFC PATCH 0/2] powerpc: CR based local atomic operation implementation
From: Rusty Russell @ 2014-12-18 10:53 UTC (permalink / raw)
  To: David Laight, 'Madhavan Srinivasan', mpe@ellerman.id.au
  Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, anton@samba.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CA0E993@AcuExch.aculab.com>

David Laight <David.Laight@ACULAB.COM> writes:
> From: Rusty Russell
>> David Laight <David.Laight@ACULAB.COM> writes:
>> > From: Madhavan Srinivasan [mailto:maddy@linux.vnet.ibm.com]
>> > ...
>> >> >>> I also wonder if it is possible to inspect the interrupted
>> >> >>> code to determine the start/end of the RAS block.
>> >> >>> (Easiest if you assume that there is a single 'write' instruction
>> >> >>> as the last entry in the block.)
>> >> >>>
>> >> >> So each local_* function also have code in the __ex_table section. IIUC,
>> >> >> __ex_table contains two address. So if the return address found in the
>> >> >> first column of the _ex_table, use the corresponding address in the
>> >> >> second column to continue from.
>> >> >
>> >> > That really doesn't scale.
>> >> > I don't know how many 1000 address pairs you table will have (and the
>> >> > ones in each loadable module), but the search isn't going to be cheap.
>> >> >
>> >> > If these sequences are restartable then they can only have one write
>> >> > to memory.
> ...
>> >> 2) resulting code with lot of condition and branch (for opcode decode)
>> >> will be lot messy and may be an issue incase of maintenance,
>> >
>> > You don't need to decode the instructions.
>> > Just look for the two specific instructions used as markers.
>> > This is only really possible with fixed-size instructions.
>> >
>> > It might also be that the 'interrupt entry' path is easier to
>> > modify than the 'interrupt exit' one (fewer code paths) and
>> > you just need to modify the 'pc' in the stack frame.
>> > You are only interested in interrupts from kernel space.
>> 
>> It's an overoptimization for case that statistically never happens.
>> You won't even be able to measure the difference.
>> 
>> The question of bloat remains, but that's also easily measured.  In
>> practice, I'd guess less than 1k.
>
> IIRC they were 'static inline' so the table of addresses is generated
> for every use site.
> (copyin/out generates a similarly enormous table of addresses on amd64)

There are about 20 callers in the entire kernel.

Cheers,
Rusty.

^ permalink raw reply

* Re: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Scott Wood @ 2014-12-18 22:01 UTC (permalink / raw)
  To: Xie Shaohui-B21989
  Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Medve Emilian-EMMEDVE1, Liberman Igal-B31950
In-Reply-To: <DM2PR0301MB08641FB289DBA17AD59FBEA0E26A0@DM2PR0301MB0864.namprd03.prod.outlook.com>

On Thu, 2014-12-18 at 06:53 -0600, Xie Shaohui-B21989 wrote:
> Ping.
> 
> Best Regards, 
> Shaohui Xie

I can't put patches in my -next until the merge window closes.

> > > > +EXAMPLE
> > > > +
> > > > +Example for FMan v2 external MDIO:
> > > > +
> > > > +mdio@f1000 {
> > > > +	compatible = "fsl,fman-xmdio";
> > > > +	reg = <0xf1000 0x1000>;
> > > > +	bus-frequency = <20000>;
> > > > +};
> > >
> > > So the bus frequency is only 20 KHz?  Or is the unit supposed to be
> > > something other than Hz?
> > [S.H] it's only an example, it could be different on real SoCs, but they always
> > lower than the standard one, The standard one is 2.5MHz, I have to use Hz for it.

Is there any SoC for which 20 kHz is the right frequency?  I just want
to make sure the example is realistic.

-Scott

^ permalink raw reply

* [PATCH v2] powerpc/kdump: Ignore failure in enabling big endian exception during crash
From: Hari Bathini @ 2014-12-18 18:06 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh J Salgaonkar

In LE kernel, we currently have a hack for kexec that resets the exception
endian before starting a new kernel as the kernel that is loaded could be a
big endian or a little endian kernel. In kdump case, resetting exception
endian fails when one or more cpus is disabled. But we can ignore the failure
and still go ahead, as in most cases crashkernel will be of same endianess
as primary kernel and reseting endianess is not even needed in those cases.
This patch adds a new inline function to say if this is kdump path. This
function is used at places where such a check is needed.

Changes from v1:
Instead of skipping, ignore failure in enabling big endian exception
during crash

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/kexec.h       |   10 ++++++++++
 arch/powerpc/kernel/machine_kexec_64.c |    2 +-
 arch/powerpc/platforms/pseries/lpar.c  |   10 +++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 19c36cb..0d96d4d 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -86,6 +86,11 @@ extern int overlaps_crashkernel(unsigned long start, unsigned long size);
 extern void reserve_crashkernel(void);
 extern void machine_kexec_mask_interrupts(void);
 
+static inline int is_kdump_path(void)
+{
+	return (crashing_cpu >= 0) ? 1 : 0;
+}
+
 #else /* !CONFIG_KEXEC */
 static inline void crash_kexec_secondary(struct pt_regs *regs) { }
 
@@ -106,6 +111,11 @@ static inline int crash_shutdown_unregister(crash_shutdown_t handler)
 	return 0;
 }
 
+static inline int is_kdump_path(void)
+{
+	return 0;
+}
+
 #endif /* CONFIG_KEXEC */
 #endif /* ! __ASSEMBLY__ */
 #endif /* __KERNEL__ */
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 879b3aa..b4fe804 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -330,7 +330,7 @@ void default_machine_kexec(struct kimage *image)
         * using debugger IPI.
         */
 
-	if (crashing_cpu == -1)
+	if (!is_kdump_path())
 		kexec_prepare_cpus();
 
 	pr_debug("kexec: Starting switchover sequence.\n");
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 469751d..63214fa 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -43,6 +43,7 @@
 #include <asm/trace.h>
 #include <asm/firmware.h>
 #include <asm/plpar_wrappers.h>
+#include <asm/kexec.h>
 #include <asm/fadump.h>
 
 #include "pseries.h"
@@ -257,6 +258,7 @@ static void pSeries_lpar_hptab_clear(void)
 	 *
 	 * This is also called on boot when a fadump happens. In that case we
 	 * must not change the exception endian mode.
+	 *
 	 */
 	if (firmware_has_feature(FW_FEATURE_SET_MODE) && !is_fadump_active()) {
 		long rc;
@@ -267,8 +269,14 @@ static void pSeries_lpar_hptab_clear(void)
 		 * out to the user, but at least this will stop us from
 		 * continuing on further and creating an even more
 		 * difficult to debug situation.
+		 *
+		 * But if we reaching here after a crash, no point panicking.
+		 * Also, in kdump path, resetting endianess may not be needed
+		 * as the crashkernel most of the times is of same endianess
+		 * as primary kernel. So, let's ignore the failure and try
+		 * kdump'ing anyway.
 		 */
-		if (rc)
+		if (rc && !is_kdump_path())
 			panic("Could not enable big endian exceptions");
 	}
 #endif

^ permalink raw reply related

* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Alexander Graf @ 2014-12-18 15:02 UTC (permalink / raw)
  To: Anton Blanchard, Michael Ellerman
  Cc: Alan Modra, ulrich.weigand, paulus, Scott Wood, linuxppc-dev
In-Reply-To: <20141218172546.432de7f2@kryten>



On 18.12.14 07:25, Anton Blanchard wrote:
> On Thu, 18 Dec 2014 16:11:54 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
>> On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
>>> On 31.10.14 04:47, Anton Blanchard wrote:
>>>> LLVM doesn't support local named register variables and is
>>>> unlikely to. current_thread_info is using one, fix it by moving
>>>> it out and calling it __current_r1().
>>>>
>>>> I gave it a bit of an obscure name because we don't want anyone
>>>> else using it - they should use current_stack_pointer(). This
>>>> specific case is performance critical and we can't afford to call
>>>> a function to get it. Furthermore it isn't important to know
>>>> exactly where in the stack we are since we mask the lower bits.
>>>>
>>>> Signed-off-by: Anton Blanchard <anton@samba.org>
>>>
>>> Git bisect managed to point me to this commit as the offender for
>>> OOPSes on e5500 and e6500 (and maybe the G4 as well, not sure).
>>>
>>> Doing a git revert of this commit on top of linus/master makes
>>> things work fine for me again.
>>>
>>>
>>> Alex
>>>
>>> Oops: Kernel access of bad area, sig: 11 [#2]
>>> SMP NR_CPUS=16 CoreNet Generic
>>> Modules linked in:
>>> CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G      D
>>> 3.18.0-09423-g988adfd #1
>>> Workqueue: rpciod .rpc_async_schedule
>>> task: c0000001f6397500 ti: c0000001f6638000 task.ti:
>>> c0000001f6638000 NIP: c0000000004817a4 LR: c0000000004817a4 CTR:
>>> 0000000000000000 REGS: c0000001f663b0e0 TRAP: 0300   Tainted:
>>> G      D (3.18.0-09423-g988adfd)
>>> MSR: 0000000080029000 <CE,EE,ME>  CR: 24ad2e42  XER: 00000000
>>> DEAR: 202031303438355f ESR: 0000000000000000 SOFTE: 1
>>       = r9 + 40
>>
>>> GPR00: c0000000004817a4 c0000001f663b360 c000000000988028
>>> 000000007f24333d GPR04: 5ff5738c1f2ebfb1 0000000000000000
>>> 0000000000000000 00000000000008f8 GPR08: c000000000480ae8
>>> 2020313034383537 36204b4220617320 6469726563740a31 GPR12:
>>> 3937302d30312d30 c00000000fff8780 c00000000007f988 c0000001f64c1600
>>
>> GPRs 9-12 say: " 1048576 KB as direct\n1970-01-0"
>>
>> Which is rarely a good sign :)
>>
>> Looks like it might be part of your dmesg from setup_page_sizes().
>>
>>> GPR16: 0000000000000000 0000000000000000 0000000000000000
>>> 00000000000005dc GPR20: c0000000009b8028 c00000007e034200
>>> 0000000000000548 c000000000000000 GPR24: c0000001f663b4b0
>>> 00000000b225831e 0000000000000000 0000000000000080 GPR28:
>>> 0000000000000548 00000000000008f8 0000000000000548 0000000000000094
>>> NIP [c0000000004817a4] .__skb_checksum+0x194/0x378 LR
>>> [c0000000004817a4] .__skb_checksum+0x194/0x378 Call Trace:
>>> [c0000001f663b360] [c0000000004817a4] .__skb_checksum+0x194/0x378
>>> (unreliable)
>>> [c0000001f663b440] [c0000000004819b4] .skb_checksum+0x2c/0x3c
>>> [c0000001f663b4c0] [c0000000004fd0a8] .udp4_hwcsum+0xa8/0x16c
>>> [c0000001f663b560] [c0000000004fd440] .udp_send_skb+0x2d4/0x370
>>> [c0000001f663b600]
>>> [c0000000004fd51c] .udp_push_pending_frames+0x40/0x94
>>> [c0000001f663b680] [c0000000004fec08] .udp_sendpage+0x150/0x1b4
>>> [c0000001f663b770] [c00000000050ae54] .inet_sendpage+0xa0/0x120
>>> [c0000001f663b810] [c00000000059c8cc] .xs_sendpages+0x2d0/0x30c
>>> [c0000001f663b8d0]
>>> [c00000000059cae4] .xs_udp_send_request+0x58/0x120
>>> [c0000001f663b970] [c000000000598f04] .xprt_transmit+0x80/0x36c
>>> [c0000001f663ba20] [c0000000005942d8] .call_transmit+0x19c/0x254
>>> [c0000001f663bab0] [c00000000059ff64] .__rpc_execute+0xbc/0x3c0
>>> [c0000001f663bb90] [c0000000000797f8] .process_one_work+0x1c0/0x474
>>> [c0000001f663bc40] [c00000000007a518] .worker_thread+0x17c/0x54c
>>> [c0000001f663bd30] [c00000000007fa8c] .kthread+0x104/0x124
>>> [c0000001f663be30]
>>> [c000000000000884] .ret_from_kernel_thread+0x58/0xd4 Instruction
>>> dump: 7d1f3a14 7c6a1850 e9580000 7fbd4050 786334e4 e90a0000
>>> 7c63ba14 f8490028 7c63ea14 7d0903a6 e84a0008 4e800421 <e8490028>
>>> 7c641b78 78270464 e9580008
>>
>> Which is:
>>
>> add      r8, r31, r7
>> subf     r3, r10, r3
>> ld      r10, 0(r24)
>> subf    r29, r29, r8
>> rldicr   r3, r3, 6, 51
>> ld       r8, 0(r10)
>> add      r3, r3, r23
>> std      r2, 40(r9)
>> add      r3, r3, r29
>> mtctr    r8
>> ld       r2, 8(r10)
>> bctrl
>> ld       r2, 40(r9)	<---
>> mr       r4, r3
>> rldicr   r7, r1, 0, 49
>> ld      r10, 8(r24)
>>
>>
>> Which looks a bit odd. I'd expect us to be saving/restoring r2 to the
>> stack, though maybe r9 was pointing at the stack?
> 
> Nice catch! This looks like a compiler bug.
> 
>> Looking at your vmlinux.broken I don't see the same code gen.
> 
> For whatever reason we ended up with r10 this time:
> 
> 7c 2a 0b 78     mr      r10,r1
> ...
> f8 4a 00 28     std     r2,40(r10)
> 7c 63 ba 14     add     r3,r3,r23
> 7c e9 03 a6     mtctr   r7
> 7c 63 ea 14     add     r3,r3,r29
> 38 a0 00 00     li      r5,0
> e8 48 00 08     ld      r2,8(r8)
> 4e 80 04 21     bctrl
> e8 4a 00 28     ld      r2,40(r10)
> 
> The indirect function call is allowed to clobber r10, gcc is doing
> something very wrong here.

Yeah, I couldn't see why the patch really would break anything either,
but it does for me with this (not quite old) version of gcc.

I also don't see the breakage on LE machines that compile with a newer
version of gcc (4.8.3).


Alex

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Alexander Graf @ 2014-12-18 14:56 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: ulrich.weigand, paulus, Anton Blanchard, Scott Wood, linuxppc-dev
In-Reply-To: <1418879514.13333.2.camel@ellerman.id.au>



On 18.12.14 06:11, Michael Ellerman wrote:
> On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
>> On 31.10.14 04:47, Anton Blanchard wrote:
>>> LLVM doesn't support local named register variables and is unlikely
>>> to. current_thread_info is using one, fix it by moving it out and
>>> calling it __current_r1().
>>>
>>> I gave it a bit of an obscure name because we don't want anyone else
>>> using it - they should use current_stack_pointer(). This specific
>>> case is performance critical and we can't afford to call a function
>>> to get it. Furthermore it isn't important to know exactly where in
>>> the stack we are since we mask the lower bits.
>>>
>>> Signed-off-by: Anton Blanchard <anton@samba.org>
>>
>> Git bisect managed to point me to this commit as the offender for OOPSes
>> on e5500 and e6500 (and maybe the G4 as well, not sure).
>>
>> Doing a git revert of this commit on top of linus/master makes things
>> work fine for me again.
>>
>>
>> Alex
>>
>> Oops: Kernel access of bad area, sig: 11 [#2]
>> SMP NR_CPUS=16 CoreNet Generic
>> Modules linked in:
>> CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G      D
>> 3.18.0-09423-g988adfd #1
>> Workqueue: rpciod .rpc_async_schedule
>> task: c0000001f6397500 ti: c0000001f6638000 task.ti: c0000001f6638000
>> NIP: c0000000004817a4 LR: c0000000004817a4 CTR: 0000000000000000
>> REGS: c0000001f663b0e0 TRAP: 0300   Tainted: G      D
>> (3.18.0-09423-g988adfd)
>> MSR: 0000000080029000 <CE,EE,ME>  CR: 24ad2e42  XER: 00000000
>> DEAR: 202031303438355f ESR: 0000000000000000 SOFTE: 1
>       = r9 + 40
> 
>> GPR00: c0000000004817a4 c0000001f663b360 c000000000988028 000000007f24333d
>> GPR04: 5ff5738c1f2ebfb1 0000000000000000 0000000000000000 00000000000008f8
>> GPR08: c000000000480ae8 2020313034383537 36204b4220617320 6469726563740a31
>> GPR12: 3937302d30312d30 c00000000fff8780 c00000000007f988 c0000001f64c1600
> 
> GPRs 9-12 say: " 1048576 KB as direct\n1970-01-0"
> 
> Which is rarely a good sign :)
> 
> Looks like it might be part of your dmesg from setup_page_sizes().
> 
>> GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000000005dc
>> GPR20: c0000000009b8028 c00000007e034200 0000000000000548 c000000000000000
>> GPR24: c0000001f663b4b0 00000000b225831e 0000000000000000 0000000000000080
>> GPR28: 0000000000000548 00000000000008f8 0000000000000548 0000000000000094
>> NIP [c0000000004817a4] .__skb_checksum+0x194/0x378
>> LR [c0000000004817a4] .__skb_checksum+0x194/0x378
>> Call Trace:
>> [c0000001f663b360] [c0000000004817a4] .__skb_checksum+0x194/0x378
>> (unreliable)
>> [c0000001f663b440] [c0000000004819b4] .skb_checksum+0x2c/0x3c
>> [c0000001f663b4c0] [c0000000004fd0a8] .udp4_hwcsum+0xa8/0x16c
>> [c0000001f663b560] [c0000000004fd440] .udp_send_skb+0x2d4/0x370
>> [c0000001f663b600] [c0000000004fd51c] .udp_push_pending_frames+0x40/0x94
>> [c0000001f663b680] [c0000000004fec08] .udp_sendpage+0x150/0x1b4
>> [c0000001f663b770] [c00000000050ae54] .inet_sendpage+0xa0/0x120
>> [c0000001f663b810] [c00000000059c8cc] .xs_sendpages+0x2d0/0x30c
>> [c0000001f663b8d0] [c00000000059cae4] .xs_udp_send_request+0x58/0x120
>> [c0000001f663b970] [c000000000598f04] .xprt_transmit+0x80/0x36c
>> [c0000001f663ba20] [c0000000005942d8] .call_transmit+0x19c/0x254
>> [c0000001f663bab0] [c00000000059ff64] .__rpc_execute+0xbc/0x3c0
>> [c0000001f663bb90] [c0000000000797f8] .process_one_work+0x1c0/0x474
>> [c0000001f663bc40] [c00000000007a518] .worker_thread+0x17c/0x54c
>> [c0000001f663bd30] [c00000000007fa8c] .kthread+0x104/0x124
>> [c0000001f663be30] [c000000000000884] .ret_from_kernel_thread+0x58/0xd4
>> Instruction dump:
>> 7d1f3a14 7c6a1850 e9580000 7fbd4050 786334e4 e90a0000 7c63ba14 f8490028
>> 7c63ea14 7d0903a6 e84a0008 4e800421 <e8490028> 7c641b78 78270464 e9580008
> 
> Which is:
> 
> add      r8, r31, r7
> subf     r3, r10, r3
> ld      r10, 0(r24)
> subf    r29, r29, r8
> rldicr   r3, r3, 6, 51
> ld       r8, 0(r10)
> add      r3, r3, r23
> std      r2, 40(r9)
> add      r3, r3, r29
> mtctr    r8
> ld       r2, 8(r10)
> bctrl
> ld       r2, 40(r9)	<---
> mr       r4, r3
> rldicr   r7, r1, 0, 49
> ld      r10, 8(r24)
> 
> 
> Which looks a bit odd. I'd expect us to be saving/restoring r2 to the stack,
> though maybe r9 was pointing at the stack?
> 
> Looking at your vmlinux.broken I don't see the same code gen.
> 
> Can you get an oops from a kernel and upload the exact binary? Or just post us
> the full code dump of __skb_checksum() (or wherever it oopses).

Ugh, sorry - I must've copied the wrong one. The serial output below is
from the uImage that (hopefully) is belongs to the vmlinux.broken:

  http://csgraf.de/agraf/current_thread_info/dmesg.txt


Alex

^ permalink raw reply

* RE: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Shaohui Xie @ 2014-12-18 12:53 UTC (permalink / raw)
  To: Shaohui Xie, Scott Wood
  Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Emilian Medve, Igal.Liberman@freescale.com
In-Reply-To: <1416966592.15957.172.camel@freescale.com>

UGluZy4NCg0KQmVzdCBSZWdhcmRzLCANClNoYW9odWkgWGllDQo+IC0tLS0tT3JpZ2luYWwgTWVz
c2FnZS0tLS0tDQo+IEZyb206IFhpZSBTaGFvaHVpLUIyMTk4OQ0KPiBTZW50OiBXZWRuZXNkYXks
IE5vdmVtYmVyIDI2LCAyMDE0IDEwOjExIEFNDQo+IFRvOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiBD
YzogbGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmc7IGRldmljZXRyZWVAdmdlci5rZXJuZWwu
b3JnOyBNZWR2ZSBFbWlsaWFuLQ0KPiBFTU1FRFZFMTsgTGliZXJtYW4gSWdhbC1CMzE5NTANCj4g
U3ViamVjdDogUkU6IFtQQVRDSF0gW3YyXSBwb3dlci9mc2w6IGFkZCBNRElPIGR0IGJpbmRpbmcg
Zm9yIEZNYW4NCj4gDQo+ID4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiBGcm9tOiBX
b29kIFNjb3R0LUIwNzQyMQ0KPiA+IFNlbnQ6IFdlZG5lc2RheSwgTm92ZW1iZXIgMjYsIDIwMTQg
OTo1MCBBTQ0KPiA+IFRvOiBzaGgueGllQGdtYWlsLmNvbQ0KPiA+IENjOiBsaW51eHBwYy1kZXZA
bGlzdHMub3psYWJzLm9yZzsgZGV2aWNldHJlZUB2Z2VyLmtlcm5lbC5vcmc7IE1lZHZlDQo+ID4g
RW1pbGlhbi1FTU1FRFZFMTsgTGliZXJtYW4gSWdhbC1CMzE5NTA7IFhpZSBTaGFvaHVpLUIyMTk4
OQ0KPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0hdIFt2Ml0gcG93ZXIvZnNsOiBhZGQgTURJTyBkdCBi
aW5kaW5nIGZvciBGTWFuDQo+ID4NCj4gPiBPbiBGcmksIDIwMTQtMTEtMTQgYXQgMTc6NTMgKzA4
MDAsIHNoaC54aWVAZ21haWwuY29tIHdyb3RlOg0KPiA+ID4gRnJvbTogU2hhb2h1aSBYaWUgPFNo
YW9odWkuWGllQGZyZWVzY2FsZS5jb20+DQo+ID4gPg0KPiA+ID4gVGhpcyBiaW5kaW5nIGlzIGZv
ciBGTWFuIE1ESU8sIGl0IGNvdmVycyBGTWFuIHYyICYgRk1hbiB2My4NCj4gPiA+DQo+ID4gPiBT
aWduZWQtb2ZmLWJ5OiBTaGFvaHVpIFhpZSA8U2hhb2h1aS5YaWVAZnJlZXNjYWxlLmNvbT4NCj4g
PiA+IC0tLQ0KPiA+ID4gY2hhbmdlcyBpbiBWMjoNCj4gPiA+IGFkZHJlc3NlZCBjb21tZW50cyBm
cm9tIFNjb3R0IGluIFYxLg0KPiA+ID4NCj4gPiA+ICAuLi4vZGV2aWNldHJlZS9iaW5kaW5ncy9w
b3dlcnBjL2ZzbC9mbWFuLnR4dCAgICAgICB8IDY5DQo+ID4gKysrKysrKysrKysrKysrKysrKysr
Kw0KPiA+ID4gIDEgZmlsZSBjaGFuZ2VkLCA2OSBpbnNlcnRpb25zKCspDQo+ID4gPg0KPiA+ID4g
ZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9wb3dlcnBjL2Zz
bC9mbWFuLnR4dA0KPiA+ID4gYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvcG93
ZXJwYy9mc2wvZm1hbi50eHQNCj4gPiA+IGluZGV4IGVkZWVhMTYuLjE1MjNhODcgMTAwNjQ0DQo+
ID4gPiAtLS0gYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvcG93ZXJwYy9mc2wv
Zm1hbi50eHQNCj4gPiA+ICsrKyBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9w
b3dlcnBjL2ZzbC9mbWFuLnR4dA0KPiA+ID4gQEAgLTcsNiArNyw3IEBAIENPTlRFTlRTDQo+ID4g
PiAgICAtIEZNYW4gTVVSQU0gTm9kZQ0KPiA+ID4gICAgLSBGTWFuIGRUU0VDL1hHRUMvbUVNQUMg
Tm9kZQ0KPiA+ID4gICAgLSBGTWFuIElFRUUgMTU4OCBOb2RlDQo+ID4gPiArICAtIEZNYW4gTURJ
TyBOb2RlDQo+ID4gPiAgICAtIEV4YW1wbGUNCj4gPiA+DQo+ID4gPg0KPiA+ID4gPT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT0NCj4gPiA+ID09ID09PT09PT0gQEAgLTM1Nyw2ICszNTgsNjggQEAgcHRwLXRpbWVyQGZlMDAw
IHsgIH07DQo+ID4gPg0KPiA+ID4NCj4gPiA+ID09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQo+ID4gPiA9PQ0KPiA+ID4g
PT09PT09PQ0KPiA+ID4gK0ZNYW4gTURJTyBOb2RlDQo+ID4gPiArDQo+ID4gPiArREVTQ1JJUFRJ
T04NCj4gPiA+ICsNCj4gPiA+ICtUaGUgTURJTyBpcyBhIGJ1cyB0byB3aGljaCB0aGUgUEhZIGRl
dmljZXMgYXJlIGNvbm5lY3RlZC4NCj4gPiA+ICsNCj4gPiA+ICtQUk9QRVJUSUVTDQo+ID4gPiAr
DQo+ID4gPiArLSBjb21wYXRpYmxlDQo+ID4gPiArCQlVc2FnZTogcmVxdWlyZWQNCj4gPiA+ICsJ
CVZhbHVlIHR5cGU6IDxzdHJpbmdsaXN0Pg0KPiA+ID4gKwkJRGVmaW5pdGlvbjogQSBzdGFuZGFy
ZCBwcm9wZXJ0eS4NCj4gPiA+ICsJCU11c3QgaW5jbHVkZSAiZnNsLGZtYW4tbWRpbyIgZm9yIDEg
R2IvcyBNRElPIGZyb20gRk1hbiB2Mi4NCj4gPiA+ICsJCU11c3QgaW5jbHVkZSAiZnNsLGZtYW4t
eG1kaW8iIGZvciAxMCBHYi9zIE1ESU8gZnJvbSBGTWFuIHYyLg0KPiA+ID4gKwkJTXVzdCBpbmNs
dWRlICJmc2wsZm1hbi1tZW1hYy1tZGlvIiBmb3IgMS8xMCBHYi9zIE1ESU8gZnJvbQ0KPiA+ID4g
KwkJRk1hbiB2My4NCj4gPiA+ICsNCj4gPiA+ICstIHJlZw0KPiA+ID4gKwkJVXNhZ2U6IHJlcXVp
cmVkDQo+ID4gPiArCQlWYWx1ZSB0eXBlOiA8cHJvcC1lbmNvZGVkLWFycmF5Pg0KPiA+ID4gKwkJ
RGVmaW5pdGlvbjogQSBzdGFuZGFyZCBwcm9wZXJ0eS4NCj4gPiA+ICsNCj4gPiA+ICstIGJ1cy1m
cmVxdWVuY3kNCj4gPiA+ICsJCVVzYWdlOiBvcHRpb25hbA0KPiA+ID4gKwkJVmFsdWUgdHlwZTog
PHUzMj4NCj4gPiA+ICsJCURlZmluaXRpb246IFNwZWNpZmllcyBleHRlcm5hbCBNRElPIGJ1cyBj
bG9jayBzcGVlZCB3aGljaCBpcw0KPiA+ID4gKwkJZGlmZmVyZW50IGZyb20gTURJTyBzdGFuZGFy
ZCAyLjVNSHouIFNob3VsZCBiZSBkZWZpbmVkIGZvcg0KPiA+IFNvQ3MNCj4gPiA+ICsJCW9uIHdo
aWNoIHRoZSBzdGFuZGFyZCBvbmUgY2Fubm90IHdvcmsuDQo+ID4gPiArDQo+ID4gPiArLSBpbnRl
cnJ1cHRzDQo+ID4gPiArCQlVc2FnZTogb3B0aW9uYWwNCj4gPiA+ICsJCVZhbHVlIHR5cGU6IDxw
cm9wLWVuY29kZWQtYXJyYXk+DQo+ID4gPiArCQlEZWZpbml0aW9uOiBFdmVudCBpbnRlcnJ1cHQg
b2YgZXh0ZXJuYWwgTURJTyBjb250cm9sbGVyLg0KPiA+ID4gKwkJMSBHYi9zIE1ESU8gYW5kIDEw
IEdiL3MgTURJTyBoYXMgb25lIGludGVycnVwdCByZXNwZWN0aXZlbHkuDQo+ID4gPiArDQo+ID4g
PiArLSBmc2wsZm1hbi1pbnRlcm5hbC1tZGlvDQo+ID4gPiArCQlVc2FnZTogcmVxdWlyZWQgZm9y
IGludGVybmFsIE1ESU8NCj4gPiA+ICsJCVZhbHVlIHR5cGU6IGJvb2xlYW4NCj4gPiA+ICsJCURl
ZmluaXRpb246IEZtYW4gaGFzIGludGVybmFsIE1ESU8gZm9yIGludGVybmFsIFBDUyhQaHlzaWNh
bA0KPiA+ID4gKwkJQ29kaW5nIFN1YmxheWVyKSBQSFlzIGFuZCBleHRlcm5hbCBNRElPIGZvciBl
eHRlcm5hbCBQSFlzLg0KPiA+ID4gKwkJVGhlIHNldHRpbmdzIGFuZCBwcm9ncmFtbWluZyByb3V0
aW5lcyBmb3IgaW50ZXJuYWwvZXh0ZXJuYWwNCj4gPiA+ICsJCU1ESU8gYXJlIGRpZmZlcmVudC4g
TXVzdCBiZSBpbmNsdWRlZCBmb3IgaW50ZXJuYWwgTURJTy4NCj4gPiA+ICsNCj4gPiA+ICtFWEFN
UExFDQo+ID4gPiArDQo+ID4gPiArRXhhbXBsZSBmb3IgRk1hbiB2MiBleHRlcm5hbCBNRElPOg0K
PiA+ID4gKw0KPiA+ID4gK21kaW9AZjEwMDAgew0KPiA+ID4gKwljb21wYXRpYmxlID0gImZzbCxm
bWFuLXhtZGlvIjsNCj4gPiA+ICsJcmVnID0gPDB4ZjEwMDAgMHgxMDAwPjsNCj4gPiA+ICsJYnVz
LWZyZXF1ZW5jeSA9IDwyMDAwMD47DQo+ID4gPiArfTsNCj4gPg0KPiA+IFNvIHRoZSBidXMgZnJl
cXVlbmN5IGlzIG9ubHkgMjAgS0h6PyAgT3IgaXMgdGhlIHVuaXQgc3VwcG9zZWQgdG8gYmUNCj4g
PiBzb21ldGhpbmcgb3RoZXIgdGhhbiBIej8NCj4gW1MuSF0gaXQncyBvbmx5IGFuIGV4YW1wbGUs
IGl0IGNvdWxkIGJlIGRpZmZlcmVudCBvbiByZWFsIFNvQ3MsIGJ1dCB0aGV5IGFsd2F5cw0KPiBs
b3dlciB0aGFuIHRoZSBzdGFuZGFyZCBvbmUsIFRoZSBzdGFuZGFyZCBvbmUgaXMgMi41TUh6LCBJ
IGhhdmUgdG8gdXNlIEh6IGZvciBpdC4NCj4gDQo+IFRoYW5rcyENCj4gU2hhb2h1aQ0K

^ permalink raw reply

* Re: [v2 PATCH 1/2] powerpc32: adds handling of _PAGE_RO
From: Scott Wood @ 2014-12-18 11:23 UTC (permalink / raw)
  To: leroy christophe; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <54927E1E.1030407@c-s.fr>

On Thu, 2014-12-18 at 08:11 +0100, leroy christophe wrote:
> Le 18/12/2014 03:14, Scott Wood a écrit :
> > On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote:
> >> Some powerpc like the 8xx don't have a RW bit in PTE bits but a RO (Read Only) bit.
> >> This patch implements the handling of a _PAGE_RO flag to be used in place of _PAGE_RW
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >>
> >> ---
> >> v2 is a complete rework compared to v1
> >>
> >>   arch/powerpc/include/asm/pgtable-ppc32.h | 11 ++++++-----
> >>   arch/powerpc/include/asm/pgtable.h       | 10 +++++++---
> >>   arch/powerpc/include/asm/pte-common.h    | 27 ++++++++++++++++++---------
> >>   arch/powerpc/mm/gup.c                    |  2 ++
> >>   arch/powerpc/mm/mem.c                    |  2 +-
> >>   arch/powerpc/mm/pgtable_32.c             | 24 ++++++++++++++++++++----
> >>   6 files changed, 54 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
> >> index 543bb8e..64ed9e1 100644
> >> --- a/arch/powerpc/include/asm/pgtable-ppc32.h
> >> +++ b/arch/powerpc/include/asm/pgtable-ppc32.h
> >> @@ -125,7 +125,7 @@ extern int icache_44x_need_flush;
> >>   #ifndef __ASSEMBLY__
> >>   
> >>   #define pte_clear(mm, addr, ptep) \
> >> -	do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
> >> +	do { pte_update(ptep, ~_PAGE_HASHPTE, _PAGE_RO); } while (0)
> > Is this really necessary?  It's already clearing the valid bit.
> >
> > Likewise in several other places that set or check for _PAGE_RO on pages
> > for which no access is permitted.
> >
> >> @@ -287,8 +287,9 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
> >>   static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
> >>   {
> >>   	unsigned long bits = pte_val(entry) &
> >> -		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
> >> -	pte_update(ptep, 0, bits);
> >> +		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_RO |
> >> +		 _PAGE_EXEC);
> >> +	pte_update(ptep, _PAGE_RO, bits);
> >>   }
> > You're unconditionally clearing _PAGE_RO, and apparently relying on the
> > undocumented behavior of pte_update() to clear "clr" before setting
> > "set".
> >
> > Instead I'd write this as:
> >
> > 	unsigned long set = pte_val(entry) &
> > 		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
> > 	unsigned long clr = pte_val(entry) & _PAGE_RO;
> Don't you mean ?
> 
> 	unsigned long clr = ~pte_val(entry) & _PAGE_RO;
> 
> Because, we want to clear _PAGE_RO when _PAGE_RO is not set in entry.

Yes, sorry.

-Scott

^ permalink raw reply

* RE: [RFC PATCH 0/2] powerpc: CR based local atomic operation implementation
From: David Laight @ 2014-12-18  9:52 UTC (permalink / raw)
  To: 'Rusty Russell', 'Madhavan Srinivasan',
	mpe@ellerman.id.au
  Cc: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, anton@samba.org
In-Reply-To: <87k31ptwty.fsf@rustcorp.com.au>

RnJvbTogUnVzdHkgUnVzc2VsbA0KPiBEYXZpZCBMYWlnaHQgPERhdmlkLkxhaWdodEBBQ1VMQUIu
Q09NPiB3cml0ZXM6DQo+ID4gRnJvbTogTWFkaGF2YW4gU3Jpbml2YXNhbiBbbWFpbHRvOm1hZGR5
QGxpbnV4LnZuZXQuaWJtLmNvbV0NCj4gPiAuLi4NCj4gPj4gPj4+IEkgYWxzbyB3b25kZXIgaWYg
aXQgaXMgcG9zc2libGUgdG8gaW5zcGVjdCB0aGUgaW50ZXJydXB0ZWQNCj4gPj4gPj4+IGNvZGUg
dG8gZGV0ZXJtaW5lIHRoZSBzdGFydC9lbmQgb2YgdGhlIFJBUyBibG9jay4NCj4gPj4gPj4+IChF
YXNpZXN0IGlmIHlvdSBhc3N1bWUgdGhhdCB0aGVyZSBpcyBhIHNpbmdsZSAnd3JpdGUnIGluc3Ry
dWN0aW9uDQo+ID4+ID4+PiBhcyB0aGUgbGFzdCBlbnRyeSBpbiB0aGUgYmxvY2suKQ0KPiA+PiA+
Pj4NCj4gPj4gPj4gU28gZWFjaCBsb2NhbF8qIGZ1bmN0aW9uIGFsc28gaGF2ZSBjb2RlIGluIHRo
ZSBfX2V4X3RhYmxlIHNlY3Rpb24uIElJVUMsDQo+ID4+ID4+IF9fZXhfdGFibGUgY29udGFpbnMg
dHdvIGFkZHJlc3MuIFNvIGlmIHRoZSByZXR1cm4gYWRkcmVzcyBmb3VuZCBpbiB0aGUNCj4gPj4g
Pj4gZmlyc3QgY29sdW1uIG9mIHRoZSBfZXhfdGFibGUsIHVzZSB0aGUgY29ycmVzcG9uZGluZyBh
ZGRyZXNzIGluIHRoZQ0KPiA+PiA+PiBzZWNvbmQgY29sdW1uIHRvIGNvbnRpbnVlIGZyb20uDQo+
ID4+ID4NCj4gPj4gPiBUaGF0IHJlYWxseSBkb2Vzbid0IHNjYWxlLg0KPiA+PiA+IEkgZG9uJ3Qg
a25vdyBob3cgbWFueSAxMDAwIGFkZHJlc3MgcGFpcnMgeW91IHRhYmxlIHdpbGwgaGF2ZSAoYW5k
IHRoZQ0KPiA+PiA+IG9uZXMgaW4gZWFjaCBsb2FkYWJsZSBtb2R1bGUpLCBidXQgdGhlIHNlYXJj
aCBpc24ndCBnb2luZyB0byBiZSBjaGVhcC4NCj4gPj4gPg0KPiA+PiA+IElmIHRoZXNlIHNlcXVl
bmNlcyBhcmUgcmVzdGFydGFibGUgdGhlbiB0aGV5IGNhbiBvbmx5IGhhdmUgb25lIHdyaXRlDQo+
ID4+ID4gdG8gbWVtb3J5Lg0KLi4uDQo+ID4+IDIpIHJlc3VsdGluZyBjb2RlIHdpdGggbG90IG9m
IGNvbmRpdGlvbiBhbmQgYnJhbmNoIChmb3Igb3Bjb2RlIGRlY29kZSkNCj4gPj4gd2lsbCBiZSBs
b3QgbWVzc3kgYW5kIG1heSBiZSBhbiBpc3N1ZSBpbmNhc2Ugb2YgbWFpbnRlbmFuY2UsDQo+ID4N
Cj4gPiBZb3UgZG9uJ3QgbmVlZCB0byBkZWNvZGUgdGhlIGluc3RydWN0aW9ucy4NCj4gPiBKdXN0
IGxvb2sgZm9yIHRoZSB0d28gc3BlY2lmaWMgaW5zdHJ1Y3Rpb25zIHVzZWQgYXMgbWFya2Vycy4N
Cj4gPiBUaGlzIGlzIG9ubHkgcmVhbGx5IHBvc3NpYmxlIHdpdGggZml4ZWQtc2l6ZSBpbnN0cnVj
dGlvbnMuDQo+ID4NCj4gPiBJdCBtaWdodCBhbHNvIGJlIHRoYXQgdGhlICdpbnRlcnJ1cHQgZW50
cnknIHBhdGggaXMgZWFzaWVyIHRvDQo+ID4gbW9kaWZ5IHRoYW4gdGhlICdpbnRlcnJ1cHQgZXhp
dCcgb25lIChmZXdlciBjb2RlIHBhdGhzKSBhbmQNCj4gPiB5b3UganVzdCBuZWVkIHRvIG1vZGlm
eSB0aGUgJ3BjJyBpbiB0aGUgc3RhY2sgZnJhbWUuDQo+ID4gWW91IGFyZSBvbmx5IGludGVyZXN0
ZWQgaW4gaW50ZXJydXB0cyBmcm9tIGtlcm5lbCBzcGFjZS4NCj4gDQo+IEl0J3MgYW4gb3Zlcm9w
dGltaXphdGlvbiBmb3IgY2FzZSB0aGF0IHN0YXRpc3RpY2FsbHkgbmV2ZXIgaGFwcGVucy4NCj4g
WW91IHdvbid0IGV2ZW4gYmUgYWJsZSB0byBtZWFzdXJlIHRoZSBkaWZmZXJlbmNlLg0KPiANCj4g
VGhlIHF1ZXN0aW9uIG9mIGJsb2F0IHJlbWFpbnMsIGJ1dCB0aGF0J3MgYWxzbyBlYXNpbHkgbWVh
c3VyZWQuICBJbg0KPiBwcmFjdGljZSwgSSdkIGd1ZXNzIGxlc3MgdGhhbiAxay4NCg0KSUlSQyB0
aGV5IHdlcmUgJ3N0YXRpYyBpbmxpbmUnIHNvIHRoZSB0YWJsZSBvZiBhZGRyZXNzZXMgaXMgZ2Vu
ZXJhdGVkDQpmb3IgZXZlcnkgdXNlIHNpdGUuDQooY29weWluL291dCBnZW5lcmF0ZXMgYSBzaW1p
bGFybHkgZW5vcm1vdXMgdGFibGUgb2YgYWRkcmVzc2VzIG9uIGFtZDY0KQ0KDQoNCklmIHRoZXkg
d2VyZSByZWFsIGZ1bmN0aW9ucyAoc28gb25seSBhcHBlYXJlZCBvbmNlKSBpdCB3b3VsZG4ndCBi
ZSBhcyBiYWQuDQpJbmRlZWQsIGluIHRoYXQgY2FzZSwgYnkgcHV0dGluZyBhbGwgc3VjaCBmdW5j
dGlvbnMgaW50byBhIHNlcGFyYXRlIGNvZGUNCnNlY3Rpb24gYSBzaW1wbGUgJ3dpbmRvdyB0ZXN0
JyBjYW4gYmUgZG9uZSBvbiB0aGUgcmV0dXJuIGFkZHJlc3MgaW5zdGVhZA0Kb2YgcmVzZXJ2aW5n
IG9uZSBvZiB0aGUgQ1IgYml0cy4NCg0KWW91IGFsc28gb25seSBuZWVkIHRvIHNhdmUgdGhlIHN0
YXJ0IGFuZCBlbmQgb2YgZWFjaCBibG9jaywgbm90IHRoZQ0KcmVzdGFydCBhZGRyZXNzIGZvciBl
dmVyeSBpbnN0cnVjdGlvbiB3aXRoaW4gdGhlIGJsb2NrLg0KDQoJRGF2aWQNCg0K

^ permalink raw reply

* Re: [v2 PATCH 1/2] powerpc32: adds handling of _PAGE_RO
From: leroy christophe @ 2014-12-18  7:11 UTC (permalink / raw)
  To: Scott Wood; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1418868868.5581.121.camel@freescale.com>


Le 18/12/2014 03:14, Scott Wood a écrit :
> On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote:
>> Some powerpc like the 8xx don't have a RW bit in PTE bits but a RO (Read Only) bit.
>> This patch implements the handling of a _PAGE_RO flag to be used in place of _PAGE_RW
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> ---
>> v2 is a complete rework compared to v1
>>
>>   arch/powerpc/include/asm/pgtable-ppc32.h | 11 ++++++-----
>>   arch/powerpc/include/asm/pgtable.h       | 10 +++++++---
>>   arch/powerpc/include/asm/pte-common.h    | 27 ++++++++++++++++++---------
>>   arch/powerpc/mm/gup.c                    |  2 ++
>>   arch/powerpc/mm/mem.c                    |  2 +-
>>   arch/powerpc/mm/pgtable_32.c             | 24 ++++++++++++++++++++----
>>   6 files changed, 54 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pgtable-ppc32.h b/arch/powerpc/include/asm/pgtable-ppc32.h
>> index 543bb8e..64ed9e1 100644
>> --- a/arch/powerpc/include/asm/pgtable-ppc32.h
>> +++ b/arch/powerpc/include/asm/pgtable-ppc32.h
>> @@ -125,7 +125,7 @@ extern int icache_44x_need_flush;
>>   #ifndef __ASSEMBLY__
>>   
>>   #define pte_clear(mm, addr, ptep) \
>> -	do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
>> +	do { pte_update(ptep, ~_PAGE_HASHPTE, _PAGE_RO); } while (0)
> Is this really necessary?  It's already clearing the valid bit.
>
> Likewise in several other places that set or check for _PAGE_RO on pages
> for which no access is permitted.
>
>> @@ -287,8 +287,9 @@ static inline void huge_ptep_set_wrprotect(struct mm_struct *mm,
>>   static inline void __ptep_set_access_flags(pte_t *ptep, pte_t entry)
>>   {
>>   	unsigned long bits = pte_val(entry) &
>> -		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
>> -	pte_update(ptep, 0, bits);
>> +		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_RO |
>> +		 _PAGE_EXEC);
>> +	pte_update(ptep, _PAGE_RO, bits);
>>   }
> You're unconditionally clearing _PAGE_RO, and apparently relying on the
> undocumented behavior of pte_update() to clear "clr" before setting
> "set".
>
> Instead I'd write this as:
>
> 	unsigned long set = pte_val(entry) &
> 		(_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
> 	unsigned long clr = pte_val(entry) & _PAGE_RO;
Don't you mean ?

	unsigned long clr = ~pte_val(entry) & _PAGE_RO;

Because, we want to clear _PAGE_RO when _PAGE_RO is not set in entry.

Christophe

>
> 	pte_update(ptep, clr, set);
>
> -Scott
>

^ permalink raw reply

* Re: [v2 PATCH 2/2] powerpc/8xx: use _PAGE_RO instead of _PAGE_RW
From: leroy christophe @ 2014-12-18  7:05 UTC (permalink / raw)
  To: Scott Wood; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1418869363.5581.123.camel@freescale.com>


Le 18/12/2014 03:22, Scott Wood a écrit :
> On Wed, 2014-12-17 at 10:14 +0100, Christophe Leroy wrote:
>> On powerpc 8xx, in TLB entries, 0x400 bit is set to 1 for read-only pages
>> and is set to 0 for RW pages. So we should use _PAGE_RO instead of _PAGE_RW
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> ---
>> v2 is a complete rework compared to v1
>>
>>   arch/powerpc/include/asm/pte-8xx.h | 7 +++----
>>   arch/powerpc/kernel/head_8xx.S     | 3 ---
>>   2 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
>> index daa4616..4f5583e 100644
>> --- a/arch/powerpc/include/asm/pte-8xx.h
>> +++ b/arch/powerpc/include/asm/pte-8xx.h
>> @@ -46,7 +46,7 @@
>>    * require a TLB exception handler change.  It is assumed unused bits
>>    * are always zero.
>>    */
>> -#define _PAGE_RW	0x0400	/* lsb PP bits, inverted in HW */
>> +#define _PAGE_RO	0x0400	/* lsb PP bits */
> It looks like pte_update() needs to be updated to match.
>
>>   #define _PAGE_USER	0x0800	/* msb PP bits */
>>   /* set when neither _PAGE_USER nor _PAGE_RW are set */
> Also update this comment.
>
>>   #define _PAGE_KNLRO	0x0200
>> @@ -62,9 +62,8 @@
>>   #define PTE_ATOMIC_UPDATES	1
>>   
>>   /* We need to add _PAGE_SHARED to kernel pages */
>> -#define _PAGE_KERNEL_RO	(_PAGE_SHARED | _PAGE_KNLRO)
>> -#define _PAGE_KERNEL_ROX	(_PAGE_EXEC | _PAGE_KNLRO)
>> -#define _PAGE_KERNEL_RW	(_PAGE_DIRTY | _PAGE_RW | _PAGE_HWWRITE)
>> +#define _PAGE_KERNEL_RO	(_PAGE_SHARED | _PAGE_RO | _PAGE_KNLRO)
>> +#define _PAGE_KERNEL_ROX	(_PAGE_EXEC | _PAGE_RO | _PAGE_KNLRO)
>>   
>>   #endif /* __KERNEL__ */
>>   #endif /*  _ASM_POWERPC_PTE_8xx_H */
> Where did _PAGE_KERNEL_RW go?
>
>
It is already defined like this in pte-common.h:

#ifndef _PAGE_KERNEL_RW
#define _PAGE_KERNEL_RW        (_PAGE_DIRTY | _PAGE_RW | _PAGE_HWWRITE)
#endif


Christophe

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Anton Blanchard @ 2014-12-18  6:25 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Alan Modra, Alexander Graf, ulrich.weigand, paulus, Scott Wood,
	linuxppc-dev
In-Reply-To: <1418879514.13333.2.camel@ellerman.id.au>

On Thu, 18 Dec 2014 16:11:54 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
> > On 31.10.14 04:47, Anton Blanchard wrote:
> > > LLVM doesn't support local named register variables and is
> > > unlikely to. current_thread_info is using one, fix it by moving
> > > it out and calling it __current_r1().
> > > 
> > > I gave it a bit of an obscure name because we don't want anyone
> > > else using it - they should use current_stack_pointer(). This
> > > specific case is performance critical and we can't afford to call
> > > a function to get it. Furthermore it isn't important to know
> > > exactly where in the stack we are since we mask the lower bits.
> > > 
> > > Signed-off-by: Anton Blanchard <anton@samba.org>
> > 
> > Git bisect managed to point me to this commit as the offender for
> > OOPSes on e5500 and e6500 (and maybe the G4 as well, not sure).
> > 
> > Doing a git revert of this commit on top of linus/master makes
> > things work fine for me again.
> > 
> > 
> > Alex
> > 
> > Oops: Kernel access of bad area, sig: 11 [#2]
> > SMP NR_CPUS=16 CoreNet Generic
> > Modules linked in:
> > CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G      D
> > 3.18.0-09423-g988adfd #1
> > Workqueue: rpciod .rpc_async_schedule
> > task: c0000001f6397500 ti: c0000001f6638000 task.ti:
> > c0000001f6638000 NIP: c0000000004817a4 LR: c0000000004817a4 CTR:
> > 0000000000000000 REGS: c0000001f663b0e0 TRAP: 0300   Tainted:
> > G      D (3.18.0-09423-g988adfd)
> > MSR: 0000000080029000 <CE,EE,ME>  CR: 24ad2e42  XER: 00000000
> > DEAR: 202031303438355f ESR: 0000000000000000 SOFTE: 1
>       = r9 + 40
> 
> > GPR00: c0000000004817a4 c0000001f663b360 c000000000988028
> > 000000007f24333d GPR04: 5ff5738c1f2ebfb1 0000000000000000
> > 0000000000000000 00000000000008f8 GPR08: c000000000480ae8
> > 2020313034383537 36204b4220617320 6469726563740a31 GPR12:
> > 3937302d30312d30 c00000000fff8780 c00000000007f988 c0000001f64c1600
> 
> GPRs 9-12 say: " 1048576 KB as direct\n1970-01-0"
> 
> Which is rarely a good sign :)
> 
> Looks like it might be part of your dmesg from setup_page_sizes().
> 
> > GPR16: 0000000000000000 0000000000000000 0000000000000000
> > 00000000000005dc GPR20: c0000000009b8028 c00000007e034200
> > 0000000000000548 c000000000000000 GPR24: c0000001f663b4b0
> > 00000000b225831e 0000000000000000 0000000000000080 GPR28:
> > 0000000000000548 00000000000008f8 0000000000000548 0000000000000094
> > NIP [c0000000004817a4] .__skb_checksum+0x194/0x378 LR
> > [c0000000004817a4] .__skb_checksum+0x194/0x378 Call Trace:
> > [c0000001f663b360] [c0000000004817a4] .__skb_checksum+0x194/0x378
> > (unreliable)
> > [c0000001f663b440] [c0000000004819b4] .skb_checksum+0x2c/0x3c
> > [c0000001f663b4c0] [c0000000004fd0a8] .udp4_hwcsum+0xa8/0x16c
> > [c0000001f663b560] [c0000000004fd440] .udp_send_skb+0x2d4/0x370
> > [c0000001f663b600]
> > [c0000000004fd51c] .udp_push_pending_frames+0x40/0x94
> > [c0000001f663b680] [c0000000004fec08] .udp_sendpage+0x150/0x1b4
> > [c0000001f663b770] [c00000000050ae54] .inet_sendpage+0xa0/0x120
> > [c0000001f663b810] [c00000000059c8cc] .xs_sendpages+0x2d0/0x30c
> > [c0000001f663b8d0]
> > [c00000000059cae4] .xs_udp_send_request+0x58/0x120
> > [c0000001f663b970] [c000000000598f04] .xprt_transmit+0x80/0x36c
> > [c0000001f663ba20] [c0000000005942d8] .call_transmit+0x19c/0x254
> > [c0000001f663bab0] [c00000000059ff64] .__rpc_execute+0xbc/0x3c0
> > [c0000001f663bb90] [c0000000000797f8] .process_one_work+0x1c0/0x474
> > [c0000001f663bc40] [c00000000007a518] .worker_thread+0x17c/0x54c
> > [c0000001f663bd30] [c00000000007fa8c] .kthread+0x104/0x124
> > [c0000001f663be30]
> > [c000000000000884] .ret_from_kernel_thread+0x58/0xd4 Instruction
> > dump: 7d1f3a14 7c6a1850 e9580000 7fbd4050 786334e4 e90a0000
> > 7c63ba14 f8490028 7c63ea14 7d0903a6 e84a0008 4e800421 <e8490028>
> > 7c641b78 78270464 e9580008
> 
> Which is:
> 
> add      r8, r31, r7
> subf     r3, r10, r3
> ld      r10, 0(r24)
> subf    r29, r29, r8
> rldicr   r3, r3, 6, 51
> ld       r8, 0(r10)
> add      r3, r3, r23
> std      r2, 40(r9)
> add      r3, r3, r29
> mtctr    r8
> ld       r2, 8(r10)
> bctrl
> ld       r2, 40(r9)	<---
> mr       r4, r3
> rldicr   r7, r1, 0, 49
> ld      r10, 8(r24)
> 
> 
> Which looks a bit odd. I'd expect us to be saving/restoring r2 to the
> stack, though maybe r9 was pointing at the stack?

Nice catch! This looks like a compiler bug.

> Looking at your vmlinux.broken I don't see the same code gen.

For whatever reason we ended up with r10 this time:

7c 2a 0b 78     mr      r10,r1
...
f8 4a 00 28     std     r2,40(r10)
7c 63 ba 14     add     r3,r3,r23
7c e9 03 a6     mtctr   r7
7c 63 ea 14     add     r3,r3,r29
38 a0 00 00     li      r5,0
e8 48 00 08     ld      r2,8(r8)
4e 80 04 21     bctrl
e8 4a 00 28     ld      r2,40(r10)

The indirect function call is allowed to clobber r10, gcc is doing
something very wrong here.

Anton

^ permalink raw reply

* Re: [PATCH 1/3] powerpc: Don't use local named register variable in current_thread_info
From: Michael Ellerman @ 2014-12-18  5:11 UTC (permalink / raw)
  To: Alexander Graf
  Cc: ulrich.weigand, paulus, Anton Blanchard, Scott Wood, linuxppc-dev
In-Reply-To: <5490D981.7040905@suse.de>

On Wed, 2014-12-17 at 02:16 +0100, Alexander Graf wrote:
> On 31.10.14 04:47, Anton Blanchard wrote:
> > LLVM doesn't support local named register variables and is unlikely
> > to. current_thread_info is using one, fix it by moving it out and
> > calling it __current_r1().
> > 
> > I gave it a bit of an obscure name because we don't want anyone else
> > using it - they should use current_stack_pointer(). This specific
> > case is performance critical and we can't afford to call a function
> > to get it. Furthermore it isn't important to know exactly where in
> > the stack we are since we mask the lower bits.
> > 
> > Signed-off-by: Anton Blanchard <anton@samba.org>
> 
> Git bisect managed to point me to this commit as the offender for OOPSes
> on e5500 and e6500 (and maybe the G4 as well, not sure).
> 
> Doing a git revert of this commit on top of linus/master makes things
> work fine for me again.
> 
> 
> Alex
> 
> Oops: Kernel access of bad area, sig: 11 [#2]
> SMP NR_CPUS=16 CoreNet Generic
> Modules linked in:
> CPU: 1 PID: 339 Comm: kworker/1:1 Tainted: G      D
> 3.18.0-09423-g988adfd #1
> Workqueue: rpciod .rpc_async_schedule
> task: c0000001f6397500 ti: c0000001f6638000 task.ti: c0000001f6638000
> NIP: c0000000004817a4 LR: c0000000004817a4 CTR: 0000000000000000
> REGS: c0000001f663b0e0 TRAP: 0300   Tainted: G      D
> (3.18.0-09423-g988adfd)
> MSR: 0000000080029000 <CE,EE,ME>  CR: 24ad2e42  XER: 00000000
> DEAR: 202031303438355f ESR: 0000000000000000 SOFTE: 1
      = r9 + 40

> GPR00: c0000000004817a4 c0000001f663b360 c000000000988028 000000007f24333d
> GPR04: 5ff5738c1f2ebfb1 0000000000000000 0000000000000000 00000000000008f8
> GPR08: c000000000480ae8 2020313034383537 36204b4220617320 6469726563740a31
> GPR12: 3937302d30312d30 c00000000fff8780 c00000000007f988 c0000001f64c1600

GPRs 9-12 say: " 1048576 KB as direct\n1970-01-0"

Which is rarely a good sign :)

Looks like it might be part of your dmesg from setup_page_sizes().

> GPR16: 0000000000000000 0000000000000000 0000000000000000 00000000000005dc
> GPR20: c0000000009b8028 c00000007e034200 0000000000000548 c000000000000000
> GPR24: c0000001f663b4b0 00000000b225831e 0000000000000000 0000000000000080
> GPR28: 0000000000000548 00000000000008f8 0000000000000548 0000000000000094
> NIP [c0000000004817a4] .__skb_checksum+0x194/0x378
> LR [c0000000004817a4] .__skb_checksum+0x194/0x378
> Call Trace:
> [c0000001f663b360] [c0000000004817a4] .__skb_checksum+0x194/0x378
> (unreliable)
> [c0000001f663b440] [c0000000004819b4] .skb_checksum+0x2c/0x3c
> [c0000001f663b4c0] [c0000000004fd0a8] .udp4_hwcsum+0xa8/0x16c
> [c0000001f663b560] [c0000000004fd440] .udp_send_skb+0x2d4/0x370
> [c0000001f663b600] [c0000000004fd51c] .udp_push_pending_frames+0x40/0x94
> [c0000001f663b680] [c0000000004fec08] .udp_sendpage+0x150/0x1b4
> [c0000001f663b770] [c00000000050ae54] .inet_sendpage+0xa0/0x120
> [c0000001f663b810] [c00000000059c8cc] .xs_sendpages+0x2d0/0x30c
> [c0000001f663b8d0] [c00000000059cae4] .xs_udp_send_request+0x58/0x120
> [c0000001f663b970] [c000000000598f04] .xprt_transmit+0x80/0x36c
> [c0000001f663ba20] [c0000000005942d8] .call_transmit+0x19c/0x254
> [c0000001f663bab0] [c00000000059ff64] .__rpc_execute+0xbc/0x3c0
> [c0000001f663bb90] [c0000000000797f8] .process_one_work+0x1c0/0x474
> [c0000001f663bc40] [c00000000007a518] .worker_thread+0x17c/0x54c
> [c0000001f663bd30] [c00000000007fa8c] .kthread+0x104/0x124
> [c0000001f663be30] [c000000000000884] .ret_from_kernel_thread+0x58/0xd4
> Instruction dump:
> 7d1f3a14 7c6a1850 e9580000 7fbd4050 786334e4 e90a0000 7c63ba14 f8490028
> 7c63ea14 7d0903a6 e84a0008 4e800421 <e8490028> 7c641b78 78270464 e9580008

Which is:

add      r8, r31, r7
subf     r3, r10, r3
ld      r10, 0(r24)
subf    r29, r29, r8
rldicr   r3, r3, 6, 51
ld       r8, 0(r10)
add      r3, r3, r23
std      r2, 40(r9)
add      r3, r3, r29
mtctr    r8
ld       r2, 8(r10)
bctrl
ld       r2, 40(r9)	<---
mr       r4, r3
rldicr   r7, r1, 0, 49
ld      r10, 8(r24)


Which looks a bit odd. I'd expect us to be saving/restoring r2 to the stack,
though maybe r9 was pointing at the stack?

Looking at your vmlinux.broken I don't see the same code gen.

Can you get an oops from a kernel and upload the exact binary? Or just post us
the full code dump of __skb_checksum() (or wherever it oopses).

cheers

^ permalink raw reply

* Re: [1/5] powerpc: Fix handling of DSCR related facility unavailable exception
From: Anton Blanchard @ 2014-12-18  5:10 UTC (permalink / raw)
  To: Anshuman Khandual; +Cc: mikey, linuxppc-dev
In-Reply-To: <5486F607.6010706@linux.vnet.ibm.com>

Hi Anshuman,

> Yeah I wanted to convert all these tests which are related to DSCR
> into individual self tests for powerpc. All these test cases have
> Anton Blanchard and IBM's copyright on it but they are licensed with
> GPL V2. Not sure whether Anton needs to okay this before I can modify
> them for self tests, put his (Anton's) signed-off-by and then post it
> in the mailing list.
> 
> (1) http://ozlabs.org/~anton/junkcode/dscr_default_test.c
> (2) http://ozlabs.org/~anton/junkcode/dscr_explicit_test.c
> (3) http://ozlabs.org/~anton/junkcode/dscr_inherit_exec_test.c
> (4) http://ozlabs.org/~anton/junkcode/dscr_inherit_test.c
> (5) http://ozlabs.org/~anton/junkcode/user_dscr_test.c

I'm happy for the testcases to be modified and merged into the kernel!

When you submit them I'll give them a once over and attach my
Signed-off-by.

Thanks,
Anton

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox