virtio: support VIRTIO_F_IO_BARRIER
RFC 1/1 virtio: support VIRTIO_F_IO_BARRIER
This patch introduces the support for VIRTIO_F_IO_BARRIER. When this feature is negotiated, driver will use the barriers suitable for hardware devices.
--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)
drivers/virtio/virtio_ring.c +5 -0
@@ -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);
include/uapi/linux/virtio_config.h +7 -1
@@ -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 */
Signature · 1 line
2.11.0
Quoted 47 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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
include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 59 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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
Quoted 71 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers for rpmsg"?
Thanks
We already have below commit and some other related commits in the tree:
7b21e34fd1c2("virtio: harsher barriers for rpmsg.")
Quoted 8 lines
They should have already guaranteed that
virtio_Xmb()will be OK for hardware devices whenvq->weak_barriersis 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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 71 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers for rpmsg"?
Thanks
We already have below commit and some other related commits in the tree:
7b21e34fd1c2("virtio: harsher barriers for rpmsg.")
Quoted 8 lines
They should have already guaranteed that
virtio_Xmb()will be OK for hardware devices whenvq->weak_barriersis 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
Quoted 78 lines · Jason Wang
Quoted 77 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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 whenvq->weak_barriersis 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).
Quoted 6 lines
Quoted 2 lines
Best regards,
Tiwei BieWell, 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
Quoted 2 lines
Thanks
Quoted 79 lines · Tiwei Bie
Quoted 78 lines · Jason Wang
Quoted 77 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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 whenvq->weak_barriersis 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).
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Best regards,
Tiwei BieWell, 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
Quoted 5 lines
Best regards,
Tiwei BieQuoted 1 line
Thanks
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 79 lines · Tiwei Bie
Quoted 78 lines · Jason Wang
Quoted 77 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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 whenvq->weak_barriersis 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).
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Best regards,
Tiwei BieWell, 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
Quoted 5 lines
Best regards,
Tiwei BieQuoted 1 line
Thanks
Quoted 78 lines · Jason Wang
Quoted 77 lines · Tiwei Bie
Quoted 58 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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 whenvq->weak_barriersis 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).
Quoted 6 lines
Quoted 2 lines
Best regards,
Tiwei BieWell, 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
Quoted 2 lines
Thanks
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 59 lines · Jason Wang
Quoted 45 lines · Tiwei Bie
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 <[email protected]>--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +5
-0@@ -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);include/uapi/linux/virtio_config.h +1
-1@@ -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 include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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:
Quoted 7 lines
I believe this depends on Michael's patch of
"[PATCH]
virtio_ring: switch todma_XXbarriers 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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 5 lines · Tiwei Bie
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 <[email protected]>
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?
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Quoted 5 lines · Tiwei Bie
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 <[email protected]>
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?
Quoted 7 lines · Stefan Hajnoczi
Quoted 5 lines · Tiwei Bie
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 <[email protected]>
Quoted 7 lines
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_barrierswhenVIRTIO_F_IOMMU_PLATFORMis 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
Quoted 13 lines · Tiwei Bie
Quoted 12 lines · Stefan Hajnoczi
Quoted 5 lines · Tiwei Bie
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 <[email protected]>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).
Quoted 6 lines
Quoted 2 lines
Does disabling
weak_barrierswhenVIRTIO_F_IOMMU_PLATFORMis set solve the problem?The
VIRTIO_F_IOMMU_PLATFORMfeature 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
Quoted 13 lines · Tiwei Bie
Quoted 12 lines · Stefan Hajnoczi
Quoted 5 lines · Tiwei Bie
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 <[email protected]>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).
Quoted 6 lines
Quoted 2 lines
Does disabling
weak_barrierswhenVIRTIO_F_IOMMU_PLATFORMis set solve the problem?The
VIRTIO_F_IOMMU_PLATFORMfeature 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
Michael S. Tsirkin
maintainerQuoted 5 lines · Tiwei Bie
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 <[email protected]>
Thanks!
Quoted 10 lines
--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +0
-0
drivers/virtio/virtio_ring.c struct virtqueue *__vring_new_virtqueue(unsigned int index, · line 1004
Quoted 9 lines
drivers/virtio/virtio_ring.c +3
-0@@ -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.
include/uapi/linux/virtio_config.h
Quoted 13 lines
drivers/virtio/virtio_ring.c +2
-0@@ -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);include/uapi/linux/virtio_config.h +0
-0
Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists (subscriber-only, sorry about that).
Quoted 10 lines
include/uapi/linux/virtio_config.h +1
-1@@ -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
include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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.
Quoted 2 lines
Signature · 1 line
2.11.0
Michael S. Tsirkin
maintainerQuoted 5 lines · Tiwei Bie
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 <[email protected]>
Thanks!
Quoted 10 lines
--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +0
-0
drivers/virtio/virtio_ring.c struct virtqueue *__vring_new_virtqueue(unsigned int index, · line 1004
Quoted 9 lines
drivers/virtio/virtio_ring.c +3
-0@@ -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.
include/uapi/linux/virtio_config.h
Quoted 13 lines
drivers/virtio/virtio_ring.c +2
-0@@ -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);include/uapi/linux/virtio_config.h +0
-0
Any virtio UAPI changes must be CC'd to one of the virtio TC mailing lists (subscriber-only, sorry about that).
Quoted 10 lines
include/uapi/linux/virtio_config.h +1
-1@@ -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
include/uapi/linux/virtio_config.h line 80
Quoted 10 lines
include/uapi/linux/virtio_config.h +6
-0@@ -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.
Quoted 2 lines
Signature · 1 line
2.11.0
Quoted 22 lines · Michael S. Tsirkin
Quoted 5 lines · Tiwei Bie
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 <[email protected]>Thanks!
Quoted 13 lines
--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +0
-0@@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index, !context; vq->event =virtio_has_feature (vdev,VIRTIO_RING_F_EVENT_IDX );
Quoted 10 lines
drivers/virtio/virtio_ring.c struct virtqueue *__vring_new_virtqueue(unsigned int index, · line 1004
Quoted 6 lines
drivers/virtio/virtio_ring.c +3
-0+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"
Quoted 17 lines
include/uapi/linux/virtio_config.h
Quoted 14 lines
drivers/virtio/virtio_ring.c +2
-0@@ -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);include/uapi/linux/virtio_config.h +0
-0Any 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.
Quoted 8 lines
Quoted 8 lines
include/uapi/linux/virtio_config.h +1
-1@@ -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
Quoted 15 lines
include/uapi/linux/virtio_config.h line 80
Quoted 13 lines
include/uapi/linux/virtio_config.h +6
-0#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
Quoted 3 lines
Quoted 3 lines
Signature · 2 lines
-- 2.11.0
Quoted 22 lines · Michael S. Tsirkin
Quoted 5 lines · Tiwei Bie
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 <[email protected]>Thanks!
Quoted 13 lines
--- drivers/virtio/virtio_ring.c | 5 +++++ include/uapi/linux/virtio_config.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-)drivers/virtio/virtio_ring.c +0
-0@@ -996,6 +996,9 @@ struct virtqueue *__vring_new_virtqueue(unsigned int index, !context; vq->event =virtio_has_feature (vdev,VIRTIO_RING_F_EVENT_IDX );
Quoted 10 lines
drivers/virtio/virtio_ring.c struct virtqueue *__vring_new_virtqueue(unsigned int index, · line 1004
Quoted 6 lines
drivers/virtio/virtio_ring.c +3
-0+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"
Quoted 17 lines
include/uapi/linux/virtio_config.h
Quoted 14 lines
drivers/virtio/virtio_ring.c +2
-0@@ -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);include/uapi/linux/virtio_config.h +0
-0Any 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.
Quoted 8 lines
Quoted 8 lines
include/uapi/linux/virtio_config.h +1
-1@@ -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
Quoted 15 lines
include/uapi/linux/virtio_config.h line 80
Quoted 13 lines
include/uapi/linux/virtio_config.h +6
-0#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
Quoted 3 lines
Quoted 3 lines
Signature · 2 lines
-- 2.11.0