* Re: [dpdk-dev] vhost: initializing iotlb memory only when IOMMU feature is enabled.
[not found] ` <CGME20180817140517eucas1p24703d1c0234958581830c117fce93f14@eucas1p2.samsung.com>
@ 2018-08-17 14:06 ` Ilya Maximets
2018-08-23 7:33 ` Nitin Katiyar
0 siblings, 1 reply; 3+ messages in thread
From: Ilya Maximets @ 2018-08-17 14:06 UTC (permalink / raw)
To: Nitin Katiyar, dev; +Cc: Maxime Coquelin, Bie, Tiwei, Zhihong Wang
On 17.08.2018 23:19, Nitin Katiyar wrote:
> DPDK 17.11 introduced the IOMMU feature which caused additional
> DPDK memory requirement per vhostuser device as part of
> iotlb_init(). Today this is done unconditionally (from DPDK
> 17.11 onwards) i.e. irrespective of IOMMU feature being
> enabled on the vhostuser device, iotlb is initialized. This
> breaks the backward compatibility for applications like OVS
> due to increase in the DPDK memory footprint and causes upgrade
> failures.
>> This patch is to do iotlb_init only if IOMMU feature is
> enabled on device.
I guess, "Fixes" line should be here?
>
> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
> ---
> lib/librte_vhost/iotlb.c | 7 +++++++
> lib/librte_vhost/vhost_user.c | 6 ++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/lib/librte_vhost/iotlb.c b/lib/librte_vhost/iotlb.c
> index c6354fe..befa2d3 100644
> --- a/lib/librte_vhost/iotlb.c
> +++ b/lib/librte_vhost/iotlb.c
> @@ -317,6 +317,13 @@ struct vhost_iotlb_entry {
> struct vhost_virtqueue *vq = dev->virtqueue[vq_index];
> int socket = 0;
>
> + if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
vhost_user_iotlb_init() is called on the virtqueue allocation
stage, most likely while processing VHOST_USER_SET_VRING_CALL.
QEMU usually sends VHOST_USER_SET_FEATURES after the
VHOST_USER_SET_VRING_CALL. This means that 'dev->features' are
not yet initialized here.
Have you tested that IOMMU feature works with this patch applied?
> + RTE_LOG(INFO, VHOST_CONFIG,
> + "IOMMU feature is not enabled for this dev(%s)\n",
> + dev->ifname);
> + return 0;
> + }
> +
> if (vq->iotlb_pool) {
> /*
> * The cache has already been initialized,
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index a2d4c9f..7553a03 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1366,6 +1366,12 @@
> uint16_t i;
> uint64_t vva, len;
>
> + if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
> + RTE_LOG(ERR, VHOST_CONFIG,
> + "IOMMU feature is not enabled for this dev(%s)\n",
> + dev->ifname);
> + return -1;
> + }
> switch (imsg->type) {
> case VHOST_IOTLB_UPDATE:
> len = imsg->size;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [dpdk-dev] [PATCH] vhost: initializing iotlb memory only when IOMMU feature is enabled.
@ 2018-08-17 20:19 Nitin Katiyar
[not found] ` <CGME20180817140517eucas1p24703d1c0234958581830c117fce93f14@eucas1p2.samsung.com>
0 siblings, 1 reply; 3+ messages in thread
From: Nitin Katiyar @ 2018-08-17 20:19 UTC (permalink / raw)
To: dev; +Cc: Nitin Katiyar
DPDK 17.11 introduced the IOMMU feature which caused additional
DPDK memory requirement per vhostuser device as part of
iotlb_init(). Today this is done unconditionally (from DPDK
17.11 onwards) i.e. irrespective of IOMMU feature being
enabled on the vhostuser device, iotlb is initialized. This
breaks the backward compatibility for applications like OVS
due to increase in the DPDK memory footprint and causes upgrade
failures.
This patch is to do iotlb_init only if IOMMU feature is
enabled on device.
Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
---
lib/librte_vhost/iotlb.c | 7 +++++++
lib/librte_vhost/vhost_user.c | 6 ++++++
2 files changed, 13 insertions(+)
diff --git a/lib/librte_vhost/iotlb.c b/lib/librte_vhost/iotlb.c
index c6354fe..befa2d3 100644
--- a/lib/librte_vhost/iotlb.c
+++ b/lib/librte_vhost/iotlb.c
@@ -317,6 +317,13 @@ struct vhost_iotlb_entry {
struct vhost_virtqueue *vq = dev->virtqueue[vq_index];
int socket = 0;
+ if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
+ RTE_LOG(INFO, VHOST_CONFIG,
+ "IOMMU feature is not enabled for this dev(%s)\n",
+ dev->ifname);
+ return 0;
+ }
+
if (vq->iotlb_pool) {
/*
* The cache has already been initialized,
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index a2d4c9f..7553a03 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1366,6 +1366,12 @@
uint16_t i;
uint64_t vva, len;
+ if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
+ RTE_LOG(ERR, VHOST_CONFIG,
+ "IOMMU feature is not enabled for this dev(%s)\n",
+ dev->ifname);
+ return -1;
+ }
switch (imsg->type) {
case VHOST_IOTLB_UPDATE:
len = imsg->size;
--
1.9.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] vhost: initializing iotlb memory only when IOMMU feature is enabled.
2018-08-17 14:06 ` [dpdk-dev] " Ilya Maximets
@ 2018-08-23 7:33 ` Nitin Katiyar
0 siblings, 0 replies; 3+ messages in thread
From: Nitin Katiyar @ 2018-08-23 7:33 UTC (permalink / raw)
To: Ilya Maximets, dev; +Cc: Maxime Coquelin, Bie, Tiwei, Zhihong Wang
Hi Ilya,
Thanks for your comments. I haven't tested with IOMMU enabled. I covered more of memory utilization by testing OVS with DPDK. I will test out by enabling IOMMU and send the updated patch.
Regards,
Nitin
-----Original Message-----
From: Ilya Maximets [mailto:i.maximets@samsung.com]
Sent: Friday, August 17, 2018 7:36 PM
To: Nitin Katiyar <nitin.katiyar@ericsson.com>; dev@dpdk.org
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Bie, Tiwei <tiwei.bie@intel.com>; Zhihong Wang <zhihong.wang@intel.com>
Subject: Re: vhost: initializing iotlb memory only when IOMMU feature is enabled.
On 17.08.2018 23:19, Nitin Katiyar wrote:
> DPDK 17.11 introduced the IOMMU feature which caused additional DPDK
> memory requirement per vhostuser device as part of iotlb_init(). Today
> this is done unconditionally (from DPDK
> 17.11 onwards) i.e. irrespective of IOMMU feature being enabled on the
> vhostuser device, iotlb is initialized. This breaks the backward
> compatibility for applications like OVS due to increase in the DPDK
> memory footprint and causes upgrade failures.
>> This patch is to do iotlb_init only if IOMMU feature is
> enabled on device.
I guess, "Fixes" line should be here?
>
> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com>
> ---
> lib/librte_vhost/iotlb.c | 7 +++++++
> lib/librte_vhost/vhost_user.c | 6 ++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/lib/librte_vhost/iotlb.c b/lib/librte_vhost/iotlb.c index
> c6354fe..befa2d3 100644
> --- a/lib/librte_vhost/iotlb.c
> +++ b/lib/librte_vhost/iotlb.c
> @@ -317,6 +317,13 @@ struct vhost_iotlb_entry {
> struct vhost_virtqueue *vq = dev->virtqueue[vq_index];
> int socket = 0;
>
> + if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
vhost_user_iotlb_init() is called on the virtqueue allocation stage, most likely while processing VHOST_USER_SET_VRING_CALL.
QEMU usually sends VHOST_USER_SET_FEATURES after the VHOST_USER_SET_VRING_CALL. This means that 'dev->features' are not yet initialized here.
Have you tested that IOMMU feature works with this patch applied?
> + RTE_LOG(INFO, VHOST_CONFIG,
> + "IOMMU feature is not enabled for this dev(%s)\n",
> + dev->ifname);
> + return 0;
> + }
> +
> if (vq->iotlb_pool) {
> /*
> * The cache has already been initialized, diff --git
> a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index
> a2d4c9f..7553a03 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1366,6 +1366,12 @@
> uint16_t i;
> uint64_t vva, len;
>
> + if (!(dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {
> + RTE_LOG(ERR, VHOST_CONFIG,
> + "IOMMU feature is not enabled for this dev(%s)\n",
> + dev->ifname);
> + return -1;
> + }
> switch (imsg->type) {
> case VHOST_IOTLB_UPDATE:
> len = imsg->size;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-23 7:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-17 20:19 [dpdk-dev] [PATCH] vhost: initializing iotlb memory only when IOMMU feature is enabled Nitin Katiyar
[not found] ` <CGME20180817140517eucas1p24703d1c0234958581830c117fce93f14@eucas1p2.samsung.com>
2018-08-17 14:06 ` [dpdk-dev] " Ilya Maximets
2018-08-23 7:33 ` Nitin Katiyar
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).