[v2,2/2] vhost: force vIOMMU enablement with VDUSE

Message ID 20230713112905.233306-3-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vduse: TSO & vIOMMU fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

Maxime Coquelin July 13, 2023, 11:29 a.m. UTC
  Unlike Vhost-user, VDUSE requires vIOMMU support.
This patch ignores whether RTE_VHOST_USER_IOMMU_SUPPORT
flag is passed at register time to avoid having application
to pass it for having working VDUSE device creation.

Fixes: 0adb8eccc6a6 ("vhost: add VDUSE device creation and destruction")

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/socket.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
  

Comments

Kevin Traynor July 13, 2023, 12:47 p.m. UTC | #1
On 13/07/2023 12:29, Maxime Coquelin wrote:
> Unlike Vhost-user, VDUSE requires vIOMMU support.
> This patch ignores whether RTE_VHOST_USER_IOMMU_SUPPORT
> flag is passed at register time to avoid having application
> to pass it for having working VDUSE device creation.
> 
> Fixes: 0adb8eccc6a6 ("vhost: add VDUSE device creation and destruction")
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   lib/vhost/socket.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index 57dfe3d2fe..79f2138f60 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -932,7 +932,10 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>   	vsocket->async_copy = flags & RTE_VHOST_USER_ASYNC_COPY;
>   	vsocket->net_compliant_ol_flags = flags & RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS;
>   	vsocket->stats_enabled = flags & RTE_VHOST_USER_NET_STATS_ENABLE;
> -	vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
> +	if (vsocket->is_vduse)
> +		vsocket->iommu_support = true;
> +	else
> +		vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
>   
>   	if (vsocket->async_copy &&
>   		(flags & (RTE_VHOST_USER_IOMMU_SUPPORT |
                  ^^^ do you need to update this check ?

> @@ -986,7 +989,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>   		vsocket->features &= ~seg_offload_features;
>   	}
>   
> -	if (!(flags & RTE_VHOST_USER_IOMMU_SUPPORT)) {
> +	if (!vsocket->iommu_support) {
>   		vsocket->supported_features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
>   		vsocket->features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
>   	}
  
Maxime Coquelin July 13, 2023, 1:42 p.m. UTC | #2
On 7/13/23 14:47, Kevin Traynor wrote:
> On 13/07/2023 12:29, Maxime Coquelin wrote:
>> Unlike Vhost-user, VDUSE requires vIOMMU support.
>> This patch ignores whether RTE_VHOST_USER_IOMMU_SUPPORT
>> flag is passed at register time to avoid having application
>> to pass it for having working VDUSE device creation.
>>
>> Fixes: 0adb8eccc6a6 ("vhost: add VDUSE device creation and destruction")
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   lib/vhost/socket.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
>> index 57dfe3d2fe..79f2138f60 100644
>> --- a/lib/vhost/socket.c
>> +++ b/lib/vhost/socket.c
>> @@ -932,7 +932,10 @@ rte_vhost_driver_register(const char *path, 
>> uint64_t flags)
>>       vsocket->async_copy = flags & RTE_VHOST_USER_ASYNC_COPY;
>>       vsocket->net_compliant_ol_flags = flags & 
>> RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS;
>>       vsocket->stats_enabled = flags & RTE_VHOST_USER_NET_STATS_ENABLE;
>> -    vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
>> +    if (vsocket->is_vduse)
>> +        vsocket->iommu_support = true;
>> +    else
>> +        vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
>>       if (vsocket->async_copy &&
>>           (flags & (RTE_VHOST_USER_IOMMU_SUPPORT |
>                   ^^^ do you need to update this check ?

Good catch.
Yes, it would be better to replace with vsocket->iommu_support.

V3 on its way.

Thanks!
Maxime

>> @@ -986,7 +989,7 @@ rte_vhost_driver_register(const char *path, 
>> uint64_t flags)
>>           vsocket->features &= ~seg_offload_features;
>>       }
>> -    if (!(flags & RTE_VHOST_USER_IOMMU_SUPPORT)) {
>> +    if (!vsocket->iommu_support) {
>>           vsocket->supported_features &= ~(1ULL << 
>> VIRTIO_F_IOMMU_PLATFORM);
>>           vsocket->features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
>>       }
>
  

Patch

diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
index 57dfe3d2fe..79f2138f60 100644
--- a/lib/vhost/socket.c
+++ b/lib/vhost/socket.c
@@ -932,7 +932,10 @@  rte_vhost_driver_register(const char *path, uint64_t flags)
 	vsocket->async_copy = flags & RTE_VHOST_USER_ASYNC_COPY;
 	vsocket->net_compliant_ol_flags = flags & RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS;
 	vsocket->stats_enabled = flags & RTE_VHOST_USER_NET_STATS_ENABLE;
-	vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
+	if (vsocket->is_vduse)
+		vsocket->iommu_support = true;
+	else
+		vsocket->iommu_support = flags & RTE_VHOST_USER_IOMMU_SUPPORT;
 
 	if (vsocket->async_copy &&
 		(flags & (RTE_VHOST_USER_IOMMU_SUPPORT |
@@ -986,7 +989,7 @@  rte_vhost_driver_register(const char *path, uint64_t flags)
 		vsocket->features &= ~seg_offload_features;
 	}
 
-	if (!(flags & RTE_VHOST_USER_IOMMU_SUPPORT)) {
+	if (!vsocket->iommu_support) {
 		vsocket->supported_features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
 		vsocket->features &= ~(1ULL << VIRTIO_F_IOMMU_PLATFORM);
 	}