LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
From: Toshi Kani @ 2013-08-30  0:22 UTC (permalink / raw)
  To: rjw
  Cc: fenghua.yu, bp, Toshi Kani, gregkh, x86, linux-kernel, linux-acpi,
	isimatu.yasuaki, mingo, srivatsa.bhat, nfont, tglx, hpa,
	linuxppc-dev
In-Reply-To: <1377822129-4143-1-git-send-email-toshi.kani@hp.com>

cpu_hotplug_driver_lock() serializes CPU online/offline operations
when ARCH_CPU_PROBE_RELEASE is set.  This lock interface is no longer
necessary with the following reason:

 - lock_device_hotplug() now protects CPU online/offline operations,
   including the probe & release interfaces enabled by
   ARCH_CPU_PROBE_RELEASE.  The use of cpu_hotplug_driver_lock() is
   redundant.
 - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
   is defined, which is misleading and is only enabled on powerpc.

This patch removes the cpu_hotplug_driver_lock() interface.  As
a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
probe & release interface as intended.  There is no functional change
in this patch.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
Performed build test only on powerpc.
---
 arch/powerpc/kernel/smp.c              |   12 ----------
 arch/powerpc/platforms/pseries/dlpar.c |   40 ++++++++++++--------------------
 arch/x86/kernel/topology.c             |    2 --
 drivers/base/cpu.c                     |   10 +-------
 include/linux/cpu.h                    |   13 ----------
 5 files changed, 16 insertions(+), 61 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..1667269 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -763,18 +763,6 @@ void __cpu_die(unsigned int cpu)
 		smp_ops->cpu_die(cpu);
 }
 
-static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock()
-{
-	mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock()
-{
-	mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
-}
-
 void cpu_die(void)
 {
 	if (ppc_md.cpu_die)
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a1a7b9a..e39325d 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -387,18 +387,13 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 	char *cpu_name;
 	int rc;
 
-	cpu_hotplug_driver_lock();
 	rc = strict_strtoul(buf, 0, &drc_index);
-	if (rc) {
-		rc = -EINVAL;
-		goto out;
-	}
+	if (rc)
+		return -EINVAL;
 
 	dn = dlpar_configure_connector(drc_index);
-	if (!dn) {
-		rc = -EINVAL;
-		goto out;
-	}
+	if (!dn)
+		return -EINVAL;
 
 	/* configure-connector reports cpus as living in the base
 	 * directory of the device tree.  CPUs actually live in the
@@ -407,8 +402,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 	cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
 	if (!cpu_name) {
 		dlpar_free_cc_nodes(dn);
-		rc = -ENOMEM;
-		goto out;
+		return -ENOMEM;
 	}
 
 	kfree(dn->full_name);
@@ -417,22 +411,21 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
 	rc = dlpar_acquire_drc(drc_index);
 	if (rc) {
 		dlpar_free_cc_nodes(dn);
-		rc = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	rc = dlpar_attach_node(dn);
 	if (rc) {
 		dlpar_release_drc(drc_index);
 		dlpar_free_cc_nodes(dn);
-		goto out;
+		return rc;
 	}
 
 	rc = dlpar_online_cpu(dn);
-out:
-	cpu_hotplug_driver_unlock();
+	if (rc)
+		return rc;
 
-	return rc ? rc : count;
+	return count;
 }
 
 static int dlpar_offline_cpu(struct device_node *dn)
@@ -505,30 +498,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 		return -EINVAL;
 	}
 
-	cpu_hotplug_driver_lock();
 	rc = dlpar_offline_cpu(dn);
 	if (rc) {
 		of_node_put(dn);
-		rc = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	rc = dlpar_release_drc(*drc_index);
 	if (rc) {
 		of_node_put(dn);
-		goto out;
+		return rc;
 	}
 
 	rc = dlpar_detach_node(dn);
 	if (rc) {
 		dlpar_acquire_drc(*drc_index);
-		goto out;
+		return rc;
 	}
 
 	of_node_put(dn);
-out:
-	cpu_hotplug_driver_unlock();
-	return rc ? rc : count;
+
+	return count;
 }
 
 static int __init pseries_dlpar_init(void)
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index a3f35eb..649b010 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
 		return -EINVAL;
 
 	lock_device_hotplug();
-	cpu_hotplug_driver_lock();
 
 	switch (action) {
 	case 0:
@@ -91,7 +90,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
 		ret = -EINVAL;
 	}
 
-	cpu_hotplug_driver_unlock();
 	unlock_device_hotplug();
 
 	return ret;
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index dc78e45..9745f03 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -46,8 +46,6 @@ static int __ref cpu_subsys_online(struct device *dev)
 	int from_nid, to_nid;
 	int ret;
 
-	cpu_hotplug_driver_lock();
-
 	from_nid = cpu_to_node(cpuid);
 	ret = cpu_up(cpuid);
 	/*
@@ -58,18 +56,12 @@ static int __ref cpu_subsys_online(struct device *dev)
 	if (from_nid != to_nid)
 		change_cpu_under_node(cpu, from_nid, to_nid);
 
-	cpu_hotplug_driver_unlock();
 	return ret;
 }
 
 static int cpu_subsys_offline(struct device *dev)
 {
-	int ret;
-
-	cpu_hotplug_driver_lock();
-	ret = cpu_down(dev->id);
-	cpu_hotplug_driver_unlock();
-	return ret;
+	return cpu_down(dev->id);
 }
 
 void unregister_cpu(struct cpu *cpu)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 801ff9e..3434ef7 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -185,19 +185,6 @@ extern void cpu_hotplug_enable(void);
 void clear_tasks_mm_cpumask(int cpu);
 int cpu_down(unsigned int cpu);
 
-#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
-extern void cpu_hotplug_driver_lock(void);
-extern void cpu_hotplug_driver_unlock(void);
-#else
-static inline void cpu_hotplug_driver_lock(void)
-{
-}
-
-static inline void cpu_hotplug_driver_unlock(void)
-{
-}
-#endif
-
 #else		/* CONFIG_HOTPLUG_CPU */
 
 static inline void cpu_hotplug_begin(void) {}

^ permalink raw reply related

* [PATCH v2 3/4] hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
From: Toshi Kani @ 2013-08-30  0:22 UTC (permalink / raw)
  To: rjw
  Cc: fenghua.yu, bp, Toshi Kani, gregkh, x86, linux-kernel, linux-acpi,
	isimatu.yasuaki, mingo, srivatsa.bhat, nfont, tglx, hpa,
	linuxppc-dev
In-Reply-To: <1377822129-4143-1-git-send-email-toshi.kani@hp.com>

Commit d7c53c9e enabled ARCH_CPU_PROBE_RELEASE on x86 in order to
serialize CPU online/offline operations.  Although it is the config
option to enable CPU hotplug test interfaces, probe & release, it is
also the option to enable cpu_hotplug_driver_lock() as well.  Therefore,
this option had to be enabled on x86 with dummy arch_cpu_probe() and
arch_cpu_release().

Since then, lock_device_hotplug() was introduced to serialize CPU
online/offline & hotplug operations.  Therefore, this config option
is no longer required for the serialization.  This patch disables
this config option on x86 and revert the changes made by commit
d7c53c9e.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/Kconfig          |    4 ----
 arch/x86/kernel/smpboot.c |   21 ---------------------
 2 files changed, 25 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b32ebf9..c87e49a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -255,10 +255,6 @@ config ARCH_HWEIGHT_CFLAGS
 	default "-fcall-saved-ecx -fcall-saved-edx" if X86_32
 	default "-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" if X86_64
 
-config ARCH_CPU_PROBE_RELEASE
-	def_bool y
-	depends on HOTPLUG_CPU
-
 config ARCH_SUPPORTS_UPROBES
 	def_bool y
 
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index aecc98a..5b24a9d 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -82,27 +82,6 @@
 /* State of each CPU */
 DEFINE_PER_CPU(int, cpu_state) = { 0 };
 
-#ifdef CONFIG_HOTPLUG_CPU
-/*
- * We need this for trampoline_base protection from concurrent accesses when
- * off- and onlining cores wildly.
- */
-static DEFINE_MUTEX(x86_cpu_hotplug_driver_mutex);
-
-void cpu_hotplug_driver_lock(void)
-{
-	mutex_lock(&x86_cpu_hotplug_driver_mutex);
-}
-
-void cpu_hotplug_driver_unlock(void)
-{
-	mutex_unlock(&x86_cpu_hotplug_driver_mutex);
-}
-
-ssize_t arch_cpu_probe(const char *buf, size_t count) { return -1; }
-ssize_t arch_cpu_release(const char *buf, size_t count) { return -1; }
-#endif
-
 /* Number of siblings per CPU package */
 int smp_num_siblings = 1;
 EXPORT_SYMBOL(smp_num_siblings);

^ permalink raw reply related

* [PATCH v2 2/4] hotplug, x86: Add hotplug lock to missing places
From: Toshi Kani @ 2013-08-30  0:22 UTC (permalink / raw)
  To: rjw
  Cc: fenghua.yu, bp, Toshi Kani, gregkh, x86, linux-kernel, linux-acpi,
	isimatu.yasuaki, mingo, srivatsa.bhat, nfont, tglx, hpa,
	linuxppc-dev
In-Reply-To: <1377822129-4143-1-git-send-email-toshi.kani@hp.com>

lock_device_hotplug[_sysfs]() serializes CPU & Memory online/offline
and hotplug operations.  However, this lock is not held in the debug
interfaces below that initiate CPU online/offline operations.

 - _debug_hotplug_cpu(), cpu0 hotplug test interface enabled by
   CONFIG_DEBUG_HOTPLUG_CPU0.
 - cpu_probe_store() and cpu_release_store(), cpu hotplug test interface
   enabled by CONFIG_ARCH_CPU_PROBE_RELEASE.

This patch changes the above interfaces to hold lock_device_hotplug().

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/kernel/topology.c |    2 ++
 drivers/base/cpu.c         |   24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 5823bbd..a3f35eb 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -65,6 +65,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
 	if (!cpu_is_hotpluggable(cpu))
 		return -EINVAL;
 
+	lock_device_hotplug();
 	cpu_hotplug_driver_lock();
 
 	switch (action) {
@@ -91,6 +92,7 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
 	}
 
 	cpu_hotplug_driver_unlock();
+	unlock_device_hotplug();
 
 	return ret;
 }
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4cf0717..dc78e45 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -89,7 +89,17 @@ static ssize_t cpu_probe_store(struct device *dev,
 			       const char *buf,
 			       size_t count)
 {
-	return arch_cpu_probe(buf, count);
+	ssize_t cnt;
+	int ret;
+
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		return ret;
+
+	cnt = arch_cpu_probe(buf, count);
+
+	unlock_device_hotplug();
+	return cnt;
 }
 
 static ssize_t cpu_release_store(struct device *dev,
@@ -97,7 +107,17 @@ static ssize_t cpu_release_store(struct device *dev,
 				 const char *buf,
 				 size_t count)
 {
-	return arch_cpu_release(buf, count);
+	ssize_t cnt;
+	int ret;
+
+	ret = lock_device_hotplug_sysfs();
+	if (ret)
+		return ret;
+
+	cnt = arch_cpu_release(buf, count);
+
+	unlock_device_hotplug();
+	return cnt;
 }
 
 static DEVICE_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);

^ permalink raw reply related

* [PATCH v2 1/4] hotplug, x86: Fix online state in cpu0 debug interface
From: Toshi Kani @ 2013-08-30  0:22 UTC (permalink / raw)
  To: rjw
  Cc: fenghua.yu, bp, Toshi Kani, gregkh, x86, linux-kernel, linux-acpi,
	isimatu.yasuaki, mingo, srivatsa.bhat, nfont, tglx, hpa,
	linuxppc-dev
In-Reply-To: <1377822129-4143-1-git-send-email-toshi.kani@hp.com>

_debug_hotplug_cpu() is a debug interface that puts cpu0 offline during
boot-up when CONFIG_DEBUG_HOTPLUG_CPU0 is set.  After cpu0 is put offline
in this interface, however, /sys/devices/system/cpu/cpu0/online still
shows 1 (online).

This patch fixes _debug_hotplug_cpu() to update dev->offline when CPU
online/offline operation succeeded.

Signed-off-by: Toshi Kani <toshi.kani@hp.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/x86/kernel/topology.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 6e60b5f..5823bbd 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -72,16 +72,19 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
 		ret = cpu_down(cpu);
 		if (!ret) {
 			pr_info("CPU %u is now offline\n", cpu);
+			dev->offline = true;
 			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
 		} else
 			pr_debug("Can't offline CPU%d.\n", cpu);
 		break;
 	case 1:
 		ret = cpu_up(cpu);
-		if (!ret)
+		if (!ret) {
+			dev->offline = false;
 			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
-		else
+		} else {
 			pr_debug("Can't online CPU%d.\n", cpu);
+		}
 		break;
 	default:
 		ret = -EINVAL;

^ permalink raw reply related

* Re: [PATCH 0/4] Unify CPU hotplug lock interface
From: Toshi Kani @ 2013-08-30  0:12 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <21553065.6VHCMzStpF@vostro.rjw.lan>

On Fri, 2013-08-30 at 02:06 +0200, Rafael J. Wysocki wrote:
> On Thursday, August 29, 2013 11:15:10 AM Toshi Kani wrote:
> > On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> > > On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > > > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > > > online/offline and hotplug operations, along with sysfs online interface
> > > > restructure (commit 4f3549d7).  With this new locking scheme,
> > > > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > > > 
> > > > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > > > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > > > 
> > > > The patchset is based on Linus's tree, 3.11.0-rc5.
> > > 
> > > Nice series, thanks a lot for taking care of this!
> > 
> > Hi Rafael,
> > 
> > Per the recent your changes in lock_device_hotplug(), do you think it
> > makes sense to integrate this patchset into your tree?  I am also
> > considering to add one more patch to use lock_device_hotplug_sysfs() in
> > cpu_probe_store().  I will rebase to your tree and send them today if it
> > makes sense to you.
> 
> Yes, it does to me.

Great!  I will send them shortly.

Thanks,
-Toshi

^ permalink raw reply

* Re: [PATCH 0/4] Unify CPU hotplug lock interface
From: Rafael J. Wysocki @ 2013-08-30  0:06 UTC (permalink / raw)
  To: Toshi Kani
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1377796510.10300.868.camel@misato.fc.hp.com>

On Thursday, August 29, 2013 11:15:10 AM Toshi Kani wrote:
> On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> > On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > > online/offline and hotplug operations, along with sysfs online interface
> > > restructure (commit 4f3549d7).  With this new locking scheme,
> > > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > > 
> > > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > > 
> > > The patchset is based on Linus's tree, 3.11.0-rc5.
> > 
> > Nice series, thanks a lot for taking care of this!
> 
> Hi Rafael,
> 
> Per the recent your changes in lock_device_hotplug(), do you think it
> makes sense to integrate this patchset into your tree?  I am also
> considering to add one more patch to use lock_device_hotplug_sysfs() in
> cpu_probe_store().  I will rebase to your tree and send them today if it
> makes sense to you.

Yes, it does to me.

Thanks,
Rafael


> > > ---
> > > Toshi Kani (4):
> > >   hotplug, x86: Fix online state in cpu0 debug interface
> > >   hotplug, x86: Add hotplug lock to missing places
> > >   hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
> > >   hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> > > 
> > > ---
> > >  arch/powerpc/kernel/smp.c              | 12 ----------
> > >  arch/powerpc/platforms/pseries/dlpar.c | 40 +++++++++++++---------------------
> > >  arch/x86/Kconfig                       |  4 ----
> > >  arch/x86/kernel/smpboot.c              | 21 ------------------
> > >  arch/x86/kernel/topology.c             | 11 ++++++----
> > >  drivers/base/cpu.c                     | 26 ++++++++++++----------
> > >  include/linux/cpu.h                    | 13 -----------
> > >  7 files changed, 37 insertions(+), 90 deletions(-)
> > > --
> > > 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/
> 
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply

* Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for Motorola/Emerson MVME5100.
From: Stephen N Chivers @ 2013-08-29 22:36 UTC (permalink / raw)
  To: Stephen N Chivers; +Cc: Scott Wood, linuxppc-dev, paulus, Chris Proctor
In-Reply-To: <OFA6564D85.57B12E50-ONCA257BCF.00042385-CA257BCF.00055385@LocalDomain>

Stephen N Chivers/AUS/CSC wrote on 08/22/2013 10:58:10 AM:

> From: Stephen N Chivers/AUS/CSC
> To: Scott Wood <scottwood@freescale.com>
> Cc: benh@kernel.crashing.org, Chris Proctor <cproctor@csc.com.au>, 
> linuxppc-dev@lists.ozlabs.org, paulus@samba.org, Stephen N Chivers 
> <schivers@csc.com.au>
> Date: 08/22/2013 10:58 AM
> Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> Motorola/Emerson MVME5100.
> 
> Scott Wood <scottwood@freescale.com> wrote on 08/21/2013 09:20:03 AM:
> 
> > From: Scott Wood <scottwood@freescale.com>
> > To: Stephen N Chivers <schivers@csc.com.au>
> > Cc: <benh@kernel.crashing.org>, Chris Proctor <cproctor@csc.com.au>,
> > <linuxppc-dev@lists.ozlabs.org>, <paulus@samba.org>
> > Date: 08/21/2013 09:20 AM
> > Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> > Motorola/Emerson MVME5100.
> > 
> > On Tue, 2013-08-20 at 13:28 +1100, Stephen N Chivers wrote:
> > > Scott Wood <scottwood@freescale.com> wrote on 08/09/2013 11:35:20 
AM:
> > > 
> > > > From: Scott Wood <scottwood@freescale.com>
> > > > To: Stephen N Chivers <schivers@csc.com.au>
> > > > Cc: <benh@kernel.crashing.org>, <paulus@samba.org>, Chris Proctor 
> > > > <cproctor@csc.com.au>, <linuxppc-dev@lists.ozlabs.org>
> > > > Date: 08/09/2013 11:36 AM
> > > > Subject: Re: [RFC PATCH 1/1] powerpc/embedded6xx: Add support for 
> > > > Motorola/Emerson MVME5100.
> > > > 
> > > > simple-bus may be applicable here (in addition to a specific
> > > > compatible).
> > > >
> > > The HAWK ASIC is a difficult beast. I still cannot get a positive
> > > identification as to what it is (Motorola/Freescale part number
> > > unknown, not even the part number on the chip on the board 
helps....).
> > > The best I can come up with is that it is a tsi108 without
> > > the ethenets.
> > > So device_type will be tsi-bridge and compatible will be
> > > tsi108-bridge.
> > 
> > Don't use device_type.  compatible should include "hawk" in the name
> > (especially if you're not sure what's really in it), and/or the part
> > number on the chip.  If you're convinced it's fully compatible with
> > tsi108-bridge you can add that as a second compatible value, though
> > given the uncertainty it's probably better to just teach Linux to look
> > for the new compatible.
> > 
> > If devices on the bus can be used without any special bus setup or
> > knowledge, then you can add a compatible of "simple-bus" to the end.
> > 
> > > > Why not just look for a chrp,iic node directly?
> > > >
> > > I was following the model used in other places, like chrp/setup.c.
> > 
> > Not all examples are good examples. :-)
> > 
> > > > > +       if ((np = of_find_compatible_node(NULL, "pci", 
> "mpc10x-pci"))) 
> > > {
> > > > 
> > > > Why insist on the device_type?
> > > >
> > > Following the model in the linkstation (kurobox) platform support. 
> > 
> > Drop the device_type check.
> > 
> > > > > +static void
> > > > > +mvme5100_restart(char *cmd)
> > > > > +{
> > > > > +       volatile ulong          i = 10000000;
> > > > > +
> > > > > +
> > > > > +       local_irq_disable();
> > > > > +       _nmask_and_or_msr(0, MSR_IP);
> > > > 
> > > > Does "mtmsr(mfmsr() | MSR_IP)" not work?
> > > >
> > > Don't know. Is from the original code by Matt Porter.
> > 
> > It actually appears that there are no callers remaining that use the
> > "and" portion of the functionality.  In fact there are no callers that
> > use it for anything other than setting MSR_IP. :-P
> > 
> > > > > +       out_8((u_char *) BOARD_MODRST_REG, 0x01);
> > > > > +
> > > > > +       while (i-- > 0);
> > > > 
> > > > Do not use a loop to implement a delay.
> > > >
> > > Taken from the original code. But at this point the board
> > > is going to reset and reboot via firmware, as /sbin/reboot
> > > or /sbin/halt has been invoked.
> > 
> > Still, it's just a bad idea.  What's wrong with udelay()?
> > 
> > Or just use an infinite loop.  How much value is there really in 
timing
> > out here?
> > 
> > > > > +static void __init
> > > > > +mvme5100_set_bat(void)
> > > > > +{
> > > > > +
> > > > > +
> > > > > +       mb();
> > > > > +       mtspr(SPRN_DBAT1U, 0xf0001ffe);
> > > > > +       mtspr(SPRN_DBAT1L, 0xf000002a);
> > > > > +       mb();
> > > > > +       setbat(1, 0xfe000000, 0xfe000000, 0x02000000, 
> > > PAGE_KERNEL_NCG);
> > > > > +}
> > > > 
> > > > It is no longer allowed to squat on random virtual address space 
like
> > > > this.  If you really need a BAT you'll have to allocate the 
virtual
> > > > address properly.
> > > >
> > > Yes. I found that this was an anathema when researching the port in
> > > 2010 but I couldn't find any practical solution at the time.
> > > The code is called early to ensure that the hawk registers are 
available.
> > > sysdev/cpm_common.c does the same thing.
> > 
> > > What is the correct solution?
> > 
> > ioremap() has special code to function early (using ioremap_bot).
> > 
> > If you still need to use a BAT that early, reserve the space with
> > asm/fixmap.h or by adding a function to the early ioremap code to just
> > reserve the space.  Or better, improve the ioremap code to be capable 
of
> > creating a BAT (or equivalent) when requested.
> >
> It is really interesting. Given that the UART implementation on the
> HAWK is such that legacy_serial will not set up an early console it
> is very likely that the address translation set up by the bat is not
> required.
> I can probably replace the physical addresses used in:
> 
>  setup_indirec_pci(hose, 0, 0xfe000cf8, 0xfe000cfc, 0);
> 
> with remapped equivalents. But, with the setbat eliminated, the
> line:
> 
>  pcibios_alloc_controller(dev);
> 
> silently (remember no early console, due to UART reg-shift) panics.
> It is happening at the point where the newly allocated PHB
> structure is being added to the "hose_list" in pci-common.c.
> 
> So, I think there is some side effect due to the call to the setbat with 
the
> PAGE_KERNEL_NCG parameter that I do not yet understand.
>
The problem has disappeared after rearranging the code in the board 
support
file (moving mvme5100_setup_bridge did the trick) and I now have a booting
MVME5100 without resorting to "setbat" in early initialization. 
> > -Scott
> > 
> > 
> > 

^ permalink raw reply

* AUTO: Michael Barry is out of the office (returning 11/09/2013)
From: Michael Barry @ 2013-08-29 22:03 UTC (permalink / raw)
  To: linuxppc-dev


I am out of the office until 11/09/2013.




Note: This is an automated response to your message  "Linuxppc-dev Digest,
Vol 108, Issue 258" sent on 29/08/2013 21:47:50.

This is the only notification you will receive while this person is away.

^ permalink raw reply

* Re: Feedback wished on possible improvment of CPU15 errata handling on mpc8xx
From: Joakim Tjernlund @ 2013-08-29 21:26 UTC (permalink / raw)
  To: leroy christophe; +Cc: Scott Wood, LinuxPPC-dev
In-Reply-To: <521FB743.4020305@c-s.fr>

leroy christophe <christophe.leroy@c-s.fr> wrote on 2013/08/29 23:04:03:
>=20
> Le 29/08/2013 19:57, Joakim Tjernlund a =E9crit :
> > "Linuxppc-dev"
> > <linuxppc-dev-bounces+joakim.tjernlund=3Dtransmode.se@lists.ozlabs.org>
> > wrote on 2013/08/29 19:11:48:
> >> The mpc8xx powerpc has an errata identified CPU15 which is that=20
whenever
> >> the last instruction of a page is a conditional branch to the last
> >> instruction of the next page, the CPU might do crazy things.
> >>
> >> To work around this errata, one of the workarounds proposed by=20
freescale
> > is:
> >> "In the ITLB miss exception code, when loading the TLB for an MMU=20
page,
> >> also invalidate any TLB referring to the next and previous page using
> >> tlbie. This intentionally forces an ITLB miss exception on every
> >> execution across sequential MMU page boundaries"
> >>
> >> It is that workaround which has been implemented in the kernel. The
> >> drawback of this workaround is that TLB miss is encountered everytime =

we
> >> cross page boundary. On a flat program execution, it means that we=20
get a
> >> TLB miss every 1000 instructions. A TLB miss handling is around 30/40
> >> instructions, which means a degradation of about 4% of the=20
performances.
> >> It can be even worse if the program has a loop astride two pages.
> >>
> >> In the errata document from freescale, there is an example where they
> >> only invalidate the TLB when the page has the actual issue, in=20
extenso
> >> when the page has the offending instruction at offset 0xffc, and they
> >> suggest to use the available PTE bits to tag pages in advance.
> >>
> >> I checked in asm/pte-8xx.h : we still have one SW bit available
> >> (0x0080). So I was thinking about using that bit to mark pages
> >> CPU15=5FSAFE when loading them if they don't have the offending
> > instruction.
> >> Then, in the ITLBmiss handler, instead of always invalidating=20
preceeding
> >> and following pages, we would check SW bit in the PTE and invalidate
> >> following page only if current page is not marked CPU15=5FSAFE, then=20
check
> >> the PTE of preceeding page and invalidate it only if it is not marked
> >> CPU15=5FSAFE
> >>
> >> I believe this would improve the CPU15 errata handling and would=20
reduce
> >> the overhead introduced by the handling of this errata.
> >>
> >> Do you see anything wrong with my proposal ?
> > Just that you are using up the last bit of the pte which will be=20
needed at
> > some point.
> > Have you run into CPU15? We have been using 8xx for more than 10 years =

on
> > kernel 2.4 and I
> > don't think we ever run into this problem.
> Ok, indeed I have activated the CPU15 errata in the kernel because I=20
> know my CPU has the bug.
> Do you think it can be deactivated without much risk though ?

Can't say for you, all I know that our 860 and 862 CPUs seem to work OK.

> > If you go forward with this I suggest you use the WRITETHRU bit=20
instead
> > and make
> > it so the user can choose which to use.
> >
> > If you want to optimize TLB misses you might want to add support for=20
8MB
> > pages, I got
> > the TLB and kernel memory done in my 2.4 kernel. You could start with=20
that
> > and
> > add 8MB user space page.
> In 2.6 Kernel we have CONFIG=5FPIN=5FTLB which pins the first 8Mbytes in =

> ITLB and pins the first 24Mbytes in DTLB as far as I understand. Do we=20
> need more for the kernel ? I so, yes I would be interested in porting=20
> your code to 2.6

Yes, 2.4 has the same. There is a drawback with pinning though, you pin 4=20
ITLBs and 4 DTLBs.
One only needs 1 ITLB for kernel so the other 3 are unused. 24MB DTLs is=20
pretty statik, chances
are that it is either too much or too little.

>=20
> Wouldn't we waste memory by using 8Mbytes pages in user mode ?

Don't know the details of how user space deal with these pages, hopefully
someone else knows better.

> I read somewhere that Transparent Huge Pages have been ported on powerpc =


> in future kernel 3.11. Therefore I was thinking about maybe adding=20
> support for hugepages into 8xx.
> 8xx has 512kbytes hugepages, I was thinking that maybe it would be more=20
> appropriate than 8Mbytes pages.

See previous comment, although 8MB pages is less TLB insn as I recall.

> Do you think it would be feasible and usefull to do this for embeddeds=20
> system having let say 32 to 128Mbytes RAM ?

One could stop for just kernel memory. With 8MB pages there are some=20
additional=20
advantages compared with PINNED TLBs:
- you map all kernel memory
- you can also map other spaces, I got both IMMR/BCR and all my NOR FLASH
  mapped with 8MB pages.

 Jocke

^ permalink raw reply

* [PATCH] hvc_xen: Remove unnecessary __GFP_ZERO from kzalloc
From: Joe Perches @ 2013-08-29 21:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Konrad Rzeszutek Wilk, Jiri Slaby,
	linuxppc-dev, Stefano Stabellini

kzalloc already adds this __GFP_ZERO.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/tty/hvc/hvc_xen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 682210d..e61c36c 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -208,7 +208,7 @@ static int xen_hvm_console_init(void)
 
 	info = vtermno_to_xencons(HVC_COOKIE);
 	if (!info) {
-		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL);
 		if (!info)
 			return -ENOMEM;
 	} else if (info->intf != NULL) {
@@ -257,7 +257,7 @@ static int xen_pv_console_init(void)
 
 	info = vtermno_to_xencons(HVC_COOKIE);
 	if (!info) {
-		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL);
 		if (!info)
 			return -ENOMEM;
 	} else if (info->intf != NULL) {
@@ -284,7 +284,7 @@ static int xen_initial_domain_console_init(void)
 
 	info = vtermno_to_xencons(HVC_COOKIE);
 	if (!info) {
-		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | __GFP_ZERO);
+		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL);
 		if (!info)
 			return -ENOMEM;
 	}
-- 
1.8.1.2.459.gbcd45b4.dirty

^ permalink raw reply related

* Re: Feedback wished on possible improvment of CPU15 errata handling on mpc8xx
From: leroy christophe @ 2013-08-29 21:04 UTC (permalink / raw)
  To: Joakim Tjernlund; +Cc: Scott Wood, LinuxPPC-dev
In-Reply-To: <OF0B494249.C66591E0-ONC1257BD6.0060FA67-C1257BD6.0062A4F8@transmode.se>

Le 29/08/2013 19:57, Joakim Tjernlund a écrit :
> "Linuxppc-dev"
> <linuxppc-dev-bounces+joakim.tjernlund=transmode.se@lists.ozlabs.org>
> wrote on 2013/08/29 19:11:48:
>> The mpc8xx powerpc has an errata identified CPU15 which is that whenever
>> the last instruction of a page is a conditional branch to the last
>> instruction of the next page, the CPU might do crazy things.
>>
>> To work around this errata, one of the workarounds proposed by freescale
> is:
>> "In the ITLB miss exception code, when loading the TLB for an MMU page,
>> also invalidate any TLB referring to the next and previous page using
>> tlbie. This intentionally forces an ITLB miss exception on every
>> execution across sequential MMU page boundaries"
>>
>> It is that workaround which has been implemented in the kernel. The
>> drawback of this workaround is that TLB miss is encountered everytime we
>> cross page boundary. On a flat program execution, it means that we get a
>> TLB miss every 1000 instructions. A TLB miss handling is around 30/40
>> instructions, which means a degradation of about 4% of the performances.
>> It can be even worse if the program has a loop astride two pages.
>>
>> In the errata document from freescale, there is an example where they
>> only invalidate the TLB when the page has the actual issue, in extenso
>> when the page has the offending instruction at offset 0xffc, and they
>> suggest to use the available PTE bits to tag pages in advance.
>>
>> I checked in asm/pte-8xx.h : we still have one SW bit available
>> (0x0080). So I was thinking about using that bit to mark pages
>> CPU15_SAFE when loading them if they don't have the offending
> instruction.
>> Then, in the ITLBmiss handler, instead of always invalidating preceeding
>> and following pages, we would check SW bit in the PTE and invalidate
>> following page only if current page is not marked CPU15_SAFE, then check
>> the PTE of preceeding page and invalidate it only if it is not marked
>> CPU15_SAFE
>>
>> I believe this would improve the CPU15 errata handling and would reduce
>> the overhead introduced by the handling of this errata.
>>
>> Do you see anything wrong with my proposal ?
> Just that you are using up the last bit of the pte which will be needed at
> some point.
> Have you run into CPU15? We have been using 8xx for more than 10 years on
> kernel 2.4 and I
> don't think we ever run into this problem.
Ok, indeed I have activated the CPU15 errata in the kernel because I 
know my CPU has the bug.
Do you think it can be deactivated without much risk though ?
> If you go forward with this I suggest you use the WRITETHRU bit instead
> and make
> it so the user can choose which to use.
>
> If you want to optimize TLB misses you might want to add support for 8MB
> pages, I got
> the TLB and kernel memory done in my 2.4 kernel. You could start with that
> and
> add 8MB user space page.
In 2.6 Kernel we have CONFIG_PIN_TLB which pins the first 8Mbytes in 
ITLB and pins the first 24Mbytes in DTLB as far as I understand. Do we 
need more for the kernel ? I so, yes I would be interested in porting 
your code to 2.6

Wouldn't we waste memory by using 8Mbytes pages in user mode ?
I read somewhere that Transparent Huge Pages have been ported on powerpc 
in future kernel 3.11. Therefore I was thinking about maybe adding 
support for hugepages into 8xx.
8xx has 512kbytes hugepages, I was thinking that maybe it would be more 
appropriate than 8Mbytes pages.
Do you think it would be feasible and usefull to do this for embeddeds 
system having let say 32 to 128Mbytes RAM ?

Christophe

^ permalink raw reply

* Re: [PATCH] of: Feed entire flattened device tree into the random pool
From: Grant Likely @ 2013-08-29 20:47 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: devicetree, Rob Herring, Paul Mackerras, linuxppc-dev
In-Reply-To: <20130729131150.591926c3@kryten>

On Mon, 29 Jul 2013 13:11:50 +1000, Anton Blanchard <anton@samba.org> wrote:
> 
> Hi,
> 
> > be32_to_cpu(initial_boot_params->totalsize);
> 
> Ouch, thanks Grant.
> 
> Anton
> --
> 
> We feed the entire DMI table into the random pool to provide
> better random data during early boot, so do the same with the
> flattened device tree.
> 
> Signed-off-by: Anton Blanchard <anton@samba.org>

Applied, thanks

g.

^ permalink raw reply

* Re: [PATCH v12] ASoC: fsl: Add S/PDIF machine driver
From: Stephen Warren @ 2013-08-29 18:21 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: mark.rutland, devicetree, alsa-devel, lars, s.hauer, tomasz.figa,
	rob.herring, broonie, p.zabel, shawn.guo, linuxppc-dev
In-Reply-To: <1377662686-31696-1-git-send-email-b42378@freescale.com>

On 08/27/2013 10:04 PM, Nicolin Chen wrote:
> This patch implements a device-tree-only machine driver for Freescale
> i.MX series Soc. It works with spdif_transmitter/spdif_receiver and
> fsl_spdif.c drivers.

Sorry for the slow response. For the record, the binding,
Acked-by: Stephen Warren <swarren@nvidia.com>

^ permalink raw reply

* Re: Feedback wished on possible improvment of CPU15 errata handling on mpc8xx
From: Joakim Tjernlund @ 2013-08-29 17:57 UTC (permalink / raw)
  To: leroy christophe; +Cc: Scott Wood, LinuxPPC-dev
In-Reply-To: <521F80D4.2030300@c-s.fr>

"Linuxppc-dev" 
<linuxppc-dev-bounces+joakim.tjernlund=transmode.se@lists.ozlabs.org> 
wrote on 2013/08/29 19:11:48:
> The mpc8xx powerpc has an errata identified CPU15 which is that whenever 

> the last instruction of a page is a conditional branch to the last 
> instruction of the next page, the CPU might do crazy things.
> 
> To work around this errata, one of the workarounds proposed by freescale 
is:
> "In the ITLB miss exception code, when loading the TLB for an MMU page, 
> also invalidate any TLB referring to the next and previous page using 
> tlbie. This intentionally forces an ITLB miss exception on every 
> execution across sequential MMU page boundaries"
> 
> It is that workaround which has been implemented in the kernel. The 
> drawback of this workaround is that TLB miss is encountered everytime we 

> cross page boundary. On a flat program execution, it means that we get a 

> TLB miss every 1000 instructions. A TLB miss handling is around 30/40 
> instructions, which means a degradation of about 4% of the performances.
> It can be even worse if the program has a loop astride two pages.
> 
> In the errata document from freescale, there is an example where they 
> only invalidate the TLB when the page has the actual issue, in extenso 
> when the page has the offending instruction at offset 0xffc, and they 
> suggest to use the available PTE bits to tag pages in advance.
> 
> I checked in asm/pte-8xx.h : we still have one SW bit available 
> (0x0080). So I was thinking about using that bit to mark pages 
> CPU15_SAFE when loading them if they don't have the offending 
instruction.
> 
> Then, in the ITLBmiss handler, instead of always invalidating preceeding 

> and following pages, we would check SW bit in the PTE and invalidate 
> following page only if current page is not marked CPU15_SAFE, then check 

> the PTE of preceeding page and invalidate it only if it is not marked 
> CPU15_SAFE
> 
> I believe this would improve the CPU15 errata handling and would reduce 
> the overhead introduced by the handling of this errata.
> 
> Do you see anything wrong with my proposal ?

Just that you are using up the last bit of the pte which will be needed at 
some point.
Have you run into CPU15? We have been using 8xx for more than 10 years on 
kernel 2.4 and I
don't think we ever run into this problem.
If you go forward with this I suggest you use the WRITETHRU bit instead 
and make
it so the user can choose which to use.

If you want to optimize TLB misses you might want to add support for 8MB 
pages, I got
the TLB and kernel memory done in my 2.4 kernel. You could start with that 
and
add 8MB user space page.

  Jocke

^ permalink raw reply

* Re: [PATCH 0/4] Unify CPU hotplug lock interface
From: Toshi Kani @ 2013-08-29 17:15 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, isimatu.yasuaki, mingo,
	srivatsa.bhat, tglx, hpa, linuxppc-dev
In-Reply-To: <1449558.tneKXFMIWE@vostro.rjw.lan>

On Sun, 2013-08-18 at 03:02 +0200, Rafael J. Wysocki wrote:
> On Saturday, August 17, 2013 01:46:55 PM Toshi Kani wrote:
> > lock_device_hotplug() was recently introduced to serialize CPU & Memory
> > online/offline and hotplug operations, along with sysfs online interface
> > restructure (commit 4f3549d7).  With this new locking scheme,
> > cpu_hotplug_driver_lock() is redundant and is no longer necessary.
> > 
> > This patchset makes sure that lock_device_hotplug() covers all CPU online/
> > offline interfaces, and then removes cpu_hotplug_driver_lock().
> > 
> > The patchset is based on Linus's tree, 3.11.0-rc5.
> 
> Nice series, thanks a lot for taking care of this!

Hi Rafael,

Per the recent your changes in lock_device_hotplug(), do you think it
makes sense to integrate this patchset into your tree?  I am also
considering to add one more patch to use lock_device_hotplug_sysfs() in
cpu_probe_store().  I will rebase to your tree and send them today if it
makes sense to you.

Thanks,
-Toshi


> 
> Rafael
> 
> 
> > ---
> > Toshi Kani (4):
> >   hotplug, x86: Fix online state in cpu0 debug interface
> >   hotplug, x86: Add hotplug lock to missing places
> >   hotplug, x86: Disable ARCH_CPU_PROBE_RELEASE on x86
> >   hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
> > 
> > ---
> >  arch/powerpc/kernel/smp.c              | 12 ----------
> >  arch/powerpc/platforms/pseries/dlpar.c | 40 +++++++++++++---------------------
> >  arch/x86/Kconfig                       |  4 ----
> >  arch/x86/kernel/smpboot.c              | 21 ------------------
> >  arch/x86/kernel/topology.c             | 11 ++++++----
> >  drivers/base/cpu.c                     | 26 ++++++++++++----------
> >  include/linux/cpu.h                    | 13 -----------
> >  7 files changed, 37 insertions(+), 90 deletions(-)
> > --
> > 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

* Feedback wished on possible improvment of CPU15 errata handling on mpc8xx
From: leroy christophe @ 2013-08-29 17:11 UTC (permalink / raw)
  To: LinuxPPC-dev, Scott Wood

The mpc8xx powerpc has an errata identified CPU15 which is that whenever 
the last instruction of a page is a conditional branch to the last 
instruction of the next page, the CPU might do crazy things.

To work around this errata, one of the workarounds proposed by freescale is:
"In the ITLB miss exception code, when loading the TLB for an MMU page, 
also invalidate any TLB referring to the next and previous page using 
tlbie. This intentionally forces an ITLB miss exception on every 
execution across sequential MMU page boundaries"

It is that workaround which has been implemented in the kernel. The 
drawback of this workaround is that TLB miss is encountered everytime we 
cross page boundary. On a flat program execution, it means that we get a 
TLB miss every 1000 instructions. A TLB miss handling is around 30/40 
instructions, which means a degradation of about 4% of the performances.
It can be even worse if the program has a loop astride two pages.

In the errata document from freescale, there is an example where they 
only invalidate the TLB when the page has the actual issue, in extenso 
when the page has the offending instruction at offset 0xffc, and they 
suggest to use the available PTE bits to tag pages in advance.

I checked in asm/pte-8xx.h : we still have one SW bit available 
(0x0080). So I was thinking about using that bit to mark pages 
CPU15_SAFE when loading them if they don't have the offending instruction.

Then, in the ITLBmiss handler, instead of always invalidating preceeding 
and following pages, we would check SW bit in the PTE and invalidate 
following page only if current page is not marked CPU15_SAFE, then check 
the PTE of preceeding page and invalidate it only if it is not marked 
CPU15_SAFE

I believe this would improve the CPU15 errata handling and would reduce 
the overhead introduced by the handling of this errata.

Do you see anything wrong with my proposal ?

Christophe

^ permalink raw reply

* [PATCH v2 09/10] crypto: nx - fix GCM for zero length messages
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

The NX CGM implementation doesn't support zero length messages and the
current implementation has two flaws:

 - When the input data length is zero, it ignores the associated data.
 - Even when both lengths are zero, it uses the Crypto API to encrypt a
   zeroed block using ctr(aes) and because of this it allocates a new
   transformation and sets the key for this new tfm. Both operations are
   intended to be used only in user context, while the cryptographic
   operations can be called in both user and softirq contexts.

This patch replaces the nested Crypto API use and adds two special
cases:

 - When input data and associated data lengths are zero: it uses NX ECB
   mode to emulate the encryption of a zeroed block using ctr(aes).
 - When input data is zero and associated data is available: it uses NX
   GMAC mode to calculate the associated data MAC.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-gcm.c | 132 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 112 insertions(+), 20 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index 9e89bdf..025d9a8 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -187,40 +187,125 @@ static int nx_gca(struct nx_crypto_ctx  *nx_ctx,
 	return rc;
 }
 
+static int gmac(struct aead_request *req, struct blkcipher_desc *desc)
+{
+	int rc;
+	struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req->base.tfm);
+	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
+	struct nx_sg *nx_sg;
+	unsigned int nbytes = req->assoclen;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
+
+	/* Set GMAC mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_GMAC;
+
+	NX_CPB_FDM(csbcpb) &= ~NX_FDM_CONTINUATION;
+
+	/* page_limit: number of sg entries that fit on one page */
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
+
+	/* Copy IV */
+	memcpy(csbcpb->cpb.aes_gcm.iv_or_cnt, desc->info, AES_BLOCK_SIZE);
+
+	do {
+		/*
+		 * to_process: the data chunk to process in this update.
+		 * This value is bound by sg list limits.
+		 */
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+
+		if ((to_process + processed) < nbytes)
+			NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		else
+			NX_CPB_FDM(csbcpb) &= ~NX_FDM_INTERMEDIATE;
+
+		nx_sg = nx_walk_and_build(nx_ctx->in_sg, nx_ctx->ap->sglen,
+					  req->assoc, processed, to_process);
+		nx_ctx->op.inlen = (nx_ctx->in_sg - nx_sg)
+					* sizeof(struct nx_sg);
+
+		csbcpb->cpb.aes_gcm.bit_length_data = 0;
+		csbcpb->cpb.aes_gcm.bit_length_aad = 8 * nbytes;
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		memcpy(csbcpb->cpb.aes_gcm.in_pat_or_aad,
+			csbcpb->cpb.aes_gcm.out_pat_or_mac, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_gcm.in_s0,
+			csbcpb->cpb.aes_gcm.out_s0, AES_BLOCK_SIZE);
+
+		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(req->assoclen, &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
+
+out:
+	/* Restore GCM mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_GCM;
+	return rc;
+}
+
 static int gcm_empty(struct aead_request *req, struct blkcipher_desc *desc,
 		     int enc)
 {
 	int rc;
 	struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req->base.tfm);
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
+	char out[AES_BLOCK_SIZE];
+	struct nx_sg *in_sg, *out_sg;
 
 	/* For scenarios where the input message is zero length, AES CTR mode
 	 * may be used. Set the source data to be a single block (16B) of all
 	 * zeros, and set the input IV value to be the same as the GMAC IV
 	 * value. - nx_wb 4.8.1.3 */
-	char src[AES_BLOCK_SIZE] = {};
-	struct scatterlist sg;
 
-	desc->tfm = crypto_alloc_blkcipher("ctr(aes)", 0, 0);
-	if (IS_ERR(desc->tfm)) {
-		rc = -ENOMEM;
-		goto out;
-	}
-
-	crypto_blkcipher_setkey(desc->tfm, csbcpb->cpb.aes_gcm.key,
-		NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_128 ? 16 :
-		NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_192 ? 24 : 32);
-
-	sg_init_one(&sg, src, AES_BLOCK_SIZE);
+	/* Change to ECB mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_ECB;
+	memcpy(csbcpb->cpb.aes_ecb.key, csbcpb->cpb.aes_gcm.key,
+			sizeof(csbcpb->cpb.aes_ecb.key));
 	if (enc)
-		rc = crypto_blkcipher_encrypt_iv(desc, req->dst, &sg,
-						 AES_BLOCK_SIZE);
+		NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
 	else
-		rc = crypto_blkcipher_decrypt_iv(desc, req->dst, &sg,
-						 AES_BLOCK_SIZE);
-	crypto_free_blkcipher(desc->tfm);
+		NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
 
+	/* Encrypt the counter/IV */
+	in_sg = nx_build_sg_list(nx_ctx->in_sg, (u8 *) desc->info,
+				 AES_BLOCK_SIZE, nx_ctx->ap->sglen);
+	out_sg = nx_build_sg_list(nx_ctx->out_sg, (u8 *) out, sizeof(out),
+				  nx_ctx->ap->sglen);
+	nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg);
+	nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg);
+
+	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+	if (rc)
+		goto out;
+	atomic_inc(&(nx_ctx->stats->aes_ops));
+
+	/* Copy out the auth tag */
+	memcpy(csbcpb->cpb.aes_gcm.out_pat_or_mac, out,
+			crypto_aead_authsize(crypto_aead_reqtfm(req)));
 out:
+	/* Restore XCBC mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_GCM;
+
+	/*
+	 * ECB key uses the same region that GCM AAD and counter, so it's safe
+	 * to just fill it with zeroes.
+	 */
+	memset(csbcpb->cpb.aes_ecb.key, 0, sizeof(csbcpb->cpb.aes_ecb.key));
+
 	return rc;
 }
 
@@ -242,8 +327,14 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int enc)
 	*(u32 *)(desc.info + NX_GCM_CTR_OFFSET) = 1;
 
 	if (nbytes == 0) {
-		rc = gcm_empty(req, &desc, enc);
-		goto out;
+		if (req->assoclen == 0)
+			rc = gcm_empty(req, &desc, enc);
+		else
+			rc = gmac(req, &desc);
+		if (rc)
+			goto out;
+		else
+			goto mac;
 	}
 
 	/* Process associated data */
@@ -310,6 +401,7 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int enc)
 		processed += to_process;
 	} while (processed < nbytes);
 
+mac:
 	if (enc) {
 		/* copy out the auth tag */
 		scatterwalk_map_and_copy(csbcpb->cpb.aes_gcm.out_pat_or_mac,
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 10/10] crypto: nx - fix SHA-2 for chunks bigger than block size
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

Each call to the co-processor, with exception of the last call, needs to
send data that is multiple of block size. As consequence, any remaining
data is kept in the internal NX context.

This patch fixes a bug in the driver that causes it to save incorrect
data into the context when data is bigger than the block size.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-sha256.c | 2 +-
 drivers/crypto/nx/nx-sha512.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/nx/nx-sha256.c b/drivers/crypto/nx/nx-sha256.c
index 6547a71..da0b24a 100644
--- a/drivers/crypto/nx/nx-sha256.c
+++ b/drivers/crypto/nx/nx-sha256.c
@@ -129,7 +129,7 @@ static int nx_sha256_update(struct shash_desc *desc, const u8 *data,
 		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
 		total -= to_process;
-		data += to_process;
+		data += to_process - sctx->count;
 		sctx->count = 0;
 		in_sg = nx_ctx->in_sg;
 	} while (leftover >= SHA256_BLOCK_SIZE);
diff --git a/drivers/crypto/nx/nx-sha512.c b/drivers/crypto/nx/nx-sha512.c
index 236e6af..4ae5b0f 100644
--- a/drivers/crypto/nx/nx-sha512.c
+++ b/drivers/crypto/nx/nx-sha512.c
@@ -131,7 +131,7 @@ static int nx_sha512_update(struct shash_desc *desc, const u8 *data,
 		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 
 		total -= to_process;
-		data += to_process;
+		data += to_process - sctx->count[0];
 		sctx->count[0] = 0;
 		in_sg = nx_ctx->in_sg;
 	} while (leftover >= SHA512_BLOCK_SIZE);
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 08/10] crypto: nx - fix XCBC for zero length messages
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

The NX XCBC implementation doesn't support zero length messages and
because of that NX is currently returning a hard-coded hash for zero
length messages. However this approach is incorrect since the hash value
also depends on which key is used.

This patch removes the hard-coded hash and replace it with an
implementation based on the RFC 3566 using ECB.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-xcbc.c | 84 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 1a5d9e3..03c4bf5 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -56,6 +56,77 @@ static int nx_xcbc_set_key(struct crypto_shash *desc,
 	return 0;
 }
 
+/*
+ * Based on RFC 3566, for a zero-length message:
+ *
+ * n = 1
+ * K1 = E(K, 0x01010101010101010101010101010101)
+ * K3 = E(K, 0x03030303030303030303030303030303)
+ * E[0] = 0x00000000000000000000000000000000
+ * M[1] = 0x80000000000000000000000000000000 (0 length message with padding)
+ * E[1] = (K1, M[1] ^ E[0] ^ K3)
+ * Tag = M[1]
+ */
+static int nx_xcbc_empty(struct shash_desc *desc, u8 *out)
+{
+	struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(&desc->tfm->base);
+	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
+	struct nx_sg *in_sg, *out_sg;
+	u8 keys[2][AES_BLOCK_SIZE];
+	u8 key[32];
+	int rc = 0;
+
+	/* Change to ECB mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_ECB;
+	memcpy(key, csbcpb->cpb.aes_xcbc.key, AES_BLOCK_SIZE);
+	memcpy(csbcpb->cpb.aes_ecb.key, key, AES_BLOCK_SIZE);
+	NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
+
+	/* K1 and K3 base patterns */
+	memset(keys[0], 0x01, sizeof(keys[0]));
+	memset(keys[1], 0x03, sizeof(keys[1]));
+
+	/* Generate K1 and K3 encrypting the patterns */
+	in_sg = nx_build_sg_list(nx_ctx->in_sg, (u8 *) keys, sizeof(keys),
+				 nx_ctx->ap->sglen);
+	out_sg = nx_build_sg_list(nx_ctx->out_sg, (u8 *) keys, sizeof(keys),
+				  nx_ctx->ap->sglen);
+	nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg);
+	nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg);
+
+	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+	if (rc)
+		goto out;
+	atomic_inc(&(nx_ctx->stats->aes_ops));
+
+	/* XOr K3 with the padding for a 0 length message */
+	keys[1][0] ^= 0x80;
+
+	/* Encrypt the final result */
+	memcpy(csbcpb->cpb.aes_ecb.key, keys[0], AES_BLOCK_SIZE);
+	in_sg = nx_build_sg_list(nx_ctx->in_sg, (u8 *) keys[1], sizeof(keys[1]),
+				 nx_ctx->ap->sglen);
+	out_sg = nx_build_sg_list(nx_ctx->out_sg, out, AES_BLOCK_SIZE,
+				  nx_ctx->ap->sglen);
+	nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) * sizeof(struct nx_sg);
+	nx_ctx->op.outlen = (nx_ctx->out_sg - out_sg) * sizeof(struct nx_sg);
+
+	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+	if (rc)
+		goto out;
+	atomic_inc(&(nx_ctx->stats->aes_ops));
+
+out:
+	/* Restore XCBC mode */
+	csbcpb->cpb.hdr.mode = NX_MODE_AES_XCBC_MAC;
+	memcpy(csbcpb->cpb.aes_xcbc.key, key, AES_BLOCK_SIZE);
+	NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
+
+	return rc;
+}
+
 static int nx_xcbc_init(struct shash_desc *desc)
 {
 	struct xcbc_state *sctx = shash_desc_ctx(desc);
@@ -201,13 +272,12 @@ static int nx_xcbc_final(struct shash_desc *desc, u8 *out)
 		memcpy(csbcpb->cpb.aes_xcbc.cv,
 		       csbcpb->cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
 	} else if (sctx->count == 0) {
-		/* we've never seen an update, so this is a 0 byte op. The
-		 * hardware cannot handle a 0 byte op, so just copy out the
-		 * known 0 byte result. This is cheaper than allocating a
-		 * software context to do a 0 byte op */
-		u8 data[] = { 0x75, 0xf0, 0x25, 0x1d, 0x52, 0x8a, 0xc0, 0x1c,
-			      0x45, 0x73, 0xdf, 0xd5, 0x84, 0xd7, 0x9f, 0x29 };
-		memcpy(out, data, sizeof(data));
+		/*
+		 * we've never seen an update, so this is a 0 byte op. The
+		 * hardware cannot handle a 0 byte op, so just ECB to
+		 * generate the hash.
+		 */
+		rc = nx_xcbc_empty(desc, out);
 		goto out;
 	}
 
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 07/10] crypto: nx - fix limits to sg lists for AES-CCM
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert
  Cc: Joy Latten, linux-kernel, linux-crypto, Fionnuala Gunter,
	linuxppc-dev
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

From: Fionnuala Gunter <fin@linux.vnet.ibm.com>

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
Signed-off-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Fionnuala Gunter <fin@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-ccm.c | 297 +++++++++++++++++++++++++++++------------
 1 file changed, 215 insertions(+), 82 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 666a35b..5ecd4c2 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -179,13 +179,26 @@ static int generate_pat(u8                   *iv,
 	struct nx_sg *nx_insg = nx_ctx->in_sg;
 	struct nx_sg *nx_outsg = nx_ctx->out_sg;
 	unsigned int iauth_len = 0;
-	struct vio_pfo_op *op = NULL;
 	u8 tmp[16], *b1 = NULL, *b0 = NULL, *result = NULL;
 	int rc;
 
 	/* zero the ctr value */
 	memset(iv + 15 - iv[0], 0, iv[0] + 1);
 
+	/* page 78 of nx_wb.pdf has,
+	 * Note: RFC3610 allows the AAD data to be up to 2^64 -1 bytes
+	 * in length. If a full message is used, the AES CCA implementation
+	 * restricts the maximum AAD length to 2^32 -1 bytes.
+	 * If partial messages are used, the implementation supports
+	 * 2^64 -1 bytes maximum AAD length.
+	 *
+	 * However, in the cryptoapi's aead_request structure,
+	 * assoclen is an unsigned int, thus it cannot hold a length
+	 * value greater than 2^32 - 1.
+	 * Thus the AAD is further constrained by this and is never
+	 * greater than 2^32.
+	 */
+
 	if (!req->assoclen) {
 		b0 = nx_ctx->csbcpb->cpb.aes_ccm.in_pat_or_b0;
 	} else if (req->assoclen <= 14) {
@@ -195,7 +208,46 @@ static int generate_pat(u8                   *iv,
 		b0 = nx_ctx->csbcpb->cpb.aes_ccm.in_pat_or_b0;
 		b1 = nx_ctx->priv.ccm.iauth_tag;
 		iauth_len = req->assoclen;
+	} else if (req->assoclen <= 65280) {
+		/* if associated data is less than (2^16 - 2^8), we construct
+		 * B1 differently and feed in the associated data to a CCA
+		 * operation */
+		b0 = nx_ctx->csbcpb_aead->cpb.aes_cca.b0;
+		b1 = nx_ctx->csbcpb_aead->cpb.aes_cca.b1;
+		iauth_len = 14;
+	} else {
+		b0 = nx_ctx->csbcpb_aead->cpb.aes_cca.b0;
+		b1 = nx_ctx->csbcpb_aead->cpb.aes_cca.b1;
+		iauth_len = 10;
+	}
 
+	/* generate B0 */
+	rc = generate_b0(iv, req->assoclen, authsize, nbytes, b0);
+	if (rc)
+		return rc;
+
+	/* generate B1:
+	 * add control info for associated data
+	 * RFC 3610 and NIST Special Publication 800-38C
+	 */
+	if (b1) {
+		memset(b1, 0, 16);
+		if (req->assoclen <= 65280) {
+			*(u16 *)b1 = (u16)req->assoclen;
+			scatterwalk_map_and_copy(b1 + 2, req->assoc, 0,
+					 iauth_len, SCATTERWALK_FROM_SG);
+		} else {
+			*(u16 *)b1 = (u16)(0xfffe);
+			*(u32 *)&b1[2] = (u32)req->assoclen;
+			scatterwalk_map_and_copy(b1 + 6, req->assoc, 0,
+					 iauth_len, SCATTERWALK_FROM_SG);
+		}
+	}
+
+	/* now copy any remaining AAD to scatterlist and call nx... */
+	if (!req->assoclen) {
+		return rc;
+	} else if (req->assoclen <= 14) {
 		nx_insg = nx_build_sg_list(nx_insg, b1, 16, nx_ctx->ap->sglen);
 		nx_outsg = nx_build_sg_list(nx_outsg, tmp, 16,
 					    nx_ctx->ap->sglen);
@@ -210,56 +262,74 @@ static int generate_pat(u8                   *iv,
 		NX_CPB_FDM(nx_ctx->csbcpb) |= NX_FDM_ENDE_ENCRYPT;
 		NX_CPB_FDM(nx_ctx->csbcpb) |= NX_FDM_INTERMEDIATE;
 
-		op = &nx_ctx->op;
 		result = nx_ctx->csbcpb->cpb.aes_ccm.out_pat_or_mac;
-	} else if (req->assoclen <= 65280) {
-		/* if associated data is less than (2^16 - 2^8), we construct
-		 * B1 differently and feed in the associated data to a CCA
-		 * operation */
-		b0 = nx_ctx->csbcpb_aead->cpb.aes_cca.b0;
-		b1 = nx_ctx->csbcpb_aead->cpb.aes_cca.b1;
-		iauth_len = 14;
-
-		/* remaining assoc data must have scatterlist built for it */
-		nx_insg = nx_walk_and_build(nx_insg, nx_ctx->ap->sglen,
-					    req->assoc, iauth_len,
-					    req->assoclen - iauth_len);
-		nx_ctx->op_aead.inlen = (nx_ctx->in_sg - nx_insg) *
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			return rc;
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(req->assoclen, &(nx_ctx->stats->aes_bytes));
+
+	} else {
+		u32 max_sg_len;
+		unsigned int processed = 0, to_process;
+
+		/* page_limit: number of sg entries that fit on one page */
+		max_sg_len = min_t(u32,
+				   nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+				   nx_ctx->ap->sglen);
+
+		processed += iauth_len;
+
+		do {
+			to_process = min_t(u32, req->assoclen - processed,
+					   nx_ctx->ap->databytelen);
+			to_process = min_t(u64, to_process,
+					   NX_PAGE_SIZE * (max_sg_len - 1));
+
+			if ((to_process + processed) < req->assoclen) {
+				NX_CPB_FDM(nx_ctx->csbcpb_aead) |=
+					NX_FDM_INTERMEDIATE;
+			} else {
+				NX_CPB_FDM(nx_ctx->csbcpb_aead) &=
+					~NX_FDM_INTERMEDIATE;
+			}
+
+			nx_insg = nx_walk_and_build(nx_ctx->in_sg,
+						    nx_ctx->ap->sglen,
+						    req->assoc, processed,
+						    to_process);
+
+			nx_ctx->op_aead.inlen = (nx_ctx->in_sg - nx_insg) *
 						sizeof(struct nx_sg);
 
-		op = &nx_ctx->op_aead;
+			result = nx_ctx->csbcpb_aead->cpb.aes_cca.out_pat_or_b0;
+
+			rc = nx_hcall_sync(nx_ctx, &nx_ctx->op_aead,
+				   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+			if (rc)
+				return rc;
+
+			memcpy(nx_ctx->csbcpb_aead->cpb.aes_cca.b0,
+				nx_ctx->csbcpb_aead->cpb.aes_cca.out_pat_or_b0,
+				AES_BLOCK_SIZE);
+
+			NX_CPB_FDM(nx_ctx->csbcpb_aead) |= NX_FDM_CONTINUATION;
+
+			atomic_inc(&(nx_ctx->stats->aes_ops));
+			atomic64_add(req->assoclen,
+					&(nx_ctx->stats->aes_bytes));
+
+			processed += to_process;
+		} while (processed < req->assoclen);
+
 		result = nx_ctx->csbcpb_aead->cpb.aes_cca.out_pat_or_b0;
-	} else {
-		/* if associated data is less than (2^32), we construct B1
-		 * differently yet again and feed in the associated data to a
-		 * CCA operation */
-		pr_err("associated data len is %u bytes (returning -EINVAL)\n",
-		       req->assoclen);
-		rc = -EINVAL;
 	}
 
-	rc = generate_b0(iv, req->assoclen, authsize, nbytes, b0);
-	if (rc)
-		goto done;
+	memcpy(out, result, AES_BLOCK_SIZE);
 
-	if (b1) {
-		memset(b1, 0, 16);
-		*(u16 *)b1 = (u16)req->assoclen;
-
-		scatterwalk_map_and_copy(b1 + 2, req->assoc, 0,
-					 iauth_len, SCATTERWALK_FROM_SG);
-
-		rc = nx_hcall_sync(nx_ctx, op,
-				   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-		if (rc)
-			goto done;
-
-		atomic_inc(&(nx_ctx->stats->aes_ops));
-		atomic64_add(req->assoclen, &(nx_ctx->stats->aes_bytes));
-
-		memcpy(out, result, AES_BLOCK_SIZE);
-	}
-done:
 	return rc;
 }
 
@@ -272,15 +342,12 @@ static int ccm_nx_decrypt(struct aead_request   *req,
 	unsigned int authsize = crypto_aead_authsize(crypto_aead_reqtfm(req));
 	struct nx_ccm_priv *priv = &nx_ctx->priv.ccm;
 	unsigned long irq_flags;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 	int rc = -1;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen) {
-		rc = -EINVAL;
-		goto out;
-	}
-
 	nbytes -= authsize;
 
 	/* copy out the auth tag to compare with later */
@@ -293,22 +360,56 @@ static int ccm_nx_decrypt(struct aead_request   *req,
 	if (rc)
 		goto out;
 
-	rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src, nbytes, 0,
-			       csbcpb->cpb.aes_ccm.iv_or_ctr);
-	if (rc)
-		goto out;
+	/* page_limit: number of sg entries that fit on one page */
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
-	NX_CPB_FDM(nx_ctx->csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
-	NX_CPB_FDM(nx_ctx->csbcpb) &= ~NX_FDM_INTERMEDIATE;
+	do {
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+		/* to_process: the AES_BLOCK_SIZE data chunk to process in this
+		 * update. This value is bound by sg list limits.
+		 */
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+
+		if ((to_process + processed) < nbytes)
+			NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		else
+			NX_CPB_FDM(csbcpb) &= ~NX_FDM_INTERMEDIATE;
+
+		NX_CPB_FDM(nx_ctx->csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
+
+		rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src,
+					to_process, processed,
+					csbcpb->cpb.aes_ccm.iv_or_ctr);
+		if (rc)
+			goto out;
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
 			   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if (rc)
+			goto out;
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
+		/* for partial completion, copy following for next
+		 * entry into loop...
+		 */
+		memcpy(desc->info, csbcpb->cpb.aes_ccm.out_ctr, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_ccm.in_pat_or_b0,
+			csbcpb->cpb.aes_ccm.out_pat_or_mac, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_ccm.in_s0,
+			csbcpb->cpb.aes_ccm.out_s0, AES_BLOCK_SIZE);
+
+		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+		/* update stats */
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 
 	rc = memcmp(csbcpb->cpb.aes_ccm.out_pat_or_mac, priv->oauth_tag,
 		    authsize) ? -EBADMSG : 0;
@@ -325,41 +426,73 @@ static int ccm_nx_encrypt(struct aead_request   *req,
 	unsigned int nbytes = req->cryptlen;
 	unsigned int authsize = crypto_aead_authsize(crypto_aead_reqtfm(req));
 	unsigned long irq_flags;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 	int rc = -1;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen) {
-		rc = -EINVAL;
-		goto out;
-	}
-
 	rc = generate_pat(desc->info, req, nx_ctx, authsize, nbytes,
 			  csbcpb->cpb.aes_ccm.in_pat_or_b0);
 	if (rc)
 		goto out;
 
-	rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src, nbytes, 0,
-			       csbcpb->cpb.aes_ccm.iv_or_ctr);
-	if (rc)
-		goto out;
-
-	NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
-	NX_CPB_FDM(csbcpb) &= ~NX_FDM_INTERMEDIATE;
-
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-			   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
-
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
+	/* page_limit: number of sg entries that fit on one page */
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
+
+	do {
+		/* to process: the AES_BLOCK_SIZE data chunk to process in this
+		 * update. This value is bound by sg list limits.
+		 */
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+
+		if ((to_process + processed) < nbytes)
+			NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		else
+			NX_CPB_FDM(csbcpb) &= ~NX_FDM_INTERMEDIATE;
+
+		NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
+
+		rc = nx_build_sg_lists(nx_ctx, desc, req->dst, req->src,
+					to_process, processed,
+				       csbcpb->cpb.aes_ccm.iv_or_ctr);
+		if (rc)
+			goto out;
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		/* for partial completion, copy following for next
+		 * entry into loop...
+		 */
+		memcpy(desc->info, csbcpb->cpb.aes_ccm.out_ctr, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_ccm.in_pat_or_b0,
+			csbcpb->cpb.aes_ccm.out_pat_or_mac, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_ccm.in_s0,
+			csbcpb->cpb.aes_ccm.out_s0, AES_BLOCK_SIZE);
+
+		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+		/* update stats */
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+
+	} while (processed < nbytes);
 
 	/* copy out the auth tag */
 	scatterwalk_map_and_copy(csbcpb->cpb.aes_ccm.out_pat_or_mac,
 				 req->dst, nbytes, authsize,
 				 SCATTERWALK_TO_SG);
+
 out:
 	spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
 	return rc;
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 06/10] crypto: nx - fix limits to sg lists for AES-XCBC
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Fionnuala Gunter, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

From: Fionnuala Gunter <fin@linux.vnet.ibm.com>

This patch updates the NX driver to perform several hyper calls when necessary
so that the length limits of scatter/gather lists are respected.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Reviewed-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
Signed-off-by: Fionnuala Gunter <fin@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-xcbc.c | 107 +++++++++++++++++++++++-----------------
 1 file changed, 63 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-xcbc.c b/drivers/crypto/nx/nx-aes-xcbc.c
index 658da0f..1a5d9e3 100644
--- a/drivers/crypto/nx/nx-aes-xcbc.c
+++ b/drivers/crypto/nx/nx-aes-xcbc.c
@@ -88,78 +88,97 @@ static int nx_xcbc_update(struct shash_desc *desc,
 	struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(&desc->tfm->base);
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
 	struct nx_sg *in_sg;
-	u32 to_process, leftover;
+	u32 to_process, leftover, total;
+	u32 max_sg_len;
 	unsigned long irq_flags;
 	int rc = 0;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (NX_CPB_FDM(csbcpb) & NX_FDM_CONTINUATION) {
-		/* we've hit the nx chip previously and we're updating again,
-		 * so copy over the partial digest */
-		memcpy(csbcpb->cpb.aes_xcbc.cv,
-		       csbcpb->cpb.aes_xcbc.out_cv_mac, AES_BLOCK_SIZE);
-	}
+
+	total = sctx->count + len;
 
 	/* 2 cases for total data len:
 	 *  1: <= AES_BLOCK_SIZE: copy into state, return 0
 	 *  2: > AES_BLOCK_SIZE: process X blocks, copy in leftover
 	 */
-	if (len + sctx->count <= AES_BLOCK_SIZE) {
+	if (total <= AES_BLOCK_SIZE) {
 		memcpy(sctx->buffer + sctx->count, data, len);
 		sctx->count += len;
 		goto out;
 	}
 
-	/* to_process: the AES_BLOCK_SIZE data chunk to process in this
-	 * update */
-	to_process = (sctx->count + len) & ~(AES_BLOCK_SIZE - 1);
-	leftover = (sctx->count + len) & (AES_BLOCK_SIZE - 1);
+	in_sg = nx_ctx->in_sg;
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+				nx_ctx->ap->sglen);
 
-	/* the hardware will not accept a 0 byte operation for this algorithm
-	 * and the operation MUST be finalized to be correct. So if we happen
-	 * to get an update that falls on a block sized boundary, we must
-	 * save off the last block to finalize with later. */
-	if (!leftover) {
-		to_process -= AES_BLOCK_SIZE;
-		leftover = AES_BLOCK_SIZE;
-	}
+	do {
 
-	if (sctx->count) {
-		in_sg = nx_build_sg_list(nx_ctx->in_sg, sctx->buffer,
-					 sctx->count, nx_ctx->ap->sglen);
-		in_sg = nx_build_sg_list(in_sg, (u8 *)data,
-					 to_process - sctx->count,
-					 nx_ctx->ap->sglen);
-		nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) *
-					sizeof(struct nx_sg);
-	} else {
-		in_sg = nx_build_sg_list(nx_ctx->in_sg, (u8 *)data, to_process,
-					 nx_ctx->ap->sglen);
+		/* to_process: the AES_BLOCK_SIZE data chunk to process in this
+		 * update */
+		to_process = min_t(u64, total, nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+					NX_PAGE_SIZE * (max_sg_len - 1));
+		to_process = to_process & ~(AES_BLOCK_SIZE - 1);
+		leftover = total - to_process;
+
+		/* the hardware will not accept a 0 byte operation for this
+		 * algorithm and the operation MUST be finalized to be correct.
+		 * So if we happen to get an update that falls on a block sized
+		 * boundary, we must save off the last block to finalize with
+		 * later. */
+		if (!leftover) {
+			to_process -= AES_BLOCK_SIZE;
+			leftover = AES_BLOCK_SIZE;
+		}
+
+		if (sctx->count) {
+			in_sg = nx_build_sg_list(nx_ctx->in_sg,
+						(u8 *) sctx->buffer,
+						sctx->count,
+						max_sg_len);
+		}
+		in_sg = nx_build_sg_list(in_sg,
+					(u8 *) data,
+					to_process - sctx->count,
+					max_sg_len);
 		nx_ctx->op.inlen = (nx_ctx->in_sg - in_sg) *
 					sizeof(struct nx_sg);
-	}
 
-	NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		/* we've hit the nx chip previously and we're updating again,
+		 * so copy over the partial digest */
+		if (NX_CPB_FDM(csbcpb) & NX_FDM_CONTINUATION) {
+			memcpy(csbcpb->cpb.aes_xcbc.cv,
+				csbcpb->cpb.aes_xcbc.out_cv_mac,
+				AES_BLOCK_SIZE);
+		}
 
-	if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
-		rc = -EINVAL;
-		goto out;
-	}
+		NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
+			rc = -EINVAL;
+			goto out;
+		}
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
 			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if (rc)
+			goto out;
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+
+		/* everything after the first update is continuation */
+		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+		total -= to_process;
+		data += to_process - sctx->count;
+		sctx->count = 0;
+		in_sg = nx_ctx->in_sg;
+	} while (leftover > AES_BLOCK_SIZE);
 
 	/* copy the leftover back into the state struct */
-	memcpy(sctx->buffer, data + len - leftover, leftover);
+	memcpy(sctx->buffer, data, leftover);
 	sctx->count = leftover;
 
-	/* everything after the first update is continuation */
-	NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
 out:
 	spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
 	return rc;
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 03/10] crypto: nx - fix limits to sg lists for AES-CBC
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

This patch updates the nx-aes-cbc implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - "ibm,max-sg-len": maximum number of bytes of each scatter/gather
   list.

 - "ibm,max-sync-cop":
    - The total number of bytes that a scatter/gather list can hold.
    - The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-cbc.c | 50 +++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c
index a9e76c6..cc00b52 100644
--- a/drivers/crypto/nx/nx-aes-cbc.c
+++ b/drivers/crypto/nx/nx-aes-cbc.c
@@ -71,39 +71,49 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc,
 	struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
 	unsigned long irq_flags;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 	int rc;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen) {
-		rc = -EINVAL;
-		goto out;
-	}
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
 	if (enc)
 		NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
 	else
 		NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
 
-	rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
-			       csbcpb->cpb.aes_cbc.iv);
-	if (rc)
-		goto out;
+	do {
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+		to_process = to_process & ~(AES_BLOCK_SIZE - 1);
 
-	if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
-		rc = -EINVAL;
-		goto out;
-	}
+		rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+				       processed, csbcpb->cpb.aes_cbc.iv);
+		if (rc)
+			goto out;
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
+			rc = -EINVAL;
+			goto out;
+		}
 
-	memcpy(desc->info, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		memcpy(desc->info, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 out:
 	spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
 	return rc;
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 05/10] crypto: nx - fix limits to sg lists for AES-GCM
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

This patch updates the nx-aes-gcm implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - "ibm,max-sg-len": maximum number of bytes of each scatter/gather
   list.

 - "ibm,max-sync-cop":
    - The total number of bytes that a scatter/gather list can hold.
    - The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-gcm.c | 202 +++++++++++++++++++++++++++--------------
 1 file changed, 136 insertions(+), 66 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index c2d6f76..9e89bdf 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -125,37 +125,101 @@ static int nx_gca(struct nx_crypto_ctx  *nx_ctx,
 		  struct aead_request   *req,
 		  u8                    *out)
 {
+	int rc;
 	struct nx_csbcpb *csbcpb_aead = nx_ctx->csbcpb_aead;
-	int rc = -EINVAL;
 	struct scatter_walk walk;
 	struct nx_sg *nx_sg = nx_ctx->in_sg;
+	unsigned int nbytes = req->assoclen;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 
-	if (req->assoclen > nx_ctx->ap->databytelen)
-		goto out;
-
-	if (req->assoclen <= AES_BLOCK_SIZE) {
+	if (nbytes <= AES_BLOCK_SIZE) {
 		scatterwalk_start(&walk, req->assoc);
-		scatterwalk_copychunks(out, &walk, req->assoclen,
-				       SCATTERWALK_FROM_SG);
+		scatterwalk_copychunks(out, &walk, nbytes, SCATTERWALK_FROM_SG);
 		scatterwalk_done(&walk, SCATTERWALK_FROM_SG, 0);
-
-		rc = 0;
-		goto out;
+		return 0;
 	}
 
-	nx_sg = nx_walk_and_build(nx_sg, nx_ctx->ap->sglen, req->assoc, 0,
-				  req->assoclen);
-	nx_ctx->op_aead.inlen = (nx_ctx->in_sg - nx_sg) * sizeof(struct nx_sg);
+	NX_CPB_FDM(csbcpb_aead) &= ~NX_FDM_CONTINUATION;
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op_aead,
-			   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+	/* page_limit: number of sg entries that fit on one page */
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(req->assoclen, &(nx_ctx->stats->aes_bytes));
+	do {
+		/*
+		 * to_process: the data chunk to process in this update.
+		 * This value is bound by sg list limits.
+		 */
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+
+		if ((to_process + processed) < nbytes)
+			NX_CPB_FDM(csbcpb_aead) |= NX_FDM_INTERMEDIATE;
+		else
+			NX_CPB_FDM(csbcpb_aead) &= ~NX_FDM_INTERMEDIATE;
+
+		nx_sg = nx_walk_and_build(nx_ctx->in_sg, nx_ctx->ap->sglen,
+					  req->assoc, processed, to_process);
+		nx_ctx->op_aead.inlen = (nx_ctx->in_sg - nx_sg)
+					* sizeof(struct nx_sg);
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op_aead,
+				req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			return rc;
+
+		memcpy(csbcpb_aead->cpb.aes_gca.in_pat,
+				csbcpb_aead->cpb.aes_gca.out_pat,
+				AES_BLOCK_SIZE);
+		NX_CPB_FDM(csbcpb_aead) |= NX_FDM_CONTINUATION;
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(req->assoclen, &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 
 	memcpy(out, csbcpb_aead->cpb.aes_gca.out_pat, AES_BLOCK_SIZE);
+
+	return rc;
+}
+
+static int gcm_empty(struct aead_request *req, struct blkcipher_desc *desc,
+		     int enc)
+{
+	int rc;
+	struct nx_crypto_ctx *nx_ctx = crypto_tfm_ctx(req->base.tfm);
+	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
+
+	/* For scenarios where the input message is zero length, AES CTR mode
+	 * may be used. Set the source data to be a single block (16B) of all
+	 * zeros, and set the input IV value to be the same as the GMAC IV
+	 * value. - nx_wb 4.8.1.3 */
+	char src[AES_BLOCK_SIZE] = {};
+	struct scatterlist sg;
+
+	desc->tfm = crypto_alloc_blkcipher("ctr(aes)", 0, 0);
+	if (IS_ERR(desc->tfm)) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	crypto_blkcipher_setkey(desc->tfm, csbcpb->cpb.aes_gcm.key,
+		NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_128 ? 16 :
+		NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_192 ? 24 : 32);
+
+	sg_init_one(&sg, src, AES_BLOCK_SIZE);
+	if (enc)
+		rc = crypto_blkcipher_encrypt_iv(desc, req->dst, &sg,
+						 AES_BLOCK_SIZE);
+	else
+		rc = crypto_blkcipher_decrypt_iv(desc, req->dst, &sg,
+						 AES_BLOCK_SIZE);
+	crypto_free_blkcipher(desc->tfm);
+
 out:
 	return rc;
 }
@@ -166,79 +230,85 @@ static int gcm_aes_nx_crypt(struct aead_request *req, int enc)
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
 	struct blkcipher_desc desc;
 	unsigned int nbytes = req->cryptlen;
+	unsigned int processed = 0, to_process;
 	unsigned long irq_flags;
+	u32 max_sg_len;
 	int rc = -EINVAL;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen)
-		goto out;
-
 	desc.info = nx_ctx->priv.gcm.iv;
 	/* initialize the counter */
 	*(u32 *)(desc.info + NX_GCM_CTR_OFFSET) = 1;
 
-	/* For scenarios where the input message is zero length, AES CTR mode
-	 * may be used. Set the source data to be a single block (16B) of all
-	 * zeros, and set the input IV value to be the same as the GMAC IV
-	 * value. - nx_wb 4.8.1.3 */
 	if (nbytes == 0) {
-		char src[AES_BLOCK_SIZE] = {};
-		struct scatterlist sg;
-
-		desc.tfm = crypto_alloc_blkcipher("ctr(aes)", 0, 0);
-		if (IS_ERR(desc.tfm)) {
-			rc = -ENOMEM;
-			goto out;
-		}
-
-		crypto_blkcipher_setkey(desc.tfm, csbcpb->cpb.aes_gcm.key,
-			NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_128 ? 16 :
-			NX_CPB_KEY_SIZE(csbcpb) == NX_KS_AES_192 ? 24 : 32);
-
-		sg_init_one(&sg, src, AES_BLOCK_SIZE);
-		if (enc)
-			crypto_blkcipher_encrypt_iv(&desc, req->dst, &sg,
-						    AES_BLOCK_SIZE);
-		else
-			crypto_blkcipher_decrypt_iv(&desc, req->dst, &sg,
-						    AES_BLOCK_SIZE);
-		crypto_free_blkcipher(desc.tfm);
-
-		rc = 0;
+		rc = gcm_empty(req, &desc, enc);
 		goto out;
 	}
 
-	desc.tfm = (struct crypto_blkcipher *)req->base.tfm;
-
+	/* Process associated data */
 	csbcpb->cpb.aes_gcm.bit_length_aad = req->assoclen * 8;
-
 	if (req->assoclen) {
 		rc = nx_gca(nx_ctx, req, csbcpb->cpb.aes_gcm.in_pat_or_aad);
 		if (rc)
 			goto out;
 	}
 
-	if (enc)
+	/* Set flags for encryption */
+	NX_CPB_FDM(csbcpb) &= ~NX_FDM_CONTINUATION;
+	if (enc) {
 		NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
-	else
+	} else {
+		NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
 		nbytes -= crypto_aead_authsize(crypto_aead_reqtfm(req));
+	}
 
-	csbcpb->cpb.aes_gcm.bit_length_data = nbytes * 8;
+	/* page_limit: number of sg entries that fit on one page */
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
-	rc = nx_build_sg_lists(nx_ctx, &desc, req->dst, req->src, nbytes, 0,
-			       csbcpb->cpb.aes_gcm.iv_or_cnt);
-	if (rc)
-		goto out;
+	do {
+		/*
+		 * to_process: the data chunk to process in this update.
+		 * This value is bound by sg list limits.
+		 */
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-			   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if ((to_process + processed) < nbytes)
+			NX_CPB_FDM(csbcpb) |= NX_FDM_INTERMEDIATE;
+		else
+			NX_CPB_FDM(csbcpb) &= ~NX_FDM_INTERMEDIATE;
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
+		csbcpb->cpb.aes_gcm.bit_length_data = nbytes * 8;
+		desc.tfm = (struct crypto_blkcipher *) req->base.tfm;
+		rc = nx_build_sg_lists(nx_ctx, &desc, req->dst,
+				       req->src, to_process, processed,
+				       csbcpb->cpb.aes_gcm.iv_or_cnt);
+		if (rc)
+			goto out;
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   req->base.flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		memcpy(desc.info, csbcpb->cpb.aes_gcm.out_cnt, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_gcm.in_pat_or_aad,
+			csbcpb->cpb.aes_gcm.out_pat_or_mac, AES_BLOCK_SIZE);
+		memcpy(csbcpb->cpb.aes_gcm.in_s0,
+			csbcpb->cpb.aes_gcm.out_s0, AES_BLOCK_SIZE);
+
+		NX_CPB_FDM(csbcpb) |= NX_FDM_CONTINUATION;
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 
 	if (enc) {
 		/* copy out the auth tag */
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 04/10] crypto: nx - fix limits to sg lists for AES-CTR
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

This patch updates the nx-aes-ctr implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - "ibm,max-sg-len": maximum number of bytes of each scatter/gather
   list.

 - "ibm,max-sync-cop":
    - The total number of bytes that a scatter/gather list can hold.
    - The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-ctr.c | 50 ++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ctr.c b/drivers/crypto/nx/nx-aes-ctr.c
index 80dee8d..a37d009 100644
--- a/drivers/crypto/nx/nx-aes-ctr.c
+++ b/drivers/crypto/nx/nx-aes-ctr.c
@@ -89,33 +89,45 @@ static int ctr_aes_nx_crypt(struct blkcipher_desc *desc,
 	struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
 	unsigned long irq_flags;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 	int rc;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen) {
-		rc = -EINVAL;
-		goto out;
-	}
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
-	rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0,
-			       csbcpb->cpb.aes_ctr.iv);
-	if (rc)
-		goto out;
+	do {
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+		to_process = to_process & ~(AES_BLOCK_SIZE - 1);
 
-	if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
-		rc = -EINVAL;
-		goto out;
-	}
+		rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+				       processed, csbcpb->cpb.aes_ctr.iv);
+		if (rc)
+			goto out;
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
+			rc = -EINVAL;
+			goto out;
+		}
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		memcpy(desc->info, csbcpb->cpb.aes_cbc.cv, AES_BLOCK_SIZE);
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 out:
 	spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
 	return rc;
-- 
1.7.12

^ permalink raw reply related

* [PATCH v2 02/10] crypto: nx - fix limits to sg lists for AES-ECB
From: Marcelo Cerri @ 2013-08-29 14:36 UTC (permalink / raw)
  To: herbert; +Cc: Marcelo Cerri, linuxppc-dev, linux-kernel, linux-crypto
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>

This patch updates the nx-aes-ecb implementation to perform several
hyper calls if needed in order to always respect the length limits for
scatter/gather lists.

Two different limits are considered:

 - "ibm,max-sg-len": maximum number of bytes of each scatter/gather
   list.

 - "ibm,max-sync-cop":
    - The total number of bytes that a scatter/gather list can hold.
    - The maximum number of elements that a scatter/gather list can have.

Reviewed-by: Joy Latten <jmlatten@linux.vnet.ibm.com>
Signed-off-by: Marcelo Cerri <mhcerri@linux.vnet.ibm.com>
---
 drivers/crypto/nx/nx-aes-ecb.c | 48 ++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/crypto/nx/nx-aes-ecb.c b/drivers/crypto/nx/nx-aes-ecb.c
index fe0d803..85a8d23 100644
--- a/drivers/crypto/nx/nx-aes-ecb.c
+++ b/drivers/crypto/nx/nx-aes-ecb.c
@@ -71,37 +71,49 @@ static int ecb_aes_nx_crypt(struct blkcipher_desc *desc,
 	struct nx_crypto_ctx *nx_ctx = crypto_blkcipher_ctx(desc->tfm);
 	struct nx_csbcpb *csbcpb = nx_ctx->csbcpb;
 	unsigned long irq_flags;
+	unsigned int processed = 0, to_process;
+	u32 max_sg_len;
 	int rc;
 
 	spin_lock_irqsave(&nx_ctx->lock, irq_flags);
 
-	if (nbytes > nx_ctx->ap->databytelen) {
-		rc = -EINVAL;
-		goto out;
-	}
+	max_sg_len = min_t(u32, nx_driver.of.max_sg_len/sizeof(struct nx_sg),
+			   nx_ctx->ap->sglen);
 
 	if (enc)
 		NX_CPB_FDM(csbcpb) |= NX_FDM_ENDE_ENCRYPT;
 	else
 		NX_CPB_FDM(csbcpb) &= ~NX_FDM_ENDE_ENCRYPT;
 
-	rc = nx_build_sg_lists(nx_ctx, desc, dst, src, nbytes, 0, NULL);
-	if (rc)
-		goto out;
+	do {
+		to_process = min_t(u64, nbytes - processed,
+				   nx_ctx->ap->databytelen);
+		to_process = min_t(u64, to_process,
+				   NX_PAGE_SIZE * (max_sg_len - 1));
+		to_process = to_process & ~(AES_BLOCK_SIZE - 1);
 
-	if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
-		rc = -EINVAL;
-		goto out;
-	}
+		rc = nx_build_sg_lists(nx_ctx, desc, dst, src, to_process,
+				processed, NULL);
+		if (rc)
+			goto out;
 
-	rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
-			   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
-	if (rc)
-		goto out;
+		if (!nx_ctx->op.inlen || !nx_ctx->op.outlen) {
+			rc = -EINVAL;
+			goto out;
+		}
+
+		rc = nx_hcall_sync(nx_ctx, &nx_ctx->op,
+				   desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP);
+		if (rc)
+			goto out;
+
+		atomic_inc(&(nx_ctx->stats->aes_ops));
+		atomic64_add(csbcpb->csb.processed_byte_count,
+			     &(nx_ctx->stats->aes_bytes));
+
+		processed += to_process;
+	} while (processed < nbytes);
 
-	atomic_inc(&(nx_ctx->stats->aes_ops));
-	atomic64_add(csbcpb->csb.processed_byte_count,
-		     &(nx_ctx->stats->aes_bytes));
 out:
 	spin_unlock_irqrestore(&nx_ctx->lock, irq_flags);
 	return rc;
-- 
1.7.12

^ 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