From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 6E09A239 for ; Tue, 26 Feb 2019 08:39:14 +0100 (CET) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20190226073913euoutp016217479acd8ad332909faba10b370c70~G2fGGwg9J0248402484euoutp01A for ; Tue, 26 Feb 2019 07:39:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20190226073913euoutp016217479acd8ad332909faba10b370c70~G2fGGwg9J0248402484euoutp01A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1551166753; bh=SfUVOZKTTR0hUAJ0qrwGlE09UeWKgIcUn0f+VzIYzRA=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=jYBYNP4I7JCMRyX1GS5cZVtYvCLdzHBpWk4jJlxddsFBv36TDnFNdLgiu9FkVE1PH FOQSPx7ye0CjDSd8zxfvBq5fUSKFwgLbzAH2QzC0rIFifNir4tzGF59Qi34D2pHP6A n1y0q/VsGvY2dHlbukGePQ13bkKzReiAzKDoW5l0= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20190226073912eucas1p155c7653b416a70842416bdb060f08aa7~G2fFq8u4e0961309613eucas1p1d; Tue, 26 Feb 2019 07:39:12 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 75.C9.04806.02DE47C5; Tue, 26 Feb 2019 07:39:12 +0000 (GMT) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20190226073911eucas1p288f7b7864d64d2d8d63807ef28798222~G2fE9LVot1973719737eucas1p2Z; Tue, 26 Feb 2019 07:39:11 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20190226073911eusmtrp13d797f0eabe68daf057c0c41ea5b5c25~G2fEuvF5I1064910649eusmtrp1B; Tue, 26 Feb 2019 07:39:11 +0000 (GMT) X-AuditID: cbfec7f5-367ff700000012c6-f5-5c74ed2019a0 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 47.78.04128.F1DE47C5; Tue, 26 Feb 2019 07:39:11 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20190226073910eusmtip20e64e1624bdae30472038ca5f0e2d1bf~G2fELi1oB0741607416eusmtip2S; Tue, 26 Feb 2019 07:39:10 +0000 (GMT) To: "Liu, Changpeng" , "dev@dpdk.org" Cc: "Stojaczyk, Dariusz" , "maxime.coquelin@redhat.com" , "Bie, Tiwei" , "Wang, Zhihong" , Jason Wang From: Ilya Maximets Message-ID: <4ba6108c-78b0-c7e6-f810-3faa13c7765e@samsung.com> Date: Tue, 26 Feb 2019 10:39:04 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrJKsWRmVeSWpSXmKPExsWy7djP87oKb0tiDA5OF7a4+PMru8X732sY Ld592s5kcaX9J7vFskufmSyOde5hsdja8J/JYvPFSUwOHB6/Fixl9Vi85yWTx/t9V9k8+ras YgxgieKySUnNySxLLdK3S+DKmDBzNmPBAoeK+0frGxhvm3YxcnJICJhIHF24mqWLkYtDSGAF o0Tf80YmCOcLo8TMu2tZIZzPjBIfd91ihGlpeHERzBYSWM4oceW0NETRR0aJJTt/s4MkhAU8 JZ5c+sYCYosI+Eu8/H+QFcRmFnjOKPFsnQeIzSagI3Fq9RGwQbwCdhI7Jm5mA7FZBFQl/v5b xgxiiwpESBzufQdVIyhxcuYTsJmcAiESK/ZMYoKYKS7R9GUl1Hx5ie1v5zCDHCQhsI1dYtGs L+wQV7tIfF27E8oWlnh1fAuULSNxenIPC4RdL3G/5SUjRHMHo8T0Q/+YIBL2EltenwNq4ADa oCmxfpc+iCkh4CjReF4MwuSTuPFWEOIEPolJ26YzQ4R5JTrahCBmqEj8PricGcKWkrj57jP7 BEalWUgem4XkmVlInpmFsHYBI8sqRvHU0uLc9NRi47zUcr3ixNzi0rx0veT83E2MwAR0+t/x rzsY9/1JOsQowMGoxMN74n5xjBBrYllxZe4hRgkOZiURXutDJTFCvCmJlVWpRfnxRaU5qcWH GKU5WJTEeasZHkQLCaQnlqRmp6YWpBbBZJk4OKUaGPmSmu/MNjq5/Pd3YyZB/fypWes23vm9 4K76EV1Gq1frF5Ryd818krvzT2kP4wneLcd9fgeJmTRtP77pS+veiefPtn35+TGBoYjxwbqE 89YXRbS8ZN5oFnmveLO+3fLYtYCnIdenTq+dsGGN5Qb953pvtgewf5jLzXVY+/YStVMnTvy0 e3m36f1WJZbijERDLeai4kQACQe8UTwDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrPIsWRmVeSWpSXmKPExsVy+t/xe7ryb0tiDBofy1hc/PmV3eL97zWM Fu8+bWeyuNL+k91i2aXPTBbHOvewWGxt+M9ksfniJCYHDo9fC5ayeize85LJ4/2+q2wefVtW MQawROnZFOWXlqQqZOQXl9gqRRtaGOkZWlroGZlY6hkam8daGZkq6dvZpKTmZJalFunbJehl TJg5m7FggUPF/aP1DYy3TbsYOTkkBEwkGl5cZOxi5OIQEljKKLH/5QUWiISUxI9fF1ghbGGJ P9e62CCK3jNKHJ97FiwhLOAp8eTSN7AGEQFfiUMLTrKCFDELPGeUWNGyBWrsLCaJTTv/sYNU sQnoSJxafYQRxOYVsJPYMXEzG4jNIqAq8fffMmYQW1QgQuLj031MEDWCEidnPgHbwCkQIrFi zySwOLOAusSfeZeYIWxxiaYvK1khbHmJ7W/nME9gFJqFpH0WkpZZSFpmIWlZwMiyilEktbQ4 Nz232EivODG3uDQvXS85P3cTIzDyth37uWUHY9e74EOMAhyMSjy8Co+KY4RYE8uKK3MPMUpw MCuJ8FofKokR4k1JrKxKLcqPLyrNSS0+xGgK9NxEZinR5HxgUsgriTc0NTS3sDQ0NzY3NrNQ Euc9b1AZJSSQnliSmp2aWpBaBNPHxMEp1cBole/SP/n0TAk3IdPN/0/39vgvWnyrovxZenm8 6U73lWnFfw1Z7K03iP/X+y6gNHvrvr7ER2vTbdSLljPcX8C8J6h17awfl/YKWQaw9L9TmT/7 4NaDFU1+oZtPv9zOIeopycPwNCy2Lpl14Z6+KxPKGbe9un3J4On/s/++tG2OU76rGb7bqFZJ iaU4I9FQi7moOBEAN4ctLtICAAA= X-CMS-MailID: 20190226073911eucas1p288f7b7864d64d2d8d63807ef28798222 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20190225132001eucas1p25c1e925b895b3ab36da0aca27110e15c References: <1551081095-14286-1-git-send-email-changpeng.liu@intel.com> <2c1b8abc-47a9-0d0b-5056-0092fc58e310@samsung.com> Subject: Re: [dpdk-dev] vhost: add virtio configuration space access socket messages X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Feb 2019 07:39:14 -0000 On 26.02.2019 10:01, Liu, Changpeng wrote: > > >> -----Original Message----- >> From: Ilya Maximets [mailto:i.maximets@samsung.com] >> Sent: Monday, February 25, 2019 9:20 PM >> To: Liu, Changpeng ; dev@dpdk.org >> Cc: Stojaczyk, Dariusz ; >> maxime.coquelin@redhat.com; Bie, Tiwei ; Wang, >> Zhihong ; Jason Wang >> Subject: Re: vhost: add virtio configuration space access socket messages >> >> On 25.02.2019 10:51, Changpeng Liu wrote: >>> This patch adds new vhost user messages GET_CONFIG and SET_CONFIG >>> used to get/set virtio device's PCI configuration space. >> >> Beside the fact that some additional description and reasoning required, >> I do not see the usage of this feature. You're defining the flag >> VHOST_USER_PROTOCOL_F_CONFIG, but it's never used. So, none of dpdk vhost >> backends (vdpa, vhost-user) will use this feature. >> You, probably, missed adding it to VHOST_USER_PROTOCOL_FEATURES or >> VDPA_SUPPORTED_PROTOCOL_FEATURES. >> >> From the other side, current implementation forces application to properly >> implement the get/set_config callbacks. Otherwise, receiving of the messages >> will result in RTE_VHOST_MSG_RESULT_ERR and subsequent vhost >> disconnection. >> This looks strange, because supported protocol features normally enabled by >> default. Am I misunderstood something ? > QEMU will not send the messages if VHOST_USER_PROTOCOL_F_CONFIG wasn't enabled. So, you're going to enable it only by explicit call to 'rte_vhost_driver_set_features' ? In this case I'm assuming that you're implementing your own vhost backend. But why you're not using 'dev->extern_ops' and corresponding 'pre_msg_handle' or 'post_msg_handle' to handle your GET/SET_CONFIG messages like it does 'vhost_crypto' backend ? >> >> One more thing. According to spec: "vhost-user slave uses zero length of >> payload to indicate an error to vhost-user master". However, current version >> of 'vhost_user_get_config' replies with size equal to 'sizeof(uint64_t)'. > Yeah, can be changed to 0 here, QEMU can process it. >> >> Best regards, Ilya Maximets. >> >>> Signed-off-by: Changpeng Liu >>> Reviewed-by: Darek Stojaczyk >>> --- >>> lib/librte_vhost/rte_vhost.h | 8 ++++++++ >>> lib/librte_vhost/vhost_user.c | 44 >> +++++++++++++++++++++++++++++++++++++++++++ >>> lib/librte_vhost/vhost_user.h | 16 ++++++++++++++++ >>> 3 files changed, 68 insertions(+) >>> >>> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h >>> index 2753670..ab710ce 100644 >>> --- a/lib/librte_vhost/rte_vhost.h >>> +++ b/lib/librte_vhost/rte_vhost.h >>> @@ -63,6 +63,10 @@ >>> #define VHOST_USER_PROTOCOL_F_PAGEFAULT 8 >>> #endif >>> >>> +#ifndef VHOST_USER_PROTOCOL_F_CONFIG >>> +#define VHOST_USER_PROTOCOL_F_CONFIG 9 >>> +#endif >>> + >>> #ifndef VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD >>> #define VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD 10 >>> #endif >>> @@ -173,6 +177,10 @@ struct vhost_device_ops { >>> >>> int (*vring_state_changed)(int vid, uint16_t queue_id, int enable); >> /**< triggered when a vring is enabled or disabled */ >>> >>> + int (*get_config)(int vid, uint8_t *config, uint32_t config_len>>> + int (*set_config)(int vid, uint8_t *config, uint32_t offset, >>> + uint32_t len, uint32_t flags); >>> + >>> /** >>> * Features could be changed after the feature negotiation. >>> * For example, VHOST_F_LOG_ALL will be set/cleared at the >>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c >>> index b086ad9..8e42734 100644 >>> --- a/lib/librte_vhost/vhost_user.c >>> +++ b/lib/librte_vhost/vhost_user.c >>> @@ -73,6 +73,8 @@ >>> [VHOST_USER_NET_SET_MTU] = "VHOST_USER_NET_SET_MTU", >>> [VHOST_USER_SET_SLAVE_REQ_FD] = >> "VHOST_USER_SET_SLAVE_REQ_FD", >>> [VHOST_USER_IOTLB_MSG] = "VHOST_USER_IOTLB_MSG", >>> + [VHOST_USER_GET_CONFIG] = "VHOST_USER_GET_CONFIG", >>> + [VHOST_USER_SET_CONFIG] = "VHOST_USER_SET_CONFIG", >>> [VHOST_USER_CRYPTO_CREATE_SESS] = >> "VHOST_USER_CRYPTO_CREATE_SESS", >>> [VHOST_USER_CRYPTO_CLOSE_SESS] = >> "VHOST_USER_CRYPTO_CLOSE_SESS", >>> [VHOST_USER_POSTCOPY_ADVISE] = "VHOST_USER_POSTCOPY_ADVISE", >>> @@ -359,6 +361,46 @@ >>> return RTE_VHOST_MSG_RESULT_OK; >>> } >>> >>> +static int >>> +vhost_user_get_config(struct virtio_net **pdev, struct VhostUserMsg *msg, >>> + int main_fd __rte_unused) >>> +{ >>> + struct virtio_net *dev = *pdev; >>> + >>> + if (!dev->notify_ops->get_config) { >>> + msg->size = sizeof(uint64_t); >>> + return RTE_VHOST_MSG_RESULT_REPLY; >>> + } >>> + >>> + if (dev->notify_ops->get_config(dev->vid, >>> + msg->payload.config.region, >>> + msg->payload.config.size) != 0) { >>> + msg->size = sizeof(uint64_t); >>> + } >>> + >>> + return RTE_VHOST_MSG_RESULT_REPLY; >>> +} >>> + >>> +static int >>> +vhost_user_set_config(struct virtio_net **pdev, struct VhostUserMsg *msg, >>> + int main_fd __rte_unused) >>> +{ >>> + struct virtio_net *dev = *pdev; >>> + >>> + if (!dev->notify_ops->set_config) >>> + return RTE_VHOST_MSG_RESULT_ERR; >>> + >>> + if (dev->notify_ops->set_config(dev->vid, >>> + msg->payload.config.region, >>> + msg->payload.config.offset, >>> + msg->payload.config.size, >>> + msg->payload.config.flags) != 0) { >>> + return RTE_VHOST_MSG_RESULT_ERR; >>> + } >>> + >>> + return RTE_VHOST_MSG_RESULT_OK; >>> +} >>> + >>> /* >>> * Reallocate virtio_dev and vhost_virtqueue data structure to make them on >> the >>> * same numa node as the memory of vring descriptor. >>> @@ -1725,6 +1767,8 @@ typedef int (*vhost_message_handler_t)(struct >> virtio_net **pdev, >>> [VHOST_USER_NET_SET_MTU] = vhost_user_net_set_mtu, >>> [VHOST_USER_SET_SLAVE_REQ_FD] = vhost_user_set_req_fd, >>> [VHOST_USER_IOTLB_MSG] = vhost_user_iotlb_msg, >>> + [VHOST_USER_GET_CONFIG] = vhost_user_get_config, >>> + [VHOST_USER_SET_CONFIG] = vhost_user_set_config, >>> [VHOST_USER_POSTCOPY_ADVISE] = vhost_user_set_postcopy_advise, >>> [VHOST_USER_POSTCOPY_LISTEN] = vhost_user_set_postcopy_listen, >>> [VHOST_USER_POSTCOPY_END] = vhost_user_postcopy_end, >>> diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h >>> index 2a650fe..057cdec 100644 >>> --- a/lib/librte_vhost/vhost_user.h >>> +++ b/lib/librte_vhost/vhost_user.h >>> @@ -12,6 +12,11 @@ >>> >>> /* refer to hw/virtio/vhost-user.c */ >>> >>> +/* >>> + * Maximum size of virtio device config space >>> + */ >>> +#define VHOST_USER_MAX_CONFIG_SIZE 256 >>> + >>> #define VHOST_MEMORY_MAX_NREGIONS 8 >>> >>> #define VHOST_USER_PROTOCOL_FEATURES ((1ULL << >> VHOST_USER_PROTOCOL_F_MQ) | \ >>> @@ -49,6 +54,8 @@ >>> VHOST_USER_NET_SET_MTU = 20, >>> VHOST_USER_SET_SLAVE_REQ_FD = 21, >>> VHOST_USER_IOTLB_MSG = 22, >>> + VHOST_USER_GET_CONFIG = 24, >>> + VHOST_USER_SET_CONFIG = 25, >>> VHOST_USER_CRYPTO_CREATE_SESS = 26, >>> VHOST_USER_CRYPTO_CLOSE_SESS = 27, >>> VHOST_USER_POSTCOPY_ADVISE = 28, >>> @@ -60,6 +67,7 @@ >>> typedef enum VhostUserSlaveRequest { >>> VHOST_USER_SLAVE_NONE = 0, >>> VHOST_USER_SLAVE_IOTLB_MSG = 1, >>> + VHOST_USER_SLAVE_CONFIG_CHANGE_MSG = 2, >>> VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = 3, >>> VHOST_USER_SLAVE_MAX >>> } VhostUserSlaveRequest; >>> @@ -112,6 +120,13 @@ >>> uint64_t offset; >>> } VhostUserVringArea; >>> >>> +typedef struct VhostUserConfig { >>> + uint32_t offset; >>> + uint32_t size; >>> + uint32_t flags; >>> + uint8_t region[VHOST_USER_MAX_CONFIG_SIZE]; >>> +} VhostUserConfig; >>> + >>> typedef struct VhostUserMsg { >>> union { >>> uint32_t master; /* a VhostUserRequest value */ >>> @@ -131,6 +146,7 @@ >>> struct vhost_vring_addr addr; >>> VhostUserMemory memory; >>> VhostUserLog log; >>> + VhostUserConfig config; >>> struct vhost_iotlb_msg iotlb; >>> VhostUserCryptoSessionParam crypto_session; >>> VhostUserVringArea area; >>>