[PATCH 1/2] net/virtio: support modern device id

[PATCH 1/2] net/virtio: support modern device id

[Jason Wang]

Spec said "The PCI Device ID is calculated by adding 0x1040 to the
Virtio Device ID". So this patch makes pmd can recognize modern virtio
net id.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 1 +
 drivers/net/virtio/virtio_pci.h    | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 07d6449..f48e037 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -104,6 +104,7 @@ static int virtio_dev_queue_stats_mapping_set(
  */
 static const struct rte_pci_id pci_id_virtio_map[] = {
 	{ RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_DEVICEID_MIN) },
+	{ RTE_PCI_DEVICE(VIRTIO_PCI_VENDORID, VIRTIO_PCI_MODERN_DEVICEID_NET) },
 	{ .vendor_id = 0, /* sentinel */ },
 };
 
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index dd7693f..d3bdfa0 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -46,6 +46,7 @@ struct virtnet_ctl;
 #define VIRTIO_PCI_VENDORID     0x1AF4
 #define VIRTIO_PCI_DEVICEID_MIN 0x1000
 #define VIRTIO_PCI_DEVICEID_MAX 0x103F
+#define VIRTIO_PCI_MODERN_DEVICEID_NET 0x1041
 
 /* VirtIO ABI version, this must match exactly. */
 #define VIRTIO_PCI_ABI_VERSION 0
-- 
2.7.4

[PATCH 2/2] virtio: support IOMMU platform

[Jason Wang]

Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
completely. But recently, the work of making virtio device work with
IOMMU is near to complete. So this patch make pmd support IOMMU by:

- Allow VFIO mapping by setting RTE_PCI_DRV_NEED_MAPPING flag
- Negotiate feature VIRTIO_F_IOMMU_PLATFORM

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 2 +-
 drivers/net/virtio/virtio_ethdev.h | 3 ++-
 drivers/net/virtio/virtio_pci.h    | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index f48e037..0d6d4d2 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1307,7 +1307,7 @@ static struct eth_driver rte_virtio_pmd = {
 	.pci_drv = {
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_DETACHABLE,
+		.drv_flags = RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_NEED_MAPPING,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.eth_dev_uninit = eth_virtio_dev_uninit,
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 d3bdfa0..dd74c0f 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -139,6 +139,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
@@ -146,7 +147,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. */
-- 
2.7.4

Re: [PATCH 1/2] net/virtio: support modern device id

[Thomas Monjalon]

2016-09-02 14:36, Jason Wang:
> Spec said "The PCI Device ID is calculated by adding 0x1040 to the
> Virtio Device ID". So this patch makes pmd can recognize modern virtio
> net id.

Please could you describe what is a modern virtio-net?

>  #define VIRTIO_PCI_DEVICEID_MIN 0x1000
>  #define VIRTIO_PCI_DEVICEID_MAX 0x103F
> +#define VIRTIO_PCI_MODERN_DEVICEID_NET 0x1041

It doesn't match the sentence above: 1000 + 1040 = 2040

By the way, VIRTIO_PCI_DEVICEID_MAX is not used in the code.

Re: [PATCH 2/2] virtio: support IOMMU platform

[Thomas Monjalon]

2016-09-02 14:37, Jason Wang:
> Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
> completely. But recently, the work of making virtio device work with
> IOMMU is near to complete. 

Good news!
What are the requirements for Qemu and Linux version numbers please?

Re: [PATCH 2/2] virtio: support IOMMU platform

[Michael S. Tsirkin]

On Fri, Sep 02, 2016 at 03:04:56PM +0200, Thomas Monjalon wrote:
> 2016-09-02 14:37, Jason Wang:
> > Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
> > completely. But recently, the work of making virtio device work with
> > IOMMU is near to complete. 
> 
> Good news!
> What are the requirements for Qemu and Linux version numbers please?

I expect QEMU 2.8 and Linux 4.8 to have the support.

Re: [PATCH 2/2] virtio: support IOMMU platform

[Alejandro Lucero]

I know RedHat is working on a vIOMMU so I guess this work is related to
that effort, but it is a surprise virtio using IOMMU. I thought IOMMU just
made sense when using SRIOV. My second guess is using IOMMU with virtio is
a matter of security, but by other hand, virtio + IOMMU could imply serious
performance degradation when multiple VMs are in use. I'm talking about
IOMMU contention, exactly about IOTLB contention. This performance issue is
complex to describe or even analyze as there are several factors having an
impact on it. For example, 1GB hugepages can avoid most of it and the same
if TX & RX rings are not bigger than 256. So, my question: is RedHat aware
of this potential IOMMU contention which can limit scalability?

On Fri, Sep 2, 2016 at 6:26 PM, Michael S. Tsirkin <mst@redhat.com> wrote:

> On Fri, Sep 02, 2016 at 03:04:56PM +0200, Thomas Monjalon wrote:
> > 2016-09-02 14:37, Jason Wang:
> > > Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
> > > completely. But recently, the work of making virtio device work with
> > > IOMMU is near to complete.
> >
> > Good news!
> > What are the requirements for Qemu and Linux version numbers please?
>
> I expect QEMU 2.8 and Linux 4.8 to have the support.
>

Re: [PATCH 2/2] virtio: support IOMMU platform

[Jason Wang]

On 2016年09月02日 21:04, Thomas Monjalon wrote:
> 2016-09-02 14:37, Jason Wang:
>> Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
>> completely. But recently, the work of making virtio device work with
>> IOMMU is near to complete.
> Good news!
> What are the requirements for Qemu and Linux version numbers please?

Linux(vhost) support was in 4.8. Qemu support will be ready at 2.8

Thanks

Re: [PATCH 2/2] virtio: support IOMMU platform

[Jason Wang]

On 2016年09月02日 21:04, Thomas Monjalon wrote:
> 2016-09-02 14:37, Jason Wang:
>> Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
>> completely. But recently, the work of making virtio device work with
>> IOMMU is near to complete.
> Good news!
> What are the requirements for Qemu and Linux version numbers please?

Linux 4.8 has support for this. For qemu, it will support this for 2.8.

Thanks

Re: [PATCH 2/2] virtio: support IOMMU platform

[Jason Wang]

On 2016年09月04日 16:08, Alejandro Lucero wrote:
> I know RedHat is working on a vIOMMU so I guess this work is related 
> to that effort, but it is a surprise virtio using IOMMU. I thought 
> IOMMU just made sense when using SRIOV. My second guess is using IOMMU 
> with virtio is a matter of security, but by other hand, virtio + IOMMU 
> could imply serious performance degradation when multiple VMs are in use.

We will use qemu vIOMMU for virito, so there's no such issue.

> I'm talking about IOMMU contention, exactly about IOTLB contention.

I thought device IOTLB (ATS) was just designed to solve this contention.

> This performance issue is complex to describe or even analyze as there 
> are several factors having an impact on it. For example, 1GB hugepages 
> can avoid most of it and the same if TX & RX rings are not bigger than 
> 256. So, my question: is RedHat aware of this potential IOMMU 
> contention which can limit scalability?

For virtio, we use vIOMMU per VM and implement a device IOTLB in vhost 
side. Technically, it does not have such issue I think.

Thanks

>
> On Fri, Sep 2, 2016 at 6:26 PM, Michael S. Tsirkin <mst@redhat.com 
> <mailto:mst@redhat.com>> wrote:
>
>     On Fri, Sep 02, 2016 at 03:04:56PM +0200, Thomas Monjalon wrote:
>     > 2016-09-02 14:37, Jason Wang:
>     > > Virtio pmd doesn't support VFIO in the past since devices
>     bypass IOMMU
>     > > completely. But recently, the work of making virtio device
>     work with
>     > > IOMMU is near to complete.
>     >
>     > Good news!
>     > What are the requirements for Qemu and Linux version numbers please?
>
>     I expect QEMU 2.8 and Linux 4.8 to have the support.
>
>

Re: [PATCH 1/2] net/virtio: support modern device id

[Jason Wang]

On 2016年09月02日 20:57, Thomas Monjalon wrote:
> 2016-09-02 14:36, Jason Wang:
>> Spec said "The PCI Device ID is calculated by adding 0x1040 to the
>> Virtio Device ID". So this patch makes pmd can recognize modern virtio
>> net id.
> Please could you describe what is a modern virtio-net?

I think it means the device that only support virtio 1.0.

>
>>   #define VIRTIO_PCI_DEVICEID_MIN 0x1000
>>   #define VIRTIO_PCI_DEVICEID_MAX 0x103F
>> +#define VIRTIO_PCI_MODERN_DEVICEID_NET 0x1041
> It doesn't match the sentence above: 1000 + 1040 = 2040

According to the spec (5.1.1), Virtio Device id (not pci device id) is 1.

>
> By the way, VIRTIO_PCI_DEVICEID_MAX is not used in the code.
>

Yes, we can remove this.

Thanks

Re: [PATCH 2/2] virtio: support IOMMU platform

[Yuanhan Liu]

Oops, seems I failed to send it out (I happened to know it when
I was checking the patchwork status: I didn't find my comment).

I also happened to find out that I failed to receive quite many
emails, including some patches. Seems something went wrong :(

	--yliu

On Mon, Sep 05, 2016 at 03:16:26PM +0800, Yuanhan Liu wrote:
> On Fri, Sep 02, 2016 at 02:37:00PM +0800, Jason Wang wrote:
> > Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
> > completely. But recently, the work of making virtio device work with
> > IOMMU is near to complete. So this patch make pmd support IOMMU by:
> > 
> > - Allow VFIO mapping by setting RTE_PCI_DRV_NEED_MAPPING flag
> 
> I think that's not needed, since virtio_read_caps() invokes
> rte_eal_pci_map_device().
> 
> 	--yliu

Re: [PATCH 2/2] virtio: support IOMMU platform

[Jason Wang]

On 2016年09月06日 15:46, Yuanhan Liu wrote:
> Oops, seems I failed to send it out (I happened to know it when
> I was checking the patchwork status: I didn't find my comment).
>
> I also happened to find out that I failed to receive quite many
> emails, including some patches. Seems something went wrong :(
>
> 	--yliu
>
> On Mon, Sep 05, 2016 at 03:16:26PM +0800, Yuanhan Liu wrote:
>> On Fri, Sep 02, 2016 at 02:37:00PM +0800, Jason Wang wrote:
>>> Virtio pmd doesn't support VFIO in the past since devices bypass IOMMU
>>> completely. But recently, the work of making virtio device work with
>>> IOMMU is near to complete. So this patch make pmd support IOMMU by:
>>>
>>> - Allow VFIO mapping by setting RTE_PCI_DRV_NEED_MAPPING flag
>> I think that's not needed, since virtio_read_caps() invokes
>> rte_eal_pci_map_device().
>>
>> 	--yliu

Right.

Thanks.