LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Guenter Roeck @ 2014-10-09 15:54 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Catalin Marinas, Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141009104927.GN20647@lee--X1>

On Thu, Oct 09, 2014 at 11:49:27AM +0100, Lee Jones wrote:
> On Thu, 09 Oct 2014, Catalin Marinas wrote:
> 
> > On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> > > On Mon, 06 Oct 2014, Guenter Roeck wrote:
> > > > --- a/drivers/mfd/ab8500-sysctrl.c
> > > > +++ b/drivers/mfd/ab8500-sysctrl.c
> > > > @@ -6,6 +6,7 @@
> > > 
> > > [...]
> > > 
> > > > +static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
> > > > +			    void *unused2)
> > > >  {
> > > >  	sigset_t old;
> > > >  	sigset_t all;
> > > > @@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> > > >  	struct power_supply *psy;
> > > >  	int ret;
> > > >  
> > > > -	if (sysctrl_dev == NULL) {
> > > > -		pr_err("%s: sysctrl not initialized\n", __func__);
> > > > -		return;
> > > > -	}
> > > 
> > > Can you explain the purpose of this change please?
> > 
> > I guess it's because the sysctrl_dev is already initialised when
> > registering the power_off handler, so there isn't a way to call the
> > above function with a NULL sysctrl_dev. Probably even with the original
> > code you didn't need this check (after some race fix in
> > ab8500_sysctrl_remove but races is one of the things Guenter's patches
> > try to address).
> 
> Sounds reasonable, although I think this change should be part of
> another patch.
> 
Turns out the options are to either drop the check or to use the device
managed function to register the poweroff handler. I decided to keep
the check and use the device managed function.

Guenter

^ permalink raw reply

* Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Guenter Roeck @ 2014-10-09 15:45 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Catalin Marinas, Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141009133355.GQ20647@lee--X1>

On Thu, Oct 09, 2014 at 02:33:55PM +0100, Lee Jones wrote:
> On Thu, 09 Oct 2014, Guenter Roeck wrote:
> 
> > On 10/09/2014 03:49 AM, Lee Jones wrote:
> > >On Thu, 09 Oct 2014, Catalin Marinas wrote:
> > >
> > >>On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> > >>>On Mon, 06 Oct 2014, Guenter Roeck wrote:
> > >>>>--- a/drivers/mfd/ab8500-sysctrl.c
> > >>>>+++ b/drivers/mfd/ab8500-sysctrl.c
> > >>>>@@ -6,6 +6,7 @@
> > >>>
> > >>>[...]
> > >>>
> > >>>>+static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
> > >>>>+			    void *unused2)
> > >>>>  {
> > >>>>  	sigset_t old;
> > >>>>  	sigset_t all;
> > >>>>@@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> > >>>>  	struct power_supply *psy;
> > >>>>  	int ret;
> > >>>>
> > >>>>-	if (sysctrl_dev == NULL) {
> > >>>>-		pr_err("%s: sysctrl not initialized\n", __func__);
> > >>>>-		return;
> > >>>>-	}
> > >>>
> > >>>Can you explain the purpose of this change please?
> > >>
> > >>I guess it's because the sysctrl_dev is already initialised when
> > >>registering the power_off handler, so there isn't a way to call the
> > >>above function with a NULL sysctrl_dev. Probably even with the original
> > >>code you didn't need this check (after some race fix in
> > >>ab8500_sysctrl_remove but races is one of the things Guenter's patches
> > >>try to address).
> > >
> > >Sounds reasonable, although I think this change should be part of
> > >another patch.
> > >
> > Sure, no problem. I'll split this into two patches.
> > 
> > Since we are at it, any idea what to do with the restart function
> > in the same file ? It is not used anywhere.
> 
> You can strip it out with Linus Walleij's Ack.  Or I'll be happy to do
> it?
> 
I'll strip it out in a 3rd patch.

Guenter

^ permalink raw reply

* Re: [PATCH 01/44] kernel: Add support for poweroff handler call chain
From: Guenter Roeck @ 2014-10-09 15:38 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Heiko Stuebner,
	Len Brown, linux-xtensa, devel, linux-s390, lguest, linux-c6x-dev,
	linux-hexagon, linux-sh, Alexander Graf, linux-acpi,
	Geert Uytterhoeven, xen-devel, devicetree, user-mode-linux-devel,
	linux-pm, adi-buildroot-devel, linux-m68k, linux-am33-list,
	linux-tegra, openipmi-developer, linux-metag, linux-arm-kernel,
	linux-parisc, linux-cris-kernel, Rafael J. Wysocki, linux-kernel,
	linux-alpha, Andrew Morton, Romain Perier, linuxppc-dev
In-Reply-To: <20141009103143.GA6787@amd>

On Thu, Oct 09, 2014 at 12:31:43PM +0200, Pavel Machek wrote:
> Hi!
> 
> > +/**
> > + *	register_poweroff_handler_simple - Register function to be called to power off
> > + *					   the system
> > + *	@handler:	Function to be called to power off the system
> > + *	@priority:	Handler priority. For priority guidelines see
> > + *			register_poweroff_handler.
> > + *
> > + *	This is a simplified version of register_poweroff_handler. It does not
> > + *	take a notifier as argument, but a function pointer. The function
> > + *	registers a poweroff handler with specified priority. Poweroff
> > + *	handlers registered with this function can not be unregistered,
> > + *	and only a single poweroff handler can be installed using it.
> > + *
> > + *	This function must not be called from modules and is therefore
> > + *	not exported.
> > + *
> > + *	Returns -EBUSY if a poweroff handler has already been registered
> > + *	using register_poweroff_handler_simple. Otherwise returns zero,
> > + *	since atomic_notifier_chain_register() currently always returns zero.
> > + */
> > +int register_poweroff_handler_simple(void (*handler)(void), int priority)
> > +{
> > +	char symname[KSYM_NAME_LEN];
> > +
> > +	if (poweroff_handler_data.handler) {
> > +		lookup_symbol_name((unsigned long)poweroff_handler_data.handler,
> > +				   symname);
> > +		pr_warn("Poweroff function already registered (%s)", symname);
> > +		lookup_symbol_name((unsigned long)handler, symname);
> > +		pr_cont(", cannot register %s\n", symname);
> > +		return -EBUSY;
> > +	}
> 
> Dunno, are you maybe overdoing the debugging infrastructure a bit?
> This is not going to happen in production, and if it does happen,
> developer can look the symbol name himself.

On the other side, I don't think it hurts to have that message.
Anyway, I'll use %ps as suggested by Geert.

Guenter

^ permalink raw reply

* Re: [PATCH v4 09/16] powerpc/mm: Add new hash_page_mm()
From: Aneesh Kumar K.V @ 2014-10-09 15:28 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-10-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This adds a new function hash_page_mm() based on the existing hash_page().
> This version allows any struct mm to be passed in, rather than assuming
> current. This is useful for servicing co-processor faults which are not in the
> context of the current running process.
>
> We need to be careful here as the current hash_page() assumes current in a few
> places.

if you move cxl_slbia patch before this, then we could add additonal
information here.

1) We use this to insert hpte entries on-behalf of co-processor.
2) we don't need to flush cpu slb entries when adding new hpte entries
3) We do flush the co-processor slb cache, if adding hpte entry result
in a segement demotion or have mmu_ci_restrictions enabled.
4) We do update the slice array to indicate the new base page size
5) w.r.t co-processor on segment miss we look at slice array and add new slb entry
using newly added copro_calculate_slb "powerpc/cell: Move data segment faulting code out of cell platform"

>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/include/asm/mmu-hash64.h |  1 +
>  arch/powerpc/mm/hash_utils_64.c       | 24 +++++++++++++++++-------
>  2 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
> index aeabd02..764e141 100644
> --- a/arch/powerpc/include/asm/mmu-hash64.h
> +++ b/arch/powerpc/include/asm/mmu-hash64.h
> @@ -324,6 +324,7 @@ extern int __hash_page_64K(unsigned long ea, unsigned long access,
>  			   unsigned int local, int ssize);
>  struct mm_struct;
>  unsigned int hash_page_do_lazy_icache(unsigned int pp, pte_t pte, int trap);
> +extern int hash_page_mm(struct mm_struct *mm, unsigned long ea, unsigned long access, unsigned long trap);
>  extern int hash_page(unsigned long ea, unsigned long access, unsigned long trap);
>  int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
>  		     pte_t *ptep, unsigned long trap, int local, int ssize,
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index bbdb054..698834d 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -904,7 +904,7 @@ void demote_segment_4k(struct mm_struct *mm, unsigned long addr)
>  		return;
>  	slice_set_range_psize(mm, addr, 1, MMU_PAGE_4K);
>  	copro_flush_all_slbs(mm);
> -	if (get_paca_psize(addr) != MMU_PAGE_4K) {
> +	if ((get_paca_psize(addr) != MMU_PAGE_4K) && (current->mm == mm)) {
>  		get_paca()->context = mm->context;
>  		slb_flush_and_rebolt();
>  	}
> @@ -989,12 +989,11 @@ static void check_paca_psize(unsigned long ea, struct mm_struct *mm,
>   * -1 - critical hash insertion error
>   * -2 - access not permitted by subpage protection mechanism
>   */
> -int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
> +int hash_page_mm(struct mm_struct *mm, unsigned long ea, unsigned long access, unsigned long trap)
>  {
>  	enum ctx_state prev_state = exception_enter();
>  	pgd_t *pgdir;
>  	unsigned long vsid;
> -	struct mm_struct *mm;
>  	pte_t *ptep;
>  	unsigned hugeshift;
>  	const struct cpumask *tmp;
> @@ -1008,7 +1007,6 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>   	switch (REGION_ID(ea)) {
>  	case USER_REGION_ID:
>  		user_region = 1;
> -		mm = current->mm;
>  		if (! mm) {
>  			DBG_LOW(" user region with no mm !\n");
>  			rc = 1;
> @@ -1019,7 +1017,6 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>  		vsid = get_vsid(mm->context.id, ea, ssize);
>  		break;
>  	case VMALLOC_REGION_ID:
> -		mm = &init_mm;
>  		vsid = get_kernel_vsid(ea, mmu_kernel_ssize);
>  		if (ea < VMALLOC_END)
>  			psize = mmu_vmalloc_psize;
> @@ -1104,7 +1101,8 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>  			WARN_ON(1);
>  		}
>  #endif
> -		check_paca_psize(ea, mm, psize, user_region);
> +		if (current->mm == mm)
> +			check_paca_psize(ea, mm, psize, user_region);
>  
>  		goto bail;
>  	}
> @@ -1145,7 +1143,8 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>  		}
>  	}
>  
> -	check_paca_psize(ea, mm, psize, user_region);
> +	if (current->mm == mm)
> +		check_paca_psize(ea, mm, psize, user_region);
>  #endif /* CONFIG_PPC_64K_PAGES */
>  
>  #ifdef CONFIG_PPC_HAS_HASH_64K
> @@ -1180,6 +1179,17 @@ bail:
>  	exception_exit(prev_state);
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(hash_page_mm);
> +
> +int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
> +{
> +	struct mm_struct *mm = current->mm;
> +
> +	if (REGION_ID(ea) == VMALLOC_REGION_ID)
> +		mm = &init_mm;
> +
> +	return hash_page_mm(mm, ea, access, trap);
> +}
>  EXPORT_SYMBOL_GPL(hash_page);
>  
>  void hash_preload(struct mm_struct *mm, unsigned long ea,
> -- 
> 1.9.1
>
> --
> 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: [PATCH v4 11/16] powerpc/mm: Add hooks for cxl
From: Aneesh Kumar K.V @ 2014-10-09 15:20 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-12-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This adds hooks into the core powerpc mm code for cxl.
>
> The core powerpc code sometimes uses local tlbie. Unfortunately this won't
> work with the current cxl driver as it relies on snooping tlbie broadcasts.
>
> The cxl hardware can have TLB entries invalidated via MMIO but this is not
> currently supported by the driver. In future we can make local tlbie smarter so
> that it invalidates cxl contexts via MMIO when it needs to but for now we have
> this workaround.
>
> This workaround checks for any active cxl contexts and if so, disables local
> tlbie.
>
> This also adds a hook for when SLBs are invalidated. This ensures any
> corresponding SLBs in cxl are also invalidated at the same time. This is
> required for segment demotion.
>
> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/mm/copro_fault.c    | 2 ++
>  arch/powerpc/mm/hash_native_64.c | 6 +++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> index f2aa5a8..0f9939e 100644
> --- a/arch/powerpc/mm/copro_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -26,6 +26,7 @@
>  #include <asm/reg.h>
>  #include <asm/copro.h>
>  #include <asm/spu.h>
> +#include <misc/cxl.h>
>  
>  /*
>   * This ought to be kept in sync with the powerpc specific do_page_fault
> @@ -143,5 +144,6 @@ void copro_flush_all_slbs(struct mm_struct *mm)
>  #ifdef CONFIG_SPU_BASE
>  	spu_flush_all_slbs(mm);
>  #endif
> +	cxl_slbia(mm);
>  }

If you split this patch into two and move the above hunk in a patch
before "[PATCH v4 09/16] powerpc/mm: Add new hash_page_mm()", it would
make it much easier to follow. We could then update the commit of the
09th patch to carry additional information that talk about how the
hash_page_mm really work as I replied to that patch.


>  EXPORT_SYMBOL_GPL(copro_flush_all_slbs);
> diff --git a/arch/powerpc/mm/hash_native_64.c b/arch/powerpc/mm/hash_native_64.c
> index afc0a82..ae4962a 100644
> --- a/arch/powerpc/mm/hash_native_64.c
> +++ b/arch/powerpc/mm/hash_native_64.c
> @@ -29,6 +29,8 @@
>  #include <asm/kexec.h>
>  #include <asm/ppc-opcode.h>
>  
> +#include <misc/cxl.h>
> +
>  #ifdef DEBUG_LOW
>  #define DBG_LOW(fmt...) udbg_printf(fmt)
>  #else
> @@ -149,9 +151,11 @@ static inline void __tlbiel(unsigned long vpn, int psize, int apsize, int ssize)
>  static inline void tlbie(unsigned long vpn, int psize, int apsize,
>  			 int ssize, int local)
>  {
> -	unsigned int use_local = local && mmu_has_feature(MMU_FTR_TLBIEL);
> +	unsigned int use_local;
>  	int lock_tlbie = !mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE);
>  
> +	use_local = local && mmu_has_feature(MMU_FTR_TLBIEL) && !cxl_ctx_in_use();
> +
>  	if (use_local)
>  		use_local = mmu_psize_defs[psize].tlbiel;
>  	if (lock_tlbie && !use_local)
> -- 
> 1.9.1
>
> --
> 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: [PATCH v4 05/16] powerpc/mm: Export mmu_kernel_ssize and mmu_linear_psize
From: Aneesh Kumar K.V @ 2014-10-09 15:05 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-6-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> Export mmu_kernel_ssize and mmu_linear_psize.  These are needed by the cxl
> driver which has it's own MMU.  To setup the MMU cxl needs access to these.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/mm/hash_utils_64.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 5c0738d..bbdb054 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -98,6 +98,7 @@ unsigned long htab_size_bytes;
>  unsigned long htab_hash_mask;
>  EXPORT_SYMBOL_GPL(htab_hash_mask);
>  int mmu_linear_psize = MMU_PAGE_4K;
> +EXPORT_SYMBOL_GPL(mmu_linear_psize);
>  int mmu_virtual_psize = MMU_PAGE_4K;
>  int mmu_vmalloc_psize = MMU_PAGE_4K;
>  #ifdef CONFIG_SPARSEMEM_VMEMMAP
> @@ -105,6 +106,7 @@ int mmu_vmemmap_psize = MMU_PAGE_4K;
>  #endif
>  int mmu_io_psize = MMU_PAGE_4K;
>  int mmu_kernel_ssize = MMU_SEGSIZE_256M;
> +EXPORT_SYMBOL_GPL(mmu_kernel_ssize);
>  int mmu_highuser_ssize = MMU_SEGSIZE_256M;
>  u16 mmu_slb_size = 64;
>  EXPORT_SYMBOL_GPL(mmu_slb_size);
> -- 
> 1.9.1
>
> --
> 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: [PATCH v4 03/16] powerpc/cell: Make spu_flush_all_slbs() generic
From: Aneesh Kumar K.V @ 2014-10-09 15:04 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-4-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> This moves spu_flush_all_slbs() into a generic call copro_flush_all_slbs().
>
> This will be useful when we add cxl which also needs a similar SLB flush call.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/include/asm/copro.h |  6 ++++++
>  arch/powerpc/mm/copro_fault.c    |  9 +++++++++
>  arch/powerpc/mm/hash_utils_64.c  | 10 +++-------
>  arch/powerpc/mm/slice.c          | 10 +++-------
>  4 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/copro.h b/arch/powerpc/include/asm/copro.h
> index b0e6a18..ce216df 100644
> --- a/arch/powerpc/include/asm/copro.h
> +++ b/arch/powerpc/include/asm/copro.h
> @@ -20,4 +20,10 @@ int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
>  
>  int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb);
>  
> +
> +#ifdef CONFIG_PPC_COPRO_BASE
> +void copro_flush_all_slbs(struct mm_struct *mm);
> +#else
> +static inline void copro_flush_all_slbs(struct mm_struct *mm) {}
> +#endif
>  #endif /* _ASM_POWERPC_COPRO_H */
> diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> index a15a23e..f2aa5a8 100644
> --- a/arch/powerpc/mm/copro_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -25,6 +25,7 @@
>  #include <linux/export.h>
>  #include <asm/reg.h>
>  #include <asm/copro.h>
> +#include <asm/spu.h>
>  
>  /*
>   * This ought to be kept in sync with the powerpc specific do_page_fault
> @@ -136,3 +137,11 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(copro_calculate_slb);
> +
> +void copro_flush_all_slbs(struct mm_struct *mm)
> +{
> +#ifdef CONFIG_SPU_BASE
> +	spu_flush_all_slbs(mm);
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(copro_flush_all_slbs);
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index daee7f4..5c0738d 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -51,7 +51,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/cputable.h>
>  #include <asm/sections.h>
> -#include <asm/spu.h>
> +#include <asm/copro.h>
>  #include <asm/udbg.h>
>  #include <asm/code-patching.h>
>  #include <asm/fadump.h>
> @@ -901,9 +901,7 @@ void demote_segment_4k(struct mm_struct *mm, unsigned long addr)
>  	if (get_slice_psize(mm, addr) == MMU_PAGE_4K)
>  		return;
>  	slice_set_range_psize(mm, addr, 1, MMU_PAGE_4K);
> -#ifdef CONFIG_SPU_BASE
> -	spu_flush_all_slbs(mm);
> -#endif
> +	copro_flush_all_slbs(mm);
>  	if (get_paca_psize(addr) != MMU_PAGE_4K) {
>  		get_paca()->context = mm->context;
>  		slb_flush_and_rebolt();
> @@ -1141,9 +1139,7 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap)
>  			       "to 4kB pages because of "
>  			       "non-cacheable mapping\n");
>  			psize = mmu_vmalloc_psize = MMU_PAGE_4K;
> -#ifdef CONFIG_SPU_BASE
> -			spu_flush_all_slbs(mm);
> -#endif
> +			copro_flush_all_slbs(mm);
>  		}
>  	}
>  
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index b0c75cc..a81791c 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -32,7 +32,7 @@
>  #include <linux/export.h>
>  #include <asm/mman.h>
>  #include <asm/mmu.h>
> -#include <asm/spu.h>
> +#include <asm/copro.h>
>  
>  /* some sanity checks */
>  #if (PGTABLE_RANGE >> 43) > SLICE_MASK_SIZE
> @@ -232,9 +232,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
>  
>  	spin_unlock_irqrestore(&slice_convert_lock, flags);
>  
> -#ifdef CONFIG_SPU_BASE
> -	spu_flush_all_slbs(mm);
> -#endif
> +	copro_flush_all_slbs(mm);
>  }
>  
>  /*
> @@ -671,9 +669,7 @@ void slice_set_psize(struct mm_struct *mm, unsigned long address,
>  
>  	spin_unlock_irqrestore(&slice_convert_lock, flags);
>  
> -#ifdef CONFIG_SPU_BASE
> -	spu_flush_all_slbs(mm);
> -#endif
> +	copro_flush_all_slbs(mm);
>  }
>  
>  void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
> -- 
> 1.9.1
>
> --
> 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: [PATCH v4 02/16] powerpc/cell: Move data segment faulting code out of cell platform
From: Aneesh Kumar K.V @ 2014-10-09 15:04 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-3-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> __spu_trap_data_seg() currently contains code to determine the VSID and ESID
> required for a particular EA and mm struct.
>
> This code is generically useful for other co-processors. This moves the code of
> the cell platform so it can be used by other powerpc code. It also adds 1TB
> segment handling which Cell didn't support.  The new function is called
> copro_calculate_slb().
>
> This also moves the internal struct spu_slb to a generic struct copro_slb which
> is now used in the Cell and copro code.  We use this new struct instead of
> passing around esid and vsid parameters.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/include/asm/copro.h       |  7 +++++
>  arch/powerpc/include/asm/mmu-hash64.h  |  7 +++++
>  arch/powerpc/mm/copro_fault.c          | 46 ++++++++++++++++++++++++++++
>  arch/powerpc/mm/slb.c                  |  3 --
>  arch/powerpc/platforms/cell/spu_base.c | 55 ++++++----------------------------
>  5 files changed, 69 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/copro.h b/arch/powerpc/include/asm/copro.h
> index 51cae85..b0e6a18 100644
> --- a/arch/powerpc/include/asm/copro.h
> +++ b/arch/powerpc/include/asm/copro.h
> @@ -10,7 +10,14 @@
>  #ifndef _ASM_POWERPC_COPRO_H
>  #define _ASM_POWERPC_COPRO_H
>  
> +struct copro_slb
> +{
> +	u64 esid, vsid;
> +};
> +
>  int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
>  			  unsigned long dsisr, unsigned *flt);
>  
> +int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb);
> +
>  #endif /* _ASM_POWERPC_COPRO_H */
> diff --git a/arch/powerpc/include/asm/mmu-hash64.h b/arch/powerpc/include/asm/mmu-hash64.h
> index d765144..aeabd02 100644
> --- a/arch/powerpc/include/asm/mmu-hash64.h
> +++ b/arch/powerpc/include/asm/mmu-hash64.h
> @@ -190,6 +190,13 @@ static inline unsigned int mmu_psize_to_shift(unsigned int mmu_psize)
>  
>  #ifndef __ASSEMBLY__
>  
> +static inline int slb_vsid_shift(int ssize)
> +{
> +	if (ssize == MMU_SEGSIZE_256M)
> +		return SLB_VSID_SHIFT;
> +	return SLB_VSID_SHIFT_1T;
> +}
> +
>  static inline int segment_shift(int ssize)
>  {
>  	if (ssize == MMU_SEGSIZE_256M)
> diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
> index ba7df14..a15a23e 100644
> --- a/arch/powerpc/mm/copro_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -24,6 +24,7 @@
>  #include <linux/mm.h>
>  #include <linux/export.h>
>  #include <asm/reg.h>
> +#include <asm/copro.h>
>  
>  /*
>   * This ought to be kept in sync with the powerpc specific do_page_fault
> @@ -90,3 +91,48 @@ out_unlock:
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(copro_handle_mm_fault);
> +
> +int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
> +{
> +	u64 vsid;
> +	int psize, ssize;
> +
> +	slb->esid = (ea & ESID_MASK) | SLB_ESID_V;
> +
> +	switch (REGION_ID(ea)) {
> +	case USER_REGION_ID:
> +		pr_devel("%s: 0x%llx -- USER_REGION_ID\n", __func__, ea);
> +		psize = get_slice_psize(mm, ea);
> +		ssize = user_segment_size(ea);
> +		vsid = get_vsid(mm->context.id, ea, ssize);
> +		break;
> +	case VMALLOC_REGION_ID:
> +		pr_devel("%s: 0x%llx -- VMALLOC_REGION_ID\n", __func__, ea);
> +		if (ea < VMALLOC_END)
> +			psize = mmu_vmalloc_psize;
> +		else
> +			psize = mmu_io_psize;
> +		ssize = mmu_kernel_ssize;
> +		vsid = get_kernel_vsid(ea, mmu_kernel_ssize);
> +		break;
> +	case KERNEL_REGION_ID:
> +		pr_devel("%s: 0x%llx -- KERNEL_REGION_ID\n", __func__, ea);
> +		psize = mmu_linear_psize;
> +		ssize = mmu_kernel_ssize;
> +		vsid = get_kernel_vsid(ea, mmu_kernel_ssize);
> +		break;
> +	default:
> +		pr_debug("%s: invalid region access at %016llx\n", __func__, ea);
> +		return 1;
> +	}
> +
> +	vsid = (vsid << slb_vsid_shift(ssize)) | SLB_VSID_USER;
> +
> +	vsid |= mmu_psize_defs[psize].sllp |
> +		((ssize == MMU_SEGSIZE_1T) ? SLB_VSID_B_1T : 0);
> +

s/0/SLB_VSID_B_256M/ 

> +	slb->vsid = vsid;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(copro_calculate_slb);
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 0399a67..6e450ca 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -46,9 +46,6 @@ static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
>  	return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
>  }
>  
> -#define slb_vsid_shift(ssize)	\
> -	((ssize) == MMU_SEGSIZE_256M? SLB_VSID_SHIFT: SLB_VSID_SHIFT_1T)
> -
>  static inline unsigned long mk_vsid_data(unsigned long ea, int ssize,
>  					 unsigned long flags)
>  {
> diff --git a/arch/powerpc/platforms/cell/spu_base.c b/arch/powerpc/platforms/cell/spu_base.c
> index 2930d1e..ffcbd24 100644
> --- a/arch/powerpc/platforms/cell/spu_base.c
> +++ b/arch/powerpc/platforms/cell/spu_base.c
> @@ -76,10 +76,6 @@ static LIST_HEAD(spu_full_list);
>  static DEFINE_SPINLOCK(spu_full_list_lock);
>  static DEFINE_MUTEX(spu_full_list_mutex);
>  
> -struct spu_slb {
> -	u64 esid, vsid;
> -};
> -
>  void spu_invalidate_slbs(struct spu *spu)
>  {
>  	struct spu_priv2 __iomem *priv2 = spu->priv2;
> @@ -149,7 +145,7 @@ static void spu_restart_dma(struct spu *spu)
>  	}
>  }
>  
> -static inline void spu_load_slb(struct spu *spu, int slbe, struct spu_slb *slb)
> +static inline void spu_load_slb(struct spu *spu, int slbe, struct copro_slb *slb)
>  {
>  	struct spu_priv2 __iomem *priv2 = spu->priv2;
>  
> @@ -167,45 +163,12 @@ static inline void spu_load_slb(struct spu *spu, int slbe, struct spu_slb *slb)
>  
>  static int __spu_trap_data_seg(struct spu *spu, unsigned long ea)
>  {
> -	struct mm_struct *mm = spu->mm;
> -	struct spu_slb slb;
> -	int psize;
> -
> -	pr_debug("%s\n", __func__);
> -
> -	slb.esid = (ea & ESID_MASK) | SLB_ESID_V;
> +	struct copro_slb slb;
> +	int ret;
>  
> -	switch(REGION_ID(ea)) {
> -	case USER_REGION_ID:
> -#ifdef CONFIG_PPC_MM_SLICES
> -		psize = get_slice_psize(mm, ea);
> -#else
> -		psize = mm->context.user_psize;
> -#endif
> -		slb.vsid = (get_vsid(mm->context.id, ea, MMU_SEGSIZE_256M)
> -				<< SLB_VSID_SHIFT) | SLB_VSID_USER;
> -		break;
> -	case VMALLOC_REGION_ID:
> -		if (ea < VMALLOC_END)
> -			psize = mmu_vmalloc_psize;
> -		else
> -			psize = mmu_io_psize;
> -		slb.vsid = (get_kernel_vsid(ea, MMU_SEGSIZE_256M)
> -				<< SLB_VSID_SHIFT) | SLB_VSID_KERNEL;
> -		break;
> -	case KERNEL_REGION_ID:
> -		psize = mmu_linear_psize;
> -		slb.vsid = (get_kernel_vsid(ea, MMU_SEGSIZE_256M)
> -				<< SLB_VSID_SHIFT) | SLB_VSID_KERNEL;
> -		break;
> -	default:
> -		/* Future: support kernel segments so that drivers
> -		 * can use SPUs.
> -		 */
> -		pr_debug("invalid region access at %016lx\n", ea);
> -		return 1;
> -	}
> -	slb.vsid |= mmu_psize_defs[psize].sllp;
> +	ret = copro_calculate_slb(spu->mm, ea, &slb);
> +	if (ret)
> +		return ret;
>  
>  	spu_load_slb(spu, spu->slb_replace, &slb);
>  
> @@ -253,7 +216,7 @@ static int __spu_trap_data_map(struct spu *spu, unsigned long ea, u64 dsisr)
>  	return 0;
>  }
>  
> -static void __spu_kernel_slb(void *addr, struct spu_slb *slb)
> +static void __spu_kernel_slb(void *addr, struct copro_slb *slb)
>  {
>  	unsigned long ea = (unsigned long)addr;
>  	u64 llp;
> @@ -272,7 +235,7 @@ static void __spu_kernel_slb(void *addr, struct spu_slb *slb)
>   * Given an array of @nr_slbs SLB entries, @slbs, return non-zero if the
>   * address @new_addr is present.
>   */
> -static inline int __slb_present(struct spu_slb *slbs, int nr_slbs,
> +static inline int __slb_present(struct copro_slb *slbs, int nr_slbs,
>  		void *new_addr)
>  {
>  	unsigned long ea = (unsigned long)new_addr;
> @@ -297,7 +260,7 @@ static inline int __slb_present(struct spu_slb *slbs, int nr_slbs,
>  void spu_setup_kernel_slbs(struct spu *spu, struct spu_lscsa *lscsa,
>  		void *code, int code_size)
>  {
> -	struct spu_slb slbs[4];
> +	struct copro_slb slbs[4];
>  	int i, nr_slbs = 0;
>  	/* start and end addresses of both mappings */
>  	void *addrs[] = {
> -- 
> 1.9.1
>
> --
> 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: [PATCH v4 01/16] powerpc/cell: Move spu_handle_mm_fault() out of cell platform
From: Aneesh Kumar K.V @ 2014-10-09 15:01 UTC (permalink / raw)
  To: Michael Neuling, greg, arnd, mpe, benh
  Cc: cbe-oss-dev, mikey, linux-kernel, linuxppc-dev, jk, imunsie,
	anton
In-Reply-To: <1412758505-23495-2-git-send-email-mikey@neuling.org>

Michael Neuling <mikey@neuling.org> writes:

> From: Ian Munsie <imunsie@au1.ibm.com>
>
> Currently spu_handle_mm_fault() is in the cell platform.
>
> This code is generically useful for other non-cell co-processors on powerpc.
>
> This patch moves this function out of the cell platform into arch/powerpc/mm so
> that others may use it.
>

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  arch/powerpc/Kconfig                                     |  4 ++++
>  arch/powerpc/include/asm/copro.h                         | 16 ++++++++++++++++
>  arch/powerpc/include/asm/spu.h                           |  5 ++---
>  arch/powerpc/mm/Makefile                                 |  1 +
>  .../{platforms/cell/spu_fault.c => mm/copro_fault.c}     | 14 ++++++--------
>  arch/powerpc/platforms/cell/Kconfig                      |  1 +
>  arch/powerpc/platforms/cell/Makefile                     |  2 +-
>  arch/powerpc/platforms/cell/spufs/fault.c                |  4 ++--
>  8 files changed, 33 insertions(+), 14 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/copro.h
>  rename arch/powerpc/{platforms/cell/spu_fault.c => mm/copro_fault.c} (89%)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 4bc7b62..8f094e9 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -603,6 +603,10 @@ config PPC_SUBPAGE_PROT
>  	  to set access permissions (read/write, readonly, or no access)
>  	  on the 4k subpages of each 64k page.
>  
> +config PPC_COPRO_BASE
> +	bool
> +	default n
> +
>  config SCHED_SMT
>  	bool "SMT (Hyperthreading) scheduler support"
>  	depends on PPC64 && SMP
> diff --git a/arch/powerpc/include/asm/copro.h b/arch/powerpc/include/asm/copro.h
> new file mode 100644
> index 0000000..51cae85
> --- /dev/null
> +++ b/arch/powerpc/include/asm/copro.h
> @@ -0,0 +1,16 @@
> +/*
> + * Copyright 2014 IBM Corp.
> + *
> + * 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 _ASM_POWERPC_COPRO_H
> +#define _ASM_POWERPC_COPRO_H
> +
> +int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
> +			  unsigned long dsisr, unsigned *flt);
> +
> +#endif /* _ASM_POWERPC_COPRO_H */
> diff --git a/arch/powerpc/include/asm/spu.h b/arch/powerpc/include/asm/spu.h
> index 37b7ca3..a6e6e2b 100644
> --- a/arch/powerpc/include/asm/spu.h
> +++ b/arch/powerpc/include/asm/spu.h
> @@ -27,6 +27,8 @@
>  #include <linux/workqueue.h>
>  #include <linux/device.h>
>  #include <linux/mutex.h>
> +#include <asm/reg.h>
> +#include <asm/copro.h>
>  
>  #define LS_SIZE (256 * 1024)
>  #define LS_ADDR_MASK (LS_SIZE - 1)
> @@ -277,9 +279,6 @@ void spu_remove_dev_attr(struct device_attribute *attr);
>  int spu_add_dev_attr_group(struct attribute_group *attrs);
>  void spu_remove_dev_attr_group(struct attribute_group *attrs);
>  
> -int spu_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
> -		unsigned long dsisr, unsigned *flt);
> -
>  /*
>   * Notifier blocks:
>   *
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index d0130ff..325e861 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_TRANSPARENT_HUGEPAGE) += hugepage-hash64.o
>  obj-$(CONFIG_PPC_SUBPAGE_PROT)	+= subpage-prot.o
>  obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-noncoherent.o
>  obj-$(CONFIG_HIGHMEM)		+= highmem.o
> +obj-$(CONFIG_PPC_COPRO_BASE)	+= copro_fault.o
> diff --git a/arch/powerpc/platforms/cell/spu_fault.c b/arch/powerpc/mm/copro_fault.c
> similarity index 89%
> rename from arch/powerpc/platforms/cell/spu_fault.c
> rename to arch/powerpc/mm/copro_fault.c
> index 641e727..ba7df14 100644
> --- a/arch/powerpc/platforms/cell/spu_fault.c
> +++ b/arch/powerpc/mm/copro_fault.c
> @@ -1,5 +1,5 @@
>  /*
> - * SPU mm fault handler
> + * CoProcessor (SPU/AFU) mm fault handler
>   *
>   * (C) Copyright IBM Deutschland Entwicklung GmbH 2007
>   *
> @@ -23,16 +23,14 @@
>  #include <linux/sched.h>
>  #include <linux/mm.h>
>  #include <linux/export.h>
> -
> -#include <asm/spu.h>
> -#include <asm/spu_csa.h>
> +#include <asm/reg.h>
>  
>  /*
>   * This ought to be kept in sync with the powerpc specific do_page_fault
>   * function. Currently, there are a few corner cases that we haven't had
>   * to handle fortunately.
>   */
> -int spu_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
> +int copro_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
>  		unsigned long dsisr, unsigned *flt)
>  {
>  	struct vm_area_struct *vma;
> @@ -58,12 +56,12 @@ int spu_handle_mm_fault(struct mm_struct *mm, unsigned long ea,
>  			goto out_unlock;
>  	}
>  
> -	is_write = dsisr & MFC_DSISR_ACCESS_PUT;
> +	is_write = dsisr & DSISR_ISSTORE;
>  	if (is_write) {
>  		if (!(vma->vm_flags & VM_WRITE))
>  			goto out_unlock;
>  	} else {
> -		if (dsisr & MFC_DSISR_ACCESS_DENIED)
> +		if (dsisr & DSISR_PROTFAULT)
>  			goto out_unlock;
>  		if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
>  			goto out_unlock;
> @@ -91,4 +89,4 @@ out_unlock:
>  	up_read(&mm->mmap_sem);
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(spu_handle_mm_fault);
> +EXPORT_SYMBOL_GPL(copro_handle_mm_fault);
> diff --git a/arch/powerpc/platforms/cell/Kconfig b/arch/powerpc/platforms/cell/Kconfig
> index 9978f59..870b6db 100644
> --- a/arch/powerpc/platforms/cell/Kconfig
> +++ b/arch/powerpc/platforms/cell/Kconfig
> @@ -86,6 +86,7 @@ config SPU_FS_64K_LS
>  config SPU_BASE
>  	bool
>  	default n
> +	select PPC_COPRO_BASE
>  
>  config CBE_RAS
>  	bool "RAS features for bare metal Cell BE"
> diff --git a/arch/powerpc/platforms/cell/Makefile b/arch/powerpc/platforms/cell/Makefile
> index fe053e7..2d16884 100644
> --- a/arch/powerpc/platforms/cell/Makefile
> +++ b/arch/powerpc/platforms/cell/Makefile
> @@ -20,7 +20,7 @@ spu-manage-$(CONFIG_PPC_CELL_COMMON)	+= spu_manage.o
>  
>  obj-$(CONFIG_SPU_BASE)			+= spu_callbacks.o spu_base.o \
>  					   spu_notify.o \
> -					   spu_syscalls.o spu_fault.o \
> +					   spu_syscalls.o \
>  					   $(spu-priv1-y) \
>  					   $(spu-manage-y) \
>  					   spufs/
> diff --git a/arch/powerpc/platforms/cell/spufs/fault.c b/arch/powerpc/platforms/cell/spufs/fault.c
> index 8cb6260..e45894a 100644
> --- a/arch/powerpc/platforms/cell/spufs/fault.c
> +++ b/arch/powerpc/platforms/cell/spufs/fault.c
> @@ -138,7 +138,7 @@ int spufs_handle_class1(struct spu_context *ctx)
>  	if (ctx->state == SPU_STATE_RUNNABLE)
>  		ctx->spu->stats.hash_flt++;
>  
> -	/* we must not hold the lock when entering spu_handle_mm_fault */
> +	/* we must not hold the lock when entering copro_handle_mm_fault */
>  	spu_release(ctx);
>  
>  	access = (_PAGE_PRESENT | _PAGE_USER);
> @@ -149,7 +149,7 @@ int spufs_handle_class1(struct spu_context *ctx)
>  
>  	/* hashing failed, so try the actual fault handler */
>  	if (ret)
> -		ret = spu_handle_mm_fault(current->mm, ea, dsisr, &flt);
> +		ret = copro_handle_mm_fault(current->mm, ea, dsisr, &flt);
>  
>  	/*
>  	 * This is nasty: we need the state_mutex for all the bookkeeping even
> -- 
> 1.9.1
>
> --
> 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

* [PATCH] net: fs_enet: error: 'SCCE_ENET_TXF' undeclared
From: Christophe Leroy @ 2014-10-09 14:54 UTC (permalink / raw)
  To: Pantelis Antoniou, Vitaly Bordug, davem
  Cc: netdev, linuxppc-dev, linux-kernel

[linux-devel:devel-hourly-2014100909 3763/3915] drivers/net/ethernet/freescale/fs_enet/mac-scc.c:119:32: error: 'SCCE_ENET_TXF' undeclared

Due to patch d43a396 net: fs_enet: Add NAPI TX, it appears that some target
compilations are broken.
This is due to the fact that unlike the FEC, the SCC and FCC don't have a TXF
event (complete Frame transmitted) but only TXB (buffer transmitted).

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
 drivers/net/ethernet/freescale/fs_enet/mac-fcc.c | 2 +-
 drivers/net/ethernet/freescale/fs_enet/mac-scc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
index 2c578db..08f5b91 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-fcc.c
@@ -125,7 +125,7 @@ out:
 }
 
 #define FCC_NAPI_RX_EVENT_MSK	(FCC_ENET_RXF | FCC_ENET_RXB)
-#define FCC_NAPI_TX_EVENT_MSK	(FCC_ENET_TXF | FCC_ENET_TXB)
+#define FCC_NAPI_TX_EVENT_MSK	(FCC_ENET_TXB)
 #define FCC_RX_EVENT		(FCC_ENET_RXF)
 #define FCC_TX_EVENT		(FCC_ENET_TXB)
 #define FCC_ERR_EVENT_MSK	(FCC_ENET_TXE)
diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-scc.c b/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
index 5b592a6..90c9652 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-scc.c
@@ -117,7 +117,7 @@ static int do_pd_setup(struct fs_enet_private *fep)
 }
 
 #define SCC_NAPI_RX_EVENT_MSK	(SCCE_ENET_RXF | SCCE_ENET_RXB)
-#define SCC_NAPI_TX_EVENT_MSK	(SCCE_ENET_TXF | SCCE_ENET_TXB)
+#define SCC_NAPI_TX_EVENT_MSK	(SCCE_ENET_TXB)
 #define SCC_RX_EVENT		(SCCE_ENET_RXF)
 #define SCC_TX_EVENT		(SCCE_ENET_TXB)
 #define SCC_ERR_EVENT_MSK	(SCCE_ENET_TXE | SCCE_ENET_BSY)
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH 08/44] kernel: Move pm_power_off to common code
From: Guenter Roeck @ 2014-10-09 13:14 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Steven Miao,
	linux-xtensa, Boris Ostrovsky, Catalin Marinas, Will Deacon,
	David Howells, Max Filippov, Paul Mackerras, Ralf Baechle,
	H. Peter Anvin, Guan Xuetao, Thomas Gleixner, Lennox Wu,
	Hans-Christian Egtvedt, devel, linux-s390, Jesper Nilsson, lguest,
	Russell King, linux-c6x-dev, Len Brown, David S. Miller,
	linux-hexagon, Hirokazu Takata, linux-sh, James E.J. Bottomley,
	linux-acpi, Ingo Molnar, Geert Uytterhoeven, Mark Salter,
	xen-devel, Matt Turner, Chen Liqin, Jonas Bonn,
	Haavard Skinnemoen, devicetree, James Hogan,
	user-mode-linux-devel, linux-pm, Aurelien Jacquiot,
	Heiko Carstens, Jeff Dike, adi-buildroot-devel, Chris Metcalf,
	Konrad Rzeszutek Wilk, Mikael Starvik, Richard Weinberger,
	linux-m68k, linux-am33-list, Ivan Kokshaysky, linux-tegra,
	openipmi-developer, linux-metag, linux-arm-kernel,
	Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
	linux-parisc, linux-cris-kernel, Vineet Gupta, Rafael J. Wysocki,
	linux-kernel, Fenghua Yu, Richard Kuo, David Vrabel, linux-alpha,
	Martin Schwidefsky, Koichi Yasutake, linuxppc-dev, Helge Deller
In-Reply-To: <20141009103847.GC6787@amd>

On 10/09/2014 03:38 AM, Pavel Machek wrote:
> Hi!
>
>> @@ -184,6 +179,8 @@ machine_halt(void)
>>   void
>>   machine_power_off(void)
>>   {
>> +	do_kernel_poweroff();
>> +
>
> poweroff -> power_off for consistency.
>
Dunno; matter of personal preference. I started with that, but ultimately went
with poweroff to distinguish poweroff handler functions from existing code,
specifically kernel_power_off().

Does anyone else have an opinion ?

>
>> index c4f50a3..1da27d1 100644
>> --- a/arch/blackfin/kernel/reboot.c
>> +++ b/arch/blackfin/kernel/reboot.c
>> @@ -106,6 +107,7 @@ void machine_halt(void)
>>   __attribute__((weak))
>>   void native_machine_power_off(void)
>>   {
>> +	do_kernel_poweroff();
>>   	idle_with_irq_disabled();
>>   }
>>
>
> So here we handle do_kernel_poweroff() returning,
>
>> diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c
>> index b78498e..eaafad0 100644
>> --- a/arch/cris/kernel/process.c
>> +++ b/arch/cris/kernel/process.c
>> @@ -60,6 +57,7 @@ void machine_halt(void)
>>
>>   void machine_power_off(void)
>>   {
>> +	do_kernel_poweroff();
>>   }
>>
>
>
> Here we don't.
>
>> diff --git a/arch/frv/kernel/process.c b/arch/frv/kernel/process.c
>> index 5d40aeb77..a673725 100644
>> --- a/arch/frv/kernel/process.c
>> +++ b/arch/frv/kernel/process.c
>> @@ -107,6 +104,8 @@ void machine_power_off(void)
>>   	gdbstub_exit(0);
>>   #endif
>>
>> +	do_kernel_poweroff();
>> +
>>   	for (;;);
>>   }
>>
>
> And here we do.
>
> What is right?
> 								Pavel

Up to the architecture maintainer to decide. My goal was to not change
existing behavior if no poweroff handler is registered.

Guenter

^ permalink raw reply

* Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Lee Jones @ 2014-10-09 13:33 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Catalin Marinas, Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <54368D16.40404@roeck-us.net>

On Thu, 09 Oct 2014, Guenter Roeck wrote:

> On 10/09/2014 03:49 AM, Lee Jones wrote:
> >On Thu, 09 Oct 2014, Catalin Marinas wrote:
> >
> >>On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> >>>On Mon, 06 Oct 2014, Guenter Roeck wrote:
> >>>>--- a/drivers/mfd/ab8500-sysctrl.c
> >>>>+++ b/drivers/mfd/ab8500-sysctrl.c
> >>>>@@ -6,6 +6,7 @@
> >>>
> >>>[...]
> >>>
> >>>>+static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
> >>>>+			    void *unused2)
> >>>>  {
> >>>>  	sigset_t old;
> >>>>  	sigset_t all;
> >>>>@@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> >>>>  	struct power_supply *psy;
> >>>>  	int ret;
> >>>>
> >>>>-	if (sysctrl_dev == NULL) {
> >>>>-		pr_err("%s: sysctrl not initialized\n", __func__);
> >>>>-		return;
> >>>>-	}
> >>>
> >>>Can you explain the purpose of this change please?
> >>
> >>I guess it's because the sysctrl_dev is already initialised when
> >>registering the power_off handler, so there isn't a way to call the
> >>above function with a NULL sysctrl_dev. Probably even with the original
> >>code you didn't need this check (after some race fix in
> >>ab8500_sysctrl_remove but races is one of the things Guenter's patches
> >>try to address).
> >
> >Sounds reasonable, although I think this change should be part of
> >another patch.
> >
> Sure, no problem. I'll split this into two patches.
> 
> Since we are at it, any idea what to do with the restart function
> in the same file ? It is not used anywhere.

You can strip it out with Linus Walleij's Ack.  Or I'll be happy to do
it?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 01/44] kernel: Add support for poweroff handler call chain
From: Guenter Roeck @ 2014-10-09 13:25 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-m32r-ja, Linux MIPS Mailing List, linux-efi,
	linux-ia64@vger.kernel.org, Heiko Stuebner, Len Brown,
	linux-xtensa@linux-xtensa.org, Pavel Machek, driverdevel,
	linux-s390, lguest, linux-c6x-dev, open list:QUALCOMM HEXAGON...,
	Linux-sh list, Alexander Graf, ACPI Devel Maling List, xen-devel,
	devicetree@vger.kernel.org, uml-devel, Linux PM list,
	adi-buildroot-devel@lists.sourceforge.net, linux-m68k,
	moderated list:PANASONIC MN10300..., linux-tegra,
	openipmi-developer, METAG ARCHITECTURE,
	linux-arm-kernel@lists.infradead.org, Parisc List, Cris,
	Rafael J. Wysocki, linux-kernel@vger.kernel.org, alpha,
	Andrew Morton, Romain Perier, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CAMuHMdVOBnZ=pyVeGSxbOT9MtRR2iNY4V-PUm0NU=UFQ2pxE_g@mail.gmail.com>

On 10/09/2014 04:31 AM, Geert Uytterhoeven wrote:
> On Tue, Oct 7, 2014 at 7:28 AM, Guenter Roeck <linux@roeck-us.net> wrote:
>> +int register_poweroff_handler_simple(void (*handler)(void), int priority)
>> +{
>> +       char symname[KSYM_NAME_LEN];
>> +
>> +       if (poweroff_handler_data.handler) {
>> +               lookup_symbol_name((unsigned long)poweroff_handler_data.handler,
>> +                                  symname);
>> +               pr_warn("Poweroff function already registered (%s)", symname);
>> +               lookup_symbol_name((unsigned long)handler, symname);
>> +               pr_cont(", cannot register %s\n", symname);
>
> Doesn't %ps work to look up symbols?
>
> pr_warn("Poweroff function already registered (%ps), cannot register
> %ps\n", poweroff_handler_data.handler, handler);
>

Hi Geert,

That is great. One never stops learning. I'll use that.

Thanks!

Guenter

^ permalink raw reply

* Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Guenter Roeck @ 2014-10-09 13:26 UTC (permalink / raw)
  To: Lee Jones, Catalin Marinas
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141009104927.GN20647@lee--X1>

On 10/09/2014 03:49 AM, Lee Jones wrote:
> On Thu, 09 Oct 2014, Catalin Marinas wrote:
>
>> On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
>>> On Mon, 06 Oct 2014, Guenter Roeck wrote:
>>>> --- a/drivers/mfd/ab8500-sysctrl.c
>>>> +++ b/drivers/mfd/ab8500-sysctrl.c
>>>> @@ -6,6 +6,7 @@
>>>
>>> [...]
>>>
>>>> +static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
>>>> +			    void *unused2)
>>>>   {
>>>>   	sigset_t old;
>>>>   	sigset_t all;
>>>> @@ -34,11 +36,6 @@ static void ab8500_power_off(void)
>>>>   	struct power_supply *psy;
>>>>   	int ret;
>>>>
>>>> -	if (sysctrl_dev == NULL) {
>>>> -		pr_err("%s: sysctrl not initialized\n", __func__);
>>>> -		return;
>>>> -	}
>>>
>>> Can you explain the purpose of this change please?
>>
>> I guess it's because the sysctrl_dev is already initialised when
>> registering the power_off handler, so there isn't a way to call the
>> above function with a NULL sysctrl_dev. Probably even with the original
>> code you didn't need this check (after some race fix in
>> ab8500_sysctrl_remove but races is one of the things Guenter's patches
>> try to address).
>
> Sounds reasonable, although I think this change should be part of
> another patch.
>
Sure, no problem. I'll split this into two patches.

Since we are at it, any idea what to do with the restart function
in the same file ? It is not used anywhere.

Guenter

^ permalink raw reply

* Re: [PATCH 03/44] hibernate: Call have_kernel_poweroff instead of checking pm_power_off
From: Guenter Roeck @ 2014-10-09 13:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Len Brown,
	linux-xtensa, devel, linux-s390, lguest, linux-c6x-dev,
	linux-hexagon, linux-sh, linux-acpi, xen-devel, devicetree,
	user-mode-linux-devel, linux-pm, adi-buildroot-devel, linux-m68k,
	linux-am33-list, linux-tegra, openipmi-developer, linux-metag,
	linux-arm-kernel, linux-parisc, linux-cris-kernel,
	Rafael J. Wysocki, linux-kernel, linux-alpha, linuxppc-dev
In-Reply-To: <20141009103254.GB6787@amd>

On 10/09/2014 03:32 AM, Pavel Machek wrote:
> On Mon 2014-10-06 22:28:05, Guenter Roeck wrote:
>> Poweroff handlers may now be installed with register_poweroff_handler.
>> Use the new API function have_kernel_poweroff to determine if a poweroff
>> handler has been installed.
>>
>> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
>> Cc: Pavel Machek <pavel@ucw.cz>
>> Cc: Len Brown <len.brown@intel.com>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   kernel/power/hibernate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index a9dfa79..20353c5 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -602,7 +602,7 @@ static void power_down(void)
>>   	case HIBERNATION_PLATFORM:
>>   		hibernation_platform_enter();
>>   	case HIBERNATION_SHUTDOWN:
>> -		if (pm_power_off)
>> +		if (have_kernel_poweroff())
>>   			kernel_power_off();
>>   		break;
>
> poweroff -> power_off.
>
As mentioned in my other reply, that was on purpose to distinguish
existing functions from poweroff handler functions.

> But if you are playing with this, anyway... does it make sense to
> introduce kernel_power_off() that just works, no need to check
> have_..?
> 									Pavel

I am trying not to change existing behavior.

kernel_power_off is an existing function which does some cleanup
before calling machine_power_off which in turn calls do_kernel_poweroff
(or currently pm_power_off and may do some other machine specific stuff.

Sure, poweroff handling could be unified further. We could decide to
enter an endless loop if machine_power_off() returns, or we could decide
to dump a warning or panic in this case. But that is all separate from
the issue I am trying to solve here, which is to provide a capability to
register more than one poweroff handler. It would also not be that simple,
since some architectures call machine_power_off() directly from various
places.

Guenter

^ permalink raw reply

* [PATCH V3] powerpc, xmon: Enable HW instruction breakpoint on POWER8
From: Anshuman Khandual @ 2014-10-09 11:48 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: mikey

This patch enables support for hardware instruction breakpoints
on POWER8 with the help of a new register CIABR (Completed
Instruction Address Breakpoint Register). With this patch, single
hardware instruction breakpoint can be added and cleared during
any active xmon debug session. This hardware based instruction
breakpoint mechanism works correctly along with the existing TRAP
based instruction breakpoints available on xmon.

With this new patch, we would be able to interact with xmon debugger
as demonstrated in the following sample debug session.
-----------------------------
(A) Start xmon session:
$echo x > /proc/sysrq-trigger
SysRq : Entering xmon
cpu 0x0: Vector: 0  at [c000001f6c67f960]
    pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
    lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
    sp: c000001f6c67fac0
   msr: 9000000000009032
  current = 0xc000001f6e709ac0
  paca    = 0xc00000000fffa000 softe: 0 irq_happened: 0x00
    pid   = 3250, comm = bash
enter ? for help
0:mon> b
   type            address

(B) Set one HW instruction breakpoint:
0:mon> ls .power_pmu_add
.power_pmu_add: c000000000078f50
0:mon> bi c000000000078f50
0:mon> b
   type            address
 1 inst   c000000000078f50  .power_pmu_add+0x0/0x2e0
0:mon> ls .perf_event_interrupt
.perf_event_interrupt: c00000000007aee0
0:mon> bi c00000000007aee0
One instruction breakpoint possible with CIABR [as expected]
0:mon> x

(C) Run the sampple workload [with the breakpoint in place]:
$./perf record ls
cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
    pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
    lr: c000000000155be0: .event_sched_in+0x90/0x1d0
    sp: c000001f71813620
   msr: 9000000040109032
  current = 0xc000001f6ce30000
  paca    = 0xc00000000fffa600 softe: 0 irq_happened: 0x01
    pid   = 3270, comm = ls
        std     r22,-80(r1)
enter ? for help

(D) Clear the breakpoint:
2:mon> bc
All breakpoints cleared
2:mon> x
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]

(E) Run the workload again [without any breakpoints]:
$./perf record ls
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
---------------------------
Changes in V3:
- Moved the 'ciabr_used' early init inside 'cmds' function
- Some minor code cleanup
- Added more in-code documentation
- Changed the commit message

Changes in V2: (http://patchwork.ozlabs.org/patch/373114/)
- Fixed the compilation problem in 32 bit archs
- Selective inclusion of plapr_set_ciabr for required platforms
- Cleaned up the white space issues

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/xmon.h |  6 +++
 arch/powerpc/xmon/xmon.c        | 83 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/xmon.h b/arch/powerpc/include/asm/xmon.h
index 5eb8e59..5d17aec 100644
--- a/arch/powerpc/include/asm/xmon.h
+++ b/arch/powerpc/include/asm/xmon.h
@@ -29,5 +29,11 @@ static inline void xmon_register_spus(struct list_head *list) { };
 extern int cpus_are_in_xmon(void);
 #endif
 
+#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_SPLPAR)
+#include <asm/plpar_wrappers.h>
+#else
+static inline long plapr_set_ciabr(unsigned long ciabr) {return 0; };
+#endif
+
 #endif /* __KERNEL __ */
 #endif /* __ASM_POWERPC_XMON_H */
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index b988b5a..c2f601a 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -90,6 +90,7 @@ struct bpt {
 /* Bits in bpt.enabled */
 #define BP_IABR_TE	1		/* IABR translation enabled */
 #define BP_IABR		2
+#define BP_CIABR	4
 #define BP_TRAP		8
 #define BP_DABR		0x10
 
@@ -98,6 +99,7 @@ static struct bpt bpts[NBPTS];
 static struct bpt dabr;
 static struct bpt *iabr;
 static unsigned bpinstr = 0x7fe00008;	/* trap */
+static bool ciabr_used;			/* CIABR instruction breakpoint */
 
 #define BP_NUM(bp)	((bp) - bpts + 1)
 
@@ -271,6 +273,55 @@ static inline void cinval(void *p)
 }
 
 /*
+ * write_ciabr
+ *
+ * This function writes a value to the
+ * CIARB register either directly through
+ * mtspr instruction if the kernel is in HV
+ * privilege mode or call a hypervisor function
+ * to achieve the same in case the kernel is in
+ * supervisor privilege mode.
+ */
+static void write_ciabr(unsigned long ciabr)
+{
+	if (cpu_has_feature(CPU_FTR_HVMODE)) {
+		mtspr(SPRN_CIABR, ciabr);
+		return;
+	}
+	plapr_set_ciabr(ciabr);
+}
+
+/*
+ * set_ciabr
+ *
+ * This function sets the correct privilege
+ * value into the the HW breakpoint address
+ * before writing it up in the CIABR register.
+ */
+static void set_ciabr(unsigned long addr)
+{
+	addr &= ~CIABR_PRIV;
+	if (cpu_has_feature(CPU_FTR_HVMODE))
+		addr |= CIABR_PRIV_HYPER;
+	else
+		addr |= CIABR_PRIV_SUPER;
+	write_ciabr(addr);
+}
+
+/*
+ * clear_ciabr
+ *
+ * This function clears the CIABR register
+ * which in turn removes the hardware
+ * instruction breakpoint from the CPU.
+ */
+static void clear_ciabr(void)
+{
+	if (cpu_has_feature(CPU_FTR_ARCH_207S))
+		write_ciabr(0);
+}
+
+/*
  * Disable surveillance (the service processor watchdog function)
  * while we are in xmon.
  * XXX we should re-enable it when we leave. :)
@@ -767,6 +818,9 @@ static void insert_cpu_bpts(void)
 	if (iabr && cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, iabr->address
 			 | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
+
+	if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
+		set_ciabr(iabr->address);
 }
 
 static void remove_bpts(void)
@@ -794,6 +848,7 @@ static void remove_cpu_bpts(void)
 	hw_breakpoint_disable();
 	if (cpu_has_feature(CPU_FTR_IABR))
 		mtspr(SPRN_IABR, 0);
+	clear_ciabr();
 }
 
 /* Command interpreting routine */
@@ -806,6 +861,7 @@ cmds(struct pt_regs *excp)
 
 	last_cmd = NULL;
 	xmon_regs = excp;
+	ciabr_used = false;
 
 	if (!xmon_no_auto_backtrace) {
 		xmon_no_auto_backtrace = 1;
@@ -1127,7 +1183,7 @@ static char *breakpoint_help_string =
     "b <addr> [cnt]   set breakpoint at given instr addr\n"
     "bc               clear all breakpoints\n"
     "bc <n/addr>      clear breakpoint number n or at addr\n"
-    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64 only)\n"
+    "bi <addr> [cnt]  set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
     "bd <addr> [cnt]  set hardware data breakpoint\n"
     "";
 
@@ -1166,11 +1222,21 @@ bpt_cmds(void)
 		break;
 
 	case 'i':	/* bi - hardware instr breakpoint */
-		if (!cpu_has_feature(CPU_FTR_IABR)) {
+		if (!cpu_has_feature(CPU_FTR_IABR) &&
+					!cpu_has_feature(CPU_FTR_ARCH_207S)) {
 			printf("Hardware instruction breakpoint "
 			       "not supported on this cpu\n");
 			break;
 		}
+
+		if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
+			if (ciabr_used) {
+				printf("One instruction breakpoint "
+					"possible with CIABR\n");
+				break;
+			}
+		}
+
 		if (iabr) {
 			iabr->enabled &= ~(BP_IABR | BP_IABR_TE);
 			iabr = NULL;
@@ -1181,7 +1247,11 @@ bpt_cmds(void)
 			break;
 		bp = new_breakpoint(a);
 		if (bp != NULL) {
-			bp->enabled |= BP_IABR | BP_IABR_TE;
+			if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
+				bp->enabled |= BP_CIABR;
+				ciabr_used = true;
+			} else
+				bp->enabled |= BP_IABR | BP_IABR_TE;
 			iabr = bp;
 		}
 		break;
@@ -1194,6 +1264,7 @@ bpt_cmds(void)
 				bpts[i].enabled = 0;
 			iabr = NULL;
 			dabr.enabled = 0;
+			ciabr_used = false;
 			printf("All breakpoints cleared\n");
 			break;
 		}
@@ -1210,6 +1281,9 @@ bpt_cmds(void)
 			}
 		}
 
+		if (bp->enabled & BP_CIABR)
+			ciabr_used = false;
+
 		printf("Cleared breakpoint %lx (", BP_NUM(bp));
 		xmon_print_symbol(bp->address, " ", ")\n");
 		bp->enabled = 0;
@@ -1238,7 +1312,8 @@ bpt_cmds(void)
 				if (!bp->enabled)
 					continue;
 				printf("%2x %s   ", BP_NUM(bp),
-				    (bp->enabled & BP_IABR)? "inst": "trap");
+					(bp->enabled & (BP_IABR | BP_CIABR))
+							? "inst" : "trap");
 				xmon_print_symbol(bp->address, "  ", "\n");
 			}
 			break;
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH 01/44] kernel: Add support for poweroff handler call chain
From: Geert Uytterhoeven @ 2014-10-09 11:31 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja, Linux MIPS Mailing List, linux-efi,
	linux-ia64@vger.kernel.org, Heiko Stuebner, Len Brown,
	linux-xtensa@linux-xtensa.org, Pavel Machek, driverdevel,
	linux-s390, lguest, linux-c6x-dev, open list:QUALCOMM HEXAGON...,
	Linux-sh list, Alexander Graf, ACPI Devel Maling List, xen-devel,
	devicetree@vger.kernel.org, uml-devel, Linux PM list,
	adi-buildroot-devel@lists.sourceforge.net, linux-m68k,
	moderated list:PANASONIC MN10300..., linux-tegra,
	openipmi-developer, open list:METAG ARCHITECTURE,
	linux-arm-kernel@lists.infradead.org, Parisc List, Cris,
	Rafael J. Wysocki, linux-kernel@vger.kernel.org, alpha,
	Andrew Morton, Romain Perier, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1412659726-29957-2-git-send-email-linux@roeck-us.net>

On Tue, Oct 7, 2014 at 7:28 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> +int register_poweroff_handler_simple(void (*handler)(void), int priority)
> +{
> +       char symname[KSYM_NAME_LEN];
> +
> +       if (poweroff_handler_data.handler) {
> +               lookup_symbol_name((unsigned long)poweroff_handler_data.handler,
> +                                  symname);
> +               pr_warn("Poweroff function already registered (%s)", symname);
> +               lookup_symbol_name((unsigned long)handler, symname);
> +               pr_cont(", cannot register %s\n", symname);

Doesn't %ps work to look up symbols?

pr_warn("Poweroff function already registered (%ps), cannot register
%ps\n", poweroff_handler_data.handler, handler);

> +               return -EBUSY;
> +       }

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 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Lee Jones @ 2014-10-09 10:49 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, Guenter Roeck,
	linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141009103656.GF17836@e104818-lin.cambridge.arm.com>

On Thu, 09 Oct 2014, Catalin Marinas wrote:

> On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> > On Mon, 06 Oct 2014, Guenter Roeck wrote:
> > > --- a/drivers/mfd/ab8500-sysctrl.c
> > > +++ b/drivers/mfd/ab8500-sysctrl.c
> > > @@ -6,6 +6,7 @@
> > 
> > [...]
> > 
> > > +static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
> > > +			    void *unused2)
> > >  {
> > >  	sigset_t old;
> > >  	sigset_t all;
> > > @@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> > >  	struct power_supply *psy;
> > >  	int ret;
> > >  
> > > -	if (sysctrl_dev == NULL) {
> > > -		pr_err("%s: sysctrl not initialized\n", __func__);
> > > -		return;
> > > -	}
> > 
> > Can you explain the purpose of this change please?
> 
> I guess it's because the sysctrl_dev is already initialised when
> registering the power_off handler, so there isn't a way to call the
> above function with a NULL sysctrl_dev. Probably even with the original
> code you didn't need this check (after some race fix in
> ab8500_sysctrl_remove but races is one of the things Guenter's patches
> try to address).

Sounds reasonable, although I think this change should be part of
another patch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 32/44] arm64: psci: Register with kernel poweroff handler
From: Catalin Marinas @ 2014-10-09 10:41 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-xtensa@linux-xtensa.org,
	Will Deacon, devel@driverdev.osuosl.org,
	linux-s390@vger.kernel.org, lguest@lists.ozlabs.org,
	linux-c6x-dev@linux-c6x.org, linux-hexagon@vger.kernel.org,
	linux-sh@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, devicetree@vger.kernel.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-parisc@vger.kernel.org, linux-cris-kernel@axis.com,
	linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1412659726-29957-33-git-send-email-linux@roeck-us.net>

On Tue, Oct 07, 2014 at 06:28:34AM +0100, Guenter Roeck wrote:
> Register with kernel poweroff handler instead of setting pm_power_off
> directly.
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  arch/arm64/kernel/psci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
> index 5539547..c1f3d09 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -286,7 +286,7 @@ static int __init psci_0_2_init(struct device_node *np)
>  
>  	arm_pm_restart = psci_sys_reset;
>  
> -	pm_power_off = psci_sys_poweroff;
> +	register_poweroff_handler_simple(psci_sys_poweroff, 128);
>  
>  out_put_node:
>  	of_node_put(np);

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* Re: [PATCH 08/44] kernel: Move pm_power_off to common code
From: Pavel Machek @ 2014-10-09 10:38 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Steven Miao,
	linux-xtensa, Boris Ostrovsky, Catalin Marinas, Will Deacon,
	David Howells, Max Filippov, Paul Mackerras, Ralf Baechle,
	H. Peter Anvin, Guan Xuetao, Thomas Gleixner, Lennox Wu,
	Hans-Christian Egtvedt, devel, linux-s390, Jesper Nilsson, lguest,
	Russell King, linux-c6x-dev, Len Brown, David S. Miller,
	linux-hexagon, Hirokazu Takata, linux-sh, James E.J. Bottomley,
	linux-acpi, Ingo Molnar, Geert Uytterhoeven, Mark Salter,
	xen-devel, Matt Turner, Chen Liqin, Jonas Bonn,
	Haavard Skinnemoen, devicetree, James Hogan,
	user-mode-linux-devel, linux-pm, Aurelien Jacquiot,
	Heiko Carstens, Jeff Dike, adi-buildroot-devel, Chris Metcalf,
	Konrad Rzeszutek Wilk, Mikael Starvik, Richard Weinberger,
	linux-m68k, linux-am33-list, Ivan Kokshaysky, linux-tegra,
	openipmi-developer, linux-metag, linux-arm-kernel,
	Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
	linux-parisc, linux-cris-kernel, Vineet Gupta, Rafael J. Wysocki,
	linux-kernel, Fenghua Yu, Richard Kuo, David Vrabel, linux-alpha,
	Martin Schwidefsky, Koichi Yasutake, linuxppc-dev, Helge Deller
In-Reply-To: <1412659726-29957-9-git-send-email-linux@roeck-us.net>

Hi!

> @@ -184,6 +179,8 @@ machine_halt(void)
>  void
>  machine_power_off(void)
>  {
> +	do_kernel_poweroff();
> +

poweroff -> power_off for consistency.


> index c4f50a3..1da27d1 100644
> --- a/arch/blackfin/kernel/reboot.c
> +++ b/arch/blackfin/kernel/reboot.c
> @@ -106,6 +107,7 @@ void machine_halt(void)
>  __attribute__((weak))
>  void native_machine_power_off(void)
>  {
> +	do_kernel_poweroff();
>  	idle_with_irq_disabled();
>  }
>

So here we handle do_kernel_poweroff() returning,

> diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c
> index b78498e..eaafad0 100644
> --- a/arch/cris/kernel/process.c
> +++ b/arch/cris/kernel/process.c
> @@ -60,6 +57,7 @@ void machine_halt(void)
>  
>  void machine_power_off(void)
>  {
> +	do_kernel_poweroff();
>  }
>


Here we don't.

> diff --git a/arch/frv/kernel/process.c b/arch/frv/kernel/process.c
> index 5d40aeb77..a673725 100644
> --- a/arch/frv/kernel/process.c
> +++ b/arch/frv/kernel/process.c
> @@ -107,6 +104,8 @@ void machine_power_off(void)
>  	gdbstub_exit(0);
>  #endif
>  
> +	do_kernel_poweroff();
> +
>  	for (;;);
>  }
>  

And here we do.

What is right?
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH 12/44] mfd: ab8500-sysctrl: Register with kernel poweroff handler
From: Catalin Marinas @ 2014-10-09 10:36 UTC (permalink / raw)
  To: Lee Jones
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org,
	Linus Walleij, devicetree@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-s390@vger.kernel.org,
	lguest@lists.ozlabs.org, linux-c6x-dev@linux-c6x.org,
	linux-hexagon@vger.kernel.org, linux-acpi@vger.kernel.org,
	xen-devel@lists.xenproject.org, Guenter Roeck,
	linux-xtensa@linux-xtensa.org,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org,
	adi-buildroot-devel@lists.sourceforge.net,
	linux-am33-list@redhat.com, linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Samuel Ortiz, linux-cris-kernel@axis.com,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20141007080048.GB25331@lee--X1>

On Tue, Oct 07, 2014 at 09:00:48AM +0100, Lee Jones wrote:
> On Mon, 06 Oct 2014, Guenter Roeck wrote:
> > --- a/drivers/mfd/ab8500-sysctrl.c
> > +++ b/drivers/mfd/ab8500-sysctrl.c
> > @@ -6,6 +6,7 @@
> 
> [...]
> 
> > +static int ab8500_power_off(struct notifier_block *this, unsigned long unused1,
> > +			    void *unused2)
> >  {
> >  	sigset_t old;
> >  	sigset_t all;
> > @@ -34,11 +36,6 @@ static void ab8500_power_off(void)
> >  	struct power_supply *psy;
> >  	int ret;
> >  
> > -	if (sysctrl_dev == NULL) {
> > -		pr_err("%s: sysctrl not initialized\n", __func__);
> > -		return;
> > -	}
> 
> Can you explain the purpose of this change please?

I guess it's because the sysctrl_dev is already initialised when
registering the power_off handler, so there isn't a way to call the
above function with a NULL sysctrl_dev. Probably even with the original
code you didn't need this check (after some race fix in
ab8500_sysctrl_remove but races is one of the things Guenter's patches
try to address).

-- 
Catalin

^ permalink raw reply

* Re: [PATCH 08/44] kernel: Move pm_power_off to common code
From: Vineet Gupta @ 2014-10-09 10:35 UTC (permalink / raw)
  To: Guenter Roeck, linux-kernel@vger.kernel.org
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-efi@vger.kernel.org, linux-ia64@vger.kernel.org,
	Steven Miao, linux-xtensa@linux-xtensa.org, Boris Ostrovsky,
	Catalin Marinas, Will Deacon, Jeff Dike, David Howells,
	Max Filippov, Paul Mackerras, Ralf Baechle, Pavel Machek,
	H. Peter Anvin, Guan Xuetao, Thomas Gleixner, Lennox Wu,
	Hans-Christian Egtvedt, devel@driverdev.osuosl.org,
	linux-s390@vger.kernel.org, Jesper Nilsson,
	lguest@lists.ozlabs.org, Russell King,
	linux-c6x-dev@linux-c6x.org, Len Brown, David S. Miller,
	linux-hexagon@vger.kernel.org, Hirokazu Takata,
	linux-sh@vger.kernel.org, James E.J. Bottomley,
	linux-acpi@vger.kernel.org, Ingo Molnar, Geert Uytterhoeven,
	Mark Salter, xen-devel@lists.xenproject.org, Matt Turner,
	Chen Liqin, Jonas Bonn, Haavard Skinnemoen,
	devicetree@vger.kernel.org, James Hogan,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org, Aurelien Jacquiot, Heiko Carstens,
	Vineet Gupta, adi-buildroot-devel@lists.sourceforge.net,
	Chris Metcalf, Konrad Rzeszutek Wilk, Mikael Starvik,
	Richard Weinberger, linux-m68k@lists.linux-m68k.org,
	linux-am33-list@redhat.com, Ivan Kokshaysky,
	linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
	linux-parisc@vger.kernel.org, linux-cris-kernel@axis.com,
	Rafael J. Wysocki, Fenghua Yu, Richard Kuo, David Vrabel,
	linux-alpha@vger.kernel.org, Martin Schwidefsky, Koichi Yasutake,
	linuxppc-dev@lists.ozlabs.org, Helge Deller
In-Reply-To: <1412659726-29957-9-git-send-email-linux@roeck-us.net>

On Tuesday 07 October 2014 11:01 AM, Guenter Roeck wrote:

diff --git a/arch/arc/kernel/reset.c b/arch/arc/kernel/reset.c
index 2768fa1..8a4fc47 100644
--- a/arch/arc/kernel/reset.c
+++ b/arch/arc/kernel/reset.c
@@ -26,9 +26,6 @@ void machine_restart(char *__unused)

 void machine_power_off(void)
 {
-       /* FIXME ::  power off ??? */
+       do_kernel_poweroff();
        machine_halt();
 }
-
-void (*pm_power_off) (void) =3D NULL;
-EXPORT_SYMBOL(pm_power_off);

Acked-by: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>

Thx,
-Vineet

^ permalink raw reply related

* Re: [PATCH 03/44] hibernate: Call have_kernel_poweroff instead of checking pm_power_off
From: Pavel Machek @ 2014-10-09 10:32 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Len Brown,
	linux-xtensa, devel, linux-s390, lguest, linux-c6x-dev,
	linux-hexagon, linux-sh, linux-acpi, xen-devel, devicetree,
	user-mode-linux-devel, linux-pm, adi-buildroot-devel, linux-m68k,
	linux-am33-list, linux-tegra, openipmi-developer, linux-metag,
	linux-arm-kernel, linux-parisc, linux-cris-kernel,
	Rafael J. Wysocki, linux-kernel, linux-alpha, linuxppc-dev
In-Reply-To: <1412659726-29957-4-git-send-email-linux@roeck-us.net>

On Mon 2014-10-06 22:28:05, Guenter Roeck wrote:
> Poweroff handlers may now be installed with register_poweroff_handler.
> Use the new API function have_kernel_poweroff to determine if a poweroff
> handler has been installed.
> 
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>  kernel/power/hibernate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index a9dfa79..20353c5 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -602,7 +602,7 @@ static void power_down(void)
>  	case HIBERNATION_PLATFORM:
>  		hibernation_platform_enter();
>  	case HIBERNATION_SHUTDOWN:
> -		if (pm_power_off)
> +		if (have_kernel_poweroff())
>  			kernel_power_off();
>  		break;

poweroff -> power_off.

But if you are playing with this, anyway... does it make sense to
introduce kernel_power_off() that just works, no need to check
have_..?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH 01/44] kernel: Add support for poweroff handler call chain
From: Pavel Machek @ 2014-10-09 10:31 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja, linux-mips, linux-efi, linux-ia64, Heiko Stuebner,
	Len Brown, linux-xtensa, devel, linux-s390, lguest, linux-c6x-dev,
	linux-hexagon, linux-sh, Alexander Graf, linux-acpi,
	Geert Uytterhoeven, xen-devel, devicetree, user-mode-linux-devel,
	linux-pm, adi-buildroot-devel, linux-m68k, linux-am33-list,
	linux-tegra, openipmi-developer, linux-metag, linux-arm-kernel,
	linux-parisc, linux-cris-kernel, Rafael J. Wysocki, linux-kernel,
	linux-alpha, Andrew Morton, Romain Perier, linuxppc-dev
In-Reply-To: <1412659726-29957-2-git-send-email-linux@roeck-us.net>

Hi!

> +/**
> + *	register_poweroff_handler_simple - Register function to be called to power off
> + *					   the system
> + *	@handler:	Function to be called to power off the system
> + *	@priority:	Handler priority. For priority guidelines see
> + *			register_poweroff_handler.
> + *
> + *	This is a simplified version of register_poweroff_handler. It does not
> + *	take a notifier as argument, but a function pointer. The function
> + *	registers a poweroff handler with specified priority. Poweroff
> + *	handlers registered with this function can not be unregistered,
> + *	and only a single poweroff handler can be installed using it.
> + *
> + *	This function must not be called from modules and is therefore
> + *	not exported.
> + *
> + *	Returns -EBUSY if a poweroff handler has already been registered
> + *	using register_poweroff_handler_simple. Otherwise returns zero,
> + *	since atomic_notifier_chain_register() currently always returns zero.
> + */
> +int register_poweroff_handler_simple(void (*handler)(void), int priority)
> +{
> +	char symname[KSYM_NAME_LEN];
> +
> +	if (poweroff_handler_data.handler) {
> +		lookup_symbol_name((unsigned long)poweroff_handler_data.handler,
> +				   symname);
> +		pr_warn("Poweroff function already registered (%s)", symname);
> +		lookup_symbol_name((unsigned long)handler, symname);
> +		pr_cont(", cannot register %s\n", symname);
> +		return -EBUSY;
> +	}

Dunno, are you maybe overdoing the debugging infrastructure a bit?
This is not going to happen in production, and if it does happen,
developer can look the symbol name himself.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH 08/44] kernel: Move pm_power_off to common code
From: Catalin Marinas @ 2014-10-09 10:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-m32r-ja@ml.linux-m32r.org, linux-mips@linux-mips.org,
	linux-m68k@vger.kernel.org, linux-efi@vger.kernel.org,
	linux-ia64@vger.kernel.org, Steven Miao,
	linux-xtensa@linux-xtensa.org, Boris Ostrovsky, Will Deacon,
	David Howells, Max Filippov, Paul Mackerras, Ralf Baechle,
	Pavel Machek, H. Peter Anvin, Guan Xuetao, Thomas Gleixner,
	Lennox Wu, Hans-Christian Egtvedt, devel@driverdev.osuosl.org,
	linux-s390@vger.kernel.org, Jesper Nilsson,
	lguest@lists.ozlabs.org, Russell King,
	linux-c6x-dev@linux-c6x.org, Len Brown, David S. Miller,
	linux-hexagon@vger.kernel.org, Hirokazu Takata,
	linux-sh@vger.kernel.org, James E.J. Bottomley,
	linux-acpi@vger.kernel.org, Ingo Molnar, Geert Uytterhoeven,
	msalter@redhat.com, xen-devel@lists.xenproject.org, Matt Turner,
	Chen Liqin, Jonas Bonn, Haavard Skinnemoen,
	devicetree@vger.kernel.org, James Hogan,
	user-mode-linux-devel@lists.sourceforge.net,
	linux-pm@vger.kernel.org, Aurelien Jacquiot, Heiko Carstens,
	Jeff Dike, adi-buildroot-devel@lists.sourceforge.net,
	Chris Metcalf, Konrad Rzeszutek Wilk, Mikael Starvik,
	Richard Weinberger, linux-am33-list@redhat.com, Ivan Kokshaysky,
	linux-tegra@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	linux-metag@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Richard Henderson, Chris Zankel, Michal Simek, Tony Luck,
	linux-parisc@vger.kernel.org, linux-cris-kernel@axis.com,
	Vineet Gupta, Rafael J. Wysocki, linux-kernel@vger.kernel.org,
	Fenghua Yu, Richard Kuo, David Vrabel,
	linux-alpha@vger.kernel.org, Martin Schwidefsky,
	yasutake.koichi@jp.panasonic.com, linuxppc-dev@lists.ozlabs.org,
	Helge Deller
In-Reply-To: <1412659726-29957-9-git-send-email-linux@roeck-us.net>

On Tue, Oct 07, 2014 at 06:28:10AM +0100, Guenter Roeck wrote:
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index e0ef8ba..db396bb 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -94,8 +94,6 @@ void soft_restart(unsigned long addr)
>  /*
>   * Function pointers to optional machine specific functions
>   */
> -void (*pm_power_off)(void);
> -EXPORT_SYMBOL_GPL(pm_power_off);
> 
>  void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd);
> 
> @@ -155,8 +153,7 @@ void machine_power_off(void)
>  {
>         local_irq_disable();
>         smp_send_stop();
> -       if (pm_power_off)
> -               pm_power_off();
> +       do_kernel_poweroff();
>  }

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ 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