LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac
From: Larry Finger @ 2019-06-13 18:51 UTC (permalink / raw)
  To: Christoph Hellwig, benh, paulus, mpe
  Cc: linuxppc-dev, linux-kernel, aaro.koskinen
In-Reply-To: <20190613082446.18685-1-hch@lst.de>

On 6/13/19 3:24 AM, Christoph Hellwig wrote:
> With the strict dma mask checking introduced with the switch to
> the generic DMA direct code common wifi chips on 32-bit powerbooks
> stopped working.  Add a 30-bit ZONE_DMA to the 32-bit pmac builds
> to allow them to reliably allocate dma coherent memory.
> 
> Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
> Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

As expected, it works. The patch needs
Cc: Stable <stable*vger.kernel.org> # v5.1+

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks for the help, and the patience with my debugging problems with u64 variables.

Larry

^ permalink raw reply

* [RFC PATCH] Replaces long number representation by BIT() macro
From: Leonardo Bras @ 2019-06-13 18:02 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Leonardo Bras, Paul Mackerras, linuxppc-dev, linux-kernel

The main reason of this change is to make these bitmasks more readable.

The macro ASM_CONST() just appends an UL to it's parameter, so it can be
easily replaced by BIT_MASK, that already uses a UL representation.

ASM_CONST() in this file may behave different if __ASSEMBLY__ is defined,
as it is used on .S files, just leaving the parameter as is.

However, I have noticed no difference in the generated binary after this
change.

Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
 arch/powerpc/include/asm/firmware.h | 75 ++++++++++++++---------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 00bc42d95679..7a5b0cc0bc85 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -14,46 +14,45 @@
 
 #ifdef __KERNEL__
 
-#include <asm/asm-const.h>
-
+#include <linux/bits.h>
 /* firmware feature bitmask values */
 
-#define FW_FEATURE_PFT		ASM_CONST(0x0000000000000001)
-#define FW_FEATURE_TCE		ASM_CONST(0x0000000000000002)
-#define FW_FEATURE_SPRG0	ASM_CONST(0x0000000000000004)
-#define FW_FEATURE_DABR		ASM_CONST(0x0000000000000008)
-#define FW_FEATURE_COPY		ASM_CONST(0x0000000000000010)
-#define FW_FEATURE_ASR		ASM_CONST(0x0000000000000020)
-#define FW_FEATURE_DEBUG	ASM_CONST(0x0000000000000040)
-#define FW_FEATURE_TERM		ASM_CONST(0x0000000000000080)
-#define FW_FEATURE_PERF		ASM_CONST(0x0000000000000100)
-#define FW_FEATURE_DUMP		ASM_CONST(0x0000000000000200)
-#define FW_FEATURE_INTERRUPT	ASM_CONST(0x0000000000000400)
-#define FW_FEATURE_MIGRATE	ASM_CONST(0x0000000000000800)
-#define FW_FEATURE_PERFMON	ASM_CONST(0x0000000000001000)
-#define FW_FEATURE_CRQ		ASM_CONST(0x0000000000002000)
-#define FW_FEATURE_VIO		ASM_CONST(0x0000000000004000)
-#define FW_FEATURE_RDMA		ASM_CONST(0x0000000000008000)
-#define FW_FEATURE_LLAN		ASM_CONST(0x0000000000010000)
-#define FW_FEATURE_BULK_REMOVE	ASM_CONST(0x0000000000020000)
-#define FW_FEATURE_XDABR	ASM_CONST(0x0000000000040000)
-#define FW_FEATURE_MULTITCE	ASM_CONST(0x0000000000080000)
-#define FW_FEATURE_SPLPAR	ASM_CONST(0x0000000000100000)
-#define FW_FEATURE_LPAR		ASM_CONST(0x0000000000400000)
-#define FW_FEATURE_PS3_LV1	ASM_CONST(0x0000000000800000)
-#define FW_FEATURE_HPT_RESIZE	ASM_CONST(0x0000000001000000)
-#define FW_FEATURE_CMO		ASM_CONST(0x0000000002000000)
-#define FW_FEATURE_VPHN		ASM_CONST(0x0000000004000000)
-#define FW_FEATURE_XCMO		ASM_CONST(0x0000000008000000)
-#define FW_FEATURE_OPAL		ASM_CONST(0x0000000010000000)
-#define FW_FEATURE_SET_MODE	ASM_CONST(0x0000000040000000)
-#define FW_FEATURE_BEST_ENERGY	ASM_CONST(0x0000000080000000)
-#define FW_FEATURE_TYPE1_AFFINITY ASM_CONST(0x0000000100000000)
-#define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
-#define FW_FEATURE_DRMEM_V2	ASM_CONST(0x0000000400000000)
-#define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
-#define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0000001000000000)
-#define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000002000000000)
+#define FW_FEATURE_PFT		BIT(0)
+#define FW_FEATURE_TCE		BIT(1)
+#define FW_FEATURE_SPRG0		BIT(2)
+#define FW_FEATURE_DABR		BIT(3)
+#define FW_FEATURE_COPY		BIT(4)
+#define FW_FEATURE_ASR		BIT(5)
+#define FW_FEATURE_DEBUG		BIT(6)
+#define FW_FEATURE_TERM		BIT(7)
+#define FW_FEATURE_PERF		BIT(8)
+#define FW_FEATURE_DUMP		BIT(9)
+#define FW_FEATURE_INTERRUPT	BIT(10)
+#define FW_FEATURE_MIGRATE	BIT(11)
+#define FW_FEATURE_PERFMON	BIT(12)
+#define FW_FEATURE_CRQ		BIT(13)
+#define FW_FEATURE_VIO		BIT(14)
+#define FW_FEATURE_RDMA		BIT(15)
+#define FW_FEATURE_LLAN		BIT(16)
+#define FW_FEATURE_BULK_REMOVE	BIT(17)
+#define FW_FEATURE_XDABR		BIT(18)
+#define FW_FEATURE_MULTITCE	BIT(19)
+#define FW_FEATURE_SPLPAR	BIT(20)
+#define FW_FEATURE_LPAR		BIT(22)
+#define FW_FEATURE_PS3_LV1	BIT(23)
+#define FW_FEATURE_HPT_RESIZE	BIT(24)
+#define FW_FEATURE_CMO		BIT(25)
+#define FW_FEATURE_VPHN		BIT(26)
+#define FW_FEATURE_XCMO		BIT(27)
+#define FW_FEATURE_OPAL		BIT(28)
+#define FW_FEATURE_SET_MODE	BIT(30)
+#define FW_FEATURE_BEST_ENERGY	BIT(31)
+#define FW_FEATURE_TYPE1_AFFINITY BIT(32)
+#define FW_FEATURE_PRRN		BIT(33)
+#define FW_FEATURE_DRMEM_V2	BIT(34)
+#define FW_FEATURE_DRC_INFO	BIT(35)
+#define FW_FEATURE_BLOCK_REMOVE	BIT(36)
+#define FW_FEATURE_PAPR_SCM	BIT(37)
 
 #ifndef __ASSEMBLY__
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2] cxl: no need to check return value of debugfs_create functions
From: Frederic Barrat @ 2019-06-13 18:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linuxppc-dev, Andrew Donnellan, Arnd Bergmann
In-Reply-To: <20190612155418.GB22739@kroah.com>



Le 12/06/2019 à 17:54, Greg Kroah-Hartman a écrit :
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 
> Because there's no need to check, also make the return value of the
> local debugfs_create_io_x64() call void, as no one ever did anything
> with the return value (as they did not need to.)
> 
> And make the cxl_debugfs_* calls return void as no one was even checking
> their return value at all.
> 
> Cc: Frederic Barrat <fbarrat@linux.ibm.com>
> Cc: Andrew Donnellan <ajd@linux.ibm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---

Thanks!
Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>


> v2: make the return value of all of the cxl_debugfs_* calls void as no
>      one was checking the return values of them.
> 
>   drivers/misc/cxl/cxl.h     | 15 ++++++---------
>   drivers/misc/cxl/debugfs.c | 36 +++++++++++-------------------------
>   2 files changed, 17 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index a73c9e669d78..5dc0f6093f9d 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -908,11 +908,11 @@ void cxl_update_dedicated_ivtes_psl8(struct cxl_context *ctx);
>   
>   #ifdef CONFIG_DEBUG_FS
>   
> -int cxl_debugfs_init(void);
> +void cxl_debugfs_init(void);
>   void cxl_debugfs_exit(void);
> -int cxl_debugfs_adapter_add(struct cxl *adapter);
> +void cxl_debugfs_adapter_add(struct cxl *adapter);
>   void cxl_debugfs_adapter_remove(struct cxl *adapter);
> -int cxl_debugfs_afu_add(struct cxl_afu *afu);
> +void cxl_debugfs_afu_add(struct cxl_afu *afu);
>   void cxl_debugfs_afu_remove(struct cxl_afu *afu);
>   void cxl_debugfs_add_adapter_regs_psl9(struct cxl *adapter, struct dentry *dir);
>   void cxl_debugfs_add_adapter_regs_psl8(struct cxl *adapter, struct dentry *dir);
> @@ -921,27 +921,24 @@ void cxl_debugfs_add_afu_regs_psl8(struct cxl_afu *afu, struct dentry *dir);
>   
>   #else /* CONFIG_DEBUG_FS */
>   
> -static inline int __init cxl_debugfs_init(void)
> +static inline void __init cxl_debugfs_init(void)
>   {
> -	return 0;
>   }
>   
>   static inline void cxl_debugfs_exit(void)
>   {
>   }
>   
> -static inline int cxl_debugfs_adapter_add(struct cxl *adapter)
> +static inline void cxl_debugfs_adapter_add(struct cxl *adapter)
>   {
> -	return 0;
>   }
>   
>   static inline void cxl_debugfs_adapter_remove(struct cxl *adapter)
>   {
>   }
>   
> -static inline int cxl_debugfs_afu_add(struct cxl_afu *afu)
> +static inline void cxl_debugfs_afu_add(struct cxl_afu *afu)
>   {
> -	return 0;
>   }
>   
>   static inline void cxl_debugfs_afu_remove(struct cxl_afu *afu)
> diff --git a/drivers/misc/cxl/debugfs.c b/drivers/misc/cxl/debugfs.c
> index 1fda22c24c93..7b987bf498b5 100644
> --- a/drivers/misc/cxl/debugfs.c
> +++ b/drivers/misc/cxl/debugfs.c
> @@ -26,11 +26,11 @@ static int debugfs_io_u64_set(void *data, u64 val)
>   DEFINE_DEBUGFS_ATTRIBUTE(fops_io_x64, debugfs_io_u64_get, debugfs_io_u64_set,
>   			 "0x%016llx\n");
>   
> -static struct dentry *debugfs_create_io_x64(const char *name, umode_t mode,
> -					    struct dentry *parent, u64 __iomem *value)
> +static void debugfs_create_io_x64(const char *name, umode_t mode,
> +				  struct dentry *parent, u64 __iomem *value)
>   {
> -	return debugfs_create_file_unsafe(name, mode, parent,
> -					  (void __force *)value, &fops_io_x64);
> +	debugfs_create_file_unsafe(name, mode, parent, (void __force *)value,
> +				   &fops_io_x64);
>   }
>   
>   void cxl_debugfs_add_adapter_regs_psl9(struct cxl *adapter, struct dentry *dir)
> @@ -54,25 +54,22 @@ void cxl_debugfs_add_adapter_regs_psl8(struct cxl *adapter, struct dentry *dir)
>   	debugfs_create_io_x64("trace", S_IRUSR | S_IWUSR, dir, _cxl_p1_addr(adapter, CXL_PSL_TRACE));
>   }
>   
> -int cxl_debugfs_adapter_add(struct cxl *adapter)
> +void cxl_debugfs_adapter_add(struct cxl *adapter)
>   {
>   	struct dentry *dir;
>   	char buf[32];
>   
>   	if (!cxl_debugfs)
> -		return -ENODEV;
> +		return;
>   
>   	snprintf(buf, 32, "card%i", adapter->adapter_num);
>   	dir = debugfs_create_dir(buf, cxl_debugfs);
> -	if (IS_ERR(dir))
> -		return PTR_ERR(dir);
>   	adapter->debugfs = dir;
>   
>   	debugfs_create_io_x64("err_ivte", S_IRUSR, dir, _cxl_p1_addr(adapter, CXL_PSL_ErrIVTE));
>   
>   	if (adapter->native->sl_ops->debugfs_add_adapter_regs)
>   		adapter->native->sl_ops->debugfs_add_adapter_regs(adapter, dir);
> -	return 0;
>   }
>   
>   void cxl_debugfs_adapter_remove(struct cxl *adapter)
> @@ -96,18 +93,16 @@ void cxl_debugfs_add_afu_regs_psl8(struct cxl_afu *afu, struct dentry *dir)
>   	debugfs_create_io_x64("trace", S_IRUSR | S_IWUSR, dir, _cxl_p1n_addr(afu, CXL_PSL_SLICE_TRACE));
>   }
>   
> -int cxl_debugfs_afu_add(struct cxl_afu *afu)
> +void cxl_debugfs_afu_add(struct cxl_afu *afu)
>   {
>   	struct dentry *dir;
>   	char buf[32];
>   
>   	if (!afu->adapter->debugfs)
> -		return -ENODEV;
> +		return;
>   
>   	snprintf(buf, 32, "psl%i.%i", afu->adapter->adapter_num, afu->slice);
>   	dir = debugfs_create_dir(buf, afu->adapter->debugfs);
> -	if (IS_ERR(dir))
> -		return PTR_ERR(dir);
>   	afu->debugfs = dir;
>   
>   	debugfs_create_io_x64("sr",         S_IRUSR, dir, _cxl_p1n_addr(afu, CXL_PSL_SR_An));
> @@ -118,8 +113,6 @@ int cxl_debugfs_afu_add(struct cxl_afu *afu)
>   
>   	if (afu->adapter->native->sl_ops->debugfs_add_afu_regs)
>   		afu->adapter->native->sl_ops->debugfs_add_afu_regs(afu, dir);
> -
> -	return 0;
>   }
>   
>   void cxl_debugfs_afu_remove(struct cxl_afu *afu)
> @@ -127,19 +120,12 @@ void cxl_debugfs_afu_remove(struct cxl_afu *afu)
>   	debugfs_remove_recursive(afu->debugfs);
>   }
>   
> -int __init cxl_debugfs_init(void)
> +void __init cxl_debugfs_init(void)
>   {
> -	struct dentry *ent;
> -
>   	if (!cpu_has_feature(CPU_FTR_HVMODE))
> -		return 0;
> -
> -	ent = debugfs_create_dir("cxl", NULL);
> -	if (IS_ERR(ent))
> -		return PTR_ERR(ent);
> -	cxl_debugfs = ent;
> +		return;
>   
> -	return 0;
> +	cxl_debugfs = debugfs_create_dir("cxl", NULL);
>   }
>   
>   void cxl_debugfs_exit(void)
> 


^ permalink raw reply

* Re: [PATCH 2/3] powerpc/pseries/mobility: prevent cpu hotplug during DT update
From: Gautham R Shenoy @ 2019-06-13 16:52 UTC (permalink / raw)
  To: Nathan Lynch; +Cc: ego, linuxppc-dev
In-Reply-To: <20190612044506.16392-3-nathanl@linux.ibm.com>

Hello Nathan,

On Wed, Jun 12, 2019 at 10:19 AM Nathan Lynch <nathanl@linux.ibm.com> wrote:
>
> CPU online/offline code paths are sensitive to parts of the device
> tree (various cpu node properties, cache nodes) that can be changed as
> a result of a migration.
>
> Prevent CPU hotplug while the device tree potentially is inconsistent.
>
> Fixes: 410bccf97881 ("powerpc/pseries: Partition migration in the kernel")
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Audited the callbacks of of_reconfig_notify(). We are fine with
respect to CPU-Hotplug locking.

Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

> ---
>  arch/powerpc/platforms/pseries/mobility.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index 88925f8ca8a0..edc1ec408589 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -9,6 +9,7 @@
>   * 2 as published by the Free Software Foundation.
>   */
>
> +#include <linux/cpu.h>
>  #include <linux/kernel.h>
>  #include <linux/kobject.h>
>  #include <linux/smp.h>
> @@ -338,11 +339,19 @@ void post_mobility_fixup(void)
>         if (rc)
>                 printk(KERN_ERR "Post-mobility activate-fw failed: %d\n", rc);
>
> +       /*
> +        * We don't want CPUs to go online/offline while the device
> +        * tree is being updated.
> +        */
> +       cpus_read_lock();
> +
>         rc = pseries_devicetree_update(MIGRATION_SCOPE);
>         if (rc)
>                 printk(KERN_ERR "Post-mobility device tree update "
>                         "failed: %d\n", rc);
>
> +       cpus_read_unlock();
> +
>         /* Possibly switch to a new RFI flush type */
>         pseries_setup_rfi_flush();
>
> --
> 2.20.1
>


-- 
Thanks and Regards
gautham.

^ permalink raw reply

* Re: [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
From: Vincenzo Frascino @ 2019-06-13 15:22 UTC (permalink / raw)
  To: Michael Ellerman, linux-arch, linuxppc-dev, linux-s390,
	linux-kselftest
  Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras, Martin Schwidefsky,
	Thomas Gleixner, Shuah Khan
In-Reply-To: <afb7395f-43e9-c304-2db2-349e6727b687@arm.com>

Hi Michael,

I wanted to check with you if you had time to have a look at my new version (v5)
of the patches with the fixed test, and if they are ready to be merged or if
there is anything else I can do.

Thanks and Regards,
Vincenzo

On 28/05/2019 12:57, Vincenzo Frascino wrote:
> Hi Michael,
> 
> thank you for your reply.
> 
> On 28/05/2019 07:19, Michael Ellerman wrote:
>> Vincenzo Frascino <vincenzo.frascino@arm.com> writes:
>>
>>> The current version of the multiarch vDSO selftest verifies only
>>> gettimeofday.
>>>
>>> Extend the vDSO selftest to clock_getres, to verify that the
>>> syscall and the vDSO library function return the same information.
>>>
>>> The extension has been used to verify the hrtimer_resoltion fix.
>>
>> This is passing for me even without patch 1 applied, shouldn't it fail
>> without the fix? What am I missing?
>>
> 
> This is correct, because during the refactoring process I missed an "n" :)
> 
> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_sec·!=·y.tv_sec))
> 
> Should be:
> 
> if·((x.tv_sec·!=·y.tv_sec)·||·(x.tv_nsec·!=·y.tv_nsec))
> 
> My mistake, I am going to fix the test and re-post v5 of this set.
> 
> Without my patch if you pass "highres=off" to the kernel (as a command line
> parameter) it leads to a broken implementation of clock_getres since the value
> of CLOCK_REALTIME_RES does not change at runtime.
> 
> Expected result (with highres=off):
> 
> # uname -r
> 5.2.0-rc2
> # ./vdso_clock_getres
> clock_id: CLOCK_REALTIME [FAIL]
> clock_id: CLOCK_BOOTTIME [PASS]
> clock_id: CLOCK_TAI [PASS]
> clock_id: CLOCK_REALTIME_COARSE [PASS]
> clock_id: CLOCK_MONOTONIC [FAIL]
> clock_id: CLOCK_MONOTONIC_RAW [PASS]
> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
> 
> The reason of this behavior is that the only clocks supported by getres on
> powerpc are CLOCK_REALTIME and CLOCK_MONOTONIC, the rest on the clocks use
> always syscalls.
> 
>> # uname -r
>> 5.2.0-rc2-gcc-8.2.0
>>
>> # ./vdso_clock_getres
>> clock_id: CLOCK_REALTIME [PASS]
>> clock_id: CLOCK_BOOTTIME [PASS]
>> clock_id: CLOCK_TAI [PASS]
>> clock_id: CLOCK_REALTIME_COARSE [PASS]
>> clock_id: CLOCK_MONOTONIC [PASS]
>> clock_id: CLOCK_MONOTONIC_RAW [PASS]
>> clock_id: CLOCK_MONOTONIC_COARSE [PASS]
>>
>> cheers
>>
>>> Cc: Shuah Khan <shuah@kernel.org>
>>> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
>>> ---
>>>
>>> Note: This patch is independent from the others in this series, hence it
>>> can be merged singularly by the kselftest maintainers.
>>>
>>>  tools/testing/selftests/vDSO/Makefile         |   2 +
>>>  .../selftests/vDSO/vdso_clock_getres.c        | 124 ++++++++++++++++++
>>>  2 files changed, 126 insertions(+)
>>>  create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
> 

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/8xx: move CPM1 related files from sysdev/ to platforms/8xx
From: kbuild test robot @ 2019-06-13 14:00 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-kernel, Scott Wood, Paul Mackerras, kbuild-all,
	linuxppc-dev
In-Reply-To: <35488171038e3d40e7680b8513dfbd52ff7b6ef2.1557487355.git.christophe.leroy@c-s.fr>

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

Hi Christophe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.2-rc4 next-20190612]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-8xx-move-CPM1-related-files-from-sysdev-to-platforms-8xx/20190613-182651
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-ep8248e_defconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> make[3]: *** No rule to make target 'arch/powerpc/sysdev/cpm_gpio.o', needed by 'arch/powerpc/sysdev/built-in.a'.
   make[3]: Target '__build' not remade because of errors.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13064 bytes --]

^ permalink raw reply

* [PATCH] powerpc/booke: fix fast syscall entry on SMP
From: Christophe Leroy @ 2019-06-13 13:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Use r10 instead of r9 to calculate CPU offset as r9 contains
the value from SRR1 which is used later.

Fixes: 1a4b739bbb4f ("powerpc/32: implement fast entry for syscalls on BOOKE")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_booke.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index dec0912a6508..2ae635df9026 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -145,9 +145,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
 	tophys(r11,r11)
 	addi	r11,r11,global_dbcr0@l
 #ifdef CONFIG_SMP
-	lwz	r9,TASK_CPU(r2)
-	slwi	r9,r9,3
-	add	r11,r11,r9
+	lwz	r10, TASK_CPU(r2)
+	slwi	r10, r10, 3
+	add	r11, r11, r10
 #endif
 	lwz	r12,0(r11)
 	mtspr	SPRN_DBCR0,r12
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH v2 4/4] crypto: talitos - drop icv_ool
From: Herbert Xu @ 2019-06-13 13:09 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	David S. Miller, Horia Geanta, linux-crypto@vger.kernel.org
In-Reply-To: <b2db68d5-d89b-2f38-d5b4-7b7eccf68204@c-s.fr>

On Thu, Jun 13, 2019 at 02:28:51PM +0200, Christophe Leroy wrote:
> 
> 
> Le 13/06/2019 à 14:21, Horia Geanta a écrit :
> > On 6/11/2019 5:39 PM, Christophe Leroy wrote:
> > > icv_ool is not used anymore, drop it.
> > > 
> > > Fixes: 9cc87bc3613b ("crypto: talitos - fix AEAD processing")
> > I can't find this SHA1.
> > 
> > Are you referring to commit e345177ded17 ("crypto: talitos - fix AEAD processing.")?
> > 
> > Horia
> > 
> 
> Oops yes, that's the sha1 it had in my tree before it got merged.
> 
> Do I have to resend it or can Herbert just drop the wrong reference and take
> the following one:

Please resend since you're going to change the other patches too.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v3 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Christophe Leroy @ 2019-06-13 12:50 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <0ada8523d5765391ddc6899815e0e1eb511bcb7d.1560429844.git.christophe.leroy@c-s.fr>



Le 13/06/2019 à 14:48, Christophe Leroy a écrit :
> Moves it into talitos.h so that it can be used
> from any place in talitos.c
> 
> This will be required for next
> patch ("crypto: talitos - fix hash on SEC1")
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Cc: stable@vger.kernel.org

> ---
>   drivers/crypto/talitos.c | 30 ------------------------------
>   drivers/crypto/talitos.h | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
> index 3b3e99f1cddb..5b401aec6c84 100644
> --- a/drivers/crypto/talitos.c
> +++ b/drivers/crypto/talitos.c
> @@ -951,36 +951,6 @@ static int aead_des3_setkey(struct crypto_aead *authenc,
>   	goto out;
>   }
>   
> -/*
> - * talitos_edesc - s/w-extended descriptor
> - * @src_nents: number of segments in input scatterlist
> - * @dst_nents: number of segments in output scatterlist
> - * @icv_ool: whether ICV is out-of-line
> - * @iv_dma: dma address of iv for checking continuity and link table
> - * @dma_len: length of dma mapped link_tbl space
> - * @dma_link_tbl: bus physical address of link_tbl/buf
> - * @desc: h/w descriptor
> - * @link_tbl: input and output h/w link tables (if {src,dst}_nents > 1) (SEC2)
> - * @buf: input and output buffeur (if {src,dst}_nents > 1) (SEC1)
> - *
> - * if decrypting (with authcheck), or either one of src_nents or dst_nents
> - * is greater than 1, an integrity check value is concatenated to the end
> - * of link_tbl data
> - */
> -struct talitos_edesc {
> -	int src_nents;
> -	int dst_nents;
> -	bool icv_ool;
> -	dma_addr_t iv_dma;
> -	int dma_len;
> -	dma_addr_t dma_link_tbl;
> -	struct talitos_desc desc;
> -	union {
> -		struct talitos_ptr link_tbl[0];
> -		u8 buf[0];
> -	};
> -};
> -
>   static void talitos_sg_unmap(struct device *dev,
>   			     struct talitos_edesc *edesc,
>   			     struct scatterlist *src,
> diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
> index 32ad4fc679ed..95f78c6d9206 100644
> --- a/drivers/crypto/talitos.h
> +++ b/drivers/crypto/talitos.h
> @@ -42,6 +42,36 @@ struct talitos_desc {
>   
>   #define TALITOS_DESC_SIZE	(sizeof(struct talitos_desc) - sizeof(__be32))
>   
> +/*
> + * talitos_edesc - s/w-extended descriptor
> + * @src_nents: number of segments in input scatterlist
> + * @dst_nents: number of segments in output scatterlist
> + * @icv_ool: whether ICV is out-of-line
> + * @iv_dma: dma address of iv for checking continuity and link table
> + * @dma_len: length of dma mapped link_tbl space
> + * @dma_link_tbl: bus physical address of link_tbl/buf
> + * @desc: h/w descriptor
> + * @link_tbl: input and output h/w link tables (if {src,dst}_nents > 1) (SEC2)
> + * @buf: input and output buffeur (if {src,dst}_nents > 1) (SEC1)
> + *
> + * if decrypting (with authcheck), or either one of src_nents or dst_nents
> + * is greater than 1, an integrity check value is concatenated to the end
> + * of link_tbl data
> + */
> +struct talitos_edesc {
> +	int src_nents;
> +	int dst_nents;
> +	bool icv_ool;
> +	dma_addr_t iv_dma;
> +	int dma_len;
> +	dma_addr_t dma_link_tbl;
> +	struct talitos_desc desc;
> +	union {
> +		struct talitos_ptr link_tbl[0];
> +		u8 buf[0];
> +	};
> +};
> +
>   /**
>    * talitos_request - descriptor submission request
>    * @desc: descriptor pointer (kernel virtual)
> 

^ permalink raw reply

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Christophe Leroy @ 2019-06-13 12:50 UTC (permalink / raw)
  To: Horia Geanta, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB348514C4AA9E41C26FF4430998EF0@VI1PR0402MB3485.eurprd04.prod.outlook.com>



Le 13/06/2019 à 14:39, Horia Geanta a écrit :
> On 6/13/2019 3:32 PM, Christophe Leroy wrote:
>>
>>
>> Le 13/06/2019 à 14:24, Horia Geanta a écrit :
>>> On 6/13/2019 3:16 PM, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 13/06/2019 à 14:13, Horia Geanta a écrit :
>>>>> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>>>>>> Next patch will require struct talitos_edesc to be defined
>>>>>> earlier in talitos.c
>>>>>>
>>>>>> This patch moves it into talitos.h so that it can be used
>>>>>> from any place in talitos.c
>>>>>>
>>>>>> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
>>>>>> Cc: stable@vger.kernel.org
>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>>> Again, this patch does not qualify as a fix.
>>>>>
>>>>
>>>> But as I said, the following one is a fix and require that one, you told
>>>> me to add stable in Cc: to make it explicit it was to go into stable.
>>> Yes, but you should remove the Fixes tag.
>>> And probably replace "Next patch" with the commit headline.
>>>
>>>> If someone tries to merge following one into stable with taking that one
>>>> first, build will fail.
>>> This shouldn't happen, order from main tree should be preserved.
>>>
>>
>> When they pick up fixes, AFAIK they don't take all the preceeding commits.
>>
> This is not about Fixes tag, but Cc tag:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1
> 

Ah, ok. So I need to keep the Cc tag. I misunderstood sorry.

^ permalink raw reply

* [PATCH v3 4/4] crypto: talitos - drop icv_ool
From: Christophe Leroy @ 2019-06-13 12:48 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1560429844.git.christophe.leroy@c-s.fr>

icv_ool is not used anymore, drop it.

Fixes: e345177ded17 ("crypto: talitos - fix AEAD processing.")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 3 ---
 drivers/crypto/talitos.h | 2 --
 2 files changed, 5 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index b2de931de623..03b7a5d28fb0 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1278,9 +1278,6 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
 				 is_ipsec_esp && !encrypt);
 	tbl_off += ret;
 
-	/* ICV data */
-	edesc->icv_ool = !encrypt;
-
 	if (!encrypt && is_ipsec_esp) {
 		struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
 
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 95f78c6d9206..1469b956948a 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -46,7 +46,6 @@ struct talitos_desc {
  * talitos_edesc - s/w-extended descriptor
  * @src_nents: number of segments in input scatterlist
  * @dst_nents: number of segments in output scatterlist
- * @icv_ool: whether ICV is out-of-line
  * @iv_dma: dma address of iv for checking continuity and link table
  * @dma_len: length of dma mapped link_tbl space
  * @dma_link_tbl: bus physical address of link_tbl/buf
@@ -61,7 +60,6 @@ struct talitos_desc {
 struct talitos_edesc {
 	int src_nents;
 	int dst_nents;
-	bool icv_ool;
 	dma_addr_t iv_dma;
 	int dma_len;
 	dma_addr_t dma_link_tbl;
-- 
2.13.3


^ permalink raw reply related

* [PATCH v3 2/4] crypto: talitos - fix hash on SEC1.
From: Christophe Leroy @ 2019-06-13 12:48 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1560429844.git.christophe.leroy@c-s.fr>

On SEC1, hash provides wrong result when performing hashing in several
steps with input data SG list has more than one element. This was
detected with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:

[   44.185947] alg: hash: md5-talitos test failed (wrong result) on test vector 6, cfg="random: may_sleep use_finup src_divs=[<reimport>25.88%@+8063, <flush>24.19%@+9588, 28.63%@+16333, <reimport>4.60%@+6756, 16.70%@+16281] dst_divs=[71.61%@alignmask+16361, 14.36%@+7756, 14.3%@+"
[   44.325122] alg: hash: sha1-talitos test failed (wrong result) on test vector 3, cfg="random: inplace use_final src_divs=[<flush,nosimd>16.56%@+16378, <reimport>52.0%@+16329, 21.42%@alignmask+16380, 10.2%@alignmask+16380] iv_offset=39"
[   44.493500] alg: hash: sha224-talitos test failed (wrong result) on test vector 4, cfg="random: use_final nosimd src_divs=[<reimport>52.27%@+7401, <reimport>17.34%@+16285, <flush>17.71%@+26, 12.68%@+10644] iv_offset=43"
[   44.673262] alg: hash: sha256-talitos test failed (wrong result) on test vector 4, cfg="random: may_sleep use_finup src_divs=[<reimport>60.6%@+12790, 17.86%@+1329, <reimport>12.64%@alignmask+16300, 8.29%@+15, 0.40%@+13506, <reimport>0.51%@+16322, <reimport>0.24%@+16339] dst_divs"

This is due to two issues:
- We have an overlap between the buffer used for copying the input
data (SEC1 doesn't do scatter/gather) and the chained descriptor.
- Data copy is wrong when the previous hash left less than one
blocksize of data to hash, implying a complement of the previous
block with a few bytes from the new request.

Fix it by:
- Moving the second descriptor after the buffer, as moving the buffer
after the descriptor would make it more complex for other cipher
operations (AEAD, ABLKCIPHER)
- Rebuiding a new data SG list without the bytes taken from the new
request to complete the previous one.

Preceding patch ("crypto: talitos - move struct talitos_edesc into
talitos.h") as required for this change to build properly.

Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 63 ++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 5b401aec6c84..4f03baef952b 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -336,15 +336,18 @@ static void flush_channel(struct device *dev, int ch, int error, int reset_ch)
 	tail = priv->chan[ch].tail;
 	while (priv->chan[ch].fifo[tail].desc) {
 		__be32 hdr;
+		struct talitos_edesc *edesc;
 
 		request = &priv->chan[ch].fifo[tail];
+		edesc = container_of(request->desc, struct talitos_edesc, desc);
 
 		/* descriptors with their done bits set don't get the error */
 		rmb();
 		if (!is_sec1)
 			hdr = request->desc->hdr;
 		else if (request->desc->next_desc)
-			hdr = (request->desc + 1)->hdr1;
+			hdr = ((struct talitos_desc *)
+			       (edesc->buf + edesc->dma_len))->hdr1;
 		else
 			hdr = request->desc->hdr1;
 
@@ -476,8 +479,14 @@ static u32 current_desc_hdr(struct device *dev, int ch)
 		}
 	}
 
-	if (priv->chan[ch].fifo[iter].desc->next_desc == cur_desc)
-		return (priv->chan[ch].fifo[iter].desc + 1)->hdr;
+	if (priv->chan[ch].fifo[iter].desc->next_desc == cur_desc) {
+		struct talitos_edesc *edesc;
+
+		edesc = container_of(priv->chan[ch].fifo[iter].desc,
+				     struct talitos_edesc, desc);
+		return ((struct talitos_desc *)
+			(edesc->buf + edesc->dma_len))->hdr;
+	}
 
 	return priv->chan[ch].fifo[iter].desc->hdr;
 }
@@ -1402,15 +1411,11 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
 	edesc->dst_nents = dst_nents;
 	edesc->iv_dma = iv_dma;
 	edesc->dma_len = dma_len;
-	if (dma_len) {
-		void *addr = &edesc->link_tbl[0];
-
-		if (is_sec1 && !dst)
-			addr += sizeof(struct talitos_desc);
-		edesc->dma_link_tbl = dma_map_single(dev, addr,
+	if (dma_len)
+		edesc->dma_link_tbl = dma_map_single(dev, &edesc->link_tbl[0],
 						     edesc->dma_len,
 						     DMA_BIDIRECTIONAL);
-	}
+
 	return edesc;
 }
 
@@ -1722,14 +1727,16 @@ static void common_nonsnoop_hash_unmap(struct device *dev,
 	struct talitos_private *priv = dev_get_drvdata(dev);
 	bool is_sec1 = has_ftr_sec1(priv);
 	struct talitos_desc *desc = &edesc->desc;
-	struct talitos_desc *desc2 = desc + 1;
+	struct talitos_desc *desc2 = (struct talitos_desc *)
+				     (edesc->buf + edesc->dma_len);
 
 	unmap_single_talitos_ptr(dev, &edesc->desc.ptr[5], DMA_FROM_DEVICE);
 	if (desc->next_desc &&
 	    desc->ptr[5].ptr != desc2->ptr[5].ptr)
 		unmap_single_talitos_ptr(dev, &desc2->ptr[5], DMA_FROM_DEVICE);
 
-	talitos_sg_unmap(dev, edesc, req_ctx->psrc, NULL, 0, 0);
+	if (req_ctx->psrc)
+		talitos_sg_unmap(dev, edesc, req_ctx->psrc, NULL, 0, 0);
 
 	/* When using hashctx-in, must unmap it. */
 	if (from_talitos_ptr_len(&edesc->desc.ptr[1], is_sec1))
@@ -1796,7 +1803,6 @@ static void talitos_handle_buggy_hash(struct talitos_ctx *ctx,
 
 static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 				struct ahash_request *areq, unsigned int length,
-				unsigned int offset,
 				void (*callback) (struct device *dev,
 						  struct talitos_desc *desc,
 						  void *context, int error))
@@ -1835,9 +1841,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 
 	sg_count = edesc->src_nents ?: 1;
 	if (is_sec1 && sg_count > 1)
-		sg_pcopy_to_buffer(req_ctx->psrc, sg_count,
-				   edesc->buf + sizeof(struct talitos_desc),
-				   length, req_ctx->nbuf);
+		sg_copy_to_buffer(req_ctx->psrc, sg_count, edesc->buf, length);
 	else if (length)
 		sg_count = dma_map_sg(dev, req_ctx->psrc, sg_count,
 				      DMA_TO_DEVICE);
@@ -1850,7 +1854,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 				       DMA_TO_DEVICE);
 	} else {
 		sg_count = talitos_sg_map(dev, req_ctx->psrc, length, edesc,
-					  &desc->ptr[3], sg_count, offset, 0);
+					  &desc->ptr[3], sg_count, 0, 0);
 		if (sg_count > 1)
 			sync_needed = true;
 	}
@@ -1874,7 +1878,8 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 		talitos_handle_buggy_hash(ctx, edesc, &desc->ptr[3]);
 
 	if (is_sec1 && req_ctx->nbuf && length) {
-		struct talitos_desc *desc2 = desc + 1;
+		struct talitos_desc *desc2 = (struct talitos_desc *)
+					     (edesc->buf + edesc->dma_len);
 		dma_addr_t next_desc;
 
 		memset(desc2, 0, sizeof(*desc2));
@@ -1895,7 +1900,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
 						      DMA_TO_DEVICE);
 		copy_talitos_ptr(&desc2->ptr[2], &desc->ptr[2], is_sec1);
 		sg_count = talitos_sg_map(dev, req_ctx->psrc, length, edesc,
-					  &desc2->ptr[3], sg_count, offset, 0);
+					  &desc2->ptr[3], sg_count, 0, 0);
 		if (sg_count > 1)
 			sync_needed = true;
 		copy_talitos_ptr(&desc2->ptr[5], &desc->ptr[5], is_sec1);
@@ -2006,7 +2011,6 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
 	struct device *dev = ctx->dev;
 	struct talitos_private *priv = dev_get_drvdata(dev);
 	bool is_sec1 = has_ftr_sec1(priv);
-	int offset = 0;
 	u8 *ctx_buf = req_ctx->buf[req_ctx->buf_idx];
 
 	if (!req_ctx->last && (nbytes + req_ctx->nbuf <= blocksize)) {
@@ -2046,6 +2050,9 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
 			sg_chain(req_ctx->bufsl, 2, areq->src);
 		req_ctx->psrc = req_ctx->bufsl;
 	} else if (is_sec1 && req_ctx->nbuf && req_ctx->nbuf < blocksize) {
+		int offset;
+		struct scatterlist *sg;
+
 		if (nbytes_to_hash > blocksize)
 			offset = blocksize - req_ctx->nbuf;
 		else
@@ -2058,7 +2065,18 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
 		sg_copy_to_buffer(areq->src, nents,
 				  ctx_buf + req_ctx->nbuf, offset);
 		req_ctx->nbuf += offset;
-		req_ctx->psrc = areq->src;
+		for (sg = areq->src; sg && offset >= sg->length;
+		     offset -= sg->length, sg = sg_next(sg))
+			;
+		if (offset) {
+			sg_init_table(req_ctx->bufsl, 2);
+			sg_set_buf(req_ctx->bufsl, sg_virt(sg) + offset,
+				   sg->length - offset);
+			sg_chain(req_ctx->bufsl, 2, sg_next(sg));
+			req_ctx->psrc = req_ctx->bufsl;
+		} else {
+			req_ctx->psrc = sg;
+		}
 	} else
 		req_ctx->psrc = areq->src;
 
@@ -2098,8 +2116,7 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
 	if (ctx->keylen && (req_ctx->first || req_ctx->last))
 		edesc->desc.hdr |= DESC_HDR_MODE0_MDEU_HMAC;
 
-	return common_nonsnoop_hash(edesc, areq, nbytes_to_hash, offset,
-				    ahash_done);
+	return common_nonsnoop_hash(edesc, areq, nbytes_to_hash, ahash_done);
 }
 
 static int ahash_update(struct ahash_request *areq)
-- 
2.13.3


^ permalink raw reply related

* [PATCH v3 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Christophe Leroy @ 2019-06-13 12:48 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1560429844.git.christophe.leroy@c-s.fr>

Moves it into talitos.h so that it can be used
from any place in talitos.c

This will be required for next
patch ("crypto: talitos - fix hash on SEC1")

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 drivers/crypto/talitos.c | 30 ------------------------------
 drivers/crypto/talitos.h | 30 ++++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 3b3e99f1cddb..5b401aec6c84 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -951,36 +951,6 @@ static int aead_des3_setkey(struct crypto_aead *authenc,
 	goto out;
 }
 
-/*
- * talitos_edesc - s/w-extended descriptor
- * @src_nents: number of segments in input scatterlist
- * @dst_nents: number of segments in output scatterlist
- * @icv_ool: whether ICV is out-of-line
- * @iv_dma: dma address of iv for checking continuity and link table
- * @dma_len: length of dma mapped link_tbl space
- * @dma_link_tbl: bus physical address of link_tbl/buf
- * @desc: h/w descriptor
- * @link_tbl: input and output h/w link tables (if {src,dst}_nents > 1) (SEC2)
- * @buf: input and output buffeur (if {src,dst}_nents > 1) (SEC1)
- *
- * if decrypting (with authcheck), or either one of src_nents or dst_nents
- * is greater than 1, an integrity check value is concatenated to the end
- * of link_tbl data
- */
-struct talitos_edesc {
-	int src_nents;
-	int dst_nents;
-	bool icv_ool;
-	dma_addr_t iv_dma;
-	int dma_len;
-	dma_addr_t dma_link_tbl;
-	struct talitos_desc desc;
-	union {
-		struct talitos_ptr link_tbl[0];
-		u8 buf[0];
-	};
-};
-
 static void talitos_sg_unmap(struct device *dev,
 			     struct talitos_edesc *edesc,
 			     struct scatterlist *src,
diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
index 32ad4fc679ed..95f78c6d9206 100644
--- a/drivers/crypto/talitos.h
+++ b/drivers/crypto/talitos.h
@@ -42,6 +42,36 @@ struct talitos_desc {
 
 #define TALITOS_DESC_SIZE	(sizeof(struct talitos_desc) - sizeof(__be32))
 
+/*
+ * talitos_edesc - s/w-extended descriptor
+ * @src_nents: number of segments in input scatterlist
+ * @dst_nents: number of segments in output scatterlist
+ * @icv_ool: whether ICV is out-of-line
+ * @iv_dma: dma address of iv for checking continuity and link table
+ * @dma_len: length of dma mapped link_tbl space
+ * @dma_link_tbl: bus physical address of link_tbl/buf
+ * @desc: h/w descriptor
+ * @link_tbl: input and output h/w link tables (if {src,dst}_nents > 1) (SEC2)
+ * @buf: input and output buffeur (if {src,dst}_nents > 1) (SEC1)
+ *
+ * if decrypting (with authcheck), or either one of src_nents or dst_nents
+ * is greater than 1, an integrity check value is concatenated to the end
+ * of link_tbl data
+ */
+struct talitos_edesc {
+	int src_nents;
+	int dst_nents;
+	bool icv_ool;
+	dma_addr_t iv_dma;
+	int dma_len;
+	dma_addr_t dma_link_tbl;
+	struct talitos_desc desc;
+	union {
+		struct talitos_ptr link_tbl[0];
+		u8 buf[0];
+	};
+};
+
 /**
  * talitos_request - descriptor submission request
  * @desc: descriptor pointer (kernel virtual)
-- 
2.13.3


^ permalink raw reply related

* [PATCH v3 0/4] Additional fixes on Talitos driver
From: Christophe Leroy @ 2019-06-13 12:48 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel

This series is the last set of fixes for the Talitos driver.

We now get a fully clean boot on both SEC1 (SEC1.2 on mpc885) and
SEC2 (SEC2.2 on mpc8321E) with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:

[    3.385197] bus: 'platform': really_probe: probing driver talitos with device ff020000.crypto
[    3.450982] random: fast init done
[   12.252548] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[   12.262226] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[   43.310737] Bug in SEC1, padding ourself
[   45.603318] random: crng init done
[   54.612333] talitos ff020000.crypto: fsl,sec1.2 algorithms registered in /proc/crypto
[   54.620232] driver: 'talitos': driver_bound: bound to device 'ff020000.crypto'

[    1.193721] bus: 'platform': really_probe: probing driver talitos with device b0030000.crypto
[    1.229197] random: fast init done
[    2.714920] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos)
[    2.724312] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos-hsna)
[    4.482045] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos)
[    4.490940] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[    4.500280] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos)
[    4.509727] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[    6.631781] random: crng init done
[   11.521795] talitos b0030000.crypto: fsl,sec2.2 algorithms registered in /proc/crypto
[   11.529803] driver: 'talitos': driver_bound: bound to device 'b0030000.crypto'

v2: dropped patch 1 which was irrelevant due to a rebase weirdness. Added Cc to stable on the 2 first patches.

v3:
 - removed stable reference in patch 1
 - reworded patch 1 to include name of patch 2 for the dependency.
 - mentionned this dependency in patch 2 as well.
 - corrected the Fixes: sha1 in patch 4

Christophe Leroy (4):
  crypto: talitos - move struct talitos_edesc into talitos.h
  crypto: talitos - fix hash on SEC1.
  crypto: talitos - eliminate unneeded 'done' functions at build time
  crypto: talitos - drop icv_ool

 drivers/crypto/talitos.c | 98 ++++++++++++++++++++----------------------------
 drivers/crypto/talitos.h | 28 ++++++++++++++
 2 files changed, 69 insertions(+), 57 deletions(-)

-- 
2.13.3


^ permalink raw reply

* [PATCH v3 3/4] crypto: talitos - eliminate unneeded 'done' functions at build time
From: Christophe Leroy @ 2019-06-13 12:48 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, horia.geanta
  Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1560429844.git.christophe.leroy@c-s.fr>

When building for SEC1 only, talitos2_done functions are unneeded
and should go away.

For this, use has_ftr_sec1() which will always return true when only
SEC1 support is being built, allowing GCC to drop TALITOS2 functions.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>
---
 drivers/crypto/talitos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 4f03baef952b..b2de931de623 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -3401,7 +3401,7 @@ static int talitos_probe(struct platform_device *ofdev)
 	if (err)
 		goto err_out;
 
-	if (of_device_is_compatible(np, "fsl,sec1.0")) {
+	if (has_ftr_sec1(priv)) {
 		if (priv->num_channels == 1)
 			tasklet_init(&priv->done_task[0], talitos1_done_ch0,
 				     (unsigned long)dev);
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Horia Geanta @ 2019-06-13 12:39 UTC (permalink / raw)
  To: Christophe Leroy, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <ce6beda2-75c8-f360-9e01-5a883128d153@c-s.fr>

On 6/13/2019 3:32 PM, Christophe Leroy wrote:
> 
> 
> Le 13/06/2019 à 14:24, Horia Geanta a écrit :
>> On 6/13/2019 3:16 PM, Christophe Leroy wrote:
>>>
>>>
>>> Le 13/06/2019 à 14:13, Horia Geanta a écrit :
>>>> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>>>>> Next patch will require struct talitos_edesc to be defined
>>>>> earlier in talitos.c
>>>>>
>>>>> This patch moves it into talitos.h so that it can be used
>>>>> from any place in talitos.c
>>>>>
>>>>> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> Again, this patch does not qualify as a fix.
>>>>
>>>
>>> But as I said, the following one is a fix and require that one, you told
>>> me to add stable in Cc: to make it explicit it was to go into stable.
>> Yes, but you should remove the Fixes tag.
>> And probably replace "Next patch" with the commit headline.
>>
>>> If someone tries to merge following one into stable with taking that one
>>> first, build will fail.
>> This shouldn't happen, order from main tree should be preserved.
>>
> 
> When they pick up fixes, AFAIK they don't take all the preceeding commits.
> 
This is not about Fixes tag, but Cc tag:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-1

Horia

^ permalink raw reply

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Christophe Leroy @ 2019-06-13 12:32 UTC (permalink / raw)
  To: Horia Geanta, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB34858ABA5DE0324FA6E2CFCD98EF0@VI1PR0402MB3485.eurprd04.prod.outlook.com>



Le 13/06/2019 à 14:24, Horia Geanta a écrit :
> On 6/13/2019 3:16 PM, Christophe Leroy wrote:
>>
>>
>> Le 13/06/2019 à 14:13, Horia Geanta a écrit :
>>> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>>>> Next patch will require struct talitos_edesc to be defined
>>>> earlier in talitos.c
>>>>
>>>> This patch moves it into talitos.h so that it can be used
>>>> from any place in talitos.c
>>>>
>>>> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> Again, this patch does not qualify as a fix.
>>>
>>
>> But as I said, the following one is a fix and require that one, you told
>> me to add stable in Cc: to make it explicit it was to go into stable.
> Yes, but you should remove the Fixes tag.
> And probably replace "Next patch" with the commit headline.
> 
>> If someone tries to merge following one into stable with taking that one
>> first, build will fail.
> This shouldn't happen, order from main tree should be preserved.
> 

When they pick up fixes, AFAIK they don't take all the preceeding commits.

But ok, I'll resend.

Christophe

^ permalink raw reply

* Re: [PATCH v2 4/4] crypto: talitos - drop icv_ool
From: Christophe Leroy @ 2019-06-13 12:28 UTC (permalink / raw)
  To: Horia Geanta, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB34852F501B30A09A4E515B4798EF0@VI1PR0402MB3485.eurprd04.prod.outlook.com>



Le 13/06/2019 à 14:21, Horia Geanta a écrit :
> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>> icv_ool is not used anymore, drop it.
>>
>> Fixes: 9cc87bc3613b ("crypto: talitos - fix AEAD processing")
> I can't find this SHA1.
> 
> Are you referring to commit e345177ded17 ("crypto: talitos - fix AEAD processing.")?
> 
> Horia
> 

Oops yes, that's the sha1 it had in my tree before it got merged.

Do I have to resend it or can Herbert just drop the wrong reference and 
take the following one:

Fixes: e345177ded17 ("crypto: talitos - fix AEAD processing.")


Christophe

^ permalink raw reply

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Horia Geanta @ 2019-06-13 12:24 UTC (permalink / raw)
  To: Christophe Leroy, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <7fb54918-4524-1e6b-dd29-46be8843577b@c-s.fr>

On 6/13/2019 3:16 PM, Christophe Leroy wrote:
> 
> 
> Le 13/06/2019 à 14:13, Horia Geanta a écrit :
>> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>>> Next patch will require struct talitos_edesc to be defined
>>> earlier in talitos.c
>>>
>>> This patch moves it into talitos.h so that it can be used
>>> from any place in talitos.c
>>>
>>> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Again, this patch does not qualify as a fix.
>>
> 
> But as I said, the following one is a fix and require that one, you told 
> me to add stable in Cc: to make it explicit it was to go into stable.
Yes, but you should remove the Fixes tag.
And probably replace "Next patch" with the commit headline.

> If someone tries to merge following one into stable with taking that one 
> first, build will fail.
This shouldn't happen, order from main tree should be preserved.

Horia

^ permalink raw reply

* Re: [PATCH v2 4/4] crypto: talitos - drop icv_ool
From: Horia Geanta @ 2019-06-13 12:21 UTC (permalink / raw)
  To: Christophe Leroy, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <39be46fb40ad77e40ae5c1a979ca6a2ccfab244a.1560263641.git.christophe.leroy@c-s.fr>

On 6/11/2019 5:39 PM, Christophe Leroy wrote:
> icv_ool is not used anymore, drop it.
> 
> Fixes: 9cc87bc3613b ("crypto: talitos - fix AEAD processing")
I can't find this SHA1.

Are you referring to commit e345177ded17 ("crypto: talitos - fix AEAD processing.")?

Horia

^ permalink raw reply

* Re: [PATCH v2 3/4] crypto: talitos - eliminate unneeded 'done' functions at build time
From: Horia Geanta @ 2019-06-13 12:16 UTC (permalink / raw)
  To: Christophe Leroy, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <09bbce930ef6bf209c5bb5241fccc6b4dc583ba5.1560263641.git.christophe.leroy@c-s.fr>

On 6/11/2019 5:39 PM, Christophe Leroy wrote:
> When building for SEC1 only, talitos2_done functions are unneeded
> and should go away.
> 
> For this, use has_ftr_sec1() which will always return true when only
> SEC1 support is being built, allowing GCC to drop TALITOS2 functions.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Thanks,
Horia

^ permalink raw reply

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Christophe Leroy @ 2019-06-13 12:16 UTC (permalink / raw)
  To: Horia Geanta, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB3485C0F4CB13F8674B8B5A5598EF0@VI1PR0402MB3485.eurprd04.prod.outlook.com>



Le 13/06/2019 à 14:13, Horia Geanta a écrit :
> On 6/11/2019 5:39 PM, Christophe Leroy wrote:
>> Next patch will require struct talitos_edesc to be defined
>> earlier in talitos.c
>>
>> This patch moves it into talitos.h so that it can be used
>> from any place in talitos.c
>>
>> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Again, this patch does not qualify as a fix.
> 

But as I said, the following one is a fix and require that one, you told 
me to add stable in Cc: to make it explicit it was to go into stable.
If someone tries to merge following one into stable with taking that one 
first, build will fail.

Christophe

^ permalink raw reply

* Re: [PATCH v2 1/4] crypto: talitos - move struct talitos_edesc into talitos.h
From: Horia Geanta @ 2019-06-13 12:13 UTC (permalink / raw)
  To: Christophe Leroy, Herbert Xu, David S. Miller
  Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <d9b5fade242f0806a587392d31c272709949479f.1560263641.git.christophe.leroy@c-s.fr>

On 6/11/2019 5:39 PM, Christophe Leroy wrote:
> Next patch will require struct talitos_edesc to be defined
> earlier in talitos.c
> 
> This patch moves it into talitos.h so that it can be used
> from any place in talitos.c
> 
> Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Again, this patch does not qualify as a fix.

Horia

^ permalink raw reply

* Re: [PATCH] powerpc: Enable kernel XZ compression option on PPC_85xx
From: Michael Ellerman @ 2019-06-13 11:42 UTC (permalink / raw)
  To: Daniel Axtens, Pawel Dembicki
  Cc: Christian Lamparter, linux-kernel, Pawel Dembicki, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <877e9qp3ou.fsf@dja-thinkpad.axtens.net>

Daniel Axtens <dja@axtens.net> writes:
> Pawel Dembicki <paweldembicki@gmail.com> writes:
>
>> Enable kernel XZ compression option on PPC_85xx. Tested with
>> simpleImage on TP-Link TL-WDR4900 (Freescale P1014 processor).
>>
>> Suggested-by: Christian Lamparter <chunkeey@gmail.com>
>> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
>> ---
>>  arch/powerpc/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 8c1c636308c8..daf4cb968922 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -196,7 +196,7 @@ config PPC
>>  	select HAVE_IOREMAP_PROT
>>  	select HAVE_IRQ_EXIT_ON_IRQ_STACK
>>  	select HAVE_KERNEL_GZIP
>> -	select HAVE_KERNEL_XZ			if PPC_BOOK3S || 44x
>> +	select HAVE_KERNEL_XZ			if PPC_BOOK3S || 44x || PPC_85xx
>
> (I'm not super well versed in the compression stuff, so apologies if
> this is a dumb question.) If it's this simple, is there any reason we
> can't turn it on generally, or convert it to a blacklist of platforms
> known not to work?

For some platforms enabling XZ requires that your u-boot has XZ support,
and I'm not very clear on when that support landed in u-boot and what
boards have it. And there are boards out there with old/custom u-boots
that effectively can't be updated.

But as a server guy I don't really know the details of all that very
well. So if someone tells me that we should enable XZ for everything, or
as you say just black list some platforms, then that's fine by me.

cheers

^ permalink raw reply

* [PATCH] mm: Generalize and rename notify_page_fault() as kprobe_page_fault()
From: Anshuman Khandual @ 2019-06-13 10:07 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: Mark Rutland, Michal Hocko, linux-ia64, linux-sh, Peter Zijlstra,
	Catalin Marinas, Dave Hansen, Will Deacon, Paul Mackerras,
	sparclinux, Stephen Rothwell, linux-s390, Yoshinori Sato, x86,
	Russell King, Matthew Wilcox, Ingo Molnar, James Hogan,
	linux-snps-arc, Fenghua Yu, Anshuman Khandual, Andrey Konovalov,
	Andy Lutomirski, Thomas Gleixner, linux-arm-kernel, Tony Luck,
	Heiko Carstens, Vineet Gupta, linux-mips, Ralf Baechle,
	Paul Burton, Martin Schwidefsky, Andrew Morton, linuxppc-dev,
	David S. Miller

Architectures which support kprobes have very similar boilerplate around
calling kprobe_fault_handler(). Use a helper function in kprobes.h to unify
them, based on the x86 code.

This changes the behaviour for other architectures when preemption is
enabled. Previously, they would have disabled preemption while calling the
kprobe handler. However, preemption would be disabled if this fault was
due to a kprobe, so we know the fault was not due to a kprobe handler and
can simply return failure.

This behaviour was introduced in the commit a980c0ef9f6d ("x86/kprobes:
Refactor kprobes_fault() like kprobe_exceptions_notify()")

Cc: linux-snps-arc@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: x86@kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>

Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
Questions:

AFAICT there is no equivalent of erstwhile notify_page_fault() during page
fault handling in arc and mips archs which can call this generic function.
Please let me know if that is not the case.

Testing:

- Build and boot tested on arm64 and x86
- Build tested on some other archs (arm, sparc64, alpha, powerpc etc)

Changes in V1:

- Updated commit message per Matthew
- Changed kprobe_page_fault() to return bool per Stephen/Dave/Christophe/Matthew
- Changed kprobe_page_fault() to follow x86 code flow per Dave/Matthew
- Changed kprobe_fault variable as bool in powerpc __do_page_fault()
- Added a comment to kprobe_page_fault() per Dave

Changes in RFC V3: (https://patchwork.kernel.org/patch/10981353/)

- Updated the commit message with an explanation for new preemption behaviour
- Moved notify_page_fault() to kprobes.h with 'static nokprobe_inline' per Matthew
- Changed notify_page_fault() return type from int to bool per Michael Ellerman
- Renamed notify_page_fault() as kprobe_page_fault() per Peterz

Changes in RFC V2: (https://patchwork.kernel.org/patch/10974221/)

- Changed generic notify_page_fault() per Mathew Wilcox
- Changed x86 to use new generic notify_page_fault()
- s/must not/need not/ in commit message per Matthew Wilcox

Changes in RFC V1: (https://patchwork.kernel.org/patch/10968273/)

 arch/arm/mm/fault.c      | 24 +-----------------------
 arch/arm64/mm/fault.c    | 24 +-----------------------
 arch/ia64/mm/fault.c     | 24 +-----------------------
 arch/powerpc/mm/fault.c  | 23 ++---------------------
 arch/s390/mm/fault.c     | 16 +---------------
 arch/sh/mm/fault.c       | 18 ++----------------
 arch/sparc/mm/fault_64.c | 16 +---------------
 arch/x86/mm/fault.c      | 21 ++-------------------
 include/linux/kprobes.h  | 19 +++++++++++++++++++
 9 files changed, 30 insertions(+), 155 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 58f69fa..94a97a4 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -30,28 +30,6 @@
 
 #ifdef CONFIG_MMU
 
-#ifdef CONFIG_KPROBES
-static inline int notify_page_fault(struct pt_regs *regs, unsigned int fsr)
-{
-	int ret = 0;
-
-	if (!user_mode(regs)) {
-		/* kprobe_running() needs smp_processor_id() */
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, fsr))
-			ret = 1;
-		preempt_enable();
-	}
-
-	return ret;
-}
-#else
-static inline int notify_page_fault(struct pt_regs *regs, unsigned int fsr)
-{
-	return 0;
-}
-#endif
-
 /*
  * This is useful to dump out the page tables associated with
  * 'addr' in mm 'mm'.
@@ -266,7 +244,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	vm_fault_t fault;
 	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
-	if (notify_page_fault(regs, fsr))
+	if (kprobe_page_fault(regs, fsr))
 		return 0;
 
 	tsk = current;
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index a30818e..8fe4bbc 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -70,28 +70,6 @@ static inline const struct fault_info *esr_to_debug_fault_info(unsigned int esr)
 	return debug_fault_info + DBG_ESR_EVT(esr);
 }
 
-#ifdef CONFIG_KPROBES
-static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
-{
-	int ret = 0;
-
-	/* kprobe_running() needs smp_processor_id() */
-	if (!user_mode(regs)) {
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, esr))
-			ret = 1;
-		preempt_enable();
-	}
-
-	return ret;
-}
-#else
-static inline int notify_page_fault(struct pt_regs *regs, unsigned int esr)
-{
-	return 0;
-}
-#endif
-
 static void data_abort_decode(unsigned int esr)
 {
 	pr_alert("Data abort info:\n");
@@ -446,7 +424,7 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	unsigned long vm_flags = VM_READ | VM_WRITE;
 	unsigned int mm_flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
-	if (notify_page_fault(regs, esr))
+	if (kprobe_page_fault(regs, esr))
 		return 0;
 
 	tsk = current;
diff --git a/arch/ia64/mm/fault.c b/arch/ia64/mm/fault.c
index 5baeb02..22582f8 100644
--- a/arch/ia64/mm/fault.c
+++ b/arch/ia64/mm/fault.c
@@ -21,28 +21,6 @@
 
 extern int die(char *, struct pt_regs *, long);
 
-#ifdef CONFIG_KPROBES
-static inline int notify_page_fault(struct pt_regs *regs, int trap)
-{
-	int ret = 0;
-
-	if (!user_mode(regs)) {
-		/* kprobe_running() needs smp_processor_id() */
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, trap))
-			ret = 1;
-		preempt_enable();
-	}
-
-	return ret;
-}
-#else
-static inline int notify_page_fault(struct pt_regs *regs, int trap)
-{
-	return 0;
-}
-#endif
-
 /*
  * Return TRUE if ADDRESS points at a page in the kernel's mapped segment
  * (inside region 5, on ia64) and that page is present.
@@ -116,7 +94,7 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re
 	/*
 	 * This is to handle the kprobes on user space access instructions
 	 */
-	if (notify_page_fault(regs, TRAP_BRKPT))
+	if (kprobe_page_fault(regs, TRAP_BRKPT))
 		return;
 
 	if (user_mode(regs))
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index ec6b7ad..bc4e1af 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -42,26 +42,6 @@
 #include <asm/debug.h>
 #include <asm/kup.h>
 
-static inline bool notify_page_fault(struct pt_regs *regs)
-{
-	bool ret = false;
-
-#ifdef CONFIG_KPROBES
-	/* kprobe_running() needs smp_processor_id() */
-	if (!user_mode(regs)) {
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, 11))
-			ret = true;
-		preempt_enable();
-	}
-#endif /* CONFIG_KPROBES */
-
-	if (unlikely(debugger_fault_handler(regs)))
-		ret = true;
-
-	return ret;
-}
-
 /*
  * Check whether the instruction inst is a store using
  * an update addressing form which will update r1.
@@ -462,8 +442,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 	int is_write = page_fault_is_write(error_code);
 	vm_fault_t fault, major = 0;
 	bool must_retry = false;
+	bool kprobe_fault = kprobe_page_fault(regs, 11);
 
-	if (notify_page_fault(regs))
+	if (unlikely(debugger_fault_handler(regs) || kprobe_fault))
 		return 0;
 
 	if (unlikely(page_fault_is_bad(error_code))) {
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index df75d57..1aaae2c 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -67,20 +67,6 @@ static int __init fault_init(void)
 }
 early_initcall(fault_init);
 
-static inline int notify_page_fault(struct pt_regs *regs)
-{
-	int ret = 0;
-
-	/* kprobe_running() needs smp_processor_id() */
-	if (kprobes_built_in() && !user_mode(regs)) {
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, 14))
-			ret = 1;
-		preempt_enable();
-	}
-	return ret;
-}
-
 /*
  * Find out which address space caused the exception.
  */
@@ -414,7 +400,7 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access)
 	 */
 	clear_pt_regs_flag(regs, PIF_PER_TRAP);
 
-	if (notify_page_fault(regs))
+	if (kprobe_page_fault(regs, 14))
 		return 0;
 
 	mm = tsk->mm;
diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c
index 6defd2c6..74cd4ac 100644
--- a/arch/sh/mm/fault.c
+++ b/arch/sh/mm/fault.c
@@ -24,20 +24,6 @@
 #include <asm/tlbflush.h>
 #include <asm/traps.h>
 
-static inline int notify_page_fault(struct pt_regs *regs, int trap)
-{
-	int ret = 0;
-
-	if (kprobes_built_in() && !user_mode(regs)) {
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, trap))
-			ret = 1;
-		preempt_enable();
-	}
-
-	return ret;
-}
-
 static void
 force_sig_info_fault(int si_signo, int si_code, unsigned long address,
 		     struct task_struct *tsk)
@@ -415,14 +401,14 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
 	if (unlikely(fault_in_kernel_space(address))) {
 		if (vmalloc_fault(address) >= 0)
 			return;
-		if (notify_page_fault(regs, vec))
+		if (kprobe_page_fault(regs, vec))
 			return;
 
 		bad_area_nosemaphore(regs, error_code, address);
 		return;
 	}
 
-	if (unlikely(notify_page_fault(regs, vec)))
+	if (unlikely(kprobe_page_fault(regs, vec)))
 		return;
 
 	/* Only enable interrupts if they were on before the fault */
diff --git a/arch/sparc/mm/fault_64.c b/arch/sparc/mm/fault_64.c
index 8f8a604..6865f9c 100644
--- a/arch/sparc/mm/fault_64.c
+++ b/arch/sparc/mm/fault_64.c
@@ -38,20 +38,6 @@
 
 int show_unhandled_signals = 1;
 
-static inline __kprobes int notify_page_fault(struct pt_regs *regs)
-{
-	int ret = 0;
-
-	/* kprobe_running() needs smp_processor_id() */
-	if (kprobes_built_in() && !user_mode(regs)) {
-		preempt_disable();
-		if (kprobe_running() && kprobe_fault_handler(regs, 0))
-			ret = 1;
-		preempt_enable();
-	}
-	return ret;
-}
-
 static void __kprobes unhandled_fault(unsigned long address,
 				      struct task_struct *tsk,
 				      struct pt_regs *regs)
@@ -285,7 +271,7 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs)
 
 	fault_code = get_thread_fault_code();
 
-	if (notify_page_fault(regs))
+	if (kprobe_page_fault(regs, 0))
 		goto exit_exception;
 
 	si_code = SEGV_MAPERR;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 46df4c6..5400f4e 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -46,23 +46,6 @@ kmmio_fault(struct pt_regs *regs, unsigned long addr)
 	return 0;
 }
 
-static nokprobe_inline int kprobes_fault(struct pt_regs *regs)
-{
-	if (!kprobes_built_in())
-		return 0;
-	if (user_mode(regs))
-		return 0;
-	/*
-	 * To be potentially processing a kprobe fault and to be allowed to call
-	 * kprobe_running(), we have to be non-preemptible.
-	 */
-	if (preemptible())
-		return 0;
-	if (!kprobe_running())
-		return 0;
-	return kprobe_fault_handler(regs, X86_TRAP_PF);
-}
-
 /*
  * Prefetch quirks:
  *
@@ -1280,7 +1263,7 @@ do_kern_addr_fault(struct pt_regs *regs, unsigned long hw_error_code,
 		return;
 
 	/* kprobes don't want to hook the spurious faults: */
-	if (kprobes_fault(regs))
+	if (kprobe_page_fault(regs, X86_TRAP_PF))
 		return;
 
 	/*
@@ -1311,7 +1294,7 @@ void do_user_addr_fault(struct pt_regs *regs,
 	mm = tsk->mm;
 
 	/* kprobes don't want to hook the spurious faults: */
-	if (unlikely(kprobes_fault(regs)))
+	if (unlikely(kprobe_page_fault(regs, X86_TRAP_PF)))
 		return;
 
 	/*
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 443d980..04bdaf0 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -458,4 +458,23 @@ static inline bool is_kprobe_optinsn_slot(unsigned long addr)
 }
 #endif
 
+/* Returns true if kprobes handled the fault */
+static nokprobe_inline bool kprobe_page_fault(struct pt_regs *regs,
+					      unsigned int trap)
+{
+	if (!kprobes_built_in())
+		return false;
+	if (user_mode(regs))
+		return false;
+	/*
+	 * To be potentially processing a kprobe fault and to be allowed
+	 * to call kprobe_running(), we have to be non-preemptible.
+	 */
+	if (preemptible())
+		return false;
+	if (!kprobe_running())
+		return false;
+	return kprobe_fault_handler(regs, trap);
+}
+
 #endif /* _LINUX_KPROBES_H */
-- 
2.7.4


^ permalink raw reply related


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