diff mbox

[dpdk-dev,V2,2/2] virtio: support IOMMU platform

Message ID	1475051112-5108-2-git-send-email-jasowang@redhat.com (mailing list archive)
State	Accepted, archived
Delegated to:	Yuanhan Liu
Headers	From: Jason Wang <jasowang@redhat.com> To: dev@dpdk.org Cc: huawei.xie@intel.com, yuanhan.liu@linux.intel.com, mst@redhat.com, vkaplans@redhat.com, Jason Wang <jasowang@redhat.com> Date: Wed, 28 Sep 2016 16:25:12 +0800 Message-Id: <1475051112-5108-2-git-send-email-jasowang@redhat.com> In-Reply-To: <1475051112-5108-1-git-send-email-jasowang@redhat.com> References: <1475051112-5108-1-git-send-email-jasowang@redhat.com> Subject: [dpdk-dev] [PATCH V2 2/2] virtio: support IOMMU platform Precedence: list Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org>

Commit Message

Jason Wang Sept. 28, 2016, 8:25 a.m. UTC

  Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from v1:
- remove unnecessary NEED_MAPPING flag
---
 drivers/net/virtio/virtio_ethdev.h | 3 ++-
 drivers/net/virtio/virtio_pci.h    | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Maxime Coquelin Sept. 29, 2016, 12:39 p.m. UTC | #1

On 09/28/2016 10:25 AM, Jason Wang wrote:
> Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from v1:
> - remove unnecessary NEED_MAPPING flag
> ---
>  drivers/net/virtio/virtio_ethdev.h | 3 ++-
>  drivers/net/virtio/virtio_pci.h    | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Michael S. Tsirkin Oct. 7, 2016, 4:24 a.m. UTC | #2

On Wed, Sep 28, 2016 at 04:25:12PM +0800, Jason Wang wrote:
> Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> Changes from v1:
> - remove unnecessary NEED_MAPPING flag

One thing we probably should do is enable this flag
with VFIO but not with UIO or VFIO-noiommu.

> ---
>  drivers/net/virtio/virtio_ethdev.h | 3 ++-
>  drivers/net/virtio/virtio_pci.h    | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index 2ecec6e..04a06e2 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -63,7 +63,8 @@
>  	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
>  	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
>  	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
> -	 1ULL << VIRTIO_F_VERSION_1)
> +	 1ULL << VIRTIO_F_VERSION_1       |	\
> +	 1ULL << VIRTIO_F_IOMMU_PLATFORM )

Space before ) looks kind of ugly.

>  
>  /*
>   * CQ function prototype
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 3430a39..0aa0015 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -138,6 +138,7 @@ struct virtnet_ctl;
>  #define VIRTIO_RING_F_INDIRECT_DESC	28
>  
>  #define VIRTIO_F_VERSION_1		32
> +#define VIRTIO_F_IOMMU_PLATFORM	33
>  
>  /*
>   * Some VirtIO feature bits (currently bits 28 through 31) are
> @@ -145,7 +146,7 @@ struct virtnet_ctl;
>   * rest are per-device feature bits.
>   */
>  #define VIRTIO_TRANSPORT_F_START 28
> -#define VIRTIO_TRANSPORT_F_END   32
> +#define VIRTIO_TRANSPORT_F_END   34
>  

This seems unused. Drop it?

>  /* The Guest publishes the used index for which it expects an interrupt
>   * at the end of the avail ring. Host should ignore the avail->flags field. */
> -- 
> 2.7.4

Yuanhan Liu Oct. 11, 2016, 4:14 a.m. UTC | #3

On Fri, Oct 07, 2016 at 07:24:44AM +0300, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2016 at 04:25:12PM +0800, Jason Wang wrote:
> > Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.
> > 
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > ---
> > Changes from v1:
> > - remove unnecessary NEED_MAPPING flag
> 
> One thing we probably should do is enable this flag
> with VFIO but not with UIO or VFIO-noiommu.

Good suggestion. I think we could do that in another patch.

> > ---
> >  drivers/net/virtio/virtio_ethdev.h | 3 ++-
> >  drivers/net/virtio/virtio_pci.h    | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> > index 2ecec6e..04a06e2 100644
> > --- a/drivers/net/virtio/virtio_ethdev.h
> > +++ b/drivers/net/virtio/virtio_ethdev.h
> > @@ -63,7 +63,8 @@
> >  	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
> >  	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
> >  	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
> > -	 1ULL << VIRTIO_F_VERSION_1)
> > +	 1ULL << VIRTIO_F_VERSION_1       |	\
> > +	 1ULL << VIRTIO_F_IOMMU_PLATFORM )
> 
> Space before ) looks kind of ugly.

Yes, a bit. I will remove it while apply.

> >  #define VIRTIO_TRANSPORT_F_START 28
> > -#define VIRTIO_TRANSPORT_F_END   32
> > +#define VIRTIO_TRANSPORT_F_END   34
> >  
> 
> This seems unused. Drop it?

Indeed. I will submit a patch to remove both (_START and _END).

	--yliu

Jason Wang Oct. 20, 2016, 2:29 a.m. UTC | #4

On 2016年10月07日 12:24, Michael S. Tsirkin wrote:
> On Wed, Sep 28, 2016 at 04:25:12PM +0800, Jason Wang wrote:
>> Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Changes from v1:
>> - remove unnecessary NEED_MAPPING flag
> One thing we probably should do is enable this flag
> with VFIO but not with UIO or VFIO-noiommu.

Sounds good, will try do it in next version.

>> ---
>>   drivers/net/virtio/virtio_ethdev.h | 3 ++-
>>   drivers/net/virtio/virtio_pci.h    | 3 ++-
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
>> index 2ecec6e..04a06e2 100644
>> --- a/drivers/net/virtio/virtio_ethdev.h
>> +++ b/drivers/net/virtio/virtio_ethdev.h
>> @@ -63,7 +63,8 @@
>>   	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
>>   	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
>>   	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
>> -	 1ULL << VIRTIO_F_VERSION_1)
>> +	 1ULL << VIRTIO_F_VERSION_1       |	\
>> +	 1ULL << VIRTIO_F_IOMMU_PLATFORM )
> Space before ) looks kind of ugly.

Will fix this.

>
>>   
>>   /*
>>    * CQ function prototype
>> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>> index 3430a39..0aa0015 100644
>> --- a/drivers/net/virtio/virtio_pci.h
>> +++ b/drivers/net/virtio/virtio_pci.h
>> @@ -138,6 +138,7 @@ struct virtnet_ctl;
>>   #define VIRTIO_RING_F_INDIRECT_DESC	28
>>   
>>   #define VIRTIO_F_VERSION_1		32
>> +#define VIRTIO_F_IOMMU_PLATFORM	33
>>   
>>   /*
>>    * Some VirtIO feature bits (currently bits 28 through 31) are
>> @@ -145,7 +146,7 @@ struct virtnet_ctl;
>>    * rest are per-device feature bits.
>>    */
>>   #define VIRTIO_TRANSPORT_F_START 28
>> -#define VIRTIO_TRANSPORT_F_END   32
>> +#define VIRTIO_TRANSPORT_F_END   34
>>   
> This seems unused. Drop it?

Ok.

>
>>   /* The Guest publishes the used index for which it expects an interrupt
>>    * at the end of the avail ring. Host should ignore the avail->flags field. */
>> -- 
>> 2.7.4

Jason Wang Oct. 20, 2016, 4:07 a.m. UTC | #5

On 2016年10月11日 12:14, Yuanhan Liu wrote:
> On Fri, Oct 07, 2016 at 07:24:44AM +0300, Michael S. Tsirkin wrote:
>> On Wed, Sep 28, 2016 at 04:25:12PM +0800, Jason Wang wrote:
>>> Negotiate VIRTIO_F_IOMMU_PLATFORM to have IOMMU support.
>>>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>> Changes from v1:
>>> - remove unnecessary NEED_MAPPING flag
>> One thing we probably should do is enable this flag
>> with VFIO but not with UIO or VFIO-noiommu.
> Good suggestion. I think we could do that in another patch.

Yes.

>
>>> ---
>>>   drivers/net/virtio/virtio_ethdev.h | 3 ++-
>>>   drivers/net/virtio/virtio_pci.h    | 3 ++-
>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
>>> index 2ecec6e..04a06e2 100644
>>> --- a/drivers/net/virtio/virtio_ethdev.h
>>> +++ b/drivers/net/virtio/virtio_ethdev.h
>>> @@ -63,7 +63,8 @@
>>>   	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
>>>   	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
>>>   	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
>>> -	 1ULL << VIRTIO_F_VERSION_1)
>>> +	 1ULL << VIRTIO_F_VERSION_1       |	\
>>> +	 1ULL << VIRTIO_F_IOMMU_PLATFORM )
>> Space before ) looks kind of ugly.
> Yes, a bit. I will remove it while apply.
>
>>>   #define VIRTIO_TRANSPORT_F_START 28
>>> -#define VIRTIO_TRANSPORT_F_END   32
>>> +#define VIRTIO_TRANSPORT_F_END   34
>>>   
>> This seems unused. Drop it?
> Indeed. I will submit a patch to remove both (_START and _END).
>
> 	--yliu

Thanks

diff mbox

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 2ecec6e..04a06e2 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -63,7 +63,8 @@ 
 	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
 	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
-	 1ULL << VIRTIO_F_VERSION_1)
+	 1ULL << VIRTIO_F_VERSION_1       |	\
+	 1ULL << VIRTIO_F_IOMMU_PLATFORM )
 
 /*
  * CQ function prototype
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 3430a39..0aa0015 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -138,6 +138,7 @@  struct virtnet_ctl;
 #define VIRTIO_RING_F_INDIRECT_DESC	28
 
 #define VIRTIO_F_VERSION_1		32
+#define VIRTIO_F_IOMMU_PLATFORM	33
 
 /*
  * Some VirtIO feature bits (currently bits 28 through 31) are
@@ -145,7 +146,7 @@  struct virtnet_ctl;
  * rest are per-device feature bits.
  */
 #define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END   32
+#define VIRTIO_TRANSPORT_F_END   34
 
 /* The Guest publishes the used index for which it expects an interrupt
  * at the end of the avail ring. Host should ignore the avail->flags field. */