Re: [PATCH v2 4/6] crypto: virtio: convert to new crypto engine API

[Michael S. Tsirkin]

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 <clabbe.montj...@gmail.com>

Acked-by: Michael S. Tsirkin <m...@redhat.com>

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 = _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(_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

[Michael S. Tsirkin]

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<m...@redhat.com>
> > > > ---
> > > 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

[Michael S. Tsirkin]

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<m...@redhat.com>
> > ---
> 
> 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

[Michael S. Tsirkin]

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 <m...@redhat.com>
---
 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,
-   );
+   err = virtio_find_vqs(vdev, num_vqs, vqs, callbacks, names, );
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, >vq_tx, _cbs, ,
-   NULL);
+   err = virtio_find_vqs(vdev, 1, >vq_tx, _cbs, , 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 *vdev)
init_waitqueue_head(>sendq);
 

Re: [PATCH v8 1/1] crypto: add virtio-crypto driver

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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(>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

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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 <torva...@linux-foundation.org>
Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
---

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

[PATCH 10/10] virtio: enable endian checks for sparse builds

[Michael S. Tsirkin]

__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 <m...@redhat.com>
---

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__

[PATCH 00/10] virtio: sparse fixes

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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 <m...@redhat.com>
> > CC: Cornelia Huck <cornelia.h...@de.ibm.com>
> > CC: Stefan Hajnoczi <stefa...@redhat.com>
> > CC: Herbert Xu <herb...@gondor.apana.org.au>
> > CC: Halil Pasic <pa...@linux.vnet.ibm.com>
> > CC: David S. Miller <da...@davemloft.net>
> > CC: Zeng Xin <xin.z...@intel.com>
> > Signed-off-by: Gonglei <arei.gong...@huawei.com>
> > ---
> >  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 <borntrae...@de.ibm.com>
> > @@ -12846,6 +12847,14 @@ S: Maintained
> >  F: drivers/virtio/virtio_input.c
> >  F: include/uapi/linux/virtio_input.h
> >  
> > +VIRTIO CRYPTO DRIVER
> > +M:  Gonglei <arei.gong...@huawei.com>
> > +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
> > in

Re: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver

[Michael S. Tsirkin]

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

[Michael S. Tsirkin]

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 <arei.gong...@huawei.com>
> > > > ---
> > > >  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, , sizeof(ret));
> > > > +   return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > > +  unsigned int offset, __le16 val)
> > > > +{
> > > > +   vdev->config->set(vdev, offset, , sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > > +unsigned int offset)
> > > > +{
> > > > +   __le32 ret;
> > > > +
> > > > +   vdev->config->get(vdev, offset, , sizeof(ret));
> > > > +   return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > > +  unsigned int offset, __le32 val)
> > > > +{
> > > > +   vdev->config->set(vdev, offset, , sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > > +unsigned int offset)
> > > > +{
> > > > +   __le64 ret;
> > > > +
> > > > +   __virtio_cread_many(vdev, offset, , 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, , 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 

Re: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family

[Michael S. Tsirkin]

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 <arei.gong...@huawei.com>
> > > ---
> > >  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, , sizeof(ret));
> > > + return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > +unsigned int offset, __le16 val)
> > > +{
> > > + vdev->config->set(vdev, offset, , sizeof(val));
> > > +}
> > > +
> > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > +  unsigned int offset)
> > > +{
> > > + __le32 ret;
> > > +
> > > + vdev->config->get(vdev, offset, , sizeof(ret));
> > > + return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > +unsigned int offset, __le32 val)
> > > +{
> > > + vdev->config->set(vdev, offset, , sizeof(val));
> > > +}
> > > +
> > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > +  unsigned int offset)
> > > +{
> > > + __le64 ret;
> > > +
> > > + __virtio_cread_many(vdev, offset, , 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, , 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

[Michael S. Tsirkin]

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, , sizeof(ret));
> + return ret;
> +}
> +
> +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> +unsigned int offset, __le16 val)
> +{
> + vdev->config->set(vdev, offset, , sizeof(val));
> +}
> +
> +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> +  unsigned int offset)
> +{
> + __le32 ret;
> +
> + vdev->config->get(vdev, offset, , sizeof(ret));
> + return ret;
> +}
> +
> +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> +unsigned int offset, __le32 val)
> +{
> + vdev->config->set(vdev, offset, , sizeof(val));
> +}
> +
> +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> +  unsigned int offset)
> +{
> + __le64 ret;
> +
> + __virtio_cread_many(vdev, offset, , 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, , 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

[Michael S. Tsirkin]

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 <m...@redhat.com>
> CC: Cornelia Huck <cornelia.h...@de.ibm.com>
> CC: Stefan Hajnoczi <stefa...@redhat.com>
> CC: Herbert Xu <herb...@gondor.apana.org.au>
> CC: Halil Pasic <pa...@linux.vnet.ibm.com>
> CC: David S. Miller <da...@davemloft.net>
> CC: Zeng Xin <xin.z...@intel.com>
> Signed-off-by: Gonglei <arei.gong...@huawei.com>
> ---
>  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 <arei.gong...@huawei.com>
> +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
>

[PATCH v2 1/6] crypto/ccp: drop linux/pci dependencies

[Michael S. Tsirkin]

pci code is in ccp-pci.c, don't include pci
headers from ccp/ccp-ops.c.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 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 linux/module.h
 #include linux/kernel.h
-#include linux/pci.h
-#include linux/pci_ids.h
 #include linux/kthread.h
 #include linux/sched.h
 #include linux/interrupt.h
-- 
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

[Michael S. Tsirkin]

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 m...@redhat.com
---
 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 linux/module.h
 #include linux/kernel.h
 #include linux/pci.h
-#include linux/pci_ids.h
+#include uapi/linux/pci_ids.h
 #include linux/crypto.h
 #include linux/spinlock.h
 #include crypto/algapi.h
-- 
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

[Michael S. Tsirkin]

pci code is in ccp-pci.c, don't include pci
headers from ccp/ccp-ops.c.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 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 linux/module.h
 #include linux/kernel.h
-#include linux/pci.h
-#include uapi/linux/pci_ids.h
 #include linux/kthread.h
 #include linux/sched.h
 #include linux/interrupt.h
-- 
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

[Michael S. Tsirkin]

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 m...@redhat.com
---
 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 linux/module.h
 #include linux/kernel.h
 #include linux/pci.h
-#include linux/pci_ids.h
+#include uapi/linux/pci_ids.h
 #include linux/kthread.h
 #include linux/sched.h
 #include linux/interrupt.h
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 linux/kernel.h
 #include linux/device.h
 #include linux/pci.h
-#include linux/pci_ids.h
+#include uapi/linux/pci_ids.h
 #include linux/dma-mapping.h
 #include linux/kthread.h
 #include linux/sched.h
-- 
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

[Michael S. Tsirkin]

makes code look a bit prettier.

Cc: linux-crypto@vger.kernel.org.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 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

[Michael S. Tsirkin]

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 m...@redhat.com
 
 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 m...@redhat.com


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...@selenic.com
 M: Herbert Xu herb...@gondor.apana.org.au
+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