From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 15E0F2C30 for ; Tue, 26 Feb 2019 09:42:10 +0100 (CET) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20190226084210euoutp02186bca4cf8a2e0878ad1c9048ddcd3c4~G3WDrPtKS0778407784euoutp02m for ; Tue, 26 Feb 2019 08:42:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20190226084210euoutp02186bca4cf8a2e0878ad1c9048ddcd3c4~G3WDrPtKS0778407784euoutp02m DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1551170530; bh=0IVdny6Zl6Y8TrGVLH0xc4ak5wUQl0oEwxeaUBAiyK8=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=HjXAo2D35//YBKWNLkahppMeSufFchGskplnsZX+cjgFvPIrNCcpKyzWgB/Y923hT jxXFozewQeeSuPwSYTebcXA1ibha12EyOD7oUl2hn6ezllqIVkYEy+yg3VOe2jrv6A 6frJloeE8zNozHY6tDhiCqL+eSWtuuknx993e3iw= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20190226084209eucas1p13c171d1192724b499aaff83533dd9e52~G3WDOSprn2823528235eucas1p1B; Tue, 26 Feb 2019 08:42:09 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id C2.54.04806.1EBF47C5; Tue, 26 Feb 2019 08:42:09 +0000 (GMT) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20190226084208eucas1p1ba33b23cc21b0ace6229c61bdc84b027~G3WCcoDv00234302343eucas1p1c; Tue, 26 Feb 2019 08:42:08 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20190226084208eusmtrp25523a4df905fb79932faa948435a3186~G3WCNvWsH0799907999eusmtrp2O; Tue, 26 Feb 2019 08:42:08 +0000 (GMT) X-AuditID: cbfec7f5-34dff700000012c6-a5-5c74fbe1dad2 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 5E.D1.04128.0EBF47C5; Tue, 26 Feb 2019 08:42:08 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20190226084207eusmtip259ce4bd02e485a7681fd415d046ab010~G3WBpYCHB2417124171eusmtip2B; Tue, 26 Feb 2019 08:42:07 +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: <60245e11-ff68-133d-2577-8526c89e6264@samsung.com> Date: Tue, 26 Feb 2019 11:42:07 +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+NgFlrBKsWRmVeSWpSXmKPExsWy7djPc7oPf5fEGHy8K2Bx8edXdov3v9cw Wrz7tJ3J4kr7T3aLZZc+M1kc69zDYrG14T+TxeaLk5gcODx+LVjK6rF4z0smj/f7rrJ59G1Z xRjAEsVlk5Kak1mWWqRvl8CV0X5rAWtBt2/FkpW9zA2MPx26GDk5JARMJLacu8XcxcjFISSw glHi+cRpjBDOF0aJljvtjCBVQgKfGSXezcqC6di/7SEbRNFyRolv97awQBR9ZJTYvE8CxBYW 8JR4cukbWFxEwF/i5f+DrCA2s8BzRoln6zxAbDYBHYlTq4+ALeAVsJNY/uQ5O4jNIqAqse31 DSYQW1QgQuJw7zuoGkGJkzOfgM3kFAiReLO1jx1iprhE05eVUPPlJba/ncMMceg2dolbj2sg bBeJE5cOMkHYwhKvjm9hh7BlJP7vnA8Vr5e43/IS7HsJgQ5GiemH/kEl7CW2vD4H1MABtEBT Yv0ufRBTQsBRovG8GITJJ3HjrSDEBXwSk7ZNZ4YI80p0tAlBzFCR+H1wOdRhUhI3331mn8Co NAvJX7OQ/DILyS+zENYuYGRZxSieWlqcm55abJyXWq5XnJhbXJqXrpecn7uJEZh+Tv87/nUH 474/SYcYBTgYlXh4T9wvjhFiTSwrrsw9xCjBwawkwiv9qyRGiDclsbIqtSg/vqg0J7X4EKM0 B4uSOG81w4NoIYH0xJLU7NTUgtQimCwTB6dUA+PZtM7gJZn5v0WTEh8FNXH598w+p/3idsX0 A433LlRwKYecjHU/qv/uuVSHE3vAz3MLNrM+2b731qpzS6T63mclV/E5dx8xdNOXl7LRcGqs Odbi/GzVgneysnsKdJ65v2+YrfDe7u2XiVl1T5MeX9frzTN/e/EYQ7nYy/KDDhM+zr1Wqr19 i5USS3FGoqEWc1FxIgA1OXo8OwMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrAIsWRmVeSWpSXmKPExsVy+t/xe7oPfpfEGBzuELW4+PMru8X732sY Ld592s5kcaX9J7vFskufmSyOde5hsdja8J/JYvPFSUwOHB6/Fixl9Vi85yWTx/t9V9k8+ras YgxgidKzKcovLUlVyMgvLrFVija0MNIztLTQMzKx1DM0No+1MjJV0rezSUnNySxLLdK3S9DL aL+1gLWg27diycpe5gbGnw5djJwcEgImEvu3PWTrYuTiEBJYyijRufoEO0RCSuLHrwusELaw xJ9rXVBF7xklnq1qYAFJCAt4Sjy59A3MFhHwlTi04CQrSBGzwHNGiRUtWxghOmYySzQsPAxW xSagI3Fq9RFGEJtXwE5i+ZPnYOtYBFQltr2+wQRiiwpESHx8uo8JokZQ4uTMJ2C9nAIhEm+2 9oHVMwuoS/yZd4kZwhaXaPqykhXClpfY/nYO8wRGoVlI2mchaZmFpGUWkpYFjCyrGEVSS4tz 03OLjfSKE3OLS/PS9ZLzczcxAmNv27GfW3Ywdr0LPsQowMGoxMOr8Kg4Rog1say4MvcQowQH s5IIr/Svkhgh3pTEyqrUovz4otKc1OJDjKZAz01klhJNzgemhbySeENTQ3MLS0NzY3NjMwsl cd7zBpVRQgLpiSWp2ampBalFMH1MHJxSDYzLy9cJ/coIUNm/WJtx7rVOJwGZQ3q/rm67eHbb rL4fd15tsF4y5980ZclJkaeO6SrIqHyzKq369vth0f943X2yp+1W+EnN+uqcU9pQn19ybcmp dxdrn+9a++nHrdqE+9k/us92LYpdfmSuetb+9tBz+hrn+W9eutLHofFst7X86Zq97xi2871Q UGIpzkg01GIuKk4EAIjrBj3TAgAA X-CMS-MailID: 20190226084208eucas1p1ba33b23cc21b0ace6229c61bdc84b027 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> <4ba6108c-78b0-c7e6-f810-3faa13c7765e@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 08:42:11 -0000 On 26.02.2019 11:13, Liu, Changpeng wrote: > > >> -----Original Message----- >> From: Ilya Maximets [mailto:i.maximets@samsung.com] >> Sent: Tuesday, February 26, 2019 3:39 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 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 ? > The patch was developed one year ago, while DPDK didn't have external ops. So, maybe it's time to reconsider the implementation. > The get_config/set_config was defined for all the virtio devices, so I think it makes > more sense adding here. VHOST_USER_*_CRYPTO_SESSION messages are defined for all the virtio devices too, however they makes sense for vhost_crypto backend only. These messages (GET/SET_CONFIG) makes sense only when callbacks (get/set_config) are implemented, so IMHO it's better to implement their handlers along with the callbacks, i.e. inside the implementation of your vhost backend. Maxime, Tiwei, what do you think ? >> >> >>>> >>>> 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; >>>>>