* Re: [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Michael S. Tsirkin @ 2019-01-23 14:08 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190123095557.30168-6-jasowang@redhat.com>
On Wed, Jan 23, 2019 at 05:55:57PM +0800, Jason Wang wrote:
> It was noticed that the copy_user() friends that was used to access
> virtqueue metdata tends to be very expensive for dataplane
> implementation like vhost since it involves lots of software checks,
> speculation barrier, hardware feature toggling (e.g SMAP). The
> extra cost will be more obvious when transferring small packets since
> the time spent on metadata accessing become more significant.
>
> This patch tries to eliminate those overheads by accessing them
> through kernel virtual address by vmap(). To make the pages can be
> migrated, instead of pinning them through GUP, we use MMU notifiers to
> invalidate vmaps and re-establish vmaps during each round of metadata
> prefetching if necessary. For devices that doesn't use metadata
> prefetching, the memory accessors fallback to normal copy_user()
> implementation gracefully. The invalidation was synchronized with
> datapath through vq mutex, and in order to avoid hold vq mutex during
> range checking, MMU notifier was teared down when trying to modify vq
> metadata.
>
> Another thing is kernel lacks efficient solution for tracking dirty
> pages by vmap(), this will lead issues if vhost is using file backed
> memory which needs care of writeback. This patch solves this issue by
> just skipping the vma that is file backed and fallback to normal
> copy_user() friends. This might introduce some overheads for file
> backed users but consider this use case is rare we could do
> optimizations on top.
>
> Note that this was only done when device IOTLB is not enabled. We
> could use similar method to optimize it in the future.
>
> Tests shows at most about 22% improvement on TX PPS when using
> virtio-user + vhost_net + xdp1 + TAP on 2.6GHz Broadwell:
>
> SMAP on | SMAP off
> Before: 5.0Mpps | 6.6Mpps
> After: 6.1Mpps | 7.4Mpps
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
So this is the bulk of the change.
Threee things that I need to look into
- Are there any security issues with bypassing the speculation barrier
that is normally present after access_ok?
- How hard does the special handling for
file backed storage make testing?
On the one hand we could add a module parameter to
force copy to/from user. on the other that's
another configuration we need to support.
But iotlb is not using vmap, so maybe that's enough
for testing.
- How hard is it to figure out which mode uses which code.
Meanwhile, could you pls post data comparing this last patch with the
below? This removes the speculation barrier replacing it with a
(useless but at least more lightweight) data dependency.
Thanks!
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index bac939af8dbb..352ee7e14476 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -739,7 +739,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to,
int ret;
if (!vq->iotlb)
- return __copy_to_user(to, from, size);
+ return copy_to_user(to, from, size);
else {
/* This function should be called after iotlb
* prefetch, which means we're sure that all vq
@@ -752,7 +752,7 @@ static int vhost_copy_to_user(struct vhost_virtqueue *vq, void __user *to,
VHOST_ADDR_USED);
if (uaddr)
- return __copy_to_user(uaddr, from, size);
+ return copy_to_user(uaddr, from, size);
ret = translate_desc(vq, (u64)(uintptr_t)to, size, vq->iotlb_iov,
ARRAY_SIZE(vq->iotlb_iov),
@@ -774,7 +774,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
int ret;
if (!vq->iotlb)
- return __copy_from_user(to, from, size);
+ return copy_from_user(to, from, size);
else {
/* This function should be called after iotlb
* prefetch, which means we're sure that vq
@@ -787,7 +787,7 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
struct iov_iter f;
if (uaddr)
- return __copy_from_user(to, uaddr, size);
+ return copy_from_user(to, uaddr, size);
ret = translate_desc(vq, (u64)(uintptr_t)from, size, vq->iotlb_iov,
ARRAY_SIZE(vq->iotlb_iov),
@@ -855,13 +855,13 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
({ \
int ret = -EFAULT; \
if (!vq->iotlb) { \
- ret = __put_user(x, ptr); \
+ ret = put_user(x, ptr); \
} else { \
__typeof__(ptr) to = \
(__typeof__(ptr)) __vhost_get_user(vq, ptr, \
sizeof(*ptr), VHOST_ADDR_USED); \
if (to != NULL) \
- ret = __put_user(x, to); \
+ ret = put_user(x, to); \
else \
ret = -EFAULT; \
} \
@@ -872,14 +872,14 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
({ \
int ret; \
if (!vq->iotlb) { \
- ret = __get_user(x, ptr); \
+ ret = get_user(x, ptr); \
} else { \
__typeof__(ptr) from = \
(__typeof__(ptr)) __vhost_get_user(vq, ptr, \
sizeof(*ptr), \
type); \
if (from != NULL) \
- ret = __get_user(x, from); \
+ ret = get_user(x, from); \
else \
ret = -EFAULT; \
} \
^ permalink raw reply related
* Re: [PATCH net-next V4 0/5] vhost: accelerate metadata access through vmap()
From: Michael S. Tsirkin @ 2019-01-23 13:58 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
On Wed, Jan 23, 2019 at 05:55:52PM +0800, Jason Wang wrote:
> This series tries to access virtqueue metadata through kernel virtual
> address instead of copy_user() friends since they had too much
> overheads like checks, spec barriers or even hardware feature
> toggling.
>
> Test shows about 24% improvement on TX PPS. It should benefit other
> cases as well.
ok I think this addresses most comments but it's a big change and we
just started 1.1 review so to pls give me a week to review this ok?
> Changes from V3:
> - don't try to use vmap for file backed pages
> - rebase to master
> Changes from V2:
> - fix buggy range overlapping check
> - tear down MMU notifier during vhost ioctl to make sure invalidation
> request can read metadata userspace address and vq size without
> holding vq mutex.
> Changes from V1:
> - instead of pinning pages, use MMU notifier to invalidate vmaps and
> remap duing metadata prefetch
> - fix build warning on MIPS
>
> Jason Wang (5):
> vhost: generalize adding used elem
> vhost: fine grain userspace memory accessors
> vhost: rename vq_iotlb_prefetch() to vq_meta_prefetch()
> vhost: introduce helpers to get the size of metadata area
> vhost: access vq metadata through kernel virtual address
>
> drivers/vhost/net.c | 4 +-
> drivers/vhost/vhost.c | 441 +++++++++++++++++++++++++++++++++++++-----
> drivers/vhost/vhost.h | 15 +-
> mm/shmem.c | 1 +
> 4 files changed, 410 insertions(+), 51 deletions(-)
>
> --
> 2.17.1
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Michael S. Tsirkin @ 2019-01-23 13:49 UTC (permalink / raw)
To: Jason Wang; +Cc: virtio-dev, linux-kernel, virtualization
In-Reply-To: <7f07c837-d3d0-b5c0-9ecb-29b545b377c3@redhat.com>
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
* [PATCH net-next V4 5/5] vhost: access vq metadata through kernel virtual address
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
It was noticed that the copy_user() friends that was used to access
virtqueue metdata tends to be very expensive for dataplane
implementation like vhost since it involves lots of software checks,
speculation barrier, hardware feature toggling (e.g SMAP). The
extra cost will be more obvious when transferring small packets since
the time spent on metadata accessing become more significant.
This patch tries to eliminate those overheads by accessing them
through kernel virtual address by vmap(). To make the pages can be
migrated, instead of pinning them through GUP, we use MMU notifiers to
invalidate vmaps and re-establish vmaps during each round of metadata
prefetching if necessary. For devices that doesn't use metadata
prefetching, the memory accessors fallback to normal copy_user()
implementation gracefully. The invalidation was synchronized with
datapath through vq mutex, and in order to avoid hold vq mutex during
range checking, MMU notifier was teared down when trying to modify vq
metadata.
Another thing is kernel lacks efficient solution for tracking dirty
pages by vmap(), this will lead issues if vhost is using file backed
memory which needs care of writeback. This patch solves this issue by
just skipping the vma that is file backed and fallback to normal
copy_user() friends. This might introduce some overheads for file
backed users but consider this use case is rare we could do
optimizations on top.
Note that this was only done when device IOTLB is not enabled. We
could use similar method to optimize it in the future.
Tests shows at most about 22% improvement on TX PPS when using
virtio-user + vhost_net + xdp1 + TAP on 2.6GHz Broadwell:
SMAP on | SMAP off
Before: 5.0Mpps | 6.6Mpps
After: 6.1Mpps | 7.4Mpps
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 288 +++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.h | 13 ++
mm/shmem.c | 1 +
3 files changed, 300 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 37e2cac8e8b0..096ae3298d62 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -440,6 +440,9 @@ void vhost_dev_init(struct vhost_dev *dev,
vq->indirect = NULL;
vq->heads = NULL;
vq->dev = dev;
+ memset(&vq->avail_ring, 0, sizeof(vq->avail_ring));
+ memset(&vq->used_ring, 0, sizeof(vq->used_ring));
+ memset(&vq->desc_ring, 0, sizeof(vq->desc_ring));
mutex_init(&vq->mutex);
vhost_vq_reset(dev, vq);
if (vq->handle_kick)
@@ -510,6 +513,73 @@ static size_t vhost_get_desc_size(struct vhost_virtqueue *vq, int num)
return sizeof(*vq->desc) * num;
}
+static void vhost_uninit_vmap(struct vhost_vmap *map)
+{
+ if (map->addr)
+ vunmap(map->unmap_addr);
+
+ map->addr = NULL;
+ map->unmap_addr = NULL;
+}
+
+static int vhost_invalidate_vmap(struct vhost_virtqueue *vq,
+ struct vhost_vmap *map,
+ unsigned long ustart,
+ size_t size,
+ unsigned long start,
+ unsigned long end,
+ bool blockable)
+{
+ if (end < ustart || start > ustart - 1 + size)
+ return 0;
+
+ if (!blockable)
+ return -EAGAIN;
+
+ mutex_lock(&vq->mutex);
+ vhost_uninit_vmap(map);
+ mutex_unlock(&vq->mutex);
+
+ return 0;
+}
+
+static int vhost_invalidate_range_start(struct mmu_notifier *mn,
+ const struct mmu_notifier_range *range)
+{
+ struct vhost_dev *dev = container_of(mn, struct vhost_dev,
+ mmu_notifier);
+ int i;
+
+ for (i = 0; i < dev->nvqs; i++) {
+ struct vhost_virtqueue *vq = dev->vqs[i];
+
+ if (vhost_invalidate_vmap(vq, &vq->avail_ring,
+ (unsigned long)vq->avail,
+ vhost_get_avail_size(vq, vq->num),
+ range->start, range->end,
+ range->blockable))
+ return -EAGAIN;
+ if (vhost_invalidate_vmap(vq, &vq->desc_ring,
+ (unsigned long)vq->desc,
+ vhost_get_desc_size(vq, vq->num),
+ range->start, range->end,
+ range->blockable))
+ return -EAGAIN;
+ if (vhost_invalidate_vmap(vq, &vq->used_ring,
+ (unsigned long)vq->used,
+ vhost_get_used_size(vq, vq->num),
+ range->start, range->end,
+ range->blockable))
+ return -EAGAIN;
+ }
+
+ return 0;
+}
+
+static const struct mmu_notifier_ops vhost_mmu_notifier_ops = {
+ .invalidate_range_start = vhost_invalidate_range_start,
+};
+
/* Caller should have device mutex */
long vhost_dev_set_owner(struct vhost_dev *dev)
{
@@ -541,7 +611,14 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
if (err)
goto err_cgroup;
+ dev->mmu_notifier.ops = &vhost_mmu_notifier_ops;
+ err = mmu_notifier_register(&dev->mmu_notifier, dev->mm);
+ if (err)
+ goto err_mmu_notifier;
+
return 0;
+err_mmu_notifier:
+ vhost_dev_free_iovecs(dev);
err_cgroup:
kthread_stop(worker);
dev->worker = NULL;
@@ -632,6 +709,97 @@ static void vhost_clear_msg(struct vhost_dev *dev)
spin_unlock(&dev->iotlb_lock);
}
+/* Suppress the vma that needs writeback since we can not track dirty
+ * pages now.
+ */
+static bool vma_can_vmap(struct vm_area_struct *vma)
+{
+ return vma_is_anonymous(vma) || is_vm_hugetlb_page(vma) ||
+ vma_is_shmem(vma);
+}
+
+static int vhost_init_vmap(struct vhost_dev *dev,
+ struct vhost_vmap *map, unsigned long uaddr,
+ size_t size, int write)
+{
+ struct mm_struct *mm = dev->mm;
+ struct vm_area_struct *vma;
+ struct page **pages;
+ int npages = DIV_ROUND_UP(size, PAGE_SIZE);
+ int npinned;
+ void *vaddr;
+ int err = 0;
+
+ down_read(&mm->mmap_sem);
+ vma = find_vma(mm, uaddr);
+ if (!vma || !vma_can_vmap(vma) ||
+ vma->vm_end < uaddr - 1 + size) {
+ err = -EINVAL;
+ goto err_vma;
+ }
+
+ pages = kmalloc_array(npages, sizeof(struct page *), GFP_KERNEL);
+ if (!pages) {
+ err = -ENOMEM;
+ goto err_alloc;
+ }
+
+ npinned = get_user_pages_fast(uaddr, npages, write, pages);
+ if (npinned != npages) {
+ err = -EFAULT;
+ goto err_gup;
+ }
+
+ vaddr = vmap(pages, npages, VM_MAP, PAGE_KERNEL);
+ if (!vaddr) {
+ err = EFAULT;
+ goto err_gup;
+ }
+
+ map->addr = vaddr + (uaddr & (PAGE_SIZE - 1));
+ map->unmap_addr = vaddr;
+
+err_gup:
+ /* Don't pin pages, mmu notifier will notify us about page
+ * migration.
+ */
+ if (npinned > 0)
+ release_pages(pages, npinned);
+err_alloc:
+ kfree(pages);
+err_vma:
+ up_read(&mm->mmap_sem);
+ return err;
+}
+
+static void vhost_clean_vmaps(struct vhost_virtqueue *vq)
+{
+ vhost_uninit_vmap(&vq->avail_ring);
+ vhost_uninit_vmap(&vq->desc_ring);
+ vhost_uninit_vmap(&vq->used_ring);
+}
+
+static int vhost_setup_avail_vmap(struct vhost_virtqueue *vq,
+ unsigned long avail)
+{
+ return vhost_init_vmap(vq->dev, &vq->avail_ring, avail,
+ vhost_get_avail_size(vq, vq->num), false);
+}
+
+static int vhost_setup_desc_vmap(struct vhost_virtqueue *vq,
+ unsigned long desc)
+{
+ return vhost_init_vmap(vq->dev, &vq->desc_ring, desc,
+ vhost_get_desc_size(vq, vq->num), false);
+}
+
+static int vhost_setup_used_vmap(struct vhost_virtqueue *vq,
+ unsigned long used)
+{
+ return vhost_init_vmap(vq->dev, &vq->used_ring, used,
+ vhost_get_used_size(vq, vq->num), true);
+}
+
void vhost_dev_cleanup(struct vhost_dev *dev)
{
int i;
@@ -661,8 +829,12 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
kthread_stop(dev->worker);
dev->worker = NULL;
}
- if (dev->mm)
+ if (dev->mm) {
+ mmu_notifier_unregister(&dev->mmu_notifier, dev->mm);
mmput(dev->mm);
+ }
+ for (i = 0; i < dev->nvqs; i++)
+ vhost_clean_vmaps(dev->vqs[i]);
dev->mm = NULL;
}
EXPORT_SYMBOL_GPL(vhost_dev_cleanup);
@@ -891,6 +1063,16 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
static inline int vhost_put_avail_event(struct vhost_virtqueue *vq)
{
+ if (!vq->iotlb) {
+ struct vring_used *used = vq->used_ring.addr;
+
+ if (likely(used)) {
+ *((__virtio16 *)&used->ring[vq->num]) =
+ cpu_to_vhost16(vq, vq->avail_idx);
+ return 0;
+ }
+ }
+
return vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
vhost_avail_event(vq));
}
@@ -899,6 +1081,16 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
struct vring_used_elem *head, int idx,
int count)
{
+ if (!vq->iotlb) {
+ struct vring_used *used = vq->used_ring.addr;
+
+ if (likely(used)) {
+ memcpy(used->ring + idx, head,
+ count * sizeof(*head));
+ return 0;
+ }
+ }
+
return vhost_copy_to_user(vq, vq->used->ring + idx, head,
count * sizeof(*head));
}
@@ -906,6 +1098,15 @@ static inline int vhost_put_used(struct vhost_virtqueue *vq,
static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
{
+ if (!vq->iotlb) {
+ struct vring_used *used = vq->used_ring.addr;
+
+ if (likely(used)) {
+ used->flags = cpu_to_vhost16(vq, vq->used_flags);
+ return 0;
+ }
+ }
+
return vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
&vq->used->flags);
}
@@ -913,6 +1114,15 @@ static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
static inline int vhost_put_used_idx(struct vhost_virtqueue *vq)
{
+ if (!vq->iotlb) {
+ struct vring_used *used = vq->used_ring.addr;
+
+ if (likely(used)) {
+ used->idx = cpu_to_vhost16(vq, vq->last_used_idx);
+ return 0;
+ }
+ }
+
return vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
&vq->used->idx);
}
@@ -958,12 +1168,30 @@ static void vhost_dev_unlock_vqs(struct vhost_dev *d)
static inline int vhost_get_avail_idx(struct vhost_virtqueue *vq,
__virtio16 *idx)
{
+ if (!vq->iotlb) {
+ struct vring_avail *avail = vq->avail_ring.addr;
+
+ if (likely(avail)) {
+ *idx = avail->idx;
+ return 0;
+ }
+ }
+
return vhost_get_avail(vq, *idx, &vq->avail->idx);
}
static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
__virtio16 *head, int idx)
{
+ if (!vq->iotlb) {
+ struct vring_avail *avail = vq->avail_ring.addr;
+
+ if (likely(avail)) {
+ *head = avail->ring[idx & (vq->num - 1)];
+ return 0;
+ }
+ }
+
return vhost_get_avail(vq, *head,
&vq->avail->ring[idx & (vq->num - 1)]);
}
@@ -971,24 +1199,60 @@ static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
static inline int vhost_get_avail_flags(struct vhost_virtqueue *vq,
__virtio16 *flags)
{
+ if (!vq->iotlb) {
+ struct vring_avail *avail = vq->avail_ring.addr;
+
+ if (likely(avail)) {
+ *flags = avail->flags;
+ return 0;
+ }
+ }
+
return vhost_get_avail(vq, *flags, &vq->avail->flags);
}
static inline int vhost_get_used_event(struct vhost_virtqueue *vq,
__virtio16 *event)
{
+ if (!vq->iotlb) {
+ struct vring_avail *avail = vq->avail_ring.addr;
+
+ if (likely(avail)) {
+ *event = (__virtio16)avail->ring[vq->num];
+ return 0;
+ }
+ }
+
return vhost_get_avail(vq, *event, vhost_used_event(vq));
}
static inline int vhost_get_used_idx(struct vhost_virtqueue *vq,
__virtio16 *idx)
{
+ if (!vq->iotlb) {
+ struct vring_used *used = vq->used_ring.addr;
+
+ if (likely(used)) {
+ *idx = used->idx;
+ return 0;
+ }
+ }
+
return vhost_get_used(vq, *idx, &vq->used->idx);
}
static inline int vhost_get_desc(struct vhost_virtqueue *vq,
struct vring_desc *desc, int idx)
{
+ if (!vq->iotlb) {
+ struct vring_desc *d = vq->desc_ring.addr;
+
+ if (likely(d)) {
+ *desc = *(d + idx);
+ return 0;
+ }
+ }
+
return vhost_copy_from_user(vq, desc, vq->desc + idx, sizeof(*desc));
}
@@ -1329,8 +1593,16 @@ int vq_meta_prefetch(struct vhost_virtqueue *vq)
{
unsigned int num = vq->num;
- if (!vq->iotlb)
+ if (!vq->iotlb) {
+ if (unlikely(!vq->avail_ring.addr))
+ vhost_setup_avail_vmap(vq, (unsigned long)vq->avail);
+ if (unlikely(!vq->desc_ring.addr))
+ vhost_setup_desc_vmap(vq, (unsigned long)vq->desc);
+ if (unlikely(!vq->used_ring.addr))
+ vhost_setup_used_vmap(vq, (unsigned long)vq->used);
+
return 1;
+ }
return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
vhost_get_desc_size(vq, num), VHOST_ADDR_DESC) &&
@@ -1482,6 +1754,13 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
mutex_lock(&vq->mutex);
+ /* Unregister MMU notifer to allow invalidation callback
+ * can access vq->avail, vq->desc , vq->used and vq->num
+ * without holding vq->mutex.
+ */
+ if (d->mm)
+ mmu_notifier_unregister(&d->mmu_notifier, d->mm);
+
switch (ioctl) {
case VHOST_SET_VRING_NUM:
/* Resizing ring with an active backend?
@@ -1498,6 +1777,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
r = -EINVAL;
break;
}
+ vhost_clean_vmaps(vq);
vq->num = s.num;
break;
case VHOST_SET_VRING_BASE:
@@ -1575,6 +1855,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
}
}
+ vhost_clean_vmaps(vq);
+
vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));
vq->desc = (void __user *)(unsigned long)a.desc_user_addr;
vq->avail = (void __user *)(unsigned long)a.avail_user_addr;
@@ -1655,6 +1937,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
if (pollstart && vq->handle_kick)
r = vhost_poll_start(&vq->poll, vq->kick);
+ if (d->mm)
+ mmu_notifier_register(&d->mmu_notifier, d->mm);
mutex_unlock(&vq->mutex);
if (pollstop && vq->handle_kick)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 4e21011b6628..c04bc327db9f 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -12,6 +12,8 @@
#include <linux/virtio_config.h>
#include <linux/virtio_ring.h>
#include <linux/atomic.h>
+#include <linux/pagemap.h>
+#include <linux/mmu_notifier.h>
struct vhost_work;
typedef void (*vhost_work_fn_t)(struct vhost_work *work);
@@ -80,6 +82,11 @@ enum vhost_uaddr_type {
VHOST_NUM_ADDRS = 3,
};
+struct vhost_vmap {
+ void *addr;
+ void *unmap_addr;
+};
+
/* The virtqueue structure describes a queue attached to a device. */
struct vhost_virtqueue {
struct vhost_dev *dev;
@@ -90,6 +97,11 @@ struct vhost_virtqueue {
struct vring_desc __user *desc;
struct vring_avail __user *avail;
struct vring_used __user *used;
+
+ struct vhost_vmap avail_ring;
+ struct vhost_vmap desc_ring;
+ struct vhost_vmap used_ring;
+
const struct vhost_umem_node *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
@@ -158,6 +170,7 @@ struct vhost_msg_node {
struct vhost_dev {
struct mm_struct *mm;
+ struct mmu_notifier mmu_notifier;
struct mutex mutex;
struct vhost_virtqueue **vqs;
int nvqs;
diff --git a/mm/shmem.c b/mm/shmem.c
index 6ece1e2fe76e..745e7c7f7a6c 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -237,6 +237,7 @@ bool vma_is_shmem(struct vm_area_struct *vma)
{
return vma->vm_ops == &shmem_vm_ops;
}
+EXPORT_SYMBOL_GPL(vma_is_shmem);
static LIST_HEAD(shmem_swaplist);
static DEFINE_MUTEX(shmem_swaplist_mutex);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next V4 4/5] vhost: introduce helpers to get the size of metadata area
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 46 ++++++++++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 24c74c60c093..37e2cac8e8b0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -489,6 +489,27 @@ bool vhost_dev_has_owner(struct vhost_dev *dev)
}
EXPORT_SYMBOL_GPL(vhost_dev_has_owner);
+static size_t vhost_get_avail_size(struct vhost_virtqueue *vq, int num)
+{
+ size_t event = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
+
+ return sizeof(*vq->avail) +
+ sizeof(*vq->avail->ring) * num + event;
+}
+
+static size_t vhost_get_used_size(struct vhost_virtqueue *vq, int num)
+{
+ size_t event = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
+
+ return sizeof(*vq->used) +
+ sizeof(*vq->used->ring) * num + event;
+}
+
+static size_t vhost_get_desc_size(struct vhost_virtqueue *vq, int num)
+{
+ return sizeof(*vq->desc) * num;
+}
+
/* Caller should have device mutex */
long vhost_dev_set_owner(struct vhost_dev *dev)
{
@@ -1252,13 +1273,9 @@ static bool vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
struct vring_used __user *used)
{
- size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
-
- return access_ok(desc, num * sizeof *desc) &&
- access_ok(avail,
- sizeof *avail + num * sizeof *avail->ring + s) &&
- access_ok(used,
- sizeof *used + num * sizeof *used->ring + s);
+ return access_ok(desc, vhost_get_desc_size(vq, num)) &&
+ access_ok(avail, vhost_get_avail_size(vq, num)) &&
+ access_ok(used, vhost_get_used_size(vq, num));
}
static void vhost_vq_meta_update(struct vhost_virtqueue *vq,
@@ -1310,22 +1327,18 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq,
int vq_meta_prefetch(struct vhost_virtqueue *vq)
{
- size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
unsigned int num = vq->num;
if (!vq->iotlb)
return 1;
return iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->desc,
- num * sizeof(*vq->desc), VHOST_ADDR_DESC) &&
+ vhost_get_desc_size(vq, num), VHOST_ADDR_DESC) &&
iotlb_access_ok(vq, VHOST_ACCESS_RO, (u64)(uintptr_t)vq->avail,
- sizeof *vq->avail +
- num * sizeof(*vq->avail->ring) + s,
+ vhost_get_avail_size(vq, num),
VHOST_ADDR_AVAIL) &&
iotlb_access_ok(vq, VHOST_ACCESS_WO, (u64)(uintptr_t)vq->used,
- sizeof *vq->used +
- num * sizeof(*vq->used->ring) + s,
- VHOST_ADDR_USED);
+ vhost_get_used_size(vq, num), VHOST_ADDR_USED);
}
EXPORT_SYMBOL_GPL(vq_meta_prefetch);
@@ -1342,13 +1355,10 @@ EXPORT_SYMBOL_GPL(vhost_log_access_ok);
static bool vq_log_access_ok(struct vhost_virtqueue *vq,
void __user *log_base)
{
- size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
-
return vq_memory_access_ok(log_base, vq->umem,
vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
(!vq->log_used || log_access_ok(log_base, vq->log_addr,
- sizeof *vq->used +
- vq->num * sizeof *vq->used->ring + s));
+ vhost_get_used_size(vq, vq->num)));
}
/* Can we start vq? */
--
2.17.1
^ permalink raw reply related
* [PATCH net-next V4 3/5] vhost: rename vq_iotlb_prefetch() to vq_meta_prefetch()
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
Rename the function to be more accurate since it actually tries to
prefetch vq metadata address in IOTLB. And this will be used by
following patch to prefetch metadata virtual addresses.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 4 ++--
drivers/vhost/vhost.c | 4 ++--
drivers/vhost/vhost.h | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index bca86bf7189f..9c83c1837464 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -971,7 +971,7 @@ static void handle_tx(struct vhost_net *net)
if (!sock)
goto out;
- if (!vq_iotlb_prefetch(vq))
+ if (!vq_meta_prefetch(vq))
goto out;
vhost_disable_notify(&net->dev, vq);
@@ -1140,7 +1140,7 @@ static void handle_rx(struct vhost_net *net)
if (!sock)
goto out;
- if (!vq_iotlb_prefetch(vq))
+ if (!vq_meta_prefetch(vq))
goto out;
vhost_disable_notify(&net->dev, vq);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 96dd87531ba0..24c74c60c093 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1308,7 +1308,7 @@ static bool iotlb_access_ok(struct vhost_virtqueue *vq,
return true;
}
-int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
+int vq_meta_prefetch(struct vhost_virtqueue *vq)
{
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
unsigned int num = vq->num;
@@ -1327,7 +1327,7 @@ int vq_iotlb_prefetch(struct vhost_virtqueue *vq)
num * sizeof(*vq->used->ring) + s,
VHOST_ADDR_USED);
}
-EXPORT_SYMBOL_GPL(vq_iotlb_prefetch);
+EXPORT_SYMBOL_GPL(vq_meta_prefetch);
/* Can we log writes? */
/* Caller should have device mutex but not vq mutex */
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 1b675dad5e05..4e21011b6628 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -207,7 +207,7 @@ bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
unsigned int log_num, u64 len,
struct iovec *iov, int count);
-int vq_iotlb_prefetch(struct vhost_virtqueue *vq);
+int vq_meta_prefetch(struct vhost_virtqueue *vq);
struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type);
void vhost_enqueue_msg(struct vhost_dev *dev,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next V4 2/5] vhost: fine grain userspace memory accessors
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
This is used to hide the metadata address from virtqueue helpers. This
will allow to implement a vmap based fast accessing to metadata.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 94 +++++++++++++++++++++++++++++++++++--------
1 file changed, 77 insertions(+), 17 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 14fad2577df3..96dd87531ba0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -868,6 +868,34 @@ static inline void __user *__vhost_get_user(struct vhost_virtqueue *vq,
ret; \
})
+static inline int vhost_put_avail_event(struct vhost_virtqueue *vq)
+{
+ return vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
+ vhost_avail_event(vq));
+}
+
+static inline int vhost_put_used(struct vhost_virtqueue *vq,
+ struct vring_used_elem *head, int idx,
+ int count)
+{
+ return vhost_copy_to_user(vq, vq->used->ring + idx, head,
+ count * sizeof(*head));
+}
+
+static inline int vhost_put_used_flags(struct vhost_virtqueue *vq)
+
+{
+ return vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
+ &vq->used->flags);
+}
+
+static inline int vhost_put_used_idx(struct vhost_virtqueue *vq)
+
+{
+ return vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
+ &vq->used->idx);
+}
+
#define vhost_get_user(vq, x, ptr, type) \
({ \
int ret; \
@@ -906,6 +934,43 @@ static void vhost_dev_unlock_vqs(struct vhost_dev *d)
mutex_unlock(&d->vqs[i]->mutex);
}
+static inline int vhost_get_avail_idx(struct vhost_virtqueue *vq,
+ __virtio16 *idx)
+{
+ return vhost_get_avail(vq, *idx, &vq->avail->idx);
+}
+
+static inline int vhost_get_avail_head(struct vhost_virtqueue *vq,
+ __virtio16 *head, int idx)
+{
+ return vhost_get_avail(vq, *head,
+ &vq->avail->ring[idx & (vq->num - 1)]);
+}
+
+static inline int vhost_get_avail_flags(struct vhost_virtqueue *vq,
+ __virtio16 *flags)
+{
+ return vhost_get_avail(vq, *flags, &vq->avail->flags);
+}
+
+static inline int vhost_get_used_event(struct vhost_virtqueue *vq,
+ __virtio16 *event)
+{
+ return vhost_get_avail(vq, *event, vhost_used_event(vq));
+}
+
+static inline int vhost_get_used_idx(struct vhost_virtqueue *vq,
+ __virtio16 *idx)
+{
+ return vhost_get_used(vq, *idx, &vq->used->idx);
+}
+
+static inline int vhost_get_desc(struct vhost_virtqueue *vq,
+ struct vring_desc *desc, int idx)
+{
+ return vhost_copy_from_user(vq, desc, vq->desc + idx, sizeof(*desc));
+}
+
static int vhost_new_umem_range(struct vhost_umem *umem,
u64 start, u64 size, u64 end,
u64 userspace_addr, int perm)
@@ -1839,8 +1904,7 @@ EXPORT_SYMBOL_GPL(vhost_log_write);
static int vhost_update_used_flags(struct vhost_virtqueue *vq)
{
void __user *used;
- if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->used_flags),
- &vq->used->flags) < 0)
+ if (vhost_put_used_flags(vq))
return -EFAULT;
if (unlikely(vq->log_used)) {
/* Make sure the flag is seen before log. */
@@ -1857,8 +1921,7 @@ static int vhost_update_used_flags(struct vhost_virtqueue *vq)
static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event)
{
- if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->avail_idx),
- vhost_avail_event(vq)))
+ if (vhost_put_avail_event(vq))
return -EFAULT;
if (unlikely(vq->log_used)) {
void __user *used;
@@ -1894,7 +1957,7 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq)
r = -EFAULT;
goto err;
}
- r = vhost_get_used(vq, last_used_idx, &vq->used->idx);
+ r = vhost_get_used_idx(vq, &last_used_idx);
if (r) {
vq_err(vq, "Can't access used idx at %p\n",
&vq->used->idx);
@@ -2093,7 +2156,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
last_avail_idx = vq->last_avail_idx;
if (vq->avail_idx == vq->last_avail_idx) {
- if (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {
+ if (unlikely(vhost_get_avail_idx(vq, &avail_idx))) {
vq_err(vq, "Failed to access avail idx at %p\n",
&vq->avail->idx);
return -EFAULT;
@@ -2120,8 +2183,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
/* Grab the next descriptor number they're advertising, and increment
* the index we've seen. */
- if (unlikely(vhost_get_avail(vq, ring_head,
- &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) {
+ if (unlikely(vhost_get_avail_head(vq, &ring_head, last_avail_idx))) {
vq_err(vq, "Failed to read head: idx %d address %p\n",
last_avail_idx,
&vq->avail->ring[last_avail_idx % vq->num]);
@@ -2156,8 +2218,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
i, vq->num, head);
return -EINVAL;
}
- ret = vhost_copy_from_user(vq, &desc, vq->desc + i,
- sizeof desc);
+ ret = vhost_get_desc(vq, &desc, i);
if (unlikely(ret)) {
vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",
i, vq->desc + i);
@@ -2250,7 +2311,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
- if (vhost_copy_to_user(vq, used, heads, count * sizeof *used)) {
+ if (vhost_put_used(vq, heads, start, count)) {
vq_err(vq, "Failed to write used");
return -EFAULT;
}
@@ -2292,8 +2353,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
/* Make sure buffer is written before we update index. */
smp_wmb();
- if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
- &vq->used->idx)) {
+ if (vhost_put_used_idx(vq)) {
vq_err(vq, "Failed to increment used idx");
return -EFAULT;
}
@@ -2326,7 +2386,7 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
__virtio16 flags;
- if (vhost_get_avail(vq, flags, &vq->avail->flags)) {
+ if (vhost_get_avail_flags(vq, &flags)) {
vq_err(vq, "Failed to get flags");
return true;
}
@@ -2340,7 +2400,7 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (unlikely(!v))
return true;
- if (vhost_get_avail(vq, event, vhost_used_event(vq))) {
+ if (vhost_get_used_event(vq, &event)) {
vq_err(vq, "Failed to get used event idx");
return true;
}
@@ -2385,7 +2445,7 @@ bool vhost_vq_avail_empty(struct vhost_dev *dev, struct vhost_virtqueue *vq)
if (vq->avail_idx != vq->last_avail_idx)
return false;
- r = vhost_get_avail(vq, avail_idx, &vq->avail->idx);
+ r = vhost_get_avail_idx(vq, &avail_idx);
if (unlikely(r))
return false;
vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
@@ -2421,7 +2481,7 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
/* They could have slipped one in as we were doing that: make
* sure it's written, then check again. */
smp_mb();
- r = vhost_get_avail(vq, avail_idx, &vq->avail->idx);
+ r = vhost_get_avail_idx(vq, &avail_idx);
if (r) {
vq_err(vq, "Failed to check avail idx at %p: %d\n",
&vq->avail->idx, r);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next V4 1/5] vhost: generalize adding used elem
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
Use one generic vhost_copy_to_user() instead of two dedicated
accessor. This will simplify the conversion to fine grain
accessors. About 2% improvement of PPS were seen during vitio-user
txonly test.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 15a216cdd507..14fad2577df3 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2250,16 +2250,7 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
start = vq->last_used_idx & (vq->num - 1);
used = vq->used->ring + start;
- if (count == 1) {
- if (vhost_put_user(vq, heads[0].id, &used->id)) {
- vq_err(vq, "Failed to write used id");
- return -EFAULT;
- }
- if (vhost_put_user(vq, heads[0].len, &used->len)) {
- vq_err(vq, "Failed to write used len");
- return -EFAULT;
- }
- } else if (vhost_copy_to_user(vq, used, heads, count * sizeof *used)) {
+ if (vhost_copy_to_user(vq, used, heads, count * sizeof *used)) {
vq_err(vq, "Failed to write used");
return -EFAULT;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next V4 0/5] vhost: accelerate metadata access through vmap()
From: Jason Wang @ 2019-01-23 9:55 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, jasowang; +Cc: kvm, mst
This series tries to access virtqueue metadata through kernel virtual
address instead of copy_user() friends since they had too much
overheads like checks, spec barriers or even hardware feature
toggling.
Test shows about 24% improvement on TX PPS. It should benefit other
cases as well.
Changes from V3:
- don't try to use vmap for file backed pages
- rebase to master
Changes from V2:
- fix buggy range overlapping check
- tear down MMU notifier during vhost ioctl to make sure invalidation
request can read metadata userspace address and vq size without
holding vq mutex.
Changes from V1:
- instead of pinning pages, use MMU notifier to invalidate vmaps and
remap duing metadata prefetch
- fix build warning on MIPS
Jason Wang (5):
vhost: generalize adding used elem
vhost: fine grain userspace memory accessors
vhost: rename vq_iotlb_prefetch() to vq_meta_prefetch()
vhost: introduce helpers to get the size of metadata area
vhost: access vq metadata through kernel virtual address
drivers/vhost/net.c | 4 +-
drivers/vhost/vhost.c | 441 +++++++++++++++++++++++++++++++++++++-----
drivers/vhost/vhost.h | 15 +-
mm/shmem.c | 1 +
4 files changed, 410 insertions(+), 51 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Tiwei Bie @ 2019-01-23 9:50 UTC (permalink / raw)
To: mst, jasowang, virtualization, linux-kernel, virtio-dev
This patch introduces the support for VIRTIO_F_ORDER_PLATFORM.
If this feature is negotiated, the driver must use the barriers
suitable for hardware devices. Otherwise, the device and driver
are assumed to be implemented in software, that is they can be
assumed to run on identical CPUs in an SMP configuration. Thus
a weaker form of memory barriers is sufficient to yield better
performance.
It is recommended that an add-in card based PCI device offers
this feature for portability. The device will fail to operate
further or will operate in a slower emulation mode if this
feature is offered but not accepted.
Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
---
v2:
- Add more explanations in commit log (MST);
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
* Re: [PATCH v7 0/7] Add virtio-iommu driver
From: Joerg Roedel @ 2019-01-23 8:34 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: mark.rutland, virtio-dev, lorenzo.pieralisi, tnowicki, mst,
marc.zyngier, linux-pci, will.deacon, robin.murphy,
virtualization, eric.auger, iommu, robh+dt, bhelgaas, kvmarm,
devicetree
In-Reply-To: <20190115121959.23763-1-jean-philippe.brucker@arm.com>
Hi Jean-Philippe,
thanks for all your hard work on this!
On Tue, Jan 15, 2019 at 12:19:52PM +0000, Jean-Philippe Brucker wrote:
> Implement the virtio-iommu driver, following specification v0.9 [1].
To make progress on this I think the spec needs to be close to something
that can be included into the official virtio-specification. Have you
proposed the specification for inclusion there?
This is because I can't merge a driver that might be incompatible to
future implementations because the specification needs to be changed on
its way to an official standard.
I had a short discussion with Michael S. Tsirkin about that and from
what I understood the spec needs to be proposed for inclusion on the
virtio-comment[1] mailing list and later the TC needs to vote on it.
Please work with Michael on this to get the specification official (or
at least pretty close to something that will be part of the official
virtio standard).
Regards,
Joerg
[1] https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio
^ permalink raw reply
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Tiwei Bie @ 2019-01-23 8:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-dev, linux-kernel, virtualization
In-Reply-To: <20190122230247-mutt-send-email-mst@kernel.org>
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
* Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain
From: hch @ 2019-01-23 7:12 UTC (permalink / raw)
To: Stefano Stabellini
Cc: jgross, Peng Fan, mst@redhat.com,
linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@infradead.org,
luto, xen-devel@lists.xenproject.org, boris.ostrovsky
In-Reply-To: <alpine.DEB.2.10.1901221113410.17936@sstabellini-ThinkPad-X260>
On Tue, Jan 22, 2019 at 11:59:31AM -0800, Stefano Stabellini wrote:
> > if (!virtio_has_iommu_quirk(vdev))
> > return true;
> >
> > @@ -260,7 +262,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > * the DMA API if we're a Xen guest, which at least allows
> > * all of the sensible Xen configurations to work correctly.
> > */
> > - if (xen_domain())
> > + if (xen_domain() && !dma_dev->dma_mem)
> > return true;
> >
> > return false;
>
> I can see you spotted a real issue, but this is not the right fix. We
> just need something a bit more flexible than xen_domain(): there are
> many kinds of Xen domains on different architectures, we basically want
> to enable this (return true from vring_use_dma_api) only when the xen
> swiotlb is meant to be used. Does the appended patch fix the issue you
> have?
The problem generally is the other way around - if dma_dev->dma_mem
is set the device decription in the device tree explicitly requires
using this memory, so we must _always_ use the DMA API.
The problem is just that that rproc driver absuses the DMA API
in horrible ways.
^ permalink raw reply
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Jason Wang @ 2019-01-23 6:56 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-dev, linux-kernel, virtualization
In-Reply-To: <20190122224203-mutt-send-email-mst@kernel.org>
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
* Re: [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Michael S. Tsirkin @ 2019-01-23 4:04 UTC (permalink / raw)
To: Tiwei Bie; +Cc: virtio-dev, linux-kernel, virtualization
In-Reply-To: <20190122170346.6279-1-tiwei.bie@intel.com>
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
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Michael S. Tsirkin @ 2019-01-23 3:49 UTC (permalink / raw)
To: Jason Wang; +Cc: virtio-dev, linux-kernel, virtualization
In-Reply-To: <ecf89b02-57ba-478f-1e28-4286888a4b26@redhat.com>
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
* Re: [virtio-dev] [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
From: Jason Wang @ 2019-01-23 3:08 UTC (permalink / raw)
To: Tiwei Bie, mst, virtualization, linux-kernel, virtio-dev
In-Reply-To: <20190122170346.6279-1-tiwei.bie@intel.com>
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
* Re: [Xen-devel] [RFC] virtio_ring: check dma_mem for xen_domain
From: Michael S. Tsirkin @ 2019-01-23 2:57 UTC (permalink / raw)
To: Stefano Stabellini
Cc: jgross, Peng Fan, linux-remoteproc@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, hch@infradead.org,
luto, xen-devel@lists.xenproject.org, boris.ostrovsky
In-Reply-To: <alpine.DEB.2.10.1901221113410.17936@sstabellini-ThinkPad-X260>
On Tue, Jan 22, 2019 at 11:59:31AM -0800, Stefano Stabellini wrote:
> On Mon, 21 Jan 2019, Peng Fan wrote:
> > on i.MX8QM, M4_1 is communicating with DomU using rpmsg with a fixed
> > address as the dma mem buffer which is predefined.
> >
> > Without this patch, the flow is:
> > vring_map_one_sg -> vring_use_dma_api
> > -> dma_map_page
> > -> __swiotlb_map_page
> > ->swiotlb_map_page
> > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, dev_addr)), size, dir);
> > However we are using per device dma area for rpmsg, phys_to_virt
> > could not return a correct virtual address for virtual address in
> > vmalloc area. Then kernel panic.
> >
> > With this patch, vring_use_dma_api will return false, and
> > vring_map_one_sg will return sg_phys(sg) which is the correct phys
> > address in the predefined memory region.
> > vring_map_one_sg -> vring_use_dma_api
> > -> sg_phys(sg)
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > drivers/virtio/virtio_ring.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd7e755484e3..8993d7cb3592 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -248,6 +248,8 @@ static inline bool virtqueue_use_indirect(struct virtqueue *_vq,
> >
> > static bool vring_use_dma_api(struct virtio_device *vdev)
> > {
> > + struct device *dma_dev = vdev->dev.parent;
> > +
> > if (!virtio_has_iommu_quirk(vdev))
> > return true;
> >
> > @@ -260,7 +262,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > * the DMA API if we're a Xen guest, which at least allows
> > * all of the sensible Xen configurations to work correctly.
> > */
> > - if (xen_domain())
> > + if (xen_domain() && !dma_dev->dma_mem)
> > return true;
> >
> > return false;
>
> I can see you spotted a real issue, but this is not the right fix. We
> just need something a bit more flexible than xen_domain(): there are
> many kinds of Xen domains on different architectures, we basically want
> to enable this (return true from vring_use_dma_api) only when the xen
> swiotlb is meant to be used. Does the appended patch fix the issue you
> have?
>
> ---
>
> xen: introduce xen_vring_use_dma
>
> From: Stefano Stabellini <stefanos@xilinx.com>
>
> Export xen_swiotlb on arm and arm64.
>
> Use xen_swiotlb to determine when vring should use dma APIs to map the
> ring: when xen_swiotlb is enabled the dma API is required. When it is
> disabled, it is not required.
>
> Reported-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
>
> diff --git a/arch/arm/include/asm/xen/swiotlb-xen.h b/arch/arm/include/asm/xen/swiotlb-xen.h
> new file mode 100644
> index 0000000..455ade5
> --- /dev/null
> +++ b/arch/arm/include/asm/xen/swiotlb-xen.h
> @@ -0,0 +1 @@
> +#include <xen/arm/swiotlb-xen.h>
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index cb44aa2..8592863 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -21,6 +21,8 @@
> #include <asm/xen/hypercall.h>
> #include <asm/xen/interface.h>
>
> +int xen_swiotlb __read_mostly;
> +
> unsigned long xen_get_swiotlb_free_pages(unsigned int order)
> {
> struct memblock_region *reg;
> @@ -189,6 +191,7 @@ int __init xen_mm_init(void)
> struct gnttab_cache_flush cflush;
> if (!xen_initial_domain())
> return 0;
> + xen_swiotlb = 1;
> xen_swiotlb_init(1, false);
> xen_dma_ops = &xen_swiotlb_dma_ops;
>
> diff --git a/arch/arm64/include/asm/xen/swiotlb-xen.h b/arch/arm64/include/asm/xen/swiotlb-xen.h
> new file mode 100644
> index 0000000..455ade5
> --- /dev/null
> +++ b/arch/arm64/include/asm/xen/swiotlb-xen.h
> @@ -0,0 +1 @@
> +#include <xen/arm/swiotlb-xen.h>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755..bf8badc 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -260,7 +260,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> * the DMA API if we're a Xen guest, which at least allows
> * all of the sensible Xen configurations to work correctly.
> */
> - if (xen_domain())
> + if (xen_vring_use_dma())
> return true;
>
> return false;
> diff --git a/include/xen/arm/swiotlb-xen.h b/include/xen/arm/swiotlb-xen.h
> new file mode 100644
> index 0000000..2aac7c4
> --- /dev/null
> +++ b/include/xen/arm/swiotlb-xen.h
> @@ -0,0 +1,10 @@
> +#ifndef _ASM_ARM_XEN_SWIOTLB_XEN_H
> +#define _ASM_ARM_XEN_SWIOTLB_XEN_H
> +
> +#ifdef CONFIG_SWIOTLB_XEN
> +extern int xen_swiotlb;
> +#else
> +#define xen_swiotlb (0)
> +#endif
> +
> +#endif
> diff --git a/include/xen/xen.h b/include/xen/xen.h
> index 0e21567..74a536d 100644
> --- a/include/xen/xen.h
> +++ b/include/xen/xen.h
> @@ -46,4 +46,10 @@ enum xen_domain_type {
> bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
> const struct bio_vec *vec2);
>
> +#include <asm/xen/swiotlb-xen.h>
> +static inline int xen_vring_use_dma(void)
> +{
> + return !!xen_swiotlb;
Given xen_swiotlb is only defined on arm, how will
this build on other architectures?
> +}
> +
> #endif /* _XEN_XEN_H */
I'd say at this point, I'm sorry we didn't come up with PLATFORM_ACCESS
when we added the xen hack.
I'm not objecting to this patch but I would also like to bypass the xen
hack for VIRTIO 1 devices. In particular I know rpmsg is still virtio 0
so it's not an issue for it. Is Xen already using VIRTIO 1, and without
setting PLATFORM_ACCESS? If not I would like to teach
vring_use_dma_api to return false on VIRTIO_1 && !PLATFORM_ACCESS.
Would that be acceptable?
--
MST
^ permalink raw reply
* [PATCH] virtio: support VIRTIO_F_ORDER_PLATFORM
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
* CFP PECCS 2019 - 9th Int.l Conf. on Pervasive and Embedded Computing and Communication Systems (Vienna/Austria)
From: peccs @ 2019-01-22 15:22 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
9th International Conference on Pervasive and Embedded Computing and Communication Systems
Submission Deadline: April 29, 2019
http://www.peccs.org/
September 19 - 20, 2019
Vienna, Austria.
PECCS is organized in 5 major tracks:
- Wireless and Mobile Technologies
- Sensor Networks: Software, Architectures and Applications
- Intelligent Data Analysis and Processing
- Mobile Computing Systems and Services
- Multimedia Signal Processing
Proceedings will be submitted for indexation by: DBLP, Thomson Reuters, EI, SCOPUS, Semantic Scholar and Google Scholar.
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don't hesitate contacting me.
Kind regards,
PECCS Secretariat
Address: Avenida de São Francisco Xavier Lote 7L Cave
2910-595 Setubal, Portugal
Tel: +351 265 520 185
Fax: +351 265 520 186
Web: http://www.peccs.org/
e-mail: peccs.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* CFP WEBIST 2019 - 14th Int.l Conf. on Web Information Systems and Technologies (Vienna/Austria)
From: webist @ 2019-01-22 15:22 UTC (permalink / raw)
To: virtualization
SUBMISSION DEADLINE
14th International Conference on Web Information Systems and Technologies
Submission Deadline: April 29, 2019
http://www.webist.org/
September 18 - 20, 2019
Vienna, Austria.
WEBIST is organized in 5 major tracks:
- Internet Technology
- Mobile and NLP Information Systems
- Service Based Information Systems, Platforms and Eco-Systems
- Web Intelligence
- Web Interfaces
Proceedings will be submitted for indexation by: DBLP, Thomson Reuters, EI, SCOPUS, Semantic Scholar and Google Scholar.
A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
Should you have any question please don't hesitate contacting me.
Kind regards,
WEBIST Secretariat
Address: Avenida de São Francisco Xavier Lote 7L Cave
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://www.webist.org/
e-mail: webist.secretariat@insticc.org
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] drm: Split out drm_probe_helper.h
From: Daniel Vetter @ 2019-01-22 8:03 UTC (permalink / raw)
To: Sam Ravnborg
Cc: Neil Armstrong, Nouveau Dev, Liviu Dudau, DRI Development,
open list:VIRTIO CORE, NET..., Laurent Pinchart, Daniel Vetter,
linux-stm32, linux-samsung-soc, Oleksandr Andrushchenko,
amd-gfx list, open list:ARM/Rockchip SoC...,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Jani Nikula,
linux-arm-msm, intel-gfx, The etnaviv authors
In-Reply-To: <20190121221329.GA6512@ravnborg.org>
On Mon, Jan 21, 2019 at 11:13 PM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Daniel et al.
>
> > >
> > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy
> > > kms drivers. Just removing it from all the atomic drivers caused lots of
> > > fallout, I expect even more if you entirely remove the includes it has.
> > > Maybe a todo, care to pls create that patch since it's your idea?
> >
> > The main reason I bailed out initially was that this would create
> > small changes to several otherwise seldomly touched files.
> > And then we would later come and remove drmP.h - so lots of
> > small but incremental changes to the same otherwise seldomly
> > edited files.
> > And the job was only partially done.
> >
> > I will try to experiment with an approach where I clean up the
> > include/drm/*.h files a little (like suggested above, +delete drmP.h
> > and maybe a bit more).
> >
> > Then to try on a driver by driver basis to make it build with a
> > cleaned set of include files.
> > I hope that the cleaned up driver can still build without the
> > cleaned header files so the changes can be submitted piecemal.
> >
> > Will do so with an eye on the lesser maintained drivers to try it
> > out to avoid creating too much chrunch for others.
>
> I have now a few patches queued, but the result is not too pretty.
> I did the following:
>
> - For all files in include/drm/*.h the set of include files
> were adjusted to the minimum number of files required to make
> them build without any other files included first.
>
> Created one .c file for each .h file. Then included the .h
> file and adjusted to the minimal set of include files.
> In the process a lot of forwards were added.
>
> - Deleted drmP.h
>
> - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via
>
> Some observations:
>
> - Killing all the includes not needed in the headers files
> results in a a lot of extra changes.
> Examples:
> drm_modseset_helper_vtables.h is no longer
> included by anyone, so needs to be added in many files
>
> drm_atomic_state_helper.h is no longer included
> by anyone so likewise needs to be added in many files
>
> - It is very tedious to do this properly.
> The process I followed was:
> - delete / comment out all include files
> - add back the obvious from a quick scan of the code
> - build - fix - build - fix - build - fix ...
> - next file...
>
> - The result is errorprone as only the allyesconfig + allmodconfig
> variants are tested. But reallife configurations are more diverse.
>
> Current diffstat:
> 111 files changed, 771 insertions(+), 401 deletions(-)
>
> This is for the 5 drivers alone and not the header cleanup.
> So long story short - this is not good and not the way forward.
>
> I will try to come up with a few improvements to make the
> headers files selfcontained, but restricted to the changes that
> add forwards/include to avoid the chrunch in all the drivers.
>
> And then post for review a few patches to clean up some headers.
> If the cleanup gets a go I will try to persuade the introduction
> of these.
> This will include, but will not be limited to, the above mentioned
> drm_crtc_helper.h header file.
>
> For now too much time was already spent on this, so it is at the
> moment pushed back on my TODO list.
> This mail serve also as a kind of "where had I left", when/if I
> pick this up again.
>
> If there are anyone that knows some tooling that can help in the
> process of adjusting the header files I am all ears.
Yeah in the process of splitting up drmP.h we've created a few smaller
such piles of headers. I think in some cases it's just not going to be
worth it to fully split them up, e.g. drm_crtc_helper.h is going to be
a pure legacy helper, only needed by pre-atomic drivers. Splitting
that up doesn't seem to useful to me. Similarly we might want
drm_atomic_helper.h to keep pulling in the other helper headers. So
probably going to be a judgement call on a case-by-case basis.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* Re: [RFC] virtio_ring: check dma_mem for xen_domain
From: Michael S. Tsirkin @ 2019-01-22 2:36 UTC (permalink / raw)
To: hch@infradead.org
Cc: Peng Fan, sstabellini@kernel.org,
linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
xen-devel@lists.xenproject.org
In-Reply-To: <20190121082830.GC12420@infradead.org>
On Mon, Jan 21, 2019 at 12:28:30AM -0800, hch@infradead.org wrote:
> On Mon, Jan 21, 2019 at 04:51:57AM +0000, Peng Fan wrote:
> > on i.MX8QM, M4_1 is communicating with DomU using rpmsg with a fixed
> > address as the dma mem buffer which is predefined.
> >
> > Without this patch, the flow is:
> > vring_map_one_sg -> vring_use_dma_api
> > -> dma_map_page
> > -> __swiotlb_map_page
> > ->swiotlb_map_page
> > ->__dma_map_area(phys_to_virt(dma_to_phys(dev, dev_addr)), size, dir);
> > However we are using per device dma area for rpmsg, phys_to_virt
> > could not return a correct virtual address for virtual address in
> > vmalloc area. Then kernel panic.
>
> And that is the right thing to do. You must not call dma_map_* on
> memory that was allocated from dma_alloc_*.
>
> We actually have another thread which appears to be for this same issue.
Sorry, which thread do you refer to?
--
MST
^ permalink raw reply
* Re: [PATCH] drm: Split out drm_probe_helper.h
From: Sam Ravnborg @ 2019-01-21 22:13 UTC (permalink / raw)
To: Daniel Vetter
Cc: Neil Armstrong, Daniel Vetter, Liviu Dudau, DRI Development,
virtualization, Laurent Pinchart, Daniel Vetter, linux-stm32,
linux-samsung-soc, Oleksandr Andrushchenko, amd-gfx,
linux-rockchip, nouveau, spice-devel, Jani Nikula, linux-arm-msm,
intel-gfx, etnaviv, linux-mediatek, Rodrigo Vivi, linux-tegra,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20190117174531.GA14041@ravnborg.org>
Hi Daniel et al.
> >
> > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy
> > kms drivers. Just removing it from all the atomic drivers caused lots of
> > fallout, I expect even more if you entirely remove the includes it has.
> > Maybe a todo, care to pls create that patch since it's your idea?
>
> The main reason I bailed out initially was that this would create
> small changes to several otherwise seldomly touched files.
> And then we would later come and remove drmP.h - so lots of
> small but incremental changes to the same otherwise seldomly
> edited files.
> And the job was only partially done.
>
> I will try to experiment with an approach where I clean up the
> include/drm/*.h files a little (like suggested above, +delete drmP.h
> and maybe a bit more).
>
> Then to try on a driver by driver basis to make it build with a
> cleaned set of include files.
> I hope that the cleaned up driver can still build without the
> cleaned header files so the changes can be submitted piecemal.
>
> Will do so with an eye on the lesser maintained drivers to try it
> out to avoid creating too much chrunch for others.
I have now a few patches queued, but the result is not too pretty.
I did the following:
- For all files in include/drm/*.h the set of include files
were adjusted to the minimum number of files required to make
them build without any other files included first.
Created one .c file for each .h file. Then included the .h
file and adjusted to the minimal set of include files.
In the process a lot of forwards were added.
- Deleted drmP.h
- Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via
Some observations:
- Killing all the includes not needed in the headers files
results in a a lot of extra changes.
Examples:
drm_modseset_helper_vtables.h is no longer
included by anyone, so needs to be added in many files
drm_atomic_state_helper.h is no longer included
by anyone so likewise needs to be added in many files
- It is very tedious to do this properly.
The process I followed was:
- delete / comment out all include files
- add back the obvious from a quick scan of the code
- build - fix - build - fix - build - fix ...
- next file...
- The result is errorprone as only the allyesconfig + allmodconfig
variants are tested. But reallife configurations are more diverse.
Current diffstat:
111 files changed, 771 insertions(+), 401 deletions(-)
This is for the 5 drivers alone and not the header cleanup.
So long story short - this is not good and not the way forward.
I will try to come up with a few improvements to make the
headers files selfcontained, but restricted to the changes that
add forwards/include to avoid the chrunch in all the drivers.
And then post for review a few patches to clean up some headers.
If the cleanup gets a go I will try to persuade the introduction
of these.
This will include, but will not be limited to, the above mentioned
drm_crtc_helper.h header file.
For now too much time was already spent on this, so it is at the
moment pushed back on my TODO list.
This mail serve also as a kind of "where had I left", when/if I
pick this up again.
If there are anyone that knows some tooling that can help in the
process of adjusting the header files I am all ears.
Sam
^ permalink raw reply
* Re: [PATCH 0/3] virtio-ccw: updates
From: Michael S. Tsirkin @ 2019-01-21 19:30 UTC (permalink / raw)
To: Cornelia Huck; +Cc: Halil Pasic, linux-s390, kvm, virtualization
In-Reply-To: <20190121121944.12309-1-cohuck@redhat.com>
On Mon, Jan 21, 2019 at 01:19:41PM +0100, Cornelia Huck wrote:
> [Now not as pull request, as requested. The first patch had already been
> in my last pull request. Patches are against master, but should apply
> basically anywhere.
>
> Git branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git virtio-ccw]
>
> Some updates: documentation, gracefully handle invalid queues, and wire
> up the ->bus_name callback (which had somehow fallen through the cracks.)
Thanks, will queue.
> Cornelia Huck (2):
> virtio-ccw: diag 500 may return a negative cookie
> virtio-ccw: wire up ->bus_name callback
>
> Halil Pasic (1):
> s390/virtio: handle find on invalid queue gracefully
>
> Documentation/virtual/kvm/s390-diag.txt | 3 ++-
> drivers/s390/virtio/virtio_ccw.c | 12 +++++++++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> --
> 2.17.2
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox