Re: [PATCH v3] vdpa/vp_vdpa : add vdpa tool support in vp_vdpa

[Jason Wang]

在 2022/4/28 15:37, Cindy Lu 写道:

this patch is to add the support for vdpa tool in vp_vdpa
here is the example steps

modprobe vp_vdpa
modprobe vhost_vdpa
echo :00:06.0>/sys/bus/pci/drivers/virtio-pci/unbind
echo 1af4 1041 > /sys/bus/pci/drivers/vp-vdpa/new_id

vdpa dev add name vdpa1 mgmtdev pci/:00:06.0

Signed-off-by: Cindy Lu 
---
  drivers/vdpa/virtio_pci/vp_vdpa.c | 159 --
  include/linux/vdpa.h  |   2 +-
  2 files changed, 128 insertions(+), 33 deletions(-)

diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c 
b/drivers/vdpa/virtio_pci/vp_vdpa.c
index cce101e6a940..a3827e496b8f 100644
--- a/drivers/vdpa/virtio_pci/vp_vdpa.c
+++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
@@ -32,7 +32,7 @@ struct vp_vring {
  
  struct vp_vdpa {

struct vdpa_device vdpa;
-   struct virtio_pci_modern_device mdev;
+   struct virtio_pci_modern_device *mdev;
struct vp_vring *vring;
struct vdpa_callback config_cb;
char msix_name[VP_VDPA_NAME_SIZE];
@@ -41,6 +41,12 @@ struct vp_vdpa {
int vectors;
  };
  
+struct vp_vdpa_mgmtdev {

+   struct vdpa_mgmt_dev mgtdev;
+   struct virtio_pci_modern_device *mdev;
+   struct vp_vdpa *vp_vdpa;
+};
+
  static struct vp_vdpa *vdpa_to_vp(struct vdpa_device *vdpa)
  {
return container_of(vdpa, struct vp_vdpa, vdpa);
@@ -50,7 +56,12 @@ static struct virtio_pci_modern_device *vdpa_to_mdev(struct 
vdpa_device *vdpa)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
  
-	return &vp_vdpa->mdev;

+   return vp_vdpa->mdev;
+}
+
+static struct virtio_pci_modern_device *vp_vdpa_to_mdev(struct vp_vdpa 
*vp_vdpa)
+{
+   return vp_vdpa->mdev;
  }
  
  static u64 vp_vdpa_get_device_features(struct vdpa_device *vdpa)

@@ -96,7 +107,7 @@ static int vp_vdpa_get_vq_irq(struct vdpa_device *vdpa, u16 
idx)
  
  static void vp_vdpa_free_irq(struct vp_vdpa *vp_vdpa)

  {
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
struct pci_dev *pdev = mdev->pci_dev;
int i;
  
@@ -143,7 +154,7 @@ static irqreturn_t vp_vdpa_config_handler(int irq, void *arg)
  
  static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa)

  {
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
struct pci_dev *pdev = mdev->pci_dev;
int i, ret, irq;
int queues = vp_vdpa->queues;
@@ -198,7 +209,7 @@ static int vp_vdpa_request_irq(struct vp_vdpa *vp_vdpa)
  static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 status)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
u8 s = vp_vdpa_get_status(vdpa);
  
  	if (status & VIRTIO_CONFIG_S_DRIVER_OK &&

@@ -212,7 +223,7 @@ static void vp_vdpa_set_status(struct vdpa_device *vdpa, u8 
status)
  static int vp_vdpa_reset(struct vdpa_device *vdpa)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
u8 s = vp_vdpa_get_status(vdpa);
  
  	vp_modern_set_status(mdev, 0);

@@ -372,7 +383,7 @@ static void vp_vdpa_get_config(struct vdpa_device *vdpa,
   void *buf, unsigned int len)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
u8 old, new;
u8 *p;
int i;
@@ -392,7 +403,7 @@ static void vp_vdpa_set_config(struct vdpa_device *vdpa,
   unsigned int len)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
const u8 *p = buf;
int i;
  
@@ -412,7 +423,7 @@ static struct vdpa_notification_area

  vp_vdpa_get_vq_notification(struct vdpa_device *vdpa, u16 qid)
  {
struct vp_vdpa *vp_vdpa = vdpa_to_vp(vdpa);
-   struct virtio_pci_modern_device *mdev = &vp_vdpa->mdev;
+   struct virtio_pci_modern_device *mdev = vp_vdpa_to_mdev(vp_vdpa);
struct vdpa_notification_area notify;
  
  	notify.addr = vp_vdpa->vring[qid].notify_pa;

@@ -454,38 +465,31 @@ static void vp_vdpa_free_irq_vectors(void *data)
pci_free_irq_vectors(data);
  }
  
-static int vp_vdpa_probe(struct pci_dev *pdev, const struct pci_device_id *id)

+static int vp_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
+  const struct vdpa_dev_set_config *add_config)
  {
-   struct virtio_pci_modern_device *mdev;
+   struct vp_vdpa_mgmtdev *vp_vdpa_mgtdev =
+   container_of(v

Re: [PATCH] vdpa_sim_blk: add support for VIRTIO_BLK_T_FLUSH

[Jason Wang]

On Thu, Apr 28, 2022 at 11:13 PM Stefano Garzarella  wrote:
>
> The simulator behaves like a ramdisk, so we don't have to do
> anything when a VIRTIO_BLK_T_FLUSH request is received, but it
> could be useful to test driver behavior.
>
> Let's expose the VIRTIO_BLK_F_FLUSH feature to inform the driver
> that we support the flush command.
>
> Signed-off-by: Stefano Garzarella 
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c 
> b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index 42d401d43911..a6dd1233797c 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -25,6 +25,7 @@
>  #define DRV_LICENSE  "GPL v2"
>
>  #define VDPASIM_BLK_FEATURES   (VDPASIM_FEATURES | \
> +(1ULL << VIRTIO_BLK_F_FLUSH)| \
>  (1ULL << VIRTIO_BLK_F_SIZE_MAX) | \
>  (1ULL << VIRTIO_BLK_F_SEG_MAX)  | \
>  (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \
> @@ -166,6 +167,17 @@ static bool vdpasim_blk_handle_req(struct vdpasim 
> *vdpasim,
> pushed += bytes;
> break;
>
> +   case VIRTIO_BLK_T_FLUSH:
> +   if (sector != 0) {
> +   dev_err(&vdpasim->vdpa.dev,
> +   "A driver MUST set sector to 0 for a 
> VIRTIO_BLK_T_FLUSH request - sector: 0x%llx\n",
> +   sector);

If this is something that could be triggered by userspace/guest, then
we should avoid this.

Thanks

> +   status = VIRTIO_BLK_S_IOERR;
> +   break;
> +   }
> +
> +   break;
> +
> default:
> dev_warn(&vdpasim->vdpa.dev,
>  "Unsupported request type %d\n", type);
> --
> 2.35.1
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 3/9] virtio: introduce config op to synchronize vring callbacks

[Jason Wang]

On Thu, Apr 28, 2022 at 5:13 PM Cornelia Huck  wrote:
>
> On Mon, Apr 25 2022, Jason Wang  wrote:
>
> > This patch introduces new virtio config op to vring
> > callbacks. Transport specific method is required to make sure the
> > write before this function is visible to the vring_interrupt() that is
>
> Which kind of writes? I.e., what is the scope?

Any writes before synchronize_cbs(). Is something like the following better?

The function guarantees that all memory operations before it are
visible to the vring_interrupt() that is called after it.

>
> > called after the return of this function. For the transport that
> > doesn't provide synchronize_vqs(), use synchornize_rcu() which
>
> Typo: synchronize_rcu()

Will fix it.

Thanks

>
> > synchronize with IRQ implicitly as a fallback.
> >
> > Cc: Thomas Gleixner 
> > Cc: Peter Zijlstra 
> > Cc: "Paul E. McKenney" 
> > Cc: Marc Zyngier 
> > Cc: Halil Pasic 
> > Cc: Cornelia Huck 
> > Signed-off-by: Jason Wang 
> > ---
> >  include/linux/virtio_config.h | 24 
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > index b341dd62aa4d..14fe89ff99c7 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -57,6 +57,10 @@ struct virtio_shm_region {
> >   *   include a NULL entry for vqs unused by driver
> >   *   Returns 0 on success or error status
> >   * @del_vqs: free virtqueues found by find_vqs().
> > + * @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
> > + *  Make sure the writes commited before this method is visible to
> > + *  vring_interrupt() which is called after this method.
>
> Same here, I think the description needs to be a bit more explicit about
> which writes we care about here.
>
> > + *  vdev: the virtio_device
> >   * @get_features: get the array of feature bits for this device.
> >   *   vdev: the virtio_device
> >   *   Returns the first 64 feature bits (all we currently need).
> > @@ -89,6 +93,7 @@ struct virtio_config_ops {
> >   const char * const names[], const bool *ctx,
> >   struct irq_affinity *desc);
> >   void (*del_vqs)(struct virtio_device *);
> > + void (*synchronize_cbs)(struct virtio_device *);
> >   u64 (*get_features)(struct virtio_device *vdev);
> >   int (*finalize_features)(struct virtio_device *vdev);
> >   const char *(*bus_name)(struct virtio_device *vdev);
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 6/9] virtio-ccw: implement synchronize_cbs()

[Jason Wang]

On Thu, Apr 28, 2022 at 3:42 PM Cornelia Huck  wrote:
>
> On Thu, Apr 28 2022, Jason Wang  wrote:
>
> > On Thu, Apr 28, 2022 at 1:55 PM Michael S. Tsirkin  wrote:
> >>
> >> On Thu, Apr 28, 2022 at 01:51:59PM +0800, Jason Wang wrote:
> >> > On Thu, Apr 28, 2022 at 1:24 PM Michael S. Tsirkin  
> >> > wrote:
> >> > >
> >> > > On Thu, Apr 28, 2022 at 11:04:41AM +0800, Jason Wang wrote:
> >> > > > > But my guess is that rwlock + some testing for the legacy 
> >> > > > > indicator case
> >> > > > > just to double check if there is a heavy regression despite of our
> >> > > > > expectations to see none should do the trick.
> >> > > >
> >> > > > I suggest this, rwlock (for not airq) seems better than spinlock, but
> >> > > > at worst case it will cause cache line bouncing. But I wonder if it's
> >> > > > noticeable (anyhow it has been used for airq).
> >> > > >
> >> > > > Thanks
> >> > >
> >> > > Which existing rwlock does airq use right now? Can we take it to sync?
> >> >
> >> > It's the rwlock in airq_info, it has already been used in this patch.
> >> >
> >> > write_lock(&info->lock);
> >> > write_unlock(&info->lock);
> >> >
> >> > But the problem is, it looks to me there could be a case that airq is
> >> > not used, (virtio_ccw_int_hander()). That's why the patch use a
> >> > spinlock, it could be optimized with using a rwlock as well.
> >> >
> >> > Thanks
> >>
> >> Ah, right. So let's take that on the legacy path too and Halil promises
> >> to test to make sure performance isn't impacted too badly?
> >
> > I think what you meant is using a dedicated rwlock instead of trying
> > to reuse one of the airq_info locks.
> >
> > If this is true, it should be fine.
>
> FWIW, that approach makes sense to me as well.
>

Good to know that. Let me post a new version.

Thanks

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] vdpa_sim_blk: add support for VIRTIO_BLK_T_FLUSH

[Stefano Garzarella]

The simulator behaves like a ramdisk, so we don't have to do
anything when a VIRTIO_BLK_T_FLUSH request is received, but it
could be useful to test driver behavior.

Let's expose the VIRTIO_BLK_F_FLUSH feature to inform the driver
that we support the flush command.

Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c 
b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
index 42d401d43911..a6dd1233797c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
@@ -25,6 +25,7 @@
 #define DRV_LICENSE  "GPL v2"
 
 #define VDPASIM_BLK_FEATURES   (VDPASIM_FEATURES | \
+(1ULL << VIRTIO_BLK_F_FLUSH)| \
 (1ULL << VIRTIO_BLK_F_SIZE_MAX) | \
 (1ULL << VIRTIO_BLK_F_SEG_MAX)  | \
 (1ULL << VIRTIO_BLK_F_BLK_SIZE) | \
@@ -166,6 +167,17 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
pushed += bytes;
break;
 
+   case VIRTIO_BLK_T_FLUSH:
+   if (sector != 0) {
+   dev_err(&vdpasim->vdpa.dev,
+   "A driver MUST set sector to 0 for a 
VIRTIO_BLK_T_FLUSH request - sector: 0x%llx\n",
+   sector);
+   status = VIRTIO_BLK_S_IOERR;
+   break;
+   }
+
+   break;
+
default:
dev_warn(&vdpasim->vdpa.dev,
 "Unsupported request type %d\n", type);
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v5 9/9] crypto: Introduce RSA algorithm

[zhenwei pi]

There are two parts in this patch:
1, support akcipher service by cryptodev-builtin driver
2, virtio-crypto driver supports akcipher service

In principle, we should separate this into two patches, to avoid
compiling error, merge them into one.

Then virtio-crypto gets request from guest side, and forwards the
request to builtin driver to handle it.

Test with a guest linux:
1, The self-test framework of crypto layer works fine in guest kernel
2, Test with Linux guest(with asym support), the following script
test(note that pkey_XXX is supported only in a newer version of keyutils):
  - both public key & private key
  - create/close session
  - encrypt/decrypt/sign/verify basic driver operation
  - also test with kernel crypto layer(pkey add/query)

All the cases work fine.

Run script in guest:
rm -rf *.der *.pem *.pfx
modprobe pkcs8_key_parser # if CONFIG_PKCS8_PRIVATE_KEY_PARSER=m
rm -rf /tmp/data
dd if=/dev/random of=/tmp/data count=1 bs=20

openssl req -nodes -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -subj 
"/C=CN/ST=BJ/L=HD/O=qemu/OU=dev/CN=qemu/emailAddress=q...@qemu.org"
openssl pkcs8 -in key.pem -topk8 -nocrypt -outform DER -out key.der
openssl x509 -in cert.pem -inform PEM -outform DER -out cert.der

PRIV_KEY_ID=`cat key.der | keyctl padd asymmetric test_priv_key @s`
echo "priv key id = "$PRIV_KEY_ID
PUB_KEY_ID=`cat cert.der | keyctl padd asymmetric test_pub_key @s`
echo "pub key id = "$PUB_KEY_ID

keyctl pkey_query $PRIV_KEY_ID 0
keyctl pkey_query $PUB_KEY_ID 0

echo "Enc with priv key..."
keyctl pkey_encrypt $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.priv
echo "Dec with pub key..."
keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.priv enc=pkcs1 >/tmp/dec
cmp /tmp/data /tmp/dec

echo "Sign with priv key..."
keyctl pkey_sign $PRIV_KEY_ID 0 /tmp/data enc=pkcs1 hash=sha1 > /tmp/sig
echo "Verify with pub key..."
keyctl pkey_verify $PRIV_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1

echo "Enc with pub key..."
keyctl pkey_encrypt $PUB_KEY_ID 0 /tmp/data enc=pkcs1 >/tmp/enc.pub
echo "Dec with priv key..."
keyctl pkey_decrypt $PRIV_KEY_ID 0 /tmp/enc.pub enc=pkcs1 >/tmp/dec
cmp /tmp/data /tmp/dec

echo "Verify with pub key..."
keyctl pkey_verify $PUB_KEY_ID 0 /tmp/data /tmp/sig enc=pkcs1 hash=sha1

Signed-off-by: zhenwei pi 
Signed-off-by: lei he conf.crypto_services =
  1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
  1u << VIRTIO_CRYPTO_SERVICE_HASH |
- 1u << VIRTIO_CRYPTO_SERVICE_MAC;
+ 1u << VIRTIO_CRYPTO_SERVICE_MAC |
+ 1u << VIRTIO_CRYPTO_SERVICE_AKCIPHER;
 backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
 backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
+backend->conf.akcipher_algo = 1u << VIRTIO_CRYPTO_AKCIPHER_RSA;
 /*
  * Set the Maximum length of crypto request.
  * Why this value? Just avoid to overflow when
  * memory allocation for each crypto request.
  */
-backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendSymOpInfo);
+backend->conf.max_size = LONG_MAX - sizeof(CryptoDevBackendOpInfo);
 backend->conf.max_cipher_key_len = CRYPTODEV_BUITLIN_MAX_CIPHER_KEY_LEN;
 backend->conf.max_auth_key_len = CRYPTODEV_BUITLIN_MAX_AUTH_KEY_LEN;
 
@@ -148,6 +152,53 @@ err:
return -1;
 }
 
+static int cryptodev_builtin_get_rsa_hash_algo(
+int virtio_rsa_hash, Error **errp)
+{
+switch (virtio_rsa_hash) {
+case VIRTIO_CRYPTO_RSA_MD5:
+return QCRYPTO_HASH_ALG_MD5;
+
+case VIRTIO_CRYPTO_RSA_SHA1:
+return QCRYPTO_HASH_ALG_SHA1;
+
+case VIRTIO_CRYPTO_RSA_SHA256:
+return QCRYPTO_HASH_ALG_SHA256;
+
+case VIRTIO_CRYPTO_RSA_SHA512:
+return QCRYPTO_HASH_ALG_SHA512;
+
+default:
+error_setg(errp, "Unsupported rsa hash algo: %d", virtio_rsa_hash);
+return -1;
+}
+}
+
+static int cryptodev_builtin_set_rsa_options(
+int virtio_padding_algo,
+int virtio_hash_algo,
+QCryptoAkCipherOptionsRSA *opt,
+Error **errp)
+{
+if (virtio_padding_algo == VIRTIO_CRYPTO_RSA_PKCS1_PADDING) {
+opt->padding_alg = QCRYPTO_RSA_PADDING_ALG_PKCS1;
+opt->hash_alg =
+cryptodev_builtin_get_rsa_hash_algo(virtio_hash_algo, errp);
+if (opt->hash_alg < 0) {
+return -1;
+}
+return 0;
+}
+
+if (virtio_padding_algo == VIRTIO_CRYPTO_RSA_RAW_PADDING) {
+opt->padding_alg = QCRYPTO_RSA_PADDING_ALG_RAW;
+return 0;
+}
+
+error_setg(errp, "Unsupported rsa padding algo: %d", virtio_padding_algo);
+return -1;
+}
+
 static int cryptodev_builtin_create_cipher_session(
 CryptoDevBackendBuiltin *builtin,
 CryptoDevBackendSymSessionInfo *sess_info,
@@ -240,26 +291,89 @@ static int cryptodev_builtin_create_cipher_session(
 return index;
 }
 
-static int64_t cryptodev_

[PATCH v5 7/9] test/crypto: Add test suite for crypto akcipher

[zhenwei pi]

From: Lei He 

Add unit test and benchmark test for crypto akcipher.

Signed-off-by: lei he 
Signed-off-by: zhenwei pi 
Reviewed-by: Daniel P. Berrangé 
---
 tests/bench/benchmark-crypto-akcipher.c | 157 ++
 tests/bench/meson.build |   4 +
 tests/bench/test_akcipher_keys.inc  | 537 ++
 tests/unit/meson.build  |   1 +
 tests/unit/test-crypto-akcipher.c   | 711 
 5 files changed, 1410 insertions(+)
 create mode 100644 tests/bench/benchmark-crypto-akcipher.c
 create mode 100644 tests/bench/test_akcipher_keys.inc
 create mode 100644 tests/unit/test-crypto-akcipher.c

diff --git a/tests/bench/benchmark-crypto-akcipher.c 
b/tests/bench/benchmark-crypto-akcipher.c
new file mode 100644
index 00..c6c80c0be1
--- /dev/null
+++ b/tests/bench/benchmark-crypto-akcipher.c
@@ -0,0 +1,157 @@
+/*
+ * QEMU Crypto akcipher speed benchmark
+ *
+ * Copyright (c) 2022 Bytedance
+ *
+ * Authors:
+ *lei he 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/init.h"
+#include "crypto/akcipher.h"
+#include "standard-headers/linux/virtio_crypto.h"
+
+#include "test_akcipher_keys.inc"
+
+static bool keep_running;
+
+static void alarm_handler(int sig)
+{
+keep_running = false;
+}
+
+static QCryptoAkCipher *create_rsa_akcipher(const uint8_t *priv_key,
+size_t keylen,
+QCryptoRSAPaddingAlgorithm padding,
+QCryptoHashAlgorithm hash)
+{
+QCryptoAkCipherOptions opt;
+QCryptoAkCipher *rsa;
+
+opt.alg = QCRYPTO_AKCIPHER_ALG_RSA;
+opt.u.rsa.padding_alg = padding;
+opt.u.rsa.hash_alg = hash;
+rsa = qcrypto_akcipher_new(&opt, QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE,
+   priv_key, keylen, &error_abort);
+return rsa;
+}
+
+static void test_rsa_speed(const uint8_t *priv_key, size_t keylen,
+   size_t key_size)
+{
+#define BYTE 8
+#define SHA1_DGST_LEN 20
+#define DURATION_SECONDS 10
+#define PADDING QCRYPTO_RSA_PADDING_ALG_PKCS1
+#define HASH QCRYPTO_HASH_ALG_SHA1
+
+g_autoptr(QCryptoAkCipher) rsa =
+create_rsa_akcipher(priv_key, keylen, PADDING, HASH);
+g_autofree uint8_t *dgst = NULL;
+g_autofree uint8_t *signature = NULL;
+size_t count;
+
+dgst = g_new0(uint8_t, SHA1_DGST_LEN);
+memset(dgst, g_test_rand_int(), SHA1_DGST_LEN);
+signature = g_new0(uint8_t, key_size / BYTE);
+
+g_test_message("benchmark rsa%lu (%s-%s) sign in %d seconds", key_size,
+   QCryptoRSAPaddingAlgorithm_str(PADDING),
+   QCryptoHashAlgorithm_str(HASH),
+   DURATION_SECONDS);
+alarm(DURATION_SECONDS);
+g_test_timer_start();
+for (keep_running = true, count = 0; keep_running; ++count) {
+g_assert(qcrypto_akcipher_sign(rsa, dgst, SHA1_DGST_LEN,
+   signature, key_size / BYTE,
+   &error_abort) > 0);
+}
+g_test_timer_elapsed();
+g_test_message("rsa%lu (%s-%s) sign %lu times in %.2f seconds,"
+   " %.2f times/sec ",
+   key_size,  QCryptoRSAPaddingAlgorithm_str(PADDING),
+   QCryptoHashAlgorithm_str(HASH),
+   count, g_test_timer_last(),
+   (double)count / g_test_timer_last());
+
+g_test_message("benchmark rsa%lu (%s-%s) verify in %d seconds", key_size,
+   QCryptoRSAPaddingAlgorithm_str(PADDING),
+   QCryptoHashAlgorithm_str(HASH),
+   DURATION_SECONDS);
+alarm(DURATION_SECONDS);
+g_test_timer_start();
+for (keep_running = true, count = 0; keep_running; ++count) {
+g_assert(qcrypto_akcipher_verify(rsa, signature, key_size / BYTE,
+ dgst, SHA1_DGST_LEN,
+ &error_abort) == 0);
+}
+g_test_timer_elapsed();
+g_test_message("rsa%lu (%s-%s) verify %lu times in %.2f seconds,"
+   " %.2f times/sec ",
+   key_size, QCryptoRSAPaddingAlgorithm_str(PADDING),
+   QCryptoHashAlgorithm_str(HASH),
+   count, g_test_timer_last(),
+   (double)count / g_test_timer_last());
+}
+
+static void test_rsa_1024_speed(const void *opaque)
+{
+size_t key_size = (size_t)opaque;
+test_rsa_speed(rsa1024_priv_key, sizeof(rsa1024_priv_key), key_size);
+}
+
+static void test_rsa_2048_speed(const void *opaque)
+{
+size_t key_size = (size_t)opaque;
+test_rsa_speed(rsa2048_priv_key, sizeof(rsa2048_priv_key), key_size);
+}
+
+static void test_rsa_4096_speed(const void *opaque)
+{
+size_t key_size = (size_t)opaque;
+test_

[PATCH v5 8/9] tests/crypto: Add test suite for RSA keys

[zhenwei pi]

From: Lei He 

As Daniel suggested, Add tests suite for rsakey, as a way to prove
that we can handle DER errors correctly.

Signed-off-by: lei he 
---
 tests/unit/test-crypto-akcipher.c | 285 +-
 1 file changed, 282 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-crypto-akcipher.c 
b/tests/unit/test-crypto-akcipher.c
index b5be563884..4f1f4214dd 100644
--- a/tests/unit/test-crypto-akcipher.c
+++ b/tests/unit/test-crypto-akcipher.c
@@ -517,6 +517,158 @@ static const uint8_t exp_ciphertext_rsa2048_pkcs1[] = {
 0xd0, 0x28, 0x03, 0x19, 0xa6, 0x06, 0x13, 0x45,
 };
 
+static const uint8_t rsa_private_key_lack_element[] = {
+/* RSAPrivateKey, offset: 0, length: 176 */
+0x30, 0x81, 0xb0,
+/* version, offset: 4, length: 1 */
+0x02, 0x01, 0x00,
+/* n, offset: 7, length: 65 */
+0x02, 0x41,
+0x00, 0xb9, 0xe1, 0x22, 0xdb, 0x56, 0x2f, 0xb6,
+0xf7, 0xf0, 0x0a, 0x87, 0x43, 0x07, 0x12, 0xdb,
+0x6d, 0xb6, 0x2b, 0x41, 0x8d, 0x2c, 0x3c, 0xa5,
+0xdd, 0x78, 0x9a, 0x8f, 0xab, 0x8e, 0xf2, 0x4a,
+0xc8, 0x34, 0x0c, 0x12, 0x4f, 0x11, 0x90, 0xc6,
+0xc2, 0xa5, 0xd0, 0xcd, 0xfb, 0xfc, 0x2c, 0x95,
+0x56, 0x82, 0xdf, 0x39, 0xf3, 0x3b, 0x1d, 0x62,
+0x26, 0x97, 0xb7, 0x93, 0x25, 0xc7, 0xec, 0x7e,
+0xf7,
+/* e, offset: 74, length: 3 */
+0x02, 0x03, 0x01, 0x00, 0x01,
+/* d, offset: 79, length: 64 */
+0x02, 0x40,
+0x1e, 0x80, 0xfe, 0xda, 0x65, 0xdb, 0x70, 0xb8,
+0x61, 0x91, 0x28, 0xbf, 0x6c, 0x32, 0xc1, 0x05,
+0xd1, 0x26, 0x6a, 0x1c, 0x83, 0xcc, 0xf4, 0x1f,
+0x53, 0x42, 0x72, 0x1f, 0x62, 0x57, 0x0a, 0xc4,
+0x66, 0x76, 0x30, 0x87, 0xb9, 0xb1, 0xb9, 0x6a,
+0x63, 0xfd, 0x8f, 0x3e, 0xfc, 0x35, 0x3f, 0xd6,
+0x2e, 0x6c, 0xc8, 0x70, 0x8a, 0x17, 0xc1, 0x28,
+0x6a, 0xfe, 0x51, 0x56, 0xb3, 0x92, 0x6f, 0x09,
+/* p, offset: 145, length: 33 */
+0x02, 0x21,
+0x00, 0xe3, 0x2e, 0x2d, 0x8d, 0xba, 0x1c, 0x34,
+0x4c, 0x49, 0x9f, 0xc1, 0xa6, 0xdd, 0xd7, 0x13,
+0x8d, 0x05, 0x48, 0xdd, 0xff, 0x5c, 0x30, 0xbc,
+0x6b, 0xc4, 0x18, 0x9d, 0xfc, 0xa2, 0xd0, 0x9b,
+0x4d,
+/* q, offset: 180, length: 33 */
+0x02, 0x21,
+0x00, 0xd1, 0x75, 0xaf, 0x4b, 0xc6, 0x1a, 0xb0,
+0x98, 0x14, 0x42, 0xae, 0x33, 0xf3, 0x44, 0xde,
+0x21, 0xcb, 0x04, 0xda, 0xfb, 0x1e, 0x35, 0x92,
+0xcd, 0x69, 0xc0, 0x83, 0x06, 0x83, 0x8e, 0x39,
+0x53,
+/* lack element: dp, dq, u */
+};
+
+static const uint8_t rsa_public_key_lack_element[] = {
+/* RSAPublicKey, offset: 0, length: 67 */
+0x30, 0x81, 0x43,
+/* n, offset: 7, length: 65 */
+0x02, 0x41,
+0x00, 0xb9, 0xe1, 0x22, 0xdb, 0x56, 0x2f, 0xb6,
+0xf7, 0xf0, 0x0a, 0x87, 0x43, 0x07, 0x12, 0xdb,
+0x6d, 0xb6, 0x2b, 0x41, 0x8d, 0x2c, 0x3c, 0xa5,
+0xdd, 0x78, 0x9a, 0x8f, 0xab, 0x8e, 0xf2, 0x4a,
+0xc8, 0x34, 0x0c, 0x12, 0x4f, 0x11, 0x90, 0xc6,
+0xc2, 0xa5, 0xd0, 0xcd, 0xfb, 0xfc, 0x2c, 0x95,
+0x56, 0x82, 0xdf, 0x39, 0xf3, 0x3b, 0x1d, 0x62,
+0x26, 0x97, 0xb7, 0x93, 0x25, 0xc7, 0xec, 0x7e,
+0xf7,
+/* lack element: e */
+};
+
+static const uint8_t rsa_public_key_empty_element[] = {
+/* RSAPublicKey, offset: 0, length: 69 */
+0x30, 0x81, 0x45,
+/* n, offset: 7, length: 65 */
+0x02, 0x41,
+0x00, 0xb9, 0xe1, 0x22, 0xdb, 0x56, 0x2f, 0xb6,
+0xf7, 0xf0, 0x0a, 0x87, 0x43, 0x07, 0x12, 0xdb,
+0x6d, 0xb6, 0x2b, 0x41, 0x8d, 0x2c, 0x3c, 0xa5,
+0xdd, 0x78, 0x9a, 0x8f, 0xab, 0x8e, 0xf2, 0x4a,
+0xc8, 0x34, 0x0c, 0x12, 0x4f, 0x11, 0x90, 0xc6,
+0xc2, 0xa5, 0xd0, 0xcd, 0xfb, 0xfc, 0x2c, 0x95,
+0x56, 0x82, 0xdf, 0x39, 0xf3, 0x3b, 0x1d, 0x62,
+0x26, 0x97, 0xb7, 0x93, 0x25, 0xc7, 0xec, 0x7e,
+0xf7,
+/* e: empty element */
+0x02, 0x00,
+};
+
+static const uint8_t rsa_private_key_empty_element[] = {
+/* RSAPrivateKey, offset: 0, length: 19 */
+0x30, 0x81, 0x13,
+/* version, offset: 4, length: 1 */
+0x02, 0x01, 0x00,
+/* n: empty element */
+0x02, 0x00,
+/* e: empty element */
+0x02, 0x00,
+/* d: empty element */
+0x02, 0x00,
+/* p: empty element */
+0x02, 0x00,
+/* q: empty element */
+0x02, 0x00,
+/* dp: empty element */
+0x02, 0x00,
+/* dq: empty element */
+0x02, 0x00,
+/* u: empty element */
+0x02, 0x00,
+};
+
+static const uint8_t rsa_public_key_invalid_length_val[] = {
+/* RSAPublicKey, INVALID length: 313 */
+0x30, 0x82, 0x01, 0x39,
+/* n, offset: 7, length: 65 */
+0x02, 0x41,
+0x00, 0xb9, 0xe1, 0x22, 0xdb, 0x56, 0x2f, 0xb6,
+0xf7, 0xf0, 0x0a, 0x87, 0x43, 0x07, 0x12, 0xdb,
+0x6d, 0xb6, 0x2b, 0x41, 0x8d, 0x2c, 0x3c, 0xa5,
+0xdd, 0x78, 0x9a, 0x8f, 0xab, 0x8e, 0xf2, 0x4a,
+0xc8, 0x34, 0x0c, 0x12, 0x4f, 0x11, 0x90, 0xc6,
+0xc2, 0xa5, 0xd0, 0xcd, 0xfb, 0xfc, 0x2c, 0x95,
+0x56, 0x82, 0xdf, 0x39, 0xf3, 0x3b, 0x1d, 0x62,
+0x26, 0x97, 0xb7, 0x93, 0x25, 0xc7, 0xec, 0x7e,
+0xf7,
+/* e, */
+0x02, 0x03, 0x01, 0x00, 0x01,  /* INTEGER, 

[PATCH v5 6/9] crypto: Implement RSA algorithm by gcrypt

[zhenwei pi]

From: Lei He 

Added gcryt implementation of RSA algorithm, RSA algorithm
implemented by gcrypt has a higher priority than nettle because
it supports raw padding.

Signed-off-by: lei he 
---
 crypto/akcipher-gcrypt.c.inc | 520 +++
 crypto/akcipher.c|   4 +-
 2 files changed, 523 insertions(+), 1 deletion(-)
 create mode 100644 crypto/akcipher-gcrypt.c.inc

diff --git a/crypto/akcipher-gcrypt.c.inc b/crypto/akcipher-gcrypt.c.inc
new file mode 100644
index 00..32ff502f71
--- /dev/null
+++ b/crypto/akcipher-gcrypt.c.inc
@@ -0,0 +1,520 @@
+/*
+ * QEMU Crypto akcipher algorithms
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include 
+
+#include "qemu/osdep.h"
+#include "qemu/host-utils.h"
+#include "crypto/akcipher.h"
+#include "crypto/random.h"
+#include "qapi/error.h"
+#include "sysemu/cryptodev.h"
+#include "rsakey.h"
+
+typedef struct QCryptoGcryptRSA {
+QCryptoAkCipher akcipher;
+gcry_sexp_t key;
+QCryptoRSAPaddingAlgorithm padding_alg;
+QCryptoHashAlgorithm hash_alg;
+} QCryptoGcryptRSA;
+
+static void qcrypto_gcrypt_rsa_free(QCryptoAkCipher *akcipher)
+{
+QCryptoGcryptRSA *rsa = (QCryptoGcryptRSA *)akcipher;
+if (!rsa) {
+return;
+}
+
+gcry_sexp_release(rsa->key);
+g_free(rsa);
+}
+
+static QCryptoGcryptRSA *qcrypto_gcrypt_rsa_new(
+const QCryptoAkCipherOptionsRSA *opt,
+QCryptoAkCipherKeyType type,
+const uint8_t *key,  size_t keylen,
+Error **errp);
+
+QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
+  QCryptoAkCipherKeyType type,
+  const uint8_t *key, size_t keylen,
+  Error **errp)
+{
+switch (opts->alg) {
+case QCRYPTO_AKCIPHER_ALG_RSA:
+return (QCryptoAkCipher *)qcrypto_gcrypt_rsa_new(
+&opts->u.rsa, type, key, keylen, errp);
+
+default:
+error_setg(errp, "Unsupported algorithm: %u", opts->alg);
+return NULL;
+}
+
+return NULL;
+}
+
+static void qcrypto_gcrypt_set_rsa_size(QCryptoAkCipher *akcipher, gcry_mpi_t 
n)
+{
+size_t key_size = (gcry_mpi_get_nbits(n) + 7) / 8;
+akcipher->max_plaintext_len = key_size;
+akcipher->max_ciphertext_len = key_size;
+akcipher->max_dgst_len = key_size;
+akcipher->max_signature_len = key_size;
+}
+
+static int qcrypto_gcrypt_parse_rsa_private_key(
+QCryptoGcryptRSA *rsa,
+const uint8_t *key, size_t keylen)
+{
+g_autoptr(QCryptoAkCipherRSAKey) rsa_key = qcrypto_akcipher_rsakey_parse(
+QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE, key, keylen);
+gcry_mpi_t n = NULL, e = NULL, d = NULL, p = NULL, q = NULL, u = NULL;
+bool compute_mul_inv = false;
+int ret = -1;
+gcry_error_t err;
+
+if (!rsa_key) {
+return ret;
+}
+
+err = gcry_mpi_scan(&n, GCRYMPI_FMT_STD,
+rsa_key->n.data, rsa_key->n.len, NULL);
+if (gcry_err_code(err) != 0) {
+goto cleanup;
+}
+
+err = gcry_mpi_scan(&e, GCRYMPI_FMT_STD,
+rsa_key->e.data, rsa_key->e.len, NULL);
+if (gcry_err_code(err) != 0) {
+goto cleanup;
+}
+
+err = gcry_mpi_scan(&d, GCRYMPI_FMT_STD,
+rsa_key->d.data, rsa_key->d.len, NULL);
+if (gcry_err_code(err) != 0) {
+goto cleanup;
+}
+
+err = gcry_mpi_scan(&p, GCRYMPI_FMT_STD,
+rsa_key->p.data, rsa_key->p.len, NULL);
+if (gcry_err_code(err) != 0) {
+goto cleanup;
+}
+
+err = gcry_mpi_scan(&q, GCRYMPI_FMT_STD,
+rsa_key->q.data, rsa_key->q.len, NULL);
+if (gcry_err_code(err) != 0) {
+goto cleanup;
+}
+
+if (gcry_mpi_cmp_ui(p, 0) > 0 && gcry_mpi_cmp_ui(q, 0) > 0) {
+compute_mul_inv = true;
+
+u = gcry_mpi_new(0);
+if (gcry_mpi_cmp(p, q) > 0) {
+gcry_mpi_swap(p, q);
+}
+gcry_mpi_invm(u, p, q);
+}
+
+if (compute_mul_inv) {
+err = gcry_sexp_build(&rsa->key, NULL,
+"(private-key (rsa (n %m) (e %m) (d %m) (p %m) (q %m) (u %m)))",
+n, e, d, p, q, u);
+} else {
+err = gcry_sexp_build(&rsa->key, NULL,
+"(private-key (r

[PATCH v5 5/9] crypto: Implement RSA algorithm by hogweed

[zhenwei pi]

From: Lei He 

Implement RSA algorithm by hogweed from nettle. Thus QEMU supports
a 'real' RSA backend to handle request from guest side. It's
important to test RSA offload case without OS & hardware requirement.

Signed-off-by: lei he 
Signed-off-by: zhenwei pi 
---
 crypto/akcipher-nettle.c.inc | 432 +++
 crypto/akcipher.c|   4 +
 crypto/meson.build   |   4 +
 crypto/rsakey-builtin.c.inc  | 209 +
 crypto/rsakey-nettle.c.inc   | 154 +
 crypto/rsakey.c  |  44 
 crypto/rsakey.h  |  94 
 meson.build  |  11 +
 8 files changed, 952 insertions(+)
 create mode 100644 crypto/akcipher-nettle.c.inc
 create mode 100644 crypto/rsakey-builtin.c.inc
 create mode 100644 crypto/rsakey-nettle.c.inc
 create mode 100644 crypto/rsakey.c
 create mode 100644 crypto/rsakey.h

diff --git a/crypto/akcipher-nettle.c.inc b/crypto/akcipher-nettle.c.inc
new file mode 100644
index 00..1f688aa0f1
--- /dev/null
+++ b/crypto/akcipher-nettle.c.inc
@@ -0,0 +1,432 @@
+/*
+ * QEMU Crypto akcipher algorithms
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include 
+
+#include "qemu/osdep.h"
+#include "qemu/host-utils.h"
+#include "crypto/akcipher.h"
+#include "crypto/random.h"
+#include "qapi/error.h"
+#include "sysemu/cryptodev.h"
+#include "rsakey.h"
+
+typedef struct QCryptoNettleRSA {
+QCryptoAkCipher akcipher;
+struct rsa_public_key pub;
+struct rsa_private_key priv;
+QCryptoRSAPaddingAlgorithm padding_alg;
+QCryptoHashAlgorithm hash_alg;
+} QCryptoNettleRSA;
+
+static void qcrypto_nettle_rsa_free(QCryptoAkCipher *akcipher)
+{
+QCryptoNettleRSA *rsa = (QCryptoNettleRSA *)akcipher;
+if (!rsa) {
+return;
+}
+
+rsa_public_key_clear(&rsa->pub);
+rsa_private_key_clear(&rsa->priv);
+g_free(rsa);
+}
+
+static QCryptoAkCipher *qcrypto_nettle_rsa_new(
+const QCryptoAkCipherOptionsRSA *opt,
+QCryptoAkCipherKeyType type,
+const uint8_t *key,  size_t keylen,
+Error **errp);
+
+QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
+  QCryptoAkCipherKeyType type,
+  const uint8_t *key, size_t keylen,
+  Error **errp)
+{
+switch (opts->alg) {
+case QCRYPTO_AKCIPHER_ALG_RSA:
+return qcrypto_nettle_rsa_new(&opts->u.rsa, type, key, keylen, errp);
+
+default:
+error_setg(errp, "Unsupported algorithm: %u", opts->alg);
+return NULL;
+}
+
+return NULL;
+}
+
+static void qcrypto_nettle_rsa_set_akcipher_size(QCryptoAkCipher *akcipher,
+ int key_size)
+{
+akcipher->max_plaintext_len = key_size;
+akcipher->max_ciphertext_len = key_size;
+akcipher->max_signature_len = key_size;
+akcipher->max_dgst_len = key_size;
+}
+
+static int qcrypt_nettle_parse_rsa_private_key(QCryptoNettleRSA *rsa,
+   const uint8_t *key,
+   size_t keylen)
+{
+g_autoptr(QCryptoAkCipherRSAKey) rsa_key = qcrypto_akcipher_rsakey_parse(
+QCRYPTO_AKCIPHER_KEY_TYPE_PRIVATE, key, keylen);
+
+if (!rsa_key) {
+return -1;
+}
+
+nettle_mpz_init_set_str_256_u(rsa->pub.n, rsa_key->n.len, rsa_key->n.data);
+nettle_mpz_init_set_str_256_u(rsa->pub.e, rsa_key->e.len, rsa_key->e.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.d, rsa_key->d.len, 
rsa_key->d.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.p, rsa_key->p.len, 
rsa_key->p.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.q, rsa_key->q.len, 
rsa_key->q.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.a, rsa_key->dp.len,
+  rsa_key->dp.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.b, rsa_key->dq.len,
+  rsa_key->dq.data);
+nettle_mpz_init_set_str_256_u(rsa->priv.c, rsa_key->u.len, 
rsa_key->u.data);
+
+if (!rsa_public_key_prepare(&rsa->pub)) {
+return -1;
+}
+
+/**
+ * Since in the kernel's unit test, the p, q, a, b, c of some
+ * private keys is 0, only the simplest length check is don

[PATCH v5 4/9] crypto: add ASN.1 DER decoder

[zhenwei pi]

From: Lei He 

Add an ANS.1 DER decoder which is used to parse asymmetric
cipher keys

Signed-off-by: zhenwei pi 
Signed-off-by: lei he 
---
 crypto/der.c | 190 +++
 crypto/der.h |  82 ++
 crypto/meson.build   |   1 +
 tests/unit/meson.build   |   1 +
 tests/unit/test-crypto-der.c | 290 +++
 5 files changed, 564 insertions(+)
 create mode 100644 crypto/der.c
 create mode 100644 crypto/der.h
 create mode 100644 tests/unit/test-crypto-der.c

diff --git a/crypto/der.c b/crypto/der.c
new file mode 100644
index 00..7907bcfd51
--- /dev/null
+++ b/crypto/der.c
@@ -0,0 +1,190 @@
+/*
+ * QEMU Crypto ASN.1 DER decoder
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: lei he 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include 
+#include 
+
+#include "crypto/der.h"
+
+enum QCryptoDERTypeTag {
+QCRYPTO_DER_TYPE_TAG_BOOL = 0x1,
+QCRYPTO_DER_TYPE_TAG_INT = 0x2,
+QCRYPTO_DER_TYPE_TAG_BIT_STR = 0x3,
+QCRYPTO_DER_TYPE_TAG_OCT_STR = 0x4,
+QCRYPTO_DER_TYPE_TAG_OCT_NULL = 0x5,
+QCRYPTO_DER_TYPE_TAG_OCT_OID = 0x6,
+QCRYPTO_DER_TYPE_TAG_SEQ = 0x10,
+QCRYPTO_DER_TYPE_TAG_SET = 0x11,
+};
+
+#define QCRYPTO_DER_CONSTRUCTED_MASK 0x20
+#define QCRYPTO_DER_SHORT_LEN_MASK 0x80
+
+static uint8_t qcrypto_der_peek_byte(const uint8_t **data, size_t *dlen)
+{
+return **data;
+}
+
+static void qcrypto_der_cut_nbytes(const uint8_t **data,
+   size_t *dlen,
+   size_t nbytes)
+{
+*data += nbytes;
+*dlen -= nbytes;
+}
+
+static uint8_t qcrypto_der_cut_byte(const uint8_t **data, size_t *dlen)
+{
+uint8_t val = qcrypto_der_peek_byte(data, dlen);
+
+qcrypto_der_cut_nbytes(data, dlen, 1);
+
+return val;
+}
+
+static int qcrypto_der_invoke_callback(DERDecodeCb cb, void *ctx,
+   const uint8_t *value, size_t vlen,
+   Error **errp)
+{
+if (!cb) {
+return 0;
+}
+
+return cb(ctx, value, vlen, errp);
+}
+
+static int qcrypto_der_extract_definite_data(const uint8_t **data, size_t 
*dlen,
+ DERDecodeCb cb, void *ctx,
+ Error **errp)
+{
+const uint8_t *value;
+size_t vlen = 0;
+uint8_t byte_count = qcrypto_der_cut_byte(data, dlen);
+
+/* short format of definite-length */
+if (!(byte_count & QCRYPTO_DER_SHORT_LEN_MASK)) {
+if (byte_count > *dlen) {
+error_setg(errp, "Invalid content length: %u", byte_count);
+return -1;
+}
+
+value = *data;
+vlen = byte_count;
+qcrypto_der_cut_nbytes(data, dlen, vlen);
+
+if (qcrypto_der_invoke_callback(cb, ctx, value, vlen, errp) != 0) {
+return -1;
+}
+return vlen;
+}
+
+/* Ignore highest bit */
+byte_count &= ~QCRYPTO_DER_SHORT_LEN_MASK;
+
+/*
+ * size_t is enough to store the value of length, although the DER
+ * encoding standard supports larger length.
+ */
+if (byte_count > sizeof(size_t)) {
+error_setg(errp, "Invalid byte count of content length: %u",
+   byte_count);
+return -1;
+}
+
+if (*dlen < byte_count) {
+error_setg(errp, "Invalid content length: %u", byte_count);
+return -1;
+}
+while (byte_count--) {
+vlen <<= 8;
+vlen += qcrypto_der_cut_byte(data, dlen);
+}
+
+if (vlen > *dlen) {
+error_setg(errp, "Invalid content length: %lu", vlen);
+return -1;
+}
+
+value = *data;
+qcrypto_der_cut_nbytes(data, dlen, vlen);
+
+if (qcrypto_der_invoke_callback(cb, ctx, value, vlen, errp) != 0) {
+return -1;
+}
+return vlen;
+}
+
+static int qcrypto_der_extract_data(const uint8_t **data, size_t *dlen,
+DERDecodeCb cb, void *ctx, Error **errp)
+{
+uint8_t val;
+if (*dlen < 1) {
+error_setg(errp, "Need more data");
+return -1;
+}
+val = qcrypto_der_peek_byte(data, dlen);
+
+/* must use definite length format */
+if (val == QCRYPTO_DER_SHORT_LEN_MASK) {
+error_setg(errp, "Only definite length format is allowed");
+  

[PATCH v5 3/9] crypto: Introduce akcipher crypto class

[zhenwei pi]

Introduce new akcipher crypto class 'QCryptoAkCIpher', which supports
basic asymmetric operations: encrypt, decrypt, sign and verify.

Suggested by Daniel P. Berrangé, also add autoptr cleanup for the new
class. Thanks to Daniel!

Co-developed-by: lei he 
Signed-off-by: lei he 
Signed-off-by: zhenwei pi 
---
 crypto/akcipher.c | 102 
 crypto/akcipherpriv.h |  55 +
 crypto/meson.build|   1 +
 include/crypto/akcipher.h | 158 ++
 4 files changed, 316 insertions(+)
 create mode 100644 crypto/akcipher.c
 create mode 100644 crypto/akcipherpriv.h
 create mode 100644 include/crypto/akcipher.h

diff --git a/crypto/akcipher.c b/crypto/akcipher.c
new file mode 100644
index 00..ab28bf415b
--- /dev/null
+++ b/crypto/akcipher.c
@@ -0,0 +1,102 @@
+/*
+ * QEMU Crypto akcipher algorithms
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: zhenwei pi 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "crypto/akcipher.h"
+#include "akcipherpriv.h"
+
+QCryptoAkCipher *qcrypto_akcipher_new(const QCryptoAkCipherOptions *opts,
+  QCryptoAkCipherKeyType type,
+  const uint8_t *key, size_t keylen,
+  Error **errp)
+{
+QCryptoAkCipher *akcipher = NULL;
+
+return akcipher;
+}
+
+bool qcrypto_akcipher_supports(QCryptoAkCipherOptions *opts)
+{
+return false;
+}
+
+int qcrypto_akcipher_encrypt(QCryptoAkCipher *akcipher,
+ const void *in, size_t in_len,
+ void *out, size_t out_len, Error **errp)
+{
+const QCryptoAkCipherDriver *drv = akcipher->driver;
+
+return drv->encrypt(akcipher, in, in_len, out, out_len, errp);
+}
+
+int qcrypto_akcipher_decrypt(QCryptoAkCipher *akcipher,
+ const void *in, size_t in_len,
+ void *out, size_t out_len, Error **errp)
+{
+const QCryptoAkCipherDriver *drv = akcipher->driver;
+
+return drv->decrypt(akcipher, in, in_len, out, out_len, errp);
+}
+
+int qcrypto_akcipher_sign(QCryptoAkCipher *akcipher,
+  const void *in, size_t in_len,
+  void *out, size_t out_len, Error **errp)
+{
+const QCryptoAkCipherDriver *drv = akcipher->driver;
+
+return drv->sign(akcipher, in, in_len, out, out_len, errp);
+}
+
+int qcrypto_akcipher_verify(QCryptoAkCipher *akcipher,
+const void *in, size_t in_len,
+const void *in2, size_t in2_len, Error **errp)
+{
+const QCryptoAkCipherDriver *drv = akcipher->driver;
+
+return drv->verify(akcipher, in, in_len, in2, in2_len, errp);
+}
+
+int qcrypto_akcipher_max_plaintext_len(QCryptoAkCipher *akcipher)
+{
+return akcipher->max_plaintext_len;
+}
+
+int qcrypto_akcipher_max_ciphertext_len(QCryptoAkCipher *akcipher)
+{
+return akcipher->max_ciphertext_len;
+}
+
+int qcrypto_akcipher_max_signature_len(QCryptoAkCipher *akcipher)
+{
+return akcipher->max_signature_len;
+}
+
+int qcrypto_akcipher_max_dgst_len(QCryptoAkCipher *akcipher)
+{
+return akcipher->max_dgst_len;
+}
+
+void qcrypto_akcipher_free(QCryptoAkCipher *akcipher)
+{
+const QCryptoAkCipherDriver *drv = akcipher->driver;
+
+drv->free(akcipher);
+}
diff --git a/crypto/akcipherpriv.h b/crypto/akcipherpriv.h
new file mode 100644
index 00..739f639bcf
--- /dev/null
+++ b/crypto/akcipherpriv.h
@@ -0,0 +1,55 @@
+/*
+ * QEMU Crypto asymmetric algorithms
+ *
+ * Copyright (c) 2022 Bytedance
+ * Author: zhenwei pi 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .

[PATCH v5 2/9] qapi: crypto-akcipher: Introduce akcipher types to qapi

[zhenwei pi]

From: Lei He 

Introduce akcipher types, also include RSA related types.

Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Lei He 
Signed-off-by: zhenwei pi 
---
 qapi/crypto.json | 64 
 1 file changed, 64 insertions(+)

diff --git a/qapi/crypto.json b/qapi/crypto.json
index 1ec54c15ca..f7bb9a42d0 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -540,3 +540,67 @@
   'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
 '*sanity-check': 'bool',
 '*passwordid': 'str' } }
+##
+# @QCryptoAkCipherAlgorithm:
+#
+# The supported algorithms for asymmetric encryption ciphers
+#
+# @rsa: RSA algorithm
+#
+# Since: 7.1
+##
+{ 'enum': 'QCryptoAkCipherAlgorithm',
+  'prefix': 'QCRYPTO_AKCIPHER_ALG',
+  'data': ['rsa']}
+
+##
+# @QCryptoAkCipherKeyType:
+#
+# The type of asymmetric keys.
+#
+# Since: 7.1
+##
+{ 'enum': 'QCryptoAkCipherKeyType',
+  'prefix': 'QCRYPTO_AKCIPHER_KEY_TYPE',
+  'data': ['public', 'private']}
+
+##
+# @QCryptoRSAPaddingAlgorithm:
+#
+# The padding algorithm for RSA.
+#
+# @raw: no padding used
+# @pkcs1: pkcs1#v1.5
+#
+# Since: 7.1
+##
+{ 'enum': 'QCryptoRSAPaddingAlgorithm',
+  'prefix': 'QCRYPTO_RSA_PADDING_ALG',
+  'data': ['raw', 'pkcs1']}
+
+##
+# @QCryptoAkCipherOptionsRSA:
+#
+# Specific parameters for RSA algorithm.
+#
+# @hash-alg: QCryptoHashAlgorithm
+# @padding-alg: QCryptoRSAPaddingAlgorithm
+#
+# Since: 7.1
+##
+{ 'struct': 'QCryptoAkCipherOptionsRSA',
+  'data': { 'hash-alg':'QCryptoHashAlgorithm',
+'padding-alg': 'QCryptoRSAPaddingAlgorithm'}}
+
+##
+# @QCryptoAkCipherOptions:
+#
+# The options that are available for all asymmetric key algorithms
+# when creating a new QCryptoAkCipher.
+#
+# Since: 7.1
+##
+{ 'union': 'QCryptoAkCipherOptions',
+  'base': { 'alg': 'QCryptoAkCipherAlgorithm' },
+  'discriminator': 'alg',
+  'data': { 'rsa': 'QCryptoAkCipherOptionsRSA' }}
-- 
2.20.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v5 1/9] virtio-crypto: header update

[zhenwei pi]

Update header from linux, support akcipher service.

Reviewed-by: Gonglei 
Signed-off-by: lei he 
Signed-off-by: zhenwei pi 
---
 .../standard-headers/linux/virtio_crypto.h| 82 ++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/linux/virtio_crypto.h 
b/include/standard-headers/linux/virtio_crypto.h
index 5ff0b4ee59..68066dafb6 100644
--- a/include/standard-headers/linux/virtio_crypto.h
+++ b/include/standard-headers/linux/virtio_crypto.h
@@ -37,6 +37,7 @@
 #define VIRTIO_CRYPTO_SERVICE_HASH   1
 #define VIRTIO_CRYPTO_SERVICE_MAC2
 #define VIRTIO_CRYPTO_SERVICE_AEAD   3
+#define VIRTIO_CRYPTO_SERVICE_AKCIPHER 4
 
 #define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
 
@@ -57,6 +58,10 @@ struct virtio_crypto_ctrl_header {
   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
 #define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
+#define VIRTIO_CRYPTO_AKCIPHER_CREATE_SESSION \
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x04)
+#define VIRTIO_CRYPTO_AKCIPHER_DESTROY_SESSION \
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x05)
uint32_t opcode;
uint32_t algo;
uint32_t flag;
@@ -180,6 +185,58 @@ struct virtio_crypto_aead_create_session_req {
uint8_t padding[32];
 };
 
+struct virtio_crypto_rsa_session_para {
+#define VIRTIO_CRYPTO_RSA_RAW_PADDING   0
+#define VIRTIO_CRYPTO_RSA_PKCS1_PADDING 1
+   uint32_t padding_algo;
+
+#define VIRTIO_CRYPTO_RSA_NO_HASH   0
+#define VIRTIO_CRYPTO_RSA_MD2   1
+#define VIRTIO_CRYPTO_RSA_MD3   2
+#define VIRTIO_CRYPTO_RSA_MD4   3
+#define VIRTIO_CRYPTO_RSA_MD5   4
+#define VIRTIO_CRYPTO_RSA_SHA1  5
+#define VIRTIO_CRYPTO_RSA_SHA2566
+#define VIRTIO_CRYPTO_RSA_SHA3847
+#define VIRTIO_CRYPTO_RSA_SHA5128
+#define VIRTIO_CRYPTO_RSA_SHA2249
+   uint32_t hash_algo;
+};
+
+struct virtio_crypto_ecdsa_session_para {
+#define VIRTIO_CRYPTO_CURVE_UNKNOWN   0
+#define VIRTIO_CRYPTO_CURVE_NIST_P192 1
+#define VIRTIO_CRYPTO_CURVE_NIST_P224 2
+#define VIRTIO_CRYPTO_CURVE_NIST_P256 3
+#define VIRTIO_CRYPTO_CURVE_NIST_P384 4
+#define VIRTIO_CRYPTO_CURVE_NIST_P521 5
+   uint32_t curve_id;
+   uint32_t padding;
+};
+
+struct virtio_crypto_akcipher_session_para {
+#define VIRTIO_CRYPTO_NO_AKCIPHER0
+#define VIRTIO_CRYPTO_AKCIPHER_RSA   1
+#define VIRTIO_CRYPTO_AKCIPHER_DSA   2
+#define VIRTIO_CRYPTO_AKCIPHER_ECDSA 3
+   uint32_t algo;
+
+#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PUBLIC  1
+#define VIRTIO_CRYPTO_AKCIPHER_KEY_TYPE_PRIVATE 2
+   uint32_t keytype;
+   uint32_t keylen;
+
+   union {
+   struct virtio_crypto_rsa_session_para rsa;
+   struct virtio_crypto_ecdsa_session_para ecdsa;
+   } u;
+};
+
+struct virtio_crypto_akcipher_create_session_req {
+   struct virtio_crypto_akcipher_session_para para;
+   uint8_t padding[36];
+};
+
 struct virtio_crypto_alg_chain_session_para {
 #define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
 #define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
@@ -247,6 +304,8 @@ struct virtio_crypto_op_ctrl_req {
mac_create_session;
struct virtio_crypto_aead_create_session_req
aead_create_session;
+   struct virtio_crypto_akcipher_create_session_req
+   akcipher_create_session;
struct virtio_crypto_destroy_session_req
destroy_session;
uint8_t padding[56];
@@ -266,6 +325,14 @@ struct virtio_crypto_op_header {
VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
 #define VIRTIO_CRYPTO_AEAD_DECRYPT \
VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
+#define VIRTIO_CRYPTO_AKCIPHER_ENCRYPT \
+   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x00)
+#define VIRTIO_CRYPTO_AKCIPHER_DECRYPT \
+   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x01)
+#define VIRTIO_CRYPTO_AKCIPHER_SIGN \
+   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x02)
+#define VIRTIO_CRYPTO_AKCIPHER_VERIFY \
+   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x03)
uint32_t opcode;
/* algo should be service-specific algorithms */
uint32_t algo;
@@ -390,6 +457,16 @@ struct virtio_crypto_aead_data_req {
uint8_t padding[32];
 };
 
+struct virtio_crypto_akcipher_para {
+   uint32_t src_data_len;
+   uint32_t dst_data_len;
+};
+
+struct virtio_crypto_akcipher_data_req {
+   struct virtio_crypto_akcipher_para para;
+   uint8_t padding[40];
+};
+
 /* The request of the data virtqueue's packet */
 struct virtio_crypto_op_data_req {
struct virtio_crypto_op_header header;
@@ -399,6 +476,7 @@ struct virtio_crypto_op_data_req {
struct virtio_crypto_hash_data_req hash_req;
struc

[PATCH v5 0/9] Introduce akcipher service for virtio-crypto

[zhenwei pi]

Hi, Lei & MST

Daniel has started to review the akcipher framework and nettle & gcrypt
implementation, this part seems to be ready soon. Thanks a lot to Daniel!

And the last patch "crypto: Introduce RSA algorithm" handles akcipher
requests from guest and uses the new akcipher service. The new feature
can be used to test by the builtin driver. I would appreciate it if you
could review patch.

v4 -> v5:
- Move QCryptoAkCipher into akcipherpriv.h, and modify the related comments.
- Rename asn1_decoder.c to der.c.
- Code style fix: use 'cleanup' & 'error' lables.
- Allow autoptr type to auto-free.
- Add test cases for rsakey to handle DER error.
- Other minor fixes.

v3 -> v4:
- Coding style fix: Akcipher -> AkCipher, struct XXX -> XXX, Rsa -> RSA,
XXX-alg -> XXX-algo.
- Change version info in qapi/crypto.json, from 7.0 -> 7.1.
- Remove ecdsa from qapi/crypto.json, it would be introduced with the 
implemetion later.
- Use QCryptoHashAlgothrim instead of QCryptoRSAHashAlgorithm(removed) in 
qapi/crypto.json.
- Rename arguments of qcrypto_akcipher_XXX to keep aligned with 
qcrypto_cipher_XXX(dec/enc/sign/vefiry -> in/out/in2), and add 
qcrypto_akcipher_max_XXX APIs.
- Add new API: qcrypto_akcipher_supports.
- Change the return value of qcrypto_akcipher_enc/dec/sign, these functions 
return the actual length of result.
- Separate ASN.1 source code and test case clean.
- Disable RSA raw encoding for akcipher-nettle.
- Separate RSA key parser into rsakey.{hc}, and implememts it with 
builtin-asn1-decoder and nettle respectivly.
- Implement RSA(pkcs1 and raw encoding) algorithm by gcrypt. This has higher 
priority than nettle.
- For some akcipher operations(eg, decryption of pkcs1pad(rsa)), the length of 
returned result maybe less than the dst buffer size, return the actual length 
of result instead of the buffer length to the guest side. (in function 
virtio_crypto_akcipher_input_data_helper)
- Other minor changes.

Thanks to Daniel!

Eric pointed out this missing part of use case, send it here again.

In our plan, the feature is designed for HTTPS offloading case and other 
applications which use kernel RSA/ecdsa by keyctl syscall. The full picture 
shows bellow:


  Nginx/openssl[1] ... Apps
Guest   -
   virtio-crypto driver[2]
-
   virtio-crypto backend[3]
Host-
  /  |  \
  builtin[4]   vhost keyctl[5] ...


[1] User applications can offload RSA calculation to kernel by keyctl syscall. 
There is no keyctl engine in openssl currently, we developed a engine and tried 
to contribute it to openssl upstream, but openssl 1.x does not accept new 
feature. Link:
https://github.com/openssl/openssl/pull/16689

This branch is available and maintained by Lei 
https://github.com/TousakaRin/openssl/tree/OpenSSL_1_1_1-kctl_engine

We tested nginx(change config file only) with openssl keyctl engine, it works 
fine.

[2] virtio-crypto driver is used to communicate with host side, send requests 
to host side to do asymmetric calculation.
https://lkml.org/lkml/2022/3/1/1425

[3] virtio-crypto backend handles requests from guest side, and forwards 
request to crypto backend driver of QEMU.

[4] Currently RSA is supported only in builtin driver. This driver is supposed 
to test the full feature without other software(Ex vhost process) and hardware 
dependence. ecdsa is introduced into qapi type without implementation, this may 
be implemented in Q3-2022 or later. If ecdsa type definition should be added 
with the implementation together, I'll remove this in next version.

[5] keyctl backend is in development, we will post this feature in Q2-2022. 
keyctl backend can use hardware acceleration(Ex, Intel QAT).

Setup the full environment, tested with Intel QAT on host side, the QPS of 
HTTPS increase to ~200% in a guest.

VS PCI passthrough: the most important benefit of this solution makes the VM 
migratable.

v2 -> v3:
- Introduce akcipher types to qapi
- Add test/benchmark suite for akcipher class
- Seperate 'virtio_crypto: Support virtio crypto asym operation' into:
  - crypto: Introduce akcipher crypto class
  - virtio-crypto: Introduce RSA algorithm

v1 -> v2:
- Update virtio_crypto.h from v2 version of related kernel patch.

v1:
- Support akcipher for virtio-crypto.
- Introduce akcipher class.
- Introduce ASN1 decoder into QEMU.
- Implement RSA backend by nettle/hogweed.

Lei He (6):
  qapi: crypto-akcipher: Introduce akcipher types to qapi
  crypto: add ASN.1 DER decoder
  crypto: Implement RSA algorithm by hogweed
  crypto: Implement RSA algorithm by gcrypt
  test/crypto: Add test suite for crypto akcipher
  tests/crypto: Add test suite for RSA keys

Zhenwei Pi (3):
  virtio-crypto: header update
  crypto: Introduce akcipher crypto class
  crypto: Introduce RSA algorithm

 backends/cryptod

[PATCH net-next 2/2] vsock/virtio: add support for device suspend/resume

[Stefano Garzarella]

Implement .freeze and .restore callbacks of struct virtio_driver
to support device suspend/resume.

During suspension all connected sockets are reset and VQs deleted.
During resume the VQs are re-initialized.

Reported by: Vilas R K 
Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 47 
 1 file changed, 47 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 31f4f6f40614..ad64f403536a 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -743,6 +743,49 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
kfree(vsock);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int virtio_vsock_freeze(struct virtio_device *vdev)
+{
+   struct virtio_vsock *vsock = vdev->priv;
+
+   mutex_lock(&the_virtio_vsock_mutex);
+
+   rcu_assign_pointer(the_virtio_vsock, NULL);
+   synchronize_rcu();
+
+   virtio_vsock_vqs_del(vsock);
+
+   mutex_unlock(&the_virtio_vsock_mutex);
+
+   return 0;
+}
+
+static int virtio_vsock_restore(struct virtio_device *vdev)
+{
+   struct virtio_vsock *vsock = vdev->priv;
+   int ret;
+
+   mutex_lock(&the_virtio_vsock_mutex);
+
+   /* Only one virtio-vsock device per guest is supported */
+   if (rcu_dereference_protected(the_virtio_vsock,
+   lockdep_is_held(&the_virtio_vsock_mutex))) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   ret = virtio_vsock_vqs_init(vsock);
+   if (ret < 0)
+   goto out;
+
+   rcu_assign_pointer(the_virtio_vsock, vsock);
+
+out:
+   mutex_unlock(&the_virtio_vsock_mutex);
+   return ret;
+}
+#endif /* CONFIG_PM_SLEEP */
+
 static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_VSOCK, VIRTIO_DEV_ANY_ID },
{ 0 },
@@ -760,6 +803,10 @@ static struct virtio_driver virtio_vsock_driver = {
.id_table = id_table,
.probe = virtio_vsock_probe,
.remove = virtio_vsock_remove,
+#ifdef CONFIG_PM_SLEEP
+   .freeze = virtio_vsock_freeze,
+   .restore = virtio_vsock_restore,
+#endif
 };
 
 static int __init virtio_vsock_init(void)
-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net-next 1/2] vsock/virtio: factor our the code to initialize and delete VQs

[Stefano Garzarella]

Add virtio_vsock_vqs_init() and virtio_vsock_vqs_del() with the code
that was in virtio_vsock_probe() and virtio_vsock_remove to initialize
and delete VQs.

These new functions will be used in the next commit to support device
suspend/resume

Signed-off-by: Stefano Garzarella 
---
 net/vmw_vsock/virtio_transport.c | 150 +--
 1 file changed, 84 insertions(+), 66 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index ba1c8cc0c467..31f4f6f40614 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -566,67 +566,28 @@ static void virtio_transport_rx_work(struct work_struct 
*work)
mutex_unlock(&vsock->rx_lock);
 }
 
-static int virtio_vsock_probe(struct virtio_device *vdev)
+static int virtio_vsock_vqs_init(struct virtio_vsock *vsock)
 {
-   vq_callback_t *callbacks[] = {
-   virtio_vsock_rx_done,
-   virtio_vsock_tx_done,
-   virtio_vsock_event_done,
-   };
+   struct virtio_device *vdev = vsock->vdev;
static const char * const names[] = {
"rx",
"tx",
"event",
};
-   struct virtio_vsock *vsock = NULL;
+   vq_callback_t *callbacks[] = {
+   virtio_vsock_rx_done,
+   virtio_vsock_tx_done,
+   virtio_vsock_event_done,
+   };
int ret;
 
-   ret = mutex_lock_interruptible(&the_virtio_vsock_mutex);
-   if (ret)
-   return ret;
-
-   /* Only one virtio-vsock device per guest is supported */
-   if (rcu_dereference_protected(the_virtio_vsock,
-   lockdep_is_held(&the_virtio_vsock_mutex))) {
-   ret = -EBUSY;
-   goto out;
-   }
-
-   vsock = kzalloc(sizeof(*vsock), GFP_KERNEL);
-   if (!vsock) {
-   ret = -ENOMEM;
-   goto out;
-   }
-
-   vsock->vdev = vdev;
-
-   ret = virtio_find_vqs(vsock->vdev, VSOCK_VQ_MAX,
- vsock->vqs, callbacks, names,
+   ret = virtio_find_vqs(vdev, VSOCK_VQ_MAX, vsock->vqs, callbacks, names,
  NULL);
if (ret < 0)
-   goto out;
+   return ret;
 
virtio_vsock_update_guest_cid(vsock);
 
-   vsock->rx_buf_nr = 0;
-   vsock->rx_buf_max_nr = 0;
-   atomic_set(&vsock->queued_replies, 0);
-
-   mutex_init(&vsock->tx_lock);
-   mutex_init(&vsock->rx_lock);
-   mutex_init(&vsock->event_lock);
-   spin_lock_init(&vsock->send_pkt_list_lock);
-   INIT_LIST_HEAD(&vsock->send_pkt_list);
-   INIT_WORK(&vsock->rx_work, virtio_transport_rx_work);
-   INIT_WORK(&vsock->tx_work, virtio_transport_tx_work);
-   INIT_WORK(&vsock->event_work, virtio_transport_event_work);
-   INIT_WORK(&vsock->send_pkt_work, virtio_transport_send_pkt_work);
-
-   if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_SEQPACKET))
-   vsock->seqpacket_allow = true;
-
-   vdev->priv = vsock;
-
virtio_device_ready(vdev);
 
mutex_lock(&vsock->tx_lock);
@@ -643,30 +604,15 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
vsock->event_run = true;
mutex_unlock(&vsock->event_lock);
 
-   rcu_assign_pointer(the_virtio_vsock, vsock);
-
-   mutex_unlock(&the_virtio_vsock_mutex);
-
return 0;
-
-out:
-   kfree(vsock);
-   mutex_unlock(&the_virtio_vsock_mutex);
-   return ret;
 }
 
-static void virtio_vsock_remove(struct virtio_device *vdev)
+static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
 {
-   struct virtio_vsock *vsock = vdev->priv;
+   struct virtio_device *vdev = vsock->vdev;
struct virtio_vsock_pkt *pkt;
 
-   mutex_lock(&the_virtio_vsock_mutex);
-
-   vdev->priv = NULL;
-   rcu_assign_pointer(the_virtio_vsock, NULL);
-   synchronize_rcu();
-
-   /* Reset all connected sockets when the device disappear */
+   /* Reset all connected sockets when the VQs disappear */
vsock_for_each_connected_socket(&virtio_transport.transport,
virtio_vsock_reset_sock);
 
@@ -711,6 +657,78 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 
/* Delete virtqueues and flush outstanding callbacks if any */
vdev->config->del_vqs(vdev);
+}
+
+static int virtio_vsock_probe(struct virtio_device *vdev)
+{
+   struct virtio_vsock *vsock = NULL;
+   int ret;
+
+   ret = mutex_lock_interruptible(&the_virtio_vsock_mutex);
+   if (ret)
+   return ret;
+
+   /* Only one virtio-vsock device per guest is supported */
+   if (rcu_dereference_protected(the_virtio_vsock,
+   lockdep_is_held(&the_virtio_vsock_mutex))) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   vsock = kzalloc(sizeof(*vsock), GFP_KERNEL);
+   if (!vsock) {

[PATCH net-next 0/2] vsock/virtio: add support for device suspend/resume

[Stefano Garzarella]

Vilas reported that virtio-vsock no longer worked properly after
suspend/resume (echo mem >/sys/power/state).
It was impossible to connect to the host and vice versa.

Indeed, the support has never been implemented.

This series implement .freeze and .restore callbacks of struct virtio_driver
to support device suspend/resume.

The first patch factors our the code to initialize and delete VQs.
The second patch uses that code to support device suspend/resume.

Signed-off-by: Stefano Garzarella 

Stefano Garzarella (2):
  vsock/virtio: factor our the code to initialize and delete VQs
  vsock/virtio: add support for device suspend/resume

 net/vmw_vsock/virtio_transport.c | 197 ---
 1 file changed, 131 insertions(+), 66 deletions(-)

-- 
2.35.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v5 15/17] drm/shmem-helper: Make drm_gem_shmem_get_pages() private

[Daniel Vetter]

On Sun, Apr 24, 2022 at 10:04:22PM +0300, Dmitry Osipenko wrote:
> VirtIO-GPU driver was the only user of drm_gem_shmem_get_pages()
> and it now uses drm_gem_shmem_get_pages_sgt(). Make the get_pages()
> private to drm_gem_shmem_helper.
> 
> Signed-off-by: Dmitry Osipenko 
> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 3 +--
>  include/drm/drm_gem_shmem_helper.h | 1 -
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
> b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 25e9bc2803ee..7ec5f8002f68 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -490,7 +490,7 @@ static int drm_gem_shmem_get_pages_locked(struct 
> drm_gem_shmem_object *shmem)
>   * Returns:
>   * 0 on success or a negative error code on failure.
>   */

We also delete the kerneldoc for functions not exported (kerneldoc is
geared towards driver writes). If there's anything critical the comment
explains about the internals, you can keep that as a normal C style
comment without the /** but generally there's no need for these anymore.
-Daniel

> -int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
> +static int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem)
>  {
>   int ret;
>  
> @@ -507,7 +507,6 @@ int drm_gem_shmem_get_pages(struct drm_gem_shmem_object 
> *shmem)
>  
>   return ret;
>  }
> -EXPORT_SYMBOL(drm_gem_shmem_get_pages);
>  
>  static void drm_gem_shmem_get_pages_no_fail(struct drm_gem_shmem_object 
> *shmem)
>  {
> diff --git a/include/drm/drm_gem_shmem_helper.h 
> b/include/drm/drm_gem_shmem_helper.h
> index 638cb16a4576..5b351933c293 100644
> --- a/include/drm/drm_gem_shmem_helper.h
> +++ b/include/drm/drm_gem_shmem_helper.h
> @@ -180,7 +180,6 @@ struct drm_gem_shmem_object {
>  struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, 
> size_t size);
>  void drm_gem_shmem_free(struct drm_gem_shmem_object *shmem);
>  
> -int drm_gem_shmem_get_pages(struct drm_gem_shmem_object *shmem);
>  void drm_gem_shmem_put_pages(struct drm_gem_shmem_object *shmem);
>  int drm_gem_shmem_pin(struct drm_gem_shmem_object *shmem);
>  void drm_gem_shmem_unpin(struct drm_gem_shmem_object *shmem);
> -- 
> 2.35.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Christophe Marie Francois Dupont de Dinechin]

[Resend, still struggling with new laptop email settings]

> On 28 Apr 2022, at 13:03, Michael S. Tsirkin  wrote:
> 
> On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de 
> Dinechin wrote:
>> 
>> 
>>> On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin 
>>>  wrote:
>>> 
>>> 
>>> 
 On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin 
  wrote:
 
 
 
> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
> wrote:
> 
> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
>> GCC 12 enhanced -Waddress when comparing array address to null [0],
>> which warns:
>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
>> always evaluate as ‘true’ for the pointer operand in 
>> ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ 
>> must not be NULL [-Waddress]
>> 257 | if (vp_dev->msix_affinity_masks[i])
>> | ^~
>> In fact, the verification is comparing the result of a pointer
>> arithmetic, the address "msix_affinity_masks + i", which will always
>> evaluate to true.
>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
>> NULL, not requiring non-null verification. So remove the verification
>> to make compiler happy (happy compiler, happy life).
>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
>> Signed-off-by: Murilo Opsfelder Araujo 
>> ---
>> drivers/virtio/virtio_pci_common.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> diff --git a/drivers/virtio/virtio_pci_common.c 
>> b/drivers/virtio/virtio_pci_common.c
>> index d724f676608b..5046efcffb4c 100644
>> --- a/drivers/virtio/virtio_pci_common.c
>> +++ b/drivers/virtio/virtio_pci_common.c
>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>>  if (vp_dev->msix_affinity_masks) {
>>  for (i = 0; i < vp_dev->msix_vectors; i++)
>> -if (vp_dev->msix_affinity_masks[i])
>> -
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>> +
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>  }
>>  if (vp_dev->msix_enabled) {
> 
> After I sent this message, I realized that Christophe (copied here)
> had already proposed a fix:
> 
> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
> 
> Christophe,
> 
> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
> can we just drop this null verification and call free_cpumask_var() right 
> away?
 
 Apologies for the delay in responding, broken laptop…
 
 In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
 
typedef struct cpumask cpumask_var_t[1];
 
 So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
 warning)
 but also a static pointer, so not kfree-safe IMO.
>>> 
>>> … which also renders my own patch invalid :-/
>>> 
>>> Compiler warnings are good. Clearly not sufficient.
>> 
>> Ah, I just noticed that free_cpumask_var is a noop in that case.
>> 
>> So yes, your fix is better :-)
> 
> ACK then?

Yes.

Acked-by: Christophe de Dinechin 

> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Christophe Marie Francois Dupont de Dinechin]

> On 28 Apr 2022, at 13:03, Michael S. Tsirkin  wrote:
> 
> On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de 
> Dinechin wrote:
>> 
>> 
>>> On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin 
>>>  wrote:
>>> 
>>> 
>>> 
 On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin 
  wrote:
 
 
 
> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
> wrote:
> 
> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
>> GCC 12 enhanced -Waddress when comparing array address to null [0],
>> which warns:
>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
>> always evaluate as ‘true’ for the pointer operand in 
>> ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ 
>> must not be NULL [-Waddress]
>> 257 | if (vp_dev->msix_affinity_masks[i])
>> | ^~
>> In fact, the verification is comparing the result of a pointer
>> arithmetic, the address "msix_affinity_masks + i", which will always
>> evaluate to true.
>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
>> NULL, not requiring non-null verification. So remove the verification
>> to make compiler happy (happy compiler, happy life).
>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
>> Signed-off-by: Murilo Opsfelder Araujo 
>> ---
>> drivers/virtio/virtio_pci_common.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> diff --git a/drivers/virtio/virtio_pci_common.c 
>> b/drivers/virtio/virtio_pci_common.c
>> index d724f676608b..5046efcffb4c 100644
>> --- a/drivers/virtio/virtio_pci_common.c
>> +++ b/drivers/virtio/virtio_pci_common.c
>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>>  if (vp_dev->msix_affinity_masks) {
>>  for (i = 0; i < vp_dev->msix_vectors; i++)
>> -if (vp_dev->msix_affinity_masks[i])
>> -
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>> +
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>  }
>>  if (vp_dev->msix_enabled) {
> 
> After I sent this message, I realized that Christophe (copied here)
> had already proposed a fix:
> 
> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
> 
> Christophe,
> 
> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
> can we just drop this null verification and call free_cpumask_var() right 
> away?
 
 Apologies for the delay in responding, broken laptop…
 
 In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
 
typedef struct cpumask cpumask_var_t[1];
 
 So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
 warning)
 but also a static pointer, so not kfree-safe IMO.
>>> 
>>> … which also renders my own patch invalid :-/
>>> 
>>> Compiler warnings are good. Clearly not sufficient.
>> 
>> Ah, I just noticed that free_cpumask_var is a noop in that case.
>> 
>> So yes, your fix is better :-)
> 
> ACK then?

Yes.

Acked-by: Christophe de Dinechin mailto:dinec...@redhat.com>>

> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/3] virtio-pci: Use cpumask_available to fix compilation error

[Michael S. Tsirkin]

On Thu, Apr 28, 2022 at 11:48:01AM +0200, Christophe Marie Francois Dupont de 
Dinechin wrote:
> 
> 
> > On 15 Apr 2022, at 10:48, Michael S. Tsirkin  wrote:
> > 
> > On Thu, Apr 14, 2022 at 05:08:55PM +0200, Christophe de Dinechin wrote:
> >> With GCC 12 and defconfig, we get the following error:
> >> 
> >> |   CC  drivers/virtio/virtio_pci_common.o
> >> | drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
> >> | drivers/virtio/virtio_pci_common.c:257:29: error: the comparison will
> >> |  always evaluate as ‘true’ for the pointer operand in
> >> |  ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 8)’
> >> |  must not be NULL [-Werror=address]
> >> |   257 | if (vp_dev->msix_affinity_masks[i])
> >> |   | ^~
> >> 
> >> This happens in the case where CONFIG_CPUMASK_OFFSTACK is not defined,
> >> since we typedef cpumask_var_t as an array. The compiler is essentially
> >> complaining that an array pointer cannot be NULL. This is not a very
> >> important warning, but there is a function called cpumask_available that
> >> seems to be defined just for that case, so the fix is easy.
> >> 
> >> Signed-off-by: Christophe de Dinechin 
> >> Signed-off-by: Christophe de Dinechin 
> > 
> > There was an alternate patch proposed for this by
> > Murilo Opsfelder Araujo. What do you think about that approach?
> 
> I responded on the other thread, but let me share the response here:
> 
> [to muri...@linux.ibm.com]
> Apologies for the delay in responding, broken laptop…
> 
> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
> 
>   typedef struct cpumask cpumask_var_t[1];
> 
> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
> warning)
> but also a static pointer, so not kfree-safe IMO.


Not sure I understand what you are saying here.

> > 
> > 
> >> ---
> >> drivers/virtio/virtio_pci_common.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/virtio/virtio_pci_common.c 
> >> b/drivers/virtio/virtio_pci_common.c
> >> index d724f676608b..5c44a2f13c93 100644
> >> --- a/drivers/virtio/virtio_pci_common.c
> >> +++ b/drivers/virtio/virtio_pci_common.c
> >> @@ -254,7 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
> >> 
> >>if (vp_dev->msix_affinity_masks) {
> >>for (i = 0; i < vp_dev->msix_vectors; i++)
> >> -  if (vp_dev->msix_affinity_masks[i])
> >> +  if (cpumask_available(vp_dev->msix_affinity_masks[i]))
> >>
> >> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
> >>}
> >> 
> >> -- 
> >> 2.35.1
> > 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Michael S. Tsirkin]

On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de 
Dinechin wrote:
> 
> 
> > On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin 
> >  wrote:
> > 
> > 
> > 
> >> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin 
> >>  wrote:
> >> 
> >> 
> >> 
> >>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
> >>> wrote:
> >>> 
> >>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
>  GCC 12 enhanced -Waddress when comparing array address to null [0],
>  which warns:
>  drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>  drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
>  always evaluate as ‘true’ for the pointer operand in 
>  ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ 
>  must not be NULL [-Waddress]
>  257 | if (vp_dev->msix_affinity_masks[i])
>  | ^~
>  In fact, the verification is comparing the result of a pointer
>  arithmetic, the address "msix_affinity_masks + i", which will always
>  evaluate to true.
>  Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
>  NULL, not requiring non-null verification. So remove the verification
>  to make compiler happy (happy compiler, happy life).
>  [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
>  Signed-off-by: Murilo Opsfelder Araujo 
>  ---
>  drivers/virtio/virtio_pci_common.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>  diff --git a/drivers/virtio/virtio_pci_common.c 
>  b/drivers/virtio/virtio_pci_common.c
>  index d724f676608b..5046efcffb4c 100644
>  --- a/drivers/virtio/virtio_pci_common.c
>  +++ b/drivers/virtio/virtio_pci_common.c
>  @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>   if (vp_dev->msix_affinity_masks) {
>   for (i = 0; i < vp_dev->msix_vectors; i++)
>  -if (vp_dev->msix_affinity_masks[i])
>  -
>  free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>  +
>  free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>   }
>   if (vp_dev->msix_enabled) {
> >>> 
> >>> After I sent this message, I realized that Christophe (copied here)
> >>> had already proposed a fix:
> >>> 
> >>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
> >>> 
> >>> Christophe,
> >>> 
> >>> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
> >>> can we just drop this null verification and call free_cpumask_var() right 
> >>> away?
> >> 
> >> Apologies for the delay in responding, broken laptop…
> >> 
> >> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
> >> 
> >>typedef struct cpumask cpumask_var_t[1];
> >> 
> >> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
> >> warning)
> >> but also a static pointer, so not kfree-safe IMO.
> > 
> > … which also renders my own patch invalid :-/
> > 
> > Compiler warnings are good. Clearly not sufficient.
> 
> Ah, I just noticed that free_cpumask_var is a noop in that case.
> 
> So yes, your fix is better :-)

ACK then?

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Christophe Marie Francois Dupont de Dinechin]

> On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin 
>  wrote:
> 
> 
> 
>> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin 
>>  wrote:
>> 
>> 
>> 
>>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
>>> wrote:
>>> 
>>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
 GCC 12 enhanced -Waddress when comparing array address to null [0],
 which warns:
 drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
 drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
 always evaluate as ‘true’ for the pointer operand in 
 ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ 
 must not be NULL [-Waddress]
 257 | if (vp_dev->msix_affinity_masks[i])
 | ^~
 In fact, the verification is comparing the result of a pointer
 arithmetic, the address "msix_affinity_masks + i", which will always
 evaluate to true.
 Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
 NULL, not requiring non-null verification. So remove the verification
 to make compiler happy (happy compiler, happy life).
 [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
 Signed-off-by: Murilo Opsfelder Araujo 
 ---
 drivers/virtio/virtio_pci_common.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
 diff --git a/drivers/virtio/virtio_pci_common.c 
 b/drivers/virtio/virtio_pci_common.c
 index d724f676608b..5046efcffb4c 100644
 --- a/drivers/virtio/virtio_pci_common.c
 +++ b/drivers/virtio/virtio_pci_common.c
 @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
if (vp_dev->msix_affinity_masks) {
for (i = 0; i < vp_dev->msix_vectors; i++)
 -  if (vp_dev->msix_affinity_masks[i])
 -  
 free_cpumask_var(vp_dev->msix_affinity_masks[i]);
 +  free_cpumask_var(vp_dev->msix_affinity_masks[i]);
}
if (vp_dev->msix_enabled) {
>>> 
>>> After I sent this message, I realized that Christophe (copied here)
>>> had already proposed a fix:
>>> 
>>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
>>> 
>>> Christophe,
>>> 
>>> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
>>> can we just drop this null verification and call free_cpumask_var() right 
>>> away?
>> 
>> Apologies for the delay in responding, broken laptop…
>> 
>> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
>> 
>>  typedef struct cpumask cpumask_var_t[1];
>> 
>> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
>> warning)
>> but also a static pointer, so not kfree-safe IMO.
> 
> … which also renders my own patch invalid :-/
> 
> Compiler warnings are good. Clearly not sufficient.

Ah, I just noticed that free_cpumask_var is a noop in that case.

So yes, your fix is better :-)

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Christophe Marie Francois Dupont de Dinechin]

> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin 
>  wrote:
> 
> 
> 
>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
>> wrote:
>> 
>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
>>> GCC 12 enhanced -Waddress when comparing array address to null [0],
>>> which warns:
>>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
>>> always evaluate as ‘true’ for the pointer operand in 
>>> ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must 
>>> not be NULL [-Waddress]
>>> 257 | if (vp_dev->msix_affinity_masks[i])
>>> | ^~
>>> In fact, the verification is comparing the result of a pointer
>>> arithmetic, the address "msix_affinity_masks + i", which will always
>>> evaluate to true.
>>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
>>> NULL, not requiring non-null verification. So remove the verification
>>> to make compiler happy (happy compiler, happy life).
>>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
>>> Signed-off-by: Murilo Opsfelder Araujo 
>>> ---
>>> drivers/virtio/virtio_pci_common.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>> diff --git a/drivers/virtio/virtio_pci_common.c 
>>> b/drivers/virtio/virtio_pci_common.c
>>> index d724f676608b..5046efcffb4c 100644
>>> --- a/drivers/virtio/virtio_pci_common.c
>>> +++ b/drivers/virtio/virtio_pci_common.c
>>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>>> if (vp_dev->msix_affinity_masks) {
>>> for (i = 0; i < vp_dev->msix_vectors; i++)
>>> -   if (vp_dev->msix_affinity_masks[i])
>>> -   
>>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>> +   free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>> }
>>> if (vp_dev->msix_enabled) {
>> 
>> After I sent this message, I realized that Christophe (copied here)
>> had already proposed a fix:
>> 
>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
>> 
>> Christophe,
>> 
>> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
>> can we just drop this null verification and call free_cpumask_var() right 
>> away?
> 
> Apologies for the delay in responding, broken laptop…
> 
> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:
> 
>   typedef struct cpumask cpumask_var_t[1];
> 
> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
> warning)
> but also a static pointer, so not kfree-safe IMO.

… which also renders my own patch invalid :-/

Compiler warnings are good. Clearly not sufficient.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/3] virtio-pci: Use cpumask_available to fix compilation error

[Christophe Marie Francois Dupont de Dinechin]

> On 15 Apr 2022, at 10:48, Michael S. Tsirkin  wrote:
> 
> On Thu, Apr 14, 2022 at 05:08:55PM +0200, Christophe de Dinechin wrote:
>> With GCC 12 and defconfig, we get the following error:
>> 
>> |   CC  drivers/virtio/virtio_pci_common.o
>> | drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>> | drivers/virtio/virtio_pci_common.c:257:29: error: the comparison will
>> |  always evaluate as ‘true’ for the pointer operand in
>> |  ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 8)’
>> |  must not be NULL [-Werror=address]
>> |   257 | if (vp_dev->msix_affinity_masks[i])
>> |   | ^~
>> 
>> This happens in the case where CONFIG_CPUMASK_OFFSTACK is not defined,
>> since we typedef cpumask_var_t as an array. The compiler is essentially
>> complaining that an array pointer cannot be NULL. This is not a very
>> important warning, but there is a function called cpumask_available that
>> seems to be defined just for that case, so the fix is easy.
>> 
>> Signed-off-by: Christophe de Dinechin 
>> Signed-off-by: Christophe de Dinechin 
> 
> There was an alternate patch proposed for this by
> Murilo Opsfelder Araujo. What do you think about that approach?

I responded on the other thread, but let me share the response here:

[to muri...@linux.ibm.com]
Apologies for the delay in responding, broken laptop…

In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:

typedef struct cpumask cpumask_var_t[1];

So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
warning)
but also a static pointer, so not kfree-safe IMO.

> 
> 
>> ---
>> drivers/virtio/virtio_pci_common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/virtio/virtio_pci_common.c 
>> b/drivers/virtio/virtio_pci_common.c
>> index d724f676608b..5c44a2f13c93 100644
>> --- a/drivers/virtio/virtio_pci_common.c
>> +++ b/drivers/virtio/virtio_pci_common.c
>> @@ -254,7 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>> 
>>  if (vp_dev->msix_affinity_masks) {
>>  for (i = 0; i < vp_dev->msix_vectors; i++)
>> -if (vp_dev->msix_affinity_masks[i])
>> +if (cpumask_available(vp_dev->msix_affinity_masks[i]))
>>  
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>  }
>> 
>> -- 
>> 2.35.1
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs()

[Christophe Marie Francois Dupont de Dinechin]

> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo  
> wrote:
> 
> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote:
>> GCC 12 enhanced -Waddress when comparing array address to null [0],
>> which warns:
>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’:
>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will 
>> always evaluate as ‘true’ for the pointer operand in 
>> ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must 
>> not be NULL [-Waddress]
>>   257 | if (vp_dev->msix_affinity_masks[i])
>>   | ^~
>> In fact, the verification is comparing the result of a pointer
>> arithmetic, the address "msix_affinity_masks + i", which will always
>> evaluate to true.
>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass
>> NULL, not requiring non-null verification.  So remove the verification
>> to make compiler happy (happy compiler, happy life).
>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103
>> Signed-off-by: Murilo Opsfelder Araujo 
>> ---
>>  drivers/virtio/virtio_pci_common.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>> diff --git a/drivers/virtio/virtio_pci_common.c 
>> b/drivers/virtio/virtio_pci_common.c
>> index d724f676608b..5046efcffb4c 100644
>> --- a/drivers/virtio/virtio_pci_common.c
>> +++ b/drivers/virtio/virtio_pci_common.c
>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev)
>>  if (vp_dev->msix_affinity_masks) {
>>  for (i = 0; i < vp_dev->msix_vectors; i++)
>> -if (vp_dev->msix_affinity_masks[i])
>> -
>> free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>> +free_cpumask_var(vp_dev->msix_affinity_masks[i]);
>>  }
>>  if (vp_dev->msix_enabled) {
> 
> After I sent this message, I realized that Christophe (copied here)
> had already proposed a fix:
> 
>https://lore.kernel.org/lkml/20220414150855.2407137-4-dinec...@redhat.com/
> 
> Christophe,
> 
> Since free_cpumask_var() calls kfree() and kfree() is null-safe,
> can we just drop this null verification and call free_cpumask_var() right 
> away?

Apologies for the delay in responding, broken laptop…

In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have:

typedef struct cpumask cpumask_var_t[1];

So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the 
warning)
but also a static pointer, so not kfree-safe IMO.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 3/9] virtio: introduce config op to synchronize vring callbacks

[Cornelia Huck]

On Mon, Apr 25 2022, Jason Wang  wrote:

> This patch introduces new virtio config op to vring
> callbacks. Transport specific method is required to make sure the
> write before this function is visible to the vring_interrupt() that is

Which kind of writes? I.e., what is the scope?

> called after the return of this function. For the transport that
> doesn't provide synchronize_vqs(), use synchornize_rcu() which

Typo: synchronize_rcu()

> synchronize with IRQ implicitly as a fallback.
>
> Cc: Thomas Gleixner 
> Cc: Peter Zijlstra 
> Cc: "Paul E. McKenney" 
> Cc: Marc Zyngier 
> Cc: Halil Pasic 
> Cc: Cornelia Huck 
> Signed-off-by: Jason Wang 
> ---
>  include/linux/virtio_config.h | 24 
>  1 file changed, 24 insertions(+)
>
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index b341dd62aa4d..14fe89ff99c7 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -57,6 +57,10 @@ struct virtio_shm_region {
>   *   include a NULL entry for vqs unused by driver
>   *   Returns 0 on success or error status
>   * @del_vqs: free virtqueues found by find_vqs().
> + * @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
> + *  Make sure the writes commited before this method is visible to
> + *  vring_interrupt() which is called after this method.

Same here, I think the description needs to be a bit more explicit about
which writes we care about here.

> + *  vdev: the virtio_device
>   * @get_features: get the array of feature bits for this device.
>   *   vdev: the virtio_device
>   *   Returns the first 64 feature bits (all we currently need).
> @@ -89,6 +93,7 @@ struct virtio_config_ops {
>   const char * const names[], const bool *ctx,
>   struct irq_affinity *desc);
>   void (*del_vqs)(struct virtio_device *);
> + void (*synchronize_cbs)(struct virtio_device *);
>   u64 (*get_features)(struct virtio_device *vdev);
>   int (*finalize_features)(struct virtio_device *vdev);
>   const char *(*bus_name)(struct virtio_device *vdev);

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 2/9] virtio: use virtio_reset_device() when possible

[Cornelia Huck]

On Mon, Apr 25 2022, Jason Wang  wrote:

> This allows us to do common extension without duplicating code.
>
> Cc: Thomas Gleixner 
> Cc: Peter Zijlstra 
> Cc: "Paul E. McKenney" 
> Cc: Marc Zyngier 
> Cc: Halil Pasic 
> Cc: Cornelia Huck 
> Signed-off-by: Jason Wang 
> ---
>  drivers/virtio/virtio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 1/9] virtio: use virtio_device_ready() in virtio_device_restore()

[Cornelia Huck]

On Mon, Apr 25 2022, Jason Wang  wrote:

> From: Stefano Garzarella 
>
> It will allow us to do extension on virtio_device_ready() without
> duplicating code.
>
> Cc: Thomas Gleixner 
> Cc: Peter Zijlstra 
> Cc: "Paul E. McKenney" 
> Cc: Marc Zyngier 
> Cc: Halil Pasic 
> Cc: Cornelia Huck 
> Signed-off-by: Stefano Garzarella 
> Signed-off-by: Jason Wang 
> ---
>  drivers/virtio/virtio.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH V3 6/9] virtio-ccw: implement synchronize_cbs()

[Cornelia Huck]

On Thu, Apr 28 2022, Jason Wang  wrote:

> On Thu, Apr 28, 2022 at 1:55 PM Michael S. Tsirkin  wrote:
>>
>> On Thu, Apr 28, 2022 at 01:51:59PM +0800, Jason Wang wrote:
>> > On Thu, Apr 28, 2022 at 1:24 PM Michael S. Tsirkin  wrote:
>> > >
>> > > On Thu, Apr 28, 2022 at 11:04:41AM +0800, Jason Wang wrote:
>> > > > > But my guess is that rwlock + some testing for the legacy indicator 
>> > > > > case
>> > > > > just to double check if there is a heavy regression despite of our
>> > > > > expectations to see none should do the trick.
>> > > >
>> > > > I suggest this, rwlock (for not airq) seems better than spinlock, but
>> > > > at worst case it will cause cache line bouncing. But I wonder if it's
>> > > > noticeable (anyhow it has been used for airq).
>> > > >
>> > > > Thanks
>> > >
>> > > Which existing rwlock does airq use right now? Can we take it to sync?
>> >
>> > It's the rwlock in airq_info, it has already been used in this patch.
>> >
>> > write_lock(&info->lock);
>> > write_unlock(&info->lock);
>> >
>> > But the problem is, it looks to me there could be a case that airq is
>> > not used, (virtio_ccw_int_hander()). That's why the patch use a
>> > spinlock, it could be optimized with using a rwlock as well.
>> >
>> > Thanks
>>
>> Ah, right. So let's take that on the legacy path too and Halil promises
>> to test to make sure performance isn't impacted too badly?
>
> I think what you meant is using a dedicated rwlock instead of trying
> to reuse one of the airq_info locks.
>
> If this is true, it should be fine.

FWIW, that approach makes sense to me as well.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization