Linux virtualization list
 help / color / mirror / Atom feed
* [RFC PATCH net-next v2 0/2] Enable virtio to act as a backup for a passthru device
From: Sridhar Samudrala @ 2018-01-12  5:58 UTC (permalink / raw)
  To: mst, stephen, davem, netdev, virtualization, virtio-dev,
	jesse.brandeburg, alexander.h.duyck, kubakici, sridhar.samudrala

This patch series extends virtio_net to take over VF datapath by
simulating a transparent bond without creating any additional netdev.

I understand that there are some comments suggesting an alternate model
that is based on 3 driver model(virtio_net, VF driver, a new driver 
virt_bond that acts as a master to virtio_net and VF).

Would like to get some feedback on the right way to solve the live
migration problem with direct attached devices in KVM environment.

Stephen,
Is the netvsc transparent bond implemenation robust enough and deployed
in real environments? Or would netvsc switch over to a 3-driver model if
that solution becomes available?

Can we start with this implementation that is similar to netvsc and if
needed we can move to the 3 driver model later?

This patch series enables virtio to switch over to a VF datapath when a 
VF netdev is present with the same MAC address. It allows live migration 
of a VM with a direct attached VF without the need to setup a bond/team
between a VF and virtio net device in the guest.

The hypervisor needs to unplug the VF device from the guest on the source
host and reset the MAC filter of the VF to initiate failover of datapath
to virtio before starting the migration. After the migration is completed,
the destination hypervisor sets the MAC filter on the VF and plugs it back
to the guest to switch over to VF datapath.

It is based on netvsc implementation and it should be possible to make this
code generic and move it to a common location that can be shared by netvsc
and virtio.

This patch series is based on the discussion initiated by Jesse on this thread.
https://marc.info/?l=linux-virtualization&m=151189725224231&w=2

v2:
- Changed VIRTIO_NET_F_MASTER to VIRTIO_NET_F_BACKUP (mst)
- made a small change to the virtio-net xmit path to only use VF datapath
  for unicasts. Broadcasts/multicasts use virtio datapath. This avoids
  east-west broadcasts to go over the PCI link.
- added suppport for the feature bit in qemu

Sridhar Samudrala (2):
  virtio_net: Introduce VIRTIO_NET_F_BACKUP feature bit
  virtio_net: Extend virtio to use VF datapath when available

 drivers/net/virtio_net.c        | 309 +++++++++++++++++++++++++++++++++++++++-
 include/uapi/linux/virtio_net.h |   3 +
 2 files changed, 309 insertions(+), 3 deletions(-)

Sridhar Samudrala (1):
  qemu: Introduce VIRTIO_NET_F_BACKUP feature bit to virtio_net

 hw/net/virtio-net.c                         | 2 ++
 include/standard-headers/linux/virtio_net.h | 3 +++
 2 files changed, 5 insertions(+)

-- 
2.14.3

^ permalink raw reply

* Re: [PATCH] drm/virtio: Add window server support
From: Dave Airlie @ 2018-01-12  4:11 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: Michael S. Tsirkin, David Airlie, LKML, dri-devel,
	open list:VIRTIO CORE, NET...
In-Reply-To: <20171228115333.12289-1-tomeu.vizoso@collabora.com>

>
> this work is based on the virtio_wl driver in the ChromeOS kernel by
> Zach Reizner, currently at:
>
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/virtio/virtio_wl.c
>
> There's two features missing in this patch when compared with virtio_wl:
>
> * Allow the guest access directly host memory, without having to resort
> to TRANSFER_TO_HOST
>
> * Pass FDs from host to guest (Wayland specifies that the compositor
> shares keyboard data with the guest via a shared buffer)
>
> I plan to work on this next, but I would like to get some comments on
> the general approach so I can better choose which patch to follow.

Shouldn't qemu expose some kind of capability to enable this so we know to
look for the extra vqs?

What happens if you run this on plain qemu, does it fallback correctly?

Are there any scenarios where we don't want to expose this API because there
is nothing to back it.

Dave.

^ permalink raw reply

* Re: [PATCH 6/6] crypto: stm32-cryp: convert to the new crypto engine API
From: Fabien DESSENNE @ 2018-01-11  7:45 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <ccffd847-3df7-d690-4102-5a10f6439f87@st.com>

(Adding my tested by)


On 10/01/18 15:25, Fabien DESSENNE wrote:
>
> On 03/01/18 21:11, Corentin Labbe wrote:
>> This patch convert the stm32-cryp driver to the new crypto engine API.
>> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Tested-by: Fabien Dessenne <fabien.dessenne@st.com>

>> ---
>>    drivers/crypto/stm32/stm32-cryp.c | 21 ++++++++++++++++-----
>>    1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/crypto/stm32/stm32-cryp.c b/drivers/crypto/stm32/stm32-cryp.c
>> index cf1dddbeaa2c..99e0473ef247 100644
>> --- a/drivers/crypto/stm32/stm32-cryp.c
>> +++ b/drivers/crypto/stm32/stm32-cryp.c
>> @@ -91,6 +91,7 @@
>>    #define _walked_out             (cryp->out_walk.offset - cryp->out_sg->offset)
>>    
>>    struct stm32_cryp_ctx {
>> +	struct crypto_engine_reqctx enginectx;
>>    	struct stm32_cryp       *cryp;
>>    	int                     keylen;
>>    	u32                     key[AES_KEYSIZE_256 / sizeof(u32)];
>> @@ -494,10 +495,20 @@ static int stm32_cryp_cpu_start(struct stm32_cryp *cryp)
>>    	return 0;
>>    }
>>    
>> +static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
>> +				     void *areq);
> Merge these 2 lines in a single one
>
>> +static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
>> +					 void *areq);
>> +
>>    static int stm32_cryp_cra_init(struct crypto_tfm *tfm)
>>    {
>> +	struct stm32_cryp_ctx *ctx = crypto_tfm_ctx(tfm);
>> +
>>    	tfm->crt_ablkcipher.reqsize = sizeof(struct stm32_cryp_reqctx);
>>    
>> +	ctx->enginectx.op.do_one_request = stm32_cryp_cipher_one_req;
>> +	ctx->enginectx.op.prepare_request = stm32_cryp_prepare_cipher_req;
>> +	ctx->enginectx.op.unprepare_request = NULL;
>>    	return 0;
>>    }
>>    
>> @@ -695,14 +706,17 @@ static int stm32_cryp_prepare_req(struct crypto_engine *engine,
>>    }
>>    
>>    static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
>> -					 struct ablkcipher_request *req)
>> +					 void *areq)
>>    {
>> +	struct ablkcipher_request *req = container_of(areq, struct ablkcipher_request, base);
>   > 80 characters (CHECKPATCH)
>
>> +
>>    	return stm32_cryp_prepare_req(engine, req);
>>    }
>>    
>>    static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
>> -				     struct ablkcipher_request *req)
>> +				     void *areq)
> Merge these 2 lines in a single one
>
>>    {
>> +	struct ablkcipher_request *req = container_of(areq, struct ablkcipher_request, base);
>   > 80 characters (CHECKPATCH)
>
>>    	struct stm32_cryp_ctx *ctx = crypto_ablkcipher_ctx(
>>    			crypto_ablkcipher_reqtfm(req));
>>    	struct stm32_cryp *cryp = ctx->cryp;
>> @@ -1104,9 +1118,6 @@ static int stm32_cryp_probe(struct platform_device *pdev)
>>    		goto err_engine1;
>>    	}
>>    
>> -	cryp->engine->prepare_cipher_request = stm32_cryp_prepare_cipher_req;
>> -	cryp->engine->cipher_one_request = stm32_cryp_cipher_one_req;
>> -
>>    	ret = crypto_engine_start(cryp->engine);
>>    	if (ret) {
>>    		dev_err(dev, "Could not start crypto engine\n");

^ permalink raw reply

* Re: [PATCH 5/6] crypto: stm32-hash: convert to the new crypto engine API
From: Fabien DESSENNE @ 2018-01-11  7:44 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Lionel DEBIEVE, Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <17d0497d-5c13-f93e-4249-8adff5e71b19@st.com>

(adding my tested my)


On 10/01/18 15:24, Fabien DESSENNE wrote:
>
> On 03/01/18 21:11, Corentin Labbe wrote:
>> This patch convert the stm32-hash driver to the new crypto engine API.
>>
>> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Tested-by: Fabien Dessenne <fabien.dessenne@st.com>

>> ---
>>    drivers/crypto/stm32/stm32-hash.c | 18 +++++++++++++-----
>>    1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
>> index 4ca4a264a833..9790c2c936c7 100644
>> --- a/drivers/crypto/stm32/stm32-hash.c
>> +++ b/drivers/crypto/stm32/stm32-hash.c
>> @@ -122,6 +122,7 @@ enum stm32_hash_data_format {
>>    #define HASH_DMA_THRESHOLD		50
>>    
>>    struct stm32_hash_ctx {
>> +	struct crypto_engine_reqctx enginectx;
>>    	struct stm32_hash_dev	*hdev;
>>    	unsigned long		flags;
>>    
>> @@ -828,6 +829,11 @@ static int stm32_hash_hw_init(struct stm32_hash_dev *hdev,
>>    	return 0;
>>    }
>>    
>> +static int stm32_hash_one_request(struct crypto_engine *engine,
>> +				  void *areq);
> merge these two lines in a single one
>
>> +static int stm32_hash_prepare_req(struct crypto_engine *engine,
>> +				  void *areq);
> merge these two lines in a single one
>
>> +
>>    static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>>    				   struct ahash_request *req)
>>    {
>> @@ -835,8 +841,9 @@ static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>>    }
>>    
>>    static int stm32_hash_prepare_req(struct crypto_engine *engine,
>> -				  struct ahash_request *req)
>> +				  void *areq)
> merge these two lines in a single one
>
>>    {
>> +	struct ahash_request *req = container_of(areq, struct ahash_request, base);
>   > 80 characters (CHECKPATCH)
>
>>    	struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>>    	struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>>    	struct stm32_hash_request_ctx *rctx;
>> @@ -855,8 +862,9 @@ static int stm32_hash_prepare_req(struct crypto_engine *engine,
>>    }
>>    
>>    static int stm32_hash_one_request(struct crypto_engine *engine,
>> -				  struct ahash_request *req)
>> +				  void *areq)
> merge these two lines in a single one
>
>>    {
>> +	struct ahash_request *req = container_of(areq, struct ahash_request, base);
>   > 80 characters (CHECKPATCH)
>
>>    	struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>>    	struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>>    	struct stm32_hash_request_ctx *rctx;
>> @@ -1033,6 +1041,9 @@ static int stm32_hash_cra_init_algs(struct crypto_tfm *tfm,
>>    	if (algs_hmac_name)
>>    		ctx->flags |= HASH_FLAGS_HMAC;
>>    
>> +	ctx->enginectx.op.do_one_request = stm32_hash_one_request;
>> +	ctx->enginectx.op.prepare_request = stm32_hash_prepare_req;
>> +	ctx->enginectx.op.unprepare_request = NULL;
>>    	return 0;
>>    }
>>    
>> @@ -1493,9 +1504,6 @@ static int stm32_hash_probe(struct platform_device *pdev)
>>    		goto err_engine;
>>    	}
>>    
>> -	hdev->engine->prepare_hash_request = stm32_hash_prepare_req;
>> -	hdev->engine->hash_one_request = stm32_hash_one_request;
>> -
>>    	ret = crypto_engine_start(hdev->engine);
>>    	if (ret)
>>    		goto err_engine_start;

^ permalink raw reply

* Re: [PATCH 2/6] crypto: engine - Permit to enqueue all async requests
From: Fabien DESSENNE @ 2018-01-11  7:44 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Lionel DEBIEVE, Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <83fa00f2-6e93-d83f-fd9a-0048bd6d60ec@st.com>

(adding my tested by)


On 10/01/18 15:19, Fabien DESSENNE wrote:
> On 03/01/18 21:11, Corentin Labbe wrote:
>> The crypto engine could actually only enqueue hash and ablkcipher request.
>> This patch permit it to enqueue any type of crypto_async_request.
>>
>> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Tested-by: Fabien Dessenne <fabien.dessenne@st.com>

>> ---
>>    crypto/crypto_engine.c  | 230 ++++++++++++++++++++++++------------------------
>>    include/crypto/engine.h |  59 +++++++------
>>    2 files changed, 148 insertions(+), 141 deletions(-)
>>
>> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
>> index 61e7c4e02fd2..036270b61648 100644
>> --- a/crypto/crypto_engine.c
>> +++ b/crypto/crypto_engine.c
>> @@ -15,7 +15,6 @@
>>    #include <linux/err.h>
>>    #include <linux/delay.h>
>>    #include <crypto/engine.h>
>> -#include <crypto/internal/hash.h>
>>    #include <uapi/linux/sched/types.h>
>>    #include "internal.h"
>>    
>> @@ -34,11 +33,10 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>>    				 bool in_kthread)
>>    {
>>    	struct crypto_async_request *async_req, *backlog;
>> -	struct ahash_request *hreq;
>> -	struct ablkcipher_request *breq;
>>    	unsigned long flags;
>>    	bool was_busy = false;
>> -	int ret, rtype;
>> +	int ret;
>> +	struct crypto_engine_reqctx *enginectx;
>>    
>>    	spin_lock_irqsave(&engine->queue_lock, flags);
>>    
>> @@ -94,7 +92,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>>    
>>    	spin_unlock_irqrestore(&engine->queue_lock, flags);
>>    
>> -	rtype = crypto_tfm_alg_type(engine->cur_req->tfm);
>>    	/* Until here we get the request need to be encrypted successfully */
>>    	if (!was_busy && engine->prepare_crypt_hardware) {
>>    		ret = engine->prepare_crypt_hardware(engine);
>> @@ -104,57 +101,31 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>>    		}
>>    	}
>>    
>> -	switch (rtype) {
>> -	case CRYPTO_ALG_TYPE_AHASH:
>> -		hreq = ahash_request_cast(engine->cur_req);
>> -		if (engine->prepare_hash_request) {
>> -			ret = engine->prepare_hash_request(engine, hreq);
>> -			if (ret) {
>> -				dev_err(engine->dev, "failed to prepare request: %d\n",
>> -					ret);
>> -				goto req_err;
>> -			}
>> -			engine->cur_req_prepared = true;
>> -		}
>> -		ret = engine->hash_one_request(engine, hreq);
>> -		if (ret) {
>> -			dev_err(engine->dev, "failed to hash one request from queue\n");
>> -			goto req_err;
>> -		}
>> -		return;
>> -	case CRYPTO_ALG_TYPE_ABLKCIPHER:
>> -		breq = ablkcipher_request_cast(engine->cur_req);
>> -		if (engine->prepare_cipher_request) {
>> -			ret = engine->prepare_cipher_request(engine, breq);
>> -			if (ret) {
>> -				dev_err(engine->dev, "failed to prepare request: %d\n",
>> -					ret);
>> -				goto req_err;
>> -			}
>> -			engine->cur_req_prepared = true;
>> -		}
>> -		ret = engine->cipher_one_request(engine, breq);
>> +	enginectx = crypto_tfm_ctx(async_req->tfm);
>> +
>> +	if (enginectx->op.prepare_request) {
>> +		ret = enginectx->op.prepare_request(engine, async_req);
>>    		if (ret) {
>> -			dev_err(engine->dev, "failed to cipher one request from queue\n");
>> +			dev_err(engine->dev, "failed to prepare request: %d\n",
>> +				ret);
>>    			goto req_err;
>>    		}
>> -		return;
>> -	default:
>> -		dev_err(engine->dev, "failed to prepare request of unknown type\n");
>> -		return;
>> +		engine->cur_req_prepared = true;
>> +	}
>> +	if (!enginectx->op.do_one_request) {
>> +		dev_err(engine->dev, "failed to do request\n");
>> +		ret = -EINVAL;
>> +		goto req_err;
>> +	}
>> +	ret = enginectx->op.do_one_request(engine, async_req);
>> +	if (ret) {
>> +		dev_err(engine->dev, "Failed to do one request from queue: %d\n", ret);
>> +		goto req_err;
>>    	}
>> +	return;
>>    
>>    req_err:
>> -	switch (rtype) {
>> -	case CRYPTO_ALG_TYPE_AHASH:
>> -		hreq = ahash_request_cast(engine->cur_req);
>> -		crypto_finalize_hash_request(engine, hreq, ret);
>> -		break;
>> -	case CRYPTO_ALG_TYPE_ABLKCIPHER:
>> -		breq = ablkcipher_request_cast(engine->cur_req);
>> -		crypto_finalize_cipher_request(engine, breq, ret);
>> -		break;
>> -	}
>> +	crypto_finalize_request(engine, async_req, ret);
>>    	return;
>>    
>>    out:
>> @@ -170,13 +141,12 @@ static void crypto_pump_work(struct kthread_work *work)
>>    }
>>    
>>    /**
>> - * crypto_transfer_cipher_request - transfer the new request into the
>> - * enginequeue
>> + * crypto_transfer_request - transfer the new request into the engine queue
>>     * @engine: the hardware engine
>>     * @req: the request need to be listed into the engine queue
>>     */
>> -int crypto_transfer_cipher_request(struct crypto_engine *engine,
>> -				   struct ablkcipher_request *req,
>> +static int crypto_transfer_request(struct crypto_engine *engine,
>> +				   struct crypto_async_request *req,
>>    				   bool need_pump)
>>    {
>>    	unsigned long flags;
>> @@ -189,7 +159,7 @@ int crypto_transfer_cipher_request(struct crypto_engine *engine,
>>    		return -ESHUTDOWN;
>>    	}
>>    
>> -	ret = ablkcipher_enqueue_request(&engine->queue, req);
>> +	ret = crypto_enqueue_request(&engine->queue, req);
>>    
>>    	if (!engine->busy && need_pump)
>>    		kthread_queue_work(engine->kworker, &engine->pump_requests);
>> @@ -197,85 +167,97 @@ int crypto_transfer_cipher_request(struct crypto_engine *engine,
>>    	spin_unlock_irqrestore(&engine->queue_lock, flags);
>>    	return ret;
>>    }
>> -EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request);
>> +EXPORT_SYMBOL_GPL(crypto_transfer_request);
> Do not export this function which is a static one.
>
>>    
>>    /**
>> - * crypto_transfer_cipher_request_to_engine - transfer one request to list
>> + * crypto_transfer_request_to_engine - transfer one request to list
>>     * into the engine queue
>>     * @engine: the hardware engine
>>     * @req: the request need to be listed into the engine queue
>>     */
>> +static int crypto_transfer_request_to_engine(struct crypto_engine *engine,
>> +					     struct crypto_async_request *req)
>> +{
>> +	return crypto_transfer_request(engine, req, true);
>> +}
>> +
>> +/**
>> + * crypto_transfer_cipher_request_to_engine - transfer one ablkcipher_request
>> + * to list into the engine queue
>> + * @engine: the hardware engine
>> + * @req: the request need to be listed into the engine queue
>> + * TODO: Remove this function when skcipher conversion is finished
>> + */
>>    int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
>>    					     struct ablkcipher_request *req)
>>    {
>> -	return crypto_transfer_cipher_request(engine, req, true);
>> +	return crypto_transfer_request_to_engine(engine, &req->base);
>>    }
>>    EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request_to_engine);
>>    
>>    /**
>> - * crypto_transfer_hash_request - transfer the new request into the
>> - * enginequeue
>> + * crypto_transfer_skcipher_request_to_engine - transfer one skcipher_request
>> + * to list into the engine queue
>>     * @engine: the hardware engine
>>     * @req: the request need to be listed into the engine queue
>>     */
>> -int crypto_transfer_hash_request(struct crypto_engine *engine,
>> -				 struct ahash_request *req, bool need_pump)
>> +int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
>> +					       struct skcipher_request *req)
>>    {
>> -	unsigned long flags;
>> -	int ret;
>> -
>> -	spin_lock_irqsave(&engine->queue_lock, flags);
>> -
>> -	if (!engine->running) {
>> -		spin_unlock_irqrestore(&engine->queue_lock, flags);
>> -		return -ESHUTDOWN;
>> -	}
>> -
>> -	ret = ahash_enqueue_request(&engine->queue, req);
>> -
>> -	if (!engine->busy && need_pump)
>> -		kthread_queue_work(engine->kworker, &engine->pump_requests);
>> +	return crypto_transfer_request_to_engine(engine, &req->base);
>> +}
>> +EXPORT_SYMBOL_GPL(crypto_transfer_skcipher_request_to_engine);
>>    
>> -	spin_unlock_irqrestore(&engine->queue_lock, flags);
>> -	return ret;
>> +/**
>> + * crypto_transfer_akcipher_request_to_engine - transfer one akcipher_request
>> + * to list into the engine queue
>> + * @engine: the hardware engine
>> + * @req: the request need to be listed into the engine queue
>> + */
>> +int crypto_transfer_akcipher_request_to_engine(struct crypto_engine *engine,
>> +					       struct akcipher_request *req)
>> +{
>> +	return crypto_transfer_request_to_engine(engine, &req->base);
>>    }
>> -EXPORT_SYMBOL_GPL(crypto_transfer_hash_request);
>> +EXPORT_SYMBOL_GPL(crypto_transfer_akcipher_request_to_engine);
>>    
>>    /**
>> - * crypto_transfer_hash_request_to_engine - transfer one request to list
>> - * into the engine queue
>> + * crypto_transfer_hash_request_to_engine - transfer one ahash_request
>> + * to list into the engine queue
>>     * @engine: the hardware engine
>>     * @req: the request need to be listed into the engine queue
>>     */
>>    int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
>>    					   struct ahash_request *req)
>>    {
>> -	return crypto_transfer_hash_request(engine, req, true);
>> +	return crypto_transfer_request_to_engine(engine, &req->base);
>>    }
>>    EXPORT_SYMBOL_GPL(crypto_transfer_hash_request_to_engine);
>>    
> Please add this EXPORTed function:
>
> crypto_transfer_aead_request_to_engine(struct crypto_engine *engine,
> struct aead_request *req)
>
>>    /**
>> - * crypto_finalize_cipher_request - finalize one request if the request is done
>> + * crypto_finalize_request - finalize one request if the request is done
>>     * @engine: the hardware engine
>>     * @req: the request need to be finalized
>>     * @err: error number
>>     */
>> -void crypto_finalize_cipher_request(struct crypto_engine *engine,
>> -				    struct ablkcipher_request *req, int err)
>> +void crypto_finalize_request(struct crypto_engine *engine,
> shall be static
>
>> +			     struct crypto_async_request *req, int err)
>>    {
>>    	unsigned long flags;
>>    	bool finalize_cur_req = false;
>>    	int ret;
>> +	struct crypto_engine_reqctx *enginectx;
>>    
>>    	spin_lock_irqsave(&engine->queue_lock, flags);
>> -	if (engine->cur_req == &req->base)
>> +	if (engine->cur_req == req)
>>    		finalize_cur_req = true;
>>    	spin_unlock_irqrestore(&engine->queue_lock, flags);
>>    
>>    	if (finalize_cur_req) {
>> +		enginectx = crypto_tfm_ctx(req->tfm);
>>    		if (engine->cur_req_prepared &&
>> -		    engine->unprepare_cipher_request) {
>> -			ret = engine->unprepare_cipher_request(engine, req);
>> +		    enginectx->op.unprepare_request) {
>> +			ret = enginectx->op.unprepare_request(engine, req);
>>    			if (ret)
>>    				dev_err(engine->dev, "failed to unprepare request\n");
>>    		}
>> @@ -285,46 +267,64 @@ void crypto_finalize_cipher_request(struct crypto_engine *engine,
>>    		spin_unlock_irqrestore(&engine->queue_lock, flags);
>>    	}
>>    
>> -	req->base.complete(&req->base, err);
>> +	req->complete(req, err);
>>    
>>    	kthread_queue_work(engine->kworker, &engine->pump_requests);
>>    }
>> -EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
>>    
>>    /**
>> - * crypto_finalize_hash_request - finalize one request if the request is done
>> + * crypto_finalize_cipher_request - finalize one ablkcipher_request if
>> + * the request is done
>>     * @engine: the hardware engine
>>     * @req: the request need to be finalized
>>     * @err: error number
>>     */
>> -void crypto_finalize_hash_request(struct crypto_engine *engine,
>> -				  struct ahash_request *req, int err)
>> +void crypto_finalize_cipher_request(struct crypto_engine *engine,
>> +				    struct ablkcipher_request *req, int err)
>>    {
>> -	unsigned long flags;
>> -	bool finalize_cur_req = false;
>> -	int ret;
>> -
>> -	spin_lock_irqsave(&engine->queue_lock, flags);
>> -	if (engine->cur_req == &req->base)
>> -		finalize_cur_req = true;
>> -	spin_unlock_irqrestore(&engine->queue_lock, flags);
>> +	return crypto_finalize_request(engine, &req->base, err);
>> +}
>> +EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
>>    
>> -	if (finalize_cur_req) {
>> -		if (engine->cur_req_prepared &&
>> -		    engine->unprepare_hash_request) {
>> -			ret = engine->unprepare_hash_request(engine, req);
>> -			if (ret)
>> -				dev_err(engine->dev, "failed to unprepare request\n");
>> -		}
>> -		spin_lock_irqsave(&engine->queue_lock, flags);
>> -		engine->cur_req = NULL;
>> -		engine->cur_req_prepared = false;
>> -		spin_unlock_irqrestore(&engine->queue_lock, flags);
>> -	}
>> +/**
>> + * crypto_finalize_skcipher_request - finalize one skcipher_request if
>> + * the request is done
>> + * @engine: the hardware engine
>> + * @req: the request need to be finalized
>> + * @err: error number
>> + */
>> +void crypto_finalize_skcipher_request(struct crypto_engine *engine,
>> +				      struct skcipher_request *req, int err)
>> +{
>> +	return crypto_finalize_request(engine, &req->base, err);
>> +}
>> +EXPORT_SYMBOL_GPL(crypto_finalize_skcipher_request);
>>    
>> -	req->base.complete(&req->base, err);
>> +/**
>> + * crypto_finalize_akcipher_request - finalize one akcipher_request if
>> + * the request is done
>> + * @engine: the hardware engine
>> + * @req: the request need to be finalized
>> + * @err: error number
>> + */
>> +void crypto_finalize_akcipher_request(struct crypto_engine *engine,
>> +				      struct akcipher_request *req, int err)
>> +{
>> +	return crypto_finalize_request(engine, &req->base, err);
>> +}
>> +EXPORT_SYMBOL_GPL(crypto_finalize_akcipher_request);
>>    
>> -	kthread_queue_work(engine->kworker, &engine->pump_requests);
>> +/**
>> + * crypto_finalize_hash_request - finalize one ahash_request if
>> + * the request is done
>> + * @engine: the hardware engine
>> + * @req: the request need to be finalized
>> + * @err: error number
>> + */
>> +void crypto_finalize_hash_request(struct crypto_engine *engine,
>> +				  struct ahash_request *req, int err)
>> +{
>> +	return crypto_finalize_request(engine, &req->base, err);
>>    }
>>    EXPORT_SYMBOL_GPL(crypto_finalize_hash_request);
> Add
> crypto_finalize_aead_request(struct crypto_engine *engine, struct
> aead_request *req, int err)
>
>>    
>> diff --git a/include/crypto/engine.h b/include/crypto/engine.h
>> index dd04c1699b51..1ea7cbe92eaf 100644
>> --- a/include/crypto/engine.h
>> +++ b/include/crypto/engine.h
>> @@ -17,7 +17,9 @@
>>    #include <linux/kernel.h>
>>    #include <linux/kthread.h>
>>    #include <crypto/algapi.h>
>> +#include <crypto/akcipher.h>
>>    #include <crypto/hash.h>
>> +#include <crypto/skcipher.h>
>>    
>>    #define ENGINE_NAME_LEN	30
>>    /*
>> @@ -37,12 +39,6 @@
>>     * @unprepare_crypt_hardware: there are currently no more requests on the
>>     * queue so the subsystem notifies the driver that it may relax the
>>     * hardware by issuing this call
>> - * @prepare_cipher_request: do some prepare if need before handle the current request
>> - * @unprepare_cipher_request: undo any work done by prepare_cipher_request()
>> - * @cipher_one_request: do encryption for current request
>> - * @prepare_hash_request: do some prepare if need before handle the current request
>> - * @unprepare_hash_request: undo any work done by prepare_hash_request()
>> - * @hash_one_request: do hash for current request
>>     * @kworker: kthread worker struct for request pump
>>     * @pump_requests: work struct for scheduling work to the request pump
>>     * @priv_data: the engine private data
>> @@ -65,19 +61,6 @@ struct crypto_engine {
>>    	int (*prepare_crypt_hardware)(struct crypto_engine *engine);
>>    	int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
>>    
>> -	int (*prepare_cipher_request)(struct crypto_engine *engine,
>> -				      struct ablkcipher_request *req);
>> -	int (*unprepare_cipher_request)(struct crypto_engine *engine,
>> -					struct ablkcipher_request *req);
>> -	int (*prepare_hash_request)(struct crypto_engine *engine,
>> -				    struct ahash_request *req);
>> -	int (*unprepare_hash_request)(struct crypto_engine *engine,
>> -				      struct ahash_request *req);
>> -	int (*cipher_one_request)(struct crypto_engine *engine,
>> -				  struct ablkcipher_request *req);
>> -	int (*hash_one_request)(struct crypto_engine *engine,
>> -				struct ahash_request *req);
>> -
>>    	struct kthread_worker           *kworker;
>>    	struct kthread_work             pump_requests;
>>    
>> @@ -85,19 +68,43 @@ struct crypto_engine {
>>    	struct crypto_async_request	*cur_req;
>>    };
>>    
>> -int crypto_transfer_cipher_request(struct crypto_engine *engine,
>> -				   struct ablkcipher_request *req,
>> -				   bool need_pump);
>> +/*
>> + * struct crypto_engine_op - crypto hardware engine operations
>> + * @prepare__request: do some prepare if need before handle the current request
>> + * @unprepare_request: undo any work done by prepare_request()
>> + * @do_one_request: do encryption for current request
>> + */
>> +struct crypto_engine_op {
>> +	int (*prepare_request)(struct crypto_engine *engine,
>> +			       void *areq);
>> +	int (*unprepare_request)(struct crypto_engine *engine,
>> +				 void *areq);
>> +	int (*do_one_request)(struct crypto_engine *engine,
>> +			      void *areq);
>> +};
>> +
>> +struct crypto_engine_reqctx {
>> +	struct crypto_engine_op op;
>> +};
>> +
>> +int crypto_transfer_akcipher_request_to_engine(struct crypto_engine *engine,
>> +					       struct akcipher_request *req);
>>    int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
>> -					     struct ablkcipher_request *req);
>> -int crypto_transfer_hash_request(struct crypto_engine *engine,
>> -				 struct ahash_request *req, bool need_pump);
>> +				      struct ablkcipher_request *req);
>>    int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
>> -					   struct ahash_request *req);
>> +					       struct ahash_request *req);
>> +int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
>> +					       struct skcipher_request *req);
> + transfer_aead
>
>> +void crypto_finalize_request(struct crypto_engine *engine,
>> +			     struct crypto_async_request *req, int err);
> static (+move to  .c file?)
>
>> +void crypto_finalize_akcipher_request(struct crypto_engine *engine,
>> +				      struct akcipher_request *req, int err);
>>    void crypto_finalize_cipher_request(struct crypto_engine *engine,
>>    				    struct ablkcipher_request *req, int err);
>>    void crypto_finalize_hash_request(struct crypto_engine *engine,
>>    				  struct ahash_request *req, int err);
>> +void crypto_finalize_skcipher_request(struct crypto_engine *engine,
>> +				      struct skcipher_request *req, int err);
> + finalize_aead
>
>>    int crypto_engine_start(struct crypto_engine *engine);
>>    int crypto_engine_stop(struct crypto_engine *engine);
>>    struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt);
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net-next] vhost_net: batch used ring update in rx
From: David Miller @ 2018-01-10 20:04 UTC (permalink / raw)
  To: jasowang; +Cc: willemb, kvm, mst, netdev, linux-kernel, virtualization
In-Reply-To: <1515493665-35306-1-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Tue,  9 Jan 2018 18:27:45 +0800

> This patch tries to batched used ring update during RX. This is pretty
> fit for the case when guest is much faster (e.g dpdk based
> backend). In this case, used ring is almost empty:
> 
> - we may get serious cache line misses/contending on both used ring
>   and used idx.
> - at most 1 packet could be dequeued at one time, batching in guest
>   does not make much effect.
> 
> Update used ring in a batch can help since guest won't access the used
> ring until used idx was advanced for several descriptors and since we
> advance used ring for every N packets, guest will only need to access
> used idx for every N packet since it can cache the used idx. To have a
> better interaction for both batch dequeuing and dpdk batching,
> VHOST_RX_BATCH was used as the maximum number of descriptors that
> could be batched.
> 
> Test were done between two machines with 2.40GHz Intel(R) Xeon(R) CPU
> E5-2630 connected back to back through ixgbe. Traffic were generated
> on one remote ixgbe through MoonGen and measure the RX pps through
> testpmd in guest when do xdp_redirect_map from local ixgbe to
> tap. RX pps were increased from 3.05 Mpps to 4.00 Mpps (about 31%
> improvement).
> 
> One possible concern for this is the implications for TCP (especially
> latency sensitive workload). Result[1] does not show obvious changes
> for most of the netperf test (RR, TX, and RX). And we do get some
> improvements for RX on some specific size.
 ...
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied, thanks Jason.

^ permalink raw reply

* Re: Question about eliminating virtio-scsi 30s timeout and hot-unplug
From: Hannes Reinecke @ 2018-01-10 19:39 UTC (permalink / raw)
  To: virtualization
In-Reply-To: <CAMmhGq+9qZhJRdaeXZtdu6AyJUPBKkbL4F2=NeXhvjUbxEoyow@mail.gmail.com>

On 12/21/2017 08:29 PM, Jason Behmer via Virtualization wrote:
> Hi Paolo,
> I had a question about this patch that eliminates virtio-scsi timeout on
> IOs - https://patchwork.kernel.org/patch/9802047/.  How does this work
> when we have IOs outstanding to a device that is then hot-unplugged. 
> I'm under the impression that those IOs will never get returned with any
> status (please correct me if I'm wrong), and we will then end up waiting
> on them forever with this patch.
> 
It's no different from the virtio-block path; the I/O is pending in the
qemu process. When a device is hot-unplugged the I/O will be returned to
the qemu process, which then will return the final status to the guest.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH 1/6] Documentation: crypto: document crypto engine API
From: Corentin Labbe @ 2018-01-10 19:14 UTC (permalink / raw)
  To: Fabien DESSENNE
  Cc: Benjamin GAIGNARD, herbert@gondor.apana.org.au, corbet@lwn.net,
	mst@redhat.com, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, mcoquelin.stm32@gmail.com,
	davem@davemloft.net, linux-arm-kernel@lists.infradead.org,
	Alexandre TORGUE
In-Reply-To: <849600f6-3082-c090-205d-75e3d14b38f9@st.com>

On Wed, Jan 10, 2018 at 02:13:13PM +0000, Fabien DESSENNE wrote:
> Hi Corentin,
> 
> 
> Thank you for this new version which I have testes successfully with the 
> stm32 hash & cryp drivers.
> 
> As a general comment on this patchset, I would say that it does not 
> cover all async requests: typically I need (for the pending stm32 cryp 
> driver uprade) to use CryptoEngine to process AEAD requests which is not 
> covered here.
> 
> Could you please consider adding the 'transfer' and 'finalize' EXPORTed 
> functions for aead requests? (the implementation is quite trivial)
> 
> Have also a look at struct acomp_req (acompress.h) and struct 
> kpp_request (kpp.h) which also use "struct crypto_async_request base"
> 
> 
> BR
> 
> Fabien
> 

Hello

Thanks for your review and test (could I add your tested-by ?).
I didn't add aead (and kpp/acompress), since I do not have any way to test it.
Since you have a way to test aead, I will add it to the next release.

Regards

> 
> On 03/01/18 21:11, Corentin Labbe wrote:
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >   Documentation/crypto/crypto_engine.rst | 46 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 46 insertions(+)
> >   create mode 100644 Documentation/crypto/crypto_engine.rst
> >
> > diff --git a/Documentation/crypto/crypto_engine.rst b/Documentation/crypto/crypto_engine.rst
> > new file mode 100644
> > index 000000000000..b0ed37f9fb0c
> > --- /dev/null
> > +++ b/Documentation/crypto/crypto_engine.rst
> > @@ -0,0 +1,46 @@
> > +=============
> > +CRYPTO ENGINE
> > +=============
> > +
> > +Overview
> > +--------
> > +The crypto engine API (CE), is a crypto queue manager.
> > +
> > +Requirement
> > +-----------
> > +You have to put at start of your tfm_ctx the struct crypto_engine_reqctx
> > +struct your_tfm_ctx {
> > +        struct crypto_engine_reqctx enginectx;
> > +        ...
> > +};
> > +Why: Since CE manage only crypto_async_request, it cannot know the underlying
> > +request_type and so have access only on the TFM.
> > +So using container_of for accessing __ctx is impossible.
> > +Furthermore, the crypto engine cannot know the "struct your_tfm_ctx",
> > +so it must assume that crypto_engine_reqctx is at start of it.
> > +
> > +Order of operations
> > +-------------------
> > +You have to obtain a struct crypto_engine via crypto_engine_alloc_init().
> > +And start it via crypto_engine_start().
> > +
> > +Before transferring any request, you have to fill the enginectx.
> > +- prepare_request: (taking a function pointer) If you need to do some processing before doing the request
> > +- unprepare_request: (taking a function pointer) Undoing what's done in prepare_request
> > +- do_one_request: (taking a function pointer) Do encryption for current request
> > +
> > +Note: that those three functions get the crypto_async_request associated with the received request.
> > +So your need to get the original request via container_of(areq, struct yourrequesttype_request, base);
> > +
> > +When your driver receive a crypto_request, you have to transfer it to
> > +the cryptoengine via one of:
> > +- crypto_transfer_cipher_request_to_engine()
> > +- crypto_transfer_skcipher_request_to_engine()
> > +- crypto_transfer_akcipher_request_to_engine()
> > +- crypto_transfer_hash_request_to_engine()
> > +
> > +At the end of the request process, a call to one of the following function is needed:
> > +- crypto_finalize_cipher_request
> > +- crypto_finalize_skcipher_request
> > +- crypto_finalize_akcipher_request
> > +- crypto_finalize_hash_request

^ permalink raw reply

* CFP SECRYPT 2018 - 15th Int.l Conf. on Security and Cryptography (Porto/Portugal)
From: icete @ 2018-01-10 17:17 UTC (permalink / raw)
  To: virtualization

SUBMISSION DEADLINE 

15th International Conference on Security and Cryptography

Submission Deadline: March 13, 2018

http://www.secrypt.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
SECRYPT Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.secrypt.icete.org/
e-mail: secrypt.secretariat@insticc.org

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

^ permalink raw reply

* CFP WINSYS 2018 - 15th Int.l Conf. on Wireless Networks and Mobile Systems (Porto/Portugal)
From: icete @ 2018-01-10 17:17 UTC (permalink / raw)
  To: virtualization

SUBMISSION DEADLINE 

15th International Conference on Wireless Networks and Mobile Systems

Submission Deadline: March 13, 2018

http://www.winsys.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 WINSYS is organized in 3 major tracks:

 - Sensor Networks and Ad Hoc Communications
 - Wireless and Mobile Technologies
 - Mobile Software and Services


 
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
WINSYS Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.winsys.icete.org/
e-mail: winsys.secretariat@insticc.org

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

^ permalink raw reply

* CFP DCNET 2018 - 9th Int.l Conf. on Data Communication Networking (Porto/Portugal)
From: icete @ 2018-01-10 17:17 UTC (permalink / raw)
  To: virtualization

SUBMISSION DEADLINE 

9th International Conference on Data Communication Networking

Submission Deadline: March 13, 2018

http://www.dcnet.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
DCNET Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.dcnet.icete.org/
e-mail: dcnet.secretariat@insticc.org

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

^ permalink raw reply

* CFP ICETE 2018 - 15th Int.l Joint Conf. on e-Business and Telecommunications (Porto/Portugal)
From: icete @ 2018-01-10 17:17 UTC (permalink / raw)
  To: virtualization

SUBMISSION DEADLINE 

15th International Joint Conference on e-Business and Telecommunications

Submission Deadline: March 13, 2018

http://www.icete.org/

July 26 - 28, 2018
Porto, Portugal.

 


 
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
ICETE Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.icete.org/
e-mail: icete.secretariat@insticc.org

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

^ permalink raw reply

* Re: [PATCH 6/6] crypto: stm32-cryp: convert to the new crypto engine API
From: Fabien DESSENNE @ 2018-01-10 14:25 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20180103201109.16077-7-clabbe.montjoie@gmail.com>



On 03/01/18 21:11, Corentin Labbe wrote:
> This patch convert the stm32-cryp driver to the new crypto engine API.
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>   drivers/crypto/stm32/stm32-cryp.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/stm32/stm32-cryp.c b/drivers/crypto/stm32/stm32-cryp.c
> index cf1dddbeaa2c..99e0473ef247 100644
> --- a/drivers/crypto/stm32/stm32-cryp.c
> +++ b/drivers/crypto/stm32/stm32-cryp.c
> @@ -91,6 +91,7 @@
>   #define _walked_out             (cryp->out_walk.offset - cryp->out_sg->offset)
>   
>   struct stm32_cryp_ctx {
> +	struct crypto_engine_reqctx enginectx;
>   	struct stm32_cryp       *cryp;
>   	int                     keylen;
>   	u32                     key[AES_KEYSIZE_256 / sizeof(u32)];
> @@ -494,10 +495,20 @@ static int stm32_cryp_cpu_start(struct stm32_cryp *cryp)
>   	return 0;
>   }
>   
> +static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
> +				     void *areq);

Merge these 2 lines in a single one

> +static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
> +					 void *areq);
> +
>   static int stm32_cryp_cra_init(struct crypto_tfm *tfm)
>   {
> +	struct stm32_cryp_ctx *ctx = crypto_tfm_ctx(tfm);
> +
>   	tfm->crt_ablkcipher.reqsize = sizeof(struct stm32_cryp_reqctx);
>   
> +	ctx->enginectx.op.do_one_request = stm32_cryp_cipher_one_req;
> +	ctx->enginectx.op.prepare_request = stm32_cryp_prepare_cipher_req;
> +	ctx->enginectx.op.unprepare_request = NULL;
>   	return 0;
>   }
>   
> @@ -695,14 +706,17 @@ static int stm32_cryp_prepare_req(struct crypto_engine *engine,
>   }
>   
>   static int stm32_cryp_prepare_cipher_req(struct crypto_engine *engine,
> -					 struct ablkcipher_request *req)
> +					 void *areq)
>   {
> +	struct ablkcipher_request *req = container_of(areq, struct ablkcipher_request, base);

 > 80 characters (CHECKPATCH)

> +
>   	return stm32_cryp_prepare_req(engine, req);
>   }
>   
>   static int stm32_cryp_cipher_one_req(struct crypto_engine *engine,
> -				     struct ablkcipher_request *req)
> +				     void *areq)

Merge these 2 lines in a single one

>   {
> +	struct ablkcipher_request *req = container_of(areq, struct ablkcipher_request, base);

 > 80 characters (CHECKPATCH)

>   	struct stm32_cryp_ctx *ctx = crypto_ablkcipher_ctx(
>   			crypto_ablkcipher_reqtfm(req));
>   	struct stm32_cryp *cryp = ctx->cryp;
> @@ -1104,9 +1118,6 @@ static int stm32_cryp_probe(struct platform_device *pdev)
>   		goto err_engine1;
>   	}
>   
> -	cryp->engine->prepare_cipher_request = stm32_cryp_prepare_cipher_req;
> -	cryp->engine->cipher_one_request = stm32_cryp_cipher_one_req;
> -
>   	ret = crypto_engine_start(cryp->engine);
>   	if (ret) {
>   		dev_err(dev, "Could not start crypto engine\n");

^ permalink raw reply

* Re: [PATCH 5/6] crypto: stm32-hash: convert to the new crypto engine API
From: Fabien DESSENNE @ 2018-01-10 14:24 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Lionel DEBIEVE, Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20180103201109.16077-6-clabbe.montjoie@gmail.com>



On 03/01/18 21:11, Corentin Labbe wrote:
> This patch convert the stm32-hash driver to the new crypto engine API.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>   drivers/crypto/stm32/stm32-hash.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/crypto/stm32/stm32-hash.c b/drivers/crypto/stm32/stm32-hash.c
> index 4ca4a264a833..9790c2c936c7 100644
> --- a/drivers/crypto/stm32/stm32-hash.c
> +++ b/drivers/crypto/stm32/stm32-hash.c
> @@ -122,6 +122,7 @@ enum stm32_hash_data_format {
>   #define HASH_DMA_THRESHOLD		50
>   
>   struct stm32_hash_ctx {
> +	struct crypto_engine_reqctx enginectx;
>   	struct stm32_hash_dev	*hdev;
>   	unsigned long		flags;
>   
> @@ -828,6 +829,11 @@ static int stm32_hash_hw_init(struct stm32_hash_dev *hdev,
>   	return 0;
>   }
>   
> +static int stm32_hash_one_request(struct crypto_engine *engine,
> +				  void *areq);

merge these two lines in a single one

> +static int stm32_hash_prepare_req(struct crypto_engine *engine,
> +				  void *areq);

merge these two lines in a single one

> +
>   static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>   				   struct ahash_request *req)
>   {
> @@ -835,8 +841,9 @@ static int stm32_hash_handle_queue(struct stm32_hash_dev *hdev,
>   }
>   
>   static int stm32_hash_prepare_req(struct crypto_engine *engine,
> -				  struct ahash_request *req)
> +				  void *areq)

merge these two lines in a single one

>   {
> +	struct ahash_request *req = container_of(areq, struct ahash_request, base);

 > 80 characters (CHECKPATCH)

>   	struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>   	struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>   	struct stm32_hash_request_ctx *rctx;
> @@ -855,8 +862,9 @@ static int stm32_hash_prepare_req(struct crypto_engine *engine,
>   }
>   
>   static int stm32_hash_one_request(struct crypto_engine *engine,
> -				  struct ahash_request *req)
> +				  void *areq)

merge these two lines in a single one

>   {
> +	struct ahash_request *req = container_of(areq, struct ahash_request, base);

 > 80 characters (CHECKPATCH)

>   	struct stm32_hash_ctx *ctx = crypto_ahash_ctx(crypto_ahash_reqtfm(req));
>   	struct stm32_hash_dev *hdev = stm32_hash_find_dev(ctx);
>   	struct stm32_hash_request_ctx *rctx;
> @@ -1033,6 +1041,9 @@ static int stm32_hash_cra_init_algs(struct crypto_tfm *tfm,
>   	if (algs_hmac_name)
>   		ctx->flags |= HASH_FLAGS_HMAC;
>   
> +	ctx->enginectx.op.do_one_request = stm32_hash_one_request;
> +	ctx->enginectx.op.prepare_request = stm32_hash_prepare_req;
> +	ctx->enginectx.op.unprepare_request = NULL;
>   	return 0;
>   }
>   
> @@ -1493,9 +1504,6 @@ static int stm32_hash_probe(struct platform_device *pdev)
>   		goto err_engine;
>   	}
>   
> -	hdev->engine->prepare_hash_request = stm32_hash_prepare_req;
> -	hdev->engine->hash_one_request = stm32_hash_one_request;
> -
>   	ret = crypto_engine_start(hdev->engine);
>   	if (ret)
>   		goto err_engine_start;

^ permalink raw reply

* Re: [PATCH 2/6] crypto: engine - Permit to enqueue all async requests
From: Fabien DESSENNE @ 2018-01-10 14:19 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20180103201109.16077-3-clabbe.montjoie@gmail.com>


On 03/01/18 21:11, Corentin Labbe wrote:
> The crypto engine could actually only enqueue hash and ablkcipher request.
> This patch permit it to enqueue any type of crypto_async_request.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>   crypto/crypto_engine.c  | 230 ++++++++++++++++++++++++------------------------
>   include/crypto/engine.h |  59 +++++++------
>   2 files changed, 148 insertions(+), 141 deletions(-)
>
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c
> index 61e7c4e02fd2..036270b61648 100644
> --- a/crypto/crypto_engine.c
> +++ b/crypto/crypto_engine.c
> @@ -15,7 +15,6 @@
>   #include <linux/err.h>
>   #include <linux/delay.h>
>   #include <crypto/engine.h>
> -#include <crypto/internal/hash.h>
>   #include <uapi/linux/sched/types.h>
>   #include "internal.h"
>   
> @@ -34,11 +33,10 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>   				 bool in_kthread)
>   {
>   	struct crypto_async_request *async_req, *backlog;
> -	struct ahash_request *hreq;
> -	struct ablkcipher_request *breq;
>   	unsigned long flags;
>   	bool was_busy = false;
> -	int ret, rtype;
> +	int ret;
> +	struct crypto_engine_reqctx *enginectx;
>   
>   	spin_lock_irqsave(&engine->queue_lock, flags);
>   
> @@ -94,7 +92,6 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>   
>   	spin_unlock_irqrestore(&engine->queue_lock, flags);
>   
> -	rtype = crypto_tfm_alg_type(engine->cur_req->tfm);
>   	/* Until here we get the request need to be encrypted successfully */
>   	if (!was_busy && engine->prepare_crypt_hardware) {
>   		ret = engine->prepare_crypt_hardware(engine);
> @@ -104,57 +101,31 @@ static void crypto_pump_requests(struct crypto_engine *engine,
>   		}
>   	}
>   
> -	switch (rtype) {
> -	case CRYPTO_ALG_TYPE_AHASH:
> -		hreq = ahash_request_cast(engine->cur_req);
> -		if (engine->prepare_hash_request) {
> -			ret = engine->prepare_hash_request(engine, hreq);
> -			if (ret) {
> -				dev_err(engine->dev, "failed to prepare request: %d\n",
> -					ret);
> -				goto req_err;
> -			}
> -			engine->cur_req_prepared = true;
> -		}
> -		ret = engine->hash_one_request(engine, hreq);
> -		if (ret) {
> -			dev_err(engine->dev, "failed to hash one request from queue\n");
> -			goto req_err;
> -		}
> -		return;
> -	case CRYPTO_ALG_TYPE_ABLKCIPHER:
> -		breq = ablkcipher_request_cast(engine->cur_req);
> -		if (engine->prepare_cipher_request) {
> -			ret = engine->prepare_cipher_request(engine, breq);
> -			if (ret) {
> -				dev_err(engine->dev, "failed to prepare request: %d\n",
> -					ret);
> -				goto req_err;
> -			}
> -			engine->cur_req_prepared = true;
> -		}
> -		ret = engine->cipher_one_request(engine, breq);
> +	enginectx = crypto_tfm_ctx(async_req->tfm);
> +
> +	if (enginectx->op.prepare_request) {
> +		ret = enginectx->op.prepare_request(engine, async_req);
>   		if (ret) {
> -			dev_err(engine->dev, "failed to cipher one request from queue\n");
> +			dev_err(engine->dev, "failed to prepare request: %d\n",
> +				ret);
>   			goto req_err;
>   		}
> -		return;
> -	default:
> -		dev_err(engine->dev, "failed to prepare request of unknown type\n");
> -		return;
> +		engine->cur_req_prepared = true;
> +	}
> +	if (!enginectx->op.do_one_request) {
> +		dev_err(engine->dev, "failed to do request\n");
> +		ret = -EINVAL;
> +		goto req_err;
> +	}
> +	ret = enginectx->op.do_one_request(engine, async_req);
> +	if (ret) {
> +		dev_err(engine->dev, "Failed to do one request from queue: %d\n", ret);
> +		goto req_err;
>   	}
> +	return;
>   
>   req_err:
> -	switch (rtype) {
> -	case CRYPTO_ALG_TYPE_AHASH:
> -		hreq = ahash_request_cast(engine->cur_req);
> -		crypto_finalize_hash_request(engine, hreq, ret);
> -		break;
> -	case CRYPTO_ALG_TYPE_ABLKCIPHER:
> -		breq = ablkcipher_request_cast(engine->cur_req);
> -		crypto_finalize_cipher_request(engine, breq, ret);
> -		break;
> -	}
> +	crypto_finalize_request(engine, async_req, ret);
>   	return;
>   
>   out:
> @@ -170,13 +141,12 @@ static void crypto_pump_work(struct kthread_work *work)
>   }
>   
>   /**
> - * crypto_transfer_cipher_request - transfer the new request into the
> - * enginequeue
> + * crypto_transfer_request - transfer the new request into the engine queue
>    * @engine: the hardware engine
>    * @req: the request need to be listed into the engine queue
>    */
> -int crypto_transfer_cipher_request(struct crypto_engine *engine,
> -				   struct ablkcipher_request *req,
> +static int crypto_transfer_request(struct crypto_engine *engine,
> +				   struct crypto_async_request *req,
>   				   bool need_pump)
>   {
>   	unsigned long flags;
> @@ -189,7 +159,7 @@ int crypto_transfer_cipher_request(struct crypto_engine *engine,
>   		return -ESHUTDOWN;
>   	}
>   
> -	ret = ablkcipher_enqueue_request(&engine->queue, req);
> +	ret = crypto_enqueue_request(&engine->queue, req);
>   
>   	if (!engine->busy && need_pump)
>   		kthread_queue_work(engine->kworker, &engine->pump_requests);
> @@ -197,85 +167,97 @@ int crypto_transfer_cipher_request(struct crypto_engine *engine,
>   	spin_unlock_irqrestore(&engine->queue_lock, flags);
>   	return ret;
>   }
> -EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request);
> +EXPORT_SYMBOL_GPL(crypto_transfer_request);

Do not export this function which is a static one.

>   
>   /**
> - * crypto_transfer_cipher_request_to_engine - transfer one request to list
> + * crypto_transfer_request_to_engine - transfer one request to list
>    * into the engine queue
>    * @engine: the hardware engine
>    * @req: the request need to be listed into the engine queue
>    */
> +static int crypto_transfer_request_to_engine(struct crypto_engine *engine,
> +					     struct crypto_async_request *req)
> +{
> +	return crypto_transfer_request(engine, req, true);
> +}
> +
> +/**
> + * crypto_transfer_cipher_request_to_engine - transfer one ablkcipher_request
> + * to list into the engine queue
> + * @engine: the hardware engine
> + * @req: the request need to be listed into the engine queue
> + * TODO: Remove this function when skcipher conversion is finished
> + */
>   int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
>   					     struct ablkcipher_request *req)
>   {
> -	return crypto_transfer_cipher_request(engine, req, true);
> +	return crypto_transfer_request_to_engine(engine, &req->base);
>   }
>   EXPORT_SYMBOL_GPL(crypto_transfer_cipher_request_to_engine);
>   
>   /**
> - * crypto_transfer_hash_request - transfer the new request into the
> - * enginequeue
> + * crypto_transfer_skcipher_request_to_engine - transfer one skcipher_request
> + * to list into the engine queue
>    * @engine: the hardware engine
>    * @req: the request need to be listed into the engine queue
>    */
> -int crypto_transfer_hash_request(struct crypto_engine *engine,
> -				 struct ahash_request *req, bool need_pump)
> +int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
> +					       struct skcipher_request *req)
>   {
> -	unsigned long flags;
> -	int ret;
> -
> -	spin_lock_irqsave(&engine->queue_lock, flags);
> -
> -	if (!engine->running) {
> -		spin_unlock_irqrestore(&engine->queue_lock, flags);
> -		return -ESHUTDOWN;
> -	}
> -
> -	ret = ahash_enqueue_request(&engine->queue, req);
> -
> -	if (!engine->busy && need_pump)
> -		kthread_queue_work(engine->kworker, &engine->pump_requests);
> +	return crypto_transfer_request_to_engine(engine, &req->base);
> +}
> +EXPORT_SYMBOL_GPL(crypto_transfer_skcipher_request_to_engine);
>   
> -	spin_unlock_irqrestore(&engine->queue_lock, flags);
> -	return ret;
> +/**
> + * crypto_transfer_akcipher_request_to_engine - transfer one akcipher_request
> + * to list into the engine queue
> + * @engine: the hardware engine
> + * @req: the request need to be listed into the engine queue
> + */
> +int crypto_transfer_akcipher_request_to_engine(struct crypto_engine *engine,
> +					       struct akcipher_request *req)
> +{
> +	return crypto_transfer_request_to_engine(engine, &req->base);
>   }
> -EXPORT_SYMBOL_GPL(crypto_transfer_hash_request);
> +EXPORT_SYMBOL_GPL(crypto_transfer_akcipher_request_to_engine);
>   
>   /**
> - * crypto_transfer_hash_request_to_engine - transfer one request to list
> - * into the engine queue
> + * crypto_transfer_hash_request_to_engine - transfer one ahash_request
> + * to list into the engine queue
>    * @engine: the hardware engine
>    * @req: the request need to be listed into the engine queue
>    */
>   int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
>   					   struct ahash_request *req)
>   {
> -	return crypto_transfer_hash_request(engine, req, true);
> +	return crypto_transfer_request_to_engine(engine, &req->base);
>   }
>   EXPORT_SYMBOL_GPL(crypto_transfer_hash_request_to_engine);
>   

Please add this EXPORTed function:

crypto_transfer_aead_request_to_engine(struct crypto_engine *engine, 
struct aead_request *req)

>   /**
> - * crypto_finalize_cipher_request - finalize one request if the request is done
> + * crypto_finalize_request - finalize one request if the request is done
>    * @engine: the hardware engine
>    * @req: the request need to be finalized
>    * @err: error number
>    */
> -void crypto_finalize_cipher_request(struct crypto_engine *engine,
> -				    struct ablkcipher_request *req, int err)
> +void crypto_finalize_request(struct crypto_engine *engine,

shall be static

> +			     struct crypto_async_request *req, int err)
>   {
>   	unsigned long flags;
>   	bool finalize_cur_req = false;
>   	int ret;
> +	struct crypto_engine_reqctx *enginectx;
>   
>   	spin_lock_irqsave(&engine->queue_lock, flags);
> -	if (engine->cur_req == &req->base)
> +	if (engine->cur_req == req)
>   		finalize_cur_req = true;
>   	spin_unlock_irqrestore(&engine->queue_lock, flags);
>   
>   	if (finalize_cur_req) {
> +		enginectx = crypto_tfm_ctx(req->tfm);
>   		if (engine->cur_req_prepared &&
> -		    engine->unprepare_cipher_request) {
> -			ret = engine->unprepare_cipher_request(engine, req);
> +		    enginectx->op.unprepare_request) {
> +			ret = enginectx->op.unprepare_request(engine, req);
>   			if (ret)
>   				dev_err(engine->dev, "failed to unprepare request\n");
>   		}
> @@ -285,46 +267,64 @@ void crypto_finalize_cipher_request(struct crypto_engine *engine,
>   		spin_unlock_irqrestore(&engine->queue_lock, flags);
>   	}
>   
> -	req->base.complete(&req->base, err);
> +	req->complete(req, err);
>   
>   	kthread_queue_work(engine->kworker, &engine->pump_requests);
>   }
> -EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
>   
>   /**
> - * crypto_finalize_hash_request - finalize one request if the request is done
> + * crypto_finalize_cipher_request - finalize one ablkcipher_request if
> + * the request is done
>    * @engine: the hardware engine
>    * @req: the request need to be finalized
>    * @err: error number
>    */
> -void crypto_finalize_hash_request(struct crypto_engine *engine,
> -				  struct ahash_request *req, int err)
> +void crypto_finalize_cipher_request(struct crypto_engine *engine,
> +				    struct ablkcipher_request *req, int err)
>   {
> -	unsigned long flags;
> -	bool finalize_cur_req = false;
> -	int ret;
> -
> -	spin_lock_irqsave(&engine->queue_lock, flags);
> -	if (engine->cur_req == &req->base)
> -		finalize_cur_req = true;
> -	spin_unlock_irqrestore(&engine->queue_lock, flags);
> +	return crypto_finalize_request(engine, &req->base, err);
> +}
> +EXPORT_SYMBOL_GPL(crypto_finalize_cipher_request);
>   
> -	if (finalize_cur_req) {
> -		if (engine->cur_req_prepared &&
> -		    engine->unprepare_hash_request) {
> -			ret = engine->unprepare_hash_request(engine, req);
> -			if (ret)
> -				dev_err(engine->dev, "failed to unprepare request\n");
> -		}
> -		spin_lock_irqsave(&engine->queue_lock, flags);
> -		engine->cur_req = NULL;
> -		engine->cur_req_prepared = false;
> -		spin_unlock_irqrestore(&engine->queue_lock, flags);
> -	}
> +/**
> + * crypto_finalize_skcipher_request - finalize one skcipher_request if
> + * the request is done
> + * @engine: the hardware engine
> + * @req: the request need to be finalized
> + * @err: error number
> + */
> +void crypto_finalize_skcipher_request(struct crypto_engine *engine,
> +				      struct skcipher_request *req, int err)
> +{
> +	return crypto_finalize_request(engine, &req->base, err);
> +}
> +EXPORT_SYMBOL_GPL(crypto_finalize_skcipher_request);
>   
> -	req->base.complete(&req->base, err);
> +/**
> + * crypto_finalize_akcipher_request - finalize one akcipher_request if
> + * the request is done
> + * @engine: the hardware engine
> + * @req: the request need to be finalized
> + * @err: error number
> + */
> +void crypto_finalize_akcipher_request(struct crypto_engine *engine,
> +				      struct akcipher_request *req, int err)
> +{
> +	return crypto_finalize_request(engine, &req->base, err);
> +}
> +EXPORT_SYMBOL_GPL(crypto_finalize_akcipher_request);
>   
> -	kthread_queue_work(engine->kworker, &engine->pump_requests);
> +/**
> + * crypto_finalize_hash_request - finalize one ahash_request if
> + * the request is done
> + * @engine: the hardware engine
> + * @req: the request need to be finalized
> + * @err: error number
> + */
> +void crypto_finalize_hash_request(struct crypto_engine *engine,
> +				  struct ahash_request *req, int err)
> +{
> +	return crypto_finalize_request(engine, &req->base, err);
>   }
>   EXPORT_SYMBOL_GPL(crypto_finalize_hash_request);

Add
crypto_finalize_aead_request(struct crypto_engine *engine, struct 
aead_request *req, int err)

>   
> diff --git a/include/crypto/engine.h b/include/crypto/engine.h
> index dd04c1699b51..1ea7cbe92eaf 100644
> --- a/include/crypto/engine.h
> +++ b/include/crypto/engine.h
> @@ -17,7 +17,9 @@
>   #include <linux/kernel.h>
>   #include <linux/kthread.h>
>   #include <crypto/algapi.h>
> +#include <crypto/akcipher.h>
>   #include <crypto/hash.h>
> +#include <crypto/skcipher.h>
>   
>   #define ENGINE_NAME_LEN	30
>   /*
> @@ -37,12 +39,6 @@
>    * @unprepare_crypt_hardware: there are currently no more requests on the
>    * queue so the subsystem notifies the driver that it may relax the
>    * hardware by issuing this call
> - * @prepare_cipher_request: do some prepare if need before handle the current request
> - * @unprepare_cipher_request: undo any work done by prepare_cipher_request()
> - * @cipher_one_request: do encryption for current request
> - * @prepare_hash_request: do some prepare if need before handle the current request
> - * @unprepare_hash_request: undo any work done by prepare_hash_request()
> - * @hash_one_request: do hash for current request
>    * @kworker: kthread worker struct for request pump
>    * @pump_requests: work struct for scheduling work to the request pump
>    * @priv_data: the engine private data
> @@ -65,19 +61,6 @@ struct crypto_engine {
>   	int (*prepare_crypt_hardware)(struct crypto_engine *engine);
>   	int (*unprepare_crypt_hardware)(struct crypto_engine *engine);
>   
> -	int (*prepare_cipher_request)(struct crypto_engine *engine,
> -				      struct ablkcipher_request *req);
> -	int (*unprepare_cipher_request)(struct crypto_engine *engine,
> -					struct ablkcipher_request *req);
> -	int (*prepare_hash_request)(struct crypto_engine *engine,
> -				    struct ahash_request *req);
> -	int (*unprepare_hash_request)(struct crypto_engine *engine,
> -				      struct ahash_request *req);
> -	int (*cipher_one_request)(struct crypto_engine *engine,
> -				  struct ablkcipher_request *req);
> -	int (*hash_one_request)(struct crypto_engine *engine,
> -				struct ahash_request *req);
> -
>   	struct kthread_worker           *kworker;
>   	struct kthread_work             pump_requests;
>   
> @@ -85,19 +68,43 @@ struct crypto_engine {
>   	struct crypto_async_request	*cur_req;
>   };
>   
> -int crypto_transfer_cipher_request(struct crypto_engine *engine,
> -				   struct ablkcipher_request *req,
> -				   bool need_pump);
> +/*
> + * struct crypto_engine_op - crypto hardware engine operations
> + * @prepare__request: do some prepare if need before handle the current request
> + * @unprepare_request: undo any work done by prepare_request()
> + * @do_one_request: do encryption for current request
> + */
> +struct crypto_engine_op {
> +	int (*prepare_request)(struct crypto_engine *engine,
> +			       void *areq);
> +	int (*unprepare_request)(struct crypto_engine *engine,
> +				 void *areq);
> +	int (*do_one_request)(struct crypto_engine *engine,
> +			      void *areq);
> +};
> +
> +struct crypto_engine_reqctx {
> +	struct crypto_engine_op op;
> +};
> +
> +int crypto_transfer_akcipher_request_to_engine(struct crypto_engine *engine,
> +					       struct akcipher_request *req);
>   int crypto_transfer_cipher_request_to_engine(struct crypto_engine *engine,
> -					     struct ablkcipher_request *req);
> -int crypto_transfer_hash_request(struct crypto_engine *engine,
> -				 struct ahash_request *req, bool need_pump);
> +				      struct ablkcipher_request *req);
>   int crypto_transfer_hash_request_to_engine(struct crypto_engine *engine,
> -					   struct ahash_request *req);
> +					       struct ahash_request *req);
> +int crypto_transfer_skcipher_request_to_engine(struct crypto_engine *engine,
> +					       struct skcipher_request *req);

+ transfer_aead

> +void crypto_finalize_request(struct crypto_engine *engine,
> +			     struct crypto_async_request *req, int err);

static (+move to  .c file?)

> +void crypto_finalize_akcipher_request(struct crypto_engine *engine,
> +				      struct akcipher_request *req, int err);
>   void crypto_finalize_cipher_request(struct crypto_engine *engine,
>   				    struct ablkcipher_request *req, int err);
>   void crypto_finalize_hash_request(struct crypto_engine *engine,
>   				  struct ahash_request *req, int err);
> +void crypto_finalize_skcipher_request(struct crypto_engine *engine,
> +				      struct skcipher_request *req, int err);

+ finalize_aead

>   int crypto_engine_start(struct crypto_engine *engine);
>   int crypto_engine_stop(struct crypto_engine *engine);
>   struct crypto_engine *crypto_engine_alloc_init(struct device *dev, bool rt);
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH 1/6] Documentation: crypto: document crypto engine API
From: Fabien DESSENNE @ 2018-01-10 14:13 UTC (permalink / raw)
  To: Corentin Labbe, Alexandre TORGUE, arei.gonglei@huawei.com,
	corbet@lwn.net, davem@davemloft.net, herbert@gondor.apana.org.au,
	jasowang@redhat.com, mcoquelin.stm32@gmail.com, mst@redhat.com
  Cc: Benjamin GAIGNARD, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20180103201109.16077-2-clabbe.montjoie@gmail.com>

Hi Corentin,


Thank you for this new version which I have testes successfully with the 
stm32 hash & cryp drivers.

As a general comment on this patchset, I would say that it does not 
cover all async requests: typically I need (for the pending stm32 cryp 
driver uprade) to use CryptoEngine to process AEAD requests which is not 
covered here.

Could you please consider adding the 'transfer' and 'finalize' EXPORTed 
functions for aead requests? (the implementation is quite trivial)

Have also a look at struct acomp_req (acompress.h) and struct 
kpp_request (kpp.h) which also use "struct crypto_async_request base"


BR

Fabien


On 03/01/18 21:11, Corentin Labbe wrote:
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>   Documentation/crypto/crypto_engine.rst | 46 ++++++++++++++++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>   create mode 100644 Documentation/crypto/crypto_engine.rst
>
> diff --git a/Documentation/crypto/crypto_engine.rst b/Documentation/crypto/crypto_engine.rst
> new file mode 100644
> index 000000000000..b0ed37f9fb0c
> --- /dev/null
> +++ b/Documentation/crypto/crypto_engine.rst
> @@ -0,0 +1,46 @@
> +=============
> +CRYPTO ENGINE
> +=============
> +
> +Overview
> +--------
> +The crypto engine API (CE), is a crypto queue manager.
> +
> +Requirement
> +-----------
> +You have to put at start of your tfm_ctx the struct crypto_engine_reqctx
> +struct your_tfm_ctx {
> +        struct crypto_engine_reqctx enginectx;
> +        ...
> +};
> +Why: Since CE manage only crypto_async_request, it cannot know the underlying
> +request_type and so have access only on the TFM.
> +So using container_of for accessing __ctx is impossible.
> +Furthermore, the crypto engine cannot know the "struct your_tfm_ctx",
> +so it must assume that crypto_engine_reqctx is at start of it.
> +
> +Order of operations
> +-------------------
> +You have to obtain a struct crypto_engine via crypto_engine_alloc_init().
> +And start it via crypto_engine_start().
> +
> +Before transferring any request, you have to fill the enginectx.
> +- prepare_request: (taking a function pointer) If you need to do some processing before doing the request
> +- unprepare_request: (taking a function pointer) Undoing what's done in prepare_request
> +- do_one_request: (taking a function pointer) Do encryption for current request
> +
> +Note: that those three functions get the crypto_async_request associated with the received request.
> +So your need to get the original request via container_of(areq, struct yourrequesttype_request, base);
> +
> +When your driver receive a crypto_request, you have to transfer it to
> +the cryptoengine via one of:
> +- crypto_transfer_cipher_request_to_engine()
> +- crypto_transfer_skcipher_request_to_engine()
> +- crypto_transfer_akcipher_request_to_engine()
> +- crypto_transfer_hash_request_to_engine()
> +
> +At the end of the request process, a call to one of the following function is needed:
> +- crypto_finalize_cipher_request
> +- crypto_finalize_skcipher_request
> +- crypto_finalize_akcipher_request
> +- crypto_finalize_hash_request

^ permalink raw reply

* Re: [PATCH v21 2/5 RESEND] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2018-01-10 10:26 UTC (permalink / raw)
  To: Tetsuo Handa, virtio-dev, linux-kernel, qemu-devel,
	virtualization, kvm, linux-mm, mst, mhocko, akpm, mawilcox
  Cc: aarcange, yang.zhang.wz, riel, quan.xu0, liliang.opensource,
	willy, amit.shah, cornelia.huck, pbonzini, nilal, mgorman
In-Reply-To: <201801092342.FCH56215.LJHOMVFFFOOSQt@I-love.SAKURA.ne.jp>

On 01/09/2018 10:42 PM, Tetsuo Handa wrote:
> Wei Wang wrote:
>> - enable OOM to free inflated pages maintained in the local temporary
>>    list.
> I do want to see it before applying this patch.


Fine, then what do you think of the method I shared in your post here: 
https://patchwork.kernel.org/patch/10140731/

Michael, could we merge patch 3-5 first?


>
> Please carefully check how the xbitmap implementation works, and you will
> find that you are adding a lot of redundant operations with a bug.

This version mainly added some test cases, and it passes the test run 
without any issue. Appreciate it if your comments could be more 
specific, that would make the discussion more effective, for example, I 
deliberately added "xb_find_set(xb1, 2, ULONG_MAX - 3)" for the overflow 
test, not sure if this is the "bug" you referred to, but I'm glad to 
hear your different thought.

I agree that some tests may be repeated in some degree, since we test 
the implementation from different aspects, for example, 
xbitmap_check_bit_range() may have already performed xb_zero() while we 
specifically have another xbitmap_check_zero_bits() which may test 
something that has already been tested when checking bit range. But I 
think testing twice is better than omission.
Also, I left the "Regualr test1: node=NULL" case though the new 
implementation doesn't explicitly use "node" as before, but that 
node=NULL is still a radix tree implementation internally and that case 
looks special to me, so maybe not bad to cover in the test.

You are also welcome to send a patch to remove the redundant one if you 
think that's an issue. Thanks.

Best,
Wei

^ permalink raw reply

* Re: [PATCH net-next v4 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
From: David Miller @ 2018-01-09 16:38 UTC (permalink / raw)
  To: jbaron; +Cc: virtio-dev, mst, netdev, qemu-devel, virtualization
In-Reply-To: <718139aaea9a1ef032b29429af570fb171fdacac.1515190238.git.jbaron@akamai.com>

From: Jason Baron <jbaron@akamai.com>
Date: Fri,  5 Jan 2018 17:44:54 -0500

> The ability to set speed and duplex for virtio_net is useful in various
> scenarios as described here:
> 
> 16032be virtio_net: add ethtool support for set and get of settings
> 
> However, it would be nice to be able to set this from the hypervisor,
> such that virtio_net doesn't require custom guest ethtool commands.
> 
> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> the hypervisor to export a linkspeed and duplex setting. The user can
> subsequently overwrite it later if desired via: 'ethtool -s'.
> 
> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> is that device feature bits are to grow down from bit 63, since the
> transports are starting from bit 24 and growing up.
> 
> Signed-off-by: Jason Baron <jbaron@akamai.com>

Applied, thanks Jason.

^ permalink raw reply

* Re: [PATCH net-next] vhost_net: batch used ring update in rx
From: Michael S. Tsirkin @ 2018-01-09 15:44 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, willemb, linux-kernel, kvm, virtualization
In-Reply-To: <1515493665-35306-1-git-send-email-jasowang@redhat.com>

On Tue, Jan 09, 2018 at 06:27:45PM +0800, Jason Wang wrote:
> This patch tries to batched used ring update during RX. This is pretty
> fit for the case when guest is much faster (e.g dpdk based
> backend). In this case, used ring is almost empty:
> 
> - we may get serious cache line misses/contending on both used ring
>   and used idx.
> - at most 1 packet could be dequeued at one time, batching in guest
>   does not make much effect.
> 
> Update used ring in a batch can help since guest won't access the used
> ring until used idx was advanced for several descriptors and since we
> advance used ring for every N packets, guest will only need to access
> used idx for every N packet since it can cache the used idx. To have a
> better interaction for both batch dequeuing and dpdk batching,
> VHOST_RX_BATCH was used as the maximum number of descriptors that
> could be batched.
> 
> Test were done between two machines with 2.40GHz Intel(R) Xeon(R) CPU
> E5-2630 connected back to back through ixgbe. Traffic were generated
> on one remote ixgbe through MoonGen and measure the RX pps through
> testpmd in guest when do xdp_redirect_map from local ixgbe to
> tap. RX pps were increased from 3.05 Mpps to 4.00 Mpps (about 31%
> improvement).
> 
> One possible concern for this is the implications for TCP (especially
> latency sensitive workload). Result[1] does not show obvious changes
> for most of the netperf test (RR, TX, and RX). And we do get some
> improvements for RX on some specific size.
> 
> Guest RX:
> 
> size/sessions/+thu%/+normalize%
>    64/     1/   +2%/   +2%
>    64/     2/   +2%/   -1%
>    64/     4/   +1%/   +1%
>    64/     8/    0%/    0%
>   256/     1/   +6%/   -3%
>   256/     2/   -3%/   +2%
>   256/     4/  +11%/  +11%
>   256/     8/    0%/    0%
>   512/     1/   +4%/    0%
>   512/     2/   +2%/   +2%
>   512/     4/    0%/   -1%
>   512/     8/   -8%/   -8%
>  1024/     1/   -7%/  -17%
>  1024/     2/   -8%/   -7%
>  1024/     4/   +1%/    0%
>  1024/     8/    0%/    0%
>  2048/     1/  +30%/  +14%
>  2048/     2/  +46%/  +40%
>  2048/     4/    0%/    0%
>  2048/     8/    0%/    0%
>  4096/     1/  +23%/  +22%
>  4096/     2/  +26%/  +23%
>  4096/     4/    0%/   +1%
>  4096/     8/    0%/    0%
> 16384/     1/   -2%/   -3%
> 16384/     2/   +1%/   -4%
> 16384/     4/   -1%/   -3%
> 16384/     8/    0%/   -1%
> 65535/     1/  +15%/   +7%
> 65535/     2/   +4%/   +7%
> 65535/     4/    0%/   +1%
> 65535/     8/    0%/    0%
> 
> TCP_RR:
> 
> size/sessions/+thu%/+normalize%
>     1/     1/    0%/   +1%
>     1/    25/   +2%/   +1%
>     1/    50/   +4%/   +1%
>    64/     1/    0%/   -4%
>    64/    25/   +2%/   +1%
>    64/    50/    0%/   -1%
>   256/     1/    0%/    0%
>   256/    25/    0%/    0%
>   256/    50/   +4%/   +2%
> 
> Guest TX:
> 
> size/sessions/+thu%/+normalize%
>    64/     1/   +4%/   -2%
>    64/     2/   -6%/   -5%
>    64/     4/   +3%/   +6%
>    64/     8/    0%/   +3%
>   256/     1/  +15%/  +16%
>   256/     2/  +11%/  +12%
>   256/     4/   +1%/    0%
>   256/     8/   +5%/   +5%
>   512/     1/   -1%/   -6%
>   512/     2/    0%/   -8%
>   512/     4/   -2%/   +4%
>   512/     8/   +6%/   +9%
>  1024/     1/   +3%/   +1%
>  1024/     2/   +3%/   +9%
>  1024/     4/    0%/   +7%
>  1024/     8/    0%/   +7%
>  2048/     1/   +8%/   +2%
>  2048/     2/   +3%/   -1%
>  2048/     4/   -1%/  +11%
>  2048/     8/   +3%/   +9%
>  4096/     1/   +8%/   +8%
>  4096/     2/    0%/   -7%
>  4096/     4/   +4%/   +4%
>  4096/     8/   +2%/   +5%
> 16384/     1/   -3%/   +1%
> 16384/     2/   -1%/  -12%
> 16384/     4/   -1%/   +5%
> 16384/     8/    0%/   +1%
> 65535/     1/    0%/   -3%
> 65535/     2/   +5%/  +16%
> 65535/     4/   +1%/   +2%
> 65535/     8/   +1%/   -1%
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>

I keep wondering whether we want control over this
from the guest (e.g. ethtool).

And I guess UDP_RR would be a better test.

But overall I agree it's a good default.

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


> ---
>  drivers/vhost/net.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index c7bdeb6..988af72 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -744,7 +744,7 @@ static void handle_rx(struct vhost_net *net)
>  	};
>  	size_t total_len = 0;
>  	int err, mergeable;
> -	s16 headcount;
> +	s16 headcount, nheads = 0;
>  	size_t vhost_hlen, sock_hlen;
>  	size_t vhost_len, sock_len;
>  	struct socket *sock;
> @@ -772,7 +772,7 @@ static void handle_rx(struct vhost_net *net)
>  	while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
>  		sock_len += sock_hlen;
>  		vhost_len = sock_len + vhost_hlen;
> -		headcount = get_rx_bufs(vq, vq->heads, vhost_len,
> +		headcount = get_rx_bufs(vq, vq->heads + nheads, vhost_len,
>  					&in, vq_log, &log,
>  					likely(mergeable) ? UIO_MAXIOV : 1);
>  		/* On error, stop handling until the next kick. */
> @@ -844,8 +844,12 @@ static void handle_rx(struct vhost_net *net)
>  			vhost_discard_vq_desc(vq, headcount);
>  			goto out;
>  		}
> -		vhost_add_used_and_signal_n(&net->dev, vq, vq->heads,
> -					    headcount);
> +		nheads += headcount;
> +		if (nheads > VHOST_RX_BATCH) {
> +			vhost_add_used_and_signal_n(&net->dev, vq, vq->heads,
> +						    nheads);
> +			nheads = 0;
> +		}
>  		if (unlikely(vq_log))
>  			vhost_log_write(vq, vq_log, log, vhost_len);
>  		total_len += vhost_len;
> @@ -856,6 +860,9 @@ static void handle_rx(struct vhost_net *net)
>  	}
>  	vhost_net_enable_vq(net, vq);
>  out:
> +	if (nheads)
> +		vhost_add_used_and_signal_n(&net->dev, vq, vq->heads,
> +					    nheads);
>  	mutex_unlock(&vq->mutex);
>  }
>  
> -- 
> 1.8.3.1

^ permalink raw reply

* [PATCH v2] vhost: Remove the unused variable.
From: Tonghao Zhang @ 2018-01-09 15:38 UTC (permalink / raw)
  To: kvm, virtualization, netdev, linux-kernel; +Cc: mst

The patch (7235acdb1) changed the way of the work
flushing in which the queued seq, done seq, and the
flushing are not used anymore. Then remove them now.

Fixes: 7235acdb1 ("vhost: simplify work flushing")
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 1 -
 drivers/vhost/vhost.h | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 33ac2b186b85..9b04cad91d65 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -181,7 +181,6 @@ void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
 {
 	clear_bit(VHOST_WORK_QUEUED, &work->flags);
 	work->fn = fn;
-	init_waitqueue_head(&work->done);
 }
 EXPORT_SYMBOL_GPL(vhost_work_init);
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 79c6e7a60a5e..749fe13e061c 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -20,10 +20,6 @@ typedef void (*vhost_work_fn_t)(struct vhost_work *work);
 struct vhost_work {
 	struct llist_node	  node;
 	vhost_work_fn_t		  fn;
-	wait_queue_head_t	  done;
-	int			  flushing;
-	unsigned		  queue_seq;
-	unsigned		  done_seq;
 	unsigned long		  flags;
 };
 
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH] drm/virtio: Add window server support
From: Tomeu Vizoso @ 2018-01-09 12:56 UTC (permalink / raw)
  To: dri-devel
  Cc: Tomeu Vizoso, Michael S. Tsirkin, David Airlie, open list,
	open list:VIRTIO GPU DRIVER, Zach Reizner
In-Reply-To: <20171228115333.12289-1-tomeu.vizoso@collabora.com>

On 28 December 2017 at 12:53, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> This is to allow clients running within VMs to be able to communicate
> with a compositor in the host. Clients will use the communication
> protocol that the compositor supports, and virtio-gpu will assist with
> making buffers available in both sides, and copying content as needed.

Here is the qemu side, a bit hackier atm:

https://gitlab.collabora.com/tomeu/qemu/commits/winsrv-wip

Regards,

Tomeu

> It is expected that a service in the guest will act as a proxy,
> interacting with virtio-gpu to support unmodified clients. For some
> features of the protocol, the hypervisor might have to intervene and
> also parse the protocol data to properly bridge resources. The following
> IOCTLs have been added to this effect:
>
> *_WINSRV_CONNECT: Opens a connection to the compositor in the host,
> returns a FD that represents this connection and on which the following
> IOCTLs can be used. Callers are expected to poll this FD for new
> messages from the compositor.
>
> *_WINSRV_TX: Asks the hypervisor to forward a message to the compositor
>
> *_WINSRV_RX: Returns all queued messages
>
> Alongside protocol data that is opaque to the kernel, the client can
> send file descriptors that reference GEM buffers allocated by
> virtio-gpu. The guest proxy is expected to figure out when a client is
> passing a FD that refers to shared memory in the guest and allocate a
> virtio-gpu buffer of the same size with DRM_VIRTGPU_RESOURCE_CREATE.
>
> When the client notifies the compositor that it can read from that buffer,
> the proxy should copy the contents from the SHM region to the virtio-gpu
> resource and call DRM_VIRTGPU_TRANSFER_TO_HOST.
>
> This has been tested with Wayland clients that make use of wl_shm to
> pass buffers to the compositor, but is expected to work similarly for X
> clients that make use of MIT-SHM with FD passing.
>
> v2: * Add padding to two virtio command structs
>     * Properly cast two __user pointers (kbuild test robot)
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Zach Reizner <zachr@google.com>
>
> ---
>
> Hi,
>
> this work is based on the virtio_wl driver in the ChromeOS kernel by
> Zach Reizner, currently at:
>
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.4/drivers/virtio/virtio_wl.c
>
> There's two features missing in this patch when compared with virtio_wl:
>
> * Allow the guest access directly host memory, without having to resort
> to TRANSFER_TO_HOST
>
> * Pass FDs from host to guest (Wayland specifies that the compositor
> shares keyboard data with the guest via a shared buffer)
>
> I plan to work on this next, but I would like to get some comments on
> the general approach so I can better choose which patch to follow.
>
> Thanks,
>
> Tomeu
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  39 ++++-
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 168 +++++++++++++++++++
>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  58 +++++--
>  drivers/gpu/drm/virtio/virtgpu_vq.c    | 285 ++++++++++++++++++++++++++++++++-
>  include/uapi/drm/virtgpu_drm.h         |  29 ++++
>  include/uapi/linux/virtio_gpu.h        |  41 +++++
>  6 files changed, 605 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index da2fb585fea4..268b386e1232 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -178,6 +178,8 @@ struct virtio_gpu_device {
>
>         struct virtio_gpu_queue ctrlq;
>         struct virtio_gpu_queue cursorq;
> +       struct virtio_gpu_queue winsrv_rxq;
> +       struct virtio_gpu_queue winsrv_txq;
>         struct kmem_cache *vbufs;
>         bool vqs_ready;
>
> @@ -205,10 +207,32 @@ struct virtio_gpu_device {
>
>  struct virtio_gpu_fpriv {
>         uint32_t ctx_id;
> +
> +       struct list_head winsrv_conns; /* list of virtio_gpu_winsrv_conn */
> +       spinlock_t winsrv_lock;
> +};
> +
> +struct virtio_gpu_winsrv_rx_qentry {
> +       struct virtio_gpu_winsrv_rx *cmd;
> +       struct list_head next;
> +};
> +
> +struct virtio_gpu_winsrv_conn {
> +       struct virtio_gpu_device *vgdev;
> +
> +       spinlock_t lock;
> +
> +       int fd;
> +       struct drm_file *drm_file;
> +
> +       struct list_head cmdq; /* queue of virtio_gpu_winsrv_rx_qentry */
> +       wait_queue_head_t cmdwq;
> +
> +       struct list_head next;
>  };
>
>  /* virtio_ioctl.c */
> -#define DRM_VIRTIO_NUM_IOCTLS 10
> +#define DRM_VIRTIO_NUM_IOCTLS 11
>  extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
>
>  /* virtio_kms.c */
> @@ -318,9 +342,22 @@ virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
>  void virtio_gpu_ctrl_ack(struct virtqueue *vq);
>  void virtio_gpu_cursor_ack(struct virtqueue *vq);
>  void virtio_gpu_fence_ack(struct virtqueue *vq);
> +void virtio_gpu_winsrv_tx_ack(struct virtqueue *vq);
> +void virtio_gpu_winsrv_rx_read(struct virtqueue *vq);
>  void virtio_gpu_dequeue_ctrl_func(struct work_struct *work);
>  void virtio_gpu_dequeue_cursor_func(struct work_struct *work);
> +void virtio_gpu_dequeue_winsrv_rx_func(struct work_struct *work);
> +void virtio_gpu_dequeue_winsrv_tx_func(struct work_struct *work);
>  void virtio_gpu_dequeue_fence_func(struct work_struct *work);
> +void virtio_gpu_fill_winsrv_rx(struct virtio_gpu_device *vgdev);
> +void virtio_gpu_queue_winsrv_rx_in(struct virtio_gpu_device *vgdev,
> +                                  struct virtio_gpu_winsrv_rx *cmd);
> +int virtio_gpu_cmd_winsrv_connect(struct virtio_gpu_device *vgdev, int fd);
> +void virtio_gpu_cmd_winsrv_disconnect(struct virtio_gpu_device *vgdev, int fd);
> +int virtio_gpu_cmd_winsrv_tx(struct virtio_gpu_device *vgdev,
> +                            const char __user *buffer, u32 len,
> +                            int *fds, struct virtio_gpu_winsrv_conn *conn,
> +                            bool nonblock);
>
>  /* virtio_gpu_display.c */
>  int virtio_gpu_framebuffer_init(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 0528edb4a2bf..630ed16d5f74 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -25,6 +25,9 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>
> +#include <linux/anon_inodes.h>
> +#include <linux/syscalls.h>
> +
>  #include <drm/drmP.h>
>  #include <drm/virtgpu_drm.h>
>  #include <drm/ttm/ttm_execbuf_util.h>
> @@ -527,6 +530,168 @@ static int virtio_gpu_get_caps_ioctl(struct drm_device *dev,
>         return 0;
>  }
>
> +static unsigned int winsrv_poll(struct file *filp,
> +                               struct poll_table_struct *wait)
> +{
> +       struct virtio_gpu_winsrv_conn *conn = filp->private_data;
> +       unsigned int mask = 0;
> +
> +       spin_lock(&conn->lock);
> +       poll_wait(filp, &conn->cmdwq, wait);
> +       if (!list_empty(&conn->cmdq))
> +               mask |= POLLIN | POLLRDNORM;
> +       spin_unlock(&conn->lock);
> +
> +       return mask;
> +}
> +
> +static int winsrv_ioctl_rx(struct virtio_gpu_device *vgdev,
> +                          struct virtio_gpu_winsrv_conn *conn,
> +                          struct drm_virtgpu_winsrv *cmd)
> +{
> +       struct virtio_gpu_winsrv_rx_qentry *qentry, *tmp;
> +       struct virtio_gpu_winsrv_rx *virtio_cmd;
> +       int available_len = cmd->len;
> +       int read_count = 0;
> +
> +       list_for_each_entry_safe(qentry, tmp, &conn->cmdq, next) {
> +               virtio_cmd = qentry->cmd;
> +               if (virtio_cmd->len > available_len)
> +                       return 0;
> +
> +               if (copy_to_user((void __user *)cmd->data + read_count,
> +                                virtio_cmd->data,
> +                                virtio_cmd->len)) {
> +                       /* return error unless we have some data to return */
> +                       if (read_count == 0)
> +                               return -EFAULT;
> +               }
> +
> +               /*
> +                * here we could export resource IDs to FDs, but no protocol
> +                * as of today requires it
> +                */
> +
> +               available_len -= virtio_cmd->len;
> +               read_count += virtio_cmd->len;
> +
> +               virtio_gpu_queue_winsrv_rx_in(vgdev, virtio_cmd);
> +
> +               list_del(&qentry->next);
> +               kfree(qentry);
> +       }
> +
> +       cmd->len = read_count;
> +
> +       return 0;
> +}
> +
> +static long winsrv_ioctl(struct file *filp, unsigned int cmd,
> +                        unsigned long arg)
> +{
> +       struct virtio_gpu_winsrv_conn *conn = filp->private_data;
> +       struct virtio_gpu_device *vgdev = conn->vgdev;
> +       struct drm_virtgpu_winsrv winsrv_cmd;
> +       int ret;
> +
> +       if (_IOC_SIZE(cmd) > sizeof(winsrv_cmd))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&winsrv_cmd, (void __user *)arg,
> +                          _IOC_SIZE(cmd)) != 0)
> +               return -EFAULT;
> +
> +       switch (cmd) {
> +       case DRM_IOCTL_VIRTGPU_WINSRV_RX:
> +               ret = winsrv_ioctl_rx(vgdev, conn, &winsrv_cmd);
> +               if (copy_to_user((void __user *)arg, &winsrv_cmd,
> +                                _IOC_SIZE(cmd)) != 0)
> +                       return -EFAULT;
> +
> +               break;
> +
> +       case DRM_IOCTL_VIRTGPU_WINSRV_TX:
> +               ret = virtio_gpu_cmd_winsrv_tx(vgdev,
> +                               u64_to_user_ptr(winsrv_cmd.data),
> +                               winsrv_cmd.len,
> +                               winsrv_cmd.fds,
> +                               conn,
> +                               filp->f_flags & O_NONBLOCK);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +       }
> +
> +       return ret;
> +}
> +
> +static int winsrv_release(struct inode *inodep, struct file *filp)
> +{
> +       struct virtio_gpu_winsrv_conn *conn = filp->private_data;
> +       struct virtio_gpu_device *vgdev = conn->vgdev;
> +
> +       virtio_gpu_cmd_winsrv_disconnect(vgdev, conn->fd);
> +
> +       list_del(&conn->next);
> +       kfree(conn);
> +
> +       return 0;
> +}
> +
> +static const struct file_operations winsrv_fops = {
> +
> +       .poll = winsrv_poll,
> +       .unlocked_ioctl = winsrv_ioctl,
> +       .release = winsrv_release,
> +};
> +
> +static int virtio_gpu_winsrv_connect(struct drm_device *dev, void *data,
> +                                    struct drm_file *file)
> +{
> +       struct virtio_gpu_device *vgdev = dev->dev_private;
> +       struct virtio_gpu_fpriv *vfpriv = file->driver_priv;
> +       struct drm_virtgpu_winsrv_connect *args = data;
> +       struct virtio_gpu_winsrv_conn *conn;
> +       int ret;
> +
> +       conn = kzalloc(sizeof(*conn), GFP_KERNEL);
> +       if (!conn)
> +               return -ENOMEM;
> +
> +       conn->vgdev = vgdev;
> +       conn->drm_file = file;
> +       spin_lock_init(&conn->lock);
> +       INIT_LIST_HEAD(&conn->cmdq);
> +       init_waitqueue_head(&conn->cmdwq);
> +
> +       ret = anon_inode_getfd("[virtgpu_winsrv]", &winsrv_fops, conn,
> +                              O_CLOEXEC | O_RDWR);
> +       if (ret < 0)
> +               goto free_conn;
> +
> +       conn->fd = ret;
> +
> +       ret = virtio_gpu_cmd_winsrv_connect(vgdev, conn->fd);
> +       if (ret < 0)
> +               goto close_fd;
> +
> +       spin_lock(&vfpriv->winsrv_lock);
> +       list_add_tail(&conn->next, &vfpriv->winsrv_conns);
> +       spin_unlock(&vfpriv->winsrv_lock);
> +
> +       args->fd = conn->fd;
> +
> +       return 0;
> +
> +close_fd:
> +       sys_close(conn->fd);
> +
> +free_conn:
> +       kfree(conn);
> +
> +       return ret;
> +}
> +
>  struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = {
>         DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl,
>                           DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
> @@ -558,4 +723,7 @@ struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = {
>
>         DRM_IOCTL_DEF_DRV(VIRTGPU_GET_CAPS, virtio_gpu_get_caps_ioctl,
>                           DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +
> +       DRM_IOCTL_DEF_DRV(VIRTGPU_WINSRV_CONNECT, virtio_gpu_winsrv_connect,
> +                         DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
>  };
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 6400506a06b0..ad7872037982 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -128,13 +128,15 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device *vgdev,
>  int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>  {
>         static vq_callback_t *callbacks[] = {
> -               virtio_gpu_ctrl_ack, virtio_gpu_cursor_ack
> +               virtio_gpu_ctrl_ack, virtio_gpu_cursor_ack,
> +               virtio_gpu_winsrv_rx_read, virtio_gpu_winsrv_tx_ack
>         };
> -       static const char * const names[] = { "control", "cursor" };
> +       static const char * const names[] = { "control", "cursor",
> +                                             "winsrv-rx", "winsrv-tx" };
>
>         struct virtio_gpu_device *vgdev;
>         /* this will expand later */
> -       struct virtqueue *vqs[2];
> +       struct virtqueue *vqs[4];
>         u32 num_scanouts, num_capsets;
>         int ret;
>
> @@ -158,6 +160,10 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>         init_waitqueue_head(&vgdev->resp_wq);
>         virtio_gpu_init_vq(&vgdev->ctrlq, virtio_gpu_dequeue_ctrl_func);
>         virtio_gpu_init_vq(&vgdev->cursorq, virtio_gpu_dequeue_cursor_func);
> +       virtio_gpu_init_vq(&vgdev->winsrv_rxq,
> +                          virtio_gpu_dequeue_winsrv_rx_func);
> +       virtio_gpu_init_vq(&vgdev->winsrv_txq,
> +                          virtio_gpu_dequeue_winsrv_tx_func);
>
>         vgdev->fence_drv.context = dma_fence_context_alloc(1);
>         spin_lock_init(&vgdev->fence_drv.lock);
> @@ -175,13 +181,15 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>         DRM_INFO("virgl 3d acceleration not supported by guest\n");
>  #endif
>
> -       ret = virtio_find_vqs(vgdev->vdev, 2, vqs, callbacks, names, NULL);
> +       ret = virtio_find_vqs(vgdev->vdev, 4, vqs, callbacks, names, NULL);
>         if (ret) {
>                 DRM_ERROR("failed to find virt queues\n");
>                 goto err_vqs;
>         }
>         vgdev->ctrlq.vq = vqs[0];
>         vgdev->cursorq.vq = vqs[1];
> +       vgdev->winsrv_rxq.vq = vqs[2];
> +       vgdev->winsrv_txq.vq = vqs[3];
>         ret = virtio_gpu_alloc_vbufs(vgdev);
>         if (ret) {
>                 DRM_ERROR("failed to alloc vbufs\n");
> @@ -215,6 +223,9 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>                 goto err_modeset;
>
>         virtio_device_ready(vgdev->vdev);
> +
> +       virtio_gpu_fill_winsrv_rx(vgdev);
> +
>         vgdev->vqs_ready = true;
>
>         if (num_capsets)
> @@ -256,6 +267,8 @@ void virtio_gpu_driver_unload(struct drm_device *dev)
>         vgdev->vqs_ready = false;
>         flush_work(&vgdev->ctrlq.dequeue_work);
>         flush_work(&vgdev->cursorq.dequeue_work);
> +       flush_work(&vgdev->winsrv_rxq.dequeue_work);
> +       flush_work(&vgdev->winsrv_txq.dequeue_work);
>         flush_work(&vgdev->config_changed_work);
>         vgdev->vdev->config->del_vqs(vgdev->vdev);
>
> @@ -274,25 +287,43 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file)
>         uint32_t id;
>         char dbgname[64], tmpname[TASK_COMM_LEN];
>
> -       /* can't create contexts without 3d renderer */
> -       if (!vgdev->has_virgl_3d)
> -               return 0;
> -
> -       get_task_comm(tmpname, current);
> -       snprintf(dbgname, sizeof(dbgname), "%s", tmpname);
> -       dbgname[63] = 0;
>         /* allocate a virt GPU context for this opener */
>         vfpriv = kzalloc(sizeof(*vfpriv), GFP_KERNEL);
>         if (!vfpriv)
>                 return -ENOMEM;
>
> -       virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname, &id);
> +       /* can't create contexts without 3d renderer */
> +       if (vgdev->has_virgl_3d) {
> +               get_task_comm(tmpname, current);
> +               snprintf(dbgname, sizeof(dbgname), "%s", tmpname);
> +               dbgname[63] = 0;
> +
> +               virtio_gpu_context_create(vgdev, strlen(dbgname), dbgname, &id);
> +
> +               vfpriv->ctx_id = id;
> +       }
> +
> +       spin_lock_init(&vfpriv->winsrv_lock);
> +       INIT_LIST_HEAD(&vfpriv->winsrv_conns);
>
> -       vfpriv->ctx_id = id;
>         file->driver_priv = vfpriv;
> +
>         return 0;
>  }
>
> +static void virtio_gpu_cleanup_conns(struct virtio_gpu_fpriv *vfpriv)
> +{
> +       struct virtio_gpu_winsrv_conn *conn, *tmp;
> +       struct virtio_gpu_winsrv_rx_qentry *qentry, *tmp2;
> +
> +       list_for_each_entry_safe(conn, tmp, &vfpriv->winsrv_conns, next) {
> +               list_for_each_entry_safe(qentry, tmp2, &conn->cmdq, next) {
> +                       kfree(qentry);
> +               }
> +               kfree(conn);
> +       }
> +}
> +
>  void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file)
>  {
>         struct virtio_gpu_device *vgdev = dev->dev_private;
> @@ -303,6 +334,7 @@ void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file *file)
>
>         vfpriv = file->driver_priv;
>
> +       virtio_gpu_cleanup_conns(vfpriv);
>         virtio_gpu_context_destroy(vgdev, vfpriv->ctx_id);
>         kfree(vfpriv);
>         file->driver_priv = NULL;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 9eb96fb2c147..ea5f9352d364 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -32,7 +32,7 @@
>  #include <linux/virtio_config.h>
>  #include <linux/virtio_ring.h>
>
> -#define MAX_INLINE_CMD_SIZE   96
> +#define MAX_INLINE_CMD_SIZE   144
>  #define MAX_INLINE_RESP_SIZE  24
>  #define VBUFFER_SIZE          (sizeof(struct virtio_gpu_vbuffer) \
>                                + MAX_INLINE_CMD_SIZE             \
> @@ -72,6 +72,67 @@ void virtio_gpu_cursor_ack(struct virtqueue *vq)
>         schedule_work(&vgdev->cursorq.dequeue_work);
>  }
>
> +void virtio_gpu_winsrv_rx_read(struct virtqueue *vq)
> +{
> +       struct drm_device *dev = vq->vdev->priv;
> +       struct virtio_gpu_device *vgdev = dev->dev_private;
> +
> +       schedule_work(&vgdev->winsrv_rxq.dequeue_work);
> +}
> +
> +void virtio_gpu_winsrv_tx_ack(struct virtqueue *vq)
> +{
> +       struct drm_device *dev = vq->vdev->priv;
> +       struct virtio_gpu_device *vgdev = dev->dev_private;
> +
> +       schedule_work(&vgdev->winsrv_txq.dequeue_work);
> +}
> +
> +void virtio_gpu_queue_winsrv_rx_in(struct virtio_gpu_device *vgdev,
> +                                  struct virtio_gpu_winsrv_rx *cmd)
> +{
> +       struct virtqueue *vq = vgdev->winsrv_rxq.vq;
> +       struct scatterlist sg[1];
> +       int ret;
> +
> +       sg_init_one(sg, cmd, sizeof(*cmd));
> +
> +       spin_lock(&vgdev->winsrv_rxq.qlock);
> +retry:
> +       ret = virtqueue_add_inbuf(vq, sg, 1, cmd, GFP_KERNEL);
> +       if (ret == -ENOSPC) {
> +               spin_unlock(&vgdev->winsrv_rxq.qlock);
> +               wait_event(vgdev->winsrv_rxq.ack_queue, vq->num_free);
> +               spin_lock(&vgdev->winsrv_rxq.qlock);
> +               goto retry;
> +       }
> +       virtqueue_kick(vq);
> +       spin_unlock(&vgdev->winsrv_rxq.qlock);
> +}
> +
> +void virtio_gpu_fill_winsrv_rx(struct virtio_gpu_device *vgdev)
> +{
> +       struct virtqueue *vq = vgdev->winsrv_rxq.vq;
> +       struct virtio_gpu_winsrv_rx *cmd;
> +       int ret = 0;
> +
> +       while (vq->num_free > 0) {
> +               cmd = kmalloc(sizeof(*cmd), GFP_KERNEL);
> +               if (!cmd) {
> +                       ret = -ENOMEM;
> +                       goto clear_queue;
> +               }
> +
> +               virtio_gpu_queue_winsrv_rx_in(vgdev, cmd);
> +       }
> +
> +       return;
> +
> +clear_queue:
> +       while ((cmd = virtqueue_detach_unused_buf(vq)))
> +               kfree(cmd);
> +}
> +
>  int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
>  {
>         vgdev->vbufs = kmem_cache_create("virtio-gpu-vbufs",
> @@ -258,6 +319,96 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
>         wake_up(&vgdev->cursorq.ack_queue);
>  }
>
> +void virtio_gpu_dequeue_winsrv_tx_func(struct work_struct *work)
> +{
> +       struct virtio_gpu_device *vgdev =
> +               container_of(work, struct virtio_gpu_device,
> +                            winsrv_txq.dequeue_work);
> +       struct virtio_gpu_vbuffer *vbuf;
> +       int len;
> +
> +       spin_lock(&vgdev->winsrv_txq.qlock);
> +       do {
> +               while ((vbuf = virtqueue_get_buf(vgdev->winsrv_txq.vq, &len)))
> +                       free_vbuf(vgdev, vbuf);
> +       } while (!virtqueue_enable_cb(vgdev->winsrv_txq.vq));
> +       spin_unlock(&vgdev->winsrv_txq.qlock);
> +
> +       wake_up(&vgdev->winsrv_txq.ack_queue);
> +}
> +
> +static struct virtio_gpu_winsrv_conn *find_conn(struct virtio_gpu_device *vgdev,
> +                                               int fd)
> +{
> +       struct virtio_gpu_winsrv_conn *conn;
> +       struct drm_device *ddev = vgdev->ddev;
> +       struct drm_file *file;
> +       struct virtio_gpu_fpriv *vfpriv;
> +
> +       mutex_lock(&ddev->filelist_mutex);
> +       list_for_each_entry(file, &ddev->filelist, lhead) {
> +               vfpriv = file->driver_priv;
> +               spin_lock(&vfpriv->winsrv_lock);
> +               list_for_each_entry(conn, &vfpriv->winsrv_conns, next) {
> +                       if (conn->fd == fd) {
> +                               spin_lock(&conn->lock);
> +                               spin_unlock(&vfpriv->winsrv_lock);
> +                               mutex_unlock(&ddev->filelist_mutex);
> +                               return conn;
> +                       }
> +               }
> +               spin_unlock(&vfpriv->winsrv_lock);
> +       }
> +       mutex_unlock(&ddev->filelist_mutex);
> +
> +       return NULL;
> +}
> +
> +static void handle_rx_cmd(struct virtio_gpu_device *vgdev,
> +                         struct virtio_gpu_winsrv_rx *cmd)
> +{
> +       struct virtio_gpu_winsrv_conn *conn;
> +       struct virtio_gpu_winsrv_rx_qentry *qentry;
> +
> +       conn = find_conn(vgdev, cmd->client_fd);
> +       if (!conn) {
> +               DRM_DEBUG("recv for unknown client fd %u\n", cmd->client_fd);
> +               return;
> +       }
> +
> +       qentry = kzalloc(sizeof(*qentry), GFP_KERNEL);
> +       if (!qentry) {
> +               spin_unlock(&conn->lock);
> +               DRM_DEBUG("failed to allocate qentry for winsrv connection\n");
> +               return;
> +       }
> +
> +       qentry->cmd = cmd;
> +
> +       list_add_tail(&qentry->next, &conn->cmdq);
> +       wake_up_interruptible(&conn->cmdwq);
> +       spin_unlock(&conn->lock);
> +}
> +
> +void virtio_gpu_dequeue_winsrv_rx_func(struct work_struct *work)
> +{
> +       struct virtio_gpu_device *vgdev =
> +               container_of(work, struct virtio_gpu_device,
> +                            winsrv_rxq.dequeue_work);
> +       struct virtio_gpu_winsrv_rx *cmd;
> +       unsigned int len;
> +
> +       spin_lock(&vgdev->winsrv_rxq.qlock);
> +       while ((cmd = virtqueue_get_buf(vgdev->winsrv_rxq.vq, &len)) != NULL) {
> +               spin_unlock(&vgdev->winsrv_rxq.qlock);
> +               handle_rx_cmd(vgdev, cmd);
> +               spin_lock(&vgdev->winsrv_rxq.qlock);
> +       }
> +       spin_unlock(&vgdev->winsrv_rxq.qlock);
> +
> +       virtqueue_kick(vgdev->winsrv_rxq.vq);
> +}
> +
>  static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
>                                                struct virtio_gpu_vbuffer *vbuf)
>                 __releases(&vgdev->ctrlq.qlock)
> @@ -380,6 +531,41 @@ static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
>         return ret;
>  }
>
> +static int virtio_gpu_queue_winsrv_tx(struct virtio_gpu_device *vgdev,
> +                                     struct virtio_gpu_vbuffer *vbuf)
> +{
> +       struct virtqueue *vq = vgdev->winsrv_txq.vq;
> +       struct scatterlist *sgs[2], vcmd, vout;
> +       int ret;
> +
> +       if (!vgdev->vqs_ready)
> +               return -ENODEV;
> +
> +       sg_init_one(&vcmd, vbuf->buf, vbuf->size);
> +       sgs[0] = &vcmd;
> +
> +       sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
> +       sgs[1] = &vout;
> +
> +       spin_lock(&vgdev->winsrv_txq.qlock);
> +retry:
> +       ret = virtqueue_add_sgs(vq, sgs, 2, 0, vbuf, GFP_ATOMIC);
> +       if (ret == -ENOSPC) {
> +               spin_unlock(&vgdev->winsrv_txq.qlock);
> +               wait_event(vgdev->winsrv_txq.ack_queue, vq->num_free);
> +               spin_lock(&vgdev->winsrv_txq.qlock);
> +               goto retry;
> +       }
> +
> +       virtqueue_kick(vq);
> +
> +       spin_unlock(&vgdev->winsrv_txq.qlock);
> +
> +       if (!ret)
> +               ret = vq->num_free;
> +       return ret;
> +}
> +
>  /* just create gem objects for userspace and long lived objects,
>     just use dma_alloced pages for the queue objects? */
>
> @@ -890,3 +1076,100 @@ void virtio_gpu_cursor_ping(struct virtio_gpu_device *vgdev,
>         memcpy(cur_p, &output->cursor, sizeof(output->cursor));
>         virtio_gpu_queue_cursor(vgdev, vbuf);
>  }
> +
> +int virtio_gpu_cmd_winsrv_connect(struct virtio_gpu_device *vgdev, int fd)
> +{
> +       struct virtio_gpu_winsrv_connect *cmd_p;
> +       struct virtio_gpu_vbuffer *vbuf;
> +
> +       cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> +       memset(cmd_p, 0, sizeof(*cmd_p));
> +
> +       cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_WINSRV_CONNECT);
> +       cmd_p->client_fd = cpu_to_le32(fd);
> +       return virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
> +}
> +
> +void virtio_gpu_cmd_winsrv_disconnect(struct virtio_gpu_device *vgdev, int fd)
> +{
> +       struct virtio_gpu_winsrv_disconnect *cmd_p;
> +       struct virtio_gpu_vbuffer *vbuf;
> +
> +       cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> +       memset(cmd_p, 0, sizeof(*cmd_p));
> +
> +       cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_WINSRV_DISCONNECT);
> +       cmd_p->client_fd = cpu_to_le32(fd);
> +       virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
> +}
> +
> +int virtio_gpu_cmd_winsrv_tx(struct virtio_gpu_device *vgdev,
> +                            const char __user *buffer, u32 len,
> +                            int *fds, struct virtio_gpu_winsrv_conn *conn,
> +                            bool nonblock)
> +{
> +       int client_fd = conn->fd;
> +       struct drm_file *file = conn->drm_file;
> +       struct virtio_gpu_winsrv_tx *cmd_p;
> +       struct virtio_gpu_vbuffer *vbuf;
> +       uint32_t gem_handle;
> +       struct drm_gem_object *gobj = NULL;
> +       struct virtio_gpu_object *qobj = NULL;
> +       int ret, i, fd;
> +
> +       cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> +       memset(cmd_p, 0, sizeof(*cmd_p));
> +
> +       cmd_p->hdr.type = cpu_to_le32(VIRTIO_GPU_CMD_WINSRV_TX);
> +
> +       for (i = 0; i < VIRTIO_GPU_WINSRV_MAX_ALLOCS; i++) {
> +               cmd_p->resource_ids[i] = -1;
> +
> +               fd = fds[i];
> +               if (fd < 0)
> +                       break;
> +
> +               ret = drm_gem_prime_fd_to_handle(vgdev->ddev, file, fd,
> +                                                &gem_handle);
> +               if (ret != 0)
> +                       goto err_free_vbuf;
> +
> +               gobj = drm_gem_object_lookup(file, gem_handle);
> +               if (gobj == NULL) {
> +                       ret = -ENOENT;
> +                       goto err_free_vbuf;
> +               }
> +
> +               qobj = gem_to_virtio_gpu_obj(gobj);
> +               cmd_p->resource_ids[i] = qobj->hw_res_handle;
> +       }
> +
> +       cmd_p->client_fd = client_fd;
> +       cmd_p->len = cpu_to_le32(len);
> +
> +       /* gets freed when the ring has consumed it */
> +       vbuf->data_buf = kmalloc(cmd_p->len, GFP_KERNEL);
> +       if (!vbuf->data_buf) {
> +               DRM_ERROR("failed to allocate winsrv tx buffer\n");
> +               ret = -ENOMEM;
> +               goto err_free_vbuf;
> +       }
> +
> +       vbuf->data_size = cmd_p->len;
> +
> +       if (copy_from_user(vbuf->data_buf, buffer, cmd_p->len)) {
> +               ret = -EFAULT;
> +               goto err_free_databuf;
> +       }
> +
> +       virtio_gpu_queue_winsrv_tx(vgdev, vbuf);
> +
> +       return 0;
> +
> +err_free_databuf:
> +       kfree(vbuf->data_buf);
> +err_free_vbuf:
> +       free_vbuf(vgdev, vbuf);
> +
> +       return ret;
> +}
> diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
> index 91a31ffed828..89b0a1a707a7 100644
> --- a/include/uapi/drm/virtgpu_drm.h
> +++ b/include/uapi/drm/virtgpu_drm.h
> @@ -46,6 +46,11 @@ extern "C" {
>  #define DRM_VIRTGPU_TRANSFER_TO_HOST 0x07
>  #define DRM_VIRTGPU_WAIT     0x08
>  #define DRM_VIRTGPU_GET_CAPS  0x09
> +#define DRM_VIRTGPU_WINSRV_CONNECT  0x0a
> +#define DRM_VIRTGPU_WINSRV_TX  0x0b
> +#define DRM_VIRTGPU_WINSRV_RX  0x0c
> +
> +#define VIRTGPU_WINSRV_MAX_ALLOCS 28
>
>  struct drm_virtgpu_map {
>         __u64 offset; /* use for mmap system call */
> @@ -132,6 +137,18 @@ struct drm_virtgpu_get_caps {
>         __u32 pad;
>  };
>
> +struct drm_virtgpu_winsrv {
> +       __s32 fds[VIRTGPU_WINSRV_MAX_ALLOCS];
> +       __u64 data;
> +       __u32 len;
> +       __u32 pad;
> +};
> +
> +struct drm_virtgpu_winsrv_connect {
> +       __u32 fd;   /* returned by kernel */
> +       __u32 pad;
> +};
> +
>  #define DRM_IOCTL_VIRTGPU_MAP \
>         DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_MAP, struct drm_virtgpu_map)
>
> @@ -167,6 +184,18 @@ struct drm_virtgpu_get_caps {
>         DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_GET_CAPS, \
>         struct drm_virtgpu_get_caps)
>
> +#define DRM_IOCTL_VIRTGPU_WINSRV_CONNECT \
> +       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_WINSRV_CONNECT, \
> +               struct drm_virtgpu_winsrv_connect)
> +
> +#define DRM_IOCTL_VIRTGPU_WINSRV_TX \
> +       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_WINSRV_TX, \
> +               struct drm_virtgpu_winsrv)
> +
> +#define DRM_IOCTL_VIRTGPU_WINSRV_RX \
> +       DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_WINSRV_RX, \
> +               struct drm_virtgpu_winsrv)
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
> index 4b04ead26cd9..afe830bb8d00 100644
> --- a/include/uapi/linux/virtio_gpu.h
> +++ b/include/uapi/linux/virtio_gpu.h
> @@ -71,6 +71,12 @@ enum virtio_gpu_ctrl_type {
>         VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
>         VIRTIO_GPU_CMD_MOVE_CURSOR,
>
> +       /* window server commands */
> +       VIRTIO_GPU_CMD_WINSRV_CONNECT = 0x0400,
> +       VIRTIO_GPU_CMD_WINSRV_DISCONNECT,
> +       VIRTIO_GPU_CMD_WINSRV_TX,
> +       VIRTIO_GPU_CMD_WINSRV_RX,
> +
>         /* success responses */
>         VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
>         VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
> @@ -290,6 +296,41 @@ struct virtio_gpu_resp_capset {
>         __u8 capset_data[];
>  };
>
> +/* VIRTIO_GPU_CMD_WINSRV_CONNECT */
> +struct virtio_gpu_winsrv_connect {
> +       struct virtio_gpu_ctrl_hdr hdr;
> +       __le32 client_fd;
> +       __le32 padding;
> +};
> +
> +/* VIRTIO_GPU_CMD_WINSRV_DISCONNECT */
> +struct virtio_gpu_winsrv_disconnect {
> +       struct virtio_gpu_ctrl_hdr hdr;
> +       __le32 client_fd;
> +       __le32 padding;
> +};
> +
> +#define VIRTIO_GPU_WINSRV_MAX_ALLOCS 28
> +#define VIRTIO_GPU_WINSRV_TX_MAX_DATA 4096
> +
> +/* VIRTIO_GPU_CMD_WINSRV_TX */
> +/* these commands are followed in the queue descriptor by protocol buffers */
> +struct virtio_gpu_winsrv_tx {
> +       struct virtio_gpu_ctrl_hdr hdr;
> +       __le32 client_fd;
> +       __le32 len;
> +       __le32 resource_ids[VIRTIO_GPU_WINSRV_MAX_ALLOCS];
> +};
> +
> +/* VIRTIO_GPU_CMD_WINSRV_RX */
> +struct virtio_gpu_winsrv_rx {
> +       struct virtio_gpu_ctrl_hdr hdr;
> +       __le32 client_fd;
> +       __u8 data[VIRTIO_GPU_WINSRV_TX_MAX_DATA];
> +       __le32 len;
> +       __le32 resource_ids[VIRTIO_GPU_WINSRV_MAX_ALLOCS];
> +};
> +
>  #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0)
>
>  struct virtio_gpu_config {
> --
> 2.14.3
>

^ permalink raw reply

* [PATCH v21 2/5 RESEND] virtio-balloon: VIRTIO_BALLOON_F_SG
From: Wei Wang @ 2018-01-09 12:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, mhocko, akpm, mawilcox
  Cc: aarcange, yang.zhang.wz, riel, quan.xu0, penguin-kernel,
	liliang.opensource, willy, amit.shah, cornelia.huck, pbonzini,
	nilal, mgorman

Add a new feature, VIRTIO_BALLOON_F_SG, which enables the transfer of
balloon (i.e. inflated/deflated) pages using scatter-gather lists to the
host.

The implementation of the previous virtio-balloon is not very efficient,
because the balloon pages are transferred to the host by one array each
time. Here is the breakdown of the time in percentage spent on each step
of the balloon inflating process (inflating 7GB of an 8GB idle guest).

1) allocating pages (6.5%)
2) sending PFNs to host (68.3%)
3) address translation (6.1%)
4) madvise (19%)

It takes about 4126ms for the inflating process to complete. The above
profiling shows that the bottlenecks are stage 2) and stage 4).

This patch optimizes step 2) by transferring pages to host in sgs. An sg
describes a chunk of guest physically continuous pages. With this
mechanism, step 4) can also be optimized by doing address translation and
madvise() in chunks rather than page by page.

With this new feature, the above ballooning process takes ~460ms resulting
in an improvement of ~89%.

TODO:
- optimize stage 1) by allocating/freeing a chunk of pages instead of a
single page each time.
- sort the internal balloon page queue.
- enable OOM to free inflated pages maintained in the local temporary
  list.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/virtio/virtio_balloon.c     | 233 +++++++++++++++++++++++++++++++++---
 include/uapi/linux/virtio_balloon.h |   1 +
 2 files changed, 216 insertions(+), 18 deletions(-)

RESEND ChangeLog:
- fill_balloon(): move xb_set_page before balloon_page_enqueue

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index a1fb52c..d0b8ea0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -32,6 +32,8 @@
 #include <linux/mm.h>
 #include <linux/mount.h>
 #include <linux/magic.h>
+#include <linux/xbitmap.h>
+#include <asm/page.h>
 
 /*
  * Balloon device works in 4K page units.  So each page is pointed to by
@@ -79,6 +81,9 @@ struct virtio_balloon {
 	/* Synchronize access/update to this struct virtio_balloon elements */
 	struct mutex balloon_lock;
 
+	/* The xbitmap used to record balloon pages */
+	struct xb page_xb;
+
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
 	__virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
@@ -141,15 +146,128 @@ static void set_page_pfns(struct virtio_balloon *vb,
 					  page_to_balloon_pfn(page) + i);
 }
 
+static void kick_and_wait(struct virtqueue *vq, wait_queue_head_t wq_head)
+{
+	unsigned int len;
+
+	virtqueue_kick(vq);
+	wait_event(wq_head, virtqueue_get_buf(vq, &len));
+}
+
+static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len)
+{
+	struct scatterlist sg;
+	unsigned int unused;
+	int err;
+
+	sg_init_table(&sg, 1);
+	sg_set_page(&sg, pfn_to_page(pfn), len, 0);
+
+	/* Detach all the used buffers from the vq */
+	while (virtqueue_get_buf(vq, &unused))
+		;
+
+	err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL);
+	/*
+	 * This is expected to never fail: there is always at least 1 entry
+	 * available on the vq, because when the vq is full the worker thread
+	 * that adds the sg will be put into sleep until at least 1 entry is
+	 * available to use.
+	 */
+	BUG_ON(err);
+}
+
+static void batch_balloon_page_sg(struct virtio_balloon *vb,
+				  struct virtqueue *vq,
+				  unsigned long pfn,
+				  uint32_t len)
+{
+	add_one_sg(vq, pfn, len);
+
+	/* Batch till the vq is full */
+	if (!vq->num_free)
+		kick_and_wait(vq, vb->acked);
+}
+
+/*
+ * Send balloon pages in sgs to host. The balloon pages are recorded in the
+ * page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
+ * The page xbitmap is searched for continuous "1" bits, which correspond
+ * to continuous pages, to chunk into sgs.
+ *
+ * @page_xb_start and @page_xb_end form the range of bits in the xbitmap that
+ * need to be searched.
+ */
+static void tell_host_sgs(struct virtio_balloon *vb,
+			  struct virtqueue *vq,
+			  unsigned long page_xb_start,
+			  unsigned long page_xb_end)
+{
+	unsigned long pfn_start, pfn_end;
+	uint32_t max_len = round_down(UINT_MAX, PAGE_SIZE);
+	uint64_t len;
+
+	pfn_start = page_xb_start;
+	while (pfn_start < page_xb_end) {
+		if (!xb_find_set(&vb->page_xb, page_xb_end, &pfn_start))
+			break;
+		pfn_end = pfn_start + 1;
+		if (!xb_find_zero(&vb->page_xb, page_xb_end, &pfn_end))
+			pfn_end = page_xb_end + 1;
+		len = (pfn_end - pfn_start) << PAGE_SHIFT;
+		while (len > max_len) {
+			batch_balloon_page_sg(vb, vq, pfn_start, max_len);
+			pfn_start += max_len >> PAGE_SHIFT;
+			len -= max_len;
+		}
+		batch_balloon_page_sg(vb, vq, pfn_start, (uint32_t)len);
+		pfn_start = pfn_end + 1;
+	}
+
+	/*
+	 * The last few sgs may not reach the batch size, but need a kick to
+	 * notify the device to handle them.
+	 */
+	if (vq->num_free != virtqueue_get_vring_size(vq))
+		kick_and_wait(vq, vb->acked);
+
+	xb_zero(&vb->page_xb, page_xb_start, page_xb_end);
+}
+
+static inline int xb_set_page(struct virtio_balloon *vb,
+			       struct page *page,
+			       unsigned long *pfn_min,
+			       unsigned long *pfn_max)
+{
+	unsigned long pfn = page_to_pfn(page);
+	int ret;
+
+	*pfn_min = min(pfn, *pfn_min);
+	*pfn_max = max(pfn, *pfn_max);
+
+	do {
+		if (xb_preload(GFP_NOWAIT | __GFP_NOWARN) < 0)
+			return -ENOMEM;
+
+		ret = xb_set_bit(&vb->page_xb, pfn);
+		xb_preload_end();
+	} while (unlikely(ret == -EAGAIN));
+
+	return ret;
+}
+
 static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 {
 	unsigned num_allocated_pages;
 	unsigned num_pfns;
 	struct page *page;
 	LIST_HEAD(pages);
+	bool use_sg = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_SG);
+	unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
 
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	if (!use_sg)
+		num = min(num, ARRAY_SIZE(vb->pfns));
 
 	for (num_pfns = 0; num_pfns < num;
 	     num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
@@ -172,9 +290,16 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 	vb->num_pfns = 0;
 
 	while ((page = balloon_page_pop(&pages))) {
+		if (use_sg) {
+			if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) {
+				__free_page(page);
+				continue;
+			}
+		} else {
+			set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		}
 		balloon_page_enqueue(&vb->vb_dev_info, page);
 
-		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
 		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
 		if (!virtio_has_feature(vb->vdev,
 					VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
@@ -184,8 +309,12 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 
 	num_allocated_pages = vb->num_pfns;
 	/* Did we get any? */
-	if (vb->num_pfns != 0)
-		tell_host(vb, vb->inflate_vq);
+	if (vb->num_pfns) {
+		if (use_sg)
+			tell_host_sgs(vb, vb->inflate_vq, pfn_min, pfn_max);
+		else
+			tell_host(vb, vb->inflate_vq);
+	}
 	mutex_unlock(&vb->balloon_lock);
 
 	return num_allocated_pages;
@@ -211,9 +340,12 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	struct page *page;
 	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
 	LIST_HEAD(pages);
+	bool use_sg = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_SG);
+	unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
 
-	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	/* Traditionally, we can only do one array worth at a time. */
+	if (!use_sg)
+		num = min(num, ARRAY_SIZE(vb->pfns));
 
 	mutex_lock(&vb->balloon_lock);
 	/* We can't release more pages than taken */
@@ -223,7 +355,14 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 		page = balloon_page_dequeue(vb_dev_info);
 		if (!page)
 			break;
-		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		if (use_sg) {
+			if (xb_set_page(vb, page, &pfn_min, &pfn_max) < 0) {
+				balloon_page_enqueue(&vb->vb_dev_info, page);
+				break;
+			}
+		} else {
+			set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		}
 		list_add(&page->lru, &pages);
 		vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
 	}
@@ -234,13 +373,55 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	 * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
 	 * is true, we *have* to do it in this order
 	 */
-	if (vb->num_pfns != 0)
-		tell_host(vb, vb->deflate_vq);
+	if (vb->num_pfns) {
+		if (use_sg)
+			tell_host_sgs(vb, vb->deflate_vq, pfn_min, pfn_max);
+		else
+			tell_host(vb, vb->deflate_vq);
+	}
 	release_pages_balloon(vb, &pages);
 	mutex_unlock(&vb->balloon_lock);
 	return num_freed_pages;
 }
 
+/*
+ * The regular leak_balloon() with VIRTIO_BALLOON_F_SG needs memory allocation
+ * for xbitmap, which is not suitable for the oom case. This function does not
+ * use xbitmap to chunk pages, so it can be used by oom notifier to deflate
+ * pages when VIRTIO_BALLOON_F_SG is negotiated.
+ */
+static unsigned int leak_balloon_sg_oom(struct virtio_balloon *vb)
+{
+	unsigned int n;
+	struct page *page;
+	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+	struct virtqueue *vq = vb->deflate_vq;
+	LIST_HEAD(pages);
+
+	mutex_lock(&vb->balloon_lock);
+	for (n = 0; n < oom_pages; n++) {
+		page = balloon_page_dequeue(vb_dev_info);
+		if (!page)
+			break;
+
+		list_add(&page->lru, &pages);
+		vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
+		batch_balloon_page_sg(vb, vb->deflate_vq, page_to_pfn(page),
+				      PAGE_SIZE);
+		release_pages_balloon(vb, &pages);
+	}
+
+	/*
+	 * The last few sgs may not reach the batch size, but need a kick to
+	 * notify the device to handle them.
+	 */
+	if (vq->num_free != virtqueue_get_vring_size(vq))
+		kick_and_wait(vq, vb->acked);
+	mutex_unlock(&vb->balloon_lock);
+
+	return n;
+}
+
 static inline void update_stat(struct virtio_balloon *vb, int idx,
 			       u16 tag, u64 val)
 {
@@ -380,7 +561,10 @@ static int virtballoon_oom_notify(struct notifier_block *self,
 		return NOTIFY_OK;
 
 	freed = parm;
-	num_freed_pages = leak_balloon(vb, oom_pages);
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_SG))
+		num_freed_pages = leak_balloon_sg_oom(vb);
+	else
+		num_freed_pages = leak_balloon(vb, oom_pages);
 	update_balloon_size(vb);
 	*freed += num_freed_pages;
 
@@ -477,6 +661,7 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
 {
 	struct virtio_balloon *vb = container_of(vb_dev_info,
 			struct virtio_balloon, vb_dev_info);
+	bool use_sg = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_SG);
 	unsigned long flags;
 
 	/*
@@ -498,16 +683,24 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
 	vb_dev_info->isolated_pages--;
 	__count_vm_event(BALLOON_MIGRATE);
 	spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
-	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
-	set_page_pfns(vb, vb->pfns, newpage);
-	tell_host(vb, vb->inflate_vq);
-
+	if (use_sg) {
+		add_one_sg(vb->inflate_vq, page_to_pfn(newpage), PAGE_SIZE);
+		kick_and_wait(vb->inflate_vq, vb->acked);
+	} else {
+		vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+		set_page_pfns(vb, vb->pfns, newpage);
+		tell_host(vb, vb->inflate_vq);
+	}
 	/* balloon's page migration 2nd step -- deflate "page" */
 	balloon_page_delete(page);
-	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
-	set_page_pfns(vb, vb->pfns, page);
-	tell_host(vb, vb->deflate_vq);
-
+	if (use_sg) {
+		add_one_sg(vb->deflate_vq, page_to_pfn(page), PAGE_SIZE);
+		kick_and_wait(vb->deflate_vq, vb->acked);
+	} else {
+		vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+		set_page_pfns(vb, vb->pfns, page);
+		tell_host(vb, vb->deflate_vq);
+	}
 	mutex_unlock(&vb->balloon_lock);
 
 	put_page(page); /* balloon reference */
@@ -566,6 +759,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	if (err)
 		goto out_free_vb;
 
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_SG))
+		xb_init(&vb->page_xb);
+
 	vb->nb.notifier_call = virtballoon_oom_notify;
 	vb->nb.priority = VIRTBALLOON_OOM_NOTIFY_PRIORITY;
 	err = register_oom_notifier(&vb->nb);
@@ -682,6 +878,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_MUST_TELL_HOST,
 	VIRTIO_BALLOON_F_STATS_VQ,
 	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
+	VIRTIO_BALLOON_F_SG,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 343d7dd..37780a7 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -34,6 +34,7 @@
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_SG		3 /* Use sg instead of PFN lists */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
-- 
2.7.4

^ permalink raw reply related

* [PATCH v21 5/5] virtio-balloon: don't report free pages when page poisoning is enabled
From: Wei Wang @ 2018-01-09 11:11 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, mhocko, akpm, mawilcox
  Cc: aarcange, yang.zhang.wz, riel, quan.xu0, penguin-kernel,
	liliang.opensource, willy, amit.shah, cornelia.huck, pbonzini,
	nilal, mgorman
In-Reply-To: <1515496262-7533-1-git-send-email-wei.w.wang@intel.com>

The guest free pages should not be discarded by the live migration thread
when page poisoning is enabled with PAGE_POISONING_NO_SANITY=n, because
skipping the transfer of such poisoned free pages will trigger false
positive when new pages are allocated and checked on the destination.
This patch adds a config field, poison_val. Guest writes to the config
field to tell the host about the poisoning value. The value will be 0 in
the following cases:
1) PAGE_POISONING_NO_SANITY is enabled;
2) page poisoning is disabled; or
3) PAGE_POISONING_ZERO is enabled.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Cc: Michal Hocko <mhocko@suse.com>
---
 drivers/virtio/virtio_balloon.c     | 8 ++++++++
 include/uapi/linux/virtio_balloon.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index f7e8830..7429894 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -859,6 +859,7 @@ static struct file_system_type balloon_fs = {
 static int virtballoon_probe(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb;
+	__u32 poison_val;
 	int err;
 
 	if (!vdev->config->get) {
@@ -896,6 +897,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
 					WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
 		INIT_WORK(&vb->report_free_page_work, report_free_page);
 		vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
+		if (IS_ENABLED(CONFIG_PAGE_POISONING_NO_SANITY) ||
+		    !page_poisoning_enabled())
+			poison_val = 0;
+		else
+			poison_val = PAGE_POISON;
+		virtio_cwrite(vb->vdev, struct virtio_balloon_config,
+			      poison_val, &poison_val);
 	}
 
 	vb->nb.notifier_call = virtballoon_oom_notify;
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 58f1274..f270e9e 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -48,6 +48,8 @@ struct virtio_balloon_config {
 	__u32 actual;
 	/* Free page report command id, readonly by guest */
 	__u32 free_page_report_cmd_id;
+	/* Stores PAGE_POISON if page poisoning with sanity check is in use */
+	__u32 poison_val;
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v21 4/5] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ
From: Wei Wang @ 2018-01-09 11:11 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, mhocko, akpm, mawilcox
  Cc: aarcange, yang.zhang.wz, riel, quan.xu0, penguin-kernel,
	liliang.opensource, willy, amit.shah, cornelia.huck, pbonzini,
	nilal, mgorman
In-Reply-To: <1515496262-7533-1-git-send-email-wei.w.wang@intel.com>

Negotiation of the VIRTIO_BALLOON_F_FREE_PAGE_VQ feature indicates the
support of reporting hints of guest free pages to host via virtio-balloon.

Host requests the guest to report free pages by sending a new cmd
id to the guest via the free_page_report_cmd_id configuration register.

When the guest starts to report, the first element added to the free page
vq is the cmd id given by host. When the guest finishes the reporting
of all the free pages, VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID is added
to the vq to tell host that the reporting is done. Host may also requests
the guest to stop the reporting in advance by sending the stop cmd id to
the guest via the configuration register.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Michal Hocko <mhocko@kernel.org>
---
 drivers/virtio/virtio_balloon.c     | 202 ++++++++++++++++++++++++++++++------
 include/uapi/linux/virtio_balloon.h |   4 +
 2 files changed, 174 insertions(+), 32 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 10876ea..f7e8830 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -55,7 +55,12 @@ static struct vfsmount *balloon_mnt;
 
 struct virtio_balloon {
 	struct virtio_device *vdev;
-	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
+
+	/* Balloon's own wq for cpu-intensive work items */
+	struct workqueue_struct *balloon_wq;
+	/* The free page reporting work item submitted to the balloon wq */
+	struct work_struct report_free_page_work;
 
 	/* The balloon servicing is delegated to a freezable workqueue. */
 	struct work_struct update_balloon_stats_work;
@@ -65,6 +70,13 @@ struct virtio_balloon {
 	spinlock_t stop_update_lock;
 	bool stop_update;
 
+	/* Start to report free pages */
+	bool report_free_page;
+	/* Stores the cmd id given by host to start the free page reporting */
+	uint32_t start_cmd_id;
+	/* Stores STOP_ID as a sign to tell host that the reporting is done */
+	uint32_t stop_cmd_id;
+
 	/* Waiting for host to ack the pages we released. */
 	wait_queue_head_t acked;
 
@@ -189,6 +201,28 @@ static void batch_balloon_page_sg(struct virtio_balloon *vb,
 		kick_and_wait(vq, vb->acked);
 }
 
+static void batch_free_page_sg(struct virtqueue *vq,
+			       unsigned long pfn,
+			       uint32_t len)
+{
+	add_one_sg(vq, pfn, len);
+
+	/* Batch till the vq is full */
+	if (!vq->num_free)
+		virtqueue_kick(vq);
+}
+
+static void send_cmd_id(struct virtio_balloon *vb, void *addr)
+{
+	struct scatterlist sg;
+	int err;
+
+	sg_init_one(&sg, addr, sizeof(uint32_t));
+	err = virtqueue_add_outbuf(vb->free_page_vq, &sg, 1, vb, GFP_KERNEL);
+	BUG_ON(err);
+	virtqueue_kick(vb->free_page_vq);
+}
+
 /*
  * Send balloon pages in sgs to host. The balloon pages are recorded in the
  * page xbitmap. Each bit in the bitmap corresponds to a page of PAGE_SIZE.
@@ -497,17 +531,6 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	virtqueue_kick(vq);
 }
 
-static void virtballoon_changed(struct virtio_device *vdev)
-{
-	struct virtio_balloon *vb = vdev->priv;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vb->stop_update_lock, flags);
-	if (!vb->stop_update)
-		queue_work(system_freezable_wq, &vb->update_balloon_size_work);
-	spin_unlock_irqrestore(&vb->stop_update_lock, flags);
-}
-
 static inline s64 towards_target(struct virtio_balloon *vb)
 {
 	s64 target;
@@ -524,6 +547,36 @@ static inline s64 towards_target(struct virtio_balloon *vb)
 	return target - vb->num_pages;
 }
 
+static void virtballoon_changed(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+	unsigned long flags;
+	__u32 cmd_id;
+	s64 diff = towards_target(vb);
+
+	if (diff) {
+		spin_lock_irqsave(&vb->stop_update_lock, flags);
+		if (!vb->stop_update)
+			queue_work(system_freezable_wq,
+				   &vb->update_balloon_size_work);
+		spin_unlock_irqrestore(&vb->stop_update_lock, flags);
+	}
+
+	virtio_cread(vb->vdev, struct virtio_balloon_config,
+		     free_page_report_cmd_id, &cmd_id);
+	if (cmd_id == VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID) {
+		WRITE_ONCE(vb->report_free_page, false);
+	} else if (cmd_id != vb->start_cmd_id) {
+		/*
+		 * Host requests to start the reporting by sending a new cmd
+		 * id.
+		 */
+		WRITE_ONCE(vb->report_free_page, true);
+		vb->start_cmd_id = cmd_id;
+		queue_work(vb->balloon_wq, &vb->report_free_page_work);
+	}
+}
+
 static void update_balloon_size(struct virtio_balloon *vb)
 {
 	u32 actual = vb->num_pages;
@@ -601,40 +654,116 @@ static void update_balloon_size_func(struct work_struct *work)
 
 static int init_vqs(struct virtio_balloon *vb)
 {
-	struct virtqueue *vqs[3];
-	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
-	static const char * const names[] = { "inflate", "deflate", "stats" };
-	int err, nvqs;
+	struct virtqueue **vqs;
+	vq_callback_t **callbacks;
+	const char **names;
+	struct scatterlist sg;
+	int i, nvqs, err = -ENOMEM;
+
+	/* Inflateq and deflateq are used unconditionally */
+	nvqs = 2;
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+		nvqs++;
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
+		nvqs++;
+
+	/* Allocate space for find_vqs parameters */
+	vqs = kcalloc(nvqs, sizeof(*vqs), GFP_KERNEL);
+	if (!vqs)
+		goto err_vq;
+	callbacks = kmalloc_array(nvqs, sizeof(*callbacks), GFP_KERNEL);
+	if (!callbacks)
+		goto err_callback;
+	names = kmalloc_array(nvqs, sizeof(*names), GFP_KERNEL);
+	if (!names)
+		goto err_names;
+
+	callbacks[0] = balloon_ack;
+	names[0] = "inflate";
+	callbacks[1] = balloon_ack;
+	names[1] = "deflate";
+
+	i = 2;
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+		callbacks[i] = stats_request;
+		names[i] = "stats";
+		i++;
+	}
 
-	/*
-	 * We expect two virtqueues: inflate and deflate, and
-	 * optionally stat.
-	 */
-	nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
-	err = virtio_find_vqs(vb->vdev, nvqs, vqs, callbacks, names, NULL);
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
+		callbacks[i] = NULL;
+		names[i] = "free_page_vq";
+	}
+
+	err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
+					 NULL, NULL);
 	if (err)
-		return err;
+		goto err_find;
 
 	vb->inflate_vq = vqs[0];
 	vb->deflate_vq = vqs[1];
+	i = 2;
 	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
-		struct scatterlist sg;
-		unsigned int num_stats;
-		vb->stats_vq = vqs[2];
-
+		vb->stats_vq = vqs[i++];
 		/*
 		 * Prime this virtqueue with one buffer so the hypervisor can
 		 * use it to signal us later (it can't be broken yet!).
 		 */
-		num_stats = update_balloon_stats(vb);
-
-		sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+		sg_init_one(&sg, vb->stats, sizeof(vb->stats));
 		if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)
-		    < 0)
-			BUG();
+		    < 0) {
+			dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
+				 __func__);
+			goto err_find;
+		}
 		virtqueue_kick(vb->stats_vq);
 	}
+
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ))
+		vb->free_page_vq = vqs[i];
+
+	kfree(names);
+	kfree(callbacks);
+	kfree(vqs);
 	return 0;
+
+err_find:
+	kfree(names);
+err_names:
+	kfree(callbacks);
+err_callback:
+	kfree(vqs);
+err_vq:
+	return err;
+}
+
+static bool virtio_balloon_send_free_pages(void *opaque, unsigned long pfn,
+					   unsigned long nr_pages)
+{
+	struct virtio_balloon *vb = (struct virtio_balloon *)opaque;
+	uint32_t len = nr_pages << PAGE_SHIFT;
+
+	if (!READ_ONCE(vb->report_free_page))
+		return false;
+
+	batch_free_page_sg(vb->free_page_vq, pfn, len);
+
+	return true;
+}
+
+static void report_free_page(struct work_struct *work)
+{
+	struct virtio_balloon *vb;
+
+	vb = container_of(work, struct virtio_balloon, report_free_page_work);
+	/* Start by sending the obtained cmd id to the host with an outbuf */
+	send_cmd_id(vb, &vb->start_cmd_id);
+	walk_free_mem_block(vb, 0, &virtio_balloon_send_free_pages);
+	/*
+	 * End by sending the stop id to the host with an outbuf. Use the
+	 * non-batching mode here to trigger a kick after adding the stop id.
+	 */
+	send_cmd_id(vb, &vb->stop_cmd_id);
 }
 
 #ifdef CONFIG_BALLOON_COMPACTION
@@ -762,6 +891,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_SG))
 		xb_init(&vb->page_xb);
 
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_VQ)) {
+		vb->balloon_wq = alloc_workqueue("balloon-wq",
+					WQ_FREEZABLE | WQ_CPU_INTENSIVE, 0);
+		INIT_WORK(&vb->report_free_page_work, report_free_page);
+		vb->stop_cmd_id = VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID;
+	}
+
 	vb->nb.notifier_call = virtballoon_oom_notify;
 	vb->nb.priority = VIRTBALLOON_OOM_NOTIFY_PRIORITY;
 	err = register_oom_notifier(&vb->nb);
@@ -826,6 +962,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	spin_unlock_irq(&vb->stop_update_lock);
 	cancel_work_sync(&vb->update_balloon_size_work);
 	cancel_work_sync(&vb->update_balloon_stats_work);
+	cancel_work_sync(&vb->report_free_page_work);
 
 	remove_common(vb);
 #ifdef CONFIG_BALLOON_COMPACTION
@@ -879,6 +1016,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_STATS_VQ,
 	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
 	VIRTIO_BALLOON_F_SG,
+	VIRTIO_BALLOON_F_FREE_PAGE_VQ,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 37780a7..58f1274 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -35,15 +35,19 @@
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
 #define VIRTIO_BALLOON_F_SG		3 /* Use sg instead of PFN lists */
+#define VIRTIO_BALLOON_F_FREE_PAGE_VQ	4 /* VQ to report free pages */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
 
+#define VIRTIO_BALLOON_FREE_PAGE_REPORT_STOP_ID		0
 struct virtio_balloon_config {
 	/* Number of pages host wants Guest to give up. */
 	__u32 num_pages;
 	/* Number of pages we've actually got in balloon. */
 	__u32 actual;
+	/* Free page report command id, readonly by guest */
+	__u32 free_page_report_cmd_id;
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v21 3/5] mm: support reporting free page blocks
From: Wei Wang @ 2018-01-09 11:11 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, mhocko, akpm, mawilcox
  Cc: aarcange, yang.zhang.wz, riel, quan.xu0, penguin-kernel,
	liliang.opensource, willy, amit.shah, cornelia.huck, pbonzini,
	nilal, mgorman
In-Reply-To: <1515496262-7533-1-git-send-email-wei.w.wang@intel.com>

This patch adds support to walk through the free page blocks in the
system and report them via a callback function. Some page blocks may
leave the free list after zone->lock is released, so it is the caller's
responsibility to either detect or prevent the use of such pages.

One use example of this patch is to accelerate live migration by skipping
the transfer of free pages reported from the guest. A popular method used
by the hypervisor to track which part of memory is written during live
migration is to write-protect all the guest memory. So, those pages that
are reported as free pages but are written after the report function
returns will be captured by the hypervisor, and they will be added to the
next round of memory transfer.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michal Hocko <mhocko@kernel.org>
---
 include/linux/mm.h |  6 ++++
 mm/page_alloc.c    | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index ea818ff..b3077dd 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1938,6 +1938,12 @@ extern void free_area_init_node(int nid, unsigned long * zones_size,
 		unsigned long zone_start_pfn, unsigned long *zholes_size);
 extern void free_initmem(void);
 
+extern void walk_free_mem_block(void *opaque,
+				int min_order,
+				bool (*report_pfn_range)(void *opaque,
+							 unsigned long pfn,
+							 unsigned long num));
+
 /*
  * Free reserved pages within range [PAGE_ALIGN(start), end & PAGE_MASK)
  * into the buddy system. The freed pages will be poisoned with pattern
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 76c9688..705de22 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4899,6 +4899,97 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 	show_swap_cache_info();
 }
 
+/*
+ * Walk through a free page list and report the found pfn range via the
+ * callback.
+ *
+ * Return false if the callback requests to stop reporting. Otherwise,
+ * return true.
+ */
+static bool walk_free_page_list(void *opaque,
+				struct zone *zone,
+				int order,
+				enum migratetype mt,
+				bool (*report_pfn_range)(void *,
+							 unsigned long,
+							 unsigned long))
+{
+	struct page *page;
+	struct list_head *list;
+	unsigned long pfn, flags;
+	bool ret;
+
+	spin_lock_irqsave(&zone->lock, flags);
+	list = &zone->free_area[order].free_list[mt];
+	list_for_each_entry(page, list, lru) {
+		pfn = page_to_pfn(page);
+		ret = report_pfn_range(opaque, pfn, 1 << order);
+		if (!ret)
+			break;
+	}
+	spin_unlock_irqrestore(&zone->lock, flags);
+
+	return ret;
+}
+
+/**
+ * walk_free_mem_block - Walk through the free page blocks in the system
+ * @opaque: the context passed from the caller
+ * @min_order: the minimum order of free lists to check
+ * @report_pfn_range: the callback to report the pfn range of the free pages
+ *
+ * If the callback returns false, stop iterating the list of free page blocks.
+ * Otherwise, continue to report.
+ *
+ * Please note that there are no locking guarantees for the callback and
+ * that the reported pfn range might be freed or disappear after the
+ * callback returns so the caller has to be very careful how it is used.
+ *
+ * The callback itself must not sleep or perform any operations which would
+ * require any memory allocations directly (not even GFP_NOWAIT/GFP_ATOMIC)
+ * or via any lock dependency. It is generally advisable to implement
+ * the callback as simple as possible and defer any heavy lifting to a
+ * different context.
+ *
+ * There is no guarantee that each free range will be reported only once
+ * during one walk_free_mem_block invocation.
+ *
+ * pfn_to_page on the given range is strongly discouraged and if there is
+ * an absolute need for that make sure to contact MM people to discuss
+ * potential problems.
+ *
+ * The function itself might sleep so it cannot be called from atomic
+ * contexts.
+ *
+ * In general low orders tend to be very volatile and so it makes more
+ * sense to query larger ones first for various optimizations which like
+ * ballooning etc... This will reduce the overhead as well.
+ */
+void walk_free_mem_block(void *opaque,
+			 int min_order,
+			 bool (*report_pfn_range)(void *opaque,
+						  unsigned long pfn,
+						  unsigned long num))
+{
+	struct zone *zone;
+	int order;
+	enum migratetype mt;
+	bool ret;
+
+	for_each_populated_zone(zone) {
+		for (order = MAX_ORDER - 1; order >= min_order; order--) {
+			for (mt = 0; mt < MIGRATE_TYPES; mt++) {
+				ret = walk_free_page_list(opaque, zone,
+							  order, mt,
+							  report_pfn_range);
+				if (!ret)
+					return;
+			}
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(walk_free_mem_block);
+
 static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
 {
 	zoneref->zone = zone;
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox