Re: [PATCH] powerpc/kernel: improve FP and vector registers restoration

[Michael Ellerman]

Breno Leitao  writes:

> Currently tsk->thread->load_vec and load_fp are not initialized during a
> task creation, which set garbage to these variables (non-zero value).
>
> These variables will be checked later at restore_math() to validate if the
> FP and vectors are being utilized. Since these values might be non-zero,
> the restore_math() will continue to save the FP and vectors even if they
> were never utilized before the userspace application. load_fp and load_vec
> counters will then overflow and the FP and Altivec will be finally
> disabled, but before that condition is reached (counter overflow) several
> context switches restored FP and vector registers without need, causing a
> performance degradation.
>
> Signed-off-by: Breno Leitao 
> Signed-off-by: Gustavo Romero 

Thanks, I tweaked the wording a little and added:

Fixes: 70fe3d980f5f ("powerpc: Restore FPU/VEC/VSX if previously used")
Cc: sta...@vger.kernel.org # v4.6+

cheers

Re: [PATCH v1 7/8] powerpc/Kconfig: Enable STRICT_KERNEL_RWX

[Michael Ellerman]

Balbir Singh  writes:
> On Mon, May 29, 2017 at 6:00 PM, Christophe LEROY
>  wrote:
>> Le 25/05/2017 à 18:45, kbuild test robot a écrit :
>>> All errors (new ones prefixed by >>):
>>>
> kernel//power/snapshot.c:40:28: fatal error: asm/set_memory.h: No such
> file or directory
>>>
>>>  #include 
>>
>>
>> Looks like it is linked to commit 50327ddfb ("kernel/power/snapshot.c: use
>> set_memory.h header").
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=50327ddfb
>>
>> I believe that inclusion should be conditional to
>> CONFIG_ARCH_HAS_SET_MEMORY, which is not set by powerpc arch, just like in
>> include/linux/filter.c
>>
> Agreed, it needs to be fixed in kernel/power/sanpshot.c

It should be fixed in asm-generic/set_memory.h, there should be no need
to guard an include of a generic header.

cheers

Re: [PATCH v2] cxl: Avoid double free_irq() for psl,slice interrupts

[Andrew Donnellan]

On 03/06/17 02:56, Vaibhav Jain wrote:

During an eeh call to cxl_remove can result in double free_irq of
psl,slice interrupts. This can happen if perst_reloads_same_image == 1
and call to cxl_configure_adapter() fails during slot_reset
callback. In such a case we see a kernel oops with following back-trace:

Oops: Kernel access of bad area, sig: 11 [#1]
Call Trace:
  free_irq+0x88/0xd0 (unreliable)
  cxl_unmap_irq+0x20/0x40 [cxl]
  cxl_native_release_psl_irq+0x78/0xd8 [cxl]
  pci_deconfigure_afu+0xac/0x110 [cxl]
  cxl_remove+0x104/0x210 [cxl]
  pci_device_remove+0x6c/0x110
  device_release_driver_internal+0x204/0x2e0
  pci_stop_bus_device+0xa0/0xd0
  pci_stop_and_remove_bus_device+0x28/0x40
  pci_hp_remove_devices+0xb0/0x150
  pci_hp_remove_devices+0x68/0x150
  eeh_handle_normal_event+0x140/0x580
  eeh_handle_event+0x174/0x360
  eeh_event_handler+0x1e8/0x1f0

This patch fixes the issue of double free_irq by checking that
variables that hold the virqs (err_hwirq, serr_hwirq, psl_virq) are
not '0' before un-mapping and resetting these variables to '0' when
they are un-mapped.

Cc: sta...@vger.kernel.org
Signed-off-by: Vaibhav Jain 


v2 change looks good to me

Reviewed-by: Andrew Donnellan 

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH] cxl: Fix error path on bad ioctl

[Andrew Donnellan]

On 03/06/17 02:15, Frederic Barrat wrote:

Fix error path if we can't copy user structure on
CXL_IOCTL_START_WORK ioctl.

Signed-off-by: Frederic Barrat 
Cc: sta...@vger.kernel.org


Reviewed-by: Andrew Donnellan 

--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

[PATCH] net/phy: Make phy_ethtool_ksettings_get return void

[Yuval Shaia]

Make return value void since function never return meaningfull value

Signed-off-by: Yuval Shaia 
---
 drivers/net/ethernet/broadcom/b44.c|  3 ++-
 drivers/net/ethernet/broadcom/bcm63xx_enet.c   |  3 ++-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  4 +++-
 drivers/net/ethernet/broadcom/tg3.c|  4 +++-
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c |  6 ++
 drivers/net/ethernet/freescale/ucc_geth_ethtool.c  |  4 +++-
 drivers/net/ethernet/marvell/mv643xx_eth.c |  5 ++---
 drivers/net/ethernet/ti/cpsw.c |  9 +
 drivers/net/phy/phy.c  | 10 +-
 drivers/net/usb/lan78xx.c  |  2 +-
 include/linux/phy.h|  4 ++--
 net/dsa/slave.c|  9 +
 12 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c 
b/drivers/net/ethernet/broadcom/b44.c
index 5b95bb4..9873d2d 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -1836,7 +1836,8 @@ static int b44_get_link_ksettings(struct net_device *dev,
 
if (bp->flags & B44_FLAG_EXTERNAL_PHY) {
BUG_ON(!dev->phydev);
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c 
b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 50d88d3..34ebb40 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1453,7 +1453,8 @@ static int bcm_enet_get_link_ksettings(struct net_device 
*dev,
if (priv->has_phy) {
if (!dev->phydev)
return -ENODEV;
-   return phy_ethtool_ksettings_get(dev->phydev, cmd);
+   phy_ethtool_ksettings_get(dev->phydev, cmd);
+   return 0;
} else {
cmd->base.autoneg = 0;
cmd->base.speed = (priv->force_speed_100) ?
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c 
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index a205a9f..daca1c9 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -477,7 +477,9 @@ static int bcmgenet_get_link_ksettings(struct net_device 
*dev,
if (!priv->phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(priv->phydev, cmd);
+   phy_ethtool_ksettings_get(priv->phydev, cmd);
+
+   return 0;
 }
 
 static int bcmgenet_set_link_ksettings(struct net_device *dev,
diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c
index 537d571..d600c41 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12097,7 +12097,9 @@ static int tg3_get_link_ksettings(struct net_device 
*dev,
if (!(tp->phy_flags & TG3_PHYFLG_IS_CONNECTED))
return -EAGAIN;
phydev = mdiobus_get_phy(tp->mdio_bus, tp->phy_addr);
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
}
 
supported = (SUPPORTED_Autoneg);
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 15571e2..aad825088 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -75,16 +75,14 @@ static char dpaa_stats_global[][ETH_GSTRING_LEN] = {
 static int dpaa_get_link_ksettings(struct net_device *net_dev,
   struct ethtool_link_ksettings *cmd)
 {
-   int err;
-
if (!net_dev->phydev) {
netdev_dbg(net_dev, "phy device not initialized\n");
return 0;
}
 
-   err = phy_ethtool_ksettings_get(net_dev->phydev, cmd);
+   phy_ethtool_ksettings_get(net_dev->phydev, cmd);
 
-   return err;
+   return 0;
 }
 
 static int dpaa_set_link_ksettings(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c 
b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
index b642990..4df282e 100644
--- a/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
+++ b/drivers/net/ethernet/freescale/ucc_geth_ethtool.c
@@ -113,7 +113,9 @@ uec_get_ksettings(struct net_device *netdev, struct 
ethtool_link_ksettings *cmd)
if (!phydev)
return -ENODEV;
 
-   return phy_ethtool_ksettings_get(phydev, cmd);
+   phy_ethtool_ksettings_get(phydev, cmd);
+
+   return 0;
 }
 
 static int
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c 
b/drivers/net/ethernet/marvell/mv643xx_eth.c
index 

Re: [PATCH v2 5/9] powerpc/vmlinux.lds: Align __init_begin to 16M

[Balbir Singh]

On Sun, Jun 4, 2017 at 2:22 PM, Nicholas Piggin  wrote:
> On Sat,  3 Jun 2017 17:18:39 +1000
> Balbir Singh  wrote:
>
>> For CONFIG_STRICT_KERNEL_RWX align __init_begin to 16M.
>> We use 16M since its the larger of 2M on radix and 16M
>> on hash for our linear mapping. The plan is to have
>> .text, .rodata and everything upto __init_begin marked
>> as RX. Note we still have executable read only data.
>> We could further align read only data to another 16M
>> boundary, but then the linker starts using stubs and
>> that breaks our assembler code in head_64.S
>
> Is this still the case with powerpc next?
>

Sorry, no, I was on linux-next for testing, but I think your linker
stub patches went in very recently. I'll rebase on top and test,
but I am not sure if I want two 16M alignments and bloat the size
of vmlinux. But I'll double check what happens

Balbir Singh.

[PATCH] powerpc/pseries: fix spelling mistake: "Attemping" -> "Attempting"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in pr_debug message

Signed-off-by: Colin Ian King 
---
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 7bc0e91f8715..6afd1efd3633 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -554,7 +554,7 @@ static ssize_t dlpar_cpu_remove(struct device_node *dn, u32 
drc_index)
 {
int rc;
 
-   pr_debug("Attemping to remove CPU %s, drc index: %x\n",
+   pr_debug("Attempting to remove CPU %s, drc index: %x\n",
 dn->name, drc_index);
 
rc = dlpar_offline_cpu(dn);
-- 
2.11.0

Re: [PATCH] net/phy: Make phy_ethtool_ksettings_get return void

[kbuild test robot]

Hi Yuval,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Yuval-Shaia/net-phy-Make-phy_ethtool_ksettings_get-return-void/20170605-012813
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   drivers/net//ethernet/renesas/sh_eth.c: In function 
'sh_eth_get_link_ksettings':
>> drivers/net//ethernet/renesas/sh_eth.c:1924:6: error: void value not ignored 
>> as it ought to be
 ret = phy_ethtool_ksettings_get(ndev->phydev, cmd);
 ^
--
   drivers/net//ethernet/renesas/ravb_main.c: In function 
'ravb_get_link_ksettings':
>> drivers/net//ethernet/renesas/ravb_main.c:1084:9: error: void value not 
>> ignored as it ought to be
  error = phy_ethtool_ksettings_get(ndev->phydev, cmd);
^
--
   drivers/net//ethernet/apm/xgene/xgene_enet_ethtool.c: In function 
'xgene_get_link_ksettings':
>> drivers/net//ethernet/apm/xgene/xgene_enet_ethtool.c:134:10: error: void 
>> value not ignored as it ought to be
  return phy_ethtool_ksettings_get(phydev, cmd);
 ^~
   drivers/net//ethernet/apm/xgene/xgene_enet_ethtool.c:140:11: error: void 
value not ignored as it ought to be
   return phy_ethtool_ksettings_get(phydev, cmd);
  ^~
--
   drivers/net//ethernet/apm/xgene-v2/ethtool.c: In function 
'xge_get_link_ksettings':
>> drivers/net//ethernet/apm/xgene-v2/ethtool.c:160:9: error: void value not 
>> ignored as it ought to be
 return phy_ethtool_ksettings_get(phydev, cmd);
^~
>> drivers/net//ethernet/apm/xgene-v2/ethtool.c:161:1: warning: control reaches 
>> end of non-void function [-Wreturn-type]
}
^
--
   drivers/net//ethernet/stmicro/stmmac/stmmac_ethtool.c: In function 
'stmmac_ethtool_get_link_ksettings':
>> drivers/net//ethernet/stmicro/stmmac/stmmac_ethtool.c:367:5: error: void 
>> value not ignored as it ought to be
 rc = phy_ethtool_ksettings_get(phy, cmd);
^

vim +1924 drivers/net//ethernet/renesas/sh_eth.c

9fd0375ad drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10  
1908  phy_start(ndev->phydev);
86a74ff21 drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2008-06-09  
1909  
86a74ff21 drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2008-06-09  
1910  return 0;
86a74ff21 drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2008-06-09  
1911  }
86a74ff21 drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2008-06-09  
1912  
f08aff444 drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10  
1913  static int sh_eth_get_link_ksettings(struct net_device *ndev,
f08aff444 drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10  
1914   struct ethtool_link_ksettings *cmd)
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1915  {
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1916  struct sh_eth_private *mdp = netdev_priv(ndev);
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1917  unsigned long flags;
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1918  int ret;
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1919  
9fd0375ad drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10  
1920  if (!ndev->phydev)
4f9dce230 drivers/net/ethernet/renesas/sh_eth.c Ben Hutchings 2015-01-16  
1921  return -ENODEV;
4f9dce230 drivers/net/ethernet/renesas/sh_eth.c Ben Hutchings 2015-01-16  
1922  
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1923  spin_lock_irqsave(>lock, flags);
f08aff444 drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10 
@1924  ret = phy_ethtool_ksettings_get(ndev->phydev, cmd);
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1925  spin_unlock_irqrestore(>lock, flags);
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1926  
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1927  return ret;
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1928  }
dc19e4e5e drivers/net/sh_eth.c  Nobuhiro Iwamatsu 2011-02-15  
1929  
f08aff444 drivers/net/ethernet/renesas/sh_eth.c Philippe Reynes   2016-08-10  
1930  static int 

Re: [PATCH] powerpc/kernel: improve FP and vector registers restoration

[Breno Leitao]

On Sun, Jun 04, 2017 at 11:38:14AM +1000, Anton Blanchard wrote:
> On Sat, 3 Jun 2017 19:42:14 -0300
> Breno Leitao  wrote:
> 
> > Hi Anton,
> > 
> > On Sat, Jun 03, 2017 at 08:04:11AM +1000, Anton Blanchard wrote:
> > > Hi Breno,
> > >   
> > > > Currently tsk->thread->load_vec and load_fp are not initialized
> > > > during a task creation, which set garbage to these variables
> > > > (non-zero value).  
> > > 
> > > Nice catch! It seems like we should zero load_tm too though?  
> > 
> > Yes, it seems we need to zero load_tm also, since it does not seem to
> > be zeroed anywhere else.
> > 
> > But I did some tests, and load_tm is always zero after start_thread()
> > is being called.
> > 
> > In fact, start_thread() is being called and pt_regs->load_tm is
> > already zero since the function start.
> > 
> > I also wrote a SystemTap script[1] to investigate it better, and I've
> > never seen a single load_tm != 0 in a my machine. I tested on both
> > POWER8 bare metal and KVM guests. (load_vec and load_fp happened to
> > have garbage all the time)
> > 
> > Any idea if this is just occasional event, or, if there is someone
> > zeroing it in an obscure code?
> 
> Quite likely no one uses TM :) Try:

In fact, I had tested with TM[1] and haven't seen any issue, but I was not
calling a nested application (through execve() syscall). Somehow if I
call  "$ ./tm_application ; /bin/true", I do not see a non-zero load_tm
in the new task->thread.

On the other side, I see the corruption with your test case, mainly if I
sleep after 'tbegin.' and before execlp(), giving a chance to have
load_tm incremented, and this value is being inherited in the new
task->thread.

This is obviously wrong, I will send a patch to have it fixed.

Thanks for the guidance!

[1] https://github.com/leitao/htm_torture

[PATCH] selftests/powerpc: context_switch use private futexes with threads

[Nicholas Piggin]

This reduces overhead of mutex locking and increases context switch
rate significantly (which helps to measure and profile the context
switch path).

Signed-off-by: Nicholas Piggin 
---
 .../selftests/powerpc/benchmarks/context_switch.c  | 53 ++
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/powerpc/benchmarks/context_switch.c 
b/tools/testing/selftests/powerpc/benchmarks/context_switch.c
index 778f5fbfd784..f4241339edd2 100644
--- a/tools/testing/selftests/powerpc/benchmarks/context_switch.c
+++ b/tools/testing/selftests/powerpc/benchmarks/context_switch.c
@@ -258,9 +258,14 @@ static unsigned long xchg(unsigned long *p, unsigned long 
val)
return __atomic_exchange_n(p, val, __ATOMIC_SEQ_CST);
 }
 
+static int processes;
+
 static int mutex_lock(unsigned long *m)
 {
int c;
+   int flags = FUTEX_WAIT;
+   if (!processes)
+   flags |= FUTEX_PRIVATE_FLAG;
 
c = cmpxchg(m, 0, 1);
if (!c)
@@ -270,7 +275,7 @@ static int mutex_lock(unsigned long *m)
c = xchg(m, 2);
 
while (c) {
-   sys_futex(m, FUTEX_WAIT, 2, NULL, NULL, 0);
+   sys_futex(m, flags, 2, NULL, NULL, 0);
c = xchg(m, 2);
}
 
@@ -279,12 +284,16 @@ static int mutex_lock(unsigned long *m)
 
 static int mutex_unlock(unsigned long *m)
 {
+   int flags = FUTEX_WAKE;
+   if (!processes)
+   flags |= FUTEX_PRIVATE_FLAG;
+
if (*m == 2)
*m = 0;
else if (xchg(m, 0) == 1)
return 0;
 
-   sys_futex(m, FUTEX_WAKE, 1, NULL, NULL, 0);
+   sys_futex(m, flags, 1, NULL, NULL, 0);
 
return 0;
 }
@@ -293,26 +302,32 @@ static unsigned long *m1, *m2;
 
 static void futex_setup(int cpu1, int cpu2)
 {
-   int shmid;
-   void *shmaddr;
+   if (!processes) {
+   static unsigned long _m1, _m2;
+   m1 = &_m1;
+   m2 = &_m2;
+   } else {
+   int shmid;
+   void *shmaddr;
 
-   shmid = shmget(IPC_PRIVATE, getpagesize(), SHM_R | SHM_W);
-   if (shmid < 0) {
-   perror("shmget");
-   exit(1);
-   }
+   shmid = shmget(IPC_PRIVATE, getpagesize(), SHM_R | SHM_W);
+   if (shmid < 0) {
+   perror("shmget");
+   exit(1);
+   }
 
-   shmaddr = shmat(shmid, NULL, 0);
-   if (shmaddr == (char *)-1) {
-   perror("shmat");
-   shmctl(shmid, IPC_RMID, NULL);
-   exit(1);
-   }
+   shmaddr = shmat(shmid, NULL, 0);
+   if (shmaddr == (char *)-1) {
+   perror("shmat");
+   shmctl(shmid, IPC_RMID, NULL);
+   exit(1);
+   }
 
-   shmctl(shmid, IPC_RMID, NULL);
+   shmctl(shmid, IPC_RMID, NULL);
 
-   m1 = shmaddr;
-   m2 = shmaddr + sizeof(*m1);
+   m1 = shmaddr;
+   m2 = shmaddr + sizeof(*m1);
+   }
 
*m1 = 0;
*m2 = 0;
@@ -352,8 +367,6 @@ static struct actions futex_actions = {
.thread2 = futex_thread2,
 };
 
-static int processes;
-
 static struct option options[] = {
{ "test", required_argument, 0, 't' },
{ "process", no_argument, , 1 },
-- 
2.11.0