All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] virtio: support VIRTIO_F_IO_BARRIER
@ 2018-05-03  2:59 Tiwei Bie
  2018-05-03  7:30   ` Jason Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  2:59 UTC (permalink / raw)
  To: mst, jasowang, pbonzini, stefanha, virtualization, linux-kernel
  Cc: dan.daly, cunming.liang, zhihong.wang

This patch introduces the support for VIRTIO_F_IO_BARRIER.
When this feature is negotiated, driver will use the barriers
suitable for hardware devices.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/virtio/virtio_ring.c       | 5 +++++
 include/uapi/linux/virtio_config.h | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 21d464a29cf8..edb565643bf4 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
 		!context;
 	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
 
+	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
+		vq->weak_barriers = false;
+
 	/* No callback?  Tell other side not to bother us. */
 	if (!callback) {
 		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
@@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_F_IOMMU_PLATFORM:
 			break;
+		case VIRTIO_F_IO_BARRIER:
+			break;
 		default:
 			/* We don't understand this bit. */
 			__virtio_clear_bit(vdev, i);
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..6ca8d24bf468 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START	28
-#define VIRTIO_TRANSPORT_F_END		34
+#define VIRTIO_TRANSPORT_F_END		38
 
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 /* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,10 @@
  * this is for compatibility with legacy systems.
  */
 #define VIRTIO_F_IOMMU_PLATFORM		33
+
+/*
+ * If clear - driver may use barriers suitable for CPU cores.
+ * If set - driver must use barriers suitable for hardware devices.
+ */
+#define VIRTIO_F_IO_BARRIER		37
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
@ 2018-05-03  7:30   ` Jason Wang
  2018-05-03  9:06 ` Stefan Hajnoczi
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03  7:30 UTC (permalink / raw)
  To: Tiwei Bie, mst, pbonzini, stefanha, virtualization, linux-kernel
  Cc: dan.daly, cunming.liang, zhihong.wang



On 2018年05月03日 10:59, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>   drivers/virtio/virtio_ring.c       | 5 +++++
>   include/uapi/linux/virtio_config.h | 8 +++++++-
>   2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 21d464a29cf8..edb565643bf4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>   		!context;
>   	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>   
> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> +		vq->weak_barriers = false;
> +
>   	/* No callback?  Tell other side not to bother us. */
>   	if (!callback) {
>   		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>   			break;
>   		case VIRTIO_F_IOMMU_PLATFORM:
>   			break;
> +		case VIRTIO_F_IO_BARRIER:
> +			break;
>   		default:
>   			/* We don't understand this bit. */
>   			__virtio_clear_bit(vdev, i);
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..6ca8d24bf468 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
>    * transport being used (eg. virtio_ring), the rest are per-device feature
>    * bits. */
>   #define VIRTIO_TRANSPORT_F_START	28
> -#define VIRTIO_TRANSPORT_F_END		34
> +#define VIRTIO_TRANSPORT_F_END		38
>   
>   #ifndef VIRTIO_CONFIG_NO_LEGACY
>   /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,10 @@
>    * this is for compatibility with legacy systems.
>    */
>   #define VIRTIO_F_IOMMU_PLATFORM		33
> +
> +/*
> + * If clear - driver may use barriers suitable for CPU cores.
> + * If set - driver must use barriers suitable for hardware devices.
> + */
> +#define VIRTIO_F_IO_BARRIER		37
>   #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */

Hi:

I believe this depends on Michael's patch of

"[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"

?

Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
@ 2018-05-03  7:30   ` Jason Wang
  0 siblings, 0 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03  7:30 UTC (permalink / raw)
  To: Tiwei Bie, mst, pbonzini, stefanha, virtualization, linux-kernel
  Cc: zhihong.wang



On 2018年05月03日 10:59, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
>   drivers/virtio/virtio_ring.c       | 5 +++++
>   include/uapi/linux/virtio_config.h | 8 +++++++-
>   2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 21d464a29cf8..edb565643bf4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>   		!context;
>   	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>   
> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> +		vq->weak_barriers = false;
> +
>   	/* No callback?  Tell other side not to bother us. */
>   	if (!callback) {
>   		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>   			break;
>   		case VIRTIO_F_IOMMU_PLATFORM:
>   			break;
> +		case VIRTIO_F_IO_BARRIER:
> +			break;
>   		default:
>   			/* We don't understand this bit. */
>   			__virtio_clear_bit(vdev, i);
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..6ca8d24bf468 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -49,7 +49,7 @@
>    * transport being used (eg. virtio_ring), the rest are per-device feature
>    * bits. */
>   #define VIRTIO_TRANSPORT_F_START	28
> -#define VIRTIO_TRANSPORT_F_END		34
> +#define VIRTIO_TRANSPORT_F_END		38
>   
>   #ifndef VIRTIO_CONFIG_NO_LEGACY
>   /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,10 @@
>    * this is for compatibility with legacy systems.
>    */
>   #define VIRTIO_F_IOMMU_PLATFORM		33
> +
> +/*
> + * If clear - driver may use barriers suitable for CPU cores.
> + * If set - driver must use barriers suitable for hardware devices.
> + */
> +#define VIRTIO_F_IO_BARRIER		37
>   #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */

Hi:

I believe this depends on Michael's patch of

"[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"

?

Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  7:30   ` Jason Wang
  (?)
  (?)
@ 2018-05-03  8:30   ` Tiwei Bie
  2018-05-03  9:09     ` Jason Wang
  2018-05-03  9:09     ` Jason Wang
  -1 siblings, 2 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  8:30 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, pbonzini, stefanha, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang

On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
> On 2018年05月03日 10:59, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> >   drivers/virtio/virtio_ring.c       | 5 +++++
> >   include/uapi/linux/virtio_config.h | 8 +++++++-
> >   2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 21d464a29cf8..edb565643bf4 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> >   		!context;
> >   	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > +		vq->weak_barriers = false;
> > +
> >   	/* No callback?  Tell other side not to bother us. */
> >   	if (!callback) {
> >   		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> >   			break;
> >   		case VIRTIO_F_IOMMU_PLATFORM:
> >   			break;
> > +		case VIRTIO_F_IO_BARRIER:
> > +			break;
> >   		default:
> >   			/* We don't understand this bit. */
> >   			__virtio_clear_bit(vdev, i);
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..6ca8d24bf468 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> >    * transport being used (eg. virtio_ring), the rest are per-device feature
> >    * bits. */
> >   #define VIRTIO_TRANSPORT_F_START	28
> > -#define VIRTIO_TRANSPORT_F_END		34
> > +#define VIRTIO_TRANSPORT_F_END		38
> >   #ifndef VIRTIO_CONFIG_NO_LEGACY
> >   /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,10 @@
> >    * this is for compatibility with legacy systems.
> >    */
> >   #define VIRTIO_F_IOMMU_PLATFORM		33
> > +
> > +/*
> > + * If clear - driver may use barriers suitable for CPU cores.
> > + * If set - driver must use barriers suitable for hardware devices.
> > + */
> > +#define VIRTIO_F_IO_BARRIER		37
> >   #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> 
> Hi:
> 
> I believe this depends on Michael's patch of
> 
> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
> 
> ?
> 
> Thanks

We already have below commit and some other related commits
in the tree:

7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")

They should have already guaranteed that virtio_Xmb() will
be OK for hardware devices when vq->weak_barriers is false.
If my understanding is correct, the barriers used in this
case are overkill. So Michael's patch is to make the barriers
weaker (or better).

Best regards,
Tiwei Bie

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  7:30   ` Jason Wang
  (?)
@ 2018-05-03  8:30   ` Tiwei Bie
  -1 siblings, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  8:30 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini

On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
> On 2018年05月03日 10:59, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > ---
> >   drivers/virtio/virtio_ring.c       | 5 +++++
> >   include/uapi/linux/virtio_config.h | 8 +++++++-
> >   2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 21d464a29cf8..edb565643bf4 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> >   		!context;
> >   	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > +		vq->weak_barriers = false;
> > +
> >   	/* No callback?  Tell other side not to bother us. */
> >   	if (!callback) {
> >   		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> >   			break;
> >   		case VIRTIO_F_IOMMU_PLATFORM:
> >   			break;
> > +		case VIRTIO_F_IO_BARRIER:
> > +			break;
> >   		default:
> >   			/* We don't understand this bit. */
> >   			__virtio_clear_bit(vdev, i);
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..6ca8d24bf468 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -49,7 +49,7 @@
> >    * transport being used (eg. virtio_ring), the rest are per-device feature
> >    * bits. */
> >   #define VIRTIO_TRANSPORT_F_START	28
> > -#define VIRTIO_TRANSPORT_F_END		34
> > +#define VIRTIO_TRANSPORT_F_END		38
> >   #ifndef VIRTIO_CONFIG_NO_LEGACY
> >   /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,10 @@
> >    * this is for compatibility with legacy systems.
> >    */
> >   #define VIRTIO_F_IOMMU_PLATFORM		33
> > +
> > +/*
> > + * If clear - driver may use barriers suitable for CPU cores.
> > + * If set - driver must use barriers suitable for hardware devices.
> > + */
> > +#define VIRTIO_F_IO_BARRIER		37
> >   #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> 
> Hi:
> 
> I believe this depends on Michael's patch of
> 
> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
> 
> ?
> 
> Thanks

We already have below commit and some other related commits
in the tree:

7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")

They should have already guaranteed that virtio_Xmb() will
be OK for hardware devices when vq->weak_barriers is false.
If my understanding is correct, the barriers used in this
case are overkill. So Michael's patch is to make the barriers
weaker (or better).

Best regards,
Tiwei Bie
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
  2018-05-03  7:30   ` Jason Wang
  2018-05-03  9:06 ` Stefan Hajnoczi
@ 2018-05-03  9:06 ` Stefan Hajnoczi
  2018-05-03 13:26     ` Tiwei Bie
  2018-05-03 17:57 ` Michael S. Tsirkin
  2018-05-03 17:57 ` Michael S. Tsirkin
  4 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2018-05-03  9:06 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, jasowang, pbonzini, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

I should have thought of this earlier, but why is a new feature bit
necessary?  If a hardware virtio device is in use, then the device
should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
IOMMU callbacks).

Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
the problem?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
  2018-05-03  7:30   ` Jason Wang
@ 2018-05-03  9:06 ` Stefan Hajnoczi
  2018-05-03  9:06 ` Stefan Hajnoczi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2018-05-03  9:06 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: mst, linux-kernel, virtualization, zhihong.wang, pbonzini


[-- Attachment #1.1: Type: text/plain, Size: 594 bytes --]

On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

I should have thought of this earlier, but why is a new feature bit
necessary?  If a hardware virtio device is in use, then the device
should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
IOMMU callbacks).

Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
the problem?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  8:30   ` Tiwei Bie
  2018-05-03  9:09     ` Jason Wang
@ 2018-05-03  9:09     ` Jason Wang
  2018-05-03  9:58       ` Tiwei Bie
  2018-05-03  9:58       ` Tiwei Bie
  1 sibling, 2 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03  9:09 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, pbonzini, stefanha, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang



On 2018年05月03日 16:30, Tiwei Bie wrote:
> On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
>> On 2018年05月03日 10:59, Tiwei Bie wrote:
>>> This patch introduces the support for VIRTIO_F_IO_BARRIER.
>>> When this feature is negotiated, driver will use the barriers
>>> suitable for hardware devices.
>>>
>>> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>> ---
>>>    drivers/virtio/virtio_ring.c       | 5 +++++
>>>    include/uapi/linux/virtio_config.h | 8 +++++++-
>>>    2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 21d464a29cf8..edb565643bf4 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>>>    		!context;
>>>    	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
>>> +		vq->weak_barriers = false;
>>> +
>>>    	/* No callback?  Tell other side not to bother us. */
>>>    	if (!callback) {
>>>    		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
>>> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>>    			break;
>>>    		case VIRTIO_F_IOMMU_PLATFORM:
>>>    			break;
>>> +		case VIRTIO_F_IO_BARRIER:
>>> +			break;
>>>    		default:
>>>    			/* We don't understand this bit. */
>>>    			__virtio_clear_bit(vdev, i);
>>> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
>>> index 308e2096291f..6ca8d24bf468 100644
>>> --- a/include/uapi/linux/virtio_config.h
>>> +++ b/include/uapi/linux/virtio_config.h
>>> @@ -49,7 +49,7 @@
>>>     * transport being used (eg. virtio_ring), the rest are per-device feature
>>>     * bits. */
>>>    #define VIRTIO_TRANSPORT_F_START	28
>>> -#define VIRTIO_TRANSPORT_F_END		34
>>> +#define VIRTIO_TRANSPORT_F_END		38
>>>    #ifndef VIRTIO_CONFIG_NO_LEGACY
>>>    /* Do we get callbacks when the ring is completely used, even if we've
>>> @@ -71,4 +71,10 @@
>>>     * this is for compatibility with legacy systems.
>>>     */
>>>    #define VIRTIO_F_IOMMU_PLATFORM		33
>>> +
>>> +/*
>>> + * If clear - driver may use barriers suitable for CPU cores.
>>> + * If set - driver must use barriers suitable for hardware devices.
>>> + */
>>> +#define VIRTIO_F_IO_BARRIER		37
>>>    #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
>> Hi:
>>
>> I believe this depends on Michael's patch of
>>
>> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
>>
>> ?
>>
>> Thanks
> We already have below commit and some other related commits
> in the tree:
>
> 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
>
> They should have already guaranteed that virtio_Xmb() will
> be OK for hardware devices when vq->weak_barriers is false.
> If my understanding is correct, the barriers used in this
> case are overkill. So Michael's patch is to make the barriers
> weaker (or better).
>
> Best regards,
> Tiwei Bie

Well, I think we need dma barriers for some platforms according to 
previous discussion? Without Michael's patch, we won't use any dma 
barriers in fact for virtio.

Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  8:30   ` Tiwei Bie
@ 2018-05-03  9:09     ` Jason Wang
  2018-05-03  9:09     ` Jason Wang
  1 sibling, 0 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03  9:09 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini



On 2018年05月03日 16:30, Tiwei Bie wrote:
> On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
>> On 2018年05月03日 10:59, Tiwei Bie wrote:
>>> This patch introduces the support for VIRTIO_F_IO_BARRIER.
>>> When this feature is negotiated, driver will use the barriers
>>> suitable for hardware devices.
>>>
>>> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>> ---
>>>    drivers/virtio/virtio_ring.c       | 5 +++++
>>>    include/uapi/linux/virtio_config.h | 8 +++++++-
>>>    2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index 21d464a29cf8..edb565643bf4 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>>>    		!context;
>>>    	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
>>> +		vq->weak_barriers = false;
>>> +
>>>    	/* No callback?  Tell other side not to bother us. */
>>>    	if (!callback) {
>>>    		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
>>> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>>    			break;
>>>    		case VIRTIO_F_IOMMU_PLATFORM:
>>>    			break;
>>> +		case VIRTIO_F_IO_BARRIER:
>>> +			break;
>>>    		default:
>>>    			/* We don't understand this bit. */
>>>    			__virtio_clear_bit(vdev, i);
>>> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
>>> index 308e2096291f..6ca8d24bf468 100644
>>> --- a/include/uapi/linux/virtio_config.h
>>> +++ b/include/uapi/linux/virtio_config.h
>>> @@ -49,7 +49,7 @@
>>>     * transport being used (eg. virtio_ring), the rest are per-device feature
>>>     * bits. */
>>>    #define VIRTIO_TRANSPORT_F_START	28
>>> -#define VIRTIO_TRANSPORT_F_END		34
>>> +#define VIRTIO_TRANSPORT_F_END		38
>>>    #ifndef VIRTIO_CONFIG_NO_LEGACY
>>>    /* Do we get callbacks when the ring is completely used, even if we've
>>> @@ -71,4 +71,10 @@
>>>     * this is for compatibility with legacy systems.
>>>     */
>>>    #define VIRTIO_F_IOMMU_PLATFORM		33
>>> +
>>> +/*
>>> + * If clear - driver may use barriers suitable for CPU cores.
>>> + * If set - driver must use barriers suitable for hardware devices.
>>> + */
>>> +#define VIRTIO_F_IO_BARRIER		37
>>>    #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
>> Hi:
>>
>> I believe this depends on Michael's patch of
>>
>> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
>>
>> ?
>>
>> Thanks
> We already have below commit and some other related commits
> in the tree:
>
> 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
>
> They should have already guaranteed that virtio_Xmb() will
> be OK for hardware devices when vq->weak_barriers is false.
> If my understanding is correct, the barriers used in this
> case are overkill. So Michael's patch is to make the barriers
> weaker (or better).
>
> Best regards,
> Tiwei Bie

Well, I think we need dma barriers for some platforms according to 
previous discussion? Without Michael's patch, we won't use any dma 
barriers in fact for virtio.

Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  9:09     ` Jason Wang
  2018-05-03  9:58       ` Tiwei Bie
@ 2018-05-03  9:58       ` Tiwei Bie
  2018-05-03 12:57         ` Jason Wang
  2018-05-03 12:57         ` Jason Wang
  1 sibling, 2 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  9:58 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, pbonzini, stefanha, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang

On Thu, May 03, 2018 at 05:09:44PM +0800, Jason Wang wrote:
> On 2018年05月03日 16:30, Tiwei Bie wrote:
> > On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
> > > On 2018年05月03日 10:59, Tiwei Bie wrote:
> > > > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > > > When this feature is negotiated, driver will use the barriers
> > > > suitable for hardware devices.
> > > > 
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > >    drivers/virtio/virtio_ring.c       | 5 +++++
> > > >    include/uapi/linux/virtio_config.h | 8 +++++++-
> > > >    2 files changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 21d464a29cf8..edb565643bf4 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> > > >    		!context;
> > > >    	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > > > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > > > +		vq->weak_barriers = false;
> > > > +
> > > >    	/* No callback?  Tell other side not to bother us. */
> > > >    	if (!callback) {
> > > >    		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> > > > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > > >    			break;
> > > >    		case VIRTIO_F_IOMMU_PLATFORM:
> > > >    			break;
> > > > +		case VIRTIO_F_IO_BARRIER:
> > > > +			break;
> > > >    		default:
> > > >    			/* We don't understand this bit. */
> > > >    			__virtio_clear_bit(vdev, i);
> > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > index 308e2096291f..6ca8d24bf468 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -49,7 +49,7 @@
> > > >     * transport being used (eg. virtio_ring), the rest are per-device feature
> > > >     * bits. */
> > > >    #define VIRTIO_TRANSPORT_F_START	28
> > > > -#define VIRTIO_TRANSPORT_F_END		34
> > > > +#define VIRTIO_TRANSPORT_F_END		38
> > > >    #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > >    /* Do we get callbacks when the ring is completely used, even if we've
> > > > @@ -71,4 +71,10 @@
> > > >     * this is for compatibility with legacy systems.
> > > >     */
> > > >    #define VIRTIO_F_IOMMU_PLATFORM		33
> > > > +
> > > > +/*
> > > > + * If clear - driver may use barriers suitable for CPU cores.
> > > > + * If set - driver must use barriers suitable for hardware devices.
> > > > + */
> > > > +#define VIRTIO_F_IO_BARRIER		37
> > > >    #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > > Hi:
> > > 
> > > I believe this depends on Michael's patch of
> > > 
> > > "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
> > > 
> > > ?
> > > 
> > > Thanks
> > We already have below commit and some other related commits
> > in the tree:
> > 
> > 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
> > 
> > They should have already guaranteed that virtio_Xmb() will
> > be OK for hardware devices when vq->weak_barriers is false.
> > If my understanding is correct, the barriers used in this
> > case are overkill. So Michael's patch is to make the barriers
> > weaker (or better).
> > 
> > Best regards,
> > Tiwei Bie
> 
> Well, I think we need dma barriers for some platforms according to previous
> discussion? Without Michael's patch, we won't use any dma barriers in fact
> for virtio.

You are right. Thanks! Are you suggesting to add a
reference to Michael's patch in this patch to make
sure that it won't be applied before that patch?

Best regards,
Tiwei Bie

> 
> Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  9:09     ` Jason Wang
@ 2018-05-03  9:58       ` Tiwei Bie
  2018-05-03  9:58       ` Tiwei Bie
  1 sibling, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  9:58 UTC (permalink / raw)
  To: Jason Wang
  Cc: mst, linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini

On Thu, May 03, 2018 at 05:09:44PM +0800, Jason Wang wrote:
> On 2018年05月03日 16:30, Tiwei Bie wrote:
> > On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
> > > On 2018年05月03日 10:59, Tiwei Bie wrote:
> > > > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > > > When this feature is negotiated, driver will use the barriers
> > > > suitable for hardware devices.
> > > > 
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > ---
> > > >    drivers/virtio/virtio_ring.c       | 5 +++++
> > > >    include/uapi/linux/virtio_config.h | 8 +++++++-
> > > >    2 files changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 21d464a29cf8..edb565643bf4 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> > > >    		!context;
> > > >    	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > > > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > > > +		vq->weak_barriers = false;
> > > > +
> > > >    	/* No callback?  Tell other side not to bother us. */
> > > >    	if (!callback) {
> > > >    		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> > > > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > > >    			break;
> > > >    		case VIRTIO_F_IOMMU_PLATFORM:
> > > >    			break;
> > > > +		case VIRTIO_F_IO_BARRIER:
> > > > +			break;
> > > >    		default:
> > > >    			/* We don't understand this bit. */
> > > >    			__virtio_clear_bit(vdev, i);
> > > > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > > > index 308e2096291f..6ca8d24bf468 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -49,7 +49,7 @@
> > > >     * transport being used (eg. virtio_ring), the rest are per-device feature
> > > >     * bits. */
> > > >    #define VIRTIO_TRANSPORT_F_START	28
> > > > -#define VIRTIO_TRANSPORT_F_END		34
> > > > +#define VIRTIO_TRANSPORT_F_END		38
> > > >    #ifndef VIRTIO_CONFIG_NO_LEGACY
> > > >    /* Do we get callbacks when the ring is completely used, even if we've
> > > > @@ -71,4 +71,10 @@
> > > >     * this is for compatibility with legacy systems.
> > > >     */
> > > >    #define VIRTIO_F_IOMMU_PLATFORM		33
> > > > +
> > > > +/*
> > > > + * If clear - driver may use barriers suitable for CPU cores.
> > > > + * If set - driver must use barriers suitable for hardware devices.
> > > > + */
> > > > +#define VIRTIO_F_IO_BARRIER		37
> > > >    #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> > > Hi:
> > > 
> > > I believe this depends on Michael's patch of
> > > 
> > > "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
> > > 
> > > ?
> > > 
> > > Thanks
> > We already have below commit and some other related commits
> > in the tree:
> > 
> > 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
> > 
> > They should have already guaranteed that virtio_Xmb() will
> > be OK for hardware devices when vq->weak_barriers is false.
> > If my understanding is correct, the barriers used in this
> > case are overkill. So Michael's patch is to make the barriers
> > weaker (or better).
> > 
> > Best regards,
> > Tiwei Bie
> 
> Well, I think we need dma barriers for some platforms according to previous
> discussion? Without Michael's patch, we won't use any dma barriers in fact
> for virtio.

You are right. Thanks! Are you suggesting to add a
reference to Michael's patch in this patch to make
sure that it won't be applied before that patch?

Best regards,
Tiwei Bie

> 
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  9:58       ` Tiwei Bie
@ 2018-05-03 12:57         ` Jason Wang
  2018-05-03 12:57         ` Jason Wang
  1 sibling, 0 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03 12:57 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, pbonzini, stefanha, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang



On 2018年05月03日 17:58, Tiwei Bie wrote:
> On Thu, May 03, 2018 at 05:09:44PM +0800, Jason Wang wrote:
>> On 2018年05月03日 16:30, Tiwei Bie wrote:
>>> On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
>>>> On 2018年05月03日 10:59, Tiwei Bie wrote:
>>>>> This patch introduces the support for VIRTIO_F_IO_BARRIER.
>>>>> When this feature is negotiated, driver will use the barriers
>>>>> suitable for hardware devices.
>>>>>
>>>>> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>>>> ---
>>>>>     drivers/virtio/virtio_ring.c       | 5 +++++
>>>>>     include/uapi/linux/virtio_config.h | 8 +++++++-
>>>>>     2 files changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>>> index 21d464a29cf8..edb565643bf4 100644
>>>>> --- a/drivers/virtio/virtio_ring.c
>>>>> +++ b/drivers/virtio/virtio_ring.c
>>>>> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>>>>>     		!context;
>>>>>     	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>>>> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
>>>>> +		vq->weak_barriers = false;
>>>>> +
>>>>>     	/* No callback?  Tell other side not to bother us. */
>>>>>     	if (!callback) {
>>>>>     		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
>>>>> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>>>>     			break;
>>>>>     		case VIRTIO_F_IOMMU_PLATFORM:
>>>>>     			break;
>>>>> +		case VIRTIO_F_IO_BARRIER:
>>>>> +			break;
>>>>>     		default:
>>>>>     			/* We don't understand this bit. */
>>>>>     			__virtio_clear_bit(vdev, i);
>>>>> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
>>>>> index 308e2096291f..6ca8d24bf468 100644
>>>>> --- a/include/uapi/linux/virtio_config.h
>>>>> +++ b/include/uapi/linux/virtio_config.h
>>>>> @@ -49,7 +49,7 @@
>>>>>      * transport being used (eg. virtio_ring), the rest are per-device feature
>>>>>      * bits. */
>>>>>     #define VIRTIO_TRANSPORT_F_START	28
>>>>> -#define VIRTIO_TRANSPORT_F_END		34
>>>>> +#define VIRTIO_TRANSPORT_F_END		38
>>>>>     #ifndef VIRTIO_CONFIG_NO_LEGACY
>>>>>     /* Do we get callbacks when the ring is completely used, even if we've
>>>>> @@ -71,4 +71,10 @@
>>>>>      * this is for compatibility with legacy systems.
>>>>>      */
>>>>>     #define VIRTIO_F_IOMMU_PLATFORM		33
>>>>> +
>>>>> +/*
>>>>> + * If clear - driver may use barriers suitable for CPU cores.
>>>>> + * If set - driver must use barriers suitable for hardware devices.
>>>>> + */
>>>>> +#define VIRTIO_F_IO_BARRIER		37
>>>>>     #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
>>>> Hi:
>>>>
>>>> I believe this depends on Michael's patch of
>>>>
>>>> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
>>>>
>>>> ?
>>>>
>>>> Thanks
>>> We already have below commit and some other related commits
>>> in the tree:
>>>
>>> 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
>>>
>>> They should have already guaranteed that virtio_Xmb() will
>>> be OK for hardware devices when vq->weak_barriers is false.
>>> If my understanding is correct, the barriers used in this
>>> case are overkill. So Michael's patch is to make the barriers
>>> weaker (or better).
>>>
>>> Best regards,
>>> Tiwei Bie
>> Well, I think we need dma barriers for some platforms according to previous
>> discussion? Without Michael's patch, we won't use any dma barriers in fact
>> for virtio.
> You are right. Thanks! Are you suggesting to add a
> reference to Michael's patch in this patch to make
> sure that it won't be applied before that patch?

Yes, better to have some notes, or maybe just squash that patch into 
this series.

Let's wait for Michael's reply to see.

Thanks

>
> Best regards,
> Tiwei Bie
>
>> Thanks

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  9:58       ` Tiwei Bie
  2018-05-03 12:57         ` Jason Wang
@ 2018-05-03 12:57         ` Jason Wang
  1 sibling, 0 replies; 22+ messages in thread
From: Jason Wang @ 2018-05-03 12:57 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: mst, linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini



On 2018年05月03日 17:58, Tiwei Bie wrote:
> On Thu, May 03, 2018 at 05:09:44PM +0800, Jason Wang wrote:
>> On 2018年05月03日 16:30, Tiwei Bie wrote:
>>> On Thu, May 03, 2018 at 03:30:03PM +0800, Jason Wang wrote:
>>>> On 2018年05月03日 10:59, Tiwei Bie wrote:
>>>>> This patch introduces the support for VIRTIO_F_IO_BARRIER.
>>>>> When this feature is negotiated, driver will use the barriers
>>>>> suitable for hardware devices.
>>>>>
>>>>> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>>>> ---
>>>>>     drivers/virtio/virtio_ring.c       | 5 +++++
>>>>>     include/uapi/linux/virtio_config.h | 8 +++++++-
>>>>>     2 files changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>>> index 21d464a29cf8..edb565643bf4 100644
>>>>> --- a/drivers/virtio/virtio_ring.c
>>>>> +++ b/drivers/virtio/virtio_ring.c
>>>>> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>>>>>     		!context;
>>>>>     	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>>>> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
>>>>> +		vq->weak_barriers = false;
>>>>> +
>>>>>     	/* No callback?  Tell other side not to bother us. */
>>>>>     	if (!callback) {
>>>>>     		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
>>>>> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>>>>     			break;
>>>>>     		case VIRTIO_F_IOMMU_PLATFORM:
>>>>>     			break;
>>>>> +		case VIRTIO_F_IO_BARRIER:
>>>>> +			break;
>>>>>     		default:
>>>>>     			/* We don't understand this bit. */
>>>>>     			__virtio_clear_bit(vdev, i);
>>>>> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
>>>>> index 308e2096291f..6ca8d24bf468 100644
>>>>> --- a/include/uapi/linux/virtio_config.h
>>>>> +++ b/include/uapi/linux/virtio_config.h
>>>>> @@ -49,7 +49,7 @@
>>>>>      * transport being used (eg. virtio_ring), the rest are per-device feature
>>>>>      * bits. */
>>>>>     #define VIRTIO_TRANSPORT_F_START	28
>>>>> -#define VIRTIO_TRANSPORT_F_END		34
>>>>> +#define VIRTIO_TRANSPORT_F_END		38
>>>>>     #ifndef VIRTIO_CONFIG_NO_LEGACY
>>>>>     /* Do we get callbacks when the ring is completely used, even if we've
>>>>> @@ -71,4 +71,10 @@
>>>>>      * this is for compatibility with legacy systems.
>>>>>      */
>>>>>     #define VIRTIO_F_IOMMU_PLATFORM		33
>>>>> +
>>>>> +/*
>>>>> + * If clear - driver may use barriers suitable for CPU cores.
>>>>> + * If set - driver must use barriers suitable for hardware devices.
>>>>> + */
>>>>> +#define VIRTIO_F_IO_BARRIER		37
>>>>>     #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
>>>> Hi:
>>>>
>>>> I believe this depends on Michael's patch of
>>>>
>>>> "[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"
>>>>
>>>> ?
>>>>
>>>> Thanks
>>> We already have below commit and some other related commits
>>> in the tree:
>>>
>>> 7b21e34fd1c2 ("virtio: harsher barriers for rpmsg.")
>>>
>>> They should have already guaranteed that virtio_Xmb() will
>>> be OK for hardware devices when vq->weak_barriers is false.
>>> If my understanding is correct, the barriers used in this
>>> case are overkill. So Michael's patch is to make the barriers
>>> weaker (or better).
>>>
>>> Best regards,
>>> Tiwei Bie
>> Well, I think we need dma barriers for some platforms according to previous
>> discussion? Without Michael's patch, we won't use any dma barriers in fact
>> for virtio.
> You are right. Thanks! Are you suggesting to add a
> reference to Michael's patch in this patch to make
> sure that it won't be applied before that patch?

Yes, better to have some notes, or maybe just squash that patch into 
this series.

Let's wait for Michael's reply to see.

Thanks

>
> Best regards,
> Tiwei Bie
>
>> Thanks

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  9:06 ` Stefan Hajnoczi
@ 2018-05-03 13:26     ` Tiwei Bie
  0 siblings, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03 13:26 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: mst, jasowang, pbonzini, virtualization, linux-kernel, dan.daly,
	cunming.liang, zhihong.wang

On Thu, May 03, 2018 at 10:06:52AM +0100, Stefan Hajnoczi wrote:
> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> 
> I should have thought of this earlier, but why is a new feature bit
> necessary?  If a hardware virtio device is in use, then the device
> should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
> IOMMU callbacks).
> 
> Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
> the problem?

The VIRTIO_F_IOMMU_PLATFORM feature can be set when the
device is implemented in software. And I think we don't
want the performance drop in this case.

Best regards,
Tiwei Bie

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
@ 2018-05-03 13:26     ` Tiwei Bie
  0 siblings, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03 13:26 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: mst, linux-kernel, virtualization, zhihong.wang, pbonzini

On Thu, May 03, 2018 at 10:06:52AM +0100, Stefan Hajnoczi wrote:
> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> 
> I should have thought of this earlier, but why is a new feature bit
> necessary?  If a hardware virtio device is in use, then the device
> should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
> IOMMU callbacks).
> 
> Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
> the problem?

The VIRTIO_F_IOMMU_PLATFORM feature can be set when the
device is implemented in software. And I think we don't
want the performance drop in this case.

Best regards,
Tiwei Bie

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03 13:26     ` Tiwei Bie
  (?)
@ 2018-05-03 13:50     ` Stefan Hajnoczi
  -1 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2018-05-03 13:50 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, linux-kernel,
	Linux Virtualization, Wang, Zhihong, Paolo Bonzini

On Thu, May 3, 2018 at 2:26 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> On Thu, May 03, 2018 at 10:06:52AM +0100, Stefan Hajnoczi wrote:
>> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
>> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
>> > When this feature is negotiated, driver will use the barriers
>> > suitable for hardware devices.
>> >
>> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>
>> I should have thought of this earlier, but why is a new feature bit
>> necessary?  If a hardware virtio device is in use, then the device
>> should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
>> IOMMU callbacks).
>>
>> Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
>> the problem?
>
> The VIRTIO_F_IOMMU_PLATFORM feature can be set when the
> device is implemented in software. And I think we don't
> want the performance drop in this case.

Good point.

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03 13:26     ` Tiwei Bie
  (?)
  (?)
@ 2018-05-03 13:50     ` Stefan Hajnoczi
  -1 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2018-05-03 13:50 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: Michael S. Tsirkin, linux-kernel, Linux Virtualization,
	Stefan Hajnoczi, Wang, Zhihong, Paolo Bonzini

On Thu, May 3, 2018 at 2:26 PM, Tiwei Bie <tiwei.bie@intel.com> wrote:
> On Thu, May 03, 2018 at 10:06:52AM +0100, Stefan Hajnoczi wrote:
>> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
>> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
>> > When this feature is negotiated, driver will use the barriers
>> > suitable for hardware devices.
>> >
>> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>>
>> I should have thought of this earlier, but why is a new feature bit
>> necessary?  If a hardware virtio device is in use, then the device
>> should already negotiate VIRTIO_F_IOMMU_PLATFORM (i.e. use DMA APIs and
>> IOMMU callbacks).
>>
>> Does disabling weak_barriers when VIRTIO_F_IOMMU_PLATFORM is set solve
>> the problem?
>
> The VIRTIO_F_IOMMU_PLATFORM feature can be set when the
> device is implemented in software. And I think we don't
> want the performance drop in this case.

Good point.

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
                   ` (2 preceding siblings ...)
  2018-05-03  9:06 ` Stefan Hajnoczi
@ 2018-05-03 17:57 ` Michael S. Tsirkin
  2018-05-04  1:14   ` Tiwei Bie
  2018-05-04  1:14   ` Tiwei Bie
  2018-05-03 17:57 ` Michael S. Tsirkin
  4 siblings, 2 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2018-05-03 17:57 UTC (permalink / raw)
  To: Tiwei Bie
  Cc: jasowang, pbonzini, stefanha, virtualization, linux-kernel,
	dan.daly, cunming.liang, zhihong.wang

On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks!

> ---
>  drivers/virtio/virtio_ring.c       | 5 +++++
>  include/uapi/linux/virtio_config.h | 8 +++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 21d464a29cf8..edb565643bf4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>  		!context;
>  	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>  
> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> +		vq->weak_barriers = false;
> +
>  	/* No callback?  Tell other side not to bother us. */
>  	if (!callback) {
>  		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;

One issue worth looking at is that at least on Intel strong barriers are
actually typically overkill.  We should probably switch weak_barriers ==
false case over to dma barriers.

> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>  			break;
>  		case VIRTIO_F_IOMMU_PLATFORM:
>  			break;
> +		case VIRTIO_F_IO_BARRIER:
> +			break;
>  		default:
>  			/* We don't understand this bit. */
>  			__virtio_clear_bit(vdev, i);
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..6ca8d24bf468 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h

Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists
(subscriber-only, sorry about that).

> @@ -49,7 +49,7 @@
>   * transport being used (eg. virtio_ring), the rest are per-device feature
>   * bits. */
>  #define VIRTIO_TRANSPORT_F_START	28
> -#define VIRTIO_TRANSPORT_F_END		34
> +#define VIRTIO_TRANSPORT_F_END		38
>  
>  #ifndef VIRTIO_CONFIG_NO_LEGACY
>  /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,10 @@
>   * this is for compatibility with legacy systems.
>   */
>  #define VIRTIO_F_IOMMU_PLATFORM		33
> +
> +/*
> + * If clear - driver may use barriers suitable for CPU cores.
> + * If set - driver must use barriers suitable for hardware devices.
> + */
> +#define VIRTIO_F_IO_BARRIER		37
>  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */

Why 37? I'd use 34 I think.

> -- 
> 2.11.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
                   ` (3 preceding siblings ...)
  2018-05-03 17:57 ` Michael S. Tsirkin
@ 2018-05-03 17:57 ` Michael S. Tsirkin
  4 siblings, 0 replies; 22+ messages in thread
From: Michael S. Tsirkin @ 2018-05-03 17:57 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini

On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_IO_BARRIER.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
> 
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks!

> ---
>  drivers/virtio/virtio_ring.c       | 5 +++++
>  include/uapi/linux/virtio_config.h | 8 +++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 21d464a29cf8..edb565643bf4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
>  		!context;
>  	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>  
> +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> +		vq->weak_barriers = false;
> +
>  	/* No callback?  Tell other side not to bother us. */
>  	if (!callback) {
>  		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;

One issue worth looking at is that at least on Intel strong barriers are
actually typically overkill.  We should probably switch weak_barriers ==
false case over to dma barriers.

> @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
>  			break;
>  		case VIRTIO_F_IOMMU_PLATFORM:
>  			break;
> +		case VIRTIO_F_IO_BARRIER:
> +			break;
>  		default:
>  			/* We don't understand this bit. */
>  			__virtio_clear_bit(vdev, i);
> diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> index 308e2096291f..6ca8d24bf468 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h

Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists
(subscriber-only, sorry about that).

> @@ -49,7 +49,7 @@
>   * transport being used (eg. virtio_ring), the rest are per-device feature
>   * bits. */
>  #define VIRTIO_TRANSPORT_F_START	28
> -#define VIRTIO_TRANSPORT_F_END		34
> +#define VIRTIO_TRANSPORT_F_END		38
>  
>  #ifndef VIRTIO_CONFIG_NO_LEGACY
>  /* Do we get callbacks when the ring is completely used, even if we've
> @@ -71,4 +71,10 @@
>   * this is for compatibility with legacy systems.
>   */
>  #define VIRTIO_F_IOMMU_PLATFORM		33
> +
> +/*
> + * If clear - driver may use barriers suitable for CPU cores.
> + * If set - driver must use barriers suitable for hardware devices.
> + */
> +#define VIRTIO_F_IO_BARRIER		37
>  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */

Why 37? I'd use 34 I think.

> -- 
> 2.11.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03 17:57 ` Michael S. Tsirkin
  2018-05-04  1:14   ` Tiwei Bie
@ 2018-05-04  1:14   ` Tiwei Bie
  1 sibling, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-04  1:14 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: jasowang, pbonzini, stefanha, virtualization, linux-kernel,
	dan.daly, cunming.liang, zhihong.wang

On Thu, May 03, 2018 at 08:57:20PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> 
> Thanks!
> 
> > ---
> >  drivers/virtio/virtio_ring.c       | 5 +++++
> >  include/uapi/linux/virtio_config.h | 8 +++++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 21d464a29cf8..edb565643bf4 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> >  		!context;
> >  	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> >  
> > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > +		vq->weak_barriers = false;
> > +
> >  	/* No callback?  Tell other side not to bother us. */
> >  	if (!callback) {
> >  		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> 
> One issue worth looking at is that at least on Intel strong barriers are
> actually typically overkill.  We should probably switch weak_barriers ==
> false case over to dma barriers.

Jason suggested me to add a reference or some notes in this
patch about your patch:

"[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"

> 
> > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> >  			break;
> >  		case VIRTIO_F_IOMMU_PLATFORM:
> >  			break;
> > +		case VIRTIO_F_IO_BARRIER:
> > +			break;
> >  		default:
> >  			/* We don't understand this bit. */
> >  			__virtio_clear_bit(vdev, i);
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..6ca8d24bf468 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> 
> Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists
> (subscriber-only, sorry about that).

Got it! I'll send a new version and Cc virtio-dev.

> 
> > @@ -49,7 +49,7 @@
> >   * transport being used (eg. virtio_ring), the rest are per-device feature
> >   * bits. */
> >  #define VIRTIO_TRANSPORT_F_START	28
> > -#define VIRTIO_TRANSPORT_F_END		34
> > +#define VIRTIO_TRANSPORT_F_END		38
> >  
> >  #ifndef VIRTIO_CONFIG_NO_LEGACY
> >  /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,10 @@
> >   * this is for compatibility with legacy systems.
> >   */
> >  #define VIRTIO_F_IOMMU_PLATFORM		33
> > +
> > +/*
> > + * If clear - driver may use barriers suitable for CPU cores.
> > + * If set - driver must use barriers suitable for hardware devices.
> > + */
> > +#define VIRTIO_F_IO_BARRIER		37
> >  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> 
> Why 37? I'd use 34 I think.

In the latest virtio spec draft, 34 and 35 have been taken
by VIRTIO_F_RING_PACKED and VIRTIO_F_IN_ORDER. And 36 had
been taken by VIRTIO_F_NOTIFICATION_DATA previously when I
sent below proposal:

https://lists.oasis-open.org/archives/virtio-dev/201804/msg00310.html

But I just noticed that NOTIFICATION_DATA has been reverted
from the repo, which means 36 is the next available bit. So
I'll use it. Thanks for the reminder!

Best regards,
Tiwei Bie

> 
> > -- 
> > 2.11.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC] virtio: support VIRTIO_F_IO_BARRIER
  2018-05-03 17:57 ` Michael S. Tsirkin
@ 2018-05-04  1:14   ` Tiwei Bie
  2018-05-04  1:14   ` Tiwei Bie
  1 sibling, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-04  1:14 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, virtualization, stefanha, zhihong.wang, pbonzini

On Thu, May 03, 2018 at 08:57:20PM +0300, Michael S. Tsirkin wrote:
> On Thu, May 03, 2018 at 10:59:55AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_IO_BARRIER.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> > 
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> 
> Thanks!
> 
> > ---
> >  drivers/virtio/virtio_ring.c       | 5 +++++
> >  include/uapi/linux/virtio_config.h | 8 +++++++-
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 21d464a29cf8..edb565643bf4 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
> >  		!context;
> >  	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> >  
> > +	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
> > +		vq->weak_barriers = false;
> > +
> >  	/* No callback?  Tell other side not to bother us. */
> >  	if (!callback) {
> >  		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> 
> One issue worth looking at is that at least on Intel strong barriers are
> actually typically overkill.  We should probably switch weak_barriers ==
> false case over to dma barriers.

Jason suggested me to add a reference or some notes in this
patch about your patch:

"[PATCH] virtio_ring: switch to dma_XX barriers for rpmsg"

> 
> > @@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
> >  			break;
> >  		case VIRTIO_F_IOMMU_PLATFORM:
> >  			break;
> > +		case VIRTIO_F_IO_BARRIER:
> > +			break;
> >  		default:
> >  			/* We don't understand this bit. */
> >  			__virtio_clear_bit(vdev, i);
> > diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
> > index 308e2096291f..6ca8d24bf468 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> 
> Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists
> (subscriber-only, sorry about that).

Got it! I'll send a new version and Cc virtio-dev.

> 
> > @@ -49,7 +49,7 @@
> >   * transport being used (eg. virtio_ring), the rest are per-device feature
> >   * bits. */
> >  #define VIRTIO_TRANSPORT_F_START	28
> > -#define VIRTIO_TRANSPORT_F_END		34
> > +#define VIRTIO_TRANSPORT_F_END		38
> >  
> >  #ifndef VIRTIO_CONFIG_NO_LEGACY
> >  /* Do we get callbacks when the ring is completely used, even if we've
> > @@ -71,4 +71,10 @@
> >   * this is for compatibility with legacy systems.
> >   */
> >  #define VIRTIO_F_IOMMU_PLATFORM		33
> > +
> > +/*
> > + * If clear - driver may use barriers suitable for CPU cores.
> > + * If set - driver must use barriers suitable for hardware devices.
> > + */
> > +#define VIRTIO_F_IO_BARRIER		37
> >  #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
> 
> Why 37? I'd use 34 I think.

In the latest virtio spec draft, 34 and 35 have been taken
by VIRTIO_F_RING_PACKED and VIRTIO_F_IN_ORDER. And 36 had
been taken by VIRTIO_F_NOTIFICATION_DATA previously when I
sent below proposal:

https://lists.oasis-open.org/archives/virtio-dev/201804/msg00310.html

But I just noticed that NOTIFICATION_DATA has been reverted
from the repo, which means 36 is the next available bit. So
I'll use it. Thanks for the reminder!

Best regards,
Tiwei Bie

> 
> > -- 
> > 2.11.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC] virtio: support VIRTIO_F_IO_BARRIER
@ 2018-05-03  2:59 Tiwei Bie
  0 siblings, 0 replies; 22+ messages in thread
From: Tiwei Bie @ 2018-05-03  2:59 UTC (permalink / raw)
  To: mst, jasowang, pbonzini, stefanha, virtualization, linux-kernel
  Cc: zhihong.wang

This patch introduces the support for VIRTIO_F_IO_BARRIER.
When this feature is negotiated, driver will use the barriers
suitable for hardware devices.

Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
 drivers/virtio/virtio_ring.c       | 5 +++++
 include/uapi/linux/virtio_config.h | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 21d464a29cf8..edb565643bf4 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index,
 		!context;
 	vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
 
+	if (virtio_has_feature(vdev, VIRTIO_F_IO_BARRIER))
+		vq->weak_barriers = false;
+
 	/* No callback?  Tell other side not to bother us. */
 	if (!callback) {
 		vq->avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
@@ -1164,6 +1167,8 @@ void vring_transport_features(struct virtio_device *vdev)
 			break;
 		case VIRTIO_F_IOMMU_PLATFORM:
 			break;
+		case VIRTIO_F_IO_BARRIER:
+			break;
 		default:
 			/* We don't understand this bit. */
 			__virtio_clear_bit(vdev, i);
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 308e2096291f..6ca8d24bf468 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
  * transport being used (eg. virtio_ring), the rest are per-device feature
  * bits. */
 #define VIRTIO_TRANSPORT_F_START	28
-#define VIRTIO_TRANSPORT_F_END		34
+#define VIRTIO_TRANSPORT_F_END		38
 
 #ifndef VIRTIO_CONFIG_NO_LEGACY
 /* Do we get callbacks when the ring is completely used, even if we've
@@ -71,4 +71,10 @@
  * this is for compatibility with legacy systems.
  */
 #define VIRTIO_F_IOMMU_PLATFORM		33
+
+/*
+ * If clear - driver may use barriers suitable for CPU cores.
+ * If set - driver must use barriers suitable for hardware devices.
+ */
+#define VIRTIO_F_IO_BARRIER		37
 #endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2018-05-04  1:14 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-03  2:59 [RFC] virtio: support VIRTIO_F_IO_BARRIER Tiwei Bie
2018-05-03  7:30 ` Jason Wang
2018-05-03  7:30   ` Jason Wang
2018-05-03  8:30   ` Tiwei Bie
2018-05-03  8:30   ` Tiwei Bie
2018-05-03  9:09     ` Jason Wang
2018-05-03  9:09     ` Jason Wang
2018-05-03  9:58       ` Tiwei Bie
2018-05-03  9:58       ` Tiwei Bie
2018-05-03 12:57         ` Jason Wang
2018-05-03 12:57         ` Jason Wang
2018-05-03  9:06 ` Stefan Hajnoczi
2018-05-03  9:06 ` Stefan Hajnoczi
2018-05-03 13:26   ` Tiwei Bie
2018-05-03 13:26     ` Tiwei Bie
2018-05-03 13:50     ` Stefan Hajnoczi
2018-05-03 13:50     ` Stefan Hajnoczi
2018-05-03 17:57 ` Michael S. Tsirkin
2018-05-04  1:14   ` Tiwei Bie
2018-05-04  1:14   ` Tiwei Bie
2018-05-03 17:57 ` Michael S. Tsirkin
2018-05-03  2:59 Tiwei Bie

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.