Re: [PATCH 0/4] Rearrange functions to remove forward declarations

[David Daney]

On 10/26/2017 08:34 AM, PrasannaKumar Muralidharan wrote:

This patch series rearranges functions such that forward declarations
becomes unnecessary. Remove those forward declaration.


Why?

The code churn and increase in difficulty of attribution are big 
drawbacks to this sort of patch set.





This patch series is boot tested without user space in qemu with
CONFIG_HW_RANDOM=y.

PrasannaKumar Muralidharan (4):
   hw_random: core: Remove forward declaration by rearranging code
   hw_random: core: Rearranging rng_get_data to remove forward
 declaration
   hw_random: core: Rearranging start_khwrngd to remove forward
 declaration
   hw_random: core: Remove forward declaration of hwrng_init

  drivers/char/hw_random/core.c | 151 --
  1 file changed, 72 insertions(+), 79 deletions(-)

Re: crypto/cavium MSI-X fixups

[David Daney]

On 02/21/2017 11:27 PM, Christoph Hellwig wrote:

On Tue, Feb 21, 2017 at 09:36:04AM -0800, David Daney wrote:

With respect to pci_enable_msix(), what do you recommend as a replacement?


pci_alloc_irq_vectors.  In fact I have a tree ready for after -rc1
that removes pci_enable_msix() entirely.


I would like to see something that handles the case of this driver that 
I am attempting to get merged:



https://lkml.org/lkml/2017/2/15/1209





For the crypto/cavium driver, you recommend pci_alloc_irq_vectors(), which
works well if the required MSI-X indexes are contiguous starting at zero.
What would be used for a device that has 184 MSI-X, but only a sparse
subset (fewer than half) of these are required for the driver operation.
It would waste system resources to use an API that forces us to allocate
184 when only 80 are required.


Currently we don't have a good API for that.  I've not been through all
users of pci_enable_msix_{range,exact} yet, but so far I've only found
one user not using all vectors from 0 to some limit.  Depending how many
such users we have and how they'll look I will have to look into an API
to support that use case.



See the patch above for the case that doesn't use 0..UPPER_LIMIT

David Daney

Re: crypto/cavium MSI-X fixups

[David Daney]

On 02/19/2017 09:32 AM, Christoph Hellwig wrote:

Herbert,

any comment?  I'd really like to avoid introducing new pci_enable_msix
users in this merge window..


Hi Cristoph,

With respect to pci_enable_msix(), what do you recommend as a 
replacement?  For the crypto/cavium driver, you recommend 
pci_alloc_irq_vectors(), which works well if the required MSI-X indexes 
are contiguous starting at zero.   What would be used for a device that 
has 184 MSI-X, but only a sparse subset (fewer than half) of these are 
required for the driver operation.  It would waste system resources to 
use an API that forces us to allocate 184 when only 80 are required.


Currently pci_enable_msix() allows an arbitrary set of MSI-X to be 
requested, which exactly fits the requirements of our (non 
crypto/cavium) hardware.


Thanks in advance for any insight you can provide,
David Daney




On Wed, Feb 15, 2017 at 02:47:09PM +0530, George Cherian wrote:

Hi Christoph,


On 02/15/2017 12:48 PM, Christoph Hellwig wrote:

Hi George,

your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
add a new caller to pci_enable_msix.  This API has long been deprecated
so this series switches it to use pci_alloc_irq_vectors instead.

Can you please test it and make sure it goes in before the end of the
merge window so that no more users of the old API hit mainline?


Yes the changes works well.
Acked-by: George Cherian <george.cher...@cavium.com>

for the series.



---end quoted text---

Re: [PATCH] crypto: cavium: fix Kconfig dependencies

[David Daney]

On 02/14/2017 10:26 AM, Randy Dunlap wrote:

On 02/14/17 09:09, David Daney wrote:

On 02/14/2017 09:07 AM, Arnd Bergmann wrote:

The driver fails to build if MSI support is disabled:

In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete 
element type 'struct msix_entry'
  struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
^~~~
drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of 
function 'pci_enable_msix';did you mean 'cpt_enable_msix'? 
[-Werror=implicit-function-declaration]

On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
so let's allow compile-testing to catch this kind of problem more easily.
The 64-bit dependency is needed for the use of readq/writeq.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
 drivers/crypto/cavium/cpt/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/cavium/cpt/Kconfig 
b/drivers/crypto/cavium/cpt/Kconfig
index 247f1cbbefc1..cbd51b1aa046 100644
--- a/drivers/crypto/cavium/cpt/Kconfig
+++ b/drivers/crypto/cavium/cpt/Kconfig
@@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT

 config CAVIUM_CPT
 tristate "Cavium Cryptographic Accelerator driver"
-depends on ARCH_THUNDER
+depends on ARCH_THUNDER || COMPILE_TEST
+depends on PCI_MSI && 64BIT



Perhaps we should select PCI and PCI_MSI instead.

These systems cannot function without those.


Then the "depends" (and hence the patch) is correct.

A driver should not enable all of PCI if it is disabled
for some other reason.


I see your point.  In that case, this patch:

Acked-by: David Daney <david.da...@cavium.com>





 select CRYPTO_DEV_CPT
 help
   Support for Cavium CPT block found in octeon-tx series of

Re: [PATCH] crypto: cavium: fix Kconfig dependencies

[David Daney]

On 02/14/2017 09:07 AM, Arnd Bergmann wrote:

The driver fails to build if MSI support is disabled:

In file included from /git/arm-soc/drivers/crypto/cavium/cpt/cptpf_main.c:18:0:
drivers/crypto/cavium/cpt/cptpf.h:57:20: error: array type has incomplete 
element type 'struct msix_entry'
  struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
^~~~
drivers/crypto/cavium/cpt/cptpf_main.c: In function 'cpt_enable_msix':
drivers/crypto/cavium/cpt/cptpf_main.c:344:8: error: implicit declaration of 
function 'pci_enable_msix';did you mean 'cpt_enable_msix'? 
[-Werror=implicit-function-declaration]

On the other hand, it doesn't seem to have any build dependency on ARCH_THUNDER,
so let's allow compile-testing to catch this kind of problem more easily.
The 64-bit dependency is needed for the use of readq/writeq.

Signed-off-by: Arnd Bergmann 
---
 drivers/crypto/cavium/cpt/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/cavium/cpt/Kconfig 
b/drivers/crypto/cavium/cpt/Kconfig
index 247f1cbbefc1..cbd51b1aa046 100644
--- a/drivers/crypto/cavium/cpt/Kconfig
+++ b/drivers/crypto/cavium/cpt/Kconfig
@@ -7,7 +7,8 @@ config CRYPTO_DEV_CPT

 config CAVIUM_CPT
tristate "Cavium Cryptographic Accelerator driver"
-   depends on ARCH_THUNDER
+   depends on ARCH_THUNDER || COMPILE_TEST
+   depends on PCI_MSI && 64BIT



Perhaps we should select PCI and PCI_MSI instead.

These systems cannot function without those.


select CRYPTO_DEV_CPT
help
  Support for Cavium CPT block found in octeon-tx series of

[PATCH] Revert "hwrng: core - zeroize buffers with random data"

[David Daney]

This reverts commit 2cc751545854d7bd7eedf4d7e377bb52e176cd07.

With this commit in place I get on a Cavium ThunderX (arm64) system:

$ if=/dev/hwrng bs=256 count=1 | od -t x1 -A x -v > rng-bad.txt
1+0 records in
1+0 records out
256 bytes (256 B) copied, 9.1171e-05 s, 2.8 MB/s
$ dd if=/dev/hwrng bs=256 count=1 | od -t x1 -A x -v >> rng-bad.txt
1+0 records in
1+0 records out
256 bytes (256 B) copied, 9.6141e-05 s, 2.7 MB/s
$ cat rng-bad.txt
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50 00 00 00 00 37 20 46 ae d0 fc 1c 55 25 6e b0 b8
60 7c 7e d7 d4 00 0f 6f b2 91 1e 30 a8 fa 3e 52 0e
70 06 2d 53 30 be a1 20 0f aa 56 6e 0e 44 6e f4 35
80 b7 6a fe d2 52 70 7e 58 56 02 41 ea d1 9c 6a 6a
90 d1 bd d8 4c da 35 45 ef 89 55 fc 59 d5 cd 57 ba
a0 4e 3e 02 1c 12 76 43 37 23 e1 9f 7a 9f 9e 99 24
b0 47 b2 de e3 79 85 f6 55 7e ad 76 13 4f a0 b5 41
c0 c6 92 42 01 d9 12 de 8f b4 7b 6e ae d7 24 fc 65
d0 4d af 0a aa 36 d9 17 8d 0e 8b 7a 3b b6 5f 96 47
e0 46 f7 d8 ce 0b e8 3e c6 13 a6 2c b6 d6 cc 17 26
f0 e3 c3 17 8e 9e 45 56 1e 41 ef 29 1a a8 65 c8 3a
000100
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
50 00 00 00 00 f4 90 65 aa 8b f2 5e 31 01 53 b4 d4
60 06 c0 23 a2 99 3d 01 e4 b0 c1 b1 55 0f 80 63 cf
70 33 24 d8 3a 1d 5e cd 2c ba c0 d0 18 6f bc 97 46
80 1e 19 51 b1 90 15 af 80 5e d1 08 0d eb b0 6c ab
90 6a b4 fe 62 37 c5 e1 ee 93 c3 58 78 91 2a d5 23
a0 63 50 eb 1f 3b 84 35 18 cf b2 a4 b8 46 69 9e cf
b0 0c 95 af 03 51 45 a8 42 f1 64 c9 55 fc 69 76 63
c0 98 9d 82 fa 76 85 24 da 80 07 29 fe 4e 76 0c 61
d0 ff 23 94 4f c8 5c ce 0b 50 e8 31 bc 9d ce f4 ca
e0 be ca 28 da e6 fa cc 64 1c ec a8 41 db fe 42 bd
f0 a0 e2 4b 32 b4 52 ba 03 70 8e c1 8e d0 50 3a c6
000100

To my untrained mental entropy detector, the first several bytes of
each read from /dev/hwrng seem to not be very random (i.e. all zero).

When I revert the patch (apply this patch), I get back to what we have
in v4.9, which looks like (much more random appearing):

$ dd if=/dev/hwrng bs=256 count=1 | od -t x1 -A x -v > rng-good.txt
1+0 records in
1+0 records out
256 bytes (256 B) copied, 0.000252233 s, 1.0 MB/s
$ dd if=/dev/hwrng bs=256 count=1 | od -t x1 -A x -v >> rng-good.txt
1+0 records in
1+0 records out
256 bytes (256 B) copied, 0.000113571 s, 2.3 MB/s
$ cat rng-good.txt
00 75 d1 2d 19 68 1f d2 26 a1 49 22 61 66 e8 09 e5
10 e0 4e 10 d0 1a 2c 45 5d 59 04 79 8e e2 b7 2c 2e
20 e8 ad da 34 d5 56 51 3d 58 29 c7 7a 8e ed 22 67
30 f9 25 b9 fb c6 b7 9c 35 1f 84 21 35 c1 1d 48 34
40 45 7c f6 f1 57 63 1a 88 38 e8 81 f0 a9 63 ad 0e
50 be 5d 3e 74 2e 4e cb 36 c2 01 a8 14 e1 38 e1 bb
60 23 79 09 56 77 19 ff 98 e8 44 f3 27 eb 6e 0a cb
70 c9 36 e3 2a 96 13 07 a0 90 3f 3b bd 1d 04 1d 67
80 be 33 14 f8 02 c2 a4 02 ab 8b 5b 74 86 17 f0 5e
90 a1 d7 aa ef a6 21 7b 93 d1 85 86 eb 4e 8c d0 4c
a0 56 ac e4 45 27 44 84 9f 71 db 36 b9 f7 47 d7 b3
b0 f2 9c 62 41 a3 46 2b 5b e3 80 63 a4 35 b5 3c f4
c0 bc 1e 3a ad e4 59 4a 98 6c e8 8d ff 1b 16 f8 52
d0 05 5c 2f 52 2a 0f 45 5b 51 fb 93 97 a4 49 4f 06
e0 f3 a0 d1 1e ba 3d ed a7 60 8f bb 84 2c 21 94 2d
f0 b3 66 a6 61 1e 58 30 24 85 f8 c8 18 c3 77 00 22
000100
00 73 ca cc a1 d9 bb 21 8d c3 5c f3 ab 43 6d a7 a4
10 4a fd c5 f4 9c ba 4a 0f b1 2e 19 15 4e 84 26 e0
20 67 c9 f2 52 4d 65 1f 81 b7 8b 6d 2b 56 7b 99 75
30 2e cd d0 db 08 0c 4b df f3 83 c6 83 00 2e 2b b8
40 0f af 61 1d f2 02 35 74 b5 a4 6f 28 f3 a1 09 12
50 f2 53 b5 d2 da 45 01 e5 12 d6 46 f8 0b db ed 51
60 7b f4 0d 54 e0 63 ea 22 e2 1d d0 d6 d0 e7 7e e0
70 93 91 fb 87 95 43 41 28 de 3d 8b a3 a8 8f c4 9e
80 30 95 12 7a b2 27 28 ff 37 04 2e 09 7c dd 7c 12
90 e1 50 60 fb 6d 5f a8 65 14 40 89 e3 4c d2 87 8f
a0 34 76 7e 66 7a 8e 6b a3 fc cf 38 52 2e f9 26 f0
b0 98 63 15 06 34 99 b2 88 4f aa d8 14 88 71 f1 81
c0 be 51 11 2b f4 7e a0 1e 12 b2 44 2e f6 8d 84 ea
d0 63 82 2b 66 b3 9a fd 08 73 5a c2 cc ab 5a af b1
e0 88 e3 a6 80 4b fc db ed 71 e0 ae c0 0a a4 8c 35
f0 eb 89 f9 8a 4b 52 59 6f 09 7c 01 3f 56 e7 c7 bf
000100

Signed-off-by: David Daney <david.da...@cavium.com>
---
 drivers/char/hw_random/core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 6ce5ce8..87fba42 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -92,7 +92,6 @@ static void add_early_randomness(struct hwrng *rng)
   

[PATCH] hwrng: cavium: Use per device name to allow for multiple devices.

[David Daney]

Systems containing the Cavium HW RNG may have one device per NUMA
node.  A typical configuration is a 2-node NUMA system, which results
in 2 RNG devices.  The hwrng subsystem refuses (and rightly so) to
register more than one device with he same name, so we get failure
messages on these systems.

Make the hwrng name unique by including the underlying device name.
Also remove spaces from the name to make it possible to switch devices
via the sysfs knobs.

Signed-off-by: David Daney <david.da...@cavium.com>
---
 drivers/char/hw_random/cavium-rng-vf.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hw_random/cavium-rng-vf.c 
b/drivers/char/hw_random/cavium-rng-vf.c
index 066ae0e..dd1007a 100644
--- a/drivers/char/hw_random/cavium-rng-vf.c
+++ b/drivers/char/hw_random/cavium-rng-vf.c
@@ -57,7 +57,11 @@ static int cavium_rng_probe_vf(structpci_dev 
*pdev,
return -ENOMEM;
}
 
-   rng->ops.name= "cavium rng";
+   rng->ops.name = devm_kasprintf(>dev, GFP_KERNEL,
+  "cavium-rng-%s", dev_name(>dev));
+   if (!rng->ops.name)
+   return -ENOMEM;
+
rng->ops.read= cavium_rng_read;
rng->ops.quality = 1000;
 
-- 
1.8.3.1

Re: [PATCH 1/3] drivers: crypto: Add Support for Octeon-tx CPT Engine

[David Daney]

On 11/18/2016 07:00 AM, gcheri...@gmail.com wrote:

From: George Cherian 

Enable the Physical Function diver for the Cavium Crypto Engine (CPT)
found in Octeon-tx series of SoC's. CPT is the Cryptographic Acceleration
Unit. CPT includes microcoded GigaCypher symmetric engines (SEs) and
asymmetric engines (AEs).

Signed-off-by: George Cherian 



How was this tested?


---
  drivers/crypto/cavium/cpt/Kconfig|  22 +
  drivers/crypto/cavium/cpt/Makefile   |   2 +
  drivers/crypto/cavium/cpt/cpt.h  |  90 +++
  drivers/crypto/cavium/cpt/cpt_common.h   | 377 +
  drivers/crypto/cavium/cpt/cpt_hw_types.h | 940 +++
  drivers/crypto/cavium/cpt/cpt_main.c | 891 +
  drivers/crypto/cavium/cpt/cpt_pf_mbox.c  | 174 ++
  7 files changed, 2496 insertions(+)
  create mode 100644 drivers/crypto/cavium/cpt/Kconfig
  create mode 100644 drivers/crypto/cavium/cpt/Makefile
  create mode 100644 drivers/crypto/cavium/cpt/cpt.h
  create mode 100644 drivers/crypto/cavium/cpt/cpt_common.h
  create mode 100644 drivers/crypto/cavium/cpt/cpt_hw_types.h
  create mode 100644 drivers/crypto/cavium/cpt/cpt_main.c
  create mode 100644 drivers/crypto/cavium/cpt/cpt_pf_mbox.c

diff --git a/drivers/crypto/cavium/cpt/Kconfig 
b/drivers/crypto/cavium/cpt/Kconfig
new file mode 100644
index 000..8fe3f44
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/Kconfig
@@ -0,0 +1,22 @@
+#
+# Cavium crypto device configuration
+#
+
+config CRYPTO_DEV_CPT
+   tristate
+   select HW_RANDOM_OCTEON


This makes no sense.  HW_RANDOM_OCTEON is for a mips64 based SOC and 
isn't present on devices that have this crypto block.  Why select this?




+   select CRYPTO_AES
+   select CRYPTO_DES
+   select CRYPTO_BLKCIPHER
+   select FW_LOADER
+
+config OCTEONTX_CPT_PF
+   tristate "Octeon-tx CPT Physical function driver"
+   depends on ARCH_THUNDER
+   select CRYPTO_DEV_CPT
+   help
+ Support for Cavium CPT block found in octeon-tx series of
+ processors.
+
+ To compile this as a module, choose M here: the module will be
+ called cptpf.
diff --git a/drivers/crypto/cavium/cpt/Makefile 
b/drivers/crypto/cavium/cpt/Makefile
new file mode 100644
index 000..bf758e2
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_OCTEONTX_CPT_PF) += cptpf.o
+cptpf-objs := cpt_main.o cpt_pf_mbox.o
diff --git a/drivers/crypto/cavium/cpt/cpt.h b/drivers/crypto/cavium/cpt/cpt.h
new file mode 100644
index 000..63d12da
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/cpt.h
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2016 Cavium, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ */
+
+#ifndef __CPT_H
+#define __CPT_H
+
+#include "cpt_common.h"
+
+#define BASE_PROC_DIR  "cavium"
+
+#define PF  0
+#define VF  1
+
+struct cpt_device;
+
+struct microcode {
+   uint8_t  is_mc_valid;


s/uint8_t/u8/  ??

That could be done everywhere.

[...]

diff --git a/drivers/crypto/cavium/cpt/cpt_common.h 
b/drivers/crypto/cavium/cpt/cpt_common.h
new file mode 100644
index 000..351ed4a
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/cpt_common.h
@@ -0,0 +1,377 @@
+/*
+ * Copyright (C) 2016 Cavium, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ */
+
+#ifndef __CPT_COMMON_H
+#define __CPT_COMMON_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "cpt_hw_types.h"
+
+/* configuration space offsets */
+#ifndef PCI_VENDOR_ID
+#define PCI_VENDOR_ID 0x00 /* 16 bits */
+#endif
+#ifndef PCI_DEVICE_ID
+#define PCI_DEVICE_ID 0x02 /* 16 bits */
+#endif
+#ifndef PCI_REVISION_ID
+#define PCI_REVISION_ID 0x08 /* Revision ID */
+#endif
+#ifndef PCI_CAPABILITY_LIST
+#define PCI_CAPABILITY_LIST 0x34 /* first capability list entry */
+#endif
+


Standard PCI core functions give you access to all that information, use 
pdev->device, pdev->revision, etc. instead of reinventing the wheel here 
with all these #defines.




+/* Device ID */
+#define PCI_VENDOR_ID_CAVIUM 0x177d


This is defined in pci_ids.h, use value from there instead of placing a 
duplicate definition here.



+#define CPT_81XX_PCI_PF_DEVICE_ID 0xa040
+#define CPT_81XX_PCI_VF_DEVICE_ID 0xa041
+
+#define PASS_1_0 0x0
+
+/* CPT Models ((Device ID<<16)|Revision ID) */
+/* CPT models */
+#define CPT_81XX_PASS1_0 ((CPT_81XX_PCI_PF_DEVICE_ID << 8) | PASS_1_0)
+#define CPTVF_81XX_PASS1_0 ((CPT_81XX_PCI_VF_DEVICE_ID << 8) | PASS_1_0)
+
+#define PF 0
+#define VF 1
+
+#define DEFAULT_DEVICE_QUEUES 

Re: [PATCH v2 2/2] HWRNG: thunderx: Add Cavium HWRNG driver for ThunderX SoC.

[David Daney]

On 08/23/2016 10:46 PM, Corentin LABBE wrote:

Hello


+/* Read data from the RNG unit */
+static int cavium_rng_read(struct hwrng *rng, void *dat, size_t max, bool wait)
+{
+   struct cavium_rng *p = container_of(rng, struct cavium_rng, ops);
+   unsigned int size = max;
+
+   while (size >= 8) {
+   *((u64 *)dat) = readq(p->result);
+   size -= 8;
+   dat += 8;
+   }


I think you could use readsq()
This will increase throughput


If you look at the implementation of readsq(), you will see that it is a 
similar loop.  Since the overhead is primarily I/O latency from the RNG 
hardware, the throughput cannot really be changed with micro 
optimizations to this simple loop.


Also, on big-endian kernels, it appears that a loop of readq() and 
readsq() will give different results as readq will byte swap the result 
and readsq does not.  Since this is a RNG, the byte swapping is not 
important, but it is a difference.


Because of this, I think it should be acceptable to stick with the loop 
we currently have.


If the hwrng maintainers want to change the loop, to a readsq(), we 
might investigate this more.


Thanks,
David Daney


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] PCI/IOV: Add function to allow Function Dependency Link override.

[David Daney]

On 08/22/2016 07:36 AM, Bjorn Helgaas wrote:

Hi David & Omer,

On Fri, Aug 19, 2016 at 03:32:12PM -0700, Omer Khaliq wrote:

From: David Daney <david.da...@cavium.com>

Some hardware presents an incorrect SR-IOV Function Dependency Link,
add a function to allow this to be overridden in the PF driver for
such devices.

Signed-off-by: David Daney <david.da...@cavium.com>
Signed-off-by: Omer Khaliq <okha...@caviumnetworks.com>
---
  drivers/pci/iov.c   | 14 ++
  include/linux/pci.h |  1 +
  2 files changed, 15 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 2194b44..81f0672 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -640,6 +640,20 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
  EXPORT_SYMBOL_GPL(pci_enable_sriov);

  /**
+ * pci_sriov_fdl_override - fix incorrect Function Dependency Link
+ * @dev: the PCI device
+ * @fdl: the corrected Function Dependency Link value
+ *
+ * For hardware presenting an incorrect Function Dependency Link in
+ * the SR-IOV Extended Capability, allow a driver to override it.
+ */
+void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl)
+{
+   dev->sriov->link = fdl;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_fdl_override);


We usually use quirks to work around problems in config space.  That's
a nice mechanism because we don't have to add new PCI core interfaces
and it makes it clear that we're working around a hardware problem.

Can you use a quirk here?  We allocate dev->sriov in the
pci_init_capabilities() path, so it looks like a pci_fixup_final quirk
should work.



The struct pci_sriov definition is private to drivers/pci, so in order 
to use a quirk to fix this, we would have to put it in 
drivers/pci/quirks.c.  I was trying to keep this very device specific 
code in the driver, which requires an accessor to be able to manipulate 
the dev->sriov->link field.


If you prefer a quirk in drivers/pci/quirks.c, we can certainly do that 
instead.


Thanks for taking the time to look at this,
David Daney





+
+/**
   * pci_disable_sriov - disable the SR-IOV capability
   * @dev: the PCI device
   */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2599a98..da8a5b3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1823,6 +1823,7 @@ int pci_num_vf(struct pci_dev *dev);
  int pci_vfs_assigned(struct pci_dev *dev);
  int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
  int pci_sriov_get_totalvfs(struct pci_dev *dev);
+void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl);
  resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
  #else
  static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: on stack dynamic allocations

[David Daney]

On 08/16/2012 02:20 PM, Kasatkin, Dmitry wrote:

Hello,

Some places in the code uses variable-size allocation on stack..
For example from hmac_setkey():

struct {
struct shash_desc shash;
char ctx[crypto_shash_descsize(hash)];
} desc;


sparse complains

CHECK   crypto/hmac.c
crypto/hmac.c:57:47: error: bad constant expression

I like it instead of kmalloc..

But what is position of kernel community about it?


If you know that the range of crypto_shash_descsize(hash) is bounded, 
just use the upper bound.


If the range of crypto_shash_descsize(hash) is unbounded, then the stack 
will overflow and ... BOOM!


David Daney



--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] crypto: api.c: adding doxygen comments to api

[David Daney]

On 11/04/2010 09:32 AM, Mark Allyn wrote:

Subject: [PATCH 1/1] crypto: api.c: adding doxygen comments to api


Do we really use doxygen in the kernel?  Perhaps a subject line 
containing the string 'kernel-doc' would be more accurate.


Reading kernel-doc-nano-HOWTO.txt will show you the proper format.


Signed-off-by: Mark A. Allynmark.a.al...@intel.com
---
  crypto/api.c |   14 ++
  1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 033a714..a11939f 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -193,6 +193,13 @@ static struct crypto_alg *crypto_larval_wait(struct 
crypto_alg *alg)
return alg;
  }

+/**
+ * This gets the crypto_alg structure for an algorithm
+ * it return struct crypto_alg
+ * @name: name of the algorithm
+ * @type: crypto type (defined at about line 31 of include/linux/crypto.h)


Referencing kernel source file line numbers in documentation seems like 
an excellent way to have the documentation quickly be incorrect.




+ * @mask: mask of allowable crypto types
+ */
  struct crypto_alg *crypto_alg_lookup(const char *name, u32 type, u32 mask)
  {
struct crypto_alg *alg;
@@ -205,6 +212,13 @@ struct crypto_alg *crypto_alg_lookup(const char *name, u32 
type, u32 mask)
  }
  EXPORT_SYMBOL_GPL(crypto_alg_lookup);

+/**
+ * This gets the large value crypto_alg structure for an algorithm
+ * it return struct crypto_alg
+ * @name: name of the algorithm
+ * @type: crypto type (defined at about line 31 of include/linux/crypto.h)
+ * @mask: mask of allowable crypto types
+ */
  struct crypto_alg *crypto_larval_lookup(const char *name, u32 type, u32 mask)
  {
struct crypto_alg *alg;


--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html