Re: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
On Thu, Nov 02, 2023 at 01:25:31PM +, Gonglei (Arei) wrote: > > > > -Original Message- > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: Thursday, November 2, 2023 9:17 PM > > To: Gonglei (Arei) > > Cc: Halil Pasic ; Herbert Xu > > ; Jason Wang ; > > virtualizat...@lists.linux-foundation.org; linux-ker...@vger.kernel.org; > > linux-crypto@vger.kernel.org; Marc Hartmayer > > Subject: Re: virtcrypto_dataq_callback calls crypto_finalize_request() from > > irq > > context > > > > On Thu, Nov 02, 2023 at 01:04:07PM +, Gonglei (Arei) wrote: > > > > > > > > > > -Original Message- > > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > > Sent: Wednesday, November 1, 2023 9:26 PM > > > > To: Halil Pasic > > > > Cc: Gonglei (Arei) ; Herbert Xu > > > > ; Jason Wang ; > > > > virtualizat...@lists.linux-foundation.org; > > > > linux-ker...@vger.kernel.org; linux-crypto@vger.kernel.org; Marc > > > > Hartmayer > > > > Subject: Re: virtcrypto_dataq_callback calls > > > > crypto_finalize_request() from irq context > > > > > > > > On Sun, Sep 24, 2023 at 07:39:41PM +0200, Halil Pasic wrote: > > > > > On Sun, 24 Sep 2023 11:56:25 + "Gonglei (Arei)" > > > > > wrote: > > > > > > > > > > > Hi Halil, > > > > > > > > > > > > Commit 4058cf08945 introduced a check for detecting crypto > > > > > > completion function called with enable BH, and indeed the > > > > > > virtio-crypto driver didn't disable BH, which needs a patch to fix > > > > > > it. > > > > > > > > > > > > P.S.: > > > > > > https://lore.kernel.org/lkml/20220221120833.2618733-5-clabbe@bay > > > > > > libr > > > > > > e.com/T/ > > > > > > > > > > > > Regards, > > > > > > -Gonglei > > > > > > > > > > Thanks Gonglei! > > > > > > > > > > Thanks! I would be glad to test that fix on s390x. Are you about > > > > > to send one? > > > > > > > > > > Regards, > > > > > Halil > > > > > > > > > > > > Gonglei did you intend to send a fix? > > > > > > Actually I sent a patch a month ago, pls see another thread. > > > > > > > > > Regards, > > > -Gonglei > > > > And I think there was an issue with that patch that you wanted to fix? > > config changed callback got fixed but this still didn't. > > > Now my concern is whether or not the judgement (commit 4058cf08945c1) is > reasonable. > > Regards, > -Gonglei So what is the plan to deal with the issue? -- MST
Re: [PATCH] crypto: virtio/akcipher - Fix stack overflow on memcpy
On Tue, Jan 30, 2024 at 07:27:40PM +0800, zhenwei pi wrote:
> sizeof(struct virtio_crypto_akcipher_session_para) is less than
> sizeof(struct virtio_crypto_op_ctrl_req::u), copying more bytes from
> stack variable leads stack overflow. Clang reports this issue by
> commands:
> make -j CC=clang-14 mrproper >/dev/null 2>&1
> make -j O=/tmp/crypto-build CC=clang-14 allmodconfig >/dev/null 2>&1
> make -j O=/tmp/crypto-build W=1 CC=clang-14 drivers/crypto/virtio/
> virtio_crypto_akcipher_algs.o
>
> Fixes: 59ca6c93387d ("virtio-crypto: implement RSA algorithm")
> Link:
> https://lore.kernel.org/all/0a194a79-e3a3-45e7-be98-83abd3e1c...@roeck-us.net/
> Signed-off-by: zhenwei pi
Cc: sta...@vger.kernel.org
Acked-by: Michael S. Tsirkin
> ---
> drivers/crypto/virtio/virtio_crypto_akcipher_algs.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> index 2621ff8a9376..de53eddf6796 100644
> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
> @@ -104,7 +104,8 @@ static void virtio_crypto_dataq_akcipher_callback(struct
> virtio_crypto_request *
> }
>
> static int virtio_crypto_alg_akcipher_init_session(struct
> virtio_crypto_akcipher_ctx *ctx,
> - struct virtio_crypto_ctrl_header *header, void *para,
> + struct virtio_crypto_ctrl_header *header,
> + struct virtio_crypto_akcipher_session_para *para,
> const uint8_t *key, unsigned int keylen)
> {
> struct scatterlist outhdr_sg, key_sg, inhdr_sg, *sgs[3];
> @@ -128,7 +129,7 @@ static int virtio_crypto_alg_akcipher_init_session(struct
> virtio_crypto_akcipher
>
> ctrl = &vc_ctrl_req->ctrl;
> memcpy(&ctrl->header, header, sizeof(ctrl->header));
> - memcpy(&ctrl->u, para, sizeof(ctrl->u));
> + memcpy(&ctrl->u.akcipher_create_session.para, para, sizeof(*para));
> input = &vc_ctrl_req->input;
> input->status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
>
> --
> 2.34.1
Re: [PATCH 5.10 000/286] 5.10.209-rc1 review
On Fri, Jan 26, 2024 at 03:55:02PM -0800, Guenter Roeck wrote:
> On 1/26/24 14:35, Nathan Chancellor wrote:
> > (slimming up the CC list, I don't think this is too relevant to the
> > wider stable community)
> >
> > On Fri, Jan 26, 2024 at 01:01:15PM -0800, Guenter Roeck wrote:
> > > On 1/26/24 12:34, Nathan Chancellor wrote:
> > > > On Fri, Jan 26, 2024 at 10:17:23AM -0800, Guenter Roeck wrote:
> > > > > On 1/26/24 09:51, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Jan 26, 2024 at 08:46:42AM -0800, Guenter Roeck wrote:
> > > > > > > On 1/22/24 15:55, Greg Kroah-Hartman wrote:
> > > > > > > > This is the start of the stable review cycle for the 5.10.209
> > > > > > > > release.
> > > > > > > > There are 286 patches in this series, all will be posted as a
> > > > > > > > response
> > > > > > > > to this one. If anyone has any issues with these being
> > > > > > > > applied, please
> > > > > > > > let me know.
> > > > > > > >
> > > > > > > > Responses should be made by Wed, 24 Jan 2024 23:56:49 +.
> > > > > > > > Anything received after that time might be too late.
> > > > > > > >
> > > > > > > [ ... ]
> > > > > > >
> > > > > > > > zhenwei pi
> > > > > > > > virtio-crypto: implement RSA algorithm
> > > > > > > >
> > > > > > >
> > > > > > > Curious: Why was this (and its subsequent fixes) backported to
> > > > > > > v5.10.y ?
> > > > > > > It is quite beyond a bug fix. Also, unless I am really missing
> > > > > > > something,
> > > > > > > the series (or at least this patch) was not applied to v5.15.y,
> > > > > > > so we now
> > > > > > > have functionality in v5.10.y which is not in v5.15.y.
> > > > > >
> > > > > > See the commit text, it was a dependency of a later fix and
> > > > > > documented
> > > > > > as such.
> > > > > >
> > > > > > Having it in 5.10 and not 5.15 is a bit odd, I agree, so patches are
> > > > > > gladly accepted :)
> > > > > >
> > > > >
> > > > > We reverted the entire series from the merge because it results in a
> > > > > build
> > > > > failure for us.
> > > > >
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:10:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/mpi.h:21:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/scatterlist.h:5:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/string.h:293:
> > > > > /home/groeck/src/linux-chromeos/include/linux/fortify-string.h:512:4:
> > > > > error: call to __read_overflow2_field declared with 'warning'
> > > > > attribute: detected read beyond size of field (2nd parameter); maybe
> > > > > use struct_group()? [-Werror,-Wattribute-warning]
> > > > > __read_overflow2_field(q_size_field, size);
> > > >
> > > > For what it's worth, this is likely self inflicted for chromeos-5.10,
> > > > which carries a revert of commit eaafc590053b ("fortify: Explicitly
> > > > disable Clang support") as commit c19861d34c003 ("CHROMIUM: Revert
> > > > "fortify: Explicitly disable Clang support""). I don't see the series
> > > > that added proper support for clang to fortify in 5.18 that ended with
> > > > commit 281d0c962752 ("fortify: Add Clang support") in that ChromeOS
> > > > branch, so this seems somewhat expected.
> > > >
> > >
> > > That explains that ;-). I don't mind if the patches stay in v5.10.y,
> > > we have them reverted anyway.
> > >
> > > The revert was a pure process issue, as you may see when looking into
> > > commit c19861d34c003, so, yes, I agree that it is self-inflicted damage.
> > > Still, that doesn't explain why the problem exists in 5.18+.
> > >
> > > > > I also see that upstream (starting with 6.1) when trying to build it
> > > > > with clang,
> > > > > so I guess it is one of those bug-for-bug compatibility things. I
> > > > > really have
> > > > > no idea what causes it, or why we don't see the problem when building
> > > > > chromeos-6.1 or chromeos-6.6, but (so far) only with chromeos-5.10
> > > > > after
> > > > > merging 5.10.209 into it. Making things worse, the problem isn't
> > > > > _always_
> > > > > seen. Sometimes I can compile the file in 6.1.y without error,
> > > > > sometimes not.
> > > > > I have no idea what triggers the problem.
> > > >
> > > > Have a .config that reproduces it on upstream? I have not personally
> > > > seen this warning in my build matrix nor has our continuous-integration
> > > > matrix (I don't see it in the warning output at the bottom but that
> > > > could have missed something for some reason) in 6.1:
> > > >
> > >
> > > The following command sequence reproduces the problem for me with all
> > > stable
> > > branches starting with 5.18.y (plus mainline).
> > >
> > > rm -rf /tmp/crypto-build
> > > mkdir /tmp/crypto-build
> > > make -j CC=clang-15 mrproper >/dev/null 2>&1
> > > make -j O=/tmp/crypto-build CC=clang-
Re: [PATCH 5.10 000/286] 5.10.209-rc1 review
On Fri, Jan 26, 2024 at 03:55:02PM -0800, Guenter Roeck wrote:
> On 1/26/24 14:35, Nathan Chancellor wrote:
> > (slimming up the CC list, I don't think this is too relevant to the
> > wider stable community)
> >
> > On Fri, Jan 26, 2024 at 01:01:15PM -0800, Guenter Roeck wrote:
> > > On 1/26/24 12:34, Nathan Chancellor wrote:
> > > > On Fri, Jan 26, 2024 at 10:17:23AM -0800, Guenter Roeck wrote:
> > > > > On 1/26/24 09:51, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Jan 26, 2024 at 08:46:42AM -0800, Guenter Roeck wrote:
> > > > > > > On 1/22/24 15:55, Greg Kroah-Hartman wrote:
> > > > > > > > This is the start of the stable review cycle for the 5.10.209
> > > > > > > > release.
> > > > > > > > There are 286 patches in this series, all will be posted as a
> > > > > > > > response
> > > > > > > > to this one. If anyone has any issues with these being
> > > > > > > > applied, please
> > > > > > > > let me know.
> > > > > > > >
> > > > > > > > Responses should be made by Wed, 24 Jan 2024 23:56:49 +.
> > > > > > > > Anything received after that time might be too late.
> > > > > > > >
> > > > > > > [ ... ]
> > > > > > >
> > > > > > > > zhenwei pi
> > > > > > > > virtio-crypto: implement RSA algorithm
> > > > > > > >
> > > > > > >
> > > > > > > Curious: Why was this (and its subsequent fixes) backported to
> > > > > > > v5.10.y ?
> > > > > > > It is quite beyond a bug fix. Also, unless I am really missing
> > > > > > > something,
> > > > > > > the series (or at least this patch) was not applied to v5.15.y,
> > > > > > > so we now
> > > > > > > have functionality in v5.10.y which is not in v5.15.y.
> > > > > >
> > > > > > See the commit text, it was a dependency of a later fix and
> > > > > > documented
> > > > > > as such.
> > > > > >
> > > > > > Having it in 5.10 and not 5.15 is a bit odd, I agree, so patches are
> > > > > > gladly accepted :)
> > > > > >
> > > > >
> > > > > We reverted the entire series from the merge because it results in a
> > > > > build
> > > > > failure for us.
> > > > >
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c:10:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/mpi.h:21:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/scatterlist.h:5:
> > > > > In file included from
> > > > > /home/groeck/src/linux-chromeos/include/linux/string.h:293:
> > > > > /home/groeck/src/linux-chromeos/include/linux/fortify-string.h:512:4:
> > > > > error: call to __read_overflow2_field declared with 'warning'
> > > > > attribute: detected read beyond size of field (2nd parameter); maybe
> > > > > use struct_group()? [-Werror,-Wattribute-warning]
> > > > > __read_overflow2_field(q_size_field, size);
> > > >
> > > > For what it's worth, this is likely self inflicted for chromeos-5.10,
> > > > which carries a revert of commit eaafc590053b ("fortify: Explicitly
> > > > disable Clang support") as commit c19861d34c003 ("CHROMIUM: Revert
> > > > "fortify: Explicitly disable Clang support""). I don't see the series
> > > > that added proper support for clang to fortify in 5.18 that ended with
> > > > commit 281d0c962752 ("fortify: Add Clang support") in that ChromeOS
> > > > branch, so this seems somewhat expected.
> > > >
> > >
> > > That explains that ;-). I don't mind if the patches stay in v5.10.y,
> > > we have them reverted anyway.
> > >
> > > The revert was a pure process issue, as you may see when looking into
> > > commit c19861d34c003, so, yes, I agree that it is self-inflicted damage.
> > > Still, that doesn't explain why the problem exists in 5.18+.
> > >
> > > > > I also see that upstream (starting with 6.1) when trying to build it
> > > > > with clang,
> > > > > so I guess it is one of those bug-for-bug compatibility things. I
> > > > > really have
> > > > > no idea what causes it, or why we don't see the problem when building
> > > > > chromeos-6.1 or chromeos-6.6, but (so far) only with chromeos-5.10
> > > > > after
> > > > > merging 5.10.209 into it. Making things worse, the problem isn't
> > > > > _always_
> > > > > seen. Sometimes I can compile the file in 6.1.y without error,
> > > > > sometimes not.
> > > > > I have no idea what triggers the problem.
> > > >
> > > > Have a .config that reproduces it on upstream? I have not personally
> > > > seen this warning in my build matrix nor has our continuous-integration
> > > > matrix (I don't see it in the warning output at the bottom but that
> > > > could have missed something for some reason) in 6.1:
> > > >
> > >
> > > The following command sequence reproduces the problem for me with all
> > > stable
> > > branches starting with 5.18.y (plus mainline).
> > >
> > > rm -rf /tmp/crypto-build
> > > mkdir /tmp/crypto-build
> > > make -j CC=clang-15 mrproper >/dev/null 2>&1
> > > make -j O=/tmp/crypto-build CC=clang-
Re: [PATCH] crypto: virtio-crypto: Handle dataq logic with tasklet
On Mon, Nov 20, 2023 at 11:49:45AM +, Gonglei (Arei) wrote:
> Doing ipsec produces a spinlock recursion warning.
> This is due to crypto_finalize_request() being called in the upper half.
> Move virtual data queue processing of virtio-crypto driver to tasklet.
>
> Fixes: dbaf0624ffa57 ("crypto: add virtio-crypto driver")
> Reported-by: Halil Pasic
> Signed-off-by: wangyangxin
> Signed-off-by: Gonglei
> ---
> drivers/crypto/virtio/virtio_crypto_common.h | 2 ++
> drivers/crypto/virtio/virtio_crypto_core.c | 23 +--
> 2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
> b/drivers/crypto/virtio/virtio_crypto_common.h
> index 59a4c02..5c17c6e 100644
> --- a/drivers/crypto/virtio/virtio_crypto_common.h
> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -28,6 +29,7 @@ struct data_queue {
> char name[32];
>
> struct crypto_engine *engine;
> + struct tasklet_struct done_task;
> };
>
> struct virtio_crypto {
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> b/drivers/crypto/virtio/virtio_crypto_core.c
> index 1198bd3..e747f4f 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -72,27 +72,28 @@ int virtio_crypto_ctrl_vq_request(struct virtio_crypto
> *vcrypto, struct scatterl
> return 0;
> }
>
> -static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +static void virtcrypto_done_task(unsigned long data)
> {
> - struct virtio_crypto *vcrypto = vq->vdev->priv;
> + struct data_queue *data_vq = (struct data_queue *)data;
> + struct virtqueue *vq = data_vq->vq;
> struct virtio_crypto_request *vc_req;
> - unsigned long flags;
> unsigned int len;
> - unsigned int qid = vq->index;
>
> - spin_lock_irqsave(&vcrypto->data_vq[qid].lock, flags);
> do {
> virtqueue_disable_cb(vq);
> while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> - spin_unlock_irqrestore(
> - &vcrypto->data_vq[qid].lock, flags);
> if (vc_req->alg_cb)
> vc_req->alg_cb(vc_req, len);
> - spin_lock_irqsave(
> - &vcrypto->data_vq[qid].lock, flags);
> }
> } while (!virtqueue_enable_cb(vq));
> - spin_unlock_irqrestore(&vcrypto->data_vq[qid].lock, flags);
> +}
> +
> +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +{
> + struct virtio_crypto *vcrypto = vq->vdev->priv;
> + struct data_queue *dq = &vcrypto->data_vq[vq->index];
> +
> + tasklet_schedule(&dq->done_task);
> }
>
Don't we then need to wait for tasklet to complete on
device remove?
> static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> @@ -150,6 +151,8 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> ret = -ENOMEM;
> goto err_engine;
> }
> + tasklet_init(&vi->data_vq[i].done_task, virtcrypto_done_task,
> + (unsigned long)&vi->data_vq[i]);
> }
>
> kfree(names);
> --
> 1.8.3.1
>
>
[PATCH v3 32/38] virtio_crypto: convert to LE accessors
Virtio crypto is modern-only. Use LE accessors for config space. Signed-off-by: Michael S. Tsirkin --- drivers/crypto/virtio/virtio_crypto_core.c | 46 +++--- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c index c8a962c62663..aeecce27fe8f 100644 --- a/drivers/crypto/virtio/virtio_crypto_core.c +++ b/drivers/crypto/virtio/virtio_crypto_core.c @@ -204,8 +204,8 @@ static int virtcrypto_update_status(struct virtio_crypto *vcrypto) u32 status; int err; - virtio_cread(vcrypto->vdev, - struct virtio_crypto_config, status, &status); + virtio_cread_le(vcrypto->vdev, + struct virtio_crypto_config, status, &status); /* * Unknown status bits would be a host error and the driver @@ -323,31 +323,31 @@ static int virtcrypto_probe(struct virtio_device *vdev) if (!vcrypto) return -ENOMEM; - virtio_cread(vdev, struct virtio_crypto_config, + virtio_cread_le(vdev, struct virtio_crypto_config, max_dataqueues, &max_data_queues); if (max_data_queues < 1) max_data_queues = 1; - virtio_cread(vdev, struct virtio_crypto_config, - max_cipher_key_len, &max_cipher_key_len); - virtio_cread(vdev, struct virtio_crypto_config, - max_auth_key_len, &max_auth_key_len); - virtio_cread(vdev, struct virtio_crypto_config, - max_size, &max_size); - virtio_cread(vdev, struct virtio_crypto_config, - crypto_services, &crypto_services); - virtio_cread(vdev, struct virtio_crypto_config, - cipher_algo_l, &cipher_algo_l); - virtio_cread(vdev, struct virtio_crypto_config, - cipher_algo_h, &cipher_algo_h); - virtio_cread(vdev, struct virtio_crypto_config, - hash_algo, &hash_algo); - virtio_cread(vdev, struct virtio_crypto_config, - mac_algo_l, &mac_algo_l); - virtio_cread(vdev, struct virtio_crypto_config, - mac_algo_h, &mac_algo_h); - virtio_cread(vdev, struct virtio_crypto_config, - aead_algo, &aead_algo); + virtio_cread_le(vdev, struct virtio_crypto_config, + max_cipher_key_len, &max_cipher_key_len); + virtio_cread_le(vdev, struct virtio_crypto_config, + max_auth_key_len, &max_auth_key_len); + virtio_cread_le(vdev, struct virtio_crypto_config, + max_size, &max_size); + virtio_cread_le(vdev, struct virtio_crypto_config, + crypto_services, &crypto_services); + virtio_cread_le(vdev, struct virtio_crypto_config, + cipher_algo_l, &cipher_algo_l); + virtio_cread_le(vdev, struct virtio_crypto_config, + cipher_algo_h, &cipher_algo_h); + virtio_cread_le(vdev, struct virtio_crypto_config, + hash_algo, &hash_algo); + virtio_cread_le(vdev, struct virtio_crypto_config, + mac_algo_l, &mac_algo_l); + virtio_cread_le(vdev, struct virtio_crypto_config, + mac_algo_h, &mac_algo_h); + virtio_cread_le(vdev, struct virtio_crypto_config, + aead_algo, &aead_algo); /* Add virtio crypto device to global table */ err = virtcrypto_devmgr_add_dev(vcrypto); -- MST
[PATCH v3 08/38] virtio_crypto: correct tags for config space fields
Since crypto is a modern-only device,
tag config space fields as having little endian-ness.
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Cornelia Huck
---
include/uapi/linux/virtio_crypto.h | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/virtio_crypto.h
b/include/uapi/linux/virtio_crypto.h
index 50cdc8aebfcf..a03932f10565 100644
--- a/include/uapi/linux/virtio_crypto.h
+++ b/include/uapi/linux/virtio_crypto.h
@@ -414,33 +414,33 @@ struct virtio_crypto_op_data_req {
struct virtio_crypto_config {
/* See VIRTIO_CRYPTO_OP_* above */
- __u32 status;
+ __le32 status;
/*
* Maximum number of data queue
*/
- __u32 max_dataqueues;
+ __le32 max_dataqueues;
/*
* Specifies the services mask which the device support,
* see VIRTIO_CRYPTO_SERVICE_* above
*/
- __u32 crypto_services;
+ __le32 crypto_services;
/* Detailed algorithms mask */
- __u32 cipher_algo_l;
- __u32 cipher_algo_h;
- __u32 hash_algo;
- __u32 mac_algo_l;
- __u32 mac_algo_h;
- __u32 aead_algo;
+ __le32 cipher_algo_l;
+ __le32 cipher_algo_h;
+ __le32 hash_algo;
+ __le32 mac_algo_l;
+ __le32 mac_algo_h;
+ __le32 aead_algo;
/* Maximum length of cipher key */
- __u32 max_cipher_key_len;
+ __le32 max_cipher_key_len;
/* Maximum length of authenticated key */
- __u32 max_auth_key_len;
- __u32 reserve;
+ __le32 max_auth_key_len;
+ __le32 reserve;
/* Maximum size of each crypto request's content */
- __u64 max_size;
+ __le64 max_size;
};
struct virtio_crypto_inhdr {
--
MST
[PATCH v2 08/24] virtio_crypto: correct tags for config space fields
Since crypto is a modern-only device,
tag config space fields as having little endian-ness.
Signed-off-by: Michael S. Tsirkin
---
include/uapi/linux/virtio_crypto.h | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/include/uapi/linux/virtio_crypto.h
b/include/uapi/linux/virtio_crypto.h
index 50cdc8aebfcf..a03932f10565 100644
--- a/include/uapi/linux/virtio_crypto.h
+++ b/include/uapi/linux/virtio_crypto.h
@@ -414,33 +414,33 @@ struct virtio_crypto_op_data_req {
struct virtio_crypto_config {
/* See VIRTIO_CRYPTO_OP_* above */
- __u32 status;
+ __le32 status;
/*
* Maximum number of data queue
*/
- __u32 max_dataqueues;
+ __le32 max_dataqueues;
/*
* Specifies the services mask which the device support,
* see VIRTIO_CRYPTO_SERVICE_* above
*/
- __u32 crypto_services;
+ __le32 crypto_services;
/* Detailed algorithms mask */
- __u32 cipher_algo_l;
- __u32 cipher_algo_h;
- __u32 hash_algo;
- __u32 mac_algo_l;
- __u32 mac_algo_h;
- __u32 aead_algo;
+ __le32 cipher_algo_l;
+ __le32 cipher_algo_h;
+ __le32 hash_algo;
+ __le32 mac_algo_l;
+ __le32 mac_algo_h;
+ __le32 aead_algo;
/* Maximum length of cipher key */
- __u32 max_cipher_key_len;
+ __le32 max_cipher_key_len;
/* Maximum length of authenticated key */
- __u32 max_auth_key_len;
- __u32 reserve;
+ __le32 max_auth_key_len;
+ __le32 reserve;
/* Maximum size of each crypto request's content */
- __u64 max_size;
+ __le64 max_size;
};
struct virtio_crypto_inhdr {
--
MST
Re: [PATCH v2 2/2] crypto: virtio: Fix use-after-free in virtio_crypto_skcipher_finalize_req()
On Tue, May 26, 2020 at 02:11:37PM +, Sasha Levin wrote:
> <20200123101000.GB24255@Red>
> References: <20200526031956.1897-3-longpe...@huawei.com>
> <20200123101000.GB24255@Red>
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: dbaf0624ffa5 ("crypto: add virtio-crypto driver").
>
> The bot has tested the following trees: v5.6.14, v5.4.42, v4.19.124,
> v4.14.181.
>
> v5.6.14: Build OK!
> v5.4.42: Failed to apply! Possible dependencies:
> eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")
>
> v4.19.124: Failed to apply! Possible dependencies:
> eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")
>
> v4.14.181: Failed to apply! Possible dependencies:
> 500e6807ce93 ("crypto: virtio - implement missing support for output IVs")
> 67189375bb3a ("crypto: virtio - convert to new crypto engine API")
> d0d859bb87ac ("crypto: virtio - Register an algo only if it's supported")
> e02b8b43f55a ("crypto: virtio - pr_err() strings should end with
> newlines")
> eee1d6fca0a0 ("crypto: virtio - switch to skcipher API")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
Mike could you comment on backporting?
> --
> Thanks
> Sasha
Re: [PATCH v2 06/35] crypto: Use kmemdup rather than duplicating its implementation
On Thu, Jul 04, 2019 at 12:27:08AM +0800, Fuqian Huang wrote: > kmemdup is introduced to duplicate a region of memory in a neat way. > Rather than kmalloc/kzalloc + memcpy, which the programmer needs to > write the size twice (sometimes lead to mistakes), kmemdup improves > readability, leads to smaller code and also reduce the chances of mistakes. > Suggestion to use kmemdup rather than using kmalloc/kzalloc + memcpy. > > Signed-off-by: Fuqian Huang virtio part: Acked-by: Michael S. Tsirkin > --- > Changes in v2: > - Fix a typo in commit message (memset -> memcpy) > > drivers/crypto/caam/caampkc.c | 11 +++ > drivers/crypto/virtio/virtio_crypto_algs.c | 4 +--- > 2 files changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c > index fe24485274e1..a03464b4c019 100644 > --- a/drivers/crypto/caam/caampkc.c > +++ b/drivers/crypto/caam/caampkc.c > @@ -816,7 +816,7 @@ static int caam_rsa_set_pub_key(struct crypto_akcipher > *tfm, const void *key, > return ret; > > /* Copy key in DMA zone */ > - rsa_key->e = kzalloc(raw_key.e_sz, GFP_DMA | GFP_KERNEL); > + rsa_key->e = kmemdup(raw_key.e, raw_key.e_sz, GFP_DMA | GFP_KERNEL); > if (!rsa_key->e) > goto err; > > @@ -838,8 +838,6 @@ static int caam_rsa_set_pub_key(struct crypto_akcipher > *tfm, const void *key, > rsa_key->e_sz = raw_key.e_sz; > rsa_key->n_sz = raw_key.n_sz; > > - memcpy(rsa_key->e, raw_key.e, raw_key.e_sz); > - > return 0; > err: > caam_rsa_free_key(rsa_key); > @@ -920,11 +918,11 @@ static int caam_rsa_set_priv_key(struct crypto_akcipher > *tfm, const void *key, > return ret; > > /* Copy key in DMA zone */ > - rsa_key->d = kzalloc(raw_key.d_sz, GFP_DMA | GFP_KERNEL); > + rsa_key->d = kmemdup(raw_key.d, raw_key.d_sz, GFP_DMA | GFP_KERNEL); > if (!rsa_key->d) > goto err; > > - rsa_key->e = kzalloc(raw_key.e_sz, GFP_DMA | GFP_KERNEL); > + rsa_key->e = kmemdup(raw_key.e, raw_key.e_sz, GFP_DMA | GFP_KERNEL); > if (!rsa_key->e) > goto err; > > @@ -947,9 +945,6 @@ static int caam_rsa_set_priv_key(struct crypto_akcipher > *tfm, const void *key, > rsa_key->e_sz = raw_key.e_sz; > rsa_key->n_sz = raw_key.n_sz; > > - memcpy(rsa_key->d, raw_key.d, raw_key.d_sz); > - memcpy(rsa_key->e, raw_key.e, raw_key.e_sz); > - > caam_rsa_set_priv_key_form(ctx, &raw_key); > > return 0; > diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c > b/drivers/crypto/virtio/virtio_crypto_algs.c > index 10f266d462d6..42d19205166b 100644 > --- a/drivers/crypto/virtio/virtio_crypto_algs.c > +++ b/drivers/crypto/virtio/virtio_crypto_algs.c > @@ -129,13 +129,11 @@ static int virtio_crypto_alg_ablkcipher_init_session( >* Avoid to do DMA from the stack, switch to using >* dynamically-allocated for the key >*/ > - uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC); > + uint8_t *cipher_key = kmemdup(key, keylen, GFP_ATOMIC); > > if (!cipher_key) > return -ENOMEM; > > - memcpy(cipher_key, key, keylen); > - > spin_lock(&vcrypto->ctrl_lock); > /* Pad ctrl header */ > vcrypto->ctrl.header.opcode = > -- > 2.11.0
Re: [PATCH] char: hw_random: Fix missing check during driver release
On Wed, Dec 26, 2018 at 11:23:31AM -0600, Aditya Pakki wrote:
> devres_release can return -ENOENT if the device is not freed. The fix
> throws a warning consistent with other invocations.
>
> Signed-off-by: Aditya Pakki
Well why not
Acked-by: Michael S. Tsirkin
> ---
> drivers/char/hw_random/core.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 95be7228f327..582d983fa93f 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -578,7 +578,11 @@ EXPORT_SYMBOL_GPL(devm_hwrng_register);
>
> void devm_hwrng_unregister(struct device *dev, struct hwrng *rng)
> {
> - devres_release(dev, devm_hwrng_release, devm_hwrng_match, rng);
> + int rc;
> +
> + rc = devres_release(dev, devm_hwrng_release, devm_hwrng_match, rng);
> + if (rc)
> + WARN_ON(rc);
> }
> EXPORT_SYMBOL_GPL(devm_hwrng_unregister);
>
> --
> 2.17.1
[PATCH] hwrng: document the quality field
quality field is currently documented as being 'per mill'. In fact the
math involved is:
add_hwgenerator_randomness((void *)rng_fillbuf, rc,
rc * current_quality * 8 >> 10);
thus the actual definition is "bits of entropy per 1024 bits of input".
The current documentation seems to have confused multiple people
in the past, let's fix the documentation to match code.
An alternative is to change core to match driver expectations, replacing
rc * current_quality * 8 >> 10
with
rc * current_quality / 1000
but that has performance costs, so probably isn't a good option.
Fixes: 0f734e6e768 ("hwrng: add per-device entropy derating")
Reported-by: "Dr. David Alan Gilbert"
Signed-off-by: Michael S. Tsirkin
---
drivers/char/hw_random/core.c | 4 ++--
include/linux/hw_random.h | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index aaf9e5afaad4..95be7228f327 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -44,10 +44,10 @@ static unsigned short default_quality; /* = 0; default to
"off" */
module_param(current_quality, ushort, 0644);
MODULE_PARM_DESC(current_quality,
-"current hwrng entropy estimation per mill");
+"current hwrng entropy estimation per 1024 bits of input");
module_param(default_quality, ushort, 0644);
MODULE_PARM_DESC(default_quality,
-"default entropy content of hwrng per mill");
+"default entropy content of hwrng per 1024 bits of input");
static void drop_current_rng(void);
static int hwrng_init(struct hwrng *rng);
diff --git a/include/linux/hw_random.h b/include/linux/hw_random.h
index bee0827766a3..c0b93e0ff0c0 100644
--- a/include/linux/hw_random.h
+++ b/include/linux/hw_random.h
@@ -33,7 +33,8 @@
* and max is a multiple of 4 and >= 32 bytes.
* @priv: Private data, for use by the RNG driver.
* @quality: Estimation of true entropy in RNG's bitstream
- * (per mill).
+ * (in bits of entropy per 1024 bits of input;
+ * valid values: 1 to 1024, or 0 for unknown).
*/
struct hwrng {
const char *name;
--
MST
Re: [PATCH v2 net-next 0/2] virtio_net: Expand affinity to arbitrary numbers of cpu and vq
On Thu, Aug 09, 2018 at 06:18:27PM -0700, Caleb Raitto wrote: > From: Caleb Raitto > > Virtio-net tries to pin each virtual queue rx and tx interrupt to a cpu if > there are as many queues as cpus. > > Expand this heuristic to configure a reasonable affinity setting also > when the number of cpus != the number of virtual queues. > > Patch 1 allows vqs to take an affinity mask with more than 1 cpu. > Patch 2 generalizes the algorithm in virtnet_set_affinity beyond > the case where #cpus == #vqs. Acked-by: Michael S. Tsirkin Let's see where does it take us. > v2 changes: > Renamed "virtio_net: Make vp_set_vq_affinity() take a mask." to > "virtio: Make vp_set_vq_affinity() take a mask." > > Tested: > > # 16 vCPU, 16 queue pairs, Debian 9 recent net-next kernel, GCE > > # Disable GCE scripts setting affinities during startup. > # > # Add the following to > # /etc/default/instance_configs.cfg.template and reboot: > [InstanceSetup] > set_multiqueue = false > > $ cd /proc/irq > $ for i in `seq 24 60` ; do sudo grep ".*" $i/smp_affinity_list; done > 0-15 > 0 > 0 > 1 > 1 > 2 > 2 > 3 > 3 > 4 > 4 > 5 > 5 > 6 > 6 > 7 > 7 > 8 > 8 > 9 > 9 > 10 > 10 > 11 > 11 > 12 > 12 > 13 > 13 > 14 > 14 > 15 > 15 > 0-15 > 0-15 > 0-15 > 0-15 > > $ cd /sys/class/net/eth0/queues/ > $ for i in `seq 0 15` ; do sudo grep ".*" tx-$i/xps_cpus; done > 0001 > 0002 > 0004 > 0008 > 0010 > 0020 > 0040 > 0080 > 0100 > 0200 > 0400 > 0800 > 1000 > 2000 > 4000 > 8000 > > # 16 vCPU, 15 queue pairs > $ sudo ethtool -L eth0 combined 15 > > $ cd /proc/irq > $ for i in `seq 24 60` ; do sudo grep ".*" $i/smp_affinity_list; done > 0-15 > 0-1 > 0-1 > 2 > 2 > 3 > 3 > 4 > 4 > 5 > 5 > 6 > 6 > 7 > 7 > 8 > 8 > 9 > 9 > 10 > 10 > 11 > 11 > 12 > 12 > 13 > 13 > 14 > 14 > 15 > 15 > 15 > 15 > 0-15 > 0-15 > 0-15 > 0-15 > > $ cd /sys/class/net/eth0/queues/ > $ for i in `seq 0 14` ; do sudo grep ".*" tx-$i/xps_cpus; done > 0003 > 0004 > 0008 > 0010 > 0020 > 0040 > 0080 > 0100 > 0200 > 0400 > 0800 > 1000 > 2000 > 4000 > 8000 > > # 16 vCPU, 8 queue pairs > $ sudo ethtool -L eth0 combined 8 > > $ cd /proc/irq > $ for i in `seq 24 60` ; do sudo grep ".*" $i/smp_affinity_list; done > 0-15 > 0-1 > 0-1 > 2-3 > 2-3 > 4-5 > 4-5 > 6-7 > 6-7 > 8-9 > 8-9 > 10-11 > 10-11 > 12-13 > 12-13 > 14-15 > 14-15 > 9 > 9 > 10 > 10 > 11 > 11 > 12 > 12 > 13 > 13 > 14 > 14 > 15 > 15 > 15 > 15 > 0-15 > 0-15 > 0-15 > 0-15 > > $ cd /sys/class/net/eth0/queues/ > $ for i in `seq 0 7` ; do sudo grep ".*" tx-$i/xps_cpus; done > 0003 > 000c > 0030 > 00c0 > 0300 > 0c00 > 3000 > c000 > > # 15 vCPU, 16 queue pairs > $ sudo ethtool -L eth0 combined 16 > $ sudo sh -c "echo 0 > /sys/devices/system/cpu/cpu15/online" > > $ cd /proc/irq > $ for i in `seq 24 60` ; do sudo grep ".*" $i/smp_affinity_list; done > 0-15 > 0 > 0 > 1 > 1 > 2 > 2 > 3 > 3 > 4 > 4 > 5 > 5 > 6 > 6 > 7 > 7 > 8 > 8 > 9 > 9 > 10 > 10 > 11 > 11 > 12 > 12 > 13 > 13 > 14 > 14 > 0 > 0 > 0-15 > 0-15 > 0-15 > 0-15 > > $ cd /sys/class/net/eth0/queues/ > $ for i in `seq 0 15` ; do sudo grep ".*" tx-$i/xps_cpus; done > 0001 > 0002 > 0004 > 0008 > 0010 > 0020 > 0040 > 0080 > 0100 > 0200 > 0400 > 0800 > 1000 > 2000 > 4000 > 0001 > > # 8 vCPU, 16 queue pairs > $ for i in `seq 8 15`; \ > do sudo sh -c "echo 0 > /sys/devices/system/cpu/cpu$i/online"; done > > $ cd /proc/irq > $ for i in `seq 24 60` ; do sudo grep ".*" $i/smp_affinity_list; done > 0-15 > 0 > 0 > 1 > 1 > 2 > 2 > 3 > 3 > 4 > 4 > 5 > 5 > 6 > 6 > 7 > 7 > 0 > 0 > 1 > 1 > 2 > 2 > 3 > 3 > 4 > 4 > 5 > 5 > 6 > 6 > 7 > 7 > 0-15 > 0-15 > 0-15 > 0-15 > > $ cd /sys/class/net/eth0/queues/ > $ for i in `seq 0 15` ; do sudo grep ".*" tx-$i/xps_cpus; done > 0001 > 0002 > 0004 > 0008 > 0010 > 0020 > 0040 > 0080 > 0001 > 0002 > 0004 > 0008 > 0010 > 0020 > 0040 > 0080 > > Caleb Raitto (2): > virtio: Make vp_set_vq_affinity() take a mask. > virtio_net: Stripe queue affinities across cores. > > drivers/crypto/virtio/virtio_crypto_core.c | 4 +- > drivers/net/virtio_net.c | 46 ++ > drivers/virtio/virtio_pci_common.c | 7 ++-- > drivers/virtio/virtio_pci_common.h | 2 +- > include/linux/virtio_config.h | 7 ++-- > 5 files changed, 39 insertions(+), 27 deletions(-) > > -- > 2.18.0.597.ga71716f1ad-goog
Re: [PATCH v2 4/6] crypto: virtio: convert to new crypto engine API
On Fri, Jan 26, 2018 at 08:15:32PM +0100, Corentin Labbe wrote:
> This patch convert the driver to the new crypto engine API.
>
> Signed-off-by: Corentin Labbe
Acked-by: Michael S. Tsirkin
Pls queue when/if rest of changes go in.
> ---
> drivers/crypto/virtio/virtio_crypto_algs.c | 16 ++--
> drivers/crypto/virtio/virtio_crypto_common.h | 3 +--
> drivers/crypto/virtio/virtio_crypto_core.c | 3 ---
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> b/drivers/crypto/virtio/virtio_crypto_algs.c
> index abe8c15450df..ba190cfa7aa1 100644
> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> @@ -29,6 +29,7 @@
>
>
> struct virtio_crypto_ablkcipher_ctx {
> + struct crypto_engine_ctx enginectx;
> struct virtio_crypto *vcrypto;
> struct crypto_tfm *tfm;
>
> @@ -491,7 +492,7 @@ static int virtio_crypto_ablkcipher_encrypt(struct
> ablkcipher_request *req)
> vc_sym_req->ablkcipher_req = req;
> vc_sym_req->encrypt = true;
>
> - return crypto_transfer_cipher_request_to_engine(data_vq->engine, req);
> + return crypto_transfer_ablkcipher_request_to_engine(data_vq->engine,
> req);
> }
>
> static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
> @@ -511,7 +512,7 @@ static int virtio_crypto_ablkcipher_decrypt(struct
> ablkcipher_request *req)
> vc_sym_req->ablkcipher_req = req;
> vc_sym_req->encrypt = false;
>
> - return crypto_transfer_cipher_request_to_engine(data_vq->engine, req);
> + return crypto_transfer_ablkcipher_request_to_engine(data_vq->engine,
> req);
> }
>
> static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm)
> @@ -521,6 +522,9 @@ static int virtio_crypto_ablkcipher_init(struct
> crypto_tfm *tfm)
> tfm->crt_ablkcipher.reqsize = sizeof(struct virtio_crypto_sym_request);
> ctx->tfm = tfm;
>
> + ctx->enginectx.op.do_one_request = virtio_crypto_ablkcipher_crypt_req;
> + ctx->enginectx.op.prepare_request = NULL;
> + ctx->enginectx.op.unprepare_request = NULL;
> return 0;
> }
>
> @@ -538,9 +542,9 @@ static void virtio_crypto_ablkcipher_exit(struct
> crypto_tfm *tfm)
> }
>
> int virtio_crypto_ablkcipher_crypt_req(
> - struct crypto_engine *engine,
> - struct ablkcipher_request *req)
> + struct crypto_engine *engine, void *vreq)
> {
> + struct ablkcipher_request *req = container_of(vreq, struct
> ablkcipher_request, base);
> struct virtio_crypto_sym_request *vc_sym_req =
> ablkcipher_request_ctx(req);
> struct virtio_crypto_request *vc_req = &vc_sym_req->base;
> @@ -561,8 +565,8 @@ static void virtio_crypto_ablkcipher_finalize_req(
> struct ablkcipher_request *req,
> int err)
> {
> - crypto_finalize_cipher_request(vc_sym_req->base.dataq->engine,
> - req, err);
> + crypto_finalize_ablkcipher_request(vc_sym_req->base.dataq->engine,
> +req, err);
> kzfree(vc_sym_req->iv);
> virtcrypto_clear_request(&vc_sym_req->base);
> }
> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
> b/drivers/crypto/virtio/virtio_crypto_common.h
> index e976539a05d9..72621bd67211 100644
> --- a/drivers/crypto/virtio/virtio_crypto_common.h
> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> @@ -107,8 +107,7 @@ struct virtio_crypto *virtcrypto_get_dev_node(int node);
> int virtcrypto_dev_start(struct virtio_crypto *vcrypto);
> void virtcrypto_dev_stop(struct virtio_crypto *vcrypto);
> int virtio_crypto_ablkcipher_crypt_req(
> - struct crypto_engine *engine,
> - struct ablkcipher_request *req);
> + struct crypto_engine *engine, void *vreq);
>
> void
> virtcrypto_clear_request(struct virtio_crypto_request *vc_req);
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> b/drivers/crypto/virtio/virtio_crypto_core.c
> index ff1410a32c2b..83326986c113 100644
> --- a/drivers/crypto/virtio/virtio_crypto_core.c
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -111,9 +111,6 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> ret = -ENOMEM;
> goto err_engine;
> }
> -
> - vi->data_vq[i].engine->cipher_one_request =
> - virtio_crypto_ablkcipher_crypt_req;
> }
>
> kfree(names);
> --
> 2.13.6
Re: [PATCH 1/6] virtio: wrap find_vqs
On Fri, Mar 31, 2017 at 12:04:55PM +0800, Jason Wang wrote: > > > On 2017年03月30日 22:32, Michael S. Tsirkin wrote: > > On Thu, Mar 30, 2017 at 02:00:08PM +0800, Jason Wang wrote: > > > > > > On 2017年03月30日 04:48, Michael S. Tsirkin wrote: > > > > We are going to add more parameters to find_vqs, let's wrap the call so > > > > we don't need to tweak all drivers every time. > > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > --- > > > A quick glance and it looks ok, but what the benefit of this series, is it > > > required by other changes? > > > > > > Thanks > > Yes - to avoid touching all devices when doing the rest of > > the patchset. > > Maybe I'm not clear. I mean the benefit of this series not this single > patch. I guess it may be used by you proposal that avoid reset when set XDP? In particular, yes. It generally simplifies things significantly if we can get the true buffer size back. > If yes, do we really want to drop some packets after XDP is set? > > Thanks We would rather not drop packets. We could detect and copy them to make XDP work. -- MST
Re: [PATCH 1/6] virtio: wrap find_vqs
On Thu, Mar 30, 2017 at 02:00:08PM +0800, Jason Wang wrote: > > > On 2017年03月30日 04:48, Michael S. Tsirkin wrote: > > We are going to add more parameters to find_vqs, let's wrap the call so > > we don't need to tweak all drivers every time. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > A quick glance and it looks ok, but what the benefit of this series, is it > required by other changes? > > Thanks Yes - to avoid touching all devices when doing the rest of the patchset.
[PATCH 1/6] virtio: wrap find_vqs
We are going to add more parameters to find_vqs, let's wrap the call so
we don't need to tweak all drivers every time.
Signed-off-by: Michael S. Tsirkin
---
drivers/block/virtio_blk.c | 3 +--
drivers/char/virtio_console.c | 6 +++---
drivers/crypto/virtio/virtio_crypto_core.c | 3 +--
drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +--
drivers/net/caif/caif_virtio.c | 3 +--
drivers/net/virtio_net.c | 3 +--
drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
drivers/scsi/virtio_scsi.c | 3 +--
drivers/virtio/virtio_balloon.c| 3 +--
drivers/virtio/virtio_input.c | 3 +--
include/linux/virtio_config.h | 9 +
net/vmw_vsock/virtio_transport.c | 6 +++---
12 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 1d4c9f8..c08c30c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -455,8 +455,7 @@ static int init_vq(struct virtio_blk *vblk)
}
/* Discover virtqueues and write information to configuration. */
- err = vdev->config->find_vqs(vdev, num_vqs, vqs, callbacks, names,
- &desc);
+ err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, &desc);
if (err)
goto out;
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e9b7e0b..5da4c8e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1945,9 +1945,9 @@ static int init_vqs(struct ports_device *portdev)
}
}
/* Find the queues. */
- err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
- io_callbacks,
- (const char **)io_names, NULL);
+ err = virtio_find_vqs(portdev->vdev, nr_queues, vqs,
+ io_callbacks,
+ (const char **)io_names, NULL);
if (err)
goto free;
diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
b/drivers/crypto/virtio/virtio_crypto_core.c
index 21472e4..a111cd72 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -119,8 +119,7 @@ static int virtcrypto_find_vqs(struct virtio_crypto *vi)
names[i] = vi->data_vq[i].name;
}
- ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
-names, NULL);
+ ret = virtio_find_vqs(vi->vdev, total_vqs, vqs, callbacks, names, NULL);
if (ret)
goto err_find;
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 4918668..1e1c90b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -175,8 +175,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned
long flags)
DRM_INFO("virgl 3d acceleration not supported by guest\n");
#endif
- ret = vgdev->vdev->config->find_vqs(vgdev->vdev, 2, vqs,
- callbacks, names, NULL);
+ ret = virtio_find_vqs(vgdev->vdev, 2, vqs, callbacks, names, NULL);
if (ret) {
DRM_ERROR("failed to find virt queues\n");
goto err_vqs;
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
index bc0eb47..6122768 100644
--- a/drivers/net/caif/caif_virtio.c
+++ b/drivers/net/caif/caif_virtio.c
@@ -679,8 +679,7 @@ static int cfv_probe(struct virtio_device *vdev)
goto err;
/* Get the TX virtio ring. This is a "guest side vring". */
- err = vdev->config->find_vqs(vdev, 1, &cfv->vq_tx, &vq_cbs, &names,
- NULL);
+ err = virtio_find_vqs(vdev, 1, &cfv->vq_tx, &vq_cbs, &names, NULL);
if (err)
goto err;
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ea9890d..6802169 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2079,8 +2079,7 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
names[txq2vq(i)] = vi->sq[i].name;
}
- ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
-names, NULL);
+ ret = virtio_find_vqs(vi->vdev, total_vqs, vqs, callbacks, names, NULL);
if (ret)
goto err_find;
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 5e66e08..f7cade0 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -869,7 +869,7 @@ static int rpmsg_probe(struct virtio_device *
Re: [PATCH v8 1/1] crypto: add virtio-crypto driver
On Thu, Jan 12, 2017 at 03:10:25PM +0100, Christian Borntraeger wrote:
> On 01/10/2017 01:56 PM, Christian Borntraeger wrote:
> > On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
> >> Hi,
> >>
> >>>
> >>> On 12/15/2016 03:03 AM, Gonglei wrote:
> >>> [...]
> +
> +static struct crypto_alg virtio_crypto_algs[] = { {
> +.cra_name = "cbc(aes)",
> +.cra_driver_name = "virtio_crypto_aes_cbc",
> +.cra_priority = 501,
> >>>
> >>>
> >>> This is still higher than the hardware-accelerators (like intel aesni or
> >>> the
> >>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported
> >>> by the
> >>> hardware virtualization and available to the guests. I do not see a way
> >>> how
> >>> virtio
> >>> crypto can be faster than that (in the end it might be cpacf/aesni +
> >>> overhead)
> >>> instead it will very likely be slower.
> >>> So we should use a number that is higher than software implementations but
> >>> lower than the hw ones.
> >>>
> >>> Just grepping around, the software ones seem be be around 100 and the
> >>> hardware
> >>> ones around 200-400. So why was 150 not enough?
> >>>
> >> I didn't find a documentation about how we use the priority, and I assumed
> >> people use virtio-crypto will configure hardware accelerators in the
> >> host. So I choosed the number which bigger than aesni's priority.
> >
> > Yes, but the aesni driver will only bind if there is HW support in the
> > guest.
> > And if aesni is available in the guest (or the s390 aes function from cpacf)
> > it will always be faster than the same in the host via virtio.So your
> > priority
> > should be smaller.
>
>
> any opinion on this?
Going forward, we might add an emulated aesni device and that might
become slower than virtio. OTOH if or when this happens, we can solve it
by adding a priority or a feature flag to virtio to raise its priority.
So I think I agree with Christian here, let's lower the priority.
Gonglei, could you send a patch like this?
--
MST
--
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: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
On Thu, Dec 15, 2016 at 01:08:51AM +, Gonglei (Arei) wrote:
>
>
>
>
> Regards,
> -Gonglei
>
>
> > -Original Message-
> > From: Zeng, Xin [mailto:xin.z...@intel.com]
> > Sent: Thursday, December 15, 2016 8:59 AM
> > To: Gonglei (Arei); Halil Pasic; linux-ker...@vger.kernel.org;
> > qemu-de...@nongnu.org; virtio-...@lists.oasis-open.org;
> > virtualizat...@lists.linux-foundation.org; linux-crypto@vger.kernel.org
> > Cc: Huangweidong (C); Claudio Fontana; m...@redhat.com; Luonengjun;
> > Hanweidong (Randy); Xuquan (Quan Xu); Wanzongshun (Vincent);
> > stefa...@redhat.com; Zhoujian (jay, Euler); cornelia.h...@de.ibm.com;
> > longpeng; arei.gong...@hotmail.com; da...@davemloft.net; Wubin (H);
> > herb...@gondor.apana.org.au
> > Subject: RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
> >
> > On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote:
> > < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c
> > < > b/drivers/crypto/virtio/virtio_crypto_core.c
> > < > > new file mode 100644
> > < > > index 000..c0854a1
> > < > > --- /dev/null
> > < > > +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> > < > > @@ -0,0 +1,474 @@
> > < > [..]
> > < > > +
> > < > > +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> > < > > +{
> > < > > + struct virtio_crypto *vcrypto = vq->vdev->priv;
> > < > > + struct virtio_crypto_request *vc_req;
> > < > > + unsigned long flags;
> > < > > + unsigned int len;
> > < > > + struct ablkcipher_request *ablk_req;
> > < > > + int error;
> > < > > +
> > < > > + spin_lock_irqsave(&vcrypto->lock, flags);
> > < >
> > < > Would it make sense to use a per virtqueue lock
> > < > like in virtio_blk for example instead of locking on the whole
> > < > device? OK, it seems you use only one dataqueue, so it
> > < > may not be that relevant.
> > < >
> > < Currently yes, both the backend device (cryptodev-backend-builtin)
> > < and the frontend driver use one dataqueue.
> > <
> >
> > I think it makes sense to use per virtqueue lock here though it only uses
> > one
> > queue so far,
> > but in the spec we already have multi queues support.
> >
> Yes, I agree. Will do that in V8 soon.
> Hope to catch up with Michael's pull request for 4.10.
>
> Regards,
> -Gonglei
I merged v7, this change will have to wait. Sorry.
--
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
[PATCH 0/8] enable endian checks for all sparse builds
This is just a reposting of the patch that enables endian checks, with addition of trivial patches that drop __bitwise__ and __CHECK_ENDIAN__ everywhere. I plan to include this in my pull request unless I hear otherwise. Michael S. Tsirkin (8): linux/types.h: enable endian checks for all sparse builds tools: enable endian checks for all sparse builds Documentation/sparse: drop __bitwise__ checkpatch: replace __bitwise__ with __bitwise linux: drop __bitwise__ everywhere Documentation/sparse: drop __CHECK_ENDIAN__ fs/logfs: drop __CHECK_ENDIAN__ Makefile: drop -D__CHECK_ENDIAN__ from cflags Documentation/translations/zh_CN/sparse.txt | 7 +-- arch/arm/plat-samsung/include/plat/gpio-cfg.h | 2 +- drivers/md/dm-cache-block-types.h | 6 +++--- drivers/net/ethernet/sun/sunhme.h | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-fw-file.h | 4 ++-- fs/logfs/logfs.h | 4 +--- include/linux/mmzone.h| 2 +- include/linux/serial_core.h | 4 ++-- include/linux/types.h | 4 ++-- include/scsi/iscsi_proto.h| 2 +- include/target/target_core_base.h | 2 +- include/uapi/linux/types.h| 4 include/uapi/linux/virtio_types.h | 6 +++--- net/ieee802154/6lowpan/6lowpan_i.h| 2 +- net/mac80211/ieee80211_i.h| 4 ++-- tools/include/linux/types.h | 4 Documentation/dev-tools/sparse.rst| 14 +- drivers/bluetooth/Makefile| 2 -- drivers/net/can/Makefile | 1 - drivers/net/ethernet/altera/Makefile | 1 - drivers/net/ethernet/atheros/alx/Makefile | 1 - drivers/net/ethernet/freescale/Makefile | 2 -- drivers/net/wireless/ath/Makefile | 2 -- drivers/net/wireless/ath/wil6210/Makefile | 2 -- drivers/net/wireless/broadcom/brcm80211/brcmfmac/Makefile | 2 -- drivers/net/wireless/broadcom/brcm80211/brcmsmac/Makefile | 1 - drivers/net/wireless/intel/iwlegacy/Makefile | 2 -- drivers/net/wireless/intel/iwlwifi/Makefile | 2 +- drivers/net/wireless/intel/iwlwifi/dvm/Makefile | 2 +- drivers/net/wireless/intel/iwlwifi/mvm/Makefile | 2 +- drivers/net/wireless/intersil/orinoco/Makefile| 3 --- drivers/net/wireless/mediatek/mt7601u/Makefile| 2 -- drivers/net/wireless/realtek/rtlwifi/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/btcoexist/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8188ee/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192c/Makefile| 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192ce/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192cu/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192de/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192ee/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8192se/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8723ae/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8723be/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8723com/Makefile | 2 -- drivers/net/wireless/realtek/rtlwifi/rtl8821ae/Makefile | 2 -- drivers/net/wireless/ti/wl1251/Makefile | 2 -- drivers/net/wireless/ti/wlcore/Makefile | 2 -- drivers/staging/rtl8188eu/Makefile| 2 +- drivers/staging/rtl8192e/Makefile | 2 -- drivers/staging/rtl8192e/rtl8192e/Makefile| 2 -- net/bluetooth/Makefile| 2 -- net/ieee802154/Makefile | 2 -- net/mac80211/Makefile | 2 +- net/mac802154/Makefile| 2 -- net/wireless/Makefile | 2 -- scripts/checkpatch.pl | 4 ++-- 56 files changed, 30 insertions(+), 120 deletions(-) -- MST -- 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 v6 2/2] crypto: add virtio-crypto driver
On Mon, Dec 12, 2016 at 06:54:07PM +0800, Herbert Xu wrote: > On Mon, Dec 12, 2016 at 06:25:12AM +, Gonglei (Arei) wrote: > > Hi, Michael & Herbert > > > > Because the virtio-crypto device emulation had been in QEMU 2.8, > > would you please merge the virtio-crypto driver for 4.10 if no other > > comments? If so, Miachel pls ack and/or review the patch, then > > Herbert will take it (I asked him last week). Thank you! > > > > Ps: Note on 4.10 merge window timing from Linus > > https://lkml.org/lkml/2016/12/7/506 > > > > Dec 23rd is the deadline for 4.10 merge window. > > Sorry but it's too late for 4.10. It needed to have been in my > tree before the merge window opened to make it for this cycle. > > Cheers, Objections to me merging this? I'm preparing my tree right now. > -- > Email: Herbert Xu > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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] linux/types.h: enable endian checks for all sparse builds
On Fri, Dec 09, 2016 at 03:18:02PM +, Bart Van Assche wrote: > On 12/08/16 22:40, Madhani, Himanshu wrote: > > We’ll take a look and send patches to resolve these warnings. > > Thanks! > > Bart. > Sounds good. I posted what I have so far so that you can start from that. -- MST -- 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] linux/types.h: enable endian checks for all sparse builds
On Thu, Dec 08, 2016 at 06:38:11AM +, Bart Van Assche wrote:
> On 12/07/16 21:54, Michael S. Tsirkin wrote:
> > On Thu, Dec 08, 2016 at 05:21:47AM +, Bart Van Assche wrote:
> >> Additionally, there are notable exceptions to the rule that most drivers
> >> are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it
> >> would remain possible to check such drivers with sparse without enabling
> >> endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__
> >> into e.g. #ifndef __DONT_CHECK_ENDIAN__?
> >
> > The right thing is probably just to fix these, isn't it?
> > Until then, why not just ignore the warnings?
>
> Neither option is realistic. With endian-checking enabled the qla2xxx
> driver triggers so many warnings that it becomes a real challenge to
> filter the non-endian warnings out manually:
>
> $ for f in "" CF=-D__CHECK_ENDIAN__; do make M=drivers/scsi/qla2xxx C=2\
> $f | &grep -c ': warning:'; done
> 4
> 752
You can always revert this patch in your tree, or whatever. It does not
look like this will get fixed otherwise.
> If you think it would be easy to fix the endian warnings triggered by
> the qla2xxx driver, you are welcome to try to fix these.
>
> Bart.
Yea, this hardware was designed by someone who thought mixing
LE and BE all over the place is a good idea.
But who said it should be easy?
Maybe this change will be enough to motivate the maintainers.
Here's a minor buglet for you as a motivator:
if (ct_rsp->header.response !=
cpu_to_be16(CT_ACCEPT_RESPONSE)) {
ql_dbg(ql_dbg_disc + ql_dbg_buffer, vha, 0x2077,
"%s failed rejected request on port_id:
%02x%02x%02x Compeltion status 0x%x, response 0x%x\n",
routine, vha->d_id.b.domain,
vha->d_id.b.area, vha->d_id.b.al_pa,
comp_status, ct_rsp->header.response);
response is BE and isn't printed correctly.
another:
eiter->a.max_frame_size = cpu_to_be32(eiter->a.max_frame_size);
size += 4 + 4;
ql_dbg(ql_dbg_disc, vha, 0x20bc,
"Max_Frame_Size = %x.\n", eiter->a.max_frame_size);
printed too late, it's be by that time.
Here's another suspicious line
ctio24->u.status1.flags = (atio->u.isp24.attr << 9) |
cpu_to_le16(CTIO7_FLAGS_STATUS_MODE_1 |
CTIO7_FLAGS_TERMINATE);
shifting attr by 9 bits gives different results on BE and LE,
mixing it with le16 looks rather strange.
Another:
ha->flags.dport_enabled =
(mid_init_cb->init_cb.firmware_options_1 & BIT_7) != 0;
BIT_7 is native endian, firmware_options_1 is LE I think.
Look at qla27xx_find_valid_image as well.
if (pri_image_status.signature != QLA27XX_IMG_STATUS_SIGN)
qla27xx_image_status seems to be data coming from flash, but is
somehow native-endian? Maybe ...
lun = a->u.isp24.fcp_cmnd.lun;
I think lun here is in hardware format (le?), code treats it
as native.
Not to speak about interface abuse all over the place.
How about this:
uint32_t *
qla24xx_read_flash_data(scsi_qla_host_t *vha, uint32_t *dwptr, uint32_t
faddr,
uint32_t dwords)
{
uint32_t i;
struct qla_hw_data *ha = vha->hw;
/* Dword reads to flash. */
for (i = 0; i < dwords; i++, faddr++)
dwptr[i] = cpu_to_le32(qla24xx_read_flash_dword(ha,
flash_data_addr(ha, faddr)));
return dwptr;
}
OK so we convert to LE ...
qla24xx_read_flash_data(vha, dcode, faddr, 4);
risc_addr = be32_to_cpu(dcode[2]);
*srisc_addr = *srisc_addr == 0 ? risc_addr : *srisc_addr;
risc_size = be32_to_cpu(dcode[3]);
then happily assume it's BE.
And again, coming from flash, it's unlikely to actually be in the native
endian-ness as callers seem to assume. I'm guessing it's all BE.
I poked at it a bit and was able to cut down # of warnings
from 1700 to 1400 in an hour. Someone familiar with the code
should look at it.
--
MST
--
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] linux/types.h: enable endian checks for all sparse builds
On Thu, Dec 08, 2016 at 05:21:47AM +, Bart Van Assche wrote: > On 12/07/16 18:29, Michael S. Tsirkin wrote: > > By now, linux is mostly endian-clean. Enabling endian-ness > > checks for everyone produces about 200 new sparse warnings for me - > > less than 10% over the 2000 sparse warnings already there. > > > > Not a big deal, OTOH enabling this helps people notice > > they are introducing new bugs. > > > > So let's just drop __CHECK_ENDIAN__. Follow-up patches > > can drop distinction between __bitwise and __bitwise__. > > Hello Michael, > > This patch makes a whole bunch of ccflags-y += -D__CHECK_ENDIAN__ > statements obsolete. Have you considered to remove these statements? Absolutely. Just waiting for feedback on the idea. > Additionally, there are notable exceptions to the rule that most drivers > are endian-clean, e.g. drivers/scsi/qla2xxx. I would appreciate it if it > would remain possible to check such drivers with sparse without enabling > endianness checks. Have you considered to change #ifdef __CHECK_ENDIAN__ > into e.g. #ifndef __DONT_CHECK_ENDIAN__? > > Thanks, > > Bart. The right thing is probably just to fix these, isn't it? Until then, why not just ignore the warnings? -- MST -- 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
[PATCH] linux/types.h: enable endian checks for all sparse builds
By now, linux is mostly endian-clean. Enabling endian-ness checks for everyone produces about 200 new sparse warnings for me - less than 10% over the 2000 sparse warnings already there. Not a big deal, OTOH enabling this helps people notice they are introducing new bugs. So let's just drop __CHECK_ENDIAN__. Follow-up patches can drop distinction between __bitwise and __bitwise__. Cc: Linus Torvalds Suggested-by: Christoph Hellwig Signed-off-by: Michael S. Tsirkin --- Linus, could you ack this for upstream? If yes I'll merge through my tree as a replacement for enabling this just for virtio. include/uapi/linux/types.h | 4 1 file changed, 4 deletions(-) diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h index acf0979..41e5914 100644 --- a/include/uapi/linux/types.h +++ b/include/uapi/linux/types.h @@ -23,11 +23,7 @@ #else #define __bitwise__ #endif -#ifdef __CHECK_ENDIAN__ #define __bitwise __bitwise__ -#else -#define __bitwise -#endif typedef __u16 __bitwise __le16; typedef __u16 __bitwise __be16; -- MST -- 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 10/10] virtio: enable endian checks for sparse builds
On Wed, Dec 07, 2016 at 07:25:51AM +0100, Johannes Berg wrote: > On Tue, 2016-12-06 at 17:41 +0200, Michael S. Tsirkin wrote: > > > It seems that there should be a better way to do it, > > but this works too. > > In some cases there might be: > > > --- a/drivers/s390/virtio/Makefile > > +++ b/drivers/s390/virtio/Makefile > > @@ -6,6 +6,8 @@ > > # it under the terms of the GNU General Public License (version 2 > > only) > > # as published by the Free Software Foundation. > > > > +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__ > > +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__ > > s390-virtio-objs := virtio_ccw.o > > ifdef CONFIG_S390_GUEST_OLD_TRANSPORT > > s390-virtio-objs += kvm_virtio.o > > Here you could use > > ccflags-y += -D__CHECK_ENDIAN__ > > for example, or even > > subdir-ccflags-y += -D__CHECK_ENDIAN__ > > (in case any subdirs ever get added here) Oh right. I forgot this directory only has virtio stuff. > > --- a/drivers/vhost/Makefile > > +++ b/drivers/vhost/Makefile > > @@ -1,3 +1,4 @@ > > +ccflags-y := -D__CHECK_ENDIAN__ > > Looks like you did that here and in some other places though - so > perhaps the s390 one was intentionally different? > > > --- a/net/packet/Makefile > > +++ b/net/packet/Makefile > > @@ -2,6 +2,7 @@ > > # Makefile for the packet AF. > > # > > > > +ccflags-y := -D__CHECK_ENDIAN__ > > Technically this is slightly more than advertised, but I guess that > still makes sense if it's clean now. > > johannes -- 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
[PATCH 10/10] virtio: enable endian checks for sparse builds
__CHECK_ENDIAN__ isn't on by default presumably because it triggers too many sparse warnings for correct code. But virtio is now clean of these warnings, and we want to keep it this way - enable this for sparse builds. Signed-off-by: Michael S. Tsirkin --- It seems that there should be a better way to do it, but this works too. drivers/block/Makefile | 1 + drivers/char/Makefile | 1 + drivers/char/hw_random/Makefile | 2 ++ drivers/gpu/drm/virtio/Makefile | 1 + drivers/net/Makefile| 3 +++ drivers/net/caif/Makefile | 1 + drivers/rpmsg/Makefile | 1 + drivers/s390/virtio/Makefile| 2 ++ drivers/scsi/Makefile | 1 + drivers/vhost/Makefile | 1 + drivers/virtio/Makefile | 3 +++ net/9p/Makefile | 1 + net/packet/Makefile | 1 + net/vmw_vsock/Makefile | 2 ++ 14 files changed, 21 insertions(+) diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 1e9661e..597481c 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_BLK_DEV_OSD) += osdblk.o obj-$(CONFIG_BLK_DEV_UMEM) += umem.o obj-$(CONFIG_BLK_DEV_NBD) += nbd.o obj-$(CONFIG_BLK_DEV_CRYPTOLOOP) += cryptoloop.o +CFLAGS_virtio_blk.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o obj-$(CONFIG_BLK_DEV_SX8) += sx8.o diff --git a/drivers/char/Makefile b/drivers/char/Makefile index 6e6c244..a99467d 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -6,6 +6,7 @@ obj-y += mem.o random.o obj-$(CONFIG_TTY_PRINTK) += ttyprintk.o obj-y += misc.o obj-$(CONFIG_ATARI_DSP56K) += dsp56k.o +CFLAGS_virtio_console.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_CONSOLE) += virtio_console.o obj-$(CONFIG_RAW_DRIVER) += raw.o obj-$(CONFIG_SGI_SNSC) += snsc.o snsc_event.o diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile index 5f52b1e..a2b0931 100644 --- a/drivers/char/hw_random/Makefile +++ b/drivers/char/hw_random/Makefile @@ -17,6 +17,8 @@ obj-$(CONFIG_HW_RANDOM_IXP4XX) += ixp4xx-rng.o obj-$(CONFIG_HW_RANDOM_OMAP) += omap-rng.o obj-$(CONFIG_HW_RANDOM_OMAP3_ROM) += omap3-rom-rng.o obj-$(CONFIG_HW_RANDOM_PASEMI) += pasemi-rng.o +CFLAGS_virtio_transport.o += -D__CHECK_ENDIAN__ +CFLAGS_virtio-rng.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_HW_RANDOM_VIRTIO) += virtio-rng.o obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o diff --git a/drivers/gpu/drm/virtio/Makefile b/drivers/gpu/drm/virtio/Makefile index 3fb8eac..1162366 100644 --- a/drivers/gpu/drm/virtio/Makefile +++ b/drivers/gpu/drm/virtio/Makefile @@ -3,6 +3,7 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. ccflags-y := -Iinclude/drm +ccflags-y += -D__CHECK_ENDIAN__ virtio-gpu-y := virtgpu_drv.o virtgpu_kms.o virtgpu_drm_bus.o virtgpu_gem.o \ virtgpu_fb.o virtgpu_display.o virtgpu_vq.o virtgpu_ttm.o \ diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 7336cbd..3f587de 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_EQUALIZER) += eql.o obj-$(CONFIG_IFB) += ifb.o obj-$(CONFIG_MACSEC) += macsec.o obj-$(CONFIG_MACVLAN) += macvlan.o +CFLAGS_macvtap.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_MACVTAP) += macvtap.o obj-$(CONFIG_MII) += mii.o obj-$(CONFIG_MDIO) += mdio.o @@ -20,8 +21,10 @@ obj-$(CONFIG_NETCONSOLE) += netconsole.o obj-$(CONFIG_PHYLIB) += phy/ obj-$(CONFIG_RIONET) += rionet.o obj-$(CONFIG_NET_TEAM) += team/ +CFLAGS_tun.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_TUN) += tun.o obj-$(CONFIG_VETH) += veth.o +CFLAGS_virtio_net.o += -D__CHECK_ENDIAN__ obj-$(CONFIG_VIRTIO_NET) += virtio_net.o obj-$(CONFIG_VXLAN) += vxlan.o obj-$(CONFIG_GENEVE) += geneve.o diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile index 9bbd453..d1a922c 100644 --- a/drivers/net/caif/Makefile +++ b/drivers/net/caif/Makefile @@ -12,3 +12,4 @@ obj-$(CONFIG_CAIF_HSI) += caif_hsi.o # Virtio interface obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o +CFLAGS_caif_virtio.o += -D__CHECK_ENDIAN__ diff --git a/drivers/rpmsg/Makefile b/drivers/rpmsg/Makefile index ae9c913..23c8b66 100644 --- a/drivers/rpmsg/Makefile +++ b/drivers/rpmsg/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_RPMSG)+= rpmsg_core.o obj-$(CONFIG_RPMSG_QCOM_SMD) += qcom_smd.o obj-$(CONFIG_RPMSG_VIRTIO) += virtio_rpmsg_bus.o +CFLAGS_virtio_rpmsg_bus.o += -D__CHECK_ENDIAN__ diff --git a/drivers/s390/virtio/Makefile b/drivers/s390/virtio/Makefile index df40692..270ada5 100644 --- a/drivers/s390/virtio/Makefile +++ b/drivers/s390/virtio/Makefile @@ -6,6 +6,8 @@ # it under the terms of the GNU General Public License (version 2 only) # as published by the Free Software Foundation. +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__ +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__ s390-virtio
[PATCH 00/10] virtio: sparse fixes
I run latest sparse from git on virtio drivers (turns out the version I had was rather outdated). This patchset fixes a couple of bugs this uncovered, and adds some annotations to make it sparse-clean. In particular, endian-ness is often tricky, so this patchset enabled endian-ness checks for sparse builds. Michael S. Tsirkin (10): virtio_console: drop unused config fields drm/virtio: fix endianness in primary_plane_update drm/virtio: fix lock context imbalance drm/virtio: annotate virtio_gpu_queue_ctrl_buffer_locked vhost: make interval tree static inline vhost: add missing __user annotations vsock/virtio: add a missing __le annotation vsock/virtio: mark an internal function static vsock/virtio: fix src/dst cid format virtio: enable endian checks for sparse builds drivers/char/virtio_console.c | 14 +++--- drivers/gpu/drm/virtio/virtgpu_plane.c | 4 ++-- drivers/gpu/drm/virtio/virtgpu_vq.c | 6 +- drivers/vhost/vhost.c | 12 ++-- net/vmw_vsock/virtio_transport.c| 2 +- net/vmw_vsock/virtio_transport_common.c | 16 drivers/block/Makefile | 1 + drivers/char/Makefile | 1 + drivers/char/hw_random/Makefile | 2 ++ drivers/gpu/drm/virtio/Makefile | 1 + drivers/net/Makefile| 3 +++ drivers/net/caif/Makefile | 1 + drivers/rpmsg/Makefile | 1 + drivers/s390/virtio/Makefile| 2 ++ drivers/scsi/Makefile | 1 + drivers/vhost/Makefile | 1 + drivers/virtio/Makefile | 3 +++ net/9p/Makefile | 1 + net/packet/Makefile | 1 + net/vmw_vsock/Makefile | 2 ++ 20 files changed, 50 insertions(+), 25 deletions(-) -- MST -- 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 v3] crypto: add virtio-crypto driver
On Mon, Nov 28, 2016 at 04:20:55PM +, Stefan Hajnoczi wrote: > On Mon, Nov 28, 2016 at 08:08:23PM +0800, Gonglei wrote: > > This patch introduces virtio-crypto driver for Linux Kernel. > > > > The virtio crypto device is a virtual cryptography device > > as well as a kind of virtual hardware accelerator for > > virtual machines. The encryption anddecryption requests > > are placed in the data queue and are ultimately handled by > > thebackend crypto accelerators. The second queue is the > > control queue used to create or destroy sessions for > > symmetric algorithms and will control some advanced features > > in the future. The virtio crypto device provides the following > > cryptoservices: CIPHER, MAC, HASH, and AEAD. > > > > For more information about virtio-crypto device, please see: > > http://qemu-project.org/Features/VirtioCrypto > > I've left some comments below. > > > > > CC: Michael S. Tsirkin > > CC: Cornelia Huck > > CC: Stefan Hajnoczi > > CC: Herbert Xu > > CC: Halil Pasic > > CC: David S. Miller > > CC: Zeng Xin > > Signed-off-by: Gonglei > > --- > > MAINTAINERS | 9 + > > drivers/crypto/Kconfig | 2 + > > drivers/crypto/Makefile | 1 + > > drivers/crypto/virtio/Kconfig| 10 + > > drivers/crypto/virtio/Makefile | 5 + > > drivers/crypto/virtio/virtio_crypto.c| 451 +++ > > drivers/crypto/virtio/virtio_crypto_algs.c | 525 > > +++ > > drivers/crypto/virtio/virtio_crypto_common.h | 124 +++ > > drivers/crypto/virtio/virtio_crypto_mgr.c| 258 + > > include/uapi/linux/Kbuild| 1 + > > include/uapi/linux/virtio_crypto.h | 450 +++ > > include/uapi/linux/virtio_ids.h | 1 + > > 12 files changed, 1837 insertions(+) > > create mode 100644 drivers/crypto/virtio/Kconfig > > create mode 100644 drivers/crypto/virtio/Makefile > > create mode 100644 drivers/crypto/virtio/virtio_crypto.c > > create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c > > create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h > > create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c > > create mode 100644 include/uapi/linux/virtio_crypto.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ad9b965..cccaaf0 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12810,6 +12810,7 @@ F: drivers/net/virtio_net.c > > F: drivers/block/virtio_blk.c > > F: include/linux/virtio_*.h > > F: include/uapi/linux/virtio_*.h > > +F: drivers/crypto/virtio/ > > > > VIRTIO DRIVERS FOR S390 > > M: Christian Borntraeger > > @@ -12846,6 +12847,14 @@ S: Maintained > > F: drivers/virtio/virtio_input.c > > F: include/uapi/linux/virtio_input.h > > > > +VIRTIO CRYPTO DRIVER > > +M: Gonglei > > +L: virtualizat...@lists.linux-foundation.org > > +L: linux-crypto@vger.kernel.org > > +S: Maintained > > +F: drivers/crypto/virtio/ > > +F: include/uapi/linux/virtio_crypto.h > > + > > VIA RHINE NETWORK DRIVER > > S: Orphan > > F: drivers/net/ethernet/via/via-rhine.c > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > > index 4d2b81f..7956478 100644 > > --- a/drivers/crypto/Kconfig > > +++ b/drivers/crypto/Kconfig > > @@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP > > > > source "drivers/crypto/chelsio/Kconfig" > > > > +source "drivers/crypto/virtio/Kconfig" > > + > > endif # CRYPTO_HW > > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile > > index ad7250f..bc53cb8 100644 > > --- a/drivers/crypto/Makefile > > +++ b/drivers/crypto/Makefile > > @@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/ > > obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/ > > obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/ > > obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/ > > +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/ > > diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig > > new file mode 100644 > > index 000..ceae88c > > --- /dev/null > > +++ b/drivers/crypto/virtio/Kconfig > > @@ -0,0 +1,10 @@ > > +config CRYPTO_DEV_VIRTIO > > + tristate "VirtIO crypto driver" > > + depends on VIRTIO > > +select CRYPTO_AEAD >
Re: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
On Mon, Nov 28, 2016 at 04:47:21AM +, Gonglei (Arei) wrote: > Michael, I'd like to add virtio-crypto stuff to your maintaining part likes > the virtio-net/blk parts so that the corresponding patches > can be CC'ed to you too because the virtio-crypto doesn't lay in > driver/virtio directory. Will you? > > Thanks! > -Gonglei Sure, that's fine. -- MST -- 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: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
On Mon, Nov 28, 2016 at 04:34:56AM +0200, Michael S. Tsirkin wrote:
> On Mon, Nov 28, 2016 at 01:56:04AM +, Gonglei (Arei) wrote:
> > Hi Michael,
> >
> > Thanks for your feedback firstly!
> >
> > > -Original Message-
> > > From: virtio-...@lists.oasis-open.org
> > > [mailto:virtio-...@lists.oasis-open.org]
> > > On Behalf Of Michael S. Tsirkin
> > > Sent: Sunday, November 27, 2016 11:33 AM
> > > To: Gonglei (Arei)
> > > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian
> > > functions for
> > > virtio_cread/write# family
> > >
> > > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > > Virtio modern devices are always little edian, let's introduce
> > > > the LE functions for read/write configuration space for
> > > > virtio modern devices, which avoid complaint by Sparse when
> > > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > > >
> > > > Signed-off-by: Gonglei
> > > > ---
> > > > include/linux/virtio_config.h | 45
> > > +++
> > > > 1 file changed, 45 insertions(+)
> > > >
> > > > diff --git a/include/linux/virtio_config.h
> > > > b/include/linux/virtio_config.h
> > > > index 26c155b..de05707 100644
> > > > --- a/include/linux/virtio_config.h
> > > > +++ b/include/linux/virtio_config.h
> > > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct
> > > > virtio_device
> > > *vdev,
> > > > _r;
> > > > \
> > > > })
> > > >
> > > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > > +unsigned int offset)
> > > > +{
> > > > + __le16 ret;
> > > > +
> > > > + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > > + unsigned int offset, __le16 val)
> > > > +{
> > > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > > +unsigned int offset)
> > > > +{
> > > > + __le32 ret;
> > > > +
> > > > + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > > + unsigned int offset, __le32 val)
> > > > +{
> > > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > > +unsigned int offset)
> > > > +{
> > > > + __le64 ret;
> > > > +
> > > > + __virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > > + unsigned int offset, __le64 val)
> > > > +{
> > > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > #endif /* _LINUX_VIRTIO_CONFIG_H */
> > >
> > > Could you please better explain what is the issue you are facing?
> > > virtio_cwrite/virtio_cread all accept and return native types.
> > >
> > virtio_cwrite/virtio_cread are used to write/read configuration
> > space for virtio devices. For virtio-crypto device, I used __le32/64
> > directly
> > in struct virtio_crypto_config. The sparse reports warnings if I use
> > virtio_cread()
> > for virtio-crypto device.
>
> I suspect that's because you are doing cread into an le32 variable.
>
>
>
> > Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> > they are definitely LE, whic
Re: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
On Mon, Nov 28, 2016 at 01:56:04AM +, Gonglei (Arei) wrote:
> Hi Michael,
>
> Thanks for your feedback firstly!
>
> > -Original Message-
> > From: virtio-...@lists.oasis-open.org
> > [mailto:virtio-...@lists.oasis-open.org]
> > On Behalf Of Michael S. Tsirkin
> > Sent: Sunday, November 27, 2016 11:33 AM
> > To: Gonglei (Arei)
> > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian
> > functions for
> > virtio_cread/write# family
> >
> > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > Virtio modern devices are always little edian, let's introduce
> > > the LE functions for read/write configuration space for
> > > virtio modern devices, which avoid complaint by Sparse when
> > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > >
> > > Signed-off-by: Gonglei
> > > ---
> > > include/linux/virtio_config.h | 45
> > +++
> > > 1 file changed, 45 insertions(+)
> > >
> > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > index 26c155b..de05707 100644
> > > --- a/include/linux/virtio_config.h
> > > +++ b/include/linux/virtio_config.h
> > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct
> > > virtio_device
> > *vdev,
> > > _r; \
> > > })
> > >
> > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > + unsigned int offset)
> > > +{
> > > + __le16 ret;
> > > +
> > > + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > + return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > +unsigned int offset, __le16 val)
> > > +{
> > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > + unsigned int offset)
> > > +{
> > > + __le32 ret;
> > > +
> > > + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > + return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > +unsigned int offset, __le32 val)
> > > +{
> > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > + unsigned int offset)
> > > +{
> > > + __le64 ret;
> > > +
> > > + __virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > + return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > +unsigned int offset, __le64 val)
> > > +{
> > > + vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > > #endif /* _LINUX_VIRTIO_CONFIG_H */
> >
> > Could you please better explain what is the issue you are facing?
> > virtio_cwrite/virtio_cread all accept and return native types.
> >
> virtio_cwrite/virtio_cread are used to write/read configuration
> space for virtio devices. For virtio-crypto device, I used __le32/64 directly
> in struct virtio_crypto_config. The sparse reports warnings if I use
> virtio_cread()
> for virtio-crypto device.
I suspect that's because you are doing cread into an le32 variable.
> Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> they are definitely LE, which it's not necessary to use
> virtio_to_cpu/cpu_to_virtio.
>
>
> PS: I googled a discussion about this topic for virtio-input device, pls see:
> http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver
>
> Regards,
> -Gonglei
Looks like we changed the macros since - at least ATM
virtio_console_config uses __u16 fields (which is probably a bug -
I'll look into fixing it up) and they
do not seem to trigger warnings.
> > If you want it in LE format, swap it!
> >
> >
> >
> > > --
> > > 1.8.3.1
> > >
> >
> > -
> > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
--
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 v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> Virtio modern devices are always little edian, let's introduce
> the LE functions for read/write configuration space for
> virtio modern devices, which avoid complaint by Sparse when
> we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
>
> Signed-off-by: Gonglei
> ---
> include/linux/virtio_config.h | 45
> +++
> 1 file changed, 45 insertions(+)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 26c155b..de05707 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device
> *vdev,
> _r; \
> })
>
> +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> + unsigned int offset)
> +{
> + __le16 ret;
> +
> + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> + return ret;
> +}
> +
> +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> +unsigned int offset, __le16 val)
> +{
> + vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
> +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> + unsigned int offset)
> +{
> + __le32 ret;
> +
> + vdev->config->get(vdev, offset, &ret, sizeof(ret));
> + return ret;
> +}
> +
> +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> +unsigned int offset, __le32 val)
> +{
> + vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
> +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> + unsigned int offset)
> +{
> + __le64 ret;
> +
> + __virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> + return ret;
> +}
> +
> +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> +unsigned int offset, __le64 val)
> +{
> + vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
> #endif /* _LINUX_VIRTIO_CONFIG_H */
Could you please better explain what is the issue you are facing?
virtio_cwrite/virtio_cread all accept and return native types.
If you want it in LE format, swap it!
> --
> 1.8.3.1
>
--
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 v2 2/2] crypto: add virtio-crypto driver
On Tue, Nov 22, 2016 at 04:10:23PM +0800, Gonglei wrote: > This patch introduces virtio-crypto driver for Linux Kernel. > > The virtio crypto device is a virtual cryptography device > as well as a kind of virtual hardware accelerator for > virtual machines. The encryption anddecryption requests > are placed in the data queue and are ultimately handled by > thebackend crypto accelerators. The second queue is the > control queue used to create or destroy sessions for > symmetric algorithms and will control some advanced features > in the future. The virtio crypto device provides the following > cryptoservices: CIPHER, MAC, HASH, and AEAD. > > For more information about virtio-crypto device, please see: > http://qemu-project.org/Features/VirtioCrypto > > CC: Michael S. Tsirkin > CC: Cornelia Huck > CC: Stefan Hajnoczi > CC: Herbert Xu > CC: Halil Pasic > CC: David S. Miller > CC: Zeng Xin > Signed-off-by: Gonglei > --- > MAINTAINERS | 8 + > drivers/crypto/Kconfig | 2 + > drivers/crypto/Makefile | 1 + > drivers/crypto/virtio/Kconfig| 10 + > drivers/crypto/virtio/Makefile | 5 + > drivers/crypto/virtio/virtio_crypto.c| 444 +++ > drivers/crypto/virtio/virtio_crypto_algs.c | 524 > +++ > drivers/crypto/virtio/virtio_crypto_common.h | 124 +++ > drivers/crypto/virtio/virtio_crypto_mgr.c| 258 + > include/uapi/linux/Kbuild| 1 + > include/uapi/linux/virtio_crypto.h | 435 ++ > include/uapi/linux/virtio_ids.h | 1 + > 12 files changed, 1813 insertions(+) > create mode 100644 drivers/crypto/virtio/Kconfig > create mode 100644 drivers/crypto/virtio/Makefile > create mode 100644 drivers/crypto/virtio/virtio_crypto.c > create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c > create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h > create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c > create mode 100644 include/uapi/linux/virtio_crypto.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 411e3b8..d6b18bb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12844,6 +12844,14 @@ S: Maintained > F: drivers/virtio/virtio_input.c > F: include/uapi/linux/virtio_input.h > > +VIRTIO CRYPTO DRIVER > +M: Gonglei > +L: virtualizat...@lists.linux-foundation.org > +L: linux-crypto@vger.kernel.org > +S: Maintained > +F: drivers/crypto/virtio/ > +F: include/uapi/linux/virtio_crypto.h > + > VIA RHINE NETWORK DRIVER > S: Orphan > F: drivers/net/ethernet/via/via-rhine.c > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > index 4d2b81f..7956478 100644 > --- a/drivers/crypto/Kconfig > +++ b/drivers/crypto/Kconfig > @@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP > > source "drivers/crypto/chelsio/Kconfig" > > +source "drivers/crypto/virtio/Kconfig" > + > endif # CRYPTO_HW > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile > index ad7250f..bc53cb8 100644 > --- a/drivers/crypto/Makefile > +++ b/drivers/crypto/Makefile > @@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/ > obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/ > obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/ > obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/ > +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/ > diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig > new file mode 100644 > index 000..ceae88c > --- /dev/null > +++ b/drivers/crypto/virtio/Kconfig > @@ -0,0 +1,10 @@ > +config CRYPTO_DEV_VIRTIO > + tristate "VirtIO crypto driver" > + depends on VIRTIO > +select CRYPTO_AEAD > +select CRYPTO_AUTHENC > +select CRYPTO_BLKCIPHER > + default m > + help > + This driver provides support for virtio crypto device. If you > + choose 'M' here, this module will be called virtio-crypto. > diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile > new file mode 100644 > index 000..a316e92 > --- /dev/null > +++ b/drivers/crypto/virtio/Makefile > @@ -0,0 +1,5 @@ > +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio-crypto.o > +virtio-crypto-objs := \ > + virtio_crypto_algs.o \ > + virtio_crypto_mgr.o \ > + virtio_crypto.o > diff --git a/drivers/crypto/virtio/virtio_crypto.c > b/drivers/crypto/virtio/virtio_crypto.c > new file mode 100644 > index 000..56fdfed > --- /dev/null > +++ b/drivers/crypto/virtio/virtio_crypto.c > @@ -0,0 +1,444 @@ > +
[PATCH v2 1/6] crypto/ccp: drop linux/pci dependencies
pci code is in ccp-pci.c, don't include pci headers from ccp/ccp-ops.c. Signed-off-by: Michael S. Tsirkin --- drivers/crypto/ccp/ccp-ops.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c index 8729364..3da1140 100644 --- a/drivers/crypto/ccp/ccp-ops.c +++ b/drivers/crypto/ccp/ccp-ops.c @@ -12,8 +12,6 @@ #include #include -#include -#include #include #include #include -- MST -- 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
[PATCH 15/86] crypto/geode: use uapi/linux/pci_ids.h directly
Header moved from linux/pci_ids.h to uapi/linux/pci_ids.h, use the new header directly so we can drop the wrapper in include/linux/pci_ids.h. Signed-off-by: Michael S. Tsirkin --- drivers/crypto/geode-aes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c index fe538e5..5775d1a 100644 --- a/drivers/crypto/geode-aes.c +++ b/drivers/crypto/geode-aes.c @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include -- MST -- 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
[PATCH 80/86] crypto/ccp: drop linux/pci dependencies
pci code is in ccp-pci.c, don't include pci headers from ccp/ccp-ops.c. Signed-off-by: Michael S. Tsirkin --- drivers/crypto/ccp/ccp-ops.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c index fd5491d..3da1140 100644 --- a/drivers/crypto/ccp/ccp-ops.c +++ b/drivers/crypto/ccp/ccp-ops.c @@ -12,8 +12,6 @@ #include #include -#include -#include #include #include #include -- MST -- 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
[PATCH 14/86] crypto/ccp: use uapi/linux/pci_ids.h directly
Header moved from linux/pci_ids.h to uapi/linux/pci_ids.h, use the new header directly so we can drop the wrapper in include/linux/pci_ids.h. Signed-off-by: Michael S. Tsirkin --- drivers/crypto/ccp/ccp-ops.c | 2 +- drivers/crypto/ccp/ccp-pci.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c index 8729364..fd5491d 100644 --- a/drivers/crypto/ccp/ccp-ops.c +++ b/drivers/crypto/ccp/ccp-ops.c @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/drivers/crypto/ccp/ccp-pci.c b/drivers/crypto/ccp/ccp-pci.c index 7f89c94..c09d0eb 100644 --- a/drivers/crypto/ccp/ccp-pci.c +++ b/drivers/crypto/ccp/ccp-pci.c @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include -- MST -- 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
[PATCH repost] virtio_rng: drop extra empty line
makes code look a bit prettier.
Cc: linux-crypto@vger.kernel.org.
Signed-off-by: Michael S. Tsirkin
---
drivers/char/hw_random/virtio-rng.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/char/hw_random/virtio-rng.c
b/drivers/char/hw_random/virtio-rng.c
index 72295ea..3fa2f8a 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -39,7 +39,6 @@ struct virtrng_info {
bool hwrng_removed;
};
-
static void random_recv_done(struct virtqueue *vq)
{
struct virtrng_info *vi = vq->vdev->priv;
--
MST
--
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] virtio_rng: drop extra empty line
On Fri, Jan 16, 2015 at 10:21:09AM +1100, Herbert Xu wrote: > On Thu, Jan 15, 2015 at 01:50:42PM +0200, Michael S. Tsirkin wrote: > > makes code look a bit prettier. > > > > Signed-off-by: Michael S. Tsirkin > > Please resend this patch with a cc to linux-crypto@vger.kernel.org. > > Thanks! So let's add this to maintainers? Will you ack something like below? ---> MAINTAINERS: add linux-crypto to hw random hw random is crypto-related, Cc the linux-crypto list on patches. Signed-off-by: Michael S. Tsirkin diff --git a/MAINTAINERS b/MAINTAINERS index 3589d67..4d54f2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4403,6 +4403,7 @@ F:include/linux/hwmon*.h HARDWARE RANDOM NUMBER GENERATOR CORE M: Matt Mackall M: Herbert Xu +L: linux-crypto@vger.kernel.org S: Odd fixes F: Documentation/hw_random.txt F: drivers/char/hw_random/ -- 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
