LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* powerpc:allmodconfig build failure (since 3.14)
From: Guenter Roeck @ 2014-05-01 16:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Paul Mackerras, linux-kernel, Geert Uytterhoeven

Hi all,

the powerpc:allmodconfig build has been broken since 3.14
with the following error.

arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
arch/powerpc/kernel/exceptions-64s.S:1315: Error: attempt to move .org backwards
make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1

Any chance to get this fixed, or is it time to drop powerpc:allmodconfig
from my test builds ?

I would give it a shot myself, but that file is one that I would prefer
not to touch.

Thanks,
Guenter

^ permalink raw reply

* Re: [4/5] IBM Akebono: Add the Akebono platform
From: Alistair Popple @ 2014-05-02  0:35 UTC (permalink / raw)
  To: Paul Bolle; +Cc: linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1398936447.4121.12.camel@x220>

Paul,

On Thu, 1 May 2014 11:27:27 Paul Bolle wrote:
> On Thu, 2014-03-06 at 14:52 +1100, Alistair Popple wrote:

[...]

> > This patch adds support for the IBM Akebono board.
> > +	select IBM_EMAC_RGMII_WOL
> 
> The patch that added this symbol (and the related driver) was submitted
> in https://lkml.org/lkml/2014/2/21/25 . It's not (yet) included in
> linux-next. Is it queued somewhere else?

To be honest I'm not sure. I will follow this up on the netdev list.

> > +	select USB
> > +	select USB_OHCI_HCD_PLATFORM
> > +	select USB_EHCI_HCD_PLATFORM
> > +	select MMC_SDHCI
> > +	select MMC_SDHCI_PLTFM
> > +	select MMC_SDHCI_OF_476GTR
> 
> The plan to add MMC_SDHCI_OF_476GTR seems to have been abandoned (see
> the discussion of https://lkml.org/lkml/2014/2/21/24 ). So this select
> is not needed. Should I submit a trivial patch to drop this select?

Thanks for pointing this out, I should have removed the select for 
MMC_SDHCI_OF_476GTR as suggested. I can submit a patch to drop the select or 
send an updated version of the original patch. Which is easiest for you Ben?

> > +	select ATA
> > +	select SATA_AHCI_PLATFORM
> > +	help
> > +	  This option enables support for the IBM Akebono (476gtr) evaluation
> > board +
> > +
> > 
> >  config ICON
> >  
> >  	bool "Icon"
> >  	depends on 44x
> 
> Paul Bolle

^ permalink raw reply

* [PATCH 1/2] powerpc: Move adb symbol exports next to function definitions
From: Anton Blanchard @ 2014-05-02  0:44 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev


Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kernel/ppc_ksyms.c
===================================================================
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -128,13 +128,6 @@ EXPORT_SYMBOL(smp_hw_index);
 #endif
 #endif
 
-#ifdef CONFIG_ADB
-EXPORT_SYMBOL(adb_request);
-EXPORT_SYMBOL(adb_register);
-EXPORT_SYMBOL(adb_unregister);
-EXPORT_SYMBOL(adb_poll);
-EXPORT_SYMBOL(adb_try_handler_change);
-#endif /* CONFIG_ADB */
 #ifdef CONFIG_ADB_CUDA
 EXPORT_SYMBOL(cuda_request);
 EXPORT_SYMBOL(cuda_poll);
Index: b/drivers/macintosh/adb.c
===================================================================
--- a/drivers/macintosh/adb.c
+++ b/drivers/macintosh/adb.c
@@ -411,6 +411,7 @@ adb_poll(void)
 		return;
 	adb_controller->poll();
 }
+EXPORT_SYMBOL(adb_poll);
 
 static void adb_sync_req_done(struct adb_request *req)
 {
@@ -460,6 +461,7 @@ adb_request(struct adb_request *req, voi
 
 	return rc;
 }
+EXPORT_SYMBOL(adb_request);
 
  /* Ultimately this should return the number of devices with
     the given default id.
@@ -495,6 +497,7 @@ adb_register(int default_id, int handler
 	mutex_unlock(&adb_handler_mutex);
 	return ids->nids;
 }
+EXPORT_SYMBOL(adb_register);
 
 int
 adb_unregister(int index)
@@ -516,6 +519,7 @@ adb_unregister(int index)
 	mutex_unlock(&adb_handler_mutex);
 	return ret;
 }
+EXPORT_SYMBOL(adb_unregister);
 
 void
 adb_input(unsigned char *buf, int nb, int autopoll)
@@ -582,6 +586,7 @@ adb_try_handler_change(int address, int
 	mutex_unlock(&adb_handler_mutex);
 	return ret;
 }
+EXPORT_SYMBOL(adb_try_handler_change);
 
 int
 adb_get_infos(int address, int *original_address, int *handler_id)

^ permalink raw reply

* [PATCH 2/2] powerpc: Move via-cuda symbol exports next to function definitions
From: Anton Blanchard @ 2014-05-02  0:46 UTC (permalink / raw)
  To: benh, paulus; +Cc: linuxppc-dev
In-Reply-To: <20140502104418.4cee8f15@kryten>


Signed-off-by: Anton Blanchard <anton@samba.org>
---

Index: b/arch/powerpc/kernel/ppc_ksyms.c
===================================================================
--- a/arch/powerpc/kernel/ppc_ksyms.c
+++ b/arch/powerpc/kernel/ppc_ksyms.c
@@ -128,10 +128,6 @@ EXPORT_SYMBOL(smp_hw_index);
 #endif
 #endif
 
-#ifdef CONFIG_ADB_CUDA
-EXPORT_SYMBOL(cuda_request);
-EXPORT_SYMBOL(cuda_poll);
-#endif /* CONFIG_ADB_CUDA */
 EXPORT_SYMBOL(to_tm);
 
 #ifdef CONFIG_PPC32
Index: b/drivers/macintosh/via-cuda.c
===================================================================
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -379,6 +379,7 @@ cuda_request(struct adb_request *req, vo
     req->reply_expected = 1;
     return cuda_write(req);
 }
+EXPORT_SYMBOL(cuda_request);
 
 static int
 cuda_write(struct adb_request *req)
@@ -441,6 +442,7 @@ cuda_poll(void)
     if (cuda_irq)
 	enable_irq(cuda_irq);
 }
+EXPORT_SYMBOL(cuda_poll);
 
 static irqreturn_t
 cuda_interrupt(int irq, void *arg)

^ permalink raw reply

* Re: [PATCH 2/5 V2] IBM Akebono: Add support for a new PHY interface to the IBM emac driver
From: Alistair Popple @ 2014-05-02  0:48 UTC (permalink / raw)
  To: davem; +Cc: netdev, linuxppc-dev, linux-kernel, devicetree
In-Reply-To: <1394498673-24447-1-git-send-email-alistair@popple.id.au>

Hi David,

Given that the Akebono board support has landed in linux-next (next-20140501) 
and there are no outstanding issues with this patch that I'm aware of (please 
let me know if I've missed anything) would it be possible to get this included 
via your tree? Or should I get Ben H to take this through his tree (with your 
Ack)?

Regards,

Alistair

On Tue, 11 Mar 2014 11:44:33 Alistair Popple wrote:
> The IBM PPC476GTR SoC that is used on the Akebono board uses a
> different ethernet PHY interface that has wake on lan (WOL) support
> with the IBM emac. This patch adds support to the IBM emac driver for
> this new PHY interface.
> 
> At this stage the wake on lan functionality has not been implemented.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> This version just fixes the coding style as suggested by David M.
> 
>  .../devicetree/bindings/powerpc/4xx/emac.txt       |   9 +
>  drivers/net/ethernet/ibm/emac/Kconfig              |   4 +
>  drivers/net/ethernet/ibm/emac/Makefile             |   1 +
>  drivers/net/ethernet/ibm/emac/core.c               |  50 ++++-
>  drivers/net/ethernet/ibm/emac/core.h               |  12 +
>  drivers/net/ethernet/ibm/emac/rgmii_wol.c          | 244
> +++++++++++++++++++++ drivers/net/ethernet/ibm/emac/rgmii_wol.h          | 
> 62 ++++++
>  7 files changed, 376 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/net/ethernet/ibm/emac/rgmii_wol.c
>  create mode 100644 drivers/net/ethernet/ibm/emac/rgmii_wol.h
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt index
> 712baf6..0c20529 100644
> --- a/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> +++ b/Documentation/devicetree/bindings/powerpc/4xx/emac.txt
> @@ -61,6 +61,8 @@
>  			  Fox Axon: present, whatever value is appropriate for 
each
>  			  EMAC, that is the content of the current (bogus) 
"phy-port"
>  			  property.
> +    - rgmii-wol-device  : 1 cell, required iff connected to a RGMII in the
> WKUP +                          power domain. phandle of the RGMII-WOL
> device node.
> 
>      Optional properties:
>      - phy-address       : 1 cell, optional, MDIO address of the PHY. If
> absent, @@ -146,3 +148,10 @@
>  			   available.
>  			   For Axon: 0x0000012a
> 
> +      iv) RGMII-WOL node
> +
> +    Required properties:
> +    - compatible         : compatible list, containing 2 entries, first is
> +			   "ibm,rgmii-wol-CHIP" where CHIP is the host ASIC 
(like
> +			   EMAC) and the second is "ibm,rgmii-wol".
> +    - reg                : <registers mapping>
> diff --git a/drivers/net/ethernet/ibm/emac/Kconfig
> b/drivers/net/ethernet/ibm/emac/Kconfig index 3f44a30..56ea346 100644
> --- a/drivers/net/ethernet/ibm/emac/Kconfig
> +++ b/drivers/net/ethernet/ibm/emac/Kconfig
> @@ -55,6 +55,10 @@ config IBM_EMAC_RGMII
>  	bool
>  	default n
> 
> +config IBM_EMAC_RGMII_WOL
> +	bool "IBM EMAC RGMII wake-on-LAN support" if COMPILE_TEST
> +	default n
> +
>  config IBM_EMAC_TAH
>  	bool
>  	default n
> diff --git a/drivers/net/ethernet/ibm/emac/Makefile
> b/drivers/net/ethernet/ibm/emac/Makefile index eba2183..8843803 100644
> --- a/drivers/net/ethernet/ibm/emac/Makefile
> +++ b/drivers/net/ethernet/ibm/emac/Makefile
> @@ -7,5 +7,6 @@ obj-$(CONFIG_IBM_EMAC) += ibm_emac.o
>  ibm_emac-y := mal.o core.o phy.o
>  ibm_emac-$(CONFIG_IBM_EMAC_ZMII) += zmii.o
>  ibm_emac-$(CONFIG_IBM_EMAC_RGMII) += rgmii.o
> +ibm_emac-$(CONFIG_IBM_EMAC_RGMII_WOL) += rgmii_wol.o
>  ibm_emac-$(CONFIG_IBM_EMAC_TAH) += tah.o
>  ibm_emac-$(CONFIG_IBM_EMAC_DEBUG) += debug.o
> diff --git a/drivers/net/ethernet/ibm/emac/core.c
> b/drivers/net/ethernet/ibm/emac/core.c index ae342fd..ff58474 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> @@ -632,6 +632,8 @@ static int emac_configure(struct emac_instance *dev)
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_set_speed(dev->rgmii_dev, dev->rgmii_port,
>  				dev->phy.speed);
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_set_speed(dev->rgmii_wol_dev, dev->phy.speed);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_ZMII))
>  		zmii_set_speed(dev->zmii_dev, dev->zmii_port, dev->phy.speed);
> 
> @@ -799,6 +801,8 @@ static int __emac_mdio_read(struct emac_instance *dev,
> u8 id, u8 reg) zmii_get_mdio(dev->zmii_dev, dev->zmii_port);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_get_mdio(dev->rgmii_dev, dev->rgmii_port);
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_get_mdio(dev->rgmii_wol_dev);
> 
>  	/* Wait for management interface to become idle */
>  	n = 20;
> @@ -846,6 +850,8 @@ static int __emac_mdio_read(struct emac_instance *dev,
> u8 id, u8 reg) DBG2(dev, "mdio_read -> %04x" NL, r);
>  	err = 0;
>   bail:
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_put_mdio(dev->rgmii_wol_dev);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_put_mdio(dev->rgmii_dev, dev->rgmii_port);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_ZMII))
> @@ -871,6 +877,8 @@ static void __emac_mdio_write(struct emac_instance *dev,
> u8 id, u8 reg, zmii_get_mdio(dev->zmii_dev, dev->zmii_port);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_get_mdio(dev->rgmii_dev, dev->rgmii_port);
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_get_mdio(dev->rgmii_wol_dev);
> 
>  	/* Wait for management interface to be idle */
>  	n = 20;
> @@ -909,6 +917,8 @@ static void __emac_mdio_write(struct emac_instance *dev,
> u8 id, u8 reg, }
>  	err = 0;
>   bail:
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_put_mdio(dev->rgmii_wol_dev);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_put_mdio(dev->rgmii_dev, dev->rgmii_port);
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_ZMII))
> @@ -2277,10 +2287,11 @@ struct emac_depentry {
>  #define	EMAC_DEP_MAL_IDX	0
>  #define	EMAC_DEP_ZMII_IDX	1
>  #define	EMAC_DEP_RGMII_IDX	2
> -#define	EMAC_DEP_TAH_IDX	3
> -#define	EMAC_DEP_MDIO_IDX	4
> -#define	EMAC_DEP_PREV_IDX	5
> -#define	EMAC_DEP_COUNT		6
> +#define EMAC_DEP_RGMII_WOL_IDX  3
> +#define	EMAC_DEP_TAH_IDX	4
> +#define	EMAC_DEP_MDIO_IDX	5
> +#define	EMAC_DEP_PREV_IDX	6
> +#define	EMAC_DEP_COUNT		7
> 
>  static int emac_check_deps(struct emac_instance *dev,
>  			   struct emac_depentry *deps)
> @@ -2358,6 +2369,7 @@ static int emac_wait_deps(struct emac_instance *dev)
>  	deps[EMAC_DEP_MAL_IDX].phandle = dev->mal_ph;
>  	deps[EMAC_DEP_ZMII_IDX].phandle = dev->zmii_ph;
>  	deps[EMAC_DEP_RGMII_IDX].phandle = dev->rgmii_ph;
> +	deps[EMAC_DEP_RGMII_WOL_IDX].phandle = dev->rgmii_wol_ph;
>  	if (dev->tah_ph)
>  		deps[EMAC_DEP_TAH_IDX].phandle = dev->tah_ph;
>  	if (dev->mdio_ph)
> @@ -2380,6 +2392,7 @@ static int emac_wait_deps(struct emac_instance *dev)
>  		dev->mal_dev = deps[EMAC_DEP_MAL_IDX].ofdev;
>  		dev->zmii_dev = deps[EMAC_DEP_ZMII_IDX].ofdev;
>  		dev->rgmii_dev = deps[EMAC_DEP_RGMII_IDX].ofdev;
> +		dev->rgmii_wol_dev = deps[EMAC_DEP_RGMII_WOL_IDX].ofdev;
>  		dev->tah_dev = deps[EMAC_DEP_TAH_IDX].ofdev;
>  		dev->mdio_dev = deps[EMAC_DEP_MDIO_IDX].ofdev;
>  	}
> @@ -2585,6 +2598,8 @@ static int emac_init_config(struct emac_instance *dev)
> dev->rgmii_ph = 0;
>  	if (emac_read_uint_prop(np, "rgmii-channel", &dev->rgmii_port, 0))
>  		dev->rgmii_port = 0xffffffff;
> +	if (emac_read_uint_prop(np, "rgmii-wol-device", &dev->rgmii_wol_ph, 
0))
> +		dev->rgmii_wol_ph = 0;
>  	if (emac_read_uint_prop(np, "fifo-entry-size", &dev->fifo_entry_size, 
0))
>  		dev->fifo_entry_size = 16;
>  	if (emac_read_uint_prop(np, "mal-burst-size", &dev->mal_burst_size, 
0))
> @@ -2671,6 +2686,16 @@ static int emac_init_config(struct emac_instance
> *dev) #endif
>  	}
> 
> +	if (dev->rgmii_wol_ph != 0) {
> +#ifdef CONFIG_IBM_EMAC_RGMII_WOL
> +		dev->features |= EMAC_FTR_HAS_RGMII_WOL;
> +#else
> +		printk(KERN_ERR "%s: RGMII WOL support not enabled !\n",
> +		       np->full_name);
> +		return -ENXIO;
> +#endif
> +	}
> +
>  	/* Read MAC-address */
>  	p = of_get_property(np, "local-mac-address", NULL);
>  	if (p == NULL) {
> @@ -2844,10 +2869,15 @@ static int emac_probe(struct platform_device *ofdev)
> (err = rgmii_attach(dev->rgmii_dev, dev->rgmii_port, dev->phy_mode)) != 0)
> goto err_detach_zmii;
> 
> +	/* Attach to RGMII_WOL, if needed */
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL) &&
> +	    (err = rgmii_wol_attach(dev->rgmii_wol_dev, dev->phy_mode)) != 0)
> +		goto err_detach_rgmii;
> +
>  	/* Attach to TAH, if needed */
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH) &&
>  	    (err = tah_attach(dev->tah_dev, dev->tah_port)) != 0)
> -		goto err_detach_rgmii;
> +		goto err_detach_rgmii_wol;
> 
>  	/* Set some link defaults before we can find out real parameters */
>  	dev->phy.speed = SPEED_100;
> @@ -2920,6 +2950,9 @@ static int emac_probe(struct platform_device *ofdev)
>   err_detach_tah:
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_TAH))
>  		tah_detach(dev->tah_dev, dev->tah_port);
> + err_detach_rgmii_wol:
> +	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII_WOL))
> +		rgmii_wol_detach(dev->rgmii_wol_dev);
>   err_detach_rgmii:
>  	if (emac_has_feature(dev, EMAC_FTR_HAS_RGMII))
>  		rgmii_detach(dev->rgmii_dev, dev->rgmii_port);
> @@ -3081,12 +3114,17 @@ static int __init emac_init(void)
>  	rc = tah_init();
>  	if (rc)
>  		goto err_rgmii;
> -	rc = platform_driver_register(&emac_driver);
> +	rc = rgmii_wol_init();
>  	if (rc)
>  		goto err_tah;
> +	rc = platform_driver_register(&emac_driver);
> +	if (rc)
> +		goto err_rgmii_wol;
> 
>  	return 0;
> 
> + err_rgmii_wol:
> +	rgmii_wol_exit();
>   err_tah:
>  	tah_exit();
>   err_rgmii:
> diff --git a/drivers/net/ethernet/ibm/emac/core.h
> b/drivers/net/ethernet/ibm/emac/core.h index 67f342a..7e1a70d 100644
> --- a/drivers/net/ethernet/ibm/emac/core.h
> +++ b/drivers/net/ethernet/ibm/emac/core.h
> @@ -42,6 +42,7 @@
>  #include "phy.h"
>  #include "zmii.h"
>  #include "rgmii.h"
> +#include "rgmii_wol.h"
>  #include "mal.h"
>  #include "tah.h"
>  #include "debug.h"
> @@ -209,6 +210,10 @@ struct emac_instance {
>  	u32				rgmii_port;
>  	struct platform_device		*rgmii_dev;
> 
> +	/* RGMII WOL infos if any */
> +	u32				rgmii_wol_ph;
> +	struct platform_device		*rgmii_wol_dev;
> +
>  	/* TAH infos if any */
>  	u32				tah_ph;
>  	u32				tah_port;
> @@ -332,6 +337,10 @@ struct emac_instance {
>   * APM821xx does not support Half Duplex mode
>   */
>  #define EMAC_FTR_APM821XX_NO_HALF_DUPLEX	0x00001000
> +/*
> + * Set if we have a RGMII with wake on LAN.
> + */
> +#define EMAC_FTR_HAS_RGMII_WOL		0x00020000
> 
>  /* Right now, we don't quite handle the always/possible masks on the
>   * most optimal way as we don't have a way to say something like
> @@ -355,6 +364,9 @@ enum {
>  #ifdef CONFIG_IBM_EMAC_RGMII
>  	    EMAC_FTR_HAS_RGMII	|
>  #endif
> +#ifdef CONFIG_IBM_EMAC_RGMII_WOL
> +	    EMAC_FTR_HAS_RGMII_WOL	|
> +#endif
>  #ifdef CONFIG_IBM_EMAC_NO_FLOW_CTRL
>  	    EMAC_FTR_NO_FLOW_CONTROL_40x |
>  #endif
> diff --git a/drivers/net/ethernet/ibm/emac/rgmii_wol.c
> b/drivers/net/ethernet/ibm/emac/rgmii_wol.c new file mode 100644
> index 0000000..69785d7
> --- /dev/null
> +++ b/drivers/net/ethernet/ibm/emac/rgmii_wol.c
> @@ -0,0 +1,244 @@
> +/* drivers/net/ethernet/ibm/emac/rgmii_wol.c
> + *
> + * Driver for PowerPC 4xx on-chip ethernet controller, RGMII bridge with
> + * wake on LAN support.
> + *
> + * Copyright 2013 Alistair Popple, IBM Corp.
> + *                <alistair@popple.id.au>
> + *
> + * Based on rgmii.h:
> + * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
> + *                <benh@kernel.crashing.org>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by
> the + * Free Software Foundation;  either version 2 of the  License, or (at
> your + * option) any later version.
> + */
> +#include <linux/slab.h>
> +#include <linux/kernel.h>
> +#include <linux/ethtool.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +
> +#include "emac.h"
> +#include "debug.h"
> +
> +/* RGMII_WOL_REG */
> +
> +#define WKUP_ETH_RGSPD      0xC0000000
> +#define WKUP_ETH_FCSEN      0x20000000
> +#define WKUP_ETH_CRSEN      0x02000000
> +#define WKUP_ETH_COLEN      0x01000000
> +#define WKUP_ETH_TX_OE      0x00040000
> +#define WKUP_ETH_RX_IE      0x00020000
> +#define WKUP_ETH_RGMIIEN    0x00010000
> +
> +#define WKUP_ETH_RGSPD_10   0x00000000
> +#define WKUP_ETH_RGSPD_100  0x40000000
> +#define WKUP_ETH_RGSPD_1000 0x80000000
> +
> +/* RGMII bridge supports only GMII/TBI and RGMII/RTBI PHYs */
> +static inline int rgmii_valid_mode(int phy_mode)
> +{
> +	return  phy_mode == PHY_MODE_GMII ||
> +		phy_mode == PHY_MODE_MII ||
> +		phy_mode == PHY_MODE_RGMII ||
> +		phy_mode == PHY_MODE_TBI ||
> +		phy_mode == PHY_MODE_RTBI;
> +}
> +
> +int rgmii_wol_attach(struct platform_device *ofdev, int mode)
> +{
> +	struct rgmii_wol_instance *dev = platform_get_drvdata(ofdev);
> +
> +	dev_dbg(&ofdev->dev, "attach\n");
> +
> +	/* Check if we need to attach to a RGMII */
> +	if (!rgmii_valid_mode(mode)) {
> +		dev_err(&ofdev->dev, "unsupported settings !\n");
> +		return -ENODEV;
> +	}
> +
> +	mutex_lock(&dev->lock);
> +
> +	/* Enable this input */
> +	out_be32(dev->reg, (in_be32(dev->reg) | WKUP_ETH_RGMIIEN |
> +				    WKUP_ETH_TX_OE | WKUP_ETH_RX_IE));
> +
> +	++dev->users;
> +
> +	mutex_unlock(&dev->lock);
> +
> +	return 0;
> +}
> +
> +void rgmii_wol_set_speed(struct platform_device *ofdev, int speed)
> +{
> +	struct rgmii_wol_instance *dev = platform_get_drvdata(ofdev);
> +	u32 reg;
> +
> +	mutex_lock(&dev->lock);
> +
> +	reg = in_be32(dev->reg) & ~WKUP_ETH_RGSPD;
> +
> +	dev_dbg(&ofdev->dev, "speed(%d)\n", speed);
> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		reg |= WKUP_ETH_RGSPD_1000;
> +		break;
> +	case SPEED_100:
> +		reg |= WKUP_ETH_RGSPD_100;
> +		break;
> +	case SPEED_10:
> +		reg |= WKUP_ETH_RGSPD_10;
> +		break;
> +	default:
> +		dev_err(&ofdev->dev, "invalid speed set!\n");
> +	}
> +
> +	out_be32(dev->reg, reg);
> +
> +	mutex_unlock(&dev->lock);
> +}
> +
> +void rgmii_wol_get_mdio(struct platform_device *ofdev)
> +{
> +	/* MDIO is always enabled when RGMII_WOL is enabled, so we
> +	 * don't have to do anything here.
> +	 */
> +	dev_dbg(&ofdev->dev, "get_mdio\n");
> +}
> +
> +void rgmii_wol_put_mdio(struct platform_device *ofdev)
> +{
> +	dev_dbg(&ofdev->dev, "put_mdio\n");
> +}
> +
> +void rgmii_wol_detach(struct platform_device *ofdev)
> +{
> +	struct rgmii_wol_instance *dev = platform_get_drvdata(ofdev);
> +
> +	BUG_ON(!dev || dev->users == 0);
> +
> +	mutex_lock(&dev->lock);
> +
> +	dev_dbg(&ofdev->dev, "detach\n");
> +
> +	/* Disable this input */
> +	out_be32(dev->reg, 0);
> +
> +	--dev->users;
> +
> +	mutex_unlock(&dev->lock);
> +}
> +
> +int rgmii_wol_get_regs_len(struct platform_device *ofdev)
> +{
> +	return sizeof(struct emac_ethtool_regs_subhdr) +
> +		sizeof(u32);
> +}
> +
> +void *rgmii_wol_dump_regs(struct platform_device *ofdev, void *buf)
> +{
> +	struct rgmii_wol_instance *dev = platform_get_drvdata(ofdev);
> +	struct emac_ethtool_regs_subhdr *hdr = buf;
> +	u32 *regs = (u32 *)(hdr + 1);
> +
> +	hdr->version = 0;
> +	hdr->index = 0; /* for now, are there chips with more than one
> +			 * rgmii ? if yes, then we'll add a cell_index
> +			 * like we do for emac
> +			 */
> +	memcpy_fromio(regs, dev->reg, sizeof(u32));
> +	return regs + 1;
> +}
> +
> +
> +static int rgmii_wol_probe(struct platform_device *ofdev)
> +{
> +	struct device_node *np = ofdev->dev.of_node;
> +	struct rgmii_wol_instance *dev;
> +	int rc;
> +
> +	rc = -ENOMEM;
> +	dev = kzalloc(sizeof(struct rgmii_wol_instance), GFP_KERNEL);
> +	if (dev == NULL)
> +		goto err_gone;
> +
> +	mutex_init(&dev->lock);
> +
> +	dev->reg = of_iomap(np, 0);
> +	if (!dev->reg) {
> +		dev_err(&ofdev->dev, "Can't map registers\n");
> +		rc = -ENXIO;
> +		goto err_free;
> +	}
> +
> +	/* Check for RGMII flags */
> +	if (of_property_read_bool(np, "has-mdio"))
> +		dev->flags |= EMAC_RGMII_FLAG_HAS_MDIO;
> +
> +	dev_dbg(&ofdev->dev, "Boot REG = 0x%08x\n", in_be32(dev->reg));
> +
> +	/* Disable all inputs by default */
> +	out_be32(dev->reg, 0);
> +
> +	dev_info(&ofdev->dev,
> +	       "RGMII %s initialized with%s MDIO support\n",
> +	       ofdev->dev.of_node->full_name,
> +	       (dev->flags & EMAC_RGMII_FLAG_HAS_MDIO) ? "" : "out");
> +
> +	wmb();
> +	platform_set_drvdata(ofdev, dev);
> +
> +	return 0;
> +
> + err_free:
> +	kfree(dev);
> + err_gone:
> +	return rc;
> +}
> +
> +static int rgmii_wol_remove(struct platform_device *ofdev)
> +{
> +	struct rgmii_wol_instance *dev = platform_get_drvdata(ofdev);
> +
> +	WARN_ON(dev->users != 0);
> +
> +	iounmap(dev->reg);
> +	kfree(dev);
> +
> +	return 0;
> +}
> +
> +static struct of_device_id rgmii_wol_match[] = {
> +	{
> +		.compatible	= "ibm,rgmii-wol",
> +	},
> +	{
> +		.type		= "emac-rgmii-wol",
> +	},
> +	{},
> +};
> +
> +static struct platform_driver rgmii_wol_driver = {
> +	.driver = {
> +		.name = "emac-rgmii-wol",
> +		.owner = THIS_MODULE,
> +		.of_match_table = rgmii_wol_match,
> +	},
> +	.probe = rgmii_wol_probe,
> +	.remove = rgmii_wol_remove,
> +};
> +
> +int __init rgmii_wol_init(void)
> +{
> +	return platform_driver_register(&rgmii_wol_driver);
> +}
> +
> +void rgmii_wol_exit(void)
> +{
> +	platform_driver_unregister(&rgmii_wol_driver);
> +}
> diff --git a/drivers/net/ethernet/ibm/emac/rgmii_wol.h
> b/drivers/net/ethernet/ibm/emac/rgmii_wol.h new file mode 100644
> index 0000000..9f0b589
> --- /dev/null
> +++ b/drivers/net/ethernet/ibm/emac/rgmii_wol.h
> @@ -0,0 +1,62 @@
> +/* drivers/net/ethernet/ibm/emac/rgmii_wol.h
> + *
> + * Driver for PowerPC 4xx on-chip ethernet controller, RGMII bridge with
> + * wake on LAN support.
> + *
> + * Copyright 2013 Alistair Popple, IBM Corp.
> + *                <alistair@popple.id.au>
> + *
> + * Based on rgmii.h:
> + * Copyright 2007 Benjamin Herrenschmidt, IBM Corp.
> + *                <benh@kernel.crashing.org>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by
> the + * Free Software Foundation;  either version 2 of the  License, or (at
> your + * option) any later version.
> + */
> +
> +#ifndef __IBM_NEWEMAC_RGMII_WOL_H
> +#define __IBM_NEWEMAC_RGMII_WOL_H
> +
> +/* RGMII device */
> +struct rgmii_wol_instance {
> +	u32 __iomem			*reg;
> +
> +	/* RGMII bridge flags */
> +	int				flags;
> +#define EMAC_RGMII_FLAG_HAS_MDIO	0x00000001
> +
> +	/* Only one EMAC whacks us at a time */
> +	struct mutex			lock;
> +
> +	/* number of EMACs using this RGMII bridge */
> +	int				users;
> +};
> +
> +#ifdef CONFIG_IBM_EMAC_RGMII_WOL
> +
> +extern int rgmii_wol_init(void);
> +extern void rgmii_wol_exit(void);
> +extern int rgmii_wol_attach(struct platform_device *ofdev, int mode);
> +extern void rgmii_wol_detach(struct platform_device *ofdev);
> +extern void rgmii_wol_get_mdio(struct platform_device *ofdev);
> +extern void rgmii_wol_put_mdio(struct platform_device *ofdev);
> +extern void rgmii_wol_set_speed(struct platform_device *ofdev, int speed);
> +extern int rgmii_wol_get_regs_len(struct platform_device *ofdev);
> +extern void *rgmii_wol_dump_regs(struct platform_device *ofdev, void *buf);
> +
> +#else
> +
> +# define rgmii_wol_init()		0
> +# define rgmii_wol_exit()		do { } while (0)
> +# define rgmii_wol_attach(x, y)		(-ENXIO)
> +# define rgmii_wol_detach(x)		do { } while (0)
> +# define rgmii_wol_get_mdio(o)		do { } while (0)
> +# define rgmii_wol_put_mdio(o)		do { } while (0)
> +# define rgmii_wol_set_speed(x, y)	do { } while (0)
> +# define rgmii_wol_get_regs_len(x)	0
> +# define rgmii_wol_dump_regs(x, buf)	(buf)
> +#endif				/* !CONFIG_IBM_EMAC_RGMII_WOL */
> +
> +#endif /* __IBM_NEWEMAC_RGMII_WOL_H */

^ permalink raw reply

* Re: cscope: issue with symlinks in tools/testing/selftests/powerpc/copyloops/
From: Michael Ellerman @ 2014-05-02  1:19 UTC (permalink / raw)
  To: Yann Droneaud
  Cc: Hans-Bernhard Bröker, Neil Horman, linux-kernel, Neil Horman,
	cscope-devel, Anton Blanchard, linuxppc-dev,
	Hans-Bernhard Broeker
In-Reply-To: <1396530975.4361.28.camel@localhost.localdomain>

On Thu, 2014-04-03 at 15:16 +0200, Yann Droneaud wrote:
> Hi,
> 
> I'm using cscope to browse kernel sources, but I'm facing warnings from
> the tool since following commit:
> 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=22d651dcef536c75f75537290bf3da5038e68b6b
> 
>     commit 22d651dcef536c75f75537290bf3da5038e68b6b
>     Author: Michael Ellerman <mpe@ellerman.id.au>
>     Date:   Tue Jan 21 15:22:17 2014 +1100
> 
>     selftests/powerpc: Import Anton's memcpy / copy_tofrom_user tests

Ooops, sorry.

> cscope reports error when generating the cross-reference database:
> 
>     $ make ALLSOURCE_ARCHS=all O=./obj-cscope/ cscope
>       GEN     cscope
>     cscope: cannot find
> file /home/ydroneaud/src/linux/tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
> 
> It's a rather uncommon side effect of having (for the first time ?)
> sources files as symlinks: looking for symlinks in the kernel sources
> returns only:
> 
>     $ find . -type l
>     ./arch/mips/boot/dts/include/dt-bindings
>     ./arch/microblaze/boot/dts/system.dts
>     ./arch/powerpc/boot/dts/include/dt-bindings
>     ./arch/metag/boot/dts/include/dt-bindings
>     ./arch/arm/boot/dts/include/dt-bindings
>     ./tools/testing/selftests/powerpc/copyloops/copyuser_power7.S
>     ./tools/testing/selftests/powerpc/copyloops/memcpy_64.S
>     ./tools/testing/selftests/powerpc/copyloops/memcpy_power7.S
>     ./tools/testing/selftests/powerpc/copyloops/copyuser_64.S


Right.

I did check that there were other symlinks already in the tree, but you're
correct I seem to be the first clever person to add symlinks to source files.

> So one can wonder if having symlinked sources files is an expected
> supported feature for kbuild and all the various kernel
> tools/infrastructure ?

Kbuild is not involved really, these files are just built with plain Makefiles,
at least from the symlink side.

FWIW ctags seems to cope, that's what I use.

But I didn't think of cscope.


Given you're the only person who's noticed I suspect most other things haven't
broken, fingers crossed :)

cheers

^ permalink raw reply

* Re: [4/5] IBM Akebono: Add the Akebono platform
From: Benjamin Herrenschmidt @ 2014-05-02  2:33 UTC (permalink / raw)
  To: Alistair Popple; +Cc: devicetree, Paul Bolle, linuxppc-dev, linux-kernel
In-Reply-To: <10214508.Vq3Ynjy2Co@mexican>

On Fri, 2014-05-02 at 10:35 +1000, Alistair Popple wrote:

> > The plan to add MMC_SDHCI_OF_476GTR seems to have been abandoned (see
> > the discussion of https://lkml.org/lkml/2014/2/21/24 ). So this select
> > is not needed. Should I submit a trivial patch to drop this select?
> 
> Thanks for pointing this out, I should have removed the select for 
> MMC_SDHCI_OF_476GTR as suggested. I can submit a patch to drop the select or 
> send an updated version of the original patch. Which is easiest for you Ben?

I don't rebase next so it has to be a followup patch.

Cheers,
Ben.

> > > +	select ATA
> > > +	select SATA_AHCI_PLATFORM
> > > +	help
> > > +	  This option enables support for the IBM Akebono (476gtr) evaluation
> > > board +
> > > +
> > > 
> > >  config ICON
> > >  
> > >  	bool "Icon"
> > >  	depends on 44x
> > 
> > Paul Bolle
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: MPC8641 based custom board Kernel stuck at 1000Mhz core clock
From: sanjeev sharma @ 2014-05-02  3:52 UTC (permalink / raw)
  To: Ashish; +Cc: scottwood, linuxppc-dev, Valdis Kletnieks, kernelnewbies
In-Reply-To: <533E6270.8070700@gmail.com>

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

Hello Ashish,

is this issue resolved and is this always happening when you are changing
core to 1Ghz because i am confused here with sometime Keyword.

Regards
Sanjeev Sharma


On Fri, Apr 4, 2014 at 1:12 PM, Ashish <curieux.khetan@gmail.com> wrote:

>  On Thursday 03 April 2014 08:55 AM, sanjeev sharma wrote:
>
>  Are you able to capture kernel logs ?
>
>  Regards
> Sanjeev Sharma
>
>
> On Thu, Mar 27, 2014 at 10:01 PM, <Valdis.Kletnieks@vt.edu> wrote:
>
>> On Thu, 27 Mar 2014 16:04:37 +0530, Ashish said:
>> > Hi,
>> >
>> >   I am using MPC8641-HPCN based custom board and able to boot linux at
>> > MPX clock 400Mhz and core clock 800mhz. When I am increasing core
>> > frequency ie MPX clock at 400Mhz and core at 1Ghz, kernel stuck.
>>
>>  Step 0:  Prove to us that your core actually runs reliable and stably at
>> 1Ghz.
>>
>> Step 1: Figure out *where* it gets stuck.  If you have earlyprintk
>> working on
>> your board, adding 'initcall_debug ignore_loglevel' to the kernel cmdline
>> often
>> helps track down where a kernel hangs during boot.
>>
>>
>> _______________________________________________
>> Kernelnewbies mailing list
>> Kernelnewbies@kernelnewbies.org
>> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
>>
>>
>  Hi,
> Does kernel logs means debugging information that kernel prints while
> booting using printk or it is something else?
> Here is kernel boot logs that kernel printed while booting...
>
> U-Boot 2013.04 (Jan 27 2014 - 11:21:21)
>
> Unicore software on multiprocessor system!!
> To enable mutlticore build define CONFIG_MP
> CPU:   8641, Version: 2.1, (0x80900021)
> Core:  E600 Core 0, Version: 2.2, (0x80040202)
> Clock Configuration:
>        CPU:1000 MHz, MPX:400  MHz
>        DDR:200  MHz (400 MT/s data rate), LBC:25   MHz
> L1:    D-cache 32 KB enabled
>        I-cache 32 KB enabled
> L2:    512 KB enabled
> Board: MPC8641-HPCN
> I2C:   ready
> DRAM:  512 MiB
> SDRAM test phase 1:
> SDRAM test phase 2:
> SDRAM test passed.
> Flash: 16 MiB
> EEPROM: NXID v1
> In:    serial
> Out:   serial
> Err:   serial
> Net:   eTSEC1, eTSEC2, eTSEC3, eTSEC4 [PRIME]
> Hit any key to stop autoboot:
> Speed: 1000, full duplex
> Using eTSEC4 device
> TFTP from server 192.168.10.1; our IP address is 192.168.10.2
> Filename 'uRamdisk'.
> Load address: 0x600000
> Loading: *
> #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      #################################################################
>      ##################################################
>      13.1 MiB/s
> done
> Bytes transferred = 22680188 (15a127c hex)
> Speed: 1000, full duplex
> Using eTSEC4 device
> TFTP from server 192.168.10.1; our IP address is 192.168.10.2
> Filename 'uImage'.
> Load address: 0x16000000
> Loading: *
> #################################################################
>      #################################################################
>      #######################################
>      14 MiB/s
> done
> Bytes transferred = 2476304 (25c910 hex)
> Speed: 1000, full duplex
> Using eTSEC4 device
> TFTP from server 192.168.10.1; our IP address is 192.168.10.2
> Filename 'mpc8641_hpcn.dtb'.
> Load address: 0x14000000
> Loading: * #
>      2.6 MiB/s
> done
> Bytes transferred = 5540 (15a4 hex)
> ## Booting kernel from Legacy Image at 16000000 ...
>    Image Name:   Linux-3.13.6
>    Image Type:   PowerPC Linux Kernel Image (gzip compressed)
>    Data Size:    2476240 Bytes = 2.4 MiB
>    Load Address: 00000000
>    Entry Point:  00000000
>    Verifying Checksum ... OK
> ## Loading init Ramdisk from Legacy Image at 00600000 ...
>    Image Name:   rootfs
>    Image Type:   PowerPC Linux RAMDisk Image (gzip compressed)
>    Data Size:    22680124 Bytes = 21.6 MiB
>    Load Address: 00000000
>    Entry Point:  00000000
>    Verifying Checksum ... OK
> ## Flattened Device Tree blob at 14000000
>    Booting using the fdt blob at 0x14000000
>    Uncompressing Kernel Image ... OK
>    Loading Ramdisk to 1e923000, end 1fec423c ... OK
>    Loading Device Tree to 007fb000, end 007ff5a3 ... OK    *(some times
> it stucking here) *
> Using MPC86xx HPCN machine description
> Total memory = 512MB; using 1024kB for hash table (at cff00000)
> Linux version 3.13.6 (ashish@ashish-VirtualBox) (gcc version 4.7.2 (GCC)
> ) #8 Sat Mar 29 10:31:58 IST 2014
> Found initrd at 0xde923000:0xdfec423c
> bootconsole [udbg0] enabled
> setup_arch: bootmem
> mpc86xx_hpcn_setup_arch()
> MPC86xx HPCN board from Freescale Semiconductor
> arch: exit
> Zone ranges:
>   DMA      [mem 0x00000000-0x1fffffff]
>   Normal   empty
>   HighMem  empty
> Movable zone start for each node
> Early memory node ranges
>   node   0: [mem 0x00000000-0x1fffffff]         *(some times here) *
>
>
>

[-- Attachment #2: Type: text/html, Size: 9351 bytes --]

^ permalink raw reply

* Re: [PATCH] PPC: BOOK3S: Disable/Enable TM looking at the ibm, pa-features device tree entry
From: Aneesh Kumar K.V @ 2014-05-02  5:17 UTC (permalink / raw)
  To: Michael Neuling; +Cc: paulus, linuxppc-dev
In-Reply-To: <87r44dbkg4.fsf@linux.vnet.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> Michael Neuling <mikey@neuling.org> writes:
>
>> Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>
>>> Runtime disable transactional memory feature looking at pa-features
>>> device tree entry. This provides a mechanism to disable TM on P8
>>> systems.
>>
>> What are we actually achieving with this?
>
> PAPR compliance  :) ? Also I wanted to disable guest kernel from doing
> TM related save restore. Guest kernel already look at the cpu feature
> before doing that. Hence needed a mechanism to disable the feature. 
>
> Things like
>
> static inline void __switch_to_tm(struct task_struct *prev)
> {
> 	if (cpu_has_feature(CPU_FTR_TM)) {
> 		tm_enable();
> 		tm_reclaim_task(prev);
> 	}
> }
>
>
>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>>  arch/powerpc/kernel/prom.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>>> index 668aa4791fd7..537bd7e7db0b 100644
>>> --- a/arch/powerpc/kernel/prom.c
>>> +++ b/arch/powerpc/kernel/prom.c
>>> @@ -161,6 +161,11 @@ static struct ibm_pa_feature {
>>>  	{CPU_FTR_NODSISRALIGN, 0, 0,	1, 1, 1},
>>>  	{0, MMU_FTR_CI_LARGE_PAGE, 0,	1, 2, 0},
>>>  	{CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
>>> +	/*
>>> +	 * We should use CPU_FTR_TM_COMP so that if we disable TM, it won't get
>>> +	 * enabled via device tree
>>> +	 */
>>> +	{CPU_FTR_TM_COMP, 0, 0,		22, 0, 0},
>>
>> What does this do to guests?  Will it turn TM unavailable into an
>> illegal instruction?
>>
>
> Good suggestion. I guess it should be facility unavailable interrupt ?
> I should also make the sure __init_HFSCR only set HFSCR_TM only if the
> cpu feature is enabled ?

I looked at this and I guess we don't need to update HFSCR considering
that the guest kernel (privileged) access to TM always happen within
if (cpu_has_feature(CPU_FTR_TM)) conditional block. Also we want to
disable this per guest and there is no easy way to suggest hypervisor
that disable TM in HFSCR.

BTW we already do this for guest problme state. We do in guest kernel

	if (cpu_has_feature(CPU_FTR_TM))
		regs->msr |= MSR_TM;

IIUC that should result in facility unavailable interrupt when problem
state try to access TM ?

I will try to run some test with the patch and update here.

-aneesh

^ permalink raw reply

* Re: [PATCH 1/1] powerpc: crtsaveres.o needed only when -Os flag is enabled
From: Aneesh Kumar K.V @ 2014-05-02  5:39 UTC (permalink / raw)
  To: Ram Pai, benh, paulus; +Cc: linuxppc-dev, Ram Pai, anton, tony, hartb
In-Reply-To: <1398729908-15787-1-git-send-email-linuxram@us.ibm.com>

Ram Pai <linuxram@us.ibm.com> writes:

>     powerpc: crtsaveres.o needed only when -Os flag is enabled
>     
>     Currently on powerpc arch, out-of-tree module fails to build without
>     crtsaveres.o, even when the module has no dependency on the symbols
>     provided by the file; when built without the -Os flag.
>     
>     BTW: '-Os' flag is enabled when CONFIG_CC_OPTIMIZE_FOR_SIZE is
>     configured.
>     
>     This patch fixes that problem.
>     
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 4c0cedf..cf12f38 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -157,7 +157,10 @@ CPP		= $(CC) -E $(KBUILD_CFLAGS)
>  
>  CHECKFLAGS	+= -m$(CONFIG_WORD_SIZE) -D__powerpc__ -D__powerpc$(CONFIG_WORD_SIZE)__
>  
> +ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
>  KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
> +endif
> +
>  
>  # No AltiVec or VSX instructions when building kernel
>  KBUILD_CFLAGS += $(call cc-option,-mno-altivec)
>

So if we enable CONFIG_CC_OPTIMIZE_FOR_SIZE can we build out-of-tree
module with this patch ? 

-aneesh

^ permalink raw reply

* [PATCH] IBM Akebono: Remove obsolete config select
From: Alistair Popple @ 2014-05-02  8:06 UTC (permalink / raw)
  To: benh; +Cc: devicetree, pebolle, linuxppc-dev, linux-kernel, Alistair Popple
In-Reply-To: <1398998002.2925.15.camel@pasglop>

The original implementation of MMC support for Akebono introduced a
new configuration symbol (MMC_SDHCI_OF_476GTR). This symbol has been
dropped in favour of using the generic platform driver however the
select for this symbol was mistakenly left in the platform
configuration.

This patch removes the obsolete symbol selection.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 arch/powerpc/platforms/44x/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 8beec7d..908bf11 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -220,7 +220,6 @@ config AKEBONO
 	select USB_EHCI_HCD_PLATFORM
 	select MMC_SDHCI
 	select MMC_SDHCI_PLTFM
-	select MMC_SDHCI_OF_476GTR
 	select ATA
 	select SATA_AHCI_PLATFORM
 	help
-- 
1.8.3.2

^ permalink raw reply related

* Re: [PATCH v2 1/4] KVM: PPC: e500mc: Revert "add load inst fixup"
From: Alexander Graf @ 2014-05-02  9:24 UTC (permalink / raw)
  To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1398905152-18091-2-git-send-email-mihai.caraman@freescale.com>

On 05/01/2014 02:45 AM, Mihai Caraman wrote:
> The commit 1d628af7 "add load inst fixup" made an attempt to handle
> failures generated by reading the guest current instruction. The fixup
> code that was added works by chance hiding the real issue.
>
> Load external pid (lwepx) instruction, used by KVM to read guest
> instructions, is executed in a subsituted guest translation context
> (EPLC[EGS] = 1). In consequence lwepx's TLB error and data storage
> interrupts need to be handled by KVM, even though these interrupts
> are generated from host context (MSR[GS] = 0).
>
> Currently, KVM hooks only interrupts generated from guest context
> (MSR[GS] = 1), doing minimal checks on the fast path to avoid host
> performance degradation. As a result, the host kernel handles lwepx
> faults searching the faulting guest data address (loaded in DEAR) in
> its own Logical Partition ID (LPID) 0 context. In case a host translation
> is found the execution returns to the lwepx instruction instead of the
> fixup, the host ending up in an infinite loop.
>
> Revert the commit "add load inst fixup". lwepx issue will be addressed
> in a subsequent patch without needing fixup code.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>

Just a random idea: Could we just switch IVOR2 during the critical lwepx 
phase? In fact, we could even do that later when we're already in C code 
and try to recover the last instruction. The code IVOR2 would point to 
would simply set the register we're trying to read to as LAST_INST_FAIL 
and rfi.


Alex

^ permalink raw reply

* Re: [PATCH v2 3/4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-05-02  9:54 UTC (permalink / raw)
  To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1398905152-18091-4-git-send-email-mihai.caraman@freescale.com>

On 05/01/2014 02:45 AM, Mihai Caraman wrote:
> On book3e, guest last instruction was read on the exist path using load
> external pid (lwepx) dedicated instruction. lwepx failures have to be
> handled by KVM and this would require additional checks in DO_KVM hooks
> (beside MSR[GS] = 1). However extra checks on host fast path are commonly
> considered too intrusive.
>
> This patch lay down the path for an altenative solution, by allowing
> kvmppc_get_lat_inst() function to fail. Booke architectures may choose
> to read guest instruction from kvmppc_ld_inst() and to instruct the
> emulation layer either to fail or to emulate again.
>
> emulation_result enum defintion is not accesible from book3 asm headers.
> Move kvmppc_get_last_inst() definitions that require emulation_result
> to source files.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v2:
>   - integrated kvmppc_get_last_inst() in book3s code and checked build
>   - addressed cosmetic feedback
>   - please validate book3s functionality!
>
>   arch/powerpc/include/asm/kvm_book3s.h    | 26 --------------------
>   arch/powerpc/include/asm/kvm_booke.h     |  5 ----
>   arch/powerpc/include/asm/kvm_ppc.h       |  2 ++
>   arch/powerpc/kvm/book3s.c                | 32 ++++++++++++++++++++++++
>   arch/powerpc/kvm/book3s.h                |  1 +
>   arch/powerpc/kvm/book3s_64_mmu_hv.c      | 16 +++---------
>   arch/powerpc/kvm/book3s_paired_singles.c | 42 ++++++++++++++++++++------------
>   arch/powerpc/kvm/book3s_pr.c             | 36 +++++++++++++++++----------
>   arch/powerpc/kvm/booke.c                 | 14 +++++++++++
>   arch/powerpc/kvm/booke.h                 |  3 +++
>   arch/powerpc/kvm/e500_mmu_host.c         |  7 ++++++
>   arch/powerpc/kvm/emulate.c               | 18 +++++++++-----
>   arch/powerpc/kvm/powerpc.c               | 10 ++++++--
>   13 files changed, 132 insertions(+), 80 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index bb1e38a..e2a89f3 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -273,32 +273,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
>   	return (vcpu->arch.shared->msr & MSR_LE) != (MSR_KERNEL & MSR_LE);
>   }
>   
> -static inline u32 kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc)
> -{
> -	/* Load the instruction manually if it failed to do so in the
> -	 * exit path */
> -	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> -		kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst, false);
> -
> -	return kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> -		vcpu->arch.last_inst;
> -}
> -
> -static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
> -{
> -	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu));
> -}
> -
> -/*
> - * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
> - * Because the sc instruction sets SRR0 to point to the following
> - * instruction, we have to fetch from pc - 4.
> - */
> -static inline u32 kvmppc_get_last_sc(struct kvm_vcpu *vcpu)
> -{
> -	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4);
> -}
> -
>   static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
>   {
>   	return vcpu->arch.fault_dar;
> diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h
> index 80d46b5..6db1ca0 100644
> --- a/arch/powerpc/include/asm/kvm_booke.h
> +++ b/arch/powerpc/include/asm/kvm_booke.h
> @@ -69,11 +69,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
>   	return false;
>   }
>   
> -static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
> -{
> -	return vcpu->arch.last_inst;
> -}
> -
>   static inline void kvmppc_set_ctr(struct kvm_vcpu *vcpu, ulong val)
>   {
>   	vcpu->arch.ctr = val;
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 4096f16..6e7c358 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -72,6 +72,8 @@ extern int kvmppc_sanity_check(struct kvm_vcpu *vcpu);
>   extern int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu);
>   extern void kvmppc_subarch_vcpu_uninit(struct kvm_vcpu *vcpu);
>   
> +extern int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst);

Phew. Moving this into a separate function sure has some performance 
implications. Was there no way to keep it in a header?

You could just move it into its own .h file which we include after 
kvm_ppc.h. That way everything's available. That would also help me a 
lot with the little endian port where I'm also struggling with header 
file inclusion order and kvmppc_need_byteswap().

> +
>   /* Core-specific hooks */
>   
>   extern void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 gvaddr, gpa_t gpaddr,
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 94e597e..3553735 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -463,6 +463,38 @@ mmio:
>   }
>   EXPORT_SYMBOL_GPL(kvmppc_ld);
>   
> +int kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc, u32 *inst)
> +{
> +	int ret = EMULATE_DONE;
> +
> +	/* Load the instruction manually if it failed to do so in the
> +	 * exit path */
> +	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> +		ret = kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst,
> +			false);
> +
> +	*inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> +		vcpu->arch.last_inst;
> +
> +	return ret;
> +}
> +
> +int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst)
> +{
> +	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu), inst);
> +}
> +
> +/*
> + * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
> + * Because the sc instruction sets SRR0 to point to the following
> + * instruction, we have to fetch from pc - 4.
> + */
> +int kvmppc_get_last_sc(struct kvm_vcpu *vcpu, u32 *inst)
> +{
> +	return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4,
> +		inst);
> +}
> +
>   int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>   {
>   	return 0;
> diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
> index 4bf956c..d85839d 100644
> --- a/arch/powerpc/kvm/book3s.h
> +++ b/arch/powerpc/kvm/book3s.h
> @@ -30,5 +30,6 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu,
>   					int sprn, ulong *spr_val);
>   extern int kvmppc_book3s_init_pr(void);
>   extern void kvmppc_book3s_exit_pr(void);
> +extern int kvmppc_get_last_sc(struct kvm_vcpu *vcpu, u32 *inst);
>   
>   #endif
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index fb25ebc..7ffcc24 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -541,21 +541,13 @@ static int instruction_is_store(unsigned int instr)
>   static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
>   				  unsigned long gpa, gva_t ea, int is_store)
>   {
> -	int ret;
>   	u32 last_inst;
> -	unsigned long srr0 = kvmppc_get_pc(vcpu);
>   
> -	/* We try to load the last instruction.  We don't let
> -	 * emulate_instruction do it as it doesn't check what
> -	 * kvmppc_ld returns.
> +	/*
>   	 * If we fail, we just return to the guest and try executing it again.
>   	 */
> -	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED) {
> -		ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> -		if (ret != EMULATE_DONE || last_inst == KVM_INST_FETCH_FAILED)
> -			return RESUME_GUEST;
> -		vcpu->arch.last_inst = last_inst;
> -	}
> +	if (kvmppc_get_last_inst(vcpu, &last_inst) != EMULATE_DONE)
> +		return RESUME_GUEST;
>   
>   	/*
>   	 * WARNING: We do not know for sure whether the instruction we just
> @@ -569,7 +561,7 @@ static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
>   	 * we just return and retry the instruction.
>   	 */
>   
> -	if (instruction_is_store(kvmppc_get_last_inst(vcpu)) != !!is_store)
> +	if (instruction_is_store(last_inst) != !!is_store)
>   		return RESUME_GUEST;
>   
>   	/*
> diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
> index c1abd95..9a6e565 100644
> --- a/arch/powerpc/kvm/book3s_paired_singles.c
> +++ b/arch/powerpc/kvm/book3s_paired_singles.c
> @@ -637,26 +637,36 @@ static int kvmppc_ps_one_in(struct kvm_vcpu *vcpu, bool rc,
>   
>   int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu)
>   {
> -	u32 inst = kvmppc_get_last_inst(vcpu);
> -	enum emulation_result emulated = EMULATE_DONE;
> -
> -	int ax_rd = inst_get_field(inst, 6, 10);
> -	int ax_ra = inst_get_field(inst, 11, 15);
> -	int ax_rb = inst_get_field(inst, 16, 20);
> -	int ax_rc = inst_get_field(inst, 21, 25);
> -	short full_d = inst_get_field(inst, 16, 31);
> -
> -	u64 *fpr_d = &VCPU_FPR(vcpu, ax_rd);
> -	u64 *fpr_a = &VCPU_FPR(vcpu, ax_ra);
> -	u64 *fpr_b = &VCPU_FPR(vcpu, ax_rb);
> -	u64 *fpr_c = &VCPU_FPR(vcpu, ax_rc);
> -
> -	bool rcomp = (inst & 1) ? true : false;
> -	u32 cr = kvmppc_get_cr(vcpu);
> +	u32 inst;
> +	enum emulation_result emulated;
> +	int ax_rd, ax_ra, ax_rb, ax_rc;
> +	short full_d;
> +	u64 *fpr_d, *fpr_a, *fpr_b, *fpr_c;
> +
> +	bool rcomp;
> +	u32 cr;
>   #ifdef DEBUG
>   	int i;
>   #endif
>   
> +	emulated = kvmppc_get_last_inst(vcpu, &inst);
> +	if (emulated != EMULATE_DONE)
> +		return emulated;
> +
> +	ax_rd = inst_get_field(inst, 6, 10);
> +	ax_ra = inst_get_field(inst, 11, 15);
> +	ax_rb = inst_get_field(inst, 16, 20);
> +	ax_rc = inst_get_field(inst, 21, 25);
> +	full_d = inst_get_field(inst, 16, 31);
> +
> +	fpr_d = &VCPU_FPR(vcpu, ax_rd);
> +	fpr_a = &VCPU_FPR(vcpu, ax_ra);
> +	fpr_b = &VCPU_FPR(vcpu, ax_rb);
> +	fpr_c = &VCPU_FPR(vcpu, ax_rc);
> +
> +	rcomp = (inst & 1) ? true : false;
> +	cr = kvmppc_get_cr(vcpu);
> +
>   	if (!kvmppc_inst_is_paired_single(vcpu, inst))
>   		return EMULATE_FAIL;
>   
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index c5c052a..b7fffd1 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -608,12 +608,9 @@ void kvmppc_giveup_ext(struct kvm_vcpu *vcpu, ulong msr)
>   
>   static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
>   {
> -	ulong srr0 = kvmppc_get_pc(vcpu);
> -	u32 last_inst = kvmppc_get_last_inst(vcpu);
> -	int ret;
> +	u32 last_inst;
>   
> -	ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> -	if (ret == -ENOENT) {
> +	if (kvmppc_get_last_inst(vcpu, &last_inst) == -ENOENT) {

ENOENT?

>   		ulong msr = vcpu->arch.shared->msr;
>   
>   		msr = kvmppc_set_field(msr, 33, 33, 1);
> @@ -867,15 +864,18 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
>   	{
>   		enum emulation_result er;
>   		ulong flags;
> +		u32 last_inst;
>   
>   program_interrupt:
>   		flags = vcpu->arch.shadow_srr1 & 0x1f0000ull;
> +		kvmppc_get_last_inst(vcpu, &last_inst);

No check for the return value?

>   
>   		if (vcpu->arch.shared->msr & MSR_PR) {
>   #ifdef EXIT_DEBUG
> -			printk(KERN_INFO "Userspace triggered 0x700 exception at 0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> +			pr_info("Userspace triggered 0x700 exception at\n"
> +			    "0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), last_inst);
>   #endif
> -			if ((kvmppc_get_last_inst(vcpu) & 0xff0007ff) !=
> +			if ((last_inst & 0xff0007ff) !=
>   			    (INS_DCBZ & 0xfffffff7)) {
>   				kvmppc_core_queue_program(vcpu, flags);
>   				r = RESUME_GUEST;
> @@ -894,7 +894,7 @@ program_interrupt:
>   			break;
>   		case EMULATE_FAIL:
>   			printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
> -			       __func__, kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> +			       __func__, kvmppc_get_pc(vcpu), last_inst);
>   			kvmppc_core_queue_program(vcpu, flags);
>   			r = RESUME_GUEST;
>   			break;
> @@ -911,8 +911,12 @@ program_interrupt:
>   		break;
>   	}
>   	case BOOK3S_INTERRUPT_SYSCALL:
> +	{
> +		u32 last_sc;
> +
> +		kvmppc_get_last_sc(vcpu, &last_sc);

No check for the return value?

>   		if (vcpu->arch.papr_enabled &&
> -		    (kvmppc_get_last_sc(vcpu) == 0x44000022) &&
> +		    (last_sc == 0x44000022) &&
>   		    !(vcpu->arch.shared->msr & MSR_PR)) {
>   			/* SC 1 papr hypercalls */
>   			ulong cmd = kvmppc_get_gpr(vcpu, 3);
> @@ -957,6 +961,7 @@ program_interrupt:
>   			r = RESUME_GUEST;
>   		}
>   		break;
> +	}
>   	case BOOK3S_INTERRUPT_FP_UNAVAIL:
>   	case BOOK3S_INTERRUPT_ALTIVEC:
>   	case BOOK3S_INTERRUPT_VSX:
> @@ -985,15 +990,20 @@ program_interrupt:
>   		break;
>   	}
>   	case BOOK3S_INTERRUPT_ALIGNMENT:
> +	{
> +		u32 last_inst;
> +
>   		if (kvmppc_read_inst(vcpu) == EMULATE_DONE) {
> -			vcpu->arch.shared->dsisr = kvmppc_alignment_dsisr(vcpu,
> -				kvmppc_get_last_inst(vcpu));
> -			vcpu->arch.shared->dar = kvmppc_alignment_dar(vcpu,
> -				kvmppc_get_last_inst(vcpu));
> +			kvmppc_get_last_inst(vcpu, &last_inst);

I think with an error returning kvmppc_get_last_inst we can just use 
completely get rid of kvmppc_read_inst() and only use 
kvmppc_get_last_inst() instead.

> +			vcpu->arch.shared->dsisr =
> +				kvmppc_alignment_dsisr(vcpu, last_inst);
> +			vcpu->arch.shared->dar =
> +				kvmppc_alignment_dar(vcpu, last_inst);
>   			kvmppc_book3s_queue_irqprio(vcpu, exit_nr);
>   		}
>   		r = RESUME_GUEST;
>   		break;
> +	}
>   	case BOOK3S_INTERRUPT_MACHINE_CHECK:
>   	case BOOK3S_INTERRUPT_TRACE:
>   		kvmppc_book3s_queue_irqprio(vcpu, exit_nr);
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index ab62109..df25db0 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -752,6 +752,9 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
>   		 * they were actually modified by emulation. */
>   		return RESUME_GUEST_NV;
>   
> +	case EMULATE_AGAIN:
> +		return RESUME_GUEST;
> +
>   	case EMULATE_DO_DCR:
>   		run->exit_reason = KVM_EXIT_DCR;
>   		return RESUME_HOST;
> @@ -1911,6 +1914,17 @@ void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu)
>   	vcpu->kvm->arch.kvm_ops->vcpu_put(vcpu);
>   }
>   
> +int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, u32 *inst)
> +{
> +	int result = EMULATE_DONE;
> +
> +	if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> +		result = kvmppc_ld_inst(vcpu, &vcpu->arch.last_inst);
> +	*inst = vcpu->arch.last_inst;

This function looks almost identical to the book3s one. Can we share the 
same code path here? Just always return false for 
kvmppc_needs_byteswap() on booke.


Alex

^ permalink raw reply

* Re: [PATCH v2 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-05-02 10:01 UTC (permalink / raw)
  To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1398905152-18091-5-git-send-email-mihai.caraman@freescale.com>

On 05/01/2014 02:45 AM, Mihai Caraman wrote:
> On bookehv vcpu's last instruction is read using load external pid
> (lwepx) instruction. lwepx exceptions (DTLB_MISS, DSI and LRAT) need
> to be handled by KVM. These exceptions originate from host state
> (MSR[GS] = 0) which implies additional checks in DO_KVM macro (beside
> the current MSR[GS] = 1) by looking at the Exception Syndrome Register
> (ESR[EPID]) and the External PID Load Context Register (EPLC[EGS]).
> Doing this on each Data TLB miss exception is obvious too intrusive
> for the host.
>
> Get rid of lwepx and acquire last instuction in kvmppc_get_last_inst()
> by searching for the physical address and kmap it. This fixes an
> infinite loop caused by lwepx's data TLB miss handled in the host
> and the TODO for TLB eviction and execute-but-not-read entries.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v2:
>   - reworked patch description
>   - used pr_* functions
>   - addressed cosmetic feedback
>
>   arch/powerpc/kvm/bookehv_interrupts.S | 37 ++++----------
>   arch/powerpc/kvm/e500_mmu_host.c      | 93 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 101 insertions(+), 29 deletions(-)
>
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index 925da71..65eff4c 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -122,38 +122,14 @@
>   1:
>   
>   	.if	\flags & NEED_EMU
> -	/*
> -	 * This assumes you have external PID support.
> -	 * To support a bookehv CPU without external PID, you'll
> -	 * need to look up the TLB entry and create a temporary mapping.
> -	 *
> -	 * FIXME: we don't currently handle if the lwepx faults.  PR-mode
> -	 * booke doesn't handle it either.  Since Linux doesn't use
> -	 * broadcast tlbivax anymore, the only way this should happen is
> -	 * if the guest maps its memory execute-but-not-read, or if we
> -	 * somehow take a TLB miss in the middle of this entry code and
> -	 * evict the relevant entry.  On e500mc, all kernel lowmem is
> -	 * bolted into TLB1 large page mappings, and we don't use
> -	 * broadcast invalidates, so we should not take a TLB miss here.
> -	 *
> -	 * Later we'll need to deal with faults here.  Disallowing guest
> -	 * mappings that are execute-but-not-read could be an option on
> -	 * e500mc, but not on chips with an LRAT if it is used.
> -	 */
> -
> -	mfspr	r3, SPRN_EPLC	/* will already have correct ELPID and EGS */
>   	PPC_STL	r15, VCPU_GPR(R15)(r4)
>   	PPC_STL	r16, VCPU_GPR(R16)(r4)
>   	PPC_STL	r17, VCPU_GPR(R17)(r4)
>   	PPC_STL	r18, VCPU_GPR(R18)(r4)
>   	PPC_STL	r19, VCPU_GPR(R19)(r4)
> -	mr	r8, r3
>   	PPC_STL	r20, VCPU_GPR(R20)(r4)
> -	rlwimi	r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
>   	PPC_STL	r21, VCPU_GPR(R21)(r4)
> -	rlwimi	r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
>   	PPC_STL	r22, VCPU_GPR(R22)(r4)
> -	rlwimi	r8, r10, EPC_EPID_SHIFT, EPC_EPID
>   	PPC_STL	r23, VCPU_GPR(R23)(r4)
>   	PPC_STL	r24, VCPU_GPR(R24)(r4)
>   	PPC_STL	r25, VCPU_GPR(R25)(r4)
> @@ -163,10 +139,15 @@
>   	PPC_STL	r29, VCPU_GPR(R29)(r4)
>   	PPC_STL	r30, VCPU_GPR(R30)(r4)
>   	PPC_STL	r31, VCPU_GPR(R31)(r4)
> -	mtspr	SPRN_EPLC, r8
> -	isync
> -	lwepx   r9, 0, r5
> -	mtspr	SPRN_EPLC, r3
> +
> +	/*
> +	 * We don't use external PID support. lwepx faults would need to be
> +	 * handled by KVM and this implies aditional code in DO_KVM (for
> +	 * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
> +	 * is too intrusive for the host. Get last instuction in
> +	 * kvmppc_get_last_inst().
> +	 */
> +	li	r9, KVM_INST_FETCH_FAILED
>   	stw	r9, VCPU_LAST_INST(r4)
>   	.endif
>   
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index fcccbb3..94b8be0 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -607,11 +607,102 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
>   	}
>   }
>   
> +#ifdef CONFIG_KVM_BOOKE_HV
> +int kvmppc_ld_inst(struct kvm_vcpu *vcpu, u32 *instr)
> +{
> +	gva_t geaddr;
> +	hpa_t addr;
> +	hfn_t pfn;
> +	hva_t eaddr;
> +	u32 mas0, mas1, mas2, mas3;
> +	u64 mas7_mas3;
> +	struct page *page;
> +	unsigned int addr_space, psize_shift;
> +	bool pr;
> +	unsigned long flags;
> +
> +	/* Search TLB for guest pc to get the real address */
> +	geaddr = kvmppc_get_pc(vcpu);
> +	addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
> +
> +	local_irq_save(flags);
> +	mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
> +	mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
> +	asm volatile("tlbsx 0, %[geaddr]\n" : :
> +		     [geaddr] "r" (geaddr));
> +	mtspr(SPRN_MAS5, 0);
> +	mtspr(SPRN_MAS8, 0);
> +	mas0 = mfspr(SPRN_MAS0);
> +	mas1 = mfspr(SPRN_MAS1);
> +	mas2 = mfspr(SPRN_MAS2);
> +	mas3 = mfspr(SPRN_MAS3);
> +	mas7_mas3 = (((u64) mfspr(SPRN_MAS7)) << 32) | mas3;
> +	local_irq_restore(flags);
> +
> +	/*
> +	 * If the TLB entry for guest pc was evicted, return to the guest.
> +	 * There are high chances to find a valid TLB entry next time.
> +	 */
> +	if (!(mas1 & MAS1_VALID))
> +		return EMULATE_AGAIN;
>   
> +	/*
> +	 * Another thread may rewrite the TLB entry in parallel, don't
> +	 * execute from the address if the execute permission is not set
> +	 */
> +	pr = vcpu->arch.shared->msr & MSR_PR;
> +	if ((pr && !(mas3 & MAS3_UX)) || (!pr && !(mas3 & MAS3_SX))) {
> +		pr_debug("Instuction emulation from a guest page\n"
> +				"withot execute permission\n");
> +		kvmppc_core_queue_program(vcpu, 0);
> +		return EMULATE_AGAIN;
> +	}
> +
> +	/*
> +	 * We will map the real address through a cacheable page, so we will
> +	 * not support cache-inhibited guest pages. Fortunately emulated
> +	 * instructions should not live there.
> +	 */
> +	if (mas2 & MAS2_I) {
> +		pr_debug("Instuction emulation from cache-inhibited\n"
> +				"guest pages is not supported\n");
> +		return EMULATE_FAIL;
> +	}
> +
> +	/* Get page size */
> +	psize_shift = MAS1_GET_TSIZE(mas1) + 10;
> +
> +	/* Map a page and get guest's instruction */
> +	addr = (mas7_mas3 & (~0ULL << psize_shift)) |
> +	       (geaddr & ((1ULL << psize_shift) - 1ULL));
> +	pfn = addr >> PAGE_SHIFT;
> +
> +	/* Guard us against emulation from devices area */
> +	if (unlikely(!page_is_ram(pfn))) {
> +		pr_debug("Instruction emulation from non-RAM host\n"
> +				"pages is not supported\n");
> +		return EMULATE_FAIL;
> +	}
> +
> +	if (unlikely(!pfn_valid(pfn))) {
> +		pr_debug("Invalid frame number\n");
> +		return EMULATE_FAIL;
> +	}
> +
> +	page = pfn_to_page(pfn);
> +	eaddr = (unsigned long)kmap_atomic(page);
> +	eaddr |= addr & ~PAGE_MASK;
> +	*instr = *(u32 *)eaddr;
> +	kunmap_atomic((u32 *)eaddr);

I think I'd rather write this as

   *instr = *(u32 *)(eaddr | (addr & ~PAGE));
   kunmap_atomic((void*)eaddr);

to make sure we pass the unmap function the same value we got from the 
map function.

Otherwise looks good to me.


Alex

^ permalink raw reply

* RE: [PATCH v2 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: David Laight @ 2014-05-02 10:12 UTC (permalink / raw)
  To: 'Alexander Graf', Mihai Caraman
  Cc: linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org,
	kvm@vger.kernel.org
In-Reply-To: <53636CFA.5050006@suse.de>

RnJvbTogQWxleGFuZGVyIEdyYWYNCi4uLg0KPiA+ICsJcGFnZSA9IHBmbl90b19wYWdlKHBmbik7
DQo+ID4gKwllYWRkciA9ICh1bnNpZ25lZCBsb25nKWttYXBfYXRvbWljKHBhZ2UpOw0KPiA+ICsJ
ZWFkZHIgfD0gYWRkciAmIH5QQUdFX01BU0s7DQo+ID4gKwkqaW5zdHIgPSAqKHUzMiAqKWVhZGRy
Ow0KPiA+ICsJa3VubWFwX2F0b21pYygodTMyICopZWFkZHIpOw0KPiANCj4gSSB0aGluayBJJ2Qg
cmF0aGVyIHdyaXRlIHRoaXMgYXMNCj4gDQo+ICAgICppbnN0ciA9ICoodTMyICopKGVhZGRyIHwg
KGFkZHIgJiB+UEFHRSkpOw0KPiAgICBrdW5tYXBfYXRvbWljKCh2b2lkKillYWRkcik7DQo+IA0K
PiB0byBtYWtlIHN1cmUgd2UgcGFzcyB0aGUgdW5tYXAgZnVuY3Rpb24gdGhlIHNhbWUgdmFsdWUg
d2UgZ290IGZyb20gdGhlDQo+IG1hcCBmdW5jdGlvbi4NCj4gDQo+IE90aGVyd2lzZSBsb29rcyBn
b29kIHRvIG1lLg0KDQpJcyB0aGVyZSBhbnkgbWlsZWFnZSBpbiBrZWVwaW5nIGEgdmlydHVhbCBh
ZGRyZXNzIHBhZ2UgYWxsb2NhdGVkIChwZXIgY3B1KQ0KZm9yIHRoaXMgKGFuZCBzaW1pbGFyKSBh
Y2Nlc3NlcyB0byBwaHlzaWNhbCBtZW1vcnk/DQpOb3QgaGF2aW5nIHRvIHNlYXJjaCBmb3IgYSBm
cmVlIFZBIHBhZ2UgbWlnaHQgc3BlZWQgdGhpbmdzIHVwIChpZiB0aGF0IG1hdHRlcnMpLg0KDQpZ
b3UgYWxzbyBwcm9iYWJseSB3YW50IHRoZSBwYWdlIG1hcHBlZCB1bmNhY2hlZCAtIG5vIHBvaW50
IHBvbGx1dGluZyB0aGUgZGF0YQ0KY2FjaGUuDQoNCglEYXZpZA0KDQo=

^ permalink raw reply

* [PATCH] powerpc: Remove non-uapi linkage.h export
From: James Hogan @ 2014-05-02 10:20 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras
  Cc: David Howells, David Woodhouse, James Hogan, linuxppc-dev,
	Al Viro

The arch/powerpc/include/asm/linkage.h is being unintentionally exported
in the kernel headers since commit e1b5bb6d1236 (consolidate
cond_syscall and SYSCALL_ALIAS declarations) when
arch/powerpc/include/uapi/asm/linkage.h was deleted but the header-y not
removed from the Kbuild file. This happens because Makefile.headersinst
still checks the old asm/ directory if the specified header doesn't
exist in the uapi directory.

The asm/linkage.h shouldn't ever have been exported anyway. No other
arch does and it doesn't contain anything useful to userland, so remove
the header-y line from the Kbuild file which triggers the export.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: David Howells <dhowells@redhat.com>
---
 arch/powerpc/include/uapi/asm/Kbuild | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/include/uapi/asm/Kbuild b/arch/powerpc/include/uapi/asm/Kbuild
index 48be855ef37b..7a3f795ac218 100644
--- a/arch/powerpc/include/uapi/asm/Kbuild
+++ b/arch/powerpc/include/uapi/asm/Kbuild
@@ -15,7 +15,6 @@ header-y += ioctls.h
 header-y += ipcbuf.h
 header-y += kvm.h
 header-y += kvm_para.h
-header-y += linkage.h
 header-y += mman.h
 header-y += msgbuf.h
 header-y += nvram.h
-- 
1.9.2

^ permalink raw reply related

* Re: [PATCH v2 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-05-02 11:10 UTC (permalink / raw)
  To: David Laight
  Cc: Mihai Caraman, linuxppc-dev@lists.ozlabs.org,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F705051@AcuExch.aculab.com>

On 05/02/2014 12:12 PM, David Laight wrote:
> From: Alexander Graf
> ...
>>> +	page = pfn_to_page(pfn);
>>> +	eaddr = (unsigned long)kmap_atomic(page);
>>> +	eaddr |= addr & ~PAGE_MASK;
>>> +	*instr = *(u32 *)eaddr;
>>> +	kunmap_atomic((u32 *)eaddr);
>> I think I'd rather write this as
>>
>>     *instr = *(u32 *)(eaddr | (addr & ~PAGE));
>>     kunmap_atomic((void*)eaddr);
>>
>> to make sure we pass the unmap function the same value we got from the
>> map function.
>>
>> Otherwise looks good to me.
> Is there any mileage in keeping a virtual address page allocated (per cpu)
> for this (and similar) accesses to physical memory?
> Not having to search for a free VA page might speed things up (if that matters).

I like the idea, though I'm not sure how that would best fit into the 
current memory mapping ecosystem.

> You also probably want the page mapped uncached - no point polluting the data
> cache.

Do e500 chips have a shared I/D cache somewhere? If they do, that 
particular instruction would already be there, so no pollution but nice 
performance.


Alex

^ permalink raw reply

* Re: [PATCH v2 4/4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Scott Wood @ 2014-05-02 15:32 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Mihai Caraman, David Laight, linuxppc-dev@lists.ozlabs.org,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
In-Reply-To: <53637D3B.4010205@suse.de>

On Fri, 2014-05-02 at 13:10 +0200, Alexander Graf wrote:
> On 05/02/2014 12:12 PM, David Laight wrote:
> > You also probably want the page mapped uncached - no point polluting the data
> > cache.

We can't do that without creating an architecturally illegal alias
between cacheable and non-cacheable mappings.

> Do e500 chips have a shared I/D cache somewhere?

Yes.  Only L1 is separate.

-Scott

^ permalink raw reply

* Re: Build regressions/improvements in v3.15-rc3
From: Geert Uytterhoeven @ 2014-05-02 15:44 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org; +Cc: linuxppc-dev@lists.ozlabs.org, uml-devel
In-Reply-To: <1399045268-18344-1-git-send-email-geert@linux-m68k.org>

On Fri, May 2, 2014 at 5:41 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> JFYI, when comparing v3.15-rc3[1]  to v3.15-rc2[3], the summaries are:
>   - build errors: +5/-9

  + /scratch/kisskb/src/arch/um/os-Linux/mem.c: error: 'TMPFS_MAGIC'
undeclared (first use in this function):  => 31
  + /scratch/kisskb/src/arch/um/os-Linux/mem.c: error: (Each
undeclared identifier is reported only once:  => 31
  + /scratch/kisskb/src/arch/um/os-Linux/mem.c: error: for each
function it appears in.):  => 31
  + /scratch/kisskb/src/arch/um/os-Linux/mem.c: error: linux/magic.h:
No such file or directory:  => 16:25

um-defconfig

  + error: No rule to make target include/config/auto.conf:  => N/A

powerpc-randconfig

> [1] http://kisskb.ellerman.id.au/kisskb/head/7427/ (all 119 configs)
> [3] http://kisskb.ellerman.id.au/kisskb/head/7401/ (all 119 configs)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v4 1/8] DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG
From: Vinod Koul @ 2014-05-02 16:48 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-2-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:44PM +0800, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> Some codes are calling chan_dbg with FSL_DMA_LD_DEBUG surrounded, it is really
> unnecessary to use such a macro because chan_dbg is a wrapper of dev_dbg, we do
> have corresponding DEBUG macro to switch on/off dev_dbg, and most of the other
> codes are also calling chan_dbg directly without using FSL_DMA_LD_DEBUG.
>
Applied, thanks

-- 
~Vinod
 

^ permalink raw reply

* Re: [PATCH v4 3/8] DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine
From: Vinod Koul @ 2014-05-02 16:49 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-4-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:46PM +0800, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> Delete attribute DMA_INTERRUPT because fsldma doesn't support this function,
> exception will be thrown if talitos is used to offload xor at the same time.
> 
Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v4 4/8] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
From: Vinod Koul @ 2014-05-02 16:49 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-5-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:47PM +0800, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> There are several places where descriptors are freed using identical code.
> This patch puts this code into a function to reduce code duplication.
> 
Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v4 2/8] DMA: Freescale: unify register access methods
From: Vinod Koul @ 2014-05-02 16:48 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-3-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:45PM +0800, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> Methods of accessing DMA controller registers are inconsistent, some registers
> are accessed by DMA_IN/OUT directly, while others are accessed by functions
> get/set_* which are wrappers of DMA_IN/OUT, and even for the BCR register, it
> is read by get_bcr but written by DMA_OUT.
> This patch unifies the inconsistent methods, all registers are accessed by
> get/set_* now.
> 
Applied, thanks

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v4 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
From: Vinod Koul @ 2014-05-02 16:50 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-7-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:49PM +0800, hongbo.zhang@freescale.com wrote:

This need review from Dan ...

-- 
~Vinod
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> Fix the potential risk when enable config NET_DMA and ASYNC_TX. Async_tx is
> lack of support in current release process of dma descriptor, all descriptors
> will be released whatever is acked or no-acked by async_tx, so there is a
> potential race condition when dma engine is uesd by others clients (e.g. when
> enable NET_DMA to offload TCP).
> 
> In our case, a race condition which is raised when use both of talitos and
> dmaengine to offload xor is because napi scheduler will sync all pending
> requests in dma channels, it affects the process of raid operations due to
> ack_tx is not checked in fsl dma. The no-acked descriptor is freed which is
> submitted just now, as a dependent tx, this freed descriptor trigger
> BUG_ON(async_tx_test_ack(depend_tx)) in async_tx_submit().
> 
> TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf40000 CPU: 0
> GPR00: 00000001 ecf41ca0 ee44/921a94a0 0000003f 00000001 c00593e4 00000000 00000001
> GPR08: 00000000 a7a7a7a7 00000001 045/920000002 42028042 100a38d4 ed576d98 00000000
> GPR16: ed5a11b0 00000000 2b162000 00000200 046/920000000 2d555000 ed3015e8 c15a7aa0
> GPR24: 00000000 c155fc40 00000000 ecb63220 ecf41d28 e47/92f640bb0 ef640c30 ecf41ca0
> NIP [c02b048c] async_tx_submit+0x6c/0x2b4
> LR [c02b068c] async_tx_submit+0x26c/0x2b4
> Call Trace:
> [ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
> [ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
> [ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
> [ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
> [ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
> [ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
> [ecf41f40] [c04329b8] md_thread+0x138/0x16c
> [ecf41f90] [c008277c] kthread+0x8c/0x90
> [ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
> 
> Another modification in this patch is the change of completed descriptors,
> there is a potential risk which caused by exception interrupt, all descriptors
> in ld_running list are seemed completed when an interrupt raised, it works fine
> under normal condition, but if there is an exception occured, it cannot work as
> our excepted. Hardware should not be depend on s/w list, the right way is to
> read current descriptor address register to find the last completed descriptor.
> If an interrupt is raised by an error, all descriptors in ld_running should not
> be seemed finished, or these unfinished descriptors in ld_running will be
> released wrongly.
> 
> A simple way to reproduce:
> Enable dmatest first, then insert some bad descriptors which can trigger
> Programming Error interrupts before the good descriptors. Last, the good
> descriptors will be freed before they are processsed because of the exception
> intrerrupt.
> 
> Note: the bad descriptors are only for simulating an exception interrupt.  This
> case can illustrate the potential risk in current fsl-dma very well.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
> Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> ---
>  drivers/dma/fsldma.c |  197 ++++++++++++++++++++++++++++++++++++--------------
>  drivers/dma/fsldma.h |   17 ++++-
>  2 files changed, 159 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index e0fec68..374ca97 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -459,6 +459,88 @@ static struct fsl_desc_sw *fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
>  }
>  
>  /**
> + * fsldma_clean_completed_descriptor - free all descriptors which
> + * has been completed and acked
> + * @chan: Freescale DMA channel
> + *
> + * This function is used on all completed and acked descriptors.
> + * All descriptors should only be freed in this function.
> + */
> +static void fsldma_clean_completed_descriptor(struct fsldma_chan *chan)
> +{
> +	struct fsl_desc_sw *desc, *_desc;
> +
> +	/* Run the callback for each descriptor, in order */
> +	list_for_each_entry_safe(desc, _desc, &chan->ld_completed, node)
> +		if (async_tx_test_ack(&desc->async_tx))
> +			fsl_dma_free_descriptor(chan, desc);
> +}
> +
> +/**
> + * fsldma_run_tx_complete_actions - cleanup a single link descriptor
> + * @chan: Freescale DMA channel
> + * @desc: descriptor to cleanup and free
> + * @cookie: Freescale DMA transaction identifier
> + *
> + * This function is used on a descriptor which has been executed by the DMA
> + * controller. It will run any callbacks, submit any dependencies.
> + */
> +static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
> +		struct fsl_desc_sw *desc, dma_cookie_t cookie)
> +{
> +	struct dma_async_tx_descriptor *txd = &desc->async_tx;
> +	dma_cookie_t ret = cookie;
> +
> +	BUG_ON(txd->cookie < 0);
> +
> +	if (txd->cookie > 0) {
> +		ret = txd->cookie;
> +
> +		/* Run the link descriptor callback function */
> +		if (txd->callback) {
> +			chan_dbg(chan, "LD %p callback\n", desc);
> +			txd->callback(txd->callback_param);
> +		}
> +	}
> +
> +	/* Run any dependencies */
> +	dma_run_dependencies(txd);
> +
> +	return ret;
> +}
> +
> +/**
> + * fsldma_clean_running_descriptor - move the completed descriptor from
> + * ld_running to ld_completed
> + * @chan: Freescale DMA channel
> + * @desc: the descriptor which is completed
> + *
> + * Free the descriptor directly if acked by async_tx api, or move it to
> + * queue ld_completed.
> + */
> +static void fsldma_clean_running_descriptor(struct fsldma_chan *chan,
> +		struct fsl_desc_sw *desc)
> +{
> +	/* Remove from the list of transactions */
> +	list_del(&desc->node);
> +
> +	/*
> +	 * the client is allowed to attach dependent operations
> +	 * until 'ack' is set
> +	 */
> +	if (!async_tx_test_ack(&desc->async_tx)) {
> +		/*
> +		 * Move this descriptor to the list of descriptors which is
> +		 * completed, but still awaiting the 'ack' bit to be set.
> +		 */
> +		list_add_tail(&desc->node, &chan->ld_completed);
> +		return;
> +	}
> +
> +	dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
> +}
> +
> +/**
>   * fsl_chan_xfer_ld_queue - transfer any pending transactions
>   * @chan : Freescale DMA channel
>   *
> @@ -526,31 +608,58 @@ static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
>  }
>  
>  /**
> - * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
> + * fsldma_cleanup_descriptors - cleanup link descriptors which are completed
> + * and move them to ld_completed to free until flag 'ack' is set
>   * @chan: Freescale DMA channel
> - * @desc: descriptor to cleanup and free
>   *
> - * This function is used on a descriptor which has been executed by the DMA
> - * controller. It will run any callbacks, submit any dependencies, and then
> - * free the descriptor.
> + * This function is used on descriptors which have been executed by the DMA
> + * controller. It will run any callbacks, submit any dependencies, then
> + * free these descriptors if flag 'ack' is set.
>   */
> -static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
> -				      struct fsl_desc_sw *desc)
> +static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
>  {
> -	struct dma_async_tx_descriptor *txd = &desc->async_tx;
> +	struct fsl_desc_sw *desc, *_desc;
> +	dma_cookie_t cookie = 0;
> +	dma_addr_t curr_phys = get_cdar(chan);
> +	int seen_current = 0;
> +
> +	fsldma_clean_completed_descriptor(chan);
> +
> +	/* Run the callback for each descriptor, in order */
> +	list_for_each_entry_safe(desc, _desc, &chan->ld_running, node) {
> +		/*
> +		 * do not advance past the current descriptor loaded into the
> +		 * hardware channel, subsequent descriptors are either in
> +		 * process or have not been submitted
> +		 */
> +		if (seen_current)
> +			break;
> +
> +		/*
> +		 * stop the search if we reach the current descriptor and the
> +		 * channel is busy
> +		 */
> +		if (desc->async_tx.phys == curr_phys) {
> +			seen_current = 1;
> +			if (!dma_is_idle(chan))
> +				break;
> +		}
> +
> +		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie);
>  
> -	/* Run the link descriptor callback function */
> -	if (txd->callback) {
> -		chan_dbg(chan, "LD %p callback\n", desc);
> -		txd->callback(txd->callback_param);
> +		fsldma_clean_running_descriptor(chan, desc);
>  	}
>  
> -	/* Run any dependencies */
> -	dma_run_dependencies(txd);
> +	/*
> +	 * Start any pending transactions automatically
> +	 *
> +	 * In the ideal case, we keep the DMA controller busy while we go
> +	 * ahead and free the descriptors below.
> +	 */
> +	fsl_chan_xfer_ld_queue(chan);
>  
> -	dma_descriptor_unmap(txd);
> -	chan_dbg(chan, "LD %p free\n", desc);
> -	dma_pool_free(chan->desc_pool, desc, txd->phys);
> +	if (cookie > 0)
> +		chan->common.completed_cookie = cookie;
>  }
>  
>  /**
> @@ -621,8 +730,10 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
>  
>  	chan_dbg(chan, "free all channel resources\n");
>  	spin_lock_irqsave(&chan->desc_lock, flags);
> +	fsldma_cleanup_descriptors(chan);
>  	fsldma_free_desc_list(chan, &chan->ld_pending);
>  	fsldma_free_desc_list(chan, &chan->ld_running);
> +	fsldma_free_desc_list(chan, &chan->ld_completed);
>  	spin_unlock_irqrestore(&chan->desc_lock, flags);
>  
>  	dma_pool_destroy(chan->desc_pool);
> @@ -860,6 +971,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  		/* Remove and free all of the descriptors in the LD queue */
>  		fsldma_free_desc_list(chan, &chan->ld_pending);
>  		fsldma_free_desc_list(chan, &chan->ld_running);
> +		fsldma_free_desc_list(chan, &chan->ld_completed);
>  		chan->idle = true;
>  
>  		spin_unlock_irqrestore(&chan->desc_lock, flags);
> @@ -919,6 +1031,17 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
>  					dma_cookie_t cookie,
>  					struct dma_tx_state *txstate)
>  {
> +	struct fsldma_chan *chan = to_fsl_chan(dchan);
> +	enum dma_status ret;
> +
> +	ret = dma_cookie_status(dchan, cookie, txstate);
> +	if (ret == DMA_COMPLETE)
> +		return ret;
> +
> +	spin_lock_bh(&chan->desc_lock);
> +	fsldma_cleanup_descriptors(chan);
> +	spin_unlock_bh(&chan->desc_lock);
> +
>  	return dma_cookie_status(dchan, cookie, txstate);
>  }
>  
> @@ -996,52 +1119,19 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
>  static void dma_do_tasklet(unsigned long data)
>  {
>  	struct fsldma_chan *chan = (struct fsldma_chan *)data;
> -	struct fsl_desc_sw *desc, *_desc;
> -	LIST_HEAD(ld_cleanup);
>  	unsigned long flags;
>  
>  	chan_dbg(chan, "tasklet entry\n");
>  
>  	spin_lock_irqsave(&chan->desc_lock, flags);
>  
> -	/* update the cookie if we have some descriptors to cleanup */
> -	if (!list_empty(&chan->ld_running)) {
> -		dma_cookie_t cookie;
> -
> -		desc = to_fsl_desc(chan->ld_running.prev);
> -		cookie = desc->async_tx.cookie;
> -		dma_cookie_complete(&desc->async_tx);
> -
> -		chan_dbg(chan, "completed_cookie=%d\n", cookie);
> -	}
> -
> -	/*
> -	 * move the descriptors to a temporary list so we can drop the lock
> -	 * during the entire cleanup operation
> -	 */
> -	list_splice_tail_init(&chan->ld_running, &ld_cleanup);
> -
>  	/* the hardware is now idle and ready for more */
>  	chan->idle = true;
>  
> -	/*
> -	 * Start any pending transactions automatically
> -	 *
> -	 * In the ideal case, we keep the DMA controller busy while we go
> -	 * ahead and free the descriptors below.
> -	 */
> -	fsl_chan_xfer_ld_queue(chan);
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> -
> -	/* Run the callback for each descriptor, in order */
> -	list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
> -
> -		/* Remove from the list of transactions */
> -		list_del(&desc->node);
> +	/* Run all cleanup for descriptors which have been completed */
> +	fsldma_cleanup_descriptors(chan);
>  
> -		/* Run all cleanup for this descriptor */
> -		fsldma_cleanup_descriptor(chan, desc);
> -	}
> +	spin_unlock_irqrestore(&chan->desc_lock, flags);
>  
>  	chan_dbg(chan, "tasklet exit\n");
>  }
> @@ -1225,6 +1315,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>  	spin_lock_init(&chan->desc_lock);
>  	INIT_LIST_HEAD(&chan->ld_pending);
>  	INIT_LIST_HEAD(&chan->ld_running);
> +	INIT_LIST_HEAD(&chan->ld_completed);
>  	chan->idle = true;
>  
>  	chan->common.device = &fdev->common;
> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> index d56e835..ec19517 100644
> --- a/drivers/dma/fsldma.h
> +++ b/drivers/dma/fsldma.h
> @@ -138,8 +138,21 @@ struct fsldma_chan {
>  	char name[8];			/* Channel name */
>  	struct fsldma_chan_regs __iomem *regs;
>  	spinlock_t desc_lock;		/* Descriptor operation lock */
> -	struct list_head ld_pending;	/* Link descriptors queue */
> -	struct list_head ld_running;	/* Link descriptors queue */
> +	/*
> +	 * Descriptors which are queued to run, but have not yet been
> +	 * submitted to the hardware for execution
> +	 */
> +	struct list_head ld_pending;
> +	/*
> +	 * Descriptors which are currently being executed by the hardware
> +	 */
> +	struct list_head ld_running;
> +	/*
> +	 * Descriptors which have finished execution by the hardware. These
> +	 * descriptors have already had their cleanup actions run. They are
> +	 * waiting for the ACK bit to be set by the async_tx API.
> +	 */
> +	struct list_head ld_completed;	/* Link descriptors queue */
>  	struct dma_chan common;		/* DMA common channel */
>  	struct dma_pool *desc_pool;	/* Descriptors pool */
>  	struct device *dev;		/* Channel device */
> -- 
> 1.7.9.5
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

^ permalink raw reply

* Re: [PATCH v4 7/8] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
From: Vinod Koul @ 2014-05-02 16:51 UTC (permalink / raw)
  To: hongbo.zhang
  Cc: leo.li, vkoul, linux-kernel, scottwood, dmaengine, dan.j.williams,
	linuxppc-dev
In-Reply-To: <1397809071-5353-8-git-send-email-hongbo.zhang@freescale.com>

On Fri, Apr 18, 2014 at 04:17:50PM +0800, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
> 
> The usage of spin_lock_irqsave() is a stronger locking mechanism than is
> required throughout the driver. The minimum locking required should be used
> instead. Interrupts will be turned off and context will be saved, it is
> unnecessary to use irqsave.
> 
> This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
> manipulation of protected fields is done using tasklet context or weaker, which
> makes spin_lock_bh() the correct choice.
>

This doesnt apply, perhpas due to depends on 6/8

-- 
~Vinod
 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
> Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
> ---
>  drivers/dma/fsldma.c |   25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 374ca97..6e1c9b3 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -396,10 +396,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
>  	struct fsldma_chan *chan = to_fsl_chan(tx->chan);
>  	struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
>  	struct fsl_desc_sw *child;
> -	unsigned long flags;
>  	dma_cookie_t cookie = -EINVAL;
>  
> -	spin_lock_irqsave(&chan->desc_lock, flags);
> +	spin_lock_bh(&chan->desc_lock);
>  
>  	/*
>  	 * assign cookies to all of the software descriptors
> @@ -412,7 +411,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
>  	/* put this transaction onto the tail of the pending queue */
>  	append_ld_queue(chan, desc);
>  
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> +	spin_unlock_bh(&chan->desc_lock);
>  
>  	return cookie;
>  }
> @@ -726,15 +725,14 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
>  static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
>  {
>  	struct fsldma_chan *chan = to_fsl_chan(dchan);
> -	unsigned long flags;
>  
>  	chan_dbg(chan, "free all channel resources\n");
> -	spin_lock_irqsave(&chan->desc_lock, flags);
> +	spin_lock_bh(&chan->desc_lock);
>  	fsldma_cleanup_descriptors(chan);
>  	fsldma_free_desc_list(chan, &chan->ld_pending);
>  	fsldma_free_desc_list(chan, &chan->ld_running);
>  	fsldma_free_desc_list(chan, &chan->ld_completed);
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> +	spin_unlock_bh(&chan->desc_lock);
>  
>  	dma_pool_destroy(chan->desc_pool);
>  	chan->desc_pool = NULL;
> @@ -953,7 +951,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  {
>  	struct dma_slave_config *config;
>  	struct fsldma_chan *chan;
> -	unsigned long flags;
>  	int size;
>  
>  	if (!dchan)
> @@ -963,7 +960,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  
>  	switch (cmd) {
>  	case DMA_TERMINATE_ALL:
> -		spin_lock_irqsave(&chan->desc_lock, flags);
> +		spin_lock_bh(&chan->desc_lock);
>  
>  		/* Halt the DMA engine */
>  		dma_halt(chan);
> @@ -974,7 +971,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  		fsldma_free_desc_list(chan, &chan->ld_completed);
>  		chan->idle = true;
>  
> -		spin_unlock_irqrestore(&chan->desc_lock, flags);
> +		spin_unlock_bh(&chan->desc_lock);
>  		return 0;
>  
>  	case DMA_SLAVE_CONFIG:
> @@ -1016,11 +1013,10 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
>  static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
>  {
>  	struct fsldma_chan *chan = to_fsl_chan(dchan);
> -	unsigned long flags;
>  
> -	spin_lock_irqsave(&chan->desc_lock, flags);
> +	spin_lock_bh(&chan->desc_lock);
>  	fsl_chan_xfer_ld_queue(chan);
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> +	spin_unlock_bh(&chan->desc_lock);
>  }
>  
>  /**
> @@ -1119,11 +1115,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
>  static void dma_do_tasklet(unsigned long data)
>  {
>  	struct fsldma_chan *chan = (struct fsldma_chan *)data;
> -	unsigned long flags;
>  
>  	chan_dbg(chan, "tasklet entry\n");
>  
> -	spin_lock_irqsave(&chan->desc_lock, flags);
> +	spin_lock_bh(&chan->desc_lock);
>  
>  	/* the hardware is now idle and ready for more */
>  	chan->idle = true;
> @@ -1131,7 +1126,7 @@ static void dma_do_tasklet(unsigned long data)
>  	/* Run all cleanup for descriptors which have been completed */
>  	fsldma_cleanup_descriptors(chan);
>  
> -	spin_unlock_irqrestore(&chan->desc_lock, flags);
> +	spin_unlock_bh(&chan->desc_lock);
>  
>  	chan_dbg(chan, "tasklet exit\n");
>  }
> -- 
> 1.7.9.5
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

^ permalink raw reply


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