* [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-22 17:03 ` Tiwei Bie
0 siblings, 0 replies; 20+ messages in thread
From: Tiwei Bie @ 2019-01-22 17:03 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, virtio-dev
This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
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 | 8 ++++++++
include/uapi/linux/virtio_config.h | 6 ++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..27d3f057493e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
!context;
vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
+ if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
+ vq->weak_barriers = false;
+
vq->packed.ring_dma_addr = ring_dma_addr;
vq->packed.driver_event_dma_addr = driver_event_dma_addr;
vq->packed.device_event_dma_addr = device_event_dma_addr;
@@ -2079,6 +2082,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_ORDER_PLATFORM))
+ vq->weak_barriers = false;
+
vq->split.queue_dma_addr = 0;
vq->split.queue_size_in_bytes = 0;
@@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_F_RING_PACKED:
break;
+ case VIRTIO_F_ORDER_PLATFORM:
+ 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -78,6 +78,12 @@
/* This feature indicates support for the packed virtqueue layout. */
#define VIRTIO_F_RING_PACKED 34
+/*
+ * This feature indicates that memory accesses by the driver and the
+ * device are ordered in a way described by the platform.
+ */
+#define VIRTIO_F_ORDER_PLATFORM 36
+
/*
* Does the device support Single Root I/O Virtualization?
*/
--
2.17.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-22 17:03 ` Tiwei Bie
(?)
@ 2019-01-23 3:08 ` Jason Wang
-1 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 3:08 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, virtio-dev
On 2019/1/23 上午1:03, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> 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 | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
I wonder whether or not this is sufficient. Is dma barrier implies a
mmio barrier? Looks not.
See ia64/include/asm/barrier.h:
* Note: "mb()" and its variants cannot be used as a fence to order
* accesses to memory mapped I/O registers. For that, mf.a needs to
* be used. However, we don't want to always use mf.a because (a)
* it's (presumably) much slower than mf and (b) mf.a is supported for
* sequential memory pages only.
*/
#define mb() ia64_mf()
#define rmb() mb()
#define wmb() mb()
#define dma_rmb() mb()
=>efine dma_wmb() mb()
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-22 17:03 ` Tiwei Bie
@ 2019-01-23 3:08 ` Jason Wang
-1 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 3:08 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, virtio-dev
On 2019/1/23 上午1:03, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> 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 | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
I wonder whether or not this is sufficient. Is dma barrier implies a
mmio barrier? Looks not.
See ia64/include/asm/barrier.h:
* Note: "mb()" and its variants cannot be used as a fence to order
* accesses to memory mapped I/O registers. For that, mf.a needs to
* be used. However, we don't want to always use mf.a because (a)
* it's (presumably) much slower than mf and (b) mf.a is supported for
* sequential memory pages only.
*/
#define mb() ia64_mf()
#define rmb() mb()
#define wmb() mb()
#define dma_rmb() mb()
=>efine dma_wmb() mb()
Thanks
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 3:08 ` Jason Wang
0 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 3:08 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, virtio-dev
On 2019/1/23 上午1:03, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> 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 | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
I wonder whether or not this is sufficient. Is dma barrier implies a
mmio barrier? Looks not.
See ia64/include/asm/barrier.h:
* Note: "mb()" and its variants cannot be used as a fence to order
* accesses to memory mapped I/O registers. For that, mf.a needs to
* be used. However, we don't want to always use mf.a because (a)
* it's (presumably) much slower than mf and (b) mf.a is supported for
* sequential memory pages only.
*/
#define mb() ia64_mf()
#define rmb() mb()
#define wmb() mb()
#define dma_rmb() mb()
=>efine dma_wmb() mb()
Thanks
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 3:08 ` Jason Wang
@ 2019-01-23 3:49 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 3:49 UTC (permalink / raw)
To: Jason Wang; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > 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 | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
>
>
> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> barrier? Looks not.
IIUC we don't need an mmio barrier because we are using a
serializing API: Documentation/memory-barriers.txt says:
Note that, when using writel(), a prior
wmb() is not needed to guarantee that the cache coherent memory writes
have completed before writing to the MMIO region.
> See ia64/include/asm/barrier.h:
>
> * Note: "mb()" and its variants cannot be used as a fence to order
> * accesses to memory mapped I/O registers. For that, mf.a needs to
> * be used. However, we don't want to always use mf.a because (a)
> * it's (presumably) much slower than mf and (b) mf.a is supported for
> * sequential memory pages only.
> */
> #define mb() ia64_mf()
> #define rmb() mb()
> #define wmb() mb()
>
> #define dma_rmb() mb()
> =>efine dma_wmb() mb()
>
> Thanks
Frankly no idea about ia64. Sorry. Are any less esoteric platforms
affected?
--
MST
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 3:49 ` Michael S. Tsirkin
0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 3:49 UTC (permalink / raw)
To: Jason Wang; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > 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 | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
>
>
> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> barrier? Looks not.
IIUC we don't need an mmio barrier because we are using a
serializing API: Documentation/memory-barriers.txt says:
Note that, when using writel(), a prior
wmb() is not needed to guarantee that the cache coherent memory writes
have completed before writing to the MMIO region.
> See ia64/include/asm/barrier.h:
>
> * Note: "mb()" and its variants cannot be used as a fence to order
> * accesses to memory mapped I/O registers. For that, mf.a needs to
> * be used. However, we don't want to always use mf.a because (a)
> * it's (presumably) much slower than mf and (b) mf.a is supported for
> * sequential memory pages only.
> */
> #define mb() ia64_mf()
> #define rmb() mb()
> #define wmb() mb()
>
> #define dma_rmb() mb()
> =>efine dma_wmb() mb()
>
> Thanks
Frankly no idea about ia64. Sorry. Are any less esoteric platforms
affected?
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 3:49 ` Michael S. Tsirkin
@ 2019-01-23 6:56 ` Jason Wang
-1 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 6:56 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>> On 2019/1/23 上午1:03, Tiwei Bie wrote:
>>> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
>>> 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 | 8 ++++++++
>>> include/uapi/linux/virtio_config.h | 6 ++++++
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index cd7e755484e3..27d3f057493e 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
>>> !context;
>>> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->packed.ring_dma_addr = ring_dma_addr;
>>> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
>>> vq->packed.device_event_dma_addr = device_event_dma_addr;
>>> @@ -2079,6 +2082,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_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->split.queue_dma_addr = 0;
>>> vq->split.queue_size_in_bytes = 0;
>>> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>> break;
>>> case VIRTIO_F_RING_PACKED:
>>> break;
>>> + case VIRTIO_F_ORDER_PLATFORM:
>>> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
>>> --- a/include/uapi/linux/virtio_config.h
>>> +++ b/include/uapi/linux/virtio_config.h
>>> @@ -78,6 +78,12 @@
>>> /* This feature indicates support for the packed virtqueue layout. */
>>> #define VIRTIO_F_RING_PACKED 34
>>> +/*
>>> + * This feature indicates that memory accesses by the driver and the
>>> + * device are ordered in a way described by the platform.
>>> + */
>>> +#define VIRTIO_F_ORDER_PLATFORM 36
>>> +
>>> /*
>>> * Does the device support Single Root I/O Virtualization?
>>> */
>>
>> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
>> barrier? Looks not.
> IIUC we don't need an mmio barrier because we are using a
> serializing API: Documentation/memory-barriers.txt says:
>
> Note that, when using writel(), a prior
> wmb() is not needed to guarantee that the cache coherent memory writes
> have completed before writing to the MMIO region.
Ah, I get this.
>
>
>> See ia64/include/asm/barrier.h:
>>
>> * Note: "mb()" and its variants cannot be used as a fence to order
>> * accesses to memory mapped I/O registers. For that, mf.a needs to
>> * be used. However, we don't want to always use mf.a because (a)
>> * it's (presumably) much slower than mf and (b) mf.a is supported for
>> * sequential memory pages only.
>> */
>> #define mb() ia64_mf()
>> #define rmb() mb()
>> #define wmb() mb()
>>
>> #define dma_rmb() mb()
>> =>efine dma_wmb() mb()
>>
>> Thanks
> Frankly no idea about ia64.
Neither did me.
> Sorry. Are any less esoteric platforms
> affected?
>
E.g ppc64?
define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
:"memo\
ry")
/*
* Enforce synchronisation of stores vs. spin_unlock
* (this does it explicitly, though our implementation of spin_unlock
* does it implicitely too)
*/
static inline void mmiowb(void)
{
unsigned long tmp;
__asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
: "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
: "memory");
}
dma_wmb() is lwsync which is more lightweight than sync I guess?
Thanks
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 6:56 ` Jason Wang
0 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 6:56 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>> On 2019/1/23 上午1:03, Tiwei Bie wrote:
>>> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
>>> 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 | 8 ++++++++
>>> include/uapi/linux/virtio_config.h | 6 ++++++
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index cd7e755484e3..27d3f057493e 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
>>> !context;
>>> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->packed.ring_dma_addr = ring_dma_addr;
>>> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
>>> vq->packed.device_event_dma_addr = device_event_dma_addr;
>>> @@ -2079,6 +2082,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_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->split.queue_dma_addr = 0;
>>> vq->split.queue_size_in_bytes = 0;
>>> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>> break;
>>> case VIRTIO_F_RING_PACKED:
>>> break;
>>> + case VIRTIO_F_ORDER_PLATFORM:
>>> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
>>> --- a/include/uapi/linux/virtio_config.h
>>> +++ b/include/uapi/linux/virtio_config.h
>>> @@ -78,6 +78,12 @@
>>> /* This feature indicates support for the packed virtqueue layout. */
>>> #define VIRTIO_F_RING_PACKED 34
>>> +/*
>>> + * This feature indicates that memory accesses by the driver and the
>>> + * device are ordered in a way described by the platform.
>>> + */
>>> +#define VIRTIO_F_ORDER_PLATFORM 36
>>> +
>>> /*
>>> * Does the device support Single Root I/O Virtualization?
>>> */
>>
>> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
>> barrier? Looks not.
> IIUC we don't need an mmio barrier because we are using a
> serializing API: Documentation/memory-barriers.txt says:
>
> Note that, when using writel(), a prior
> wmb() is not needed to guarantee that the cache coherent memory writes
> have completed before writing to the MMIO region.
Ah, I get this.
>
>
>> See ia64/include/asm/barrier.h:
>>
>> * Note: "mb()" and its variants cannot be used as a fence to order
>> * accesses to memory mapped I/O registers. For that, mf.a needs to
>> * be used. However, we don't want to always use mf.a because (a)
>> * it's (presumably) much slower than mf and (b) mf.a is supported for
>> * sequential memory pages only.
>> */
>> #define mb() ia64_mf()
>> #define rmb() mb()
>> #define wmb() mb()
>>
>> #define dma_rmb() mb()
>> =>efine dma_wmb() mb()
>>
>> Thanks
> Frankly no idea about ia64.
Neither did me.
> Sorry. Are any less esoteric platforms
> affected?
>
E.g ppc64?
define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
:"memo\
ry")
/*
* Enforce synchronisation of stores vs. spin_unlock
* (this does it explicitly, though our implementation of spin_unlock
* does it implicitely too)
*/
static inline void mmiowb(void)
{
unsigned long tmp;
__asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
: "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
: "memory");
}
dma_wmb() is lwsync which is more lightweight than sync I guess?
Thanks
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 6:56 ` Jason Wang
@ 2019-01-23 13:49 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 13:49 UTC (permalink / raw)
To: Jason Wang; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 02:56:00PM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> > On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
> > > On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > > > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > > > 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 | 8 ++++++++
> > > > include/uapi/linux/virtio_config.h | 6 ++++++
> > > > 2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd7e755484e3..27d3f057493e 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > > > !context;
> > > > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > > > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->packed.ring_dma_addr = ring_dma_addr;
> > > > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > > > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > > > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->split.queue_dma_addr = 0;
> > > > vq->split.queue_size_in_bytes = 0;
> > > > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > > > break;
> > > > case VIRTIO_F_RING_PACKED:
> > > > break;
> > > > + case VIRTIO_F_ORDER_PLATFORM:
> > > > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -78,6 +78,12 @@
> > > > /* This feature indicates support for the packed virtqueue layout. */
> > > > #define VIRTIO_F_RING_PACKED 34
> > > > +/*
> > > > + * This feature indicates that memory accesses by the driver and the
> > > > + * device are ordered in a way described by the platform.
> > > > + */
> > > > +#define VIRTIO_F_ORDER_PLATFORM 36
> > > > +
> > > > /*
> > > > * Does the device support Single Root I/O Virtualization?
> > > > */
> > >
> > > I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> > > barrier? Looks not.
> > IIUC we don't need an mmio barrier because we are using a
> > serializing API: Documentation/memory-barriers.txt says:
> >
> > Note that, when using writel(), a prior
> > wmb() is not needed to guarantee that the cache coherent memory writes
> > have completed before writing to the MMIO region.
>
>
> Ah, I get this.
>
>
> >
> >
> > > See ia64/include/asm/barrier.h:
> > >
> > > * Note: "mb()" and its variants cannot be used as a fence to order
> > > * accesses to memory mapped I/O registers. For that, mf.a needs to
> > > * be used. However, we don't want to always use mf.a because (a)
> > > * it's (presumably) much slower than mf and (b) mf.a is supported for
> > > * sequential memory pages only.
> > > */
> > > #define mb() ia64_mf()
> > > #define rmb() mb()
> > > #define wmb() mb()
> > >
> > > #define dma_rmb() mb()
> > > =>efine dma_wmb() mb()
> > >
> > > Thanks
> > Frankly no idea about ia64.
>
>
> Neither did me.
>
>
> > Sorry. Are any less esoteric platforms
> > affected?
> >
>
> E.g ppc64?
So
void iowrite32(u32 val, void __iomem *addr)
{
writel(val, addr);
}
and that eventually gets to this one:
#define DEF_MMIO_OUT_D(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
: "=m" (*addr) : "r" (val) : "memory"); \
IO_SET_SYNC_FLAG(); \
}
and
#ifdef CONFIG_PPC64
#define IO_SET_SYNC_FLAG() do { local_paca->io_sync = 1; } while(0)
#else
#define IO_SET_SYNC_FLAG()
#endif
> define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
> :"memo\
> ry")
>
> /*
> * Enforce synchronisation of stores vs. spin_unlock
> * (this does it explicitly, though our implementation of spin_unlock
I don't know which spin_unlock does it refer to here.
> * does it implicitely too)
> */
> static inline void mmiowb(void)
> {
> unsigned long tmp;
>
> __asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
> : "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
> : "memory");
> }
So sync+set io_sync here and sync+io_sync above.
> dma_wmb() is lwsync which is more lightweight than sync I guess?
>
> Thanks
>
Sounds about right.
--
MST
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 13:49 ` Michael S. Tsirkin
0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 13:49 UTC (permalink / raw)
To: Jason Wang; +Cc: Tiwei Bie, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 02:56:00PM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> > On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
> > > On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > > > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > > > 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 | 8 ++++++++
> > > > include/uapi/linux/virtio_config.h | 6 ++++++
> > > > 2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd7e755484e3..27d3f057493e 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > > > !context;
> > > > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > > > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->packed.ring_dma_addr = ring_dma_addr;
> > > > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > > > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > > > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->split.queue_dma_addr = 0;
> > > > vq->split.queue_size_in_bytes = 0;
> > > > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > > > break;
> > > > case VIRTIO_F_RING_PACKED:
> > > > break;
> > > > + case VIRTIO_F_ORDER_PLATFORM:
> > > > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -78,6 +78,12 @@
> > > > /* This feature indicates support for the packed virtqueue layout. */
> > > > #define VIRTIO_F_RING_PACKED 34
> > > > +/*
> > > > + * This feature indicates that memory accesses by the driver and the
> > > > + * device are ordered in a way described by the platform.
> > > > + */
> > > > +#define VIRTIO_F_ORDER_PLATFORM 36
> > > > +
> > > > /*
> > > > * Does the device support Single Root I/O Virtualization?
> > > > */
> > >
> > > I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> > > barrier? Looks not.
> > IIUC we don't need an mmio barrier because we are using a
> > serializing API: Documentation/memory-barriers.txt says:
> >
> > Note that, when using writel(), a prior
> > wmb() is not needed to guarantee that the cache coherent memory writes
> > have completed before writing to the MMIO region.
>
>
> Ah, I get this.
>
>
> >
> >
> > > See ia64/include/asm/barrier.h:
> > >
> > > * Note: "mb()" and its variants cannot be used as a fence to order
> > > * accesses to memory mapped I/O registers. For that, mf.a needs to
> > > * be used. However, we don't want to always use mf.a because (a)
> > > * it's (presumably) much slower than mf and (b) mf.a is supported for
> > > * sequential memory pages only.
> > > */
> > > #define mb() ia64_mf()
> > > #define rmb() mb()
> > > #define wmb() mb()
> > >
> > > #define dma_rmb() mb()
> > > =>efine dma_wmb() mb()
> > >
> > > Thanks
> > Frankly no idea about ia64.
>
>
> Neither did me.
>
>
> > Sorry. Are any less esoteric platforms
> > affected?
> >
>
> E.g ppc64?
So
void iowrite32(u32 val, void __iomem *addr)
{
writel(val, addr);
}
and that eventually gets to this one:
#define DEF_MMIO_OUT_D(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
: "=m" (*addr) : "r" (val) : "memory"); \
IO_SET_SYNC_FLAG(); \
}
and
#ifdef CONFIG_PPC64
#define IO_SET_SYNC_FLAG() do { local_paca->io_sync = 1; } while(0)
#else
#define IO_SET_SYNC_FLAG()
#endif
> define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
> :"memo\
> ry")
>
> /*
> * Enforce synchronisation of stores vs. spin_unlock
> * (this does it explicitly, though our implementation of spin_unlock
I don't know which spin_unlock does it refer to here.
> * does it implicitely too)
> */
> static inline void mmiowb(void)
> {
> unsigned long tmp;
>
> __asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
> : "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
> : "memory");
> }
So sync+set io_sync here and sync+io_sync above.
> dma_wmb() is lwsync which is more lightweight than sync I guess?
>
> Thanks
>
Sounds about right.
--
MST
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 6:56 ` Jason Wang
(?)
(?)
@ 2019-01-23 13:49 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 13:49 UTC (permalink / raw)
To: Jason Wang; +Cc: virtio-dev, linux-kernel, virtualization
On Wed, Jan 23, 2019 at 02:56:00PM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> > On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
> > > On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > > > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > > > 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 | 8 ++++++++
> > > > include/uapi/linux/virtio_config.h | 6 ++++++
> > > > 2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd7e755484e3..27d3f057493e 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > > > !context;
> > > > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > > > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->packed.ring_dma_addr = ring_dma_addr;
> > > > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > > > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > > > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > > > + vq->weak_barriers = false;
> > > > +
> > > > vq->split.queue_dma_addr = 0;
> > > > vq->split.queue_size_in_bytes = 0;
> > > > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > > > break;
> > > > case VIRTIO_F_RING_PACKED:
> > > > break;
> > > > + case VIRTIO_F_ORDER_PLATFORM:
> > > > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > > > --- a/include/uapi/linux/virtio_config.h
> > > > +++ b/include/uapi/linux/virtio_config.h
> > > > @@ -78,6 +78,12 @@
> > > > /* This feature indicates support for the packed virtqueue layout. */
> > > > #define VIRTIO_F_RING_PACKED 34
> > > > +/*
> > > > + * This feature indicates that memory accesses by the driver and the
> > > > + * device are ordered in a way described by the platform.
> > > > + */
> > > > +#define VIRTIO_F_ORDER_PLATFORM 36
> > > > +
> > > > /*
> > > > * Does the device support Single Root I/O Virtualization?
> > > > */
> > >
> > > I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> > > barrier? Looks not.
> > IIUC we don't need an mmio barrier because we are using a
> > serializing API: Documentation/memory-barriers.txt says:
> >
> > Note that, when using writel(), a prior
> > wmb() is not needed to guarantee that the cache coherent memory writes
> > have completed before writing to the MMIO region.
>
>
> Ah, I get this.
>
>
> >
> >
> > > See ia64/include/asm/barrier.h:
> > >
> > > * Note: "mb()" and its variants cannot be used as a fence to order
> > > * accesses to memory mapped I/O registers. For that, mf.a needs to
> > > * be used. However, we don't want to always use mf.a because (a)
> > > * it's (presumably) much slower than mf and (b) mf.a is supported for
> > > * sequential memory pages only.
> > > */
> > > #define mb() ia64_mf()
> > > #define rmb() mb()
> > > #define wmb() mb()
> > >
> > > #define dma_rmb() mb()
> > > =>efine dma_wmb() mb()
> > >
> > > Thanks
> > Frankly no idea about ia64.
>
>
> Neither did me.
>
>
> > Sorry. Are any less esoteric platforms
> > affected?
> >
>
> E.g ppc64?
So
void iowrite32(u32 val, void __iomem *addr)
{
writel(val, addr);
}
and that eventually gets to this one:
#define DEF_MMIO_OUT_D(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
: "=m" (*addr) : "r" (val) : "memory"); \
IO_SET_SYNC_FLAG(); \
}
and
#ifdef CONFIG_PPC64
#define IO_SET_SYNC_FLAG() do { local_paca->io_sync = 1; } while(0)
#else
#define IO_SET_SYNC_FLAG()
#endif
> define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
> :"memo\
> ry")
>
> /*
> * Enforce synchronisation of stores vs. spin_unlock
> * (this does it explicitly, though our implementation of spin_unlock
I don't know which spin_unlock does it refer to here.
> * does it implicitely too)
> */
> static inline void mmiowb(void)
> {
> unsigned long tmp;
>
> __asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
> : "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
> : "memory");
> }
So sync+set io_sync here and sync+io_sync above.
> dma_wmb() is lwsync which is more lightweight than sync I guess?
>
> Thanks
>
Sounds about right.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 3:49 ` Michael S. Tsirkin
(?)
(?)
@ 2019-01-23 6:56 ` Jason Wang
-1 siblings, 0 replies; 20+ messages in thread
From: Jason Wang @ 2019-01-23 6:56 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-dev, linux-kernel, virtualization
On 2019/1/23 上午11:49, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>> On 2019/1/23 上午1:03, Tiwei Bie wrote:
>>> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
>>> 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 | 8 ++++++++
>>> include/uapi/linux/virtio_config.h | 6 ++++++
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index cd7e755484e3..27d3f057493e 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
>>> !context;
>>> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>>> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->packed.ring_dma_addr = ring_dma_addr;
>>> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
>>> vq->packed.device_event_dma_addr = device_event_dma_addr;
>>> @@ -2079,6 +2082,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_ORDER_PLATFORM))
>>> + vq->weak_barriers = false;
>>> +
>>> vq->split.queue_dma_addr = 0;
>>> vq->split.queue_size_in_bytes = 0;
>>> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
>>> break;
>>> case VIRTIO_F_RING_PACKED:
>>> break;
>>> + case VIRTIO_F_ORDER_PLATFORM:
>>> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
>>> --- a/include/uapi/linux/virtio_config.h
>>> +++ b/include/uapi/linux/virtio_config.h
>>> @@ -78,6 +78,12 @@
>>> /* This feature indicates support for the packed virtqueue layout. */
>>> #define VIRTIO_F_RING_PACKED 34
>>> +/*
>>> + * This feature indicates that memory accesses by the driver and the
>>> + * device are ordered in a way described by the platform.
>>> + */
>>> +#define VIRTIO_F_ORDER_PLATFORM 36
>>> +
>>> /*
>>> * Does the device support Single Root I/O Virtualization?
>>> */
>>
>> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
>> barrier? Looks not.
> IIUC we don't need an mmio barrier because we are using a
> serializing API: Documentation/memory-barriers.txt says:
>
> Note that, when using writel(), a prior
> wmb() is not needed to guarantee that the cache coherent memory writes
> have completed before writing to the MMIO region.
Ah, I get this.
>
>
>> See ia64/include/asm/barrier.h:
>>
>> * Note: "mb()" and its variants cannot be used as a fence to order
>> * accesses to memory mapped I/O registers. For that, mf.a needs to
>> * be used. However, we don't want to always use mf.a because (a)
>> * it's (presumably) much slower than mf and (b) mf.a is supported for
>> * sequential memory pages only.
>> */
>> #define mb() ia64_mf()
>> #define rmb() mb()
>> #define wmb() mb()
>>
>> #define dma_rmb() mb()
>> =>efine dma_wmb() mb()
>>
>> Thanks
> Frankly no idea about ia64.
Neither did me.
> Sorry. Are any less esoteric platforms
> affected?
>
E.g ppc64?
define dma_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : :
:"memo\
ry")
/*
* Enforce synchronisation of stores vs. spin_unlock
* (this does it explicitly, though our implementation of spin_unlock
* does it implicitely too)
*/
static inline void mmiowb(void)
{
unsigned long tmp;
__asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
: "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
: "memory");
}
dma_wmb() is lwsync which is more lightweight than sync I guess?
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 3:08 ` Jason Wang
(?)
(?)
@ 2019-01-23 3:49 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 3:49 UTC (permalink / raw)
To: Jason Wang; +Cc: virtio-dev, linux-kernel, virtualization
On Wed, Jan 23, 2019 at 11:08:04AM +0800, Jason Wang wrote:
>
> On 2019/1/23 上午1:03, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > 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 | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
>
>
> I wonder whether or not this is sufficient. Is dma barrier implies a mmio
> barrier? Looks not.
IIUC we don't need an mmio barrier because we are using a
serializing API: Documentation/memory-barriers.txt says:
Note that, when using writel(), a prior
wmb() is not needed to guarantee that the cache coherent memory writes
have completed before writing to the MMIO region.
> See ia64/include/asm/barrier.h:
>
> * Note: "mb()" and its variants cannot be used as a fence to order
> * accesses to memory mapped I/O registers. For that, mf.a needs to
> * be used. However, we don't want to always use mf.a because (a)
> * it's (presumably) much slower than mf and (b) mf.a is supported for
> * sequential memory pages only.
> */
> #define mb() ia64_mf()
> #define rmb() mb()
> #define wmb() mb()
>
> #define dma_rmb() mb()
> =>efine dma_wmb() mb()
>
> Thanks
Frankly no idea about ia64. Sorry. Are any less esoteric platforms
affected?
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-22 17:03 ` Tiwei Bie
` (2 preceding siblings ...)
(?)
@ 2019-01-23 4:04 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 4:04 UTC (permalink / raw)
To: Tiwei Bie; +Cc: virtio-dev, linux-kernel, virtualization
On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Could you pls add a bit more explanation in the commit log?
E.g. which configurations are broken without this patch?
How severe is the problem?
I'm trying to decide whether this belongs in 5.0 or 5.1.
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
> --
> 2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [virtio-dev] Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-22 17:03 ` Tiwei Bie
@ 2019-01-23 4:04 ` Michael S. Tsirkin
-1 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 4:04 UTC (permalink / raw)
To: Tiwei Bie; +Cc: jasowang, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Could you pls add a bit more explanation in the commit log?
E.g. which configurations are broken without this patch?
How severe is the problem?
I'm trying to decide whether this belongs in 5.0 or 5.1.
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
> --
> 2.17.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 4:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2019-01-23 4:04 UTC (permalink / raw)
To: Tiwei Bie; +Cc: jasowang, virtualization, linux-kernel, virtio-dev
On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> When this feature is negotiated, driver will use the barriers
> suitable for hardware devices.
>
> Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
Could you pls add a bit more explanation in the commit log?
E.g. which configurations are broken without this patch?
How severe is the problem?
I'm trying to decide whether this belongs in 5.0 or 5.1.
> ---
> drivers/virtio/virtio_ring.c | 8 ++++++++
> include/uapi/linux/virtio_config.h | 6 ++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..27d3f057493e 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> !context;
> vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
>
> + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->packed.ring_dma_addr = ring_dma_addr;
> vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> vq->packed.device_event_dma_addr = device_event_dma_addr;
> @@ -2079,6 +2082,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_ORDER_PLATFORM))
> + vq->weak_barriers = false;
> +
> vq->split.queue_dma_addr = 0;
> vq->split.queue_size_in_bytes = 0;
>
> @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> break;
> case VIRTIO_F_RING_PACKED:
> break;
> + case VIRTIO_F_ORDER_PLATFORM:
> + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -78,6 +78,12 @@
> /* This feature indicates support for the packed virtqueue layout. */
> #define VIRTIO_F_RING_PACKED 34
>
> +/*
> + * This feature indicates that memory accesses by the driver and the
> + * device are ordered in a way described by the platform.
> + */
> +#define VIRTIO_F_ORDER_PLATFORM 36
> +
> /*
> * Does the device support Single Root I/O Virtualization?
> */
> --
> 2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [virtio-dev] Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 4:04 ` Michael S. Tsirkin
@ 2019-01-23 8:02 ` Tiwei Bie
-1 siblings, 0 replies; 20+ messages in thread
From: Tiwei Bie @ 2019-01-23 8:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, virtualization, linux-kernel, virtio-dev
On Tue, Jan 22, 2019 at 11:04:29PM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> Could you pls add a bit more explanation in the commit log?
> E.g. which configurations are broken without this patch?
> How severe is the problem?
Sure. Will do that.
Thanks
>
> I'm trying to decide whether this belongs in 5.0 or 5.1.
>
> > ---
> > drivers/virtio/virtio_ring.c | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> >
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> >
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> >
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
> > --
> > 2.17.1
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
@ 2019-01-23 8:02 ` Tiwei Bie
0 siblings, 0 replies; 20+ messages in thread
From: Tiwei Bie @ 2019-01-23 8:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: jasowang, virtualization, linux-kernel, virtio-dev
On Tue, Jan 22, 2019 at 11:04:29PM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> Could you pls add a bit more explanation in the commit log?
> E.g. which configurations are broken without this patch?
> How severe is the problem?
Sure. Will do that.
Thanks
>
> I'm trying to decide whether this belongs in 5.0 or 5.1.
>
> > ---
> > drivers/virtio/virtio_ring.c | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> >
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> >
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> >
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
> > --
> > 2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
2019-01-23 4:04 ` Michael S. Tsirkin
(?)
(?)
@ 2019-01-23 8:02 ` Tiwei Bie
-1 siblings, 0 replies; 20+ messages in thread
From: Tiwei Bie @ 2019-01-23 8:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-dev, linux-kernel, virtualization
On Tue, Jan 22, 2019 at 11:04:29PM -0500, Michael S. Tsirkin wrote:
> On Wed, Jan 23, 2019 at 01:03:46AM +0800, Tiwei Bie wrote:
> > This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
> > When this feature is negotiated, driver will use the barriers
> > suitable for hardware devices.
> >
> > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
>
> Could you pls add a bit more explanation in the commit log?
> E.g. which configurations are broken without this patch?
> How severe is the problem?
Sure. Will do that.
Thanks
>
> I'm trying to decide whether this belongs in 5.0 or 5.1.
>
> > ---
> > drivers/virtio/virtio_ring.c | 8 ++++++++
> > include/uapi/linux/virtio_config.h | 6 ++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..27d3f057493e 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -1609,6 +1609,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> > !context;
> > vq->event = virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX);
> >
> > + if (virtio_has_feature(vdev, VIRTIO_F_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->packed.ring_dma_addr = ring_dma_addr;
> > vq->packed.driver_event_dma_addr = driver_event_dma_addr;
> > vq->packed.device_event_dma_addr = device_event_dma_addr;
> > @@ -2079,6 +2082,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_ORDER_PLATFORM))
> > + vq->weak_barriers = false;
> > +
> > vq->split.queue_dma_addr = 0;
> > vq->split.queue_size_in_bytes = 0;
> >
> > @@ -2213,6 +2219,8 @@ void vring_transport_features(struct virtio_device *vdev)
> > break;
> > case VIRTIO_F_RING_PACKED:
> > break;
> > + case VIRTIO_F_ORDER_PLATFORM:
> > + 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 1196e1c1d4f6..ff8e7dc9d4dd 100644
> > --- a/include/uapi/linux/virtio_config.h
> > +++ b/include/uapi/linux/virtio_config.h
> > @@ -78,6 +78,12 @@
> > /* This feature indicates support for the packed virtqueue layout. */
> > #define VIRTIO_F_RING_PACKED 34
> >
> > +/*
> > + * This feature indicates that memory accesses by the driver and the
> > + * device are ordered in a way described by the platform.
> > + */
> > +#define VIRTIO_F_ORDER_PLATFORM 36
> > +
> > /*
> > * Does the device support Single Root I/O Virtualization?
> > */
> > --
> > 2.17.1
^ permalink raw reply [flat|nested] 20+ messages in thread