date:20190808

[PATCH v3 1/2] virtio_console: free unused buffers with port delete

2019-08-08 Thread Pankaj Gupta

The commit a7a69ec0d8e4 ("virtio_console: free buffers after reset")
deferred detaching of unused buffer to virtio device unplug time.
This causes unplug/replug of single port in virtio device with an
error "Error allocating inbufs\n". As we don't free the unused buffers
attached with the port. Re-plug the same port tries to allocate new
buffers in virtqueue and results in this error if queue is full.
This patch removes the unused buffers in vq's when we unplug the port.
This is the best we can do as we cannot call device_reset because virtio
device is still active.

Reported-by: Xiaohui Li 
Fixes: a7a69ec0d8e4 ("virtio_console: free buffers after reset")
Cc: sta...@vger.kernel.org
Signed-off-by: Pankaj Gupta 
---
 drivers/char/virtio_console.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7270e7b69262..e8be82f1bae9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1494,15 +1494,25 @@ static void remove_port(struct kref *kref)
kfree(port);
 }
 
+static void remove_unused_bufs(struct virtqueue *vq)
+{
+   struct port_buffer *buf;
+
+   while ((buf = virtqueue_detach_unused_buf(vq)))
+   free_buf(buf, true);
+}
+
 static void remove_port_data(struct port *port)
 {
spin_lock_irq(&port->inbuf_lock);
/* Remove unused data this port might have received. */
discard_port_data(port);
+   remove_unused_bufs(port->in_vq);
spin_unlock_irq(&port->inbuf_lock);
 
spin_lock_irq(&port->outvq_lock);
reclaim_consumed_buffers(port);
+   remove_unused_bufs(port->out_vq);
spin_unlock_irq(&port->outvq_lock);
 }
 
@@ -1938,11 +1948,9 @@ static void remove_vqs(struct ports_device *portdev)
struct virtqueue *vq;
 
virtio_device_for_each_vq(portdev->vdev, vq) {
-   struct port_buffer *buf;
 
flush_bufs(vq, true);
-   while ((buf = virtqueue_detach_unused_buf(vq)))
-   free_buf(buf, true);
+   remove_unused_bufs(vq);
}
portdev->vdev->config->del_vqs(portdev->vdev);
kfree(portdev->in_vqs);
-- 
2.21.0

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

[PATCH v3 2/2] virtio: decrement avail idx with buffer detach for packed ring

2019-08-08 Thread Pankaj Gupta

This patch decrements 'next_avail_idx' count when detaching a buffer
from vq for packed ring code. Split ring code already does this in
virtqueue_detach_unused_buf_split function. This updates the
'next_avail_idx' to the previous correct index after an unused buffer
is detatched from the vq.

Signed-off-by: Pankaj Gupta 
---
 drivers/virtio/virtio_ring.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c8be1c4f5b55..7c69181113e2 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1537,6 +1537,12 @@ static void *virtqueue_detach_unused_buf_packed(struct 
virtqueue *_vq)
/* detach_buf clears data, so grab it now. */
buf = vq->packed.desc_state[i].data;
detach_buf_packed(vq, i, NULL);
+   vq->packed.next_avail_idx--;
+   if (vq->packed.next_avail_idx < 0) {
+   vq->packed.next_avail_idx = vq->packed.vring.num - 1;
+   vq->packed.avail_wrap_counter ^= 1;
+   }
+
END_USE(vq);
return buf;
}
-- 
2.20.1

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

[PATCH v3 0/2] virtio_console: fix replug of virtio console port

2019-08-08 Thread Pankaj Gupta

This patch series fixes the issue with unplug/replug of a port in virtio
console driver which fails with an error "Error allocating inbufs\n".
Patch 1 makes use of 'virtqueue_detach_unused_buf' function to detach
the unused buffers during port hotunplug time.
Patch 2 updates the next avail index for packed ring code.
Tested the packed ring code with the qemu virtio 1.1 device code posted
here [1].

Changes from v2
 Better change log in patch2 - [Greg]
Changes from v1[2]
 Make virtio packed ring code compatible with split ring - [Michael]

[1]  https://marc.info/?l=qemu-devel&m=156471883703948&w=2
[2]  https://lkml.org/lkml/2019/3/4/517

Pankaj Gupta (2):
  virtio_console: free unused buffers with port delete
  virtio: decrement avail idx with buffer detach for packed ring

 char/virtio_console.c |   14 +++---
 virtio/virtio_ring.c  |6 ++
 2 files changed, 17 insertions(+), 3 deletions(-)
-- 
2.21.0

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

[PATCH V5 8/9] vhost: correctly set dirty pages in MMU notifiers callback

2019-08-08 Thread Jason Wang

We need make sure there's no reference on the map before trying to
mark set dirty pages.

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 29e8abe694f7..d8863aaaf0f6 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -386,13 +386,12 @@ static void vhost_invalidate_vq_start(struct 
vhost_virtqueue *vq,
++vq->invalidate_count;
 
map = vq->maps[index];
-   if (map) {
+   if (map)
vq->maps[index] = NULL;
-   vhost_set_map_dirty(vq, map, index);
-   }
spin_unlock(&vq->mmu_lock);
 
if (map) {
+   vhost_set_map_dirty(vq, map, index);
vhost_map_unprefetch(map);
}
 }
-- 
2.18.1

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

[PATCH V5 7/9] vhost: do not use RCU to synchronize MMU notifier with worker

2019-08-08 Thread Jason Wang

We used to use RCU to synchronize MMU notifier with worker. This leads
calling synchronize_rcu() in invalidate_range_start(). But on a busy
system, there would be many factors that may slow down the
synchronize_rcu() which makes it unsuitable to be called in MMU
notifier. This path switch to use a simple spinlock to do the
synchronization.

Benchmark was done through testpmd + vhost_net + XDP_DROP on
tap. Compare to copy_{to|from}_user() path, on Sandy Bridge (without
SMAP support), 1.5% PPS improvement was measured; on Broadwell (with
SMAP and enabled), 14% PPS improvement was measured.

This means we are not as fast as what 7f466032dc9e did because the
spinlock overhead in the datapath. This needs to be addressed in the
future.

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 115 ++
 drivers/vhost/vhost.h |   5 +-
 2 files changed, 62 insertions(+), 58 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index cfc11f9ed9c9..29e8abe694f7 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -324,17 +324,16 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue 
*vq)
 
spin_lock(&vq->mmu_lock);
for (i = 0; i < VHOST_NUM_ADDRS; i++) {
-   map[i] = rcu_dereference_protected(vq->maps[i],
- lockdep_is_held(&vq->mmu_lock));
+   map[i] = vq->maps[i];
if (map[i]) {
vhost_set_map_dirty(vq, map[i], i);
-   rcu_assign_pointer(vq->maps[i], NULL);
+   vq->maps[i] = NULL;
}
}
spin_unlock(&vq->mmu_lock);
 
-   /* No need for synchronize_rcu() or kfree_rcu() since we are
-* serialized with memory accessors (e.g vq mutex held).
+   /* No need for synchronization since we are serialized with
+* memory accessors (e.g vq mutex held).
 */
 
for (i = 0; i < VHOST_NUM_ADDRS; i++)
@@ -362,6 +361,16 @@ static bool vhost_map_range_overlap(struct vhost_uaddr 
*uaddr,
return !(end < uaddr->uaddr || start > uaddr->uaddr - 1 + uaddr->size);
 }
 
+static void inline vhost_vq_access_map_begin(struct vhost_virtqueue *vq)
+{
+   spin_lock(&vq->mmu_lock);
+}
+
+static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
+{
+   spin_unlock(&vq->mmu_lock);
+}
+
 static void vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
  int index,
  unsigned long start,
@@ -376,16 +385,14 @@ static void vhost_invalidate_vq_start(struct 
vhost_virtqueue *vq,
spin_lock(&vq->mmu_lock);
++vq->invalidate_count;
 
-   map = rcu_dereference_protected(vq->maps[index],
-   lockdep_is_held(&vq->mmu_lock));
+   map = vq->maps[index];
if (map) {
+   vq->maps[index] = NULL;
vhost_set_map_dirty(vq, map, index);
-   rcu_assign_pointer(vq->maps[index], NULL);
}
spin_unlock(&vq->mmu_lock);
 
if (map) {
-   synchronize_rcu();
vhost_map_unprefetch(map);
}
 }
@@ -457,7 +464,7 @@ static void vhost_init_maps(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; ++i) {
vq = dev->vqs[i];
for (j = 0; j < VHOST_NUM_ADDRS; j++)
-   RCU_INIT_POINTER(vq->maps[j], NULL);
+   vq->maps[j] = NULL;
}
 }
 #endif
@@ -921,7 +928,7 @@ static int vhost_map_prefetch(struct vhost_virtqueue *vq,
map->npages = npages;
map->pages = pages;
 
-   rcu_assign_pointer(vq->maps[index], map);
+   vq->maps[index] = map;
/* No need for a synchronize_rcu(). This function should be
 * called by dev->worker so we are serialized with all
 * readers.
@@ -1216,18 +1223,18 @@ static inline int vhost_put_avail_event(struct 
vhost_virtqueue *vq)
struct vring_used *used;
 
if (!vq->iotlb) {
-   rcu_read_lock();
+   vhost_vq_access_map_begin(vq);
 
-   map = rcu_dereference(vq->maps[VHOST_ADDR_USED]);
+   map = vq->maps[VHOST_ADDR_USED];
if (likely(map)) {
used = map->addr;
*((__virtio16 *)&used->ring[vq->num]) =
cpu_to_vhost16(vq, vq->avail_idx);
-   rcu_read_unlock();
+   vhost_vq_access_map_end(vq);
return 0;
}
 
-   rcu_read_unlock();
+   vhost_vq_access_map_end(vq);
}
 #endif
 
@@ -1245,18 +1252,18 @@ static inline int vhost_put_used(struct vhost_virtqueue 
*vq,
size_t size;
 
if (!vq->iotlb) {
-   rcu_read_

[PATCH V5 9/9] vhost: do not return -EAGAIN for non blocking invalidation too early

2019-08-08 Thread Jason Wang

Instead of returning -EAGAIN unconditionally, we'd better do that only
we're sure the range is overlapped with the metadata area.

Reported-by: Jason Gunthorpe 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 32 +++-
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d8863aaaf0f6..f98155f28f02 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -371,16 +371,19 @@ static void inline vhost_vq_access_map_end(struct 
vhost_virtqueue *vq)
spin_unlock(&vq->mmu_lock);
 }
 
-static void vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
- int index,
- unsigned long start,
- unsigned long end)
+static int vhost_invalidate_vq_start(struct vhost_virtqueue *vq,
+int index,
+unsigned long start,
+unsigned long end,
+bool blockable)
 {
struct vhost_uaddr *uaddr = &vq->uaddrs[index];
struct vhost_map *map;
 
if (!vhost_map_range_overlap(uaddr, start, end))
-   return;
+   return 0;
+   else if (!blockable)
+   return -EAGAIN;
 
spin_lock(&vq->mmu_lock);
++vq->invalidate_count;
@@ -394,6 +397,8 @@ static void vhost_invalidate_vq_start(struct 
vhost_virtqueue *vq,
vhost_set_map_dirty(vq, map, index);
vhost_map_unprefetch(map);
}
+
+   return 0;
 }
 
 static void vhost_invalidate_vq_end(struct vhost_virtqueue *vq,
@@ -414,18 +419,19 @@ static int vhost_invalidate_range_start(struct 
mmu_notifier *mn,
 {
struct vhost_dev *dev = container_of(mn, struct vhost_dev,
 mmu_notifier);
-   int i, j;
-
-   if (!mmu_notifier_range_blockable(range))
-   return -EAGAIN;
+   bool blockable = mmu_notifier_range_blockable(range);
+   int i, j, ret;
 
for (i = 0; i < dev->nvqs; i++) {
struct vhost_virtqueue *vq = dev->vqs[i];
 
-   for (j = 0; j < VHOST_NUM_ADDRS; j++)
-   vhost_invalidate_vq_start(vq, j,
- range->start,
- range->end);
+   for (j = 0; j < VHOST_NUM_ADDRS; j++) {
+   ret = vhost_invalidate_vq_start(vq, j,
+   range->start,
+   range->end, blockable);
+   if (ret)
+   return ret;
+   }
}
 
return 0;
-- 
2.18.1

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

[PATCH V5 5/9] vhost: mark dirty pages during map uninit

2019-08-08 Thread Jason Wang

We don't mark dirty pages if the map was teared down outside MMU
notifier. This will lead untracked dirty pages. Fixing by marking
dirty pages during map uninit.

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2a7217c33668..c12cdadb0855 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -305,6 +305,18 @@ static void vhost_map_unprefetch(struct vhost_map *map)
kfree(map);
 }
 
+static void vhost_set_map_dirty(struct vhost_virtqueue *vq,
+   struct vhost_map *map, int index)
+{
+   struct vhost_uaddr *uaddr = &vq->uaddrs[index];
+   int i;
+
+   if (uaddr->write) {
+   for (i = 0; i < map->npages; i++)
+   set_page_dirty(map->pages[i]);
+   }
+}
+
 static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq)
 {
struct vhost_map *map[VHOST_NUM_ADDRS];
@@ -314,8 +326,10 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue 
*vq)
for (i = 0; i < VHOST_NUM_ADDRS; i++) {
map[i] = rcu_dereference_protected(vq->maps[i],
  lockdep_is_held(&vq->mmu_lock));
-   if (map[i])
+   if (map[i]) {
+   vhost_set_map_dirty(vq, map[i], i);
rcu_assign_pointer(vq->maps[i], NULL);
+   }
}
spin_unlock(&vq->mmu_lock);
 
@@ -353,7 +367,6 @@ static void vhost_invalidate_vq_start(struct 
vhost_virtqueue *vq,
 {
struct vhost_uaddr *uaddr = &vq->uaddrs[index];
struct vhost_map *map;
-   int i;
 
if (!vhost_map_range_overlap(uaddr, start, end))
return;
@@ -364,10 +377,7 @@ static void vhost_invalidate_vq_start(struct 
vhost_virtqueue *vq,
map = rcu_dereference_protected(vq->maps[index],
lockdep_is_held(&vq->mmu_lock));
if (map) {
-   if (uaddr->write) {
-   for (i = 0; i < map->npages; i++)
-   set_page_dirty(map->pages[i]);
-   }
+   vhost_set_map_dirty(vq, map, index);
rcu_assign_pointer(vq->maps[index], NULL);
}
spin_unlock(&vq->mmu_lock);
-- 
2.18.1

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

[PATCH V5 3/9] vhost: fix vhost map leak

2019-08-08 Thread Jason Wang

We don't free map during vhost_map_unprefetch(). This means it could
be leaked. Fixing by free the map.

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 17f6abea192e..2a3154976277 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -302,9 +302,7 @@ static void vhost_vq_meta_reset(struct vhost_dev *d)
 static void vhost_map_unprefetch(struct vhost_map *map)
 {
kfree(map->pages);
-   map->pages = NULL;
-   map->npages = 0;
-   map->addr = NULL;
+   kfree(map);
 }
 
 static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq)
-- 
2.18.1

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

[PATCH V5 0/9] Fixes for vhost metadata acceleration

2019-08-08 Thread Jason Wang

Hi all:

This series try to fix several issues introduced by meta data
accelreation series. Please review.

Changes from V4:
- switch to use spinlock synchronize MMU notifier with accessors

Changes from V3:
- remove the unnecessary patch

Changes from V2:
- use seqlck helper to synchronize MMU notifier with vhost worker

Changes from V1:
- try not use RCU to syncrhonize MMU notifier with vhost worker
- set dirty pages after no readers
- return -EAGAIN only when we find the range is overlapped with
  metadata

Jason Wang (9):
  vhost: don't set uaddr for invalid address
  vhost: validate MMU notifier registration
  vhost: fix vhost map leak
  vhost: reset invalidate_count in vhost_set_vring_num_addr()
  vhost: mark dirty pages during map uninit
  vhost: don't do synchronize_rcu() in vhost_uninit_vq_maps()
  vhost: do not use RCU to synchronize MMU notifier with worker
  vhost: correctly set dirty pages in MMU notifiers callback
  vhost: do not return -EAGAIN for non blocking invalidation too early

 drivers/vhost/vhost.c | 202 +-
 drivers/vhost/vhost.h |   6 +-
 2 files changed, 122 insertions(+), 86 deletions(-)

-- 
2.18.1

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

[PATCH V5 6/9] vhost: don't do synchronize_rcu() in vhost_uninit_vq_maps()

2019-08-08 Thread Jason Wang

There's no need for RCU synchronization in vhost_uninit_vq_maps()
since we've already serialized with readers (memory accessors). This
also avoid the possible userspace DOS through ioctl() because of the
possible high latency caused by synchronize_rcu().

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c12cdadb0855..cfc11f9ed9c9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -333,7 +333,9 @@ static void vhost_uninit_vq_maps(struct vhost_virtqueue *vq)
}
spin_unlock(&vq->mmu_lock);
 
-   synchronize_rcu();
+   /* No need for synchronize_rcu() or kfree_rcu() since we are
+* serialized with memory accessors (e.g vq mutex held).
+*/
 
for (i = 0; i < VHOST_NUM_ADDRS; i++)
if (map[i])
-- 
2.18.1

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

[PATCH V5 1/9] vhost: don't set uaddr for invalid address

2019-08-08 Thread Jason Wang

We should not setup uaddr for the invalid address, otherwise we may
try to pin or prefetch mapping of wrong pages.

Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 0536f8526359..488380a581dc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2082,7 +2082,8 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
}
 
 #if VHOST_ARCH_CAN_ACCEL_UACCESS
-   vhost_setup_vq_uaddr(vq);
+   if (r == 0)
+   vhost_setup_vq_uaddr(vq);
 
if (d->mm)
mmu_notifier_register(&d->mmu_notifier, d->mm);
-- 
2.18.1

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

[PATCH V5 2/9] vhost: validate MMU notifier registration

2019-08-08 Thread Jason Wang

The return value of mmu_notifier_register() is not checked in
vhost_vring_set_num_addr(). This will cause an out of sync between mm
and MMU notifier thus a double free. To solve this, introduce a
boolean flag to track whether MMU notifier is registered and only do
unregistering when it was true.

Reported-and-tested-by:
syzbot+e58112d71f77113dd...@syzkaller.appspotmail.com
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 19 +++
 drivers/vhost/vhost.h |  1 +
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 488380a581dc..17f6abea192e 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -629,6 +629,7 @@ void vhost_dev_init(struct vhost_dev *dev,
dev->iov_limit = iov_limit;
dev->weight = weight;
dev->byte_weight = byte_weight;
+   dev->has_notifier = false;
init_llist_head(&dev->work_list);
init_waitqueue_head(&dev->wait);
INIT_LIST_HEAD(&dev->read_list);
@@ -730,6 +731,7 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
if (err)
goto err_mmu_notifier;
 #endif
+   dev->has_notifier = true;
 
return 0;
 
@@ -959,7 +961,11 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
}
if (dev->mm) {
 #if VHOST_ARCH_CAN_ACCEL_UACCESS
-   mmu_notifier_unregister(&dev->mmu_notifier, dev->mm);
+   if (dev->has_notifier) {
+   mmu_notifier_unregister(&dev->mmu_notifier,
+   dev->mm);
+   dev->has_notifier = false;
+   }
 #endif
mmput(dev->mm);
}
@@ -2064,8 +2070,10 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
/* Unregister MMU notifer to allow invalidation callback
 * can access vq->uaddrs[] without holding a lock.
 */
-   if (d->mm)
+   if (d->has_notifier) {
mmu_notifier_unregister(&d->mmu_notifier, d->mm);
+   d->has_notifier = false;
+   }
 
vhost_uninit_vq_maps(vq);
 #endif
@@ -2085,8 +2093,11 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
if (r == 0)
vhost_setup_vq_uaddr(vq);
 
-   if (d->mm)
-   mmu_notifier_register(&d->mmu_notifier, d->mm);
+   if (d->mm) {
+   r = mmu_notifier_register(&d->mmu_notifier, d->mm);
+   if (!r)
+   d->has_notifier = true;
+   }
 #endif
 
mutex_unlock(&vq->mutex);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 42a8c2a13ab1..a9a2a93857d2 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -214,6 +214,7 @@ struct vhost_dev {
int iov_limit;
int weight;
int byte_weight;
+   bool has_notifier;
 };
 
 bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
-- 
2.18.1

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

[PATCH V5 4/9] vhost: reset invalidate_count in vhost_set_vring_num_addr()

2019-08-08 Thread Jason Wang

The vhost_set_vring_num_addr() could be called in the middle of
invalidate_range_start() and invalidate_range_end(). If we don't reset
invalidate_count after the un-registering of MMU notifier, the
invalidate_cont will run out of sync (e.g never reach zero). This will
in fact disable the fast accessor path. Fixing by reset the count to
zero.

Reported-by: Michael S. Tsirkin 
Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel virtual address")
Signed-off-by: Jason Wang 
---
 drivers/vhost/vhost.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2a3154976277..2a7217c33668 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2073,6 +2073,10 @@ static long vhost_vring_set_num_addr(struct vhost_dev *d,
d->has_notifier = false;
}
 
+   /* reset invalidate_count in case we are in the middle of
+* invalidate_start() and invalidate_end().
+*/
+   vq->invalidate_count = 0;
vhost_uninit_vq_maps(vq);
 #endif
 
-- 
2.18.1

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

Re: [PATCH V4 0/9] Fixes for metadata accelreation

2019-08-08 Thread Jason Wang

On 2019/8/9 下午1:15, David Miller wrote:

From: Jason Wang 
Date: Wed,  7 Aug 2019 03:06:08 -0400

This series try to fix several issues introduced by meta data
accelreation series. Please review.

  ...

My impression is that patch #7 will be changed to use spinlocks so there
will be a v5.

Yes. V5 is on the way.

Thanks

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

Re: [PATCH V4 0/9] Fixes for metadata accelreation

2019-08-08 Thread David Miller

From: Jason Wang 
Date: Wed,  7 Aug 2019 03:06:08 -0400

> This series try to fix several issues introduced by meta data
> accelreation series. Please review.
 ...

My impression is that patch #7 will be changed to use spinlocks so there
will be a v5.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v4 07/17] drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS

2019-08-08 Thread Rob Herring

On Thu, Aug 8, 2019 at 7:44 AM Gerd Hoffmann  wrote:
>
> DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now,
> drop it.
>
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/drm/drm_gem_shmem_helper.h  | 26 -
>  drivers/gpu/drm/cirrus/cirrus.c |  2 +-
>  drivers/gpu/drm/panfrost/panfrost_drv.c |  2 +-
>  drivers/gpu/drm/v3d/v3d_drv.c   |  2 +-
>  4 files changed, 3 insertions(+), 29 deletions(-)

Acked-by: Rob Herring 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] drm/virtio: use virtio_max_dma_size

2019-08-08 Thread Gerd Hoffmann

We must make sure our scatterlist segments are not too big, otherwise
we might see swiotlb failures (happens with sev, also reproducable with
swiotlb=force).

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index b2da31310d24..6e44568813dd 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -204,6 +204,7 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device 
*qdev,
.interruptible = false,
.no_wait_gpu = false
};
+   unsigned max_segment;
 
/* wtf swapping */
if (bo->pages)
@@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct 
virtio_gpu_device *qdev,
if (!bo->pages)
goto out;
 
-   ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0,
-   nr_pages << PAGE_SHIFT, GFP_KERNEL);
+   max_segment = virtio_max_dma_size(qdev->vdev);
+   max_segment &= ~(size_t)(PAGE_SIZE - 1);
+   if (max_segment > SCATTERLIST_MAX_SEGMENT)
+   max_segment = SCATTERLIST_MAX_SEGMENT;
+   ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0,
+ nr_pages << PAGE_SHIFT,
+ max_segment, GFP_KERNEL);
if (ret)
goto out;
return 0;
-- 
2.18.1

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

[PATCH v4 16/17] drm/qxl: drop verify_access

2019-08-08 Thread Gerd Hoffmann

Not needed any more.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index dbaed0e67c21..d1d8fe6e1e93 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -110,14 +110,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
*placement = qbo->placement;
 }
 
-static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct qxl_bo *qbo = to_qxl_bo(bo);
-
-   return drm_vma_node_verify_access(&qbo->tbo.base.vma_node,
- filp->private_data);
-}
-
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
  struct ttm_mem_reg *mem)
 {
@@ -269,7 +261,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &qxl_evict_flags,
.move = &qxl_bo_move,
-   .verify_access = &qxl_verify_access,
.io_mem_reserve = &qxl_ttm_io_mem_reserve,
.io_mem_free = &qxl_ttm_io_mem_free,
.move_notify = &qxl_bo_move_notify,
-- 
2.18.1

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

[PATCH v4 12/17] drm: drop DRM_VRAM_MM_FILE_OPERATIONS

2019-08-08 Thread Gerd Hoffmann

Not needed any more because we don't have vram specific functions any
more.  DEFINE_DRM_GEM_FOPS() can be used instead.

Signed-off-by: Gerd Hoffmann 
---
 include/drm/drm_vram_mm_helper.h| 17 -
 drivers/gpu/drm/ast/ast_drv.c   |  5 +
 drivers/gpu/drm/bochs/bochs_drv.c   |  5 +
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c |  5 +
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  5 +
 drivers/gpu/drm/vboxvideo/vbox_drv.c|  5 +
 6 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/include/drm/drm_vram_mm_helper.h b/include/drm/drm_vram_mm_helper.h
index f272adc8ad37..59a8a7657a5b 100644
--- a/include/drm/drm_vram_mm_helper.h
+++ b/include/drm/drm_vram_mm_helper.h
@@ -74,21 +74,4 @@ struct drm_vram_mm *drm_vram_helper_alloc_mm(
const struct drm_vram_mm_funcs *funcs);
 void drm_vram_helper_release_mm(struct drm_device *dev);
 
-/**
- * define DRM_VRAM_MM_FILE_OPERATIONS - default callback functions for \
-   &struct file_operations
- *
- * Drivers that use VRAM MM can use this macro to initialize
- * &struct file_operations with default functions.
- */
-#define DRM_VRAM_MM_FILE_OPERATIONS \
-   .llseek = no_llseek, \
-   .read   = drm_read, \
-   .poll   = drm_poll, \
-   .unlocked_ioctl = drm_ioctl, \
-   .compat_ioctl   = drm_compat_ioctl, \
-   .mmap   = drm_gem_mmap, \
-   .open   = drm_open, \
-   .release= drm_release \
-
 #endif
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 6ed6ff49efc0..358d2a34b4e6 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -201,10 +201,7 @@ static struct pci_driver ast_pci_driver = {
.driver.pm = &ast_pm_ops,
 };
 
-static const struct file_operations ast_fops = {
-   .owner = THIS_MODULE,
-   DRM_VRAM_MM_FILE_OPERATIONS
-};
+DEFINE_DRM_GEM_FOPS(ast_fops);
 
 static struct drm_driver driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM,
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index 770e1625d05e..d7e09af0832a 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -58,10 +58,7 @@ static int bochs_load(struct drm_device *dev)
return ret;
 }
 
-static const struct file_operations bochs_fops = {
-   .owner  = THIS_MODULE,
-   DRM_VRAM_MM_FILE_OPERATIONS
-};
+DEFINE_DRM_GEM_FOPS(bochs_fops);
 
 static struct drm_driver bochs_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 6386c67e086b..b3b1275ebf51 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -27,10 +27,7 @@
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
 
-static const struct file_operations hibmc_fops = {
-   .owner  = THIS_MODULE,
-   DRM_VRAM_MM_FILE_OPERATIONS
-};
+DEFINE_DRM_GEM_FOPS(hibmc_fops);
 
 static irqreturn_t hibmc_drm_interrupt(int irq, void *arg)
 {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c 
b/drivers/gpu/drm/mgag200/mgag200_drv.c
index afd9119b6cf1..684a324990d9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -58,10 +58,7 @@ static void mga_pci_remove(struct pci_dev *pdev)
drm_put_dev(dev);
 }
 
-static const struct file_operations mgag200_driver_fops = {
-   .owner = THIS_MODULE,
-   DRM_VRAM_MM_FILE_OPERATIONS
-};
+DEFINE_DRM_GEM_FOPS(mgag200_driver_fops);
 
 static struct drm_driver driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET,
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c 
b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index 6189ea89bb71..f70360081ef1 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -189,10 +189,7 @@ static struct pci_driver vbox_pci_driver = {
 #endif
 };
 
-static const struct file_operations vbox_fops = {
-   .owner = THIS_MODULE,
-   DRM_VRAM_MM_FILE_OPERATIONS
-};
+DEFINE_DRM_GEM_FOPS(vbox_fops);
 
 static struct drm_driver driver = {
.driver_features =
-- 
2.18.1

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

[PATCH v4 17/17] drm/qxl: use DEFINE_DRM_GEM_FOPS()

2019-08-08 Thread Gerd Hoffmann

We have no qxl-specific fops any more.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 2fb1641c817e..4853082ba924 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -132,15 +132,7 @@ qxl_pci_remove(struct pci_dev *pdev)
drm_dev_put(dev);
 }
 
-static const struct file_operations qxl_fops = {
-   .owner = THIS_MODULE,
-   .open = drm_open,
-   .release = drm_release,
-   .unlocked_ioctl = drm_ioctl,
-   .poll = drm_poll,
-   .read = drm_read,
-   .mmap = drm_gem_mmap,
-};
+DEFINE_DRM_GEM_FOPS(qxl_fops);
 
 static int qxl_drm_freeze(struct drm_device *dev)
 {
-- 
2.18.1

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

[PATCH v4 14/17] drm/qxl: drop qxl_ttm_fault

2019-08-08 Thread Gerd Hoffmann

Not sure what this hook is supposed to do.  vmf->vma->vm_private_data
should never be NULL, so the extra check in qxl_ttm_fault should have no
effect.

Drop it.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 27 +--
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3a24145dd516..41edbde0e37e 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -48,24 +48,8 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device 
*bdev)
return qdev;
 }
 
-static struct vm_operations_struct qxl_ttm_vm_ops;
-static const struct vm_operations_struct *ttm_vm_ops;
-
-static vm_fault_t qxl_ttm_fault(struct vm_fault *vmf)
-{
-   struct ttm_buffer_object *bo;
-   vm_fault_t ret;
-
-   bo = (struct ttm_buffer_object *)vmf->vma->vm_private_data;
-   if (bo == NULL)
-   return VM_FAULT_NOPAGE;
-   ret = ttm_vm_ops->fault(vmf);
-   return ret;
-}
-
 int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
 {
-   int r;
struct drm_file *file_priv = filp->private_data;
struct qxl_device *qdev = file_priv->minor->dev->dev_private;
 
@@ -77,16 +61,7 @@ int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n",
  filp->private_data, vma->vm_pgoff);
 
-   r = ttm_bo_mmap(filp, vma, &qdev->mman.bdev);
-   if (unlikely(r != 0))
-   return r;
-   if (unlikely(ttm_vm_ops == NULL)) {
-   ttm_vm_ops = vma->vm_ops;
-   qxl_ttm_vm_ops = *ttm_vm_ops;
-   qxl_ttm_vm_ops.fault = &qxl_ttm_fault;
-   }
-   vma->vm_ops = &qxl_ttm_vm_ops;
-   return 0;
+   return ttm_bo_mmap(filp, vma, &qdev->mman.bdev);
 }
 
 static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-- 
2.18.1

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

[PATCH v4 15/17] drm/qxl: switch qxl to drm_gem_object_funcs->mmap codepath

2019-08-08 Thread Gerd Hoffmann

... using the use drm_gem_ttm_mmap() helper function.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.h|  1 -
 drivers/gpu/drm/qxl/qxl_drv.c|  2 +-
 drivers/gpu/drm/qxl/qxl_object.c |  1 +
 drivers/gpu/drm/qxl/qxl_ttm.c| 16 
 4 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 82efbe76062a..dc36479a54f2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -352,7 +352,6 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
 void qxl_ttm_fini(struct qxl_device *qdev);
-int qxl_mmap(struct file *filp, struct vm_area_struct *vma);
 
 /* qxl image */
 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 38467478c7b2..2fb1641c817e 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -139,7 +139,7 @@ static const struct file_operations qxl_fops = {
.unlocked_ioctl = drm_ioctl,
.poll = drm_poll,
.read = drm_read,
-   .mmap = qxl_mmap,
+   .mmap = drm_gem_mmap,
 };
 
 static int qxl_drm_freeze(struct drm_device *dev)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 29aab7b14513..5c503829c580 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -86,6 +86,7 @@ static const struct drm_gem_object_funcs qxl_object_funcs = {
.get_sg_table = qxl_gem_prime_get_sg_table,
.vmap = qxl_gem_prime_vmap,
.vunmap = qxl_gem_prime_vunmap,
+   .mmap = drm_gem_ttm_mmap,
 };
 
 int qxl_bo_create(struct qxl_device *qdev,
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 41edbde0e37e..dbaed0e67c21 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -48,22 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device 
*bdev)
return qdev;
 }
 
-int qxl_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-   struct drm_file *file_priv = filp->private_data;
-   struct qxl_device *qdev = file_priv->minor->dev->dev_private;
-
-   if (qdev == NULL) {
-   DRM_ERROR(
-"filp->private_data->minor->dev->dev_private == NULL\n");
-   return -EINVAL;
-   }
-   DRM_DEBUG_DRIVER("filp->private_data = 0x%p, vma->vm_pgoff = %lx\n",
- filp->private_data, vma->vm_pgoff);
-
-   return ttm_bo_mmap(filp, vma, &qdev->mman.bdev);
-}
-
 static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
 {
return 0;
-- 
2.18.1

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

[PATCH v4 13/17] drm/qxl: use drm_gem_object_funcs

2019-08-08 Thread Gerd Hoffmann

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.c|  8 
 drivers/gpu/drm/qxl/qxl_object.c | 12 
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 12cf85a06bed..38467478c7b2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -257,16 +257,8 @@ static struct drm_driver qxl_driver = {
 #endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
-   .gem_prime_pin = qxl_gem_prime_pin,
-   .gem_prime_unpin = qxl_gem_prime_unpin,
-   .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
.gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
-   .gem_prime_vmap = qxl_gem_prime_vmap,
-   .gem_prime_vunmap = qxl_gem_prime_vunmap,
.gem_prime_mmap = qxl_gem_prime_mmap,
-   .gem_free_object_unlocked = qxl_gem_object_free,
-   .gem_open_object = qxl_gem_object_open,
-   .gem_close_object = qxl_gem_object_close,
.fops = &qxl_fops,
.ioctls = qxl_ioctls,
.irq_handler = qxl_irq_handler,
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 548dfe6f3b26..29aab7b14513 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -77,6 +77,17 @@ void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 
domain, bool pinned)
}
 }
 
+static const struct drm_gem_object_funcs qxl_object_funcs = {
+   .free = qxl_gem_object_free,
+   .open = qxl_gem_object_open,
+   .close = qxl_gem_object_close,
+   .pin = qxl_gem_prime_pin,
+   .unpin = qxl_gem_prime_unpin,
+   .get_sg_table = qxl_gem_prime_get_sg_table,
+   .vmap = qxl_gem_prime_vmap,
+   .vunmap = qxl_gem_prime_vunmap,
+};
+
 int qxl_bo_create(struct qxl_device *qdev,
  unsigned long size, bool kernel, bool pinned, u32 domain,
  struct qxl_surface *surf,
@@ -100,6 +111,7 @@ int qxl_bo_create(struct qxl_device *qdev,
kfree(bo);
return r;
}
+   bo->tbo.base.funcs = &qxl_object_funcs;
bo->type = domain;
bo->pin_count = pinned ? 1 : 0;
bo->surface_id = 0;
-- 
2.18.1

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

[PATCH v4 07/17] drm/shmem: drop DEFINE_DRM_GEM_SHMEM_FOPS

2019-08-08 Thread Gerd Hoffmann

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now,
drop it.

Signed-off-by: Gerd Hoffmann 
---
 include/drm/drm_gem_shmem_helper.h  | 26 -
 drivers/gpu/drm/cirrus/cirrus.c |  2 +-
 drivers/gpu/drm/panfrost/panfrost_drv.c |  2 +-
 drivers/gpu/drm/v3d/v3d_drv.c   |  2 +-
 4 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/include/drm/drm_gem_shmem_helper.h 
b/include/drm/drm_gem_shmem_helper.h
index 0f7b77cdc26b..813240dcfe17 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -85,32 +85,6 @@ struct drm_gem_shmem_object {
 #define to_drm_gem_shmem_obj(obj) \
container_of(obj, struct drm_gem_shmem_object, base)
 
-/**
- * DEFINE_DRM_GEM_SHMEM_FOPS() - Macro to generate file operations for shmem 
drivers
- * @name: name for the generated structure
- *
- * This macro autogenerates a suitable &struct file_operations for shmem based
- * drivers, which can be assigned to &drm_driver.fops. Note that this structure
- * cannot be shared between drivers, because it contains a reference to the
- * current module using THIS_MODULE.
- *
- * Note that the declaration is already marked as static - if you need a
- * non-static version of this you're probably doing it wrong and will break the
- * THIS_MODULE reference by accident.
- */
-#define DEFINE_DRM_GEM_SHMEM_FOPS(name) \
-   static const struct file_operations name = {\
-   .owner  = THIS_MODULE,\
-   .open   = drm_open,\
-   .release= drm_release,\
-   .unlocked_ioctl = drm_ioctl,\
-   .compat_ioctl   = drm_compat_ioctl,\
-   .poll   = drm_poll,\
-   .read   = drm_read,\
-   .llseek = noop_llseek,\
-   .mmap   = drm_gem_mmap, \
-   }
-
 struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, 
size_t size);
 void drm_gem_shmem_free_object(struct drm_gem_object *obj);
 
diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index 36a69aec8a4b..9438af468331 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -510,7 +510,7 @@ static void cirrus_mode_config_init(struct cirrus_device 
*cirrus)
 
 /* -- */
 
-DEFINE_DRM_GEM_SHMEM_FOPS(cirrus_fops);
+DEFINE_DRM_GEM_FOPS(cirrus_fops);
 
 static struct drm_driver cirrus_driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index b187daa4da85..1c07e1c3386e 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -386,7 +386,7 @@ static const struct drm_ioctl_desc 
panfrost_drm_driver_ioctls[] = {
PANFROST_IOCTL(PERFCNT_DUMP,perfcnt_dump,   DRM_RENDER_ALLOW),
 };
 
-DEFINE_DRM_GEM_SHMEM_FOPS(panfrost_drm_driver_fops);
+DEFINE_DRM_GEM_FOPS(panfrost_drm_driver_fops);
 
 static struct drm_driver panfrost_drm_driver = {
.driver_features= DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ,
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 3506ae2723ae..03e4fbe1b92b 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -169,7 +169,7 @@ v3d_postclose(struct drm_device *dev, struct drm_file *file)
kfree(v3d_priv);
 }
 
-DEFINE_DRM_GEM_SHMEM_FOPS(v3d_drm_fops);
+DEFINE_DRM_GEM_FOPS(v3d_drm_fops);
 
 /* DRM_AUTH is required on SUBMIT_CL for now, while we don't have GMP
  * protection between clients.  Note that render nodes would be be
-- 
2.18.1

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

[PATCH v4 04/17] drm/qxl: switch qxl to the new gem_ttm_bo_device_init()

2019-08-08 Thread Gerd Hoffmann

This allows to drop qxl_mode_dumb_mmap() and qxl_bo_mmap_offset(),
the default gem function works just fine.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.h|  4 +---
 drivers/gpu/drm/qxl/qxl_object.h |  5 -
 drivers/gpu/drm/qxl/qxl_drv.c|  1 -
 drivers/gpu/drm/qxl/qxl_dumb.c   | 17 -
 drivers/gpu/drm/qxl/qxl_ioctl.c  |  5 +++--
 drivers/gpu/drm/qxl/qxl_ttm.c|  8 
 drivers/gpu/drm/qxl/Kconfig  |  1 +
 7 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..82efbe76062a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -347,9 +348,6 @@ int qxl_bo_kmap(struct qxl_bo *bo, void **ptr);
 int qxl_mode_dumb_create(struct drm_file *file_priv,
 struct drm_device *dev,
 struct drm_mode_create_dumb *args);
-int qxl_mode_dumb_mmap(struct drm_file *filp,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p);
 
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..1f0316ebcfd0 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -58,11 +58,6 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
return bo->tbo.num_pages << PAGE_SHIFT;
 }
 
-static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
-{
-   return drm_vma_node_offset_addr(&bo->tbo.base.vma_node);
-}
-
 static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
  bool no_wait)
 {
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index c1802e01d9f6..12cf85a06bed 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -252,7 +252,6 @@ static struct drm_driver qxl_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 
.dumb_create = qxl_mode_dumb_create,
-   .dumb_map_offset = qxl_mode_dumb_mmap,
 #if defined(CONFIG_DEBUG_FS)
.debugfs_init = qxl_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 272d19b677d8..bd3b16a701a6 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -69,20 +69,3 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
args->handle = handle;
return 0;
 }
-
-int qxl_mode_dumb_mmap(struct drm_file *file_priv,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p)
-{
-   struct drm_gem_object *gobj;
-   struct qxl_bo *qobj;
-
-   BUG_ON(!offset_p);
-   gobj = drm_gem_object_lookup(file_priv, handle);
-   if (gobj == NULL)
-   return -ENOENT;
-   qobj = gem_to_qxl_bo(gobj);
-   *offset_p = qxl_bo_mmap_offset(qobj);
-   drm_gem_object_put_unlocked(gobj);
-   return 0;
-}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 8117a45b3610..c8d9ea552532 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -67,8 +67,9 @@ static int qxl_map_ioctl(struct drm_device *dev, void *data,
struct qxl_device *qdev = dev->dev_private;
struct drm_qxl_map *qxl_map = data;
 
-   return qxl_mode_dumb_mmap(file_priv, &qdev->ddev, qxl_map->handle,
- &qxl_map->offset);
+   return drm_gem_dumb_map_offset(file_priv, &qdev->ddev,
+  qxl_map->handle,
+  &qxl_map->offset);
 }
 
 struct qxl_reloc_info {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 9b24514c75aa..3a24145dd516 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -322,10 +322,10 @@ int qxl_ttm_init(struct qxl_device *qdev)
int num_io_pages; /* != rom->num_io_pages, we include surface0 */
 
/* No others user of address space so set it to 0 */
-   r = ttm_bo_device_init(&qdev->mman.bdev,
-  &qxl_bo_driver,
-  qdev->ddev.anon_inode->i_mapping,
-  false);
+   r = drm_gem_ttm_bo_device_init(&qdev->ddev,
+  &qdev->mman.bdev,
+  &qxl_bo_driver,
+  false);
if (r) {
DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
return r;
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index d0d691b31f4a..bfe90c7d17b2 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -3,6 +3,7 @@ config DRM_QXL
tris

Re: [PATCH V4 7/9] vhost: do not use RCU to synchronize MMU notifier with worker

2019-08-08 Thread Jason Gunthorpe

On Thu, Aug 08, 2019 at 08:54:54PM +0800, Jason Wang wrote:

> I don't have any objection to convert  to spinlock() but just want to
> know if any case that the above smp_mb() + counter looks good to you?

This email is horribly mangled, but I don't think mixing smb_mb() and
smp_load_acquire() would be considerd a best-practice, and using
smp_store_release() instead would be the wrong barrier.

spinlock does seem to be the only existing locking primitive that does
what is needed here.

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

Re: [PATCH V4 7/9] vhost: do not use RCU to synchronize MMU notifier with worker

2019-08-08 Thread Jason Wang



- Original Message -
> 
> On 2019/8/7 下午10:02, Jason Wang wrote:
> >
> > On 2019/8/7 下午8:07, Jason Gunthorpe wrote:
> >> On Wed, Aug 07, 2019 at 03:06:15AM -0400, Jason Wang wrote:
> >>> We used to use RCU to synchronize MMU notifier with worker. This leads
> >>> calling synchronize_rcu() in invalidate_range_start(). But on a busy
> >>> system, there would be many factors that may slow down the
> >>> synchronize_rcu() which makes it unsuitable to be called in MMU
> >>> notifier.
> >>>
> >>> So this patch switches use seqlock counter to track whether or not the
> >>> map was used. The counter was increased when vq try to start or finish
> >>> uses the map. This means, when it was even, we're sure there's no
> >>> readers and MMU notifier is synchronized. When it was odd, it means
> >>> there's a reader we need to wait it to be even again then we are
> >>> synchronized. Consider the read critical section is pretty small the
> >>> synchronization should be done very fast.
> >>>
> >>> Reported-by: Michael S. Tsirkin 
> >>> Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel
> >>> virtual address")
> >>> Signed-off-by: Jason Wang 
> >>>   drivers/vhost/vhost.c | 141
> >>> ++
> >>>   drivers/vhost/vhost.h |   7 ++-
> >>>   2 files changed, 90 insertions(+), 58 deletions(-)
> >>>
> >>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >>> index cfc11f9ed9c9..57bfbb60d960 100644
> >>> +++ b/drivers/vhost/vhost.c
> >>> @@ -324,17 +324,16 @@ static void vhost_uninit_vq_maps(struct
> >>> vhost_virtqueue *vq)
> >>>     spin_lock(&vq->mmu_lock);
> >>>   for (i = 0; i < VHOST_NUM_ADDRS; i++) {
> >>> -    map[i] = rcu_dereference_protected(vq->maps[i],
> >>> -  lockdep_is_held(&vq->mmu_lock));
> >>> +    map[i] = vq->maps[i];
> >>>   if (map[i]) {
> >>>   vhost_set_map_dirty(vq, map[i], i);
> >>> -    rcu_assign_pointer(vq->maps[i], NULL);
> >>> +    vq->maps[i] = NULL;
> >>>   }
> >>>   }
> >>>   spin_unlock(&vq->mmu_lock);
> >>>   -    /* No need for synchronize_rcu() or kfree_rcu() since we are
> >>> - * serialized with memory accessors (e.g vq mutex held).
> >>> +    /* No need for synchronization since we are serialized with
> >>> + * memory accessors (e.g vq mutex held).
> >>>    */
> >>>     for (i = 0; i < VHOST_NUM_ADDRS; i++)
> >>> @@ -362,6 +361,40 @@ static bool vhost_map_range_overlap(struct
> >>> vhost_uaddr *uaddr,
> >>>   return !(end < uaddr->uaddr || start > uaddr->uaddr - 1 +
> >>> uaddr->size);
> >>>   }
> >>>   +static void inline vhost_vq_access_map_begin(struct
> >>> vhost_virtqueue *vq)
> >>> +{
> >>> +    write_seqcount_begin(&vq->seq);
> >>> +}
> >>> +
> >>> +static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
> >>> +{
> >>> +    write_seqcount_end(&vq->seq);
> >>> +}
> >> The write side of a seqlock only provides write barriers. Access to
> >>
> >> map = vq->maps[VHOST_ADDR_USED];
> >>
> >> Still needs a read side barrier, and then I think this will be no
> >> better than a normal spinlock.
> >>
> >> It also doesn't seem like this algorithm even needs a seqlock, as this
> >> is just a one bit flag
> >
> >
> > Right, so then I tend to use spinlock first for correctness.
> >
> >
> >>
> >> atomic_set_bit(using map)
> >> smp_mb__after_atomic()
> >> .. maps [...]
> >> atomic_clear_bit(using map)
> >>
> >>
> >> map = NULL;
> >> smp_mb__before_atomic();
> >> while (atomic_read_bit(using map))
> >>     relax()
> >>
> >> Again, not clear this could be faster than a spinlock when the
> >> barriers are correct...
> >
> 
> I've done some benchmark[1] on x86, and yes it looks even slower. It
> looks to me the atomic stuffs is not necessary, so in order to compare
> it better with spinlock. I tweak it a little bit through
> smp_load_acquire()/store_releaes() + mb() like:
> 

Sorry the format is messed up:

The code should be something like:

static struct vhost_map *vhost_vq_access_map_begin(struct vhost_virtqueue *vq,
   unsigned int type)
{
++vq->counter;
/* Ensure map was read after incresing the counter. Paired
 * with smp_mb() in vhost_vq_sync_access().
 */
smp_mb();
return vq->maps[type];
}

static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
{
/* Ensure all memory access through map was done before
 * reducing the counter. Paired with smp_load_acquire() in
 * vhost_vq_sync_access() */
smp_store_release(&vq->counter, --vq->counter);
}

static void inline vhost_vq_sync_access(struct vhost_virtqueue *vq)
{
/* Ensure new map value is visible before checking counter. */
smp_mb();
/* Ensure map was freed after reading counter value, paired
 * with smp_store_release() in vhost_vq_access_map_end().
 */
while (smp_load_acqu

Re: [PATCH V4 7/9] vhost: do not use RCU to synchronize MMU notifier with worker

2019-08-08 Thread Jason Wang


On 2019/8/7 下午10:02, Jason Wang wrote:
>
> On 2019/8/7 下午8:07, Jason Gunthorpe wrote:
>> On Wed, Aug 07, 2019 at 03:06:15AM -0400, Jason Wang wrote:
>>> We used to use RCU to synchronize MMU notifier with worker. This leads
>>> calling synchronize_rcu() in invalidate_range_start(). But on a busy
>>> system, there would be many factors that may slow down the
>>> synchronize_rcu() which makes it unsuitable to be called in MMU
>>> notifier.
>>>
>>> So this patch switches use seqlock counter to track whether or not the
>>> map was used. The counter was increased when vq try to start or finish
>>> uses the map. This means, when it was even, we're sure there's no
>>> readers and MMU notifier is synchronized. When it was odd, it means
>>> there's a reader we need to wait it to be even again then we are
>>> synchronized. Consider the read critical section is pretty small the
>>> synchronization should be done very fast.
>>>
>>> Reported-by: Michael S. Tsirkin 
>>> Fixes: 7f466032dc9e ("vhost: access vq metadata through kernel
>>> virtual address")
>>> Signed-off-by: Jason Wang 
>>>   drivers/vhost/vhost.c | 141
>>> ++
>>>   drivers/vhost/vhost.h |   7 ++-
>>>   2 files changed, 90 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>> index cfc11f9ed9c9..57bfbb60d960 100644
>>> +++ b/drivers/vhost/vhost.c
>>> @@ -324,17 +324,16 @@ static void vhost_uninit_vq_maps(struct
>>> vhost_virtqueue *vq)
>>>     spin_lock(&vq->mmu_lock);
>>>   for (i = 0; i < VHOST_NUM_ADDRS; i++) {
>>> -    map[i] = rcu_dereference_protected(vq->maps[i],
>>> -  lockdep_is_held(&vq->mmu_lock));
>>> +    map[i] = vq->maps[i];
>>>   if (map[i]) {
>>>   vhost_set_map_dirty(vq, map[i], i);
>>> -    rcu_assign_pointer(vq->maps[i], NULL);
>>> +    vq->maps[i] = NULL;
>>>   }
>>>   }
>>>   spin_unlock(&vq->mmu_lock);
>>>   -    /* No need for synchronize_rcu() or kfree_rcu() since we are
>>> - * serialized with memory accessors (e.g vq mutex held).
>>> +    /* No need for synchronization since we are serialized with
>>> + * memory accessors (e.g vq mutex held).
>>>    */
>>>     for (i = 0; i < VHOST_NUM_ADDRS; i++)
>>> @@ -362,6 +361,40 @@ static bool vhost_map_range_overlap(struct
>>> vhost_uaddr *uaddr,
>>>   return !(end < uaddr->uaddr || start > uaddr->uaddr - 1 +
>>> uaddr->size);
>>>   }
>>>   +static void inline vhost_vq_access_map_begin(struct
>>> vhost_virtqueue *vq)
>>> +{
>>> +    write_seqcount_begin(&vq->seq);
>>> +}
>>> +
>>> +static void inline vhost_vq_access_map_end(struct vhost_virtqueue *vq)
>>> +{
>>> +    write_seqcount_end(&vq->seq);
>>> +}
>> The write side of a seqlock only provides write barriers. Access to
>>
>> map = vq->maps[VHOST_ADDR_USED];
>>
>> Still needs a read side barrier, and then I think this will be no
>> better than a normal spinlock.
>>
>> It also doesn't seem like this algorithm even needs a seqlock, as this
>> is just a one bit flag
>
>
> Right, so then I tend to use spinlock first for correctness.
>
>
>>
>> atomic_set_bit(using map)
>> smp_mb__after_atomic()
>> .. maps [...]
>> atomic_clear_bit(using map)
>>
>>
>> map = NULL;
>> smp_mb__before_atomic();
>> while (atomic_read_bit(using map))
>>     relax()
>>
>> Again, not clear this could be faster than a spinlock when the
>> barriers are correct...
>

I've done some benchmark[1] on x86, and yes it looks even slower. It
looks to me the atomic stuffs is not necessary, so in order to compare
it better with spinlock. I tweak it a little bit through
smp_load_acquire()/store_releaes() + mb() like:

static struct vhost_map *vhost_vq_access_map_begin(struct
vhost_virtqueue
*vq,  

   unsigned int
type)   


{   


   
++vq->counter;  


    /* Ensure map was read after incresing the counter.
Paired  


 * with smp_mb() in
vhost_vq_sync_access(). 



*/  
   

   
smp_mb();   


    return
vq->maps[type];

Re: [PATCH v2 2/2] virtio_ring: packed ring: fix virtqueue_detach_unused_buf

2019-08-08 Thread Pankaj Gupta



> On Thu, Aug 08, 2019 at 08:28:46AM -0400, Pankaj Gupta wrote:
> > 
> > 
> > > > This patch makes packed ring code compatible with split ring in
> > > > function
> > > > 'virtqueue_detach_unused_buf_*'.
> > > 
> > > What does that mean?  What does this "fix"?
> > 
> > Patch 1 frees the buffers When a port is unplugged from the virtio
> > console device. It does this with the help of
> > 'virtqueue_detach_unused_buf_split/packed'
> > function. For split ring case, corresponding function decrements avail ring
> > index.
> > For packed ring code, this functionality is not available, so this patch
> > adds the
> > required support and hence help to remove the unused buffer completely.
> 
> Explain all of this in great detail in the changelog comment.  What you
> have in there today does not make any sense.

Sure. Will try to put in more clear words.

Thanks,
Pankaj

> 
> thanks,
> 
> greg k-h
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] virtio_ring: packed ring: fix virtqueue_detach_unused_buf

2019-08-08 Thread Greg KH

On Thu, Aug 08, 2019 at 08:28:46AM -0400, Pankaj Gupta wrote:
> 
> 
> > > This patch makes packed ring code compatible with split ring in function
> > > 'virtqueue_detach_unused_buf_*'.
> > 
> > What does that mean?  What does this "fix"?
> 
> Patch 1 frees the buffers When a port is unplugged from the virtio
> console device. It does this with the help of 
> 'virtqueue_detach_unused_buf_split/packed'
> function. For split ring case, corresponding function decrements avail ring 
> index.
> For packed ring code, this functionality is not available, so this patch adds 
> the
> required support and hence help to remove the unused buffer completely.

Explain all of this in great detail in the changelog comment.  What you
have in there today does not make any sense.

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] virtio_ring: packed ring: fix virtqueue_detach_unused_buf

2019-08-08 Thread Pankaj Gupta




> > This patch makes packed ring code compatible with split ring in function
> > 'virtqueue_detach_unused_buf_*'.
> 
> What does that mean?  What does this "fix"?

Patch 1 frees the buffers When a port is unplugged from the virtio
console device. It does this with the help of 
'virtqueue_detach_unused_buf_split/packed'
function. For split ring case, corresponding function decrements avail ring 
index.
For packed ring code, this functionality is not available, so this patch adds 
the
required support and hence help to remove the unused buffer completely.

Thanks,
Pankaj  

> 
> thanks,
> 
> greg k-h
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/2] virtio_console: free unused buffers with port delete

2019-08-08 Thread Pankaj Gupta



> 
> On Thu, Aug 08, 2019 at 05:06:05PM +0530, Pankaj Gupta wrote:
> >   The commit a7a69ec0d8e4 ("virtio_console: free buffers after reset")
> >   deferred detaching of unused buffer to virtio device unplug time.
> > 
> >   This causes unplug/replug of single port in virtio device with an
> >   error "Error allocating inbufs\n". As we don't free the unused buffers
> >   attached with the port. Re-plug the same port tries to allocate new
> >   buffers in virtqueue and results in this error if queue is full.
> > 
> >   This patch removes the unused buffers in vq's when we unplug the port.
> >   This is the best we can do as we cannot call device_reset because virtio
> >   device is still active.
> 
> Why is this indented?

o.k. will remove the empty lines.

> 
> > 
> > Reported-by: Xiaohui Li 
> > Fixes: b3258ff1d6 ("virtio_console: free buffers after reset")
> 
> Fixes: b3258ff1d608 ("virtio: Decrement avail idx on buffer detach")
> 
> is the correct format to use.

Sorry! for this. Commit it fixes is:
a7a69ec0d8e4 ("virtio_console: free buffers after reset")

> 
> And given that this is from 2.6.39 (and 2.6.38.5), shouldn't it also be
> backported for the stable kernels?

Yes.

Thanks,
Pankaj

> 
> thanks,
> 
> greg k-h
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] virtio_ring: packed ring: fix virtqueue_detach_unused_buf

2019-08-08 Thread Greg KH

On Thu, Aug 08, 2019 at 05:06:06PM +0530, Pankaj Gupta wrote:
> This patch makes packed ring code compatible with split ring in function
> 'virtqueue_detach_unused_buf_*'.

What does that mean?  What does this "fix"?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/2] virtio_console: free unused buffers with port delete

2019-08-08 Thread Greg KH

On Thu, Aug 08, 2019 at 05:06:05PM +0530, Pankaj Gupta wrote:
>   The commit a7a69ec0d8e4 ("virtio_console: free buffers after reset")
>   deferred detaching of unused buffer to virtio device unplug time.
> 
>   This causes unplug/replug of single port in virtio device with an
>   error "Error allocating inbufs\n". As we don't free the unused buffers
>   attached with the port. Re-plug the same port tries to allocate new
>   buffers in virtqueue and results in this error if queue is full.
> 
>   This patch removes the unused buffers in vq's when we unplug the port.
>   This is the best we can do as we cannot call device_reset because virtio
>   device is still active.

Why is this indented?

> 
> Reported-by: Xiaohui Li 
> Fixes: b3258ff1d6 ("virtio_console: free buffers after reset")

Fixes: b3258ff1d608 ("virtio: Decrement avail idx on buffer detach")

is the correct format to use.

And given that this is from 2.6.39 (and 2.6.38.5), shouldn't it also be
backported for the stable kernels?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH v2 0/2] virtio_console: fix replug of virtio console port

2019-08-08 Thread Pankaj Gupta

This patch series fixes the issue with unplug/replug of a port in virtio
console device, which fails with an error "Error allocating inbufs\n".

Patch 2 makes virtio packed ring code compatible with virtio split ring.
Tested the packed ring code with the qemu virtio 1.1 device code posted
here [1].

Changes from v1[2]
-
Make virtio packed ring code compatible with split ring - [Michael]

[1]  https://marc.info/?l=qemu-devel&m=156471883703948&w=2
[2] https://lkml.org/lkml/2019/3/4/517

Pankaj Gupta (2):
  virtio_console: free unused buffers with port delete
  virtio_ring: packed ring: fix virtqueue_detach_unused_buf

 drivers/char/virtio_console.c | 14 +++---
 drivers/virtio/virtio_ring.c  |  5 +
 2 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.21.0

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

[PATCH v2 1/2] virtio_console: free unused buffers with port delete

2019-08-08 Thread Pankaj Gupta

  The commit a7a69ec0d8e4 ("virtio_console: free buffers after reset")
  deferred detaching of unused buffer to virtio device unplug time.

  This causes unplug/replug of single port in virtio device with an
  error "Error allocating inbufs\n". As we don't free the unused buffers
  attached with the port. Re-plug the same port tries to allocate new
  buffers in virtqueue and results in this error if queue is full.

  This patch removes the unused buffers in vq's when we unplug the port.
  This is the best we can do as we cannot call device_reset because virtio
  device is still active.

Reported-by: Xiaohui Li 
Fixes: b3258ff1d6 ("virtio_console: free buffers after reset")
Signed-off-by: Pankaj Gupta 
---
 drivers/char/virtio_console.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7270e7b69262..e8be82f1bae9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1494,15 +1494,25 @@ static void remove_port(struct kref *kref)
kfree(port);
 }
 
+static void remove_unused_bufs(struct virtqueue *vq)
+{
+   struct port_buffer *buf;
+
+   while ((buf = virtqueue_detach_unused_buf(vq)))
+   free_buf(buf, true);
+}
+
 static void remove_port_data(struct port *port)
 {
spin_lock_irq(&port->inbuf_lock);
/* Remove unused data this port might have received. */
discard_port_data(port);
+   remove_unused_bufs(port->in_vq);
spin_unlock_irq(&port->inbuf_lock);
 
spin_lock_irq(&port->outvq_lock);
reclaim_consumed_buffers(port);
+   remove_unused_bufs(port->out_vq);
spin_unlock_irq(&port->outvq_lock);
 }
 
@@ -1938,11 +1948,9 @@ static void remove_vqs(struct ports_device *portdev)
struct virtqueue *vq;
 
virtio_device_for_each_vq(portdev->vdev, vq) {
-   struct port_buffer *buf;
 
flush_bufs(vq, true);
-   while ((buf = virtqueue_detach_unused_buf(vq)))
-   free_buf(buf, true);
+   remove_unused_bufs(vq);
}
portdev->vdev->config->del_vqs(portdev->vdev);
kfree(portdev->in_vqs);
-- 
2.21.0

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

[PATCH v2 2/2] virtio_ring: packed ring: fix virtqueue_detach_unused_buf

2019-08-08 Thread Pankaj Gupta

This patch makes packed ring code compatible with split ring in function
'virtqueue_detach_unused_buf_*'.

Signed-off-by: Pankaj Gupta 
---
 drivers/virtio/virtio_ring.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c8be1c4f5b55..1b98a6777b7e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1534,6 +1534,11 @@ static void *virtqueue_detach_unused_buf_packed(struct 
virtqueue *_vq)
for (i = 0; i < vq->packed.vring.num; i++) {
if (!vq->packed.desc_state[i].data)
continue;
+   vq->packed.next_avail_idx--;
+   if (vq->packed.next_avail_idx < 0) {
+   vq->packed.next_avail_idx = vq->packed.vring.num - 1;
+   vq->packed.avail_wrap_counter ^= 1;
+   }
/* detach_buf clears data, so grab it now. */
buf = vq->packed.desc_state[i].data;
detach_buf_packed(vq, i, NULL);
-- 
2.21.0

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

[PATCH v3 5/8] drm/qxl: switch qxl to the new gem_ttm_bo_device_init()

2019-08-08 Thread Gerd Hoffmann

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_drv.h|  4 +---
 drivers/gpu/drm/qxl/qxl_object.h |  5 -
 drivers/gpu/drm/qxl/qxl_drv.c|  1 -
 drivers/gpu/drm/qxl/qxl_dumb.c   | 17 -
 drivers/gpu/drm/qxl/qxl_ioctl.c  |  5 +++--
 drivers/gpu/drm/qxl/qxl_ttm.c|  8 
 drivers/gpu/drm/qxl/Kconfig  |  1 +
 7 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 9e034c5fa87d..82efbe76062a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -347,9 +348,6 @@ int qxl_bo_kmap(struct qxl_bo *bo, void **ptr);
 int qxl_mode_dumb_create(struct drm_file *file_priv,
 struct drm_device *dev,
 struct drm_mode_create_dumb *args);
-int qxl_mode_dumb_mmap(struct drm_file *filp,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p);
 
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..1f0316ebcfd0 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -58,11 +58,6 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
return bo->tbo.num_pages << PAGE_SHIFT;
 }
 
-static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
-{
-   return drm_vma_node_offset_addr(&bo->tbo.base.vma_node);
-}
-
 static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
  bool no_wait)
 {
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index c1802e01d9f6..12cf85a06bed 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -252,7 +252,6 @@ static struct drm_driver qxl_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 
.dumb_create = qxl_mode_dumb_create,
-   .dumb_map_offset = qxl_mode_dumb_mmap,
 #if defined(CONFIG_DEBUG_FS)
.debugfs_init = qxl_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 272d19b677d8..bd3b16a701a6 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -69,20 +69,3 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
args->handle = handle;
return 0;
 }
-
-int qxl_mode_dumb_mmap(struct drm_file *file_priv,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p)
-{
-   struct drm_gem_object *gobj;
-   struct qxl_bo *qobj;
-
-   BUG_ON(!offset_p);
-   gobj = drm_gem_object_lookup(file_priv, handle);
-   if (gobj == NULL)
-   return -ENOENT;
-   qobj = gem_to_qxl_bo(gobj);
-   *offset_p = qxl_bo_mmap_offset(qobj);
-   drm_gem_object_put_unlocked(gobj);
-   return 0;
-}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 8117a45b3610..c8d9ea552532 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -67,8 +67,9 @@ static int qxl_map_ioctl(struct drm_device *dev, void *data,
struct qxl_device *qdev = dev->dev_private;
struct drm_qxl_map *qxl_map = data;
 
-   return qxl_mode_dumb_mmap(file_priv, &qdev->ddev, qxl_map->handle,
- &qxl_map->offset);
+   return drm_gem_dumb_map_offset(file_priv, &qdev->ddev,
+  qxl_map->handle,
+  &qxl_map->offset);
 }
 
 struct qxl_reloc_info {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 9b24514c75aa..3a24145dd516 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -322,10 +322,10 @@ int qxl_ttm_init(struct qxl_device *qdev)
int num_io_pages; /* != rom->num_io_pages, we include surface0 */
 
/* No others user of address space so set it to 0 */
-   r = ttm_bo_device_init(&qdev->mman.bdev,
-  &qxl_bo_driver,
-  qdev->ddev.anon_inode->i_mapping,
-  false);
+   r = drm_gem_ttm_bo_device_init(&qdev->ddev,
+  &qdev->mman.bdev,
+  &qxl_bo_driver,
+  false);
if (r) {
DRM_ERROR("failed initializing buffer object driver(%d).\n", r);
return r;
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index d0d691b31f4a..bfe90c7d17b2 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -3,6 +3,7 @@ config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
select DRM_KMS_HELPER
+   select DRM_T

[PATCH v3 8/8] gem/qxl: use drm_gem_ttm_bo_driver_verify_access()

2019-08-08 Thread Gerd Hoffmann

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 3a24145dd516..bcf48b062a85 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -151,14 +151,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
*placement = qbo->placement;
 }
 
-static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct qxl_bo *qbo = to_qxl_bo(bo);
-
-   return drm_vma_node_verify_access(&qbo->tbo.base.vma_node,
- filp->private_data);
-}
-
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
  struct ttm_mem_reg *mem)
 {
@@ -310,7 +302,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &qxl_evict_flags,
.move = &qxl_bo_move,
-   .verify_access = &qxl_verify_access,
+   .verify_access = drm_gem_ttm_bo_driver_verify_access,
.io_mem_reserve = &qxl_ttm_io_mem_reserve,
.io_mem_free = &qxl_ttm_io_mem_free,
.move_notify = &qxl_bo_move_notify,
-- 
2.18.1

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

39 matches

Mail list logo