Re: [PATCH] crypto: ccp - Fix sparse warnings
On Fri, Jul 03, 2020 at 02:46:52PM +1000, Herbert Xu wrote: > This patch fixes a number of endianness marking issues in the ccp > driver. > > Signed-off-by: Herbert Xu Acked-by: John Allen
Re: [PATCH v4 07/13] crypto: ccp - permit asynchronous skcipher as fallback
On Tue, Jul 07, 2020 at 09:31:57AM +0300, Ard Biesheuvel wrote: > Even though the ccp driver implements an asynchronous version of xts(aes), > the fallback it allocates is required to be synchronous. Given that SIMD > based software implementations are usually asynchronous as well, even > though they rarely complete asynchronously (this typically only happens > in cases where the request was made from softirq context, while SIMD was > already in use in the task context that it interrupted), these > implementations are disregarded, and either the generic C version or > another table based version implemented in assembler is selected instead. > > Since falling back to synchronous AES is not only a performance issue, but > potentially a security issue as well (due to the fact that table based AES > is not time invariant), let's fix this, by allocating an ordinary skcipher > as the fallback, and invoke it with the completion routine that was given > to the outer request. > > Signed-off-by: Ard Biesheuvel Acked-by: John Allen
Re: [PATCH] crypto: ccp - Silence strncpy warning
On Thu, Jul 09, 2020 at 10:44:04PM +1000, Herbert Xu wrote: > This patch kills an strncpy by using strscpy instead. The name > would be silently truncated if it is too long. > > Signed-off-by: Herbert Xu Acked-by: John Allen
Re: [PATCH v2] crypto: ccp: sp-pci: use generic power management
On Wed, Jul 22, 2020 at 03:00:58PM +0530, Vaibhav Gupta wrote: > Drivers using legacy power management .suspen()/.resume() callbacks > have to manage PCI states and device's PM states themselves. They also > need to take care of standard configuration registers. > > Switch to generic power management framework using a single > "struct dev_pm_ops" variable to take the unnecessary load from the driver. > This also avoids the need for the driver to directly call most of the PCI > helper functions and device power state control functions as through > the generic framework, PCI Core takes care of the necessary operations, > and drivers are required to do only device-specific jobs. > > Signed-off-by: Vaibhav Gupta Acked-by: John Allen
Re: [v3 PATCH 21/31] crypto: ccp - Remove rfc3686 implementation
On Tue, Jul 28, 2020 at 05:19:26PM +1000, Herbert Xu wrote: > The rfc3686 implementation in ccp is pretty much the same > as the generic rfc3686 wrapper. So it can simply be removed to > reduce complexity. > > Signed-off-by: Herbert Xu Acked-by: John Allen
Re: [PATCH] crypto: ccp - fix error handling
On Mon, Sep 21, 2020 at 01:34:35PM +0200, Pavel Machek wrote:
> Fix resource leak in error handling.
>
> Signed-off-by: Pavel Machek (CIP)
Acked-by: John Allen
>
> diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
> index bd270e66185e..40869ea1ed20 100644
> --- a/drivers/crypto/ccp/ccp-ops.c
> +++ b/drivers/crypto/ccp/ccp-ops.c
> @@ -1744,7 +1744,7 @@ ccp_run_sha_cmd(struct ccp_cmd_queue *cmd_q, struct
> ccp_cmd *cmd)
> break;
> default:
> ret = -EINVAL;
> - goto e_ctx;
> + goto e_data;
> }
> } else {
> /* Stash the context */
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[PATCH] crypto: ccp - Fix use of merged scatterlists
Running the crypto manager self tests with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS may result in several types of errors
when using the ccp-crypto driver:
alg: skcipher: cbc-des3-ccp encryption failed on test vector 0;
expected_error=0, actual_error=-5 ...
alg: skcipher: ctr-aes-ccp decryption overran dst buffer on test vector 0 ...
alg: ahash: sha224-ccp test failed (wrong result) on test vector ...
These errors are the result of improper processing of scatterlists mapped
for DMA.
Given a scatterlist in which entries are merged as part of mapping the
scatterlist for DMA, the DMA length of a merged entry will reflect the
combined length of the entries that were merged. The subsequent
scatterlist entry will contain DMA information for the scatterlist entry
after the last merged entry, but the non-DMA information will be that of
the first merged entry.
The ccp driver does not take this scatterlist merging into account. To
address this, add a second scatterlist pointer to track the current
position in the DMA mapped representation of the scatterlist. Both the DMA
representation and the original representation of the scatterlist must be
tracked as while most of the driver can use just the DMA representation,
scatterlist_map_and_copy() must use the original representation and
expects the scatterlist pointer to be accurate to the original
representation.
In order to properly walk the original scatterlist, the scatterlist must
be walked until the combined lengths of the entries seen is equal to the
DMA length of the current entry being processed in the DMA mapped
representation.
Fixes: 63b945091a070 ("crypto: ccp - CCP device driver and interface support")
Signed-off-by: John Allen
Cc: sta...@vger.kernel.org
---
drivers/crypto/ccp/ccp-dev.h | 1 +
drivers/crypto/ccp/ccp-ops.c | 37 +---
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 3f68262d9ab4..87a34d91fdf7 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -469,6 +469,7 @@ struct ccp_sg_workarea {
unsigned int sg_used;
struct scatterlist *dma_sg;
+ struct scatterlist *dma_sg_head;
struct device *dma_dev;
unsigned int dma_count;
enum dma_data_direction dma_dir;
diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 422193690fd4..64112c736810 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -63,7 +63,7 @@ static u32 ccp_gen_jobid(struct ccp_device *ccp)
static void ccp_sg_free(struct ccp_sg_workarea *wa)
{
if (wa->dma_count)
- dma_unmap_sg(wa->dma_dev, wa->dma_sg, wa->nents, wa->dma_dir);
+ dma_unmap_sg(wa->dma_dev, wa->dma_sg_head, wa->nents,
wa->dma_dir);
wa->dma_count = 0;
}
@@ -92,6 +92,7 @@ static int ccp_init_sg_workarea(struct ccp_sg_workarea *wa,
struct device *dev,
return 0;
wa->dma_sg = sg;
+ wa->dma_sg_head = sg;
wa->dma_dev = dev;
wa->dma_dir = dma_dir;
wa->dma_count = dma_map_sg(dev, sg, wa->nents, dma_dir);
@@ -104,14 +105,28 @@ static int ccp_init_sg_workarea(struct ccp_sg_workarea
*wa, struct device *dev,
static void ccp_update_sg_workarea(struct ccp_sg_workarea *wa, unsigned int
len)
{
unsigned int nbytes = min_t(u64, len, wa->bytes_left);
+ unsigned int sg_combined_len = 0;
if (!wa->sg)
return;
wa->sg_used += nbytes;
wa->bytes_left -= nbytes;
- if (wa->sg_used == wa->sg->length) {
- wa->sg = sg_next(wa->sg);
+ if (wa->sg_used == sg_dma_len(wa->dma_sg)) {
+ /* Advance to the next DMA scatterlist entry */
+ wa->dma_sg = sg_next(wa->dma_sg);
+
+ /* In the case that the DMA mapped scatterlist has entries
+* that have been merged, the non-DMA mapped scatterlist
+* must be advanced multiple times for each merged entry.
+* This ensures that the current non-DMA mapped entry
+* corresponds to the current DMA mapped entry.
+*/
+ do {
+ sg_combined_len += wa->sg->length;
+ wa->sg = sg_next(wa->sg);
+ } while (wa->sg_used > sg_combined_len);
+
wa->sg_used = 0;
}
}
@@ -299,7 +314,7 @@ static unsigned int ccp_queue_buf(struct ccp_data *data,
unsigned int from)
/* Update the structures and generate the count */
buf_count = 0;
while (sg_wa->bytes_left && (buf_count < dm_wa->length)) {
- nbytes = min(sg_wa->sg->length - sg_wa->sg_used,
+ nbytes = min(sg_dma_len(sg_wa->dma_sg) - sg_wa->sg_used,
Re: [PATCH] crypto: ccp - Fix use of merged scatterlists
On Thu, Jun 25, 2020 at 02:53:55PM +, Sasha Levin wrote:
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: 63b945091a07 ("crypto: ccp - CCP device driver and interface
> support").
>
> The bot has tested the following trees: v5.7.5, v5.4.48, v4.19.129,
> v4.14.185, v4.9.228, v4.4.228.
>
> v5.7.5: Build OK!
> v5.4.48: Build OK!
> v4.19.129: Build OK!
> v4.14.185: Build OK!
> v4.9.228: Build OK!
> v4.4.228: Failed to apply! Possible dependencies:
> 3f19ce2054541 ("crypto: ccp - Remove check for x86 family and model")
> 553d2374db0bb ("crypto: ccp - Support for multiple CCPs")
> c7019c4d739e7 ("crypto: ccp - CCP versioning support")
> ea0375afa1728 ("crypto: ccp - Add abstraction for device-specific calls")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
Please apply this patch to v4.9.x and newer.
Thanks,
John
>
> --
> Thanks
> Sasha
Re: [PATCH v2] crypto: ccp - Update CCP driver maintainer information
On Fri, Jun 26, 2020 at 02:09:39PM -0500, Tom Lendacky wrote: > From: Tom Lendacky > > Add John Allen as a new CCP driver maintainer. Additionally, break out > the driver SEV support and create a new maintainer entry, with Brijesh > Singh and Tom Lendacky as maintainers. > > Cc: John Allen > Cc: Brijesh Singh > Signed-off-by: Tom Lendacky Acked-by: John Allen > > --- > > Changes from v1: > - Change the email for Brijesh. The previous one is an alias, use the > proper email address in case the alias is ever removed. > --- > MAINTAINERS | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 68f21d46614c..de266ca5f921 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -830,11 +830,20 @@ F: include/uapi/rdma/efa-abi.h > > AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER > M: Tom Lendacky > +M: John Allen > L: linux-crypto@vger.kernel.org > S: Supported > F: drivers/crypto/ccp/ > F: include/linux/ccp.h > > +AMD CRYPTOGRAPHIC COPROCESSOR (CCP) DRIVER - SEV SUPPORT > +M: Brijesh Singh > +M: Tom Lendacky > +L: linux-crypto@vger.kernel.org > +S: Supported > +F: drivers/crypto/ccp/sev* > +F: include/uapi/linux/psp-sev.h > + > AMD DISPLAY CORE > M: Harry Wentland > M: Leo Li > -- > 2.27.0 >
Re: problem with ccp-crypto module on apu
On Thu, Dec 17, 2020 at 07:42:52AM +, Domen Stangar wrote: > Hi, > I would like to report issue with ccp-crypto. > When I issue modprobe command, it would not continue, without SIGINT signal > (ctrl+c). > If module is compiled in kernel, boot doesn't finish. > Looks like problem is that ecb(aes) selftest do not finish ? Hi Domen, Thanks for reporting this issue. I'll look into this as soon as possible. Once I can track down some hardware, I'll reproduce and debug. Thanks, John
Re: problem with ccp-crypto module on apu
On Mon, Jan 04, 2021 at 04:10:26PM +, Domen Stangar wrote: > Device name: ccp-1 >RNG name: ccp-1-rng ># Queues: 3 > # Cmds: 0 > Version: 5 > Engines: AES 3DES SHA RSA ECC ZDE TRNG > Queues: 5 > LSB Entries: 128 > > Let me know if you need anything else. Hi Domen, Looks like we may have a lead on this problem. Could you provide the following when you're loading the module? dmesg /proc/interrupts /sys/kernel/debug/ccp/ccp-1/stats Thanks, John > Domen > > > Domen, do you have the debugfs support enabled? Could you supply the output > > from /sys/kernel/debug/ccp/ccp-X/info (where X is replaced with each of the > > present ccp ordinal values)? > > > > Thanks, > > Tom > > >
Re: [PATCH v3 06/19] crypto: ccp: convert tasklets to use new tasklet_setup() API
On Tue, Jan 12, 2021 at 07:16:37AM +0530, Allen Pais wrote:
> From: Allen Pais
>
> In preparation for unconditionally passing the
> struct tasklet_struct pointer to all tasklet
> callbacks, switch to using the new tasklet_setup()
> and from_tasklet() to pass the tasklet pointer explicitly.
>
> Signed-off-by: Romain Perier
> Signed-off-by: Allen Pais
Acked-by: John Allen
> ---
> drivers/crypto/ccp/ccp-dev-v3.c| 9 -
> drivers/crypto/ccp/ccp-dev-v5.c| 9 -
> drivers/crypto/ccp/ccp-dmaengine.c | 7 +++
> 3 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/crypto/ccp/ccp-dev-v3.c b/drivers/crypto/ccp/ccp-dev-v3.c
> index 0d5576f6ad21..858566867fa3 100644
> --- a/drivers/crypto/ccp/ccp-dev-v3.c
> +++ b/drivers/crypto/ccp/ccp-dev-v3.c
> @@ -321,9 +321,9 @@ static void ccp_enable_queue_interrupts(struct ccp_device
> *ccp)
> iowrite32(ccp->qim, ccp->io_regs + IRQ_MASK_REG);
> }
>
> -static void ccp_irq_bh(unsigned long data)
> +static void ccp_irq_bh(struct tasklet_struct *t)
> {
> - struct ccp_device *ccp = (struct ccp_device *)data;
> + struct ccp_device *ccp = from_tasklet(ccp, t, irq_tasklet);
> struct ccp_cmd_queue *cmd_q;
> u32 q_int, status;
> unsigned int i;
> @@ -361,7 +361,7 @@ static irqreturn_t ccp_irq_handler(int irq, void *data)
> if (ccp->use_tasklet)
> tasklet_schedule(&ccp->irq_tasklet);
> else
> - ccp_irq_bh((unsigned long)ccp);
> + ccp_irq_bh(&ccp->irq_tasklet);
>
> return IRQ_HANDLED;
> }
> @@ -457,8 +457,7 @@ static int ccp_init(struct ccp_device *ccp)
>
> /* Initialize the ISR tasklet? */
> if (ccp->use_tasklet)
> - tasklet_init(&ccp->irq_tasklet, ccp_irq_bh,
> - (unsigned long)ccp);
> + tasklet_setup(&ccp->irq_tasklet, ccp_irq_bh);
>
> dev_dbg(dev, "Starting threads...\n");
> /* Create a kthread for each queue */
> diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
> index 7838f63bab32..e68b05a3169b 100644
> --- a/drivers/crypto/ccp/ccp-dev-v5.c
> +++ b/drivers/crypto/ccp/ccp-dev-v5.c
> @@ -733,9 +733,9 @@ static void ccp5_enable_queue_interrupts(struct
> ccp_device *ccp)
> iowrite32(SUPPORTED_INTERRUPTS, ccp->cmd_q[i].reg_int_enable);
> }
>
> -static void ccp5_irq_bh(unsigned long data)
> +static void ccp5_irq_bh(struct tasklet_struct *t)
> {
> - struct ccp_device *ccp = (struct ccp_device *)data;
> + struct ccp_device *ccp = from_tasklet(ccp, t, irq_tasklet);
> u32 status;
> unsigned int i;
>
> @@ -772,7 +772,7 @@ static irqreturn_t ccp5_irq_handler(int irq, void *data)
> if (ccp->use_tasklet)
> tasklet_schedule(&ccp->irq_tasklet);
> else
> - ccp5_irq_bh((unsigned long)ccp);
> + ccp5_irq_bh(&ccp->irq_tasklet);
> return IRQ_HANDLED;
> }
>
> @@ -894,8 +894,7 @@ static int ccp5_init(struct ccp_device *ccp)
> }
> /* Initialize the ISR tasklet */
> if (ccp->use_tasklet)
> - tasklet_init(&ccp->irq_tasklet, ccp5_irq_bh,
> - (unsigned long)ccp);
> + tasklet_setup(&ccp->irq_tasklet, ccp5_irq_bh);
>
> dev_dbg(dev, "Loading LSB map...\n");
> /* Copy the private LSB mask to the public registers */
> diff --git a/drivers/crypto/ccp/ccp-dmaengine.c
> b/drivers/crypto/ccp/ccp-dmaengine.c
> index 0770a83bf1a5..a85690866b05 100644
> --- a/drivers/crypto/ccp/ccp-dmaengine.c
> +++ b/drivers/crypto/ccp/ccp-dmaengine.c
> @@ -121,9 +121,9 @@ static void ccp_cleanup_desc_resources(struct ccp_device
> *ccp,
> }
> }
>
> -static void ccp_do_cleanup(unsigned long data)
> +static void ccp_do_cleanup(struct tasklet_struct *t)
> {
> - struct ccp_dma_chan *chan = (struct ccp_dma_chan *)data;
> + struct ccp_dma_chan *chan = from_tasklet(chan, t, cleanup_tasklet);
> unsigned long flags;
>
> dev_dbg(chan->ccp->dev, "%s - chan=%s\n", __func__,
> @@ -712,8 +712,7 @@ int ccp_dmaengine_register(struct ccp_device *ccp)
> INIT_LIST_HEAD(&chan->active);
> INIT_LIST_HEAD(&chan->complete);
>
> - tasklet_init(&chan->cleanup_tasklet, ccp_do_cleanup,
> - (unsigned long)chan);
> + tasklet_setup(&chan->cleanup_tasklet, ccp_do_cleanup);
>
> dma_chan->device = dma_dev;
> dma_cookie_init(dma_chan);
> --
> 2.25.1
>
Re: [PATCH] crypto: ccp -A value assigned to a variable is never used.
On Tue, Mar 30, 2021 at 06:10:29PM +0800, Jiapeng Chong wrote: > Fix the following whitescan warning: > > Assigning value "64" to "dst.address" here, but that stored value is > overwritten before it can be used. > Thanks for reporting. Acked-by: John Allen > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/crypto/ccp/ccp-ops.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c > index d6a8f4e..bb88198 100644 > --- a/drivers/crypto/ccp/ccp-ops.c > +++ b/drivers/crypto/ccp/ccp-ops.c > @@ -2418,7 +2418,6 @@ static int ccp_run_ecc_pm_cmd(struct ccp_cmd_queue > *cmd_q, struct ccp_cmd *cmd) > dst.address += CCP_ECC_OUTPUT_SIZE; > ccp_reverse_get_dm_area(&dst, 0, ecc->u.pm.result.y, 0, > CCP_ECC_MODULUS_BYTES); > - dst.address += CCP_ECC_OUTPUT_SIZE; > > /* Restore the workarea address */ > dst.address = save; > -- > 1.8.3.1 >
