From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7291EC433F5 for ; Wed, 27 Apr 2022 04:11:07 +0000 (UTC) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4959C40E78; Wed, 27 Apr 2022 06:11:06 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 550EF40691 for ; Wed, 27 Apr 2022 06:11:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1651032663; x=1682568663; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=a1dmWrJHWCfMjFTGttezIdyvpkMDnuQkb2qM9twYSkw=; b=evWjteivOxOKWipv11qmI5tz0dU5FJL/goxMnB87Hvc6YpjXl6FTVVXB RfRsy38KZwrQle4d4y+3a1pVM5svu0Nt9kgZtyYwYD8fvMOfiusGtKNlG IUVZLWWCx24Kvp0lF/xa4TSucKENDKZRZFBHNm6LBb4AvYO0JE0pUf6O9 5057BE6/Sg2hzKjrICZ5rJuKMOnAfpX7Ym1aD80bt9UdyQo1s0SxVo1wd O0M88gubPpHdCm/zqhvyAb9l9aIfIyzDzjwjsXXTyw9KDgxUifB0eaXW2 CIUP83ebufs/Rhw7ON3S5ZnIgOGFK28oAYav4nxE5RZE6/NaOwFYPcFOi Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10329"; a="265616785" X-IronPort-AV: E=Sophos;i="5.90,292,1643702400"; d="scan'208";a="265616785" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Apr 2022 21:11:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,292,1643702400"; d="scan'208";a="705364233" Received: from fmsmsx606.amr.corp.intel.com ([10.18.126.86]) by fmsmga001.fm.intel.com with ESMTP; 26 Apr 2022 21:11:02 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Tue, 26 Apr 2022 21:11:01 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Tue, 26 Apr 2022 21:11:01 -0700 Received: from fmsmsx612.amr.corp.intel.com ([10.18.126.92]) by fmsmsx612.amr.corp.intel.com ([10.18.126.92]) with mapi id 15.01.2308.027; Tue, 26 Apr 2022 21:11:01 -0700 From: "Pei, Andy" To: "Xia, Chenbo" , "dev@dpdk.org" CC: "maxime.coquelin@redhat.com" , "Cao, Gang" , "Liu, Changpeng" Subject: RE: [PATCH v6 07/16] examples/vdpa: add vDPA blk support in example Thread-Topic: [PATCH v6 07/16] examples/vdpa: add vDPA blk support in example Thread-Index: AQHYVWD+zxtJ2lbyyEqxwOZglErt6q0ApY0wgAKIuWA= Date: Wed, 27 Apr 2022 04:11:01 +0000 Message-ID: References: <1643093258-47258-2-git-send-email-andy.pei@intel.com> <1650530034-59744-1-git-send-email-andy.pei@intel.com> <1650530034-59744-8-git-send-email-andy.pei@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.6.401.20 dlp-reaction: no-action x-originating-ip: [10.239.127.36] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Chenbo, Thanks for your reply. My reply is inline. > -----Original Message----- > From: Xia, Chenbo > Sent: Monday, April 25, 2022 9:39 PM > To: Pei, Andy ; dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Cao, Gang ; Liu, > Changpeng > Subject: RE: [PATCH v6 07/16] examples/vdpa: add vDPA blk support in > example >=20 > Hi Andy, >=20 > > -----Original Message----- > > From: Pei, Andy > > Sent: Thursday, April 21, 2022 4:34 PM > > To: dev@dpdk.org > > Cc: Xia, Chenbo ; maxime.coquelin@redhat.com; > > Cao, Gang ; Liu, Changpeng > > > > Subject: [PATCH v6 07/16] examples/vdpa: add vDPA blk support in > > example > > > > Add virtio blk device support to vDPA example. > > > > Signed-off-by: Andy Pei > > --- > > examples/vdpa/main.c | 61 +++++++++++++- > > examples/vdpa/vdpa_blk_compact.h | 72 +++++++++++++++++ > > examples/vdpa/vhost_user.h | 169 > > +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 > > examples/vdpa/vdpa_blk_compact.h create mode 100644 > > examples/vdpa/vhost_user.h > > > > diff --git a/examples/vdpa/main.c b/examples/vdpa/main.c index > > 5ab0765..1c809ab 100644 > > --- a/examples/vdpa/main.c > > +++ b/examples/vdpa/main.c > > @@ -20,6 +20,7 @@ > > #include > > #include > > #include > > +#include "vdpa_blk_compact.h" > > > > #define MAX_PATH_LEN 128 > > #define MAX_VDPA_SAMPLE_PORTS 1024 > > @@ -41,6 +42,7 @@ struct vdpa_port { > > static int devcnt; > > static int interactive; > > static int client_mode; > > +static int isblk; > > > > /* display usage */ > > static void > > @@ -49,7 +51,8 @@ struct vdpa_port { > > printf("Usage: %s [EAL options] -- " > > " --interactive|-i: run in interactive > > mode.\n" > > " --iface : specify the path prefix > of > > the socket files, e.g. /tmp/vhost-user-.\n" > > - " --client: register a vhost-user socket > as > > client mode.\n", > > + " --client: register a vhost-user socket > as > > client mode.\n" > > + " --isblk: device is a block device, e.g. > > virtio_blk device.\n", > > prgname); > > } > > > > @@ -61,6 +64,7 @@ struct vdpa_port { > > {"iface", required_argument, NULL, 0}, > > {"interactive", no_argument, &interactive, 1}, > > {"client", no_argument, &client_mode, 1}, > > + {"isblk", no_argument, &isblk, 1}, >=20 > I think a new API for get_device_type will be better than asking user to > specify the device type. >=20 Good suggestion. I will send out a version of patch set and try to do this. > > {NULL, 0, 0, 0}, > > }; > > int opt, idx; > > @@ -159,6 +163,52 @@ struct vdpa_port { }; > > > > static int > > +vdpa_blk_device_set_features_and_protocol(const char *path) { > > + uint64_t protocol_features =3D 0; > > + int ret; > > + > > + ret =3D rte_vhost_driver_set_features(path, > VHOST_BLK_FEATURES_BASE); > > + if (ret !=3D 0) { > > + RTE_LOG(ERR, VDPA, > > + "rte_vhost_driver_set_features for %s failed.\n", > > + path); > > + goto out; > > + } > > + > > + ret =3D rte_vhost_driver_disable_features(path, > > + VHOST_VDPA_BLK_DISABLED_FEATURES); > > + if (ret !=3D 0) { > > + RTE_LOG(ERR, VDPA, > > + "rte_vhost_driver_disable_features for %s failed.\n", > > + path); > > + goto out; > > + } > > + > > + ret =3D rte_vhost_driver_get_protocol_features(path, > > &protocol_features); > > + if (ret !=3D 0) { > > + RTE_LOG(ERR, VDPA, > > + "rte_vhost_driver_get_protocol_features for %s > > failed.\n", > > + path); > > + goto out; > > + } > > + > > + protocol_features |=3D (1ULL << VHOST_USER_PROTOCOL_F_CONFIG); > > + protocol_features |=3D (1ULL << > VHOST_USER_PROTOCOL_F_LOG_SHMFD); > > + > > + ret =3D rte_vhost_driver_set_protocol_features(path, > > protocol_features); > > + if (ret !=3D 0) { > > + RTE_LOG(ERR, VDPA, > > + "rte_vhost_driver_set_protocol_features for %s > > failed.\n", > > + path); > > + goto out; > > + } > > + > > +out: > > + return ret; > > +} > > + > > +static int > > start_vdpa(struct vdpa_port *vport) > > { > > int ret; > > @@ -192,6 +242,15 @@ struct vdpa_port { > > "attach vdpa device failed: %s\n", > > socket_path); > > > > + if (isblk) { > > + RTE_LOG(NOTICE, VDPA, "is a blk device\n"); > > + ret =3D > vdpa_blk_device_set_features_and_protocol(socket_path); > > + if (ret !=3D 0) > > + rte_exit(EXIT_FAILURE, > > + "set vhost blk driver features and protocol > > features failed: %s\n", > > + socket_path); > > + } > > + > > if (rte_vhost_driver_start(socket_path) < 0) > > rte_exit(EXIT_FAILURE, > > "start vhost driver failed: %s\n", diff --git > > a/examples/vdpa/vdpa_blk_compact.h > > b/examples/vdpa/vdpa_blk_compact.h >=20 > Please remove unused definition in this header. I didn't check all, but i= t > seems at least VHOST_BLK_PROTOCOL_FEATURES is not used. >=20 Sure. I will remove all unused definition. > > new file mode 100644 > > index 0000000..e7c0f22 > > --- /dev/null > > +++ b/examples/vdpa/vdpa_blk_compact.h > > @@ -0,0 +1,72 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2022 Intel Corporation */ > > + > > +#ifndef _VDPA_BLK_COMPACT_H_ > > +#define _VDPA_BLK_COMPACT_H_ > > + > > +/** > > + * @file > > + * > > + * Device specific vhost lib > > + */ > > + > > +#include > > + > > +#include > > +#include > > + > > +/* Feature bits */ > > +#define VIRTIO_BLK_F_SIZE_MAX 1 /* Indicates maximum segment > size > > */ > > +#define VIRTIO_BLK_F_SEG_MAX 2 /* Indicates maximum # of > segments > > */ > > +#define VIRTIO_BLK_F_GEOMETRY 4 /* Legacy geometry available *= / > > +#define VIRTIO_BLK_F_RO 5 /* Disk is read-only */ > > +#define VIRTIO_BLK_F_BLK_SIZE 6 /* Block size of disk is availa= ble > > */ > > +#define VIRTIO_BLK_F_TOPOLOGY 10 /* Topology information is > > available */ > > +#define VIRTIO_BLK_F_MQ 12 /* support more than one vq */ > > +#define VIRTIO_BLK_F_DISCARD 13 /* DISCARD is supported */ > > +#define VIRTIO_BLK_F_WRITE_ZEROES 14 /* WRITE ZEROES is supported > */ > > + > > +/* Legacy feature bits */ > > +#ifndef VIRTIO_BLK_NO_LEGACY > > +#define VIRTIO_BLK_F_BARRIER 0 /* Does host support barriers? = */ > > +#define VIRTIO_BLK_F_SCSI 7 /* Supports scsi command passth= ru > > */ > > +#define VIRTIO_BLK_F_FLUSH 9 /* Flush command supported */ > > +#define VIRTIO_BLK_F_CONFIG_WCE 11 /* Writeback mode available in > > config */ > > + > > +/* Old (deprecated) name for VIRTIO_BLK_F_FLUSH. */ #define > > +VIRTIO_BLK_F_WCE VIRTIO_BLK_F_FLUSH #endif > /* !VIRTIO_BLK_NO_LEGACY > > +*/ > > + > > +#ifndef VHOST_USER_F_PROTOCOL_FEATURES #define > > +VHOST_USER_F_PROTOCOL_FEATURES 30 #endif > > + > > +#define VHOST_BLK_FEATURES ((1ULL << VHOST_F_LOG_ALL) | \ > > + (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) | \ > > + (1ULL << VIRTIO_F_VERSION_1) | \ > > + (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) | \ > > + (1ULL << VIRTIO_RING_F_EVENT_IDX) | \ > > + (1ULL << VIRTIO_RING_F_INDIRECT_DESC)) > > + > > +#define VHOST_BLK_DISABLED_FEATURES ((1ULL << > > +VIRTIO_RING_F_EVENT_IDX) | > > \ > > + (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY)) > > + > > +#define VHOST_BLK_FEATURES_BASE (VHOST_BLK_FEATURES | \ > > + (1ULL << VIRTIO_BLK_F_SIZE_MAX) | (1ULL << > VIRTIO_BLK_F_SEG_MAX) | \ > > + (1ULL << VIRTIO_BLK_F_GEOMETRY) | (1ULL << > VIRTIO_BLK_F_BLK_SIZE) | > > \ > > + (1ULL << VIRTIO_BLK_F_TOPOLOGY) | (1ULL << > VIRTIO_BLK_F_BARRIER) | > > \ > > + (1ULL << VIRTIO_BLK_F_SCSI) | (1ULL << > VIRTIO_BLK_F_CONFIG_WCE) > > | \ > > + (1ULL << VIRTIO_BLK_F_MQ)) > > + > > +/* Not supported features */ > > +#define VHOST_VDPA_BLK_DISABLED_FEATURES > (VHOST_BLK_DISABLED_FEATURES | \ > > + (1ULL << VIRTIO_BLK_F_GEOMETRY) | (1ULL << > VIRTIO_BLK_F_CONFIG_WCE) > > | \ > > + (1ULL << VIRTIO_BLK_F_BARRIER) | (1ULL << VIRTIO_BLK_F_SCSI)) > > + > > +/* Vhost-blk support protocol features */ #define > > +VHOST_BLK_PROTOCOL_FEATURES \ > > + ((1ULL << VHOST_USER_PROTOCOL_F_CONFIG) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD)) > > + > > +#endif /* _VDPA_BLK_COMPACT_H_ */ > > diff --git a/examples/vdpa/vhost_user.h b/examples/vdpa/vhost_user.h > > new file mode 100644 index 0000000..137bbc2 > > --- /dev/null > > +++ b/examples/vdpa/vhost_user.h >=20 > I don't understand, why introduce this header? It seems never used. >=20 Sure. I will remove this file. > Thanks, > Chenbo >=20 > > @@ -0,0 +1,169 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2022 Intel Corporation > > + */ > > + > > +#ifndef _VHOST_NET_USER_H > > +#define _VHOST_NET_USER_H > > + > > +#include > > +#include > > + > > +#include "rte_vhost.h" > > + > > +/* refer to hw/virtio/vhost-user.c */ > > + > > +#define VHOST_MEMORY_MAX_NREGIONS 8 > > + > > +#ifndef VHOST_USER_MAX_CONFIG_SIZE > > +#define VHOST_USER_MAX_CONFIG_SIZE 256 > > +#endif > > + > > +#define VHOST_USER_PROTOCOL_FEATURES ((1ULL << > > VHOST_USER_PROTOCOL_F_MQ) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_LOG_SHMFD) > |\ > > + (1ULL << VHOST_USER_PROTOCOL_F_RARP) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_NET_MTU) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_SLAVE_REQ) | \ > > + (1ULL << > VHOST_USER_PROTOCOL_F_CRYPTO_SESSION) | \ > > + (1ULL << > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD) | \ > > + (1ULL << > VHOST_USER_PROTOCOL_F_HOST_NOTIFIER) | \ > > + (1ULL << VHOST_USER_PROTOCOL_F_PAGEFAULT)) > > + > > +typedef enum VhostUserRequest { > > + VHOST_USER_NONE =3D 0, > > + VHOST_USER_GET_FEATURES =3D 1, > > + VHOST_USER_SET_FEATURES =3D 2, > > + VHOST_USER_SET_OWNER =3D 3, > > + VHOST_USER_RESET_OWNER =3D 4, > > + VHOST_USER_SET_MEM_TABLE =3D 5, > > + VHOST_USER_SET_LOG_BASE =3D 6, > > + VHOST_USER_SET_LOG_FD =3D 7, > > + VHOST_USER_SET_VRING_NUM =3D 8, > > + VHOST_USER_SET_VRING_ADDR =3D 9, > > + VHOST_USER_SET_VRING_BASE =3D 10, > > + VHOST_USER_GET_VRING_BASE =3D 11, > > + VHOST_USER_SET_VRING_KICK =3D 12, > > + VHOST_USER_SET_VRING_CALL =3D 13, > > + VHOST_USER_SET_VRING_ERR =3D 14, > > + VHOST_USER_GET_PROTOCOL_FEATURES =3D 15, > > + VHOST_USER_SET_PROTOCOL_FEATURES =3D 16, > > + VHOST_USER_GET_QUEUE_NUM =3D 17, > > + VHOST_USER_SET_VRING_ENABLE =3D 18, > > + VHOST_USER_SEND_RARP =3D 19, > > + VHOST_USER_NET_SET_MTU =3D 20, > > + VHOST_USER_SET_SLAVE_REQ_FD =3D 21, > > + VHOST_USER_IOTLB_MSG =3D 22, > > + VHOST_USER_CRYPTO_CREATE_SESS =3D 26, > > + VHOST_USER_CRYPTO_CLOSE_SESS =3D 27, > > + VHOST_USER_POSTCOPY_ADVISE =3D 28, > > + VHOST_USER_POSTCOPY_LISTEN =3D 29, > > + VHOST_USER_POSTCOPY_END =3D 30, > > + VHOST_USER_GET_INFLIGHT_FD =3D 31, > > + VHOST_USER_SET_INFLIGHT_FD =3D 32, > > + VHOST_USER_MAX =3D 33 > > +} VhostUserRequest; > > + > > +typedef enum VhostUserSlaveRequest { > > + VHOST_USER_SLAVE_NONE =3D 0, > > + VHOST_USER_SLAVE_IOTLB_MSG =3D 1, > > + VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG =3D 3, > > + VHOST_USER_SLAVE_MAX > > +} VhostUserSlaveRequest; > > + > > +typedef struct VhostUserMemoryRegion { > > + uint64_t guest_phys_addr; > > + uint64_t memory_size; > > + uint64_t userspace_addr; > > + uint64_t mmap_offset; > > +} VhostUserMemoryRegion; > > + > > +typedef struct VhostUserMemory { > > + uint32_t nregions; > > + uint32_t padding; > > + VhostUserMemoryRegion > regions[VHOST_MEMORY_MAX_NREGIONS]; > > +} VhostUserMemory; > > + > > +typedef struct VhostUserLog { > > + uint64_t mmap_size; > > + uint64_t mmap_offset; > > +} VhostUserLog; > > + > > +/* Comply with Cryptodev-Linux */ > > +#define VHOST_USER_CRYPTO_MAX_HMAC_KEY_LENGTH 512 > > +#define VHOST_USER_CRYPTO_MAX_CIPHER_KEY_LENGTH 64 > > + > > +/* Same structure as vhost-user backend session info */ > > +typedef struct VhostUserCryptoSessionParam { > > + int64_t session_id; > > + uint32_t op_code; > > + uint32_t cipher_algo; > > + uint32_t cipher_key_len; > > + uint32_t hash_algo; > > + uint32_t digest_len; > > + uint32_t auth_key_len; > > + uint32_t aad_len; > > + uint8_t op_type; > > + uint8_t dir; > > + uint8_t hash_mode; > > + uint8_t chaining_dir; > > + uint8_t *ciphe_key; > > + uint8_t *auth_key; > > + uint8_t > cipher_key_buf[VHOST_USER_CRYPTO_MAX_CIPHER_KEY_LENGTH]; > > + uint8_t > auth_key_buf[VHOST_USER_CRYPTO_MAX_HMAC_KEY_LENGTH]; > > +} VhostUserCryptoSessionParam; > > + > > +typedef struct VhostUserVringArea { > > + uint64_t u64; > > + uint64_t size; > > + uint64_t offset; > > +} VhostUserVringArea; > > + > > +typedef struct VhostUserInflight { > > + uint64_t mmap_size; > > + uint64_t mmap_offset; > > + uint16_t num_queues; > > + uint16_t queue_size; > > +} VhostUserInflight; > > + > > +/** Get/set config msg payload */ > > +struct vhost_user_config { > > + uint32_t offset; > > + uint32_t size; > > + uint32_t flags; > > + uint8_t region[VHOST_USER_MAX_CONFIG_SIZE]; > > +}; > > + > > +typedef struct VhostUserMsg { > > + union { > > + uint32_t master; /* a VhostUserRequest value */ > > + uint32_t slave; /* a VhostUserSlaveRequest value*/ > > + } request; > > + > > +#define VHOST_USER_VERSION_MASK 0x3 > > +#define VHOST_USER_REPLY_MASK (0x1 << 2) > > +#define VHOST_USER_NEED_REPLY (0x1 << 3) > > + uint32_t flags; > > + uint32_t size; /* the following payload size */ > > + union { > > +#define VHOST_USER_VRING_IDX_MASK 0xff > > +#define VHOST_USER_VRING_NOFD_MASK (0x1<<8) > > + uint64_t u64; > > + struct vhost_vring_state state; > > + struct vhost_vring_addr addr; > > + VhostUserMemory memory; > > + VhostUserLog log; > > + struct vhost_iotlb_msg iotlb; > > + VhostUserCryptoSessionParam crypto_session; > > + VhostUserVringArea area; > > + VhostUserInflight inflight; > > + struct vhost_user_config cfg; > > + } payload; > > + int fds[VHOST_MEMORY_MAX_NREGIONS]; > > + int fd_num; > > +} __attribute((packed)) VhostUserMsg; > > + > > +#define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64) > > + > > +/* The version of the protocol we support */ > > +#define VHOST_USER_VERSION 0x1 > > +#endif > > -- > > 1.8.3.1 >=20