* [dpdk-dev] [PATCH 21.02 v2 0/2] Vhost-vDPA fixes @ 2020-11-25 15:21 Maxime Coquelin 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation Maxime Coquelin 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend Maxime Coquelin 0 siblings, 2 replies; 8+ messages in thread From: Maxime Coquelin @ 2020-11-25 15:21 UTC (permalink / raw) To: dev, chenbo.xia, amorenoz, jasowang, david.marchand; +Cc: Maxime Coquelin Vhost-vDPA support in Virtio-user PMD was lacking backend features negotiation and IOTLB batching support, causing intialization issues with Mellanox vDPA device managed by Kernel vDPA framework. Changes in v1: ============= - Add backend features negotiation (David, Jason) - Ensure Kernel supports IOTLB MSG v2 Maxime Coquelin (2): net/virtio: fix missing backend features negotiation net/virtio: fix memory init with vDPA backend drivers/net/virtio/virtio_user/vhost.h | 8 ++ drivers/net/virtio/virtio_user/vhost_vdpa.c | 120 +++++++++++++++++- .../net/virtio/virtio_user/virtio_user_dev.c | 15 ++- 3 files changed, 134 insertions(+), 9 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation 2020-11-25 15:21 [dpdk-dev] [PATCH 21.02 v2 0/2] Vhost-vDPA fixes Maxime Coquelin @ 2020-11-25 15:21 ` Maxime Coquelin 2020-12-21 6:23 ` Xia, Chenbo 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend Maxime Coquelin 1 sibling, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2020-11-25 15:21 UTC (permalink / raw) To: dev, chenbo.xia, amorenoz, jasowang, david.marchand Cc: Maxime Coquelin, stable This patch adds missing backend features negotiation for in Vhost-vDPA. Without it, IOTLB messages v2 could be sent by Virtio-user PMD while not supported by the backend. Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost.h | 4 ++++ drivers/net/virtio/virtio_user/vhost_vdpa.c | 14 ++++++++++++++ drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 ++++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h index 210a3704e7..c1dcc50b58 100644 --- a/drivers/net/virtio/virtio_user/vhost.h +++ b/drivers/net/virtio/virtio_user/vhost.h @@ -86,6 +86,10 @@ enum vhost_user_request { VHOST_USER_MAX }; +#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2 +#define VHOST_BACKEND_F_IOTLB_MSG_V2 1 +#endif + extern const char * const vhost_msg_strings[VHOST_USER_MAX]; struct vhost_memory_region { diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c index c7b9349fc8..b6c81d6f17 100644 --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c @@ -35,6 +35,8 @@ #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8) #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, \ struct vhost_vring_state) +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64) static uint64_t vhost_req_user_to_vdpa[] = { [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER, @@ -51,6 +53,8 @@ static uint64_t vhost_req_user_to_vdpa[] = { [VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS, [VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS, [VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE, + [VHOST_USER_GET_PROTOCOL_FEATURES] = VHOST_GET_BACKEND_FEATURES, + [VHOST_USER_SET_PROTOCOL_FEATURES] = VHOST_SET_BACKEND_FEATURES, }; /* no alignment requirement */ @@ -86,6 +90,11 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr, { struct vhost_msg msg = {}; + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) { + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); + return -1; + } + msg.type = VHOST_IOTLB_MSG_V2; msg.iotlb.type = VHOST_IOTLB_UPDATE; msg.iotlb.iova = iova; @@ -108,6 +117,11 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, __rte_unused void *addr, { struct vhost_msg msg = {}; + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) { + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); + return -1; + } + msg.type = VHOST_IOTLB_MSG_V2; msg.iotlb.type = VHOST_IOTLB_INVALIDATE; msg.iotlb.iova = iova; diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 053f0267ca..96bc6b232d 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -439,11 +439,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) 1ULL << VIRTIO_F_RING_PACKED | \ 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) -#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES \ (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ 1ULL << VHOST_USER_PROTOCOL_F_STATUS) +#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, int cq, int queue_size, const char *mac, char **ifname, @@ -462,9 +464,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, dev->mac_specified = 0; dev->frontend_features = 0; dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; - dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; dev->backend_type = backend_type; + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) + dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; + else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) + dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES; + parse_mac(dev, mac); if (*ifname) { @@ -497,8 +503,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, } - if (dev->device_features & - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { + if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES) || + dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) { if (dev->ops->send_request(dev, VHOST_USER_GET_PROTOCOL_FEATURES, &protocol_features)) -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation Maxime Coquelin @ 2020-12-21 6:23 ` Xia, Chenbo 2020-12-22 10:55 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Xia, Chenbo @ 2020-12-21 6:23 UTC (permalink / raw) To: Maxime Coquelin, dev, amorenoz, jasowang, david.marchand; +Cc: stable Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, November 25, 2020 11:21 PM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com; > jasowang@redhat.com; david.marchand@redhat.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org > Subject: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features > negotiation > > This patch adds missing backend features negotiation for > in Vhost-vDPA. Without it, IOTLB messages v2 could be sent > by Virtio-user PMD while not supported by the backend. > > Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") > Cc: stable@dpdk.org > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/net/virtio/virtio_user/vhost.h | 4 ++++ > drivers/net/virtio/virtio_user/vhost_vdpa.c | 14 ++++++++++++++ > drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 ++++++++++---- > 3 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/vhost.h > b/drivers/net/virtio/virtio_user/vhost.h > index 210a3704e7..c1dcc50b58 100644 > --- a/drivers/net/virtio/virtio_user/vhost.h > +++ b/drivers/net/virtio/virtio_user/vhost.h > @@ -86,6 +86,10 @@ enum vhost_user_request { > VHOST_USER_MAX > }; > > +#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2 > +#define VHOST_BACKEND_F_IOTLB_MSG_V2 1 > +#endif > + > extern const char * const vhost_msg_strings[VHOST_USER_MAX]; > > struct vhost_memory_region { > diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c > b/drivers/net/virtio/virtio_user/vhost_vdpa.c > index c7b9349fc8..b6c81d6f17 100644 > --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c > +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c > @@ -35,6 +35,8 @@ > #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8) > #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, \ > struct vhost_vring_state) > +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) > +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64) > > static uint64_t vhost_req_user_to_vdpa[] = { > [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER, > @@ -51,6 +53,8 @@ static uint64_t vhost_req_user_to_vdpa[] = { > [VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS, > [VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS, > [VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE, > + [VHOST_USER_GET_PROTOCOL_FEATURES] = VHOST_GET_BACKEND_FEATURES, > + [VHOST_USER_SET_PROTOCOL_FEATURES] = VHOST_SET_BACKEND_FEATURES, > }; > > /* no alignment requirement */ > @@ -86,6 +90,11 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr, > { > struct vhost_msg msg = {}; > > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > { > + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > + return -1; > + } > + > msg.type = VHOST_IOTLB_MSG_V2; > msg.iotlb.type = VHOST_IOTLB_UPDATE; > msg.iotlb.iova = iova; > @@ -108,6 +117,11 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, > __rte_unused void *addr, > { > struct vhost_msg msg = {}; > > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > { > + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > + return -1; > + } > + > msg.type = VHOST_IOTLB_MSG_V2; > msg.iotlb.type = VHOST_IOTLB_INVALIDATE; > msg.iotlb.iova = iova; > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > b/drivers/net/virtio/virtio_user/virtio_user_dev.c > index 053f0267ca..96bc6b232d 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > @@ -439,11 +439,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) > 1ULL << VIRTIO_F_RING_PACKED | \ > 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) > > -#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ > +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES \ > (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ > 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ > 1ULL << VHOST_USER_PROTOCOL_F_STATUS) > > +#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ > + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) > int > virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > int cq, int queue_size, const char *mac, char **ifname, > @@ -462,9 +464,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > dev->mac_specified = 0; > dev->frontend_features = 0; > dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; > - dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; > dev->backend_type = backend_type; > > + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) > + dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; > + else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) > + dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES; > + > parse_mac(dev, mac); > > if (*ifname) { > @@ -497,8 +503,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > *path, int queues, > } > > > - if (dev->device_features & > - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { > + if ((dev->device_features & (1ULL << > VHOST_USER_F_PROTOCOL_FEATURES) || > + dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) Do you mean: if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) || (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) { here? Besides, I think it's better to update here as vhost-vdpa also uses protocol_features. (http://code.dpdk.org/dpdk/v20.08/source/drivers/net/virtio/virtio_user/virtio_user_dev.h#L44) Thanks! Chenbo > { > if (dev->ops->send_request(dev, > VHOST_USER_GET_PROTOCOL_FEATURES, > &protocol_features)) > -- > 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation 2020-12-21 6:23 ` Xia, Chenbo @ 2020-12-22 10:55 ` Maxime Coquelin 2020-12-23 5:37 ` Xia, Chenbo 0 siblings, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2020-12-22 10:55 UTC (permalink / raw) To: Xia, Chenbo, dev, amorenoz, jasowang, david.marchand; +Cc: stable On 12/21/20 7:23 AM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Wednesday, November 25, 2020 11:21 PM >> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com; >> jasowang@redhat.com; david.marchand@redhat.com >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org >> Subject: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features >> negotiation >> >> This patch adds missing backend features negotiation for >> in Vhost-vDPA. Without it, IOTLB messages v2 could be sent >> by Virtio-user PMD while not supported by the backend. >> >> Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") >> Cc: stable@dpdk.org >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> drivers/net/virtio/virtio_user/vhost.h | 4 ++++ >> drivers/net/virtio/virtio_user/vhost_vdpa.c | 14 ++++++++++++++ >> drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 ++++++++++---- >> 3 files changed, 28 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/virtio/virtio_user/vhost.h >> b/drivers/net/virtio/virtio_user/vhost.h >> index 210a3704e7..c1dcc50b58 100644 >> --- a/drivers/net/virtio/virtio_user/vhost.h >> +++ b/drivers/net/virtio/virtio_user/vhost.h >> @@ -86,6 +86,10 @@ enum vhost_user_request { >> VHOST_USER_MAX >> }; >> >> +#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2 >> +#define VHOST_BACKEND_F_IOTLB_MSG_V2 1 >> +#endif >> + >> extern const char * const vhost_msg_strings[VHOST_USER_MAX]; >> >> struct vhost_memory_region { >> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c >> b/drivers/net/virtio/virtio_user/vhost_vdpa.c >> index c7b9349fc8..b6c81d6f17 100644 >> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c >> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c >> @@ -35,6 +35,8 @@ >> #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8) >> #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, \ >> struct vhost_vring_state) >> +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) >> +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64) >> >> static uint64_t vhost_req_user_to_vdpa[] = { >> [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER, >> @@ -51,6 +53,8 @@ static uint64_t vhost_req_user_to_vdpa[] = { >> [VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS, >> [VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS, >> [VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE, >> + [VHOST_USER_GET_PROTOCOL_FEATURES] = VHOST_GET_BACKEND_FEATURES, >> + [VHOST_USER_SET_PROTOCOL_FEATURES] = VHOST_SET_BACKEND_FEATURES, >> }; >> >> /* no alignment requirement */ >> @@ -86,6 +90,11 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr, >> { >> struct vhost_msg msg = {}; >> >> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) >> { >> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); >> + return -1; >> + } >> + >> msg.type = VHOST_IOTLB_MSG_V2; >> msg.iotlb.type = VHOST_IOTLB_UPDATE; >> msg.iotlb.iova = iova; >> @@ -108,6 +117,11 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, >> __rte_unused void *addr, >> { >> struct vhost_msg msg = {}; >> >> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) >> { >> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); >> + return -1; >> + } >> + >> msg.type = VHOST_IOTLB_MSG_V2; >> msg.iotlb.type = VHOST_IOTLB_INVALIDATE; >> msg.iotlb.iova = iova; >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> index 053f0267ca..96bc6b232d 100644 >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >> @@ -439,11 +439,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >> 1ULL << VIRTIO_F_RING_PACKED | \ >> 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) >> >> -#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ >> +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES \ >> (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ >> 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ >> 1ULL << VHOST_USER_PROTOCOL_F_STATUS) >> >> +#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ >> + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) >> int >> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >> int cq, int queue_size, const char *mac, char **ifname, >> @@ -462,9 +464,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> dev->mac_specified = 0; >> dev->frontend_features = 0; >> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; >> - dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; >> dev->backend_type = backend_type; >> >> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) >> + dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; >> + else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) >> + dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES; >> + >> parse_mac(dev, mac); >> >> if (*ifname) { >> @@ -497,8 +503,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >> *path, int queues, >> } >> >> >> - if (dev->device_features & >> - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { >> + if ((dev->device_features & (1ULL << >> VHOST_USER_F_PROTOCOL_FEATURES) || >> + dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) > > Do you mean: > > if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) || > (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) { > > here? Indeed! > Besides, I think it's better to update here as vhost-vdpa also uses protocol_features. > (http://code.dpdk.org/dpdk/v20.08/source/drivers/net/virtio/virtio_user/virtio_user_dev.h#L44) Not sure what you mean here? Can you please elaborate? Thanks! Maxime > Thanks! > Chenbo > >> { >> if (dev->ops->send_request(dev, >> VHOST_USER_GET_PROTOCOL_FEATURES, >> &protocol_features)) >> -- >> 2.26.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation 2020-12-22 10:55 ` Maxime Coquelin @ 2020-12-23 5:37 ` Xia, Chenbo 2021-01-05 16:53 ` Maxime Coquelin 0 siblings, 1 reply; 8+ messages in thread From: Xia, Chenbo @ 2020-12-23 5:37 UTC (permalink / raw) To: Maxime Coquelin, dev, amorenoz, jasowang, david.marchand; +Cc: stable Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Tuesday, December 22, 2020 6:56 PM > To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org; amorenoz@redhat.com; > jasowang@redhat.com; david.marchand@redhat.com > Cc: stable@dpdk.org > Subject: Re: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features > negotiation > > > > On 12/21/20 7:23 AM, Xia, Chenbo wrote: > > Hi Maxime, > > > >> -----Original Message----- > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Sent: Wednesday, November 25, 2020 11:21 PM > >> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com; > >> jasowang@redhat.com; david.marchand@redhat.com > >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org > >> Subject: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features > >> negotiation > >> > >> This patch adds missing backend features negotiation for > >> in Vhost-vDPA. Without it, IOTLB messages v2 could be sent > >> by Virtio-user PMD while not supported by the backend. > >> > >> Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") > >> Cc: stable@dpdk.org > >> > >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >> --- > >> drivers/net/virtio/virtio_user/vhost.h | 4 ++++ > >> drivers/net/virtio/virtio_user/vhost_vdpa.c | 14 ++++++++++++++ > >> drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 ++++++++++---- > >> 3 files changed, 28 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/net/virtio/virtio_user/vhost.h > >> b/drivers/net/virtio/virtio_user/vhost.h > >> index 210a3704e7..c1dcc50b58 100644 > >> --- a/drivers/net/virtio/virtio_user/vhost.h > >> +++ b/drivers/net/virtio/virtio_user/vhost.h > >> @@ -86,6 +86,10 @@ enum vhost_user_request { > >> VHOST_USER_MAX > >> }; > >> > >> +#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2 > >> +#define VHOST_BACKEND_F_IOTLB_MSG_V2 1 > >> +#endif > >> + > >> extern const char * const vhost_msg_strings[VHOST_USER_MAX]; > >> > >> struct vhost_memory_region { > >> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c > >> b/drivers/net/virtio/virtio_user/vhost_vdpa.c > >> index c7b9349fc8..b6c81d6f17 100644 > >> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c > >> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c > >> @@ -35,6 +35,8 @@ > >> #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8) > >> #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, \ > >> struct vhost_vring_state) > >> +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) > >> +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64) > >> > >> static uint64_t vhost_req_user_to_vdpa[] = { > >> [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER, > >> @@ -51,6 +53,8 @@ static uint64_t vhost_req_user_to_vdpa[] = { > >> [VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS, > >> [VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS, > >> [VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE, > >> + [VHOST_USER_GET_PROTOCOL_FEATURES] = VHOST_GET_BACKEND_FEATURES, > >> + [VHOST_USER_SET_PROTOCOL_FEATURES] = VHOST_SET_BACKEND_FEATURES, > >> }; > >> > >> /* no alignment requirement */ > >> @@ -86,6 +90,11 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void > *addr, > >> { > >> struct vhost_msg msg = {}; > >> > >> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > >> { > >> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > >> + return -1; > >> + } > >> + > >> msg.type = VHOST_IOTLB_MSG_V2; > >> msg.iotlb.type = VHOST_IOTLB_UPDATE; > >> msg.iotlb.iova = iova; > >> @@ -108,6 +117,11 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, > >> __rte_unused void *addr, > >> { > >> struct vhost_msg msg = {}; > >> > >> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > >> { > >> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > >> + return -1; > >> + } > >> + > >> msg.type = VHOST_IOTLB_MSG_V2; > >> msg.iotlb.type = VHOST_IOTLB_INVALIDATE; > >> msg.iotlb.iova = iova; > >> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > >> b/drivers/net/virtio/virtio_user/virtio_user_dev.c > >> index 053f0267ca..96bc6b232d 100644 > >> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > >> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > >> @@ -439,11 +439,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) > >> 1ULL << VIRTIO_F_RING_PACKED | \ > >> 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) > >> > >> -#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ > >> +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES \ > >> (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ > >> 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ > >> 1ULL << VHOST_USER_PROTOCOL_F_STATUS) > >> > >> +#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ > >> + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) > >> int > >> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > >> int cq, int queue_size, const char *mac, char **ifname, > >> @@ -462,9 +464,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > >> *path, int queues, > >> dev->mac_specified = 0; > >> dev->frontend_features = 0; > >> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; > >> - dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; > >> dev->backend_type = backend_type; > >> > >> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) > >> + dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; > >> + else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) > >> + dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES; > >> + > >> parse_mac(dev, mac); > >> > >> if (*ifname) { > >> @@ -497,8 +503,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char > >> *path, int queues, > >> } > >> > >> > >> - if (dev->device_features & > >> - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { > >> + if ((dev->device_features & (1ULL << > >> VHOST_USER_F_PROTOCOL_FEATURES) || > >> + dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) > > > > Do you mean: > > > > if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) || > > (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) > { > > > > here? > > Indeed! > > > Besides, I think it's better to update here as vhost-vdpa also uses > protocol_features. > > > (http://code.dpdk.org/dpdk/v20.08/source/drivers/net/virtio/virtio_user/virtio > _user_dev.h#L44) > > Not sure what you mean here? Can you please elaborate? Sorry, I'm not clear on this. It's just a small point about code comment. In http://code.dpdk.org/dpdk/v20.11/source/drivers/net/virtio/virtio_user/virtio_user_dev.h#L52, It says 'negotiated protocol features(vhost-user only)'. Since it's also used by vhost-vdpa now, maybe it's better to improve it. What do you think? Thanks! Chenbo > > Thanks! > Maxime > > > Thanks! > > Chenbo > > > >> { > >> if (dev->ops->send_request(dev, > >> VHOST_USER_GET_PROTOCOL_FEATURES, > >> &protocol_features)) > >> -- > >> 2.26.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation 2020-12-23 5:37 ` Xia, Chenbo @ 2021-01-05 16:53 ` Maxime Coquelin 0 siblings, 0 replies; 8+ messages in thread From: Maxime Coquelin @ 2021-01-05 16:53 UTC (permalink / raw) To: Xia, Chenbo, dev, amorenoz, jasowang, david.marchand; +Cc: stable On 12/23/20 6:37 AM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Tuesday, December 22, 2020 6:56 PM >> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org; amorenoz@redhat.com; >> jasowang@redhat.com; david.marchand@redhat.com >> Cc: stable@dpdk.org >> Subject: Re: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features >> negotiation >> >> >> >> On 12/21/20 7:23 AM, Xia, Chenbo wrote: >>> Hi Maxime, >>> >>>> -----Original Message----- >>>> From: Maxime Coquelin <maxime.coquelin@redhat.com> >>>> Sent: Wednesday, November 25, 2020 11:21 PM >>>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com; >>>> jasowang@redhat.com; david.marchand@redhat.com >>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org >>>> Subject: [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features >>>> negotiation >>>> >>>> This patch adds missing backend features negotiation for >>>> in Vhost-vDPA. Without it, IOTLB messages v2 could be sent >>>> by Virtio-user PMD while not supported by the backend. >>>> >>>> Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >>>> --- >>>> drivers/net/virtio/virtio_user/vhost.h | 4 ++++ >>>> drivers/net/virtio/virtio_user/vhost_vdpa.c | 14 ++++++++++++++ >>>> drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 ++++++++++---- >>>> 3 files changed, 28 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/net/virtio/virtio_user/vhost.h >>>> b/drivers/net/virtio/virtio_user/vhost.h >>>> index 210a3704e7..c1dcc50b58 100644 >>>> --- a/drivers/net/virtio/virtio_user/vhost.h >>>> +++ b/drivers/net/virtio/virtio_user/vhost.h >>>> @@ -86,6 +86,10 @@ enum vhost_user_request { >>>> VHOST_USER_MAX >>>> }; >>>> >>>> +#ifndef VHOST_BACKEND_F_IOTLB_MSG_V2 >>>> +#define VHOST_BACKEND_F_IOTLB_MSG_V2 1 >>>> +#endif >>>> + >>>> extern const char * const vhost_msg_strings[VHOST_USER_MAX]; >>>> >>>> struct vhost_memory_region { >>>> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c >>>> b/drivers/net/virtio/virtio_user/vhost_vdpa.c >>>> index c7b9349fc8..b6c81d6f17 100644 >>>> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c >>>> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c >>>> @@ -35,6 +35,8 @@ >>>> #define VHOST_VDPA_SET_STATUS _IOW(VHOST_VIRTIO, 0x72, __u8) >>>> #define VHOST_VDPA_SET_VRING_ENABLE _IOW(VHOST_VIRTIO, 0x75, \ >>>> struct vhost_vring_state) >>>> +#define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64) >>>> +#define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64) >>>> >>>> static uint64_t vhost_req_user_to_vdpa[] = { >>>> [VHOST_USER_SET_OWNER] = VHOST_SET_OWNER, >>>> @@ -51,6 +53,8 @@ static uint64_t vhost_req_user_to_vdpa[] = { >>>> [VHOST_USER_SET_STATUS] = VHOST_VDPA_SET_STATUS, >>>> [VHOST_USER_GET_STATUS] = VHOST_VDPA_GET_STATUS, >>>> [VHOST_USER_SET_VRING_ENABLE] = VHOST_VDPA_SET_VRING_ENABLE, >>>> + [VHOST_USER_GET_PROTOCOL_FEATURES] = VHOST_GET_BACKEND_FEATURES, >>>> + [VHOST_USER_SET_PROTOCOL_FEATURES] = VHOST_SET_BACKEND_FEATURES, >>>> }; >>>> >>>> /* no alignment requirement */ >>>> @@ -86,6 +90,11 @@ vhost_vdpa_dma_map(struct virtio_user_dev *dev, void >> *addr, >>>> { >>>> struct vhost_msg msg = {}; >>>> >>>> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) >>>> { >>>> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); >>>> + return -1; >>>> + } >>>> + >>>> msg.type = VHOST_IOTLB_MSG_V2; >>>> msg.iotlb.type = VHOST_IOTLB_UPDATE; >>>> msg.iotlb.iova = iova; >>>> @@ -108,6 +117,11 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, >>>> __rte_unused void *addr, >>>> { >>>> struct vhost_msg msg = {}; >>>> >>>> + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) >>>> { >>>> + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); >>>> + return -1; >>>> + } >>>> + >>>> msg.type = VHOST_IOTLB_MSG_V2; >>>> msg.iotlb.type = VHOST_IOTLB_INVALIDATE; >>>> msg.iotlb.iova = iova; >>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> index 053f0267ca..96bc6b232d 100644 >>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c >>>> @@ -439,11 +439,13 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) >>>> 1ULL << VIRTIO_F_RING_PACKED | \ >>>> 1ULL << VHOST_USER_F_PROTOCOL_FEATURES) >>>> >>>> -#define VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES \ >>>> +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES \ >>>> (1ULL << VHOST_USER_PROTOCOL_F_MQ | \ >>>> 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK | \ >>>> 1ULL << VHOST_USER_PROTOCOL_F_STATUS) >>>> >>>> +#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ >>>> + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) >>>> int >>>> virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, >>>> int cq, int queue_size, const char *mac, char **ifname, >>>> @@ -462,9 +464,13 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>>> *path, int queues, >>>> dev->mac_specified = 0; >>>> dev->frontend_features = 0; >>>> dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES; >>>> - dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES; >>>> dev->backend_type = backend_type; >>>> >>>> + if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) >>>> + dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES; >>>> + else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) >>>> + dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES; >>>> + >>>> parse_mac(dev, mac); >>>> >>>> if (*ifname) { >>>> @@ -497,8 +503,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char >>>> *path, int queues, >>>> } >>>> >>>> >>>> - if (dev->device_features & >>>> - (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) { >>>> + if ((dev->device_features & (1ULL << >>>> VHOST_USER_F_PROTOCOL_FEATURES) || >>>> + dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) >>> >>> Do you mean: >>> >>> if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) || >>> (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)) >> { >>> >>> here? >> >> Indeed! >> >>> Besides, I think it's better to update here as vhost-vdpa also uses >> protocol_features. >>> >> (http://code.dpdk.org/dpdk/v20.08/source/drivers/net/virtio/virtio_user/virtio >> _user_dev.h#L44) >> >> Not sure what you mean here? Can you please elaborate? > > Sorry, I'm not clear on this. It's just a small point about code comment. > > In http://code.dpdk.org/dpdk/v20.11/source/drivers/net/virtio/virtio_user/virtio_user_dev.h#L52, > It says 'negotiated protocol features(vhost-user only)'. Since it's also used by vhost-vdpa now, > maybe it's better to improve it. What do you think? I think it makes sense! Note that in my Virtio PMD rework series, this field disappears, meaning that once this series applied, I will need to rebase my rework on top of it so that vhost-vdpa backend features is moved into vhost_vdpa.c. In the mean time, I will remove the "(vhost-user only)" comment. Thanks, Maxime > Thanks! > Chenbo > >> >> Thanks! >> Maxime >> >>> Thanks! >>> Chenbo >>> >>>> { >>>> if (dev->ops->send_request(dev, >>>> VHOST_USER_GET_PROTOCOL_FEATURES, >>>> &protocol_features)) >>>> -- >>>> 2.26.2 >>> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend 2020-11-25 15:21 [dpdk-dev] [PATCH 21.02 v2 0/2] Vhost-vDPA fixes Maxime Coquelin 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation Maxime Coquelin @ 2020-11-25 15:21 ` Maxime Coquelin 2020-12-21 6:23 ` Xia, Chenbo 1 sibling, 1 reply; 8+ messages in thread From: Maxime Coquelin @ 2020-11-25 15:21 UTC (permalink / raw) To: dev, chenbo.xia, amorenoz, jasowang, david.marchand Cc: Maxime Coquelin, stable This patch fixes an overhead met with mlx5-vdpa Kernel driver, where for every page in the mapped area, all the memory tables gets updated. For example, with 2MB hugepages, a single IOTLB_UPDATE for a 1GB region causes 512 memory updates on mlx5-vdpa side. Using batching mode, the mlx5 driver will only trigger a single memory update for all the IOTLB updates that happen between the batch begin and batch end commands. Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") Cc: stable@dpdk.org Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost.h | 4 + drivers/net/virtio/virtio_user/vhost_vdpa.c | 106 +++++++++++++++++- .../net/virtio/virtio_user/virtio_user_dev.c | 3 +- 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h index c1dcc50b58..be286173b0 100644 --- a/drivers/net/virtio/virtio_user/vhost.h +++ b/drivers/net/virtio/virtio_user/vhost.h @@ -90,6 +90,10 @@ enum vhost_user_request { #define VHOST_BACKEND_F_IOTLB_MSG_V2 1 #endif +#ifndef VHOST_BACKEND_F_IOTLB_BATCH +#define VHOST_BACKEND_F_IOTLB_BATCH 2 +#endif + extern const char * const vhost_msg_strings[VHOST_USER_MAX]; struct vhost_memory_region { diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c index b6c81d6f17..269bab2f8e 100644 --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c @@ -70,6 +70,8 @@ struct vhost_iotlb_msg { #define VHOST_IOTLB_UPDATE 2 #define VHOST_IOTLB_INVALIDATE 3 #define VHOST_IOTLB_ACCESS_FAIL 4 +#define VHOST_IOTLB_BATCH_BEGIN 5 +#define VHOST_IOTLB_BATCH_END 6 uint8_t type; }; @@ -84,6 +86,56 @@ struct vhost_msg { }; }; +static int +vhost_vdpa_iotlb_batch_begin(struct virtio_user_dev *dev) +{ + struct vhost_msg msg = {}; + + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) + return 0; + + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) { + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); + return -1; + } + + msg.type = VHOST_IOTLB_MSG_V2; + msg.iotlb.type = VHOST_IOTLB_BATCH_BEGIN; + + if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) { + PMD_DRV_LOG(ERR, "Failed to send IOTLB batch begin (%s)", + strerror(errno)); + return -1; + } + + return 0; +} + +static int +vhost_vdpa_iotlb_batch_end(struct virtio_user_dev *dev) +{ + struct vhost_msg msg = {}; + + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) + return 0; + + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) { + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); + return -1; + } + + msg.type = VHOST_IOTLB_MSG_V2; + msg.iotlb.type = VHOST_IOTLB_BATCH_END; + + if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) { + PMD_DRV_LOG(ERR, "Failed to send IOTLB batch end (%s)", + strerror(errno)); + return -1; + } + + return 0; +} + static int vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr, uint64_t iova, size_t len) @@ -136,6 +188,39 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, __rte_unused void *addr, return 0; } +static int +vhost_vdpa_dma_map_batch(struct virtio_user_dev *dev, void *addr, + uint64_t iova, size_t len) +{ + int ret; + + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) + return -1; + + ret = vhost_vdpa_dma_map(dev, addr, iova, len); + + if (vhost_vdpa_iotlb_batch_end(dev) < 0) + return -1; + + return ret; +} + +static int +vhost_vdpa_dma_unmap_batch(struct virtio_user_dev *dev, void *addr, + uint64_t iova, size_t len) +{ + int ret; + + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) + return -1; + + ret = vhost_vdpa_dma_unmap(dev, addr, iova, len); + + if (vhost_vdpa_iotlb_batch_end(dev) < 0) + return -1; + + return ret; +} static int vhost_vdpa_map_contig(const struct rte_memseg_list *msl, @@ -173,21 +258,32 @@ vhost_vdpa_map(const struct rte_memseg_list *msl, const struct rte_memseg *ms, static int vhost_vdpa_dma_map_all(struct virtio_user_dev *dev) { + int ret; + + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) + return -1; + vhost_vdpa_dma_unmap(dev, NULL, 0, SIZE_MAX); if (rte_eal_iova_mode() == RTE_IOVA_VA) { /* with IOVA as VA mode, we can get away with mapping contiguous * chunks rather than going page-by-page. */ - int ret = rte_memseg_contig_walk_thread_unsafe( + ret = rte_memseg_contig_walk_thread_unsafe( vhost_vdpa_map_contig, dev); if (ret) - return ret; + goto batch_end; /* we have to continue the walk because we've skipped the * external segments during the config walk. */ } - return rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev); + ret = rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev); + +batch_end: + if (vhost_vdpa_iotlb_batch_end(dev) < 0) + return -1; + + return ret; } /* with below features, vhost vdpa does not need to do the checksum and TSO, @@ -307,6 +403,6 @@ struct virtio_user_backend_ops virtio_ops_vdpa = { .setup = vhost_vdpa_setup, .send_request = vhost_vdpa_ioctl, .enable_qp = vhost_vdpa_enable_queue_pair, - .dma_map = vhost_vdpa_dma_map, - .dma_unmap = vhost_vdpa_dma_unmap, + .dma_map = vhost_vdpa_dma_map_batch, + .dma_unmap = vhost_vdpa_dma_unmap_batch, }; diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 96bc6b232d..095deeea82 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -445,7 +445,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) 1ULL << VHOST_USER_PROTOCOL_F_STATUS) #define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ - (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | \ + 1ULL << VHOST_BACKEND_F_IOTLB_BATCH) int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, int cq, int queue_size, const char *mac, char **ifname, -- 2.26.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend Maxime Coquelin @ 2020-12-21 6:23 ` Xia, Chenbo 0 siblings, 0 replies; 8+ messages in thread From: Xia, Chenbo @ 2020-12-21 6:23 UTC (permalink / raw) To: Maxime Coquelin, dev, amorenoz, jasowang, david.marchand; +Cc: stable > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, November 25, 2020 11:21 PM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; amorenoz@redhat.com; > jasowang@redhat.com; david.marchand@redhat.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org > Subject: [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend > > This patch fixes an overhead met with mlx5-vdpa Kernel > driver, where for every page in the mapped area, all the > memory tables gets updated. For example, with 2MB hugepages, > a single IOTLB_UPDATE for a 1GB region causes 512 memory > updates on mlx5-vdpa side. > > Using batching mode, the mlx5 driver will only trigger a > single memory update for all the IOTLB updates that happen > between the batch begin and batch end commands. > > Fixes: 6b901437056e ("net/virtio: introduce vhost-vDPA backend") > Cc: stable@dpdk.org > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/net/virtio/virtio_user/vhost.h | 4 + > drivers/net/virtio/virtio_user/vhost_vdpa.c | 106 +++++++++++++++++- > .../net/virtio/virtio_user/virtio_user_dev.c | 3 +- > 3 files changed, 107 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/vhost.h > b/drivers/net/virtio/virtio_user/vhost.h > index c1dcc50b58..be286173b0 100644 > --- a/drivers/net/virtio/virtio_user/vhost.h > +++ b/drivers/net/virtio/virtio_user/vhost.h > @@ -90,6 +90,10 @@ enum vhost_user_request { > #define VHOST_BACKEND_F_IOTLB_MSG_V2 1 > #endif > > +#ifndef VHOST_BACKEND_F_IOTLB_BATCH > +#define VHOST_BACKEND_F_IOTLB_BATCH 2 > +#endif > + > extern const char * const vhost_msg_strings[VHOST_USER_MAX]; > > struct vhost_memory_region { > diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c > b/drivers/net/virtio/virtio_user/vhost_vdpa.c > index b6c81d6f17..269bab2f8e 100644 > --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c > +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c > @@ -70,6 +70,8 @@ struct vhost_iotlb_msg { > #define VHOST_IOTLB_UPDATE 2 > #define VHOST_IOTLB_INVALIDATE 3 > #define VHOST_IOTLB_ACCESS_FAIL 4 > +#define VHOST_IOTLB_BATCH_BEGIN 5 > +#define VHOST_IOTLB_BATCH_END 6 > uint8_t type; > }; > > @@ -84,6 +86,56 @@ struct vhost_msg { > }; > }; > > +static int > +vhost_vdpa_iotlb_batch_begin(struct virtio_user_dev *dev) > +{ > + struct vhost_msg msg = {}; > + > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) > + return 0; > + > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > { > + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > + return -1; > + } > + > + msg.type = VHOST_IOTLB_MSG_V2; > + msg.iotlb.type = VHOST_IOTLB_BATCH_BEGIN; > + > + if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) { > + PMD_DRV_LOG(ERR, "Failed to send IOTLB batch begin (%s)", > + strerror(errno)); > + return -1; > + } > + > + return 0; > +} > + > +static int > +vhost_vdpa_iotlb_batch_end(struct virtio_user_dev *dev) > +{ > + struct vhost_msg msg = {}; > + > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) > + return 0; > + > + if (!(dev->protocol_features & (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2))) > { > + PMD_DRV_LOG(ERR, "IOTLB_MSG_V2 not supported by the backend."); > + return -1; > + } > + > + msg.type = VHOST_IOTLB_MSG_V2; > + msg.iotlb.type = VHOST_IOTLB_BATCH_END; > + > + if (write(dev->vhostfd, &msg, sizeof(msg)) != sizeof(msg)) { > + PMD_DRV_LOG(ERR, "Failed to send IOTLB batch end (%s)", > + strerror(errno)); > + return -1; > + } > + > + return 0; > +} > + > static int > vhost_vdpa_dma_map(struct virtio_user_dev *dev, void *addr, > uint64_t iova, size_t len) > @@ -136,6 +188,39 @@ vhost_vdpa_dma_unmap(struct virtio_user_dev *dev, > __rte_unused void *addr, > return 0; > } > > +static int > +vhost_vdpa_dma_map_batch(struct virtio_user_dev *dev, void *addr, > + uint64_t iova, size_t len) > +{ > + int ret; > + > + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) > + return -1; > + > + ret = vhost_vdpa_dma_map(dev, addr, iova, len); > + > + if (vhost_vdpa_iotlb_batch_end(dev) < 0) > + return -1; > + > + return ret; > +} > + > +static int > +vhost_vdpa_dma_unmap_batch(struct virtio_user_dev *dev, void *addr, > + uint64_t iova, size_t len) > +{ > + int ret; > + > + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) > + return -1; > + > + ret = vhost_vdpa_dma_unmap(dev, addr, iova, len); > + > + if (vhost_vdpa_iotlb_batch_end(dev) < 0) > + return -1; > + > + return ret; > +} > > static int > vhost_vdpa_map_contig(const struct rte_memseg_list *msl, > @@ -173,21 +258,32 @@ vhost_vdpa_map(const struct rte_memseg_list *msl, const > struct rte_memseg *ms, > static int > vhost_vdpa_dma_map_all(struct virtio_user_dev *dev) > { > + int ret; > + > + if (vhost_vdpa_iotlb_batch_begin(dev) < 0) > + return -1; > + > vhost_vdpa_dma_unmap(dev, NULL, 0, SIZE_MAX); > > if (rte_eal_iova_mode() == RTE_IOVA_VA) { > /* with IOVA as VA mode, we can get away with mapping contiguous > * chunks rather than going page-by-page. > */ > - int ret = rte_memseg_contig_walk_thread_unsafe( > + ret = rte_memseg_contig_walk_thread_unsafe( > vhost_vdpa_map_contig, dev); > if (ret) > - return ret; > + goto batch_end; > /* we have to continue the walk because we've skipped the > * external segments during the config walk. > */ > } > - return rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev); > + ret = rte_memseg_walk_thread_unsafe(vhost_vdpa_map, dev); > + > +batch_end: > + if (vhost_vdpa_iotlb_batch_end(dev) < 0) > + return -1; > + > + return ret; > } > > /* with below features, vhost vdpa does not need to do the checksum and TSO, > @@ -307,6 +403,6 @@ struct virtio_user_backend_ops virtio_ops_vdpa = { > .setup = vhost_vdpa_setup, > .send_request = vhost_vdpa_ioctl, > .enable_qp = vhost_vdpa_enable_queue_pair, > - .dma_map = vhost_vdpa_dma_map, > - .dma_unmap = vhost_vdpa_dma_unmap, > + .dma_map = vhost_vdpa_dma_map_batch, > + .dma_unmap = vhost_vdpa_dma_unmap_batch, > }; > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > b/drivers/net/virtio/virtio_user/virtio_user_dev.c > index 96bc6b232d..095deeea82 100644 > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > @@ -445,7 +445,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) > 1ULL << VHOST_USER_PROTOCOL_F_STATUS) > > #define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES \ > - (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) > + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 | \ > + 1ULL << VHOST_BACKEND_F_IOTLB_BATCH) > int > virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, > int cq, int queue_size, const char *mac, char **ifname, > -- > 2.26.2 Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-05 16:54 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-25 15:21 [dpdk-dev] [PATCH 21.02 v2 0/2] Vhost-vDPA fixes Maxime Coquelin 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 1/2] net/virtio: fix missing backend features negotiation Maxime Coquelin 2020-12-21 6:23 ` Xia, Chenbo 2020-12-22 10:55 ` Maxime Coquelin 2020-12-23 5:37 ` Xia, Chenbo 2021-01-05 16:53 ` Maxime Coquelin 2020-11-25 15:21 ` [dpdk-dev] [PATCH 21.02 v2 2/2] net/virtio: fix memory init with vDPA backend Maxime Coquelin 2020-12-21 6:23 ` Xia, Chenbo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).