[PATCH v4 11/16] drm/bochs: drop unused gpu_addr arg from bochs_bo_pin()

2019-01-16 Thread Gerd Hoffmann
It's always NULL, so just remove it.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs.h   |  2 +-
 drivers/gpu/drm/bochs/bochs_fbdev.c |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c   |  2 +-
 drivers/gpu/drm/bochs/bochs_mm.c| 11 +--
 4 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 4dc1b6384e..d0d474e06f 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -142,7 +142,7 @@ int bochs_dumb_create(struct drm_file *file, struct 
drm_device *dev,
 int bochs_dumb_mmap_offset(struct drm_file *file, struct drm_device *dev,
   uint32_t handle, uint64_t *offset);
 
-int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag, u64 *gpu_addr);
+int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag);
 int bochs_bo_unpin(struct bochs_bo *bo);
 
 /* bochs_kms.c */
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index d9f3d42999..92feb817ff 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -81,7 +81,7 @@ static int bochsfb_create(struct drm_fb_helper *helper,
if (ret)
return ret;
 
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
+   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
if (ret) {
DRM_ERROR("failed to pin fbcon\n");
ttm_bo_unreserve(>bo);
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 5b7e1a7c6b..f663c54185 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -101,7 +101,7 @@ static int bochs_plane_prepare_fb(struct drm_plane *plane,
ret = ttm_bo_reserve(>bo, true, false, NULL);
if (ret)
return ret;
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
+   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
ttm_bo_unreserve(>bo);
return ret;
 }
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 0980411e41..5a0e092847 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -210,20 +210,13 @@ static void bochs_ttm_placement(struct bochs_bo *bo, int 
domain)
bo->placement.num_busy_placement = c;
 }
 
-static inline u64 bochs_bo_gpu_offset(struct bochs_bo *bo)
-{
-   return bo->bo.offset;
-}
-
-int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag, u64 *gpu_addr)
+int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag)
 {
struct ttm_operation_ctx ctx = { false, false };
int i, ret;
 
if (bo->pin_count) {
bo->pin_count++;
-   if (gpu_addr)
-   *gpu_addr = bochs_bo_gpu_offset(bo);
return 0;
}
 
@@ -235,8 +228,6 @@ int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag, u64 
*gpu_addr)
return ret;
 
bo->pin_count = 1;
-   if (gpu_addr)
-   *gpu_addr = bochs_bo_gpu_offset(bo);
return 0;
 }
 
-- 
2.9.3

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


[PATCH v4 12/16] drm/bochs: move ttm_bo_(un)reserve calls into bochs_bo_{pin, unpin}

2019-01-16 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_fbdev.c |  8 
 drivers/gpu/drm/bochs/bochs_kms.c   | 14 +-
 drivers/gpu/drm/bochs/bochs_mm.c|  8 
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 92feb817ff..ccf783b038 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -77,14 +77,9 @@ static int bochsfb_create(struct drm_fb_helper *helper,
 
bo = gem_to_bochs_bo(gobj);
 
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret)
-   return ret;
-
ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
if (ret) {
DRM_ERROR("failed to pin fbcon\n");
-   ttm_bo_unreserve(>bo);
return ret;
}
 
@@ -92,12 +87,9 @@ static int bochsfb_create(struct drm_fb_helper *helper,
  >kmap);
if (ret) {
DRM_ERROR("failed to kmap fbcon\n");
-   ttm_bo_unreserve(>bo);
return ret;
}
 
-   ttm_bo_unreserve(>bo);
-
/* init fb device */
info = drm_fb_helper_alloc_fbi(helper);
if (IS_ERR(info)) {
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index f663c54185..fc856a02a2 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -92,34 +92,22 @@ static int bochs_plane_prepare_fb(struct drm_plane *plane,
struct drm_plane_state *new_state)
 {
struct bochs_bo *bo;
-   int ret;
 
if (!new_state->fb)
return 0;
bo = gem_to_bochs_bo(new_state->fb->obj[0]);
-
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret)
-   return ret;
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
-   ttm_bo_unreserve(>bo);
-   return ret;
+   return bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
 }
 
 static void bochs_plane_cleanup_fb(struct drm_plane *plane,
   struct drm_plane_state *old_state)
 {
struct bochs_bo *bo;
-   int ret;
 
if (!old_state->fb)
return;
bo = gem_to_bochs_bo(old_state->fb->obj[0]);
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret)
-   return;
bochs_bo_unpin(bo);
-   ttm_bo_unreserve(>bo);
 }
 
 static const struct drm_plane_helper_funcs bochs_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 5a0e092847..fcbf35456d 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -223,7 +223,11 @@ int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag)
bochs_ttm_placement(bo, pl_flag);
for (i = 0; i < bo->placement.num_placement; i++)
bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
+   ret = ttm_bo_reserve(>bo, true, false, NULL);
+   if (ret)
+   return ret;
ret = ttm_bo_validate(>bo, >placement, );
+   ttm_bo_unreserve(>bo);
if (ret)
return ret;
 
@@ -247,7 +251,11 @@ int bochs_bo_unpin(struct bochs_bo *bo)
 
for (i = 0; i < bo->placement.num_placement; i++)
bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
+   ret = ttm_bo_reserve(>bo, true, false, NULL);
+   if (ret)
+   return ret;
ret = ttm_bo_validate(>bo, >placement, );
+   ttm_bo_unreserve(>bo);
if (ret)
return ret;
 
-- 
2.9.3

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


[PATCH v4 04/16] drm/bochs: atomic: add mode_set_nofb callback.

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, step two.
Add mode_set_nofb crtc helper callback.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 2cbd406b1f..56fd7be933 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -86,6 +86,14 @@ static int bochs_crtc_mode_set(struct drm_crtc *crtc,
return 0;
 }
 
+static void bochs_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+   struct bochs_device *bochs =
+   container_of(crtc, struct bochs_device, crtc);
+
+   bochs_hw_setmode(bochs, >mode);
+}
+
 static void bochs_crtc_prepare(struct drm_crtc *crtc)
 {
 }
@@ -149,6 +157,7 @@ static const struct drm_crtc_helper_funcs 
bochs_helper_funcs = {
.dpms = bochs_crtc_dpms,
.mode_set = bochs_crtc_mode_set,
.mode_set_base = bochs_crtc_mode_set_base,
+   .mode_set_nofb = bochs_crtc_mode_set_nofb,
.prepare = bochs_crtc_prepare,
.commit = bochs_crtc_commit,
.atomic_enable = bochs_crtc_atomic_enable,
-- 
2.9.3

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


[PATCH v4 14/16] drm/bochs: switch to generic drm fbdev emulation

2019-01-16 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_drv.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index a3f4e21078..cea42ac64d 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -16,10 +16,6 @@ static int bochs_modeset = -1;
 module_param_named(modeset, bochs_modeset, int, 0444);
 MODULE_PARM_DESC(modeset, "enable/disable kernel modesetting");
 
-static bool enable_fbdev = true;
-module_param_named(fbdev, enable_fbdev, bool, 0444);
-MODULE_PARM_DESC(fbdev, "register fbdev device");
-
 /* -- */
 /* drm interface  */
 
@@ -27,7 +23,6 @@ static void bochs_unload(struct drm_device *dev)
 {
struct bochs_device *bochs = dev->dev_private;
 
-   bochs_fbdev_fini(bochs);
bochs_kms_fini(bochs);
bochs_mm_fini(bochs);
bochs_hw_fini(dev);
@@ -58,9 +53,6 @@ static int bochs_load(struct drm_device *dev)
if (ret)
goto err;
 
-   if (enable_fbdev)
-   bochs_fbdev_init(bochs);
-
return 0;
 
 err:
@@ -110,9 +102,7 @@ static int bochs_pm_suspend(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
-   struct bochs_device *bochs = drm_dev->dev_private;
 
-   drm_fb_helper_set_suspend_unlocked(>fb.helper, 1);
return drm_mode_config_helper_suspend(drm_dev);
 }
 
@@ -120,9 +110,7 @@ static int bochs_pm_resume(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
-   struct bochs_device *bochs = drm_dev->dev_private;
 
-   drm_fb_helper_set_suspend_unlocked(>fb.helper, 0);
return drm_mode_config_helper_resume(drm_dev);
 }
 #endif
@@ -167,6 +155,7 @@ static int bochs_pci_probe(struct pci_dev *pdev,
if (ret)
goto err_unload;
 
+   drm_fbdev_generic_setup(dev, 32);
return ret;
 
 err_unload:
-- 
2.9.3

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


[PATCH net V4] vhost: log dirty page correctly

2019-01-16 Thread Jason Wang
Vhost dirty page logging API is designed to sync through GPA. But we
try to log GIOVA when device IOTLB is enabled. This is wrong and may
lead to missing data after migration.

To solve this issue, when logging with device IOTLB enabled, we will:

1) reuse the device IOTLB translation result of GIOVA->HVA mapping to
   get HVA, for writable descriptor, get HVA through iovec. For used
   ring update, translate its GIOVA to HVA
2) traverse the GPA->HVA mapping to get the possible GPA and log
   through GPA. Pay attention this reverse mapping is not guaranteed
   to be unique, so we should log each possible GPA in this case.

This fix the failure of scp to guest during migration. In -next, we
will probably support passing GIOVA->GPA instead of GIOVA->HVA.

Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Reported-by: Jintack Lim 
Cc: Jintack Lim 
Signed-off-by: Jason Wang 
---
Changes from V3:
- make sure each part of the hva was logged when crossing the boundary
  of memory regions
Changes from V2:
- check and log the case of range overlap
- remove unnecessary u64 cast
- use smp_wmb() for the case of device IOTLB as well
Changes from V1:
- return error instead of warn
---
 drivers/vhost/net.c   |  3 +-
 drivers/vhost/vhost.c | 97 ---
 drivers/vhost/vhost.h |  3 +-
 3 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 36f3d0f49e60..bca86bf7189f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1236,7 +1236,8 @@ static void handle_rx(struct vhost_net *net)
if (nvq->done_idx > VHOST_NET_BATCH)
vhost_net_signal_used(nvq);
if (unlikely(vq_log))
-   vhost_log_write(vq, vq_log, log, vhost_len);
+   vhost_log_write(vq, vq_log, log, vhost_len,
+   vq->iov, in);
total_len += vhost_len;
if (unlikely(vhost_exceeds_weight(++recv_pkts, total_len))) {
vhost_poll_queue(>poll);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9f7942cbcbb2..babbb32b9bf0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1733,13 +1733,87 @@ static int log_write(void __user *log_base,
return r;
 }
 
+static int log_write_hva(struct vhost_virtqueue *vq, u64 hva, u64 len)
+{
+   struct vhost_umem *umem = vq->umem;
+   struct vhost_umem_node *u;
+   u64 start, end, l, min;
+   int r;
+   bool hit = false;
+
+   while (len) {
+   min = len;
+   /* More than one GPAs can be mapped into a single HVA. So
+* iterate all possible umems here to be safe.
+*/
+   list_for_each_entry(u, >umem_list, link) {
+   if (u->userspace_addr > hva - 1 + len ||
+   u->userspace_addr - 1 + u->size < hva)
+   continue;
+   start = max(u->userspace_addr, hva);
+   end = min(u->userspace_addr - 1 + u->size,
+ hva - 1 + len);
+   l = end - start + 1;
+   r = log_write(vq->log_base,
+ u->start + start - u->userspace_addr,
+ l);
+   if (r < 0)
+   return r;
+   hit = true;
+   min = min(l, min);
+   }
+
+   if (!hit)
+   return -EFAULT;
+
+   len -= min;
+   hva += min;
+   }
+
+   return 0;
+}
+
+static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len)
+{
+   struct iovec iov[64];
+   int i, ret;
+
+   if (!vq->iotlb)
+   return log_write(vq->log_base, vq->log_addr + used_offset, len);
+
+   ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
+len, iov, 64, VHOST_ACCESS_WO);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < ret; i++) {
+   ret = log_write_hva(vq, (uintptr_t)iov[i].iov_base,
+   iov[i].iov_len);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+
 int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
-   unsigned int log_num, u64 len)
+   unsigned int log_num, u64 len, struct iovec *iov, int count)
 {
int i, r;
 
/* Make sure data written is seen before log. */
smp_wmb();
+
+   if (vq->iotlb) {
+   for (i = 0; i < count; i++) {
+   r = log_write_hva(vq, (uintptr_t)iov[i].iov_base,
+ iov[i].iov_len);
+   if (r < 0)
+   return r;
+  

Re: [PATCH v3 2/3] virtio-balloon: improve update_balloon_size_func

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 07, 2019 at 03:01:05PM +0800, Wei Wang wrote:
> There is no need to update the balloon actual register when there is no
> ballooning request. This patch avoids update_balloon_size when diff is 0.
> 
> Signed-off-by: Wei Wang 
> Reviewed-by: Cornelia Huck 
> Reviewed-by: Halil Pasic 

It sounds reasonable but it does have a side effect of
not writing into actual anymore.

I am not sure actual never can get out of sync as a result.

So better deferred, pls send this in the next merge window.




> ---
>  drivers/virtio/virtio_balloon.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index fb12fe2..e33dc8e 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -457,9 +457,12 @@ static void update_balloon_size_func(struct work_struct 
> *work)
> update_balloon_size_work);
>   diff = towards_target(vb);
>  
> + if (!diff)
> + return;
> +
>   if (diff > 0)
>   diff -= fill_balloon(vb, diff);
> - else if (diff < 0)
> + else
>   diff += leak_balloon(vb, -diff);
>   update_balloon_size(vb);
>  
> -- 
> 2.7.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dave Chinner
On Mon, Jan 14, 2019 at 01:35:57PM -0800, Dan Williams wrote:
> On Mon, Jan 14, 2019 at 1:25 PM Dave Chinner  wrote:
> >
> > On Mon, Jan 14, 2019 at 02:15:40AM -0500, Pankaj Gupta wrote:
> > >
> > > > > Until you have images (and hence host page cache) shared between
> > > > > multiple guests. People will want to do this, because it means they
> > > > > only need a single set of pages in host memory for executable
> > > > > binaries rather than a set of pages per guest. Then you have
> > > > > multiple guests being able to detect residency of the same set of
> > > > > pages. If the guests can then, in any way, control eviction of the
> > > > > pages from the host cache, then we have a guest-to-guest information
> > > > > leak channel.
> > > >
> > > > I don't think we should ever be considering something that would allow a
> > > > guest to evict page's from the host's pagecache [1].  The guest should
> > > > be able to kick its own references to the host's pagecache out of its
> > > > own pagecache, but not be able to influence whether the host or another
> > > > guest has a read-only mapping cached.
> > > >
> > > > [1] Unless the guest is allowed to modify the host's file; obviously
> > > > truncation, holepunching, etc are going to evict pages from the host's
> > > > page cache.
> > >
> > > This is so correct. Guest does not not evict host page cache pages 
> > > directly.
> >
> > They don't right now.
> >
> > But someone is going to end up asking for discard to work so that
> > the guest can free unused space in the underlying spares image (i.e.
> > make use of fstrim or mount -o discard) because they have workloads
> > that have bursts of space usage and they need to trim the image
> > files afterwards to keep their overall space usage under control.
> >
> > And then
> 
> ...we reject / push back on that patch citing the above concern.

So at what point do we draw the line?

We're allowing writable DAX mappings, but as I've pointed out that
means we are going to be allowing  a potential information leak via
files with shared extents to be directly mapped and written to.

But we won't allow useful admin operations that allow better
management of host side storage space similar to how normal image
files are used by guests because it's an information leak vector?

That's splitting some really fine hairs there...

> > > In case of virtio-pmem & DAX, guest clears guest page cache exceptional 
> > > entries.
> > > Its solely decision of host to take action on the host page cache pages.
> > >
> > > In case of virtio-pmem, guest does not modify host file directly i.e don't
> > > perform hole punch & truncation operation directly on host file.
> >
> > ... this will no longer be true, and the nuclear landmine in this
> > driver interface will have been armed
> 
> I agree with the need to be careful when / if explicit cache control
> is added, but that's not the case today.

"if"?

I expect it to be "when", not if. Expect the worst, plan for it now.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 1/3] drm/qxl: add mode/framebuffer check functions

2019-01-16 Thread Gerd Hoffmann
Add a helper functions to check video modes.  Also add a helper to check
framebuffer buffer objects, using the former for consistency.  That way
we should not fail in qxl_primary_atomic_check() because video modes
which are too big will not be added to the mode list in the first place.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_display.c | 37 +++--
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index 1f8fddcc34..07a37d52c4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -191,6 +191,21 @@ void qxl_display_read_client_monitors_config(struct 
qxl_device *qdev)
}
 }
 
+static int qxl_check_mode(struct qxl_device *qdev,
+ unsigned int width,
+ unsigned int height)
+{
+   if (width * height * 4 > qdev->vram_size)
+   return -ENOMEM;
+   return 0;
+}
+
+static int qxl_check_framebuffer(struct qxl_device *qdev,
+struct qxl_bo *bo)
+{
+   return qxl_check_mode(qdev, bo->surf.width, bo->surf.height);
+}
+
 static int qxl_add_monitors_config_modes(struct drm_connector *connector,
  unsigned *pwidth,
  unsigned *pheight)
@@ -466,12 +481,7 @@ static int qxl_primary_atomic_check(struct drm_plane 
*plane,
 
bo = gem_to_qxl_bo(state->fb->obj[0]);
 
-   if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
-   DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
-   return -EINVAL;
-   }
-
-   return 0;
+   return qxl_check_framebuffer(qdev, bo);
 }
 
 static int qxl_primary_apply_cursor(struct drm_plane *plane)
@@ -941,20 +951,11 @@ static enum drm_mode_status qxl_conn_mode_valid(struct 
drm_connector *connector,
 {
struct drm_device *ddev = connector->dev;
struct qxl_device *qdev = ddev->dev_private;
-   int i;
 
-   /* TODO: is this called for user defined modes? (xrandr --add-mode)
-* TODO: check that the mode fits in the framebuffer */
+   if (qxl_check_mode(qdev, mode->hdisplay, mode->vdisplay) != 0)
+   return MODE_BAD;
 
-   if (qdev->monitors_config_width == mode->hdisplay &&
-   qdev->monitors_config_height == mode->vdisplay)
-   return MODE_OK;
-
-   for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
-   if (common_modes[i].w == mode->hdisplay && common_modes[i].h == 
mode->vdisplay)
-   return MODE_OK;
-   }
-   return MODE_BAD;
+   return MODE_OK;
 }
 
 static struct drm_encoder *qxl_best_encoder(struct drm_connector *connector)
-- 
2.9.3

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


Re: [PATCH v3 1/3] virtio-balloon: tweak config_changed implementation

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 07, 2019 at 02:49:03PM +0100, Christian Borntraeger wrote:
> 
> 
> On 07.01.2019 08:01, Wei Wang wrote:
> > virtio-ccw has deadlock issues with reading the config space inside the
> > interrupt context, so we tweak the virtballoon_changed implementation
> > by moving the config read operations into the related workqueue contexts.
> > The config_read_bitmap is used as a flag to the workqueue callbacks
> > about the related config fields that need to be read.
> > 
> > The cmd_id_received is also renamed to cmd_id_received_cache, and
> > the value should be obtained via virtio_balloon_cmd_id_received.
> > 
> > Reported-by: Christian Borntraeger 
> > Signed-off-by: Wei Wang 
> > Reviewed-by: Cornelia Huck 
> > Reviewed-by: Halil Pasic 
> 
> Together with 
>   virtio_pci: use queue idx instead of array idx to set up the vq
>   virtio: don't allocate vqs when names[i] = NULL
> 
> Tested-by: Christian Borntraeger 
> 
> as already said, would be good to add cc stable (and a Fixes: tag)


I don't think you did that.
Did it manually as the pull needed some manual handling
but I won't next time and you will have to send it to
Greg yourself.


> 
> > ---
> >  drivers/virtio/virtio_balloon.c | 98 
> > +++--
> >  1 file changed, 65 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c 
> > b/drivers/virtio/virtio_balloon.c
> > index 728ecd1..fb12fe2 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -61,6 +61,10 @@ enum virtio_balloon_vq {
> > VIRTIO_BALLOON_VQ_MAX
> >  };
> >  
> > +enum virtio_balloon_config_read {
> > +   VIRTIO_BALLOON_CONFIG_READ_CMD_ID = 0,
> > +};
> > +
> >  struct virtio_balloon {
> > struct virtio_device *vdev;
> > struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *free_page_vq;
> > @@ -77,14 +81,20 @@ struct virtio_balloon {
> > /* Prevent updating balloon when it is being canceled. */
> > spinlock_t stop_update_lock;
> > bool stop_update;
> > +   /* Bitmap to indicate if reading the related config fields are needed */
> > +   unsigned long config_read_bitmap;
> >  
> > /* The list of allocated free pages, waiting to be given back to mm */
> > struct list_head free_page_list;
> > spinlock_t free_page_list_lock;
> > /* The number of free page blocks on the above list */
> > unsigned long num_free_page_blocks;
> > -   /* The cmd id received from host */
> > -   u32 cmd_id_received;
> > +   /*
> > +* The cmd id received from host.
> > +* Read it via virtio_balloon_cmd_id_received to get the latest value
> > +* sent from host.
> > +*/
> > +   u32 cmd_id_received_cache;
> > /* The cmd id that is actively in use */
> > __virtio32 cmd_id_active;
> > /* Buffer to store the stop sign */
> > @@ -390,37 +400,31 @@ static unsigned long return_free_pages_to_mm(struct 
> > virtio_balloon *vb,
> > return num_returned;
> >  }
> >  
> > +static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb)
> > +{
> > +   if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> > +   return;
> > +
> > +   /* No need to queue the work if the bit was already set. */
> > +   if (test_and_set_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID,
> > +>config_read_bitmap))
> > +   return;
> > +
> > +   queue_work(vb->balloon_wq, >report_free_page_work);
> > +}
> > +
> >  static void virtballoon_changed(struct virtio_device *vdev)
> >  {
> > struct virtio_balloon *vb = vdev->priv;
> > unsigned long flags;
> > -   s64 diff = towards_target(vb);
> > -
> > -   if (diff) {
> > -   spin_lock_irqsave(>stop_update_lock, flags);
> > -   if (!vb->stop_update)
> > -   queue_work(system_freezable_wq,
> > -  >update_balloon_size_work);
> > -   spin_unlock_irqrestore(>stop_update_lock, flags);
> > -   }
> >  
> > -   if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > -   virtio_cread(vdev, struct virtio_balloon_config,
> > -free_page_report_cmd_id, >cmd_id_received);
> > -   if (vb->cmd_id_received == VIRTIO_BALLOON_CMD_ID_DONE) {
> > -   /* Pass ULONG_MAX to give back all the free pages */
> > -   return_free_pages_to_mm(vb, ULONG_MAX);
> > -   } else if (vb->cmd_id_received != VIRTIO_BALLOON_CMD_ID_STOP &&
> > -  vb->cmd_id_received !=
> > -  virtio32_to_cpu(vdev, vb->cmd_id_active)) {
> > -   spin_lock_irqsave(>stop_update_lock, flags);
> > -   if (!vb->stop_update) {
> > -   queue_work(vb->balloon_wq,
> > -  >report_free_page_work);
> > -   }
> > -   spin_unlock_irqrestore(>stop_update_lock, flags);
> > -   }
> > +   

Re: [PATCH 1/3] swiotlb: Export maximum allocation size

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 14, 2019 at 03:49:07PM -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote:
> > On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote:
> > > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if 
> > > they
> > > need to limit the size of pages.
> > 
> > That function just exports the overall size of the swiotlb aperture, no?
> > What I need here is the maximum size for a single mapping.
> 
> Yes. The other drivers just assumed that if there is SWIOTLB they would use
> the smaller size by default (as in they knew the limitation).
> 
> But I agree it would be better to have something smarter - and also convert 
> the
> DRM drivers to piggy back on this.
> 
> Or alternatively we could make SWIOTLB handle bigger sizes..


Just a thought: is it a good idea to teach blk_queue_max_segment_size
to get the dma size? This will help us find other devices
possibly missing this check.


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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 14, 2019 at 09:19:35PM +0100, Christoph Hellwig wrote:
> > Christoph is saying that !IOMMU_PLATFORM devices should hide the
> > compatibility code in a special per-device DMA API implementation.
> > Which would be fine especially if we can manage not to introduce a bunch
> > of indirect calls all over the place and hurt performance.  It's just
> > that the benefit is unlikely to be big (e.g. we can't also get rid of
> > the virtio specific memory barriers) so no one was motivated enough to
> > work on it.
> 
> No.  

Oh ok then.

> The problem is that we still haven't fully specified what
> IOMMU_PLATFORM and !IOMMU_PLATFORM actually mean.  Your
> "ACCESS_PLATFORM/ORDER_PLATFORM" commit in the virtio-spec repo
> improves it a little bit, but it is still far from enough.
> 
> As a start VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_ACCESS_PLATFORM
> absolutely MUST be set for hardware implementations.  Otherwise said
> hardware has no chance of working on anything but the most x86-like
> systems.

I think you are exaggerating a little here.  Limited access with e.g.
need to flush caches is common on lower end but less common on higher
end systems used for virtualization.  As you point out that changes with
e.g. SEV but then SEV is a pretty niche thing so far.

So I would make that a SHOULD probably.

> Second software implementations SHOULD set VIRTIO_F_ACCESS_PLATFORM,
> because otherwise we can't add proper handling for things like SEV or
> the IBM "secure hypervisor" thing.

Yes. If host does not have access to all of memory it SHOULD set
VIRTIO_F_ACCESS_PLATFORM.

It seems rather straight-forward to add conformance clauses
for this, thanks for reminding me.


> Last but not least a lot of wording outside the area describing these
> flags really needs some major updates in terms of DMA access.

Thanks for the reminder. Yes generally spec tends to still say e.g.
"physical address" where it really means a "DMA address" in Linux terms.
Whether changing that will make things much clearer I'm not sure. E.g.
hardware designers don't much care I guess.

If you want to list the most problematic cases, e.g. in a github issue,
we might get to clarifying them sooner.

Thanks!

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


Re: [PATCH 1/3] swiotlb: Export maximum allocation size

2019-01-16 Thread Konrad Rzeszutek Wilk
On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote:
> On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote:
> > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if 
> > they
> > need to limit the size of pages.
> 
> That function just exports the overall size of the swiotlb aperture, no?
> What I need here is the maximum size for a single mapping.

Yes. The other drivers just assumed that if there is SWIOTLB they would use
the smaller size by default (as in they knew the limitation).

But I agree it would be better to have something smarter - and also convert the
DRM drivers to piggy back on this.

Or alternatively we could make SWIOTLB handle bigger sizes..
> 
> Regards,
> 
>   Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dave Chinner
On Mon, Jan 14, 2019 at 02:15:40AM -0500, Pankaj Gupta wrote:
> 
> > > Until you have images (and hence host page cache) shared between
> > > multiple guests. People will want to do this, because it means they
> > > only need a single set of pages in host memory for executable
> > > binaries rather than a set of pages per guest. Then you have
> > > multiple guests being able to detect residency of the same set of
> > > pages. If the guests can then, in any way, control eviction of the
> > > pages from the host cache, then we have a guest-to-guest information
> > > leak channel.
> > 
> > I don't think we should ever be considering something that would allow a
> > guest to evict page's from the host's pagecache [1].  The guest should
> > be able to kick its own references to the host's pagecache out of its
> > own pagecache, but not be able to influence whether the host or another
> > guest has a read-only mapping cached.
> > 
> > [1] Unless the guest is allowed to modify the host's file; obviously
> > truncation, holepunching, etc are going to evict pages from the host's
> > page cache.
> 
> This is so correct. Guest does not not evict host page cache pages directly. 

They don't right now.

But someone is going to end up asking for discard to work so that
the guest can free unused space in the underlying spares image (i.e.
make use of fstrim or mount -o discard) because they have workloads
that have bursts of space usage and they need to trim the image
files afterwards to keep their overall space usage under control.

And then

> In case of virtio-pmem & DAX, guest clears guest page cache exceptional 
> entries.
> Its solely decision of host to take action on the host page cache pages.
> 
> In case of virtio-pmem, guest does not modify host file directly i.e don't
> perform hole punch & truncation operation directly on host file.  

... this will no longer be true, and the nuclear landmine in this
driver interface will have been armed

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dan Williams
On Mon, Jan 14, 2019 at 1:25 PM Dave Chinner  wrote:
>
> On Mon, Jan 14, 2019 at 02:15:40AM -0500, Pankaj Gupta wrote:
> >
> > > > Until you have images (and hence host page cache) shared between
> > > > multiple guests. People will want to do this, because it means they
> > > > only need a single set of pages in host memory for executable
> > > > binaries rather than a set of pages per guest. Then you have
> > > > multiple guests being able to detect residency of the same set of
> > > > pages. If the guests can then, in any way, control eviction of the
> > > > pages from the host cache, then we have a guest-to-guest information
> > > > leak channel.
> > >
> > > I don't think we should ever be considering something that would allow a
> > > guest to evict page's from the host's pagecache [1].  The guest should
> > > be able to kick its own references to the host's pagecache out of its
> > > own pagecache, but not be able to influence whether the host or another
> > > guest has a read-only mapping cached.
> > >
> > > [1] Unless the guest is allowed to modify the host's file; obviously
> > > truncation, holepunching, etc are going to evict pages from the host's
> > > page cache.
> >
> > This is so correct. Guest does not not evict host page cache pages directly.
>
> They don't right now.
>
> But someone is going to end up asking for discard to work so that
> the guest can free unused space in the underlying spares image (i.e.
> make use of fstrim or mount -o discard) because they have workloads
> that have bursts of space usage and they need to trim the image
> files afterwards to keep their overall space usage under control.
>
> And then

...we reject / push back on that patch citing the above concern.

> > In case of virtio-pmem & DAX, guest clears guest page cache exceptional 
> > entries.
> > Its solely decision of host to take action on the host page cache pages.
> >
> > In case of virtio-pmem, guest does not modify host file directly i.e don't
> > perform hole punch & truncation operation directly on host file.
>
> ... this will no longer be true, and the nuclear landmine in this
> driver interface will have been armed

I agree with the need to be careful when / if explicit cache control
is added, but that's not the case today.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Christoph Hellwig
On Mon, Jan 14, 2019 at 07:12:08PM +, Robin Murphy wrote:
> Now, in theory, yes, the real issue at hand is not unique to virtio-blk nor 
> SEV - any driver whose device has a sufficiently large DMA segment size and 
> who manages to get sufficient physically-contiguous memory could 
> technically generate a scatterlist segment longer than SWIOTLB can handle. 
> However, in practice that basically never happens, not least because very 
> few drivers ever override the default 64K DMA segment limit. AFAICS nothing 
> in drivers/virtio is calling dma_set_max_seg_size() or otherwise assigning 
> any dma_parms to replace the defaults either, so the really interesting 
> question here is how are these apparently-out-of-spec 256K segments getting 
> generated at all?

drivers/block/virtio_blk.c:virtblk_probe():

/* Host can optionally specify maximum segment size and number of
 * segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
   struct virtio_blk_config, size_max, );
if (!err)
blk_queue_max_segment_size(q, v);
else
blk_queue_max_segment_size(q, -1U);

So it really is virtio_blk that is special here.  The host could
set VIRTIO_BLK_F_SIZE_MAX to paper over the problem, but I really
think we need a dma_max_segment_size API that is wired up through
the dma_map_ops to sort this out for real.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 14, 2019 at 07:12:08PM +, Robin Murphy wrote:
> On 14/01/2019 18:20, Michael S. Tsirkin wrote:
> > On Mon, Jan 14, 2019 at 08:41:37PM +0800, Jason Wang wrote:
> > > 
> > > On 2019/1/14 下午5:50, Christoph Hellwig wrote:
> > > > On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:
> > > > > On 2019/1/11 下午5:15, Joerg Roedel wrote:
> > > > > > On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:
> > > > > > > Just wonder if my understanding is correct IOMMU_PLATFORM must be 
> > > > > > > set for
> > > > > > > all virtio devices under AMD-SEV guests?
> > > > > > Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
> > > > > > aperture, because that memory is not encrypted. The guest bounces 
> > > > > > the
> > > > > > data then to its encrypted memory.
> > > > > > 
> > > > > > Regards,
> > > > > > 
> > > > > > Joerg
> > > > > 
> > > > > Thanks, have you tested vhost-net in this case. I suspect it may not 
> > > > > work
> > > > Which brings me back to my pet pevee that we need to take actions
> > > > that virtio uses the proper dma mapping API by default with quirks
> > > > for legacy cases.  The magic bypass it uses is just causing problems
> > > > over problems.
> > > 
> > > 
> > > Yes, I fully agree with you. This is probably an exact example of such
> > > problem.
> > > 
> > > Thanks
> > 
> > I don't think so - the issue is really that DMA API does not yet handle
> > the SEV case 100% correctly. I suspect passthrough devices would have
> > the same issue.
> 
> Huh? Regardless of which virtio devices use it or not, the DMA API is
> handling the SEV case as correctly as it possibly can, by forcing everything
> through the unencrypted bounce buffer. If the segments being mapped are too
> big for that bounce buffer in the first place, there's nothing it can
> possibly do except fail, gracefully or otherwise.

It seems reasonable to be able to ask it what it's capabilities are
though.

> Now, in theory, yes, the real issue at hand is not unique to virtio-blk nor
> SEV - any driver whose device has a sufficiently large DMA segment size and
> who manages to get sufficient physically-contiguous memory could technically
> generate a scatterlist segment longer than SWIOTLB can handle. However, in
> practice that basically never happens, not least because very few drivers
> ever override the default 64K DMA segment limit. AFAICS nothing in
> drivers/virtio is calling dma_set_max_seg_size() or otherwise assigning any
> dma_parms to replace the defaults either, so the really interesting question
> here is how are these apparently-out-of-spec 256K segments getting generated
> at all?
> 
> Robin.

I guess this is what you are looking for:

/* Host can optionally specify maximum segment size and number of
 * segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
   struct virtio_blk_config, size_max, );
if (!err)
blk_queue_max_segment_size(q, v);
else
blk_queue_max_segment_size(q, -1U);

virtio isn't the only caller with a value >64K:

$ git grep -A1 blk_queue_max_segment_size
Documentation/block/biodoc.txt: blk_queue_max_segment_size(q, max_seg_size)
Documentation/block/biodoc.txt- Maximum size of a clustered segment, 
64kB default.
--
block/blk-settings.c: * blk_queue_max_segment_size - set max segment size for 
blk_rq_map_sg
block/blk-settings.c- * @q:  the request queue for the device
--
block/blk-settings.c:void blk_queue_max_segment_size(struct request_queue *q, 
unsigned int max_size)
block/blk-settings.c-{
--
block/blk-settings.c:EXPORT_SYMBOL(blk_queue_max_segment_size);
block/blk-settings.c-
--
drivers/block/mtip32xx/mtip32xx.c:  blk_queue_max_segment_size(dd->queue, 
0x40);
drivers/block/mtip32xx/mtip32xx.c-  blk_queue_io_min(dd->queue, 4096);
--
drivers/block/nbd.c:blk_queue_max_segment_size(disk->queue, UINT_MAX);
drivers/block/nbd.c-blk_queue_max_segments(disk->queue, USHRT_MAX);
--
drivers/block/ps3disk.c:blk_queue_max_segment_size(queue, 
dev->bounce_size);
drivers/block/ps3disk.c-
--
drivers/block/ps3vram.c:blk_queue_max_segment_size(queue, 
BLK_MAX_SEGMENT_SIZE);
drivers/block/ps3vram.c-blk_queue_max_hw_sectors(queue, 
BLK_SAFE_MAX_SECTORS);
--
drivers/block/rbd.c:blk_queue_max_segment_size(q, UINT_MAX);
drivers/block/rbd.c-blk_queue_io_min(q, objset_bytes);
--
drivers/block/sunvdc.c: blk_queue_max_segment_size(q, PAGE_SIZE);
drivers/block/sunvdc.c-
--
drivers/block/virtio_blk.c: blk_queue_max_segment_size(q, v);
drivers/block/virtio_blk.c- else
drivers/block/virtio_blk.c: blk_queue_max_segment_size(q, -1U);
drivers/block/virtio_blk.c-
--
drivers/block/xen-blkfront.c:   blk_queue_max_segment_size(rq, PAGE_SIZE);
drivers/block/xen-blkfront.c-
--
drivers/cdrom/gdrom.c:  blk_queue_max_segment_size(gd.gdrom_rq, 0x4);
drivers/cdrom/gdrom.c-  

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Christoph Hellwig
On Mon, Jan 14, 2019 at 01:20:45PM -0500, Michael S. Tsirkin wrote:
> I don't think so - the issue is really that DMA API does not yet handle
> the SEV case 100% correctly. I suspect passthrough devices would have
> the same issue.

The DMA API handles the SEV case perfectly.  Its just that virtio_blk
supports huge segments that virtio does not generally support, but that
is not related to SEV in any way.

> In fact whoever sets IOMMU_PLATFORM is completely unaffected by
> Christoph's pet peeve.

No, the above happens only when we set IOMMU_PLATFORM.

> Christoph is saying that !IOMMU_PLATFORM devices should hide the
> compatibility code in a special per-device DMA API implementation.
> Which would be fine especially if we can manage not to introduce a bunch
> of indirect calls all over the place and hurt performance.  It's just
> that the benefit is unlikely to be big (e.g. we can't also get rid of
> the virtio specific memory barriers) so no one was motivated enough to
> work on it.

No.  The problem is that we still haven't fully specified what
IOMMU_PLATFORM and !IOMMU_PLATFORM actually mean.  Your
"ACCESS_PLATFORM/ORDER_PLATFORM" commit in the virtio-spec repo
improves it a little bit, but it is still far from enough.

As a start VIRTIO_F_ORDER_PLATFORM and VIRTIO_F_ACCESS_PLATFORM
absolutely MUST be set for hardware implementations.  Otherwise said
hardware has no chance of working on anything but the most x86-like
systems.

Second software implementations SHOULD set VIRTIO_F_ACCESS_PLATFORM,
because otherwise we can't add proper handling for things like SEV or
the IBM "secure hypervisor" thing.

Last but not least a lot of wording outside the area describing these
flags really needs some major updates in terms of DMA access.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Robin Murphy

On 14/01/2019 18:20, Michael S. Tsirkin wrote:

On Mon, Jan 14, 2019 at 08:41:37PM +0800, Jason Wang wrote:


On 2019/1/14 下午5:50, Christoph Hellwig wrote:

On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:

On 2019/1/11 下午5:15, Joerg Roedel wrote:

On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:

Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
all virtio devices under AMD-SEV guests?

Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
aperture, because that memory is not encrypted. The guest bounces the
data then to its encrypted memory.

Regards,

Joerg


Thanks, have you tested vhost-net in this case. I suspect it may not work

Which brings me back to my pet pevee that we need to take actions
that virtio uses the proper dma mapping API by default with quirks
for legacy cases.  The magic bypass it uses is just causing problems
over problems.



Yes, I fully agree with you. This is probably an exact example of such
problem.

Thanks


I don't think so - the issue is really that DMA API does not yet handle
the SEV case 100% correctly. I suspect passthrough devices would have
the same issue.


Huh? Regardless of which virtio devices use it or not, the DMA API is 
handling the SEV case as correctly as it possibly can, by forcing 
everything through the unencrypted bounce buffer. If the segments being 
mapped are too big for that bounce buffer in the first place, there's 
nothing it can possibly do except fail, gracefully or otherwise.


Now, in theory, yes, the real issue at hand is not unique to virtio-blk 
nor SEV - any driver whose device has a sufficiently large DMA segment 
size and who manages to get sufficient physically-contiguous memory 
could technically generate a scatterlist segment longer than SWIOTLB can 
handle. However, in practice that basically never happens, not least 
because very few drivers ever override the default 64K DMA segment 
limit. AFAICS nothing in drivers/virtio is calling 
dma_set_max_seg_size() or otherwise assigning any dma_parms to replace 
the defaults either, so the really interesting question here is how are 
these apparently-out-of-spec 256K segments getting generated at all?


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

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Michael S. Tsirkin
On Mon, Jan 14, 2019 at 08:41:37PM +0800, Jason Wang wrote:
> 
> On 2019/1/14 下午5:50, Christoph Hellwig wrote:
> > On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:
> > > On 2019/1/11 下午5:15, Joerg Roedel wrote:
> > > > On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:
> > > > > Just wonder if my understanding is correct IOMMU_PLATFORM must be set 
> > > > > for
> > > > > all virtio devices under AMD-SEV guests?
> > > > Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
> > > > aperture, because that memory is not encrypted. The guest bounces the
> > > > data then to its encrypted memory.
> > > > 
> > > > Regards,
> > > > 
> > > > Joerg
> > > 
> > > Thanks, have you tested vhost-net in this case. I suspect it may not work
> > Which brings me back to my pet pevee that we need to take actions
> > that virtio uses the proper dma mapping API by default with quirks
> > for legacy cases.  The magic bypass it uses is just causing problems
> > over problems.
> 
> 
> Yes, I fully agree with you. This is probably an exact example of such
> problem.
> 
> Thanks

I don't think so - the issue is really that DMA API does not yet handle
the SEV case 100% correctly. I suspect passthrough devices would have
the same issue.

In fact whoever sets IOMMU_PLATFORM is completely unaffected by
Christoph's pet peeve.

Christoph is saying that !IOMMU_PLATFORM devices should hide the
compatibility code in a special per-device DMA API implementation.
Which would be fine especially if we can manage not to introduce a bunch
of indirect calls all over the place and hurt performance.  It's just
that the benefit is unlikely to be big (e.g. we can't also get rid of
the virtio specific memory barriers) so no one was motivated enough to
work on it.

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

Re: [PATCH v3 2/5] virtio-pmem: Add virtio pmem driver

2019-01-16 Thread Michael S. Tsirkin
On Wed, Jan 09, 2019 at 08:17:33PM +0530, Pankaj Gupta wrote:
> This patch adds virtio-pmem driver for KVM guest.
> 
> Guest reads the persistent memory range information from
> Qemu over VIRTIO and registers it on nvdimm_bus. It also
> creates a nd_region object with the persistent memory
> range information so that existing 'nvdimm/pmem' driver
> can reserve this into system memory map. This way
> 'virtio-pmem' driver uses existing functionality of pmem
> driver to register persistent memory compatible for DAX
> capable filesystems.
> 
> This also provides function to perform guest flush over
> VIRTIO from 'pmem' driver when userspace performs flush
> on DAX memory range.
> 
> Signed-off-by: Pankaj Gupta 
> ---
>  drivers/nvdimm/virtio_pmem.c |  84 ++
>  drivers/virtio/Kconfig   |  10 
>  drivers/virtio/Makefile  |   1 +
>  drivers/virtio/pmem.c| 124 
> +++
>  include/linux/virtio_pmem.h  |  60 +++
>  include/uapi/linux/virtio_ids.h  |   1 +
>  include/uapi/linux/virtio_pmem.h |  10 

As with any uapi change, you need to CC the virtio dev
mailing list (subscribers only, sorry about that).


>  7 files changed, 290 insertions(+)
>  create mode 100644 drivers/nvdimm/virtio_pmem.c
>  create mode 100644 drivers/virtio/pmem.c
>  create mode 100644 include/linux/virtio_pmem.h
>  create mode 100644 include/uapi/linux/virtio_pmem.h
> 
> diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> new file mode 100644
> index 000..2a1b1ba
> --- /dev/null
> +++ b/drivers/nvdimm/virtio_pmem.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * virtio_pmem.c: Virtio pmem Driver
> + *
> + * Discovers persistent memory range information
> + * from host and provides a virtio based flushing
> + * interface.
> + */
> +#include 
> +#include "nd.h"
> +
> + /* The interrupt handler */
> +void host_ack(struct virtqueue *vq)
> +{
> + unsigned int len;
> + unsigned long flags;
> + struct virtio_pmem_request *req, *req_buf;
> + struct virtio_pmem *vpmem = vq->vdev->priv;
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> + while ((req = virtqueue_get_buf(vq, )) != NULL) {
> + req->done = true;
> + wake_up(>host_acked);
> +
> + if (!list_empty(>req_list)) {
> + req_buf = list_first_entry(>req_list,
> + struct virtio_pmem_request, list);
> + list_del(>req_list);
> + req_buf->wq_buf_avail = true;
> + wake_up(_buf->wq_buf);
> + }
> + }
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(host_ack);
> +
> + /* The request submission function */
> +int virtio_pmem_flush(struct nd_region *nd_region)
> +{
> + int err;
> + unsigned long flags;
> + struct scatterlist *sgs[2], sg, ret;
> + struct virtio_device *vdev = nd_region->provider_data;
> + struct virtio_pmem *vpmem = vdev->priv;
> + struct virtio_pmem_request *req;
> +
> + might_sleep();
> + req = kmalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + req->done = req->wq_buf_avail = false;
> + strcpy(req->name, "FLUSH");
> + init_waitqueue_head(>host_acked);
> + init_waitqueue_head(>wq_buf);
> + sg_init_one(, req->name, strlen(req->name));
> + sgs[0] = 
> + sg_init_one(, >ret, sizeof(req->ret));
> + sgs[1] = 
> +
> + spin_lock_irqsave(>pmem_lock, flags);
> + err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req, GFP_ATOMIC);
> + if (err) {
> + dev_err(>dev, "failed to send command to virtio pmem 
> device\n");
> +
> + list_add_tail(>req_list, >list);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /* When host has read buffer, this completes via host_ack */
> + wait_event(req->wq_buf, req->wq_buf_avail);
> + spin_lock_irqsave(>pmem_lock, flags);
> + }
> + virtqueue_kick(vpmem->req_vq);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /* When host has read buffer, this completes via host_ack */
> + wait_event(req->host_acked, req->done);
> + err = req->ret;
> + kfree(req);
> +
> + return err;
> +};
> +EXPORT_SYMBOL_GPL(virtio_pmem_flush);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 3589764..9f634a2 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -42,6 +42,16 @@ config VIRTIO_PCI_LEGACY
>  
> If unsure, say Y.
>  
> +config VIRTIO_PMEM
> + tristate "Support for virtio pmem driver"
> + depends on VIRTIO
> + depends on LIBNVDIMM
> + help
> + This driver provides support for virtio based flushing interface
> + for persistent memory range.
> +
> + If unsure, say M.
> +
>  config 

Re: [PATCH net V3] vhost: log dirty page correctly

2019-01-16 Thread Michael S. Tsirkin
On Fri, Jan 11, 2019 at 12:00:36PM +0800, Jason Wang wrote:
> Vhost dirty page logging API is designed to sync through GPA. But we
> try to log GIOVA when device IOTLB is enabled. This is wrong and may
> lead to missing data after migration.
> 
> To solve this issue, when logging with device IOTLB enabled, we will:
> 
> 1) reuse the device IOTLB translation result of GIOVA->HVA mapping to
>get HVA, for writable descriptor, get HVA through iovec. For used
>ring update, translate its GIOVA to HVA
> 2) traverse the GPA->HVA mapping to get the possible GPA and log
>through GPA. Pay attention this reverse mapping is not guaranteed
>to be unique, so we should log each possible GPA in this case.
> 
> This fix the failure of scp to guest during migration. In -next, we
> will probably support passing GIOVA->GPA instead of GIOVA->HVA.
> 
> Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
> Reported-by: Jintack Lim 
> Cc: Jintack Lim 
> Signed-off-by: Jason Wang 
> ---
> Changes from V2:
> - check and log the case of range overlap
> - remove unnecessary u64 cast
> - use smp_wmb() for the case of device IOTLB as well
> Changes from V1:
> - return error instead of warn
> ---
>  drivers/vhost/net.c   |  3 +-
>  drivers/vhost/vhost.c | 88 ---
>  drivers/vhost/vhost.h |  3 +-
>  3 files changed, 78 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 36f3d0f49e60..bca86bf7189f 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -1236,7 +1236,8 @@ static void handle_rx(struct vhost_net *net)
>   if (nvq->done_idx > VHOST_NET_BATCH)
>   vhost_net_signal_used(nvq);
>   if (unlikely(vq_log))
> - vhost_log_write(vq, vq_log, log, vhost_len);
> + vhost_log_write(vq, vq_log, log, vhost_len,
> + vq->iov, in);
>   total_len += vhost_len;
>   if (unlikely(vhost_exceeds_weight(++recv_pkts, total_len))) {
>   vhost_poll_queue(>poll);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 9f7942cbcbb2..55a2e8f9f8ca 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1733,13 +1733,78 @@ static int log_write(void __user *log_base,
>   return r;
>  }
>  
> +static int log_write_hva(struct vhost_virtqueue *vq, u64 hva, u64 len)
> +{
> + struct vhost_umem *umem = vq->umem;
> + struct vhost_umem_node *u;
> + u64 start, end;
> + int r;
> + bool hit = false;
> +
> + /* More than one GPAs can be mapped into a single HVA. So
> +  * iterate all possible umems here to be safe.
> +  */
> + list_for_each_entry(u, >umem_list, link) {
> + if (u->userspace_addr > hva - 1 + len ||
> + u->userspace_addr - 1 + u->size < hva)
> + continue;
> + start = max(u->userspace_addr, hva);
> + end = min(u->userspace_addr - 1 + u->size, hva - 1 + len);
> + r = log_write(vq->log_base,
> +   u->start + start - u->userspace_addr,
> +   end - start + 1);
> + if (r < 0)
> + return r;
> + hit = true;
> + }
> +
> + if (!hit)
> + return -EFAULT;
> +
> + return 0;
> +}
> +

I definitely like the simplicity.

But there's one point left here: if len crosses a region boundary,
but doesn't all fall within a region, we don't consistently report -EFAULT.

So I suspect we need to start by finding a region that contains hva.
If there are many of these - move right to the end of the
leftmost one and then repeat until we run out of len
or fail to find a region and exit with -EFAULT.




> +static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len)
> +{
> + struct iovec iov[64];
> + int i, ret;
> +
> + if (!vq->iotlb)
> + return log_write(vq->log_base, vq->log_addr + used_offset, len);
> +
> + ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
> +  len, iov, 64, VHOST_ACCESS_WO);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < ret; i++) {
> + ret = log_write_hva(vq, (uintptr_t)iov[i].iov_base,
> + iov[i].iov_len);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
>  int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
> - unsigned int log_num, u64 len)
> + unsigned int log_num, u64 len, struct iovec *iov, int count)
>  {
>   int i, r;
>  
>   /* Make sure data written is seen before log. */
>   smp_wmb();
> +
> + if (vq->iotlb) {
> + for (i = 0; i < count; i++) {
> + r = log_write_hva(vq, (uintptr_t)iov[i].iov_base,
> + 

RE: [PATCH] VMCI: Verify PPNs before sending to device

2019-01-16 Thread Jorgen S. Hansen via Virtualization
> -Original Message-
> From: Jorgen Hansen [mailto:jhan...@vmware.com]
> Sent: Friday, January 11, 2019 8:58 AM
> To: linux-ker...@vger.kernel.org; virtualization@lists.linux-foundation.org
> Cc: gre...@linuxfoundation.org; Pv-drivers ;
> Jorgen S. Hansen 
> Subject: [PATCH] VMCI: Verify PPNs before sending to device
> 
> The current version of the VMCI device only supports 32 bit PPNs, so check
> whether we are truncating PPNs, and fail the operation if we do. One such
> check did exist, but was wrong. Another check was missing.
> 
> Testing through code modification: constructed PPN not representable by
> 32-bit and observed that failure was reported.
> 
> Fixes: 1f166439917b ("VMCI: guest side driver implementation.")
> Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
> 
> Signed-off-by: Jorgen Hansen 
> Reviewed-by: Adit Ranadive 
> Reviewed-by: Vishnu Dasa 
> ---
>  drivers/misc/vmw_vmci/vmci_guest.c  | 10 +++---
>  drivers/misc/vmw_vmci/vmci_queue_pair.c | 10 --
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/misc/vmw_vmci/vmci_guest.c
> b/drivers/misc/vmw_vmci/vmci_guest.c
> index dad5abee656e..02bb3866cf9e 100644
> --- a/drivers/misc/vmw_vmci/vmci_guest.c
> +++ b/drivers/misc/vmw_vmci/vmci_guest.c
> @@ -532,10 +532,14 @@ static int vmci_guest_probe_device(struct pci_dev
> *pdev,
>   if (capabilities & VMCI_CAPS_NOTIFICATIONS) {
>   unsigned long bitmap_ppn =
>   vmci_dev->notification_base >> PAGE_SHIFT;
> - if (!vmci_dbell_register_notification_bitmap(bitmap_ppn)) {
> + u32 bitmap_ppn32 = bitmap_ppn;
> +
> + if ((sizeof(bitmap_ppn) > sizeof(bitmap_ppn32)
> +  && bitmap_ppn != bitmap_ppn32) ||
> + !vmci_dbell_register_notification_bitmap(bitmap_ppn)) {
>   dev_warn(>dev,
> -  "VMCI device unable to register notification
> bitmap with PPN 0x%x\n",
> -  (u32) bitmap_ppn);
> +  "VMCI device unable to register notification
> bitmap with PPN 0x%lx\n",
> +  bitmap_ppn);
>   error = -ENXIO;
>   goto err_remove_vmci_dev_g;
>   }
> diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> index 264f4ed8eef2..1da4f6cb01b2 100644
> --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> @@ -465,9 +465,8 @@ static int qp_alloc_ppn_set(void *prod_q,
>   for (i = 0; i < num_produce_pages; i++) {
>   unsigned long pfn;
> 
> - produce_ppns[i] =
> - produce_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
> - pfn = produce_ppns[i];
> + pfn = produce_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
> + produce_ppns[i] = pfn;
> 
>   /* Fail allocation if PFN isn't supported by hypervisor. */
>   if (sizeof(pfn) > sizeof(*produce_ppns) @@ -478,9 +477,8
> @@ static int qp_alloc_ppn_set(void *prod_q,
>   for (i = 0; i < num_consume_pages; i++) {
>   unsigned long pfn;
> 
> - consume_ppns[i] =
> - consume_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
> - pfn = consume_ppns[i];
> + pfn = consume_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
> + consume_ppns[i] = pfn;
> 
>   /* Fail allocation if PFN isn't supported by hypervisor. */
>   if (sizeof(pfn) > sizeof(*consume_ppns)
> --
> 2.17.1

Please ignore this patch.

Thomas Hellstrom reminded me that the dma mask for the device should
be able to take care of this and since the VMCI device uses the default
DMA mask, the device should only get 32-bit addresses. So we should
Just remove the (wrong) overflow checks in vmci_queue_pair.c

Another patch supporting larger addresses will be sent out later.

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


Re: [Qemu-devel] [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Jan Kara
On Sat 12-01-19 21:17:46, Pankaj Gupta wrote:
> > > > Right. Thinking about this I would be more concerned about the fact that
> > > > guest can effectively pin amount of host's page cache upto size of the
> > > > device/file passed to guest as PMEM, can't it Pankaj? Or is there some
> > > > QEMU
> > > > magic that avoids this?
> > >
> > > Yes, guest will pin these host page cache pages using 'get_user_pages' by
> > > elevating the page reference count. But these pages can be reclaimed by
> > > host
> > > at any time when there is memory pressure.
> > 
> > Wait, how can the guest pin the host pages? I would expect this to
> > happen only when using vfio and device assignment. Otherwise, no the
> > host can't reclaim a pinned page, that's the whole point of a pin to
> > prevent the mm from reclaiming ownership.
> 
> yes. You are right I just used the pin word but it does not actually pin 
> pages 
> permanently. I had gone through the discussion on existing problems with 
> get_user_pages and DMA e.g [1] to understand Jan's POV. It does mention GUP 
> pin pages so I also used the word 'pin'. But guest does not permanently pin 
> these pages and these pages can be reclaimed by host.

OK, then I was just confused how virtio-pmem is going to work. Thanks for
explanation! So can I imagine this as guest mmaping the host file and
providing the mapped range as "NVDIMM pages" to the kernel inside the
guest? Or is it more complex?

Honza

-- 
Jan Kara 
SUSE Labs, CR
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Christoph Hellwig
On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:
>
> On 2019/1/11 下午5:15, Joerg Roedel wrote:
>> On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:
>>> Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
>>> all virtio devices under AMD-SEV guests?
>> Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
>> aperture, because that memory is not encrypted. The guest bounces the
>> data then to its encrypted memory.
>>
>> Regards,
>>
>>  Joerg
>
>
> Thanks, have you tested vhost-net in this case. I suspect it may not work

Which brings me back to my pet pevee that we need to take actions
that virtio uses the proper dma mapping API by default with quirks
for legacy cases.  The magic bypass it uses is just causing problems
over problems.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Jason Wang


On 2019/1/14 下午5:50, Christoph Hellwig wrote:

On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:

On 2019/1/11 下午5:15, Joerg Roedel wrote:

On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:

Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
all virtio devices under AMD-SEV guests?

Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
aperture, because that memory is not encrypted. The guest bounces the
data then to its encrypted memory.

Regards,

Joerg


Thanks, have you tested vhost-net in this case. I suspect it may not work

Which brings me back to my pet pevee that we need to take actions
that virtio uses the proper dma mapping API by default with quirks
for legacy cases.  The magic bypass it uses is just causing problems
over problems.



Yes, I fully agree with you. This is probably an exact example of such 
problem.


Thanks

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

Re: [PATCH 1/2] virtio: fix virtio_config_ops description

2019-01-16 Thread Cornelia Huck
On Thu,  3 Jan 2019 17:08:03 +0100
Cornelia Huck  wrote:

> - get_features has returned 64 bits since commit d025477368792
>   ("virtio: add support for 64 bit features.")
> - properly mark all optional callbacks
> 
> Signed-off-by: Cornelia Huck 
> ---
>  include/linux/virtio_config.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 32baf8e26735..7087ef946ba7 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -22,7 +22,7 @@ struct irq_affinity;
>   *   offset: the offset of the configuration field
>   *   buf: the buffer to read the field value from.
>   *   len: the length of the buffer
> - * @generation: config generation counter
> + * @generation: config generation counter (optional)
>   *   vdev: the virtio_device
>   *   Returns the config generation counter
>   * @get_status: read the status byte
> @@ -48,17 +48,17 @@ struct irq_affinity;
>   * @del_vqs: free virtqueues found by find_vqs().
>   * @get_features: get the array of feature bits for this device.
>   *   vdev: the virtio_device
> - *   Returns the first 32 feature bits (all we currently need).
> + *   Returns the first 64 feature bits (all we currently need).
>   * @finalize_features: confirm what device features we'll be using.
>   *   vdev: the virtio_device
>   *   This gives the final feature bits for the device: it can change
>   *   the dev->feature bits if it wants.
>   *   Returns 0 on success or error status
> - * @bus_name: return the bus name associated with the device
> + * @bus_name: return the bus name associated with the device (optional)
>   *   vdev: the virtio_device
>   *  This returns a pointer to the bus name a la pci_name from which
>   *  the caller can then copy.
> - * @set_vq_affinity: set the affinity for a virtqueue.
> + * @set_vq_affinity: set the affinity for a virtqueue (optional).
>   * @get_vq_affinity: get the affinity for a virtqueue (optional).
>   */
>  typedef void vq_callback_t(struct virtqueue *);

Ping. Any feedback on that patch?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [Qemu-devel] [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta


> > > > > Right. Thinking about this I would be more concerned about the fact
> > > > > that
> > > > > guest can effectively pin amount of host's page cache upto size of
> > > > > the
> > > > > device/file passed to guest as PMEM, can't it Pankaj? Or is there
> > > > > some
> > > > > QEMU
> > > > > magic that avoids this?
> > > >
> > > > Yes, guest will pin these host page cache pages using 'get_user_pages'
> > > > by
> > > > elevating the page reference count. But these pages can be reclaimed by
> > > > host
> > > > at any time when there is memory pressure.
> > > 
> > > Wait, how can the guest pin the host pages? I would expect this to
> > > happen only when using vfio and device assignment. Otherwise, no the
> > > host can't reclaim a pinned page, that's the whole point of a pin to
> > > prevent the mm from reclaiming ownership.
> > 
> > yes. You are right I just used the pin word but it does not actually pin
> > pages
> > permanently. I had gone through the discussion on existing problems with
> > get_user_pages and DMA e.g [1] to understand Jan's POV. It does mention GUP
> > pin pages so I also used the word 'pin'. But guest does not permanently pin
> > these pages and these pages can be reclaimed by host.
> 
> OK, then I was just confused how virtio-pmem is going to work. Thanks for
> explanation! So can I imagine this as guest mmaping the host file and
> providing the mapped range as "NVDIMM pages" to the kernel inside the
> guest? Or is it more complex?

yes, that's correct. Host's Qemu process virtual address range is used as guest 
physical 
address and a direct mapping(EPT/NPT) is established. At guest side, this 
physical memory
range is plugged into guest system memory map and DAX mapping is setup using 
nvdimm calls.

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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Jason Wang


On 2019/1/11 下午5:15, Joerg Roedel wrote:

On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:

Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
all virtio devices under AMD-SEV guests?

Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
aperture, because that memory is not encrypted. The guest bounces the
data then to its encrypted memory.

Regards,

Joerg



Thanks, have you tested vhost-net in this case. I suspect it may not work



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

Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dave Chinner
On Fri, Jan 11, 2019 at 02:45:04AM -0500, Pankaj Gupta wrote:
> 
> > 
> > On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> > >  This patch series has implementation for "virtio pmem".
> > >  "virtio pmem" is fake persistent memory(nvdimm) in guest
> > >  which allows to bypass the guest page cache. This also
> > >  implements a VIRTIO based asynchronous flush mechanism.
> > 
> > H. Sharing the host page cache direct into the guest VM. Sounds
> > like a good idea, but.
> > 
> > This means the guest VM can now run timing attacks to observe host
> > side page cache residency, and depending on the implementation I'm
> > guessing that the guest will be able to control host side page
> > cache eviction, too (e.g. via discard or hole punch operations).
> 
> Not sure how? this is similar to mmapping virtual memory by any userspace 
> process. Any host userspace process can do such attack on host page cache
> using mincore & mmap shared file. 

Mincore is for monitoring, not cached eviction. And it's not
required to observe cache residency, either. That's a wide open
field containing an uncountable number of moles...

> But i don't think guest can do this alone. For virtio-pmem usecase
> guest won't be using page cache so timing attack from only guest
> side is not possible unless host userspace can run checks on page
> cache eviction state using mincore etc.  As rightly described by
> Rik, guest will only access its own page cache pages and if guest
> page cache is managed directly by host, this saves alot of effort
> for guest in transferring guest state of page cache.  

Until you have images (and hence host page cache) shared between
multiple guests. People will want to do this, because it means they
only need a single set of pages in host memory for executable
binaries rather than a set of pages per guest. Then you have
multiple guests being able to detect residency of the same set of
pages. If the guests can then, in any way, control eviction of the
pages from the host cache, then we have a guest-to-guest information
leak channel.

i.e. it's something we need to be aware of and really careful about
enabling infrastructure that /will/ be abused if guests can find a
way to influence the host side cache residency.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] drm/cirrus: fix connector leak at unload

2019-01-16 Thread Gerd Hoffmann
On Fri, Jan 11, 2019 at 11:16:38PM +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2019 at 11:06:20PM +0100, Daniel Vetter wrote:
> > On Fri, Jan 11, 2019 at 09:02:34AM -0500, Rob Clark wrote:
> > > This fixes an '*ERROR* connector VGA-2 leaked!' splat at driver unload.
> > > 
> > > Signed-off-by: Rob Clark 
> > > ---
> > > Similar case to the issue that was fixed recently in drm/ast
> > 
> > Reviewed-by: Daniel Vetter 
> 
> Actually I just pushed a patch to drm-misc-next to rename this function to
> drm_helper_force_disable_all(), so you need to respin ... My r-b still
> holds.

Fixed and queued up.

thanks,
  Gerd

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


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dave Chinner
On Sun, Jan 13, 2019 at 03:38:21PM -0800, Matthew Wilcox wrote:
> On Mon, Jan 14, 2019 at 10:29:02AM +1100, Dave Chinner wrote:
> > Until you have images (and hence host page cache) shared between
> > multiple guests. People will want to do this, because it means they
> > only need a single set of pages in host memory for executable
> > binaries rather than a set of pages per guest. Then you have
> > multiple guests being able to detect residency of the same set of
> > pages. If the guests can then, in any way, control eviction of the
> > pages from the host cache, then we have a guest-to-guest information
> > leak channel.
> 
> I don't think we should ever be considering something that would allow a
> guest to evict page's from the host's pagecache [1].  The guest should
> be able to kick its own references to the host's pagecache out of its
> own pagecache, but not be able to influence whether the host or another
> guest has a read-only mapping cached.
> 
> [1] Unless the guest is allowed to modify the host's file; obviously
> truncation, holepunching, etc are going to evict pages from the host's
> page cache.

Right, and that's exactly what I mean by "we need to be real careful
with functionality like this".

To be honest, I really don't think I've even touched the surface
here.

e.g. Filesystems and storage can share logical and physical extents.
Which means that image files that share storage (e.g.  because they
are all cloned from the same master image and/or there's in-line
deduplication running on the storage) and can be directly accessed
by guests may very well be susceptible to detection of host side
deduplication and subsequent copy-on-write operations.

This really doesn't seem much different to me from the guest being
able to infer host side KSM page deduplication and COW operation in
the guest side page cache.  The only difference is that DAX is being
used to probe the host side page cache and storage rather than the
guest side.

IOWs, I suspect there's a world of pain waiting for us if we punch
huge holes through the virtual machine abstractions like this.

Improving performance is a laudible goal, but at what price?

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Matthew Wilcox
On Mon, Jan 14, 2019 at 10:29:02AM +1100, Dave Chinner wrote:
> Until you have images (and hence host page cache) shared between
> multiple guests. People will want to do this, because it means they
> only need a single set of pages in host memory for executable
> binaries rather than a set of pages per guest. Then you have
> multiple guests being able to detect residency of the same set of
> pages. If the guests can then, in any way, control eviction of the
> pages from the host cache, then we have a guest-to-guest information
> leak channel.

I don't think we should ever be considering something that would allow a
guest to evict page's from the host's pagecache [1].  The guest should
be able to kick its own references to the host's pagecache out of its
own pagecache, but not be able to influence whether the host or another
guest has a read-only mapping cached.

[1] Unless the guest is allowed to modify the host's file; obviously
truncation, holepunching, etc are going to evict pages from the host's
page cache.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta


> > Until you have images (and hence host page cache) shared between
> > multiple guests. People will want to do this, because it means they
> > only need a single set of pages in host memory for executable
> > binaries rather than a set of pages per guest. Then you have
> > multiple guests being able to detect residency of the same set of
> > pages. If the guests can then, in any way, control eviction of the
> > pages from the host cache, then we have a guest-to-guest information
> > leak channel.
> 
> I don't think we should ever be considering something that would allow a
> guest to evict page's from the host's pagecache [1].  The guest should
> be able to kick its own references to the host's pagecache out of its
> own pagecache, but not be able to influence whether the host or another
> guest has a read-only mapping cached.
> 
> [1] Unless the guest is allowed to modify the host's file; obviously
> truncation, holepunching, etc are going to evict pages from the host's
> page cache.

This is so correct. Guest does not not evict host page cache pages directly. 
In case of virtio-pmem & DAX, guest clears guest page cache exceptional entries.
Its solely decision of host to take action on the host page cache pages.

In case of virtio-pmem, guest does not modify host file directly i.e don't
perform hole punch & truncation operation directly on host file.  

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


Re: [Qemu-devel] [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta


> >
> >
> >
> > >
> > > On Thu 10-01-19 12:26:17, Dave Chinner wrote:
> > > > On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> > > > >  This patch series has implementation for "virtio pmem".
> > > > >  "virtio pmem" is fake persistent memory(nvdimm) in guest
> > > > >  which allows to bypass the guest page cache. This also
> > > > >  implements a VIRTIO based asynchronous flush mechanism.
> > > >
> > > > H. Sharing the host page cache direct into the guest VM. Sounds
> > > > like a good idea, but.
> > > >
> > > > This means the guest VM can now run timing attacks to observe host
> > > > side page cache residency, and depending on the implementation I'm
> > > > guessing that the guest will be able to control host side page
> > > > cache eviction, too (e.g. via discard or hole punch operations).
> > > >
> > > > Which means this functionality looks to me like a new vector for
> > > > information leakage into and out of the guest VM via guest
> > > > controlled host page cache manipulation.
> > > >
> > > > https://arxiv.org/pdf/1901.01161
> > > >
> > > > I might be wrong, but if I'm not we're going to have to be very
> > > > careful about how guest VMs can access and manipulate host side
> > > > resources like the page cache.
> > >
> > > Right. Thinking about this I would be more concerned about the fact that
> > > guest can effectively pin amount of host's page cache upto size of the
> > > device/file passed to guest as PMEM, can't it Pankaj? Or is there some
> > > QEMU
> > > magic that avoids this?
> >
> > Yes, guest will pin these host page cache pages using 'get_user_pages' by
> > elevating the page reference count. But these pages can be reclaimed by
> > host
> > at any time when there is memory pressure.
> 
> Wait, how can the guest pin the host pages? I would expect this to
> happen only when using vfio and device assignment. Otherwise, no the
> host can't reclaim a pinned page, that's the whole point of a pin to
> prevent the mm from reclaiming ownership.

yes. You are right I just used the pin word but it does not actually pin pages 
permanently. I had gone through the discussion on existing problems with 
get_user_pages and DMA e.g [1] to understand Jan's POV. It does mention GUP 
pin pages so I also used the word 'pin'. But guest does not permanently pin 
these pages and these pages can be reclaimed by host.

> 
> > KVM does not permanently pin pages. vfio does that but we are not using
> > it here.
> 
> Right, so I'm confused by your pin assertion above.

Sorry! for the confusion. 

[1] https://lwn.net/Articles/753027/

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


Re: [PATCH] drm/cirrus: fix connector leak at unload

2019-01-16 Thread Daniel Vetter
On Fri, Jan 11, 2019 at 11:06:20PM +0100, Daniel Vetter wrote:
> On Fri, Jan 11, 2019 at 09:02:34AM -0500, Rob Clark wrote:
> > This fixes an '*ERROR* connector VGA-2 leaked!' splat at driver unload.
> > 
> > Signed-off-by: Rob Clark 
> > ---
> > Similar case to the issue that was fixed recently in drm/ast
> 
> Reviewed-by: Daniel Vetter 

Actually I just pushed a patch to drm-misc-next to rename this function to
drm_helper_force_disable_all(), so you need to respin ... My r-b still
holds.
-Daniel

> 
> > 
> >  drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
> > b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> > index 4dd499c7d1ba..bb379ec4c182 100644
> > --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> > +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> > @@ -256,6 +256,8 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
> >  {
> > struct drm_framebuffer *gfb = gfbdev->gfb;
> >  
> > +   drm_crtc_force_disable_all(dev);
> > +
> > drm_fb_helper_unregister_fbi(>helper);
> >  
> > vfree(gfbdev->sysram);
> > -- 
> > 2.20.1
> > 
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta



> 
> On Thu 10-01-19 12:26:17, Dave Chinner wrote:
> > On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> > >  This patch series has implementation for "virtio pmem".
> > >  "virtio pmem" is fake persistent memory(nvdimm) in guest
> > >  which allows to bypass the guest page cache. This also
> > >  implements a VIRTIO based asynchronous flush mechanism.
> > 
> > H. Sharing the host page cache direct into the guest VM. Sounds
> > like a good idea, but.
> > 
> > This means the guest VM can now run timing attacks to observe host
> > side page cache residency, and depending on the implementation I'm
> > guessing that the guest will be able to control host side page
> > cache eviction, too (e.g. via discard or hole punch operations).
> > 
> > Which means this functionality looks to me like a new vector for
> > information leakage into and out of the guest VM via guest
> > controlled host page cache manipulation.
> > 
> > https://arxiv.org/pdf/1901.01161
> > 
> > I might be wrong, but if I'm not we're going to have to be very
> > careful about how guest VMs can access and manipulate host side
> > resources like the page cache.
> 
> Right. Thinking about this I would be more concerned about the fact that
> guest can effectively pin amount of host's page cache upto size of the
> device/file passed to guest as PMEM, can't it Pankaj? Or is there some QEMU
> magic that avoids this?

Yes, guest will pin these host page cache pages using 'get_user_pages' by
elevating the page reference count. But these pages can be reclaimed by host
at any time when there is memory pressure.

KVM does not permanently pin pages. vfio does that but we are not using
it here.

Could you please elaborate what you are thinking?

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


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dan Williams
On Sat, Jan 12, 2019 at 5:38 PM Pankaj Gupta  wrote:
>
>
>
> >
> > On Thu 10-01-19 12:26:17, Dave Chinner wrote:
> > > On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> > > >  This patch series has implementation for "virtio pmem".
> > > >  "virtio pmem" is fake persistent memory(nvdimm) in guest
> > > >  which allows to bypass the guest page cache. This also
> > > >  implements a VIRTIO based asynchronous flush mechanism.
> > >
> > > H. Sharing the host page cache direct into the guest VM. Sounds
> > > like a good idea, but.
> > >
> > > This means the guest VM can now run timing attacks to observe host
> > > side page cache residency, and depending on the implementation I'm
> > > guessing that the guest will be able to control host side page
> > > cache eviction, too (e.g. via discard or hole punch operations).
> > >
> > > Which means this functionality looks to me like a new vector for
> > > information leakage into and out of the guest VM via guest
> > > controlled host page cache manipulation.
> > >
> > > https://arxiv.org/pdf/1901.01161
> > >
> > > I might be wrong, but if I'm not we're going to have to be very
> > > careful about how guest VMs can access and manipulate host side
> > > resources like the page cache.
> >
> > Right. Thinking about this I would be more concerned about the fact that
> > guest can effectively pin amount of host's page cache upto size of the
> > device/file passed to guest as PMEM, can't it Pankaj? Or is there some QEMU
> > magic that avoids this?
>
> Yes, guest will pin these host page cache pages using 'get_user_pages' by
> elevating the page reference count. But these pages can be reclaimed by host
> at any time when there is memory pressure.

Wait, how can the guest pin the host pages? I would expect this to
happen only when using vfio and device assignment. Otherwise, no the
host can't reclaim a pinned page, that's the whole point of a pin to
prevent the mm from reclaiming ownership.

> KVM does not permanently pin pages. vfio does that but we are not using
> it here.

Right, so I'm confused by your pin assertion above.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] drm/cirrus: fix connector leak at unload

2019-01-16 Thread Daniel Vetter
On Fri, Jan 11, 2019 at 09:02:34AM -0500, Rob Clark wrote:
> This fixes an '*ERROR* connector VGA-2 leaked!' splat at driver unload.
> 
> Signed-off-by: Rob Clark 
> ---
> Similar case to the issue that was fixed recently in drm/ast

Reviewed-by: Daniel Vetter 

> 
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
> b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> index 4dd499c7d1ba..bb379ec4c182 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
> @@ -256,6 +256,8 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
>  {
>   struct drm_framebuffer *gfb = gfbdev->gfb;
>  
> + drm_crtc_force_disable_all(dev);
> +
>   drm_fb_helper_unregister_fbi(>helper);
>  
>   vfree(gfbdev->sysram);
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] vhost/vsock: fix vhost vsock cid hashing inconsistent

2019-01-16 Thread David Miller
From: Zha Bin 
Date: Tue,  8 Jan 2019 16:07:03 +0800

> The vsock core only supports 32bit CID, but the Virtio-vsock spec define
> CID (dst_cid and src_cid) as u64 and the upper 32bits is reserved as
> zero. This inconsistency causes one bug in vhost vsock driver. The
> scenarios is:
> 
>   0. A hash table (vhost_vsock_hash) is used to map an CID to a vsock
>   object. And hash_min() is used to compute the hash key. hash_min() is
>   defined as:
>   (sizeof(val) <= 4 ? hash_32(val, bits) : hash_long(val, bits)).
>   That means the hash algorithm has dependency on the size of macro
>   argument 'val'.
>   0. In function vhost_vsock_set_cid(), a 64bit CID is passed to
>   hash_min() to compute the hash key when inserting a vsock object into
>   the hash table.
>   0. In function vhost_vsock_get(), a 32bit CID is passed to hash_min()
>   to compute the hash key when looking up a vsock for an CID.
> 
> Because the different size of the CID, hash_min() returns different hash
> key, thus fails to look up the vsock object for an CID.
> 
> To fix this bug, we keep CID as u64 in the IOCTLs and virtio message
> headers, but explicitly convert u64 to u32 when deal with the hash table
> and vsock core.
> 
> Fixes: 834e772c8db0 ("vhost/vsock: fix use-after-free in network stack 
> callers")
> Link: https://github.com/stefanha/virtio/blob/vsock/trunk/content.tex
> Signed-off-by: Zha Bin 
> Reviewed-by: Liu Jiang 

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


Re: [PATCH] drm/qxl: drop prime import/export callbacks

2019-01-16 Thread Dave Airlie
On Thu, 10 Jan 2019 at 18:17, Gerd Hoffmann  wrote:
>
> Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> userspace.
>
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Dave Airlie 
> ---
>  drivers/gpu/drm/qxl/qxl_drv.c   |  4 
>  drivers/gpu/drm/qxl/qxl_prime.c | 14 --
>  2 files changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 13c8a662f9..ccb090f3ab 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -250,14 +250,10 @@ static struct drm_driver qxl_driver = {
>  #if defined(CONFIG_DEBUG_FS)
> .debugfs_init = qxl_debugfs_init,
>  #endif
> -   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_import = drm_gem_prime_import,
> .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,
> diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> index a55dece118..df65d3c1a7 100644
> --- a/drivers/gpu/drm/qxl/qxl_prime.c
> +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> @@ -38,20 +38,6 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> WARN_ONCE(1, "not implemented");
>  }
>
> -struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> -{
> -   WARN_ONCE(1, "not implemented");
> -   return ERR_PTR(-ENOSYS);
> -}
> -
> -struct drm_gem_object *qxl_gem_prime_import_sg_table(
> -   struct drm_device *dev, struct dma_buf_attachment *attach,
> -   struct sg_table *table)
> -{
> -   WARN_ONCE(1, "not implemented");
> -   return ERR_PTR(-ENOSYS);
> -}
> -
>  void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
>  {
> WARN_ONCE(1, "not implemented");
> --
> 2.9.3
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] drm/virtio: drop prime import/export callbacks

2019-01-16 Thread Dave Airlie
On Thu, 10 Jan 2019 at 21:16, Gerd Hoffmann  wrote:
>
> Also set prime_handle_to_fd and prime_fd_to_handle to NULL,
> so drm will not advertive DRM_PRIME_CAP_{IMPORT,EXPORT} to
> userspace.

Reviewed-by: Dave Airlie 
>
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h   |  4 
>  drivers/gpu/drm/virtio/virtgpu_drv.c   |  4 
>  drivers/gpu/drm/virtio/virtgpu_prime.c | 14 --
>  3 files changed, 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index cf896d8793..4f2f3c43a4 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -354,10 +354,6 @@ int virtio_gpu_object_wait(struct virtio_gpu_object *bo, 
> bool no_wait);
>  /* virtgpu_prime.c */
>  int virtgpu_gem_prime_pin(struct drm_gem_object *obj);
>  void virtgpu_gem_prime_unpin(struct drm_gem_object *obj);
> -struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj);
> -struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
> -   struct drm_device *dev, struct dma_buf_attachment *attach,
> -   struct sg_table *sgt);
>  void *virtgpu_gem_prime_vmap(struct drm_gem_object *obj);
>  void virtgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
>  int virtgpu_gem_prime_mmap(struct drm_gem_object *obj,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index af92964b68..b996ac1d4f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -205,14 +205,10 @@ static struct drm_driver driver = {
>  #if defined(CONFIG_DEBUG_FS)
> .debugfs_init = virtio_gpu_debugfs_init,
>  #endif
> -   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> -   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_import = drm_gem_prime_import,
> .gem_prime_pin = virtgpu_gem_prime_pin,
> .gem_prime_unpin = virtgpu_gem_prime_unpin,
> -   .gem_prime_get_sg_table = virtgpu_gem_prime_get_sg_table,
> -   .gem_prime_import_sg_table = virtgpu_gem_prime_import_sg_table,
> .gem_prime_vmap = virtgpu_gem_prime_vmap,
> .gem_prime_vunmap = virtgpu_gem_prime_vunmap,
> .gem_prime_mmap = virtgpu_gem_prime_mmap,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c 
> b/drivers/gpu/drm/virtio/virtgpu_prime.c
> index 86ce0ae93f..c59ec34c80 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -39,20 +39,6 @@ void virtgpu_gem_prime_unpin(struct drm_gem_object *obj)
> WARN_ONCE(1, "not implemented");
>  }
>
> -struct sg_table *virtgpu_gem_prime_get_sg_table(struct drm_gem_object *obj)
> -{
> -   WARN_ONCE(1, "not implemented");
> -   return ERR_PTR(-ENODEV);
> -}
> -
> -struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
> -   struct drm_device *dev, struct dma_buf_attachment *attach,
> -   struct sg_table *table)
> -{
> -   WARN_ONCE(1, "not implemented");
> -   return ERR_PTR(-ENODEV);
> -}
> -
>  void *virtgpu_gem_prime_vmap(struct drm_gem_object *obj)
>  {
> struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> --
> 2.9.3
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH] VMCI: Verify PPNs before sending to device

2019-01-16 Thread Jorgen Hansen via Virtualization
The current version of the VMCI device only supports 32 bit PPNs,
so check whether we are truncating PPNs, and fail the operation
if we do. One such check did exist, but was wrong. Another
check was missing.

Testing through code modification: constructed PPN not representable
by 32-bit and observed that failure was reported.

Fixes: 1f166439917b ("VMCI: guest side driver implementation.")
Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")

Signed-off-by: Jorgen Hansen 
Reviewed-by: Adit Ranadive 
Reviewed-by: Vishnu Dasa 
---
 drivers/misc/vmw_vmci/vmci_guest.c  | 10 +++---
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 10 --
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_guest.c 
b/drivers/misc/vmw_vmci/vmci_guest.c
index dad5abee656e..02bb3866cf9e 100644
--- a/drivers/misc/vmw_vmci/vmci_guest.c
+++ b/drivers/misc/vmw_vmci/vmci_guest.c
@@ -532,10 +532,14 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
if (capabilities & VMCI_CAPS_NOTIFICATIONS) {
unsigned long bitmap_ppn =
vmci_dev->notification_base >> PAGE_SHIFT;
-   if (!vmci_dbell_register_notification_bitmap(bitmap_ppn)) {
+   u32 bitmap_ppn32 = bitmap_ppn;
+
+   if ((sizeof(bitmap_ppn) > sizeof(bitmap_ppn32)
+&& bitmap_ppn != bitmap_ppn32) ||
+   !vmci_dbell_register_notification_bitmap(bitmap_ppn)) {
dev_warn(>dev,
-"VMCI device unable to register notification 
bitmap with PPN 0x%x\n",
-(u32) bitmap_ppn);
+"VMCI device unable to register notification 
bitmap with PPN 0x%lx\n",
+bitmap_ppn);
error = -ENXIO;
goto err_remove_vmci_dev_g;
}
diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index 264f4ed8eef2..1da4f6cb01b2 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -465,9 +465,8 @@ static int qp_alloc_ppn_set(void *prod_q,
for (i = 0; i < num_produce_pages; i++) {
unsigned long pfn;
 
-   produce_ppns[i] =
-   produce_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
-   pfn = produce_ppns[i];
+   pfn = produce_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
+   produce_ppns[i] = pfn;
 
/* Fail allocation if PFN isn't supported by hypervisor. */
if (sizeof(pfn) > sizeof(*produce_ppns)
@@ -478,9 +477,8 @@ static int qp_alloc_ppn_set(void *prod_q,
for (i = 0; i < num_consume_pages; i++) {
unsigned long pfn;
 
-   consume_ppns[i] =
-   consume_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
-   pfn = consume_ppns[i];
+   pfn = consume_q->kernel_if->u.g.pas[i] >> PAGE_SHIFT;
+   consume_ppns[i] = pfn;
 
/* Fail allocation if PFN isn't supported by hypervisor. */
if (sizeof(pfn) > sizeof(*consume_ppns)
-- 
2.17.1

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


Re: [Intel-gfx] [PATCH 7/7] drm: Split out drm_probe_helper.h

2019-01-16 Thread Daniel Vetter
On Tue, Dec 18, 2018 at 10:27:47AM +0100, Benjamin Gaignard wrote:
> Le lun. 17 déc. 2018 à 21:48, Rodrigo Vivi  a écrit :
> >
> > On Mon, Dec 17, 2018 at 08:43:03PM +0100, Daniel Vetter wrote:
> > > Having the probe helper stuff (which pretty much everyone needs) in
> > > the drm_crtc_helper.h file (which atomic drivers should never need) is
> > > confusing. Split them out.
> > >
> > > To make sure I actually achieved the goal here I went through all
> > > drivers. And indeed, all atomic drivers are now free of
> > > drm_crtc_helper.h includes.
> > >
> > > v2: Make it compile. There was so much compile fail on arm drivers
> > > that I figured I'll better not include any of the acks on v1.
> > >
> > > Signed-off-by: Daniel Vetter 
> > > Cc: linux-arm-ker...@lists.infradead.org
> > > Cc: virtualization@lists.linux-foundation.org
> > > Cc: etna...@lists.freedesktop.org
> > > Cc: linux-samsung-...@vger.kernel.org
> > > Cc: intel-...@lists.freedesktop.org
> >
> > Acked-by: Rodrigo Vivi 
> 
> With this version I'm able to compile sti driver so,
> 
> Acked-by: Benjamin Gaignard 

I merged the first 3 patches, but now I'm stuck because I'm missing review
on patches 5&6. Anyone volunteering?

Thanks, Daniel

> 
> >
> > > Cc: linux-media...@lists.infradead.org
> > > Cc: linux-amlo...@lists.infradead.org
> > > Cc: linux-arm-...@vger.kernel.org
> > > Cc: freedr...@lists.freedesktop.org
> > > Cc: nouv...@lists.freedesktop.org
> > > Cc: spice-de...@lists.freedesktop.org
> > > Cc: amd-...@lists.freedesktop.org
> > > Cc: linux-renesas-...@vger.kernel.org
> > > Cc: linux-rockc...@lists.infradead.org
> > > Cc: linux-st...@st-md-mailman.stormreply.com
> > > Cc: linux-te...@vger.kernel.org
> > > Cc: xen-de...@lists.xen.org
> > > ---
> > >  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  2 +-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  2 +-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  1 +
> > >  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
> > >  .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  |  2 +-
> > >  .../display/amdgpu_dm/amdgpu_dm_services.c|  2 +-
> > >  drivers/gpu/drm/arc/arcpgu_crtc.c |  2 +-
> > >  drivers/gpu/drm/arc/arcpgu_drv.c  |  2 +-
> > >  drivers/gpu/drm/arc/arcpgu_sim.c  |  2 +-
> > >  drivers/gpu/drm/arm/hdlcd_crtc.c  |  2 +-
> > >  drivers/gpu/drm/arm/hdlcd_drv.c   |  2 +-
> > >  drivers/gpu/drm/arm/malidp_crtc.c |  2 +-
> > >  drivers/gpu/drm/arm/malidp_drv.c  |  2 +-
> > >  drivers/gpu/drm/arm/malidp_mw.c   |  2 +-
> > >  drivers/gpu/drm/armada/armada_510.c   |  2 +-
> > >  drivers/gpu/drm/armada/armada_crtc.c  |  2 +-
> > >  drivers/gpu/drm/armada/armada_crtc.h  |  2 +
> > >  drivers/gpu/drm/armada/armada_drv.c   |  2 +-
> > >  drivers/gpu/drm/armada/armada_fb.c|  2 +-
> > >  drivers/gpu/drm/ast/ast_drv.c |  1 +
> > >  drivers/gpu/drm/ast/ast_mode.c|  1 +
> > >  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  2 +-
> > >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  2 +-
> > >  drivers/gpu/drm/bochs/bochs_drv.c |  1 +
> > >  drivers/gpu/drm/bochs/bochs_kms.c |  1 +
> > >  drivers/gpu/drm/bridge/adv7511/adv7511.h  |  5 +-
> > >  drivers/gpu/drm/bridge/analogix-anx78xx.c |  2 +-
> > >  .../drm/bridge/analogix/analogix_dp_core.c|  2 +-
> > >  drivers/gpu/drm/bridge/cdns-dsi.c |  2 +-
> > >  drivers/gpu/drm/bridge/dumb-vga-dac.c |  2 +-
> > >  .../bridge/megachips-stdp-ge-b850v3-fw.c  |  2 +-
> > >  drivers/gpu/drm/bridge/nxp-ptn3460.c  |  2 +-
> > >  drivers/gpu/drm/bridge/panel.c|  2 +-
> > >  drivers/gpu/drm/bridge/parade-ps8622.c|  2 +-
> > >  drivers/gpu/drm/bridge/sii902x.c  |  2 +-
> > >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
> > >  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  2 +-
> > >  drivers/gpu/drm/bridge/tc358764.c |  2 +-
> > >  drivers/gpu/drm/bridge/tc358767.c |  2 +-
> > >  drivers/gpu/drm/bridge/ti-sn65dsi86.c |  2 +-
> > >  drivers/gpu/drm/bridge/ti-tfp410.c|  2 +-
> > >  drivers/gpu/drm/cirrus/cirrus_drv.c   |  1 +
> > >  drivers/gpu/drm/cirrus/cirrus_mode.c  |  1 +
> > >  drivers/gpu/drm/drm_atomic_helper.c   |  1 -
> > >  drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
> > >  drivers/gpu/drm/drm_modeset_helper.c  |  2 +-
> > >  drivers/gpu/drm/drm_probe_helper.c|  2 +-
> > >  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
> > >  drivers/gpu/drm/etnaviv/etnaviv_drv.h |  1 -
> > >  drivers/gpu/drm/exynos/exynos_dp.c|  3 +-
> > >  drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  2 +-
> > >  drivers/gpu/drm/exynos/exynos_drm_dpi.c   |  2 +-
> > >  

Re: [PATCH v6 0/7] Add virtio-iommu driver

2019-01-16 Thread Jean-Philippe Brucker
On 11/01/2019 12:28, Joerg Roedel wrote:
> Hi Jean-Philippe,
> 
> On Thu, Dec 13, 2018 at 12:50:29PM +, Jean-Philippe Brucker wrote:
>> We already do deferred flush: UNMAP requests are added to the queue by
>> iommu_unmap(), and then flushed out by iotlb_sync(). So we switch to the
>> host only on iotlb_sync(), or when the request queue is full.
> 
> So the mappings can stay in place until iotlb_sync() returns? What
> happens when the guest sends a map-request for a region it sent and
> unmap request before, but did not call iotlb_sync inbetween?

At that point the unmap is still in the request queue, and the host will
handle it before getting to the map request. For correctness requests
are necessarily handled in-order by the host. So if the map and unmap
refer to the same domain and IOVA, the host will remove the old mapping
before creating the new one.

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


Re: [PATCH v6 0/7] Add virtio-iommu driver

2019-01-16 Thread Joerg Roedel
Hi Jean-Philippe,

On Thu, Dec 13, 2018 at 12:50:29PM +, Jean-Philippe Brucker wrote:
> We already do deferred flush: UNMAP requests are added to the queue by
> iommu_unmap(), and then flushed out by iotlb_sync(). So we switch to the
> host only on iotlb_sync(), or when the request queue is full.

So the mappings can stay in place until iotlb_sync() returns? What
happens when the guest sends a map-request for a region it sent and
unmap request before, but did not call iotlb_sync inbetween?

Regards,

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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Joerg Roedel
On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:
> Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
> all virtio devices under AMD-SEV guests?

Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
aperture, because that memory is not encrypted. The guest bounces the
data then to its encrypted memory.

Regards,

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


Re: [PATCH 1/3] swiotlb: Export maximum allocation size

2019-01-16 Thread Joerg Roedel
On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote:
> Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure if they
> need to limit the size of pages.

That function just exports the overall size of the swiotlb aperture, no?
What I need here is the maximum size for a single mapping.

Regards,

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


Re: [RFC PATCH V3 0/5] Hi:

2019-01-16 Thread Jason Wang


On 2019/1/8 下午6:12, Jason Wang wrote:


On 2019/1/7 下午10:47, Michael S. Tsirkin wrote:

On Mon, Jan 07, 2019 at 02:58:08PM +0800, Jason Wang wrote:

On 2019/1/5 上午5:41, Michael S. Tsirkin wrote:

On Sat, Dec 29, 2018 at 08:46:51PM +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.

I think it's a reasonable approach.
However I need to look at whether and which mmu notifiers are 
invoked before

writeback. Do you know?


I don't know but just looking at the MMU notifier ops definition, 
there's no

such callback if my understanding is correct.

Thanks

In that case how are you making sure used ring updates are written back?
If they aren't guest will crash ...



I think this is the writeback issue you mentioned early. I don't do a 
followup on the pointer but it looks to me some work is ongoing to fix 
the issue.


I can investigate it more, but it's not something new, consider the 
case of VFIO.


Thanks



Ok, after some investigation. The GUP + dirty pages issue is not easy to 
be fixed so it may still take a while.


An idea is switch back to copy_user() friends if we find the metadata 
page is not anonymous.


Does this sound good to you?

Thanks

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

[PATCH v4 05/16] drm/bochs: atomic: switch planes to atomic, wire up helpers.

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, step three.
Wire up atomic helpers.  Switch planes to atomic.

We are late to the party, the transitional helpers are gone,
so this can't be splitted into smaller steps any more.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_fbdev.c |  3 ++
 drivers/gpu/drm/bochs/bochs_kms.c   | 82 +++--
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index dd3c7df267..d9f3d42999 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -6,6 +6,7 @@
  */
 
 #include "bochs.h"
+#include 
 #include 
 
 /* -- */
@@ -149,6 +150,8 @@ bochs_gem_fb_create(struct drm_device *dev, struct drm_file 
*file,
 
 const struct drm_mode_config_funcs bochs_mode_funcs = {
.fb_create = bochs_gem_fb_create,
+   .atomic_check = drm_atomic_helper_check,
+   .atomic_commit = drm_atomic_helper_commit,
 };
 
 int bochs_fbdev_init(struct bochs_device *bochs)
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 56fd7be933..c6993c2d59 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -6,7 +6,9 @@
  */
 
 #include "bochs.h"
+#include 
 #include 
+#include 
 
 static int defx = 1024;
 static int defy = 768;
@@ -113,7 +115,7 @@ static int bochs_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *old_fb = crtc->primary->fb;
unsigned long irqflags;
 
-   crtc->primary->fb = fb;
+   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
bochs_crtc_mode_set_base(crtc, 0, 0, old_fb);
if (event) {
spin_lock_irqsave(>dev->event_lock, irqflags);
@@ -151,6 +153,9 @@ static const struct drm_crtc_funcs bochs_crtc_funcs = {
.set_config = drm_crtc_helper_set_config,
.destroy = drm_crtc_cleanup,
.page_flip = bochs_crtc_page_flip,
+   .reset = drm_atomic_helper_crtc_reset,
+   .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 static const struct drm_crtc_helper_funcs bochs_helper_funcs = {
@@ -169,6 +174,71 @@ static const uint32_t bochs_formats[] = {
DRM_FORMAT_BGRX,
 };
 
+static void bochs_plane_atomic_update(struct drm_plane *plane,
+ struct drm_plane_state *old_state)
+{
+   struct bochs_device *bochs = plane->dev->dev_private;
+   struct bochs_bo *bo;
+
+   if (!plane->state->fb)
+   return;
+   bo = gem_to_bochs_bo(plane->state->fb->obj[0]);
+   bochs_hw_setbase(bochs,
+plane->state->crtc_x,
+plane->state->crtc_y,
+bo->bo.offset);
+   bochs_hw_setformat(bochs, plane->state->fb->format);
+}
+
+static int bochs_plane_prepare_fb(struct drm_plane *plane,
+   struct drm_plane_state *new_state)
+{
+   struct bochs_bo *bo;
+   int ret;
+
+   if (!new_state->fb)
+   return 0;
+   bo = gem_to_bochs_bo(new_state->fb->obj[0]);
+
+   ret = ttm_bo_reserve(>bo, true, false, NULL);
+   if (ret)
+   return ret;
+   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL);
+   ttm_bo_unreserve(>bo);
+   return ret;
+}
+
+static void bochs_plane_cleanup_fb(struct drm_plane *plane,
+  struct drm_plane_state *old_state)
+{
+   struct bochs_bo *bo;
+   int ret;
+
+   if (!old_state->fb)
+   return;
+   bo = gem_to_bochs_bo(old_state->fb->obj[0]);
+   ret = ttm_bo_reserve(>bo, true, false, NULL);
+   if (ret)
+   return;
+   bochs_bo_unpin(bo);
+   ttm_bo_unreserve(>bo);
+}
+
+static const struct drm_plane_helper_funcs bochs_plane_helper_funcs = {
+   .atomic_update = bochs_plane_atomic_update,
+   .prepare_fb = bochs_plane_prepare_fb,
+   .cleanup_fb = bochs_plane_cleanup_fb,
+};
+
+static const struct drm_plane_funcs bochs_plane_funcs = {
+   .update_plane   = drm_atomic_helper_update_plane,
+   .disable_plane  = drm_atomic_helper_disable_plane,
+   .destroy= drm_primary_helper_destroy,
+   .reset  = drm_atomic_helper_plane_reset,
+   .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+   .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+};
+
 static struct drm_plane *bochs_primary_plane(struct drm_device *dev)
 {
struct drm_plane *primary;
@@ -181,16 +251,17 @@ static struct drm_plane *bochs_primary_plane(struct 
drm_device *dev)
}
 
ret = drm_universal_plane_init(dev, primary, 0,
-  _primary_helper_funcs,
+   

Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta


> 
> On Wed, Jan 09, 2019 at 08:17:31PM +0530, Pankaj Gupta wrote:
> >  This patch series has implementation for "virtio pmem".
> >  "virtio pmem" is fake persistent memory(nvdimm) in guest
> >  which allows to bypass the guest page cache. This also
> >  implements a VIRTIO based asynchronous flush mechanism.
> 
> H. Sharing the host page cache direct into the guest VM. Sounds
> like a good idea, but.
> 
> This means the guest VM can now run timing attacks to observe host
> side page cache residency, and depending on the implementation I'm
> guessing that the guest will be able to control host side page
> cache eviction, too (e.g. via discard or hole punch operations).

Not sure how? this is similar to mmapping virtual memory by any userspace 
process. Any host userspace process can do such attack on host page cache
using mincore & mmap shared file. 

But i don't think guest can do this alone. For virtio-pmem usecase guest 
won't be using page cache so timing attack from only guest side is not 
possible unless host userspace can run checks on page cache eviction state
using mincore etc. 

As rightly described by Rik, guest will only access its own page cache pages 
and if guest page cache is managed directly by host, this saves alot of 
effort for guest in transferring guest state of page cache.  

> 
> Which means this functionality looks to me like a new vector for
> information leakage into and out of the guest VM via guest
> controlled host page cache manipulation.
> 
> https://arxiv.org/pdf/1901.01161
> 
> I might be wrong, but if I'm not we're going to have to be very
> careful about how guest VMs can access and manipulate host side
> resources like the page cache.

If I am following correctly the discussions in MM thread. 
Important steps to mitigate this:

* Avoid running mincore in privilege mode: to safeguard page evict state of any 
  page cache page.
* tweaking RWF_NOWAIT 

I think if we secure ways to find current state(cached/evicted) of a page in 
host, 
we should be able to mitigate the impact for any page cache page access attack 
including virtio-pmem.

Thanks,
Pankaj


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


[PATCH v4 10/16] drm/bochs: remove old bochs_crtc_* functions

2019-01-16 Thread Gerd Hoffmann
Remove the old, now unused crtc callbacks.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 81 ---
 1 file changed, 81 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 67c3674609..5b7e1a7c6b 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -20,74 +20,6 @@ MODULE_PARM_DESC(defy, "default y resolution");
 
 /* -- */
 
-static void bochs_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-   switch (mode) {
-   case DRM_MODE_DPMS_ON:
-   case DRM_MODE_DPMS_STANDBY:
-   case DRM_MODE_DPMS_SUSPEND:
-   case DRM_MODE_DPMS_OFF:
-   default:
-   return;
-   }
-}
-
-static int bochs_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-   struct drm_framebuffer *old_fb)
-{
-   struct bochs_device *bochs =
-   container_of(crtc, struct bochs_device, crtc);
-   struct bochs_bo *bo;
-   u64 gpu_addr = 0;
-   int ret;
-
-   if (old_fb) {
-   bo = gem_to_bochs_bo(old_fb->obj[0]);
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret) {
-   DRM_ERROR("failed to reserve old_fb bo\n");
-   } else {
-   bochs_bo_unpin(bo);
-   ttm_bo_unreserve(>bo);
-   }
-   }
-
-   if (WARN_ON(crtc->primary->fb == NULL))
-   return -EINVAL;
-
-   bo = gem_to_bochs_bo(crtc->primary->fb->obj[0]);
-   ret = ttm_bo_reserve(>bo, true, false, NULL);
-   if (ret)
-   return ret;
-
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, _addr);
-   if (ret) {
-   ttm_bo_unreserve(>bo);
-   return ret;
-   }
-
-   ttm_bo_unreserve(>bo);
-   bochs_hw_setbase(bochs, x, y, gpu_addr);
-   return 0;
-}
-
-static int bochs_crtc_mode_set(struct drm_crtc *crtc,
-  struct drm_display_mode *mode,
-  struct drm_display_mode *adjusted_mode,
-  int x, int y, struct drm_framebuffer *old_fb)
-{
-   struct bochs_device *bochs =
-   container_of(crtc, struct bochs_device, crtc);
-
-   if (WARN_ON(crtc->primary->fb == NULL))
-   return -EINVAL;
-
-   bochs_hw_setmode(bochs, mode);
-   bochs_hw_setformat(bochs, crtc->primary->fb->format);
-   bochs_crtc_mode_set_base(crtc, x, y, old_fb);
-   return 0;
-}
-
 static void bochs_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
struct bochs_device *bochs =
@@ -96,14 +28,6 @@ static void bochs_crtc_mode_set_nofb(struct drm_crtc *crtc)
bochs_hw_setmode(bochs, >mode);
 }
 
-static void bochs_crtc_prepare(struct drm_crtc *crtc)
-{
-}
-
-static void bochs_crtc_commit(struct drm_crtc *crtc)
-{
-}
-
 static void bochs_crtc_atomic_enable(struct drm_crtc *crtc,
 struct drm_crtc_state *old_crtc_state)
 {
@@ -138,12 +62,7 @@ static const struct drm_crtc_funcs bochs_crtc_funcs = {
 };
 
 static const struct drm_crtc_helper_funcs bochs_helper_funcs = {
-   .dpms = bochs_crtc_dpms,
-   .mode_set = bochs_crtc_mode_set,
-   .mode_set_base = bochs_crtc_mode_set_base,
.mode_set_nofb = bochs_crtc_mode_set_nofb,
-   .prepare = bochs_crtc_prepare,
-   .commit = bochs_crtc_commit,
.atomic_enable = bochs_crtc_atomic_enable,
.atomic_flush = bochs_crtc_atomic_flush,
 };
-- 
2.9.3

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


[PATCH v4 08/16] drm/bochs: atomic: use suspend/resume helpers

2019-01-16 Thread Gerd Hoffmann
Switch to atomic helpers: drm_mode_config_helper_suspend/resume().

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/bochs/bochs_drv.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index f3dd66ae99..08ba6029d2 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -103,11 +103,8 @@ static int bochs_pm_suspend(struct device *dev)
struct drm_device *drm_dev = pci_get_drvdata(pdev);
struct bochs_device *bochs = drm_dev->dev_private;
 
-   drm_kms_helper_poll_disable(drm_dev);
-
drm_fb_helper_set_suspend_unlocked(>fb.helper, 1);
-
-   return 0;
+   return drm_mode_config_helper_suspend(drm_dev);
 }
 
 static int bochs_pm_resume(struct device *dev)
@@ -116,12 +113,8 @@ static int bochs_pm_resume(struct device *dev)
struct drm_device *drm_dev = pci_get_drvdata(pdev);
struct bochs_device *bochs = drm_dev->dev_private;
 
-   drm_helper_resume_force_mode(drm_dev);
-
drm_fb_helper_set_suspend_unlocked(>fb.helper, 0);
-
-   drm_kms_helper_poll_enable(drm_dev);
-   return 0;
+   return drm_mode_config_helper_resume(drm_dev);
 }
 #endif
 
-- 
2.9.3

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


[PATCH v4 15/16] drm/bochs: drop old fbdev emulation code

2019-01-16 Thread Gerd Hoffmann
Not needed any more, bochs uses the generic emulation now.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs.h   |   9 ---
 drivers/gpu/drm/bochs/bochs_fbdev.c | 129 
 2 files changed, 138 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index ede22beb85..03711394f1 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -80,12 +80,6 @@ struct bochs_device {
struct ttm_bo_device bdev;
bool initialized;
} ttm;
-
-   /* fbdev */
-   struct {
-   struct drm_framebuffer *fb;
-   struct drm_fb_helper helper;
-   } fb;
 };
 
 struct bochs_bo {
@@ -157,7 +151,4 @@ int bochs_kms_init(struct bochs_device *bochs);
 void bochs_kms_fini(struct bochs_device *bochs);
 
 /* bochs_fbdev.c */
-int bochs_fbdev_init(struct bochs_device *bochs);
-void bochs_fbdev_fini(struct bochs_device *bochs);
-
 extern const struct drm_mode_config_funcs bochs_mode_funcs;
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index ccf783b038..7cac3f5253 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -11,124 +11,6 @@
 
 /* -- */
 
-static int bochsfb_mmap(struct fb_info *info,
-   struct vm_area_struct *vma)
-{
-   struct drm_fb_helper *fb_helper = info->par;
-   struct bochs_bo *bo = gem_to_bochs_bo(fb_helper->fb->obj[0]);
-
-   return ttm_fbdev_mmap(vma, >bo);
-}
-
-static struct fb_ops bochsfb_ops = {
-   .owner = THIS_MODULE,
-   DRM_FB_HELPER_DEFAULT_OPS,
-   .fb_fillrect = drm_fb_helper_cfb_fillrect,
-   .fb_copyarea = drm_fb_helper_cfb_copyarea,
-   .fb_imageblit = drm_fb_helper_cfb_imageblit,
-   .fb_mmap = bochsfb_mmap,
-};
-
-static int bochsfb_create_object(struct bochs_device *bochs,
-const struct drm_mode_fb_cmd2 *mode_cmd,
-struct drm_gem_object **gobj_p)
-{
-   struct drm_device *dev = bochs->dev;
-   struct drm_gem_object *gobj;
-   u32 size;
-   int ret = 0;
-
-   size = mode_cmd->pitches[0] * mode_cmd->height;
-   ret = bochs_gem_create(dev, size, true, );
-   if (ret)
-   return ret;
-
-   *gobj_p = gobj;
-   return ret;
-}
-
-static int bochsfb_create(struct drm_fb_helper *helper,
- struct drm_fb_helper_surface_size *sizes)
-{
-   struct bochs_device *bochs =
-   container_of(helper, struct bochs_device, fb.helper);
-   struct fb_info *info;
-   struct drm_framebuffer *fb;
-   struct drm_mode_fb_cmd2 mode_cmd;
-   struct drm_gem_object *gobj = NULL;
-   struct bochs_bo *bo = NULL;
-   int size, ret;
-
-   if (sizes->surface_bpp != 32)
-   return -EINVAL;
-
-   mode_cmd.width = sizes->surface_width;
-   mode_cmd.height = sizes->surface_height;
-   mode_cmd.pitches[0] = sizes->surface_width * 4;
-   mode_cmd.pixel_format = DRM_FORMAT_HOST_XRGB;
-   size = mode_cmd.pitches[0] * mode_cmd.height;
-
-   /* alloc, pin & map bo */
-   ret = bochsfb_create_object(bochs, _cmd, );
-   if (ret) {
-   DRM_ERROR("failed to create fbcon backing object %d\n", ret);
-   return ret;
-   }
-
-   bo = gem_to_bochs_bo(gobj);
-
-   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
-   if (ret) {
-   DRM_ERROR("failed to pin fbcon\n");
-   return ret;
-   }
-
-   ret = ttm_bo_kmap(>bo, 0, bo->bo.num_pages,
- >kmap);
-   if (ret) {
-   DRM_ERROR("failed to kmap fbcon\n");
-   return ret;
-   }
-
-   /* init fb device */
-   info = drm_fb_helper_alloc_fbi(helper);
-   if (IS_ERR(info)) {
-   DRM_ERROR("Failed to allocate fbi: %ld\n", PTR_ERR(info));
-   return PTR_ERR(info);
-   }
-
-   info->par = >fb.helper;
-
-   fb = drm_gem_fbdev_fb_create(bochs->dev, sizes, 0, gobj, NULL);
-   if (IS_ERR(fb)) {
-   DRM_ERROR("Failed to create framebuffer: %ld\n", PTR_ERR(fb));
-   return PTR_ERR(fb);
-   }
-
-   /* setup helper */
-   bochs->fb.helper.fb = fb;
-
-   strcpy(info->fix.id, "bochsdrmfb");
-
-   info->fbops = _ops;
-
-   drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
-   drm_fb_helper_fill_var(info, >fb.helper, sizes->fb_width,
-  sizes->fb_height);
-
-   info->screen_base = bo->kmap.virtual;
-   info->screen_size = size;
-
-   drm_vma_offset_remove(>bo.bdev->vma_manager, >bo.vma_node);
-   info->fix.smem_start = 0;
-   info->fix.smem_len = size;
-   return 0;
-}
-
-static 

[PATCH v4 16/16] drm/bochs: move remaining fb bits to kms

2019-01-16 Thread Gerd Hoffmann
bochs_fbdev.c is almost empty now.  Move the remaining framebuffer bits
over to bochs_kms.c.  Pure code motion. No functional change.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_fbdev.c | 29 -
 drivers/gpu/drm/bochs/bochs_kms.c   | 17 +
 drivers/gpu/drm/bochs/Makefile  |  2 +-
 3 files changed, 18 insertions(+), 30 deletions(-)
 delete mode 100644 drivers/gpu/drm/bochs/bochs_fbdev.c

diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c 
b/drivers/gpu/drm/bochs/bochs_fbdev.c
deleted file mode 100644
index 7cac3f5253..00
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include "bochs.h"
-#include 
-#include 
-
-/* -- */
-
-static struct drm_framebuffer *
-bochs_gem_fb_create(struct drm_device *dev, struct drm_file *file,
-   const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-   if (mode_cmd->pixel_format != DRM_FORMAT_XRGB &&
-   mode_cmd->pixel_format != DRM_FORMAT_BGRX)
-   return ERR_PTR(-EINVAL);
-
-   return drm_gem_fb_create(dev, file, mode_cmd);
-}
-
-const struct drm_mode_config_funcs bochs_mode_funcs = {
-   .fb_create = bochs_gem_fb_create,
-   .atomic_check = drm_atomic_helper_check,
-   .atomic_commit = drm_atomic_helper_commit,
-};
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index fc856a02a2..e9d5dbc346 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static int defx = 1024;
 static int defy = 768;
@@ -256,6 +257,22 @@ static void bochs_connector_init(struct drm_device *dev)
}
 }
 
+static struct drm_framebuffer *
+bochs_gem_fb_create(struct drm_device *dev, struct drm_file *file,
+   const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+   if (mode_cmd->pixel_format != DRM_FORMAT_XRGB &&
+   mode_cmd->pixel_format != DRM_FORMAT_BGRX)
+   return ERR_PTR(-EINVAL);
+
+   return drm_gem_fb_create(dev, file, mode_cmd);
+}
+
+const struct drm_mode_config_funcs bochs_mode_funcs = {
+   .fb_create = bochs_gem_fb_create,
+   .atomic_check = drm_atomic_helper_check,
+   .atomic_commit = drm_atomic_helper_commit,
+};
 
 int bochs_kms_init(struct bochs_device *bochs)
 {
diff --git a/drivers/gpu/drm/bochs/Makefile b/drivers/gpu/drm/bochs/Makefile
index 98ef60a19e..e9e0f8f5eb 100644
--- a/drivers/gpu/drm/bochs/Makefile
+++ b/drivers/gpu/drm/bochs/Makefile
@@ -1,3 +1,3 @@
-bochs-drm-y := bochs_drv.o bochs_mm.o bochs_kms.o bochs_fbdev.o bochs_hw.o
+bochs-drm-y := bochs_drv.o bochs_mm.o bochs_kms.o bochs_hw.o
 
 obj-$(CONFIG_DRM_BOCHS)+= bochs-drm.o
-- 
2.9.3

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


[PATCH v4 09/16] drm/bochs: atomic: set DRIVER_ATOMIC

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, final step.
Set the DRIVER_ATOMIC flag.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index 08ba6029d2..a8cb22cffe 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -81,7 +81,7 @@ static const struct file_operations bochs_fops = {
 };
 
 static struct drm_driver bochs_driver = {
-   .driver_features= DRIVER_GEM | DRIVER_MODESET,
+   .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
.fops   = _fops,
.name   = "bochs-drm",
.desc   = "bochs dispi vga interface (qemu stdvga)",
-- 
2.9.3

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


[PATCH v4 03/16] drm/bochs: atomic: add atomic_flush+atomic_enable callbacks.

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, step one.
Add atomic crtc helper callbacks.

Signed-off-by: Gerd Hoffmann 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index f7e6d1a9b3..2cbd406b1f 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -115,6 +115,29 @@ static int bochs_crtc_page_flip(struct drm_crtc *crtc,
return 0;
 }
 
+static void bochs_crtc_atomic_enable(struct drm_crtc *crtc,
+struct drm_crtc_state *old_crtc_state)
+{
+}
+
+static void bochs_crtc_atomic_flush(struct drm_crtc *crtc,
+   struct drm_crtc_state *old_crtc_state)
+{
+   struct drm_device *dev = crtc->dev;
+   struct drm_pending_vblank_event *event;
+
+   if (crtc->state && crtc->state->event) {
+   unsigned long irqflags;
+
+   spin_lock_irqsave(>event_lock, irqflags);
+   event = crtc->state->event;
+   crtc->state->event = NULL;
+   drm_crtc_send_vblank_event(crtc, event);
+   spin_unlock_irqrestore(>event_lock, irqflags);
+   }
+}
+
+
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs bochs_crtc_funcs = {
.set_config = drm_crtc_helper_set_config,
@@ -128,6 +151,8 @@ static const struct drm_crtc_helper_funcs 
bochs_helper_funcs = {
.mode_set_base = bochs_crtc_mode_set_base,
.prepare = bochs_crtc_prepare,
.commit = bochs_crtc_commit,
+   .atomic_enable = bochs_crtc_atomic_enable,
+   .atomic_flush = bochs_crtc_atomic_flush,
 };
 
 static const uint32_t bochs_formats[] = {
-- 
2.9.3

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


[PATCH v4 02/16] drm/bochs: split bochs_hw_setmode

2019-01-16 Thread Gerd Hoffmann
Create a separate bochs_hw_setformat function to configure
the framebuffer format (actually just the byteorder).

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs.h |  5 +++--
 drivers/gpu/drm/bochs/bochs_hw.c  | 19 ---
 drivers/gpu/drm/bochs/bochs_kms.c |  3 ++-
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index fb38c8b857..4dc1b6384e 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -121,8 +121,9 @@ int bochs_hw_init(struct drm_device *dev);
 void bochs_hw_fini(struct drm_device *dev);
 
 void bochs_hw_setmode(struct bochs_device *bochs,
- struct drm_display_mode *mode,
- const struct drm_format_info *format);
+ struct drm_display_mode *mode);
+void bochs_hw_setformat(struct bochs_device *bochs,
+   const struct drm_format_info *format);
 void bochs_hw_setbase(struct bochs_device *bochs,
  int x, int y, u64 addr);
 int bochs_hw_load_edid(struct bochs_device *bochs);
diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
index d0b4e1cee8..3e04b2f0ec 100644
--- a/drivers/gpu/drm/bochs/bochs_hw.c
+++ b/drivers/gpu/drm/bochs/bochs_hw.c
@@ -204,8 +204,7 @@ void bochs_hw_fini(struct drm_device *dev)
 }
 
 void bochs_hw_setmode(struct bochs_device *bochs,
- struct drm_display_mode *mode,
- const struct drm_format_info *format)
+ struct drm_display_mode *mode)
 {
bochs->xres = mode->hdisplay;
bochs->yres = mode->vdisplay;
@@ -213,12 +212,8 @@ void bochs_hw_setmode(struct bochs_device *bochs,
bochs->stride = mode->hdisplay * (bochs->bpp / 8);
bochs->yres_virtual = bochs->fb_size / bochs->stride;
 
-   DRM_DEBUG_DRIVER("%dx%d @ %d bpp, format %c%c%c%c, vy %d\n",
+   DRM_DEBUG_DRIVER("%dx%d @ %d bpp, vy %d\n",
 bochs->xres, bochs->yres, bochs->bpp,
-(format->format >>  0) & 0xff,
-(format->format >>  8) & 0xff,
-(format->format >> 16) & 0xff,
-(format->format >> 24) & 0xff,
 bochs->yres_virtual);
 
bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
@@ -236,6 +231,16 @@ void bochs_hw_setmode(struct bochs_device *bochs,
 
bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,
  VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED);
+}
+
+void bochs_hw_setformat(struct bochs_device *bochs,
+   const struct drm_format_info *format)
+{
+   DRM_DEBUG_DRIVER("format %c%c%c%c\n",
+(format->format >>  0) & 0xff,
+(format->format >>  8) & 0xff,
+(format->format >> 16) & 0xff,
+(format->format >> 24) & 0xff);
 
switch (format->format) {
case DRM_FORMAT_XRGB:
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index c8ce54498d..f7e6d1a9b3 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -80,7 +80,8 @@ static int bochs_crtc_mode_set(struct drm_crtc *crtc,
if (WARN_ON(crtc->primary->fb == NULL))
return -EINVAL;
 
-   bochs_hw_setmode(bochs, mode, crtc->primary->fb->format);
+   bochs_hw_setmode(bochs, mode);
+   bochs_hw_setformat(bochs, crtc->primary->fb->format);
bochs_crtc_mode_set_base(crtc, x, y, old_fb);
return 0;
 }
-- 
2.9.3

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


[PATCH v4 13/16] drm/bochs: add basic prime support

2019-01-16 Thread Gerd Hoffmann
Just enough to make the generic framebuffer emulation happy.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs.h |  7 ++
 drivers/gpu/drm/bochs/bochs_drv.c | 11 -
 drivers/gpu/drm/bochs/bochs_mm.c  | 49 +++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index d0d474e06f..ede22beb85 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -145,6 +145,13 @@ int bochs_dumb_mmap_offset(struct drm_file *file, struct 
drm_device *dev,
 int bochs_bo_pin(struct bochs_bo *bo, u32 pl_flag);
 int bochs_bo_unpin(struct bochs_bo *bo);
 
+int bochs_gem_prime_pin(struct drm_gem_object *obj);
+void bochs_gem_prime_unpin(struct drm_gem_object *obj);
+void *bochs_gem_prime_vmap(struct drm_gem_object *obj);
+void bochs_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
+int bochs_gem_prime_mmap(struct drm_gem_object *obj,
+struct vm_area_struct *vma);
+
 /* bochs_kms.c */
 int bochs_kms_init(struct bochs_device *bochs);
 void bochs_kms_fini(struct bochs_device *bochs);
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index a8cb22cffe..a3f4e21078 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -81,7 +81,8 @@ static const struct file_operations bochs_fops = {
 };
 
 static struct drm_driver bochs_driver = {
-   .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
+   .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
+ DRIVER_PRIME,
.fops   = _fops,
.name   = "bochs-drm",
.desc   = "bochs dispi vga interface (qemu stdvga)",
@@ -91,6 +92,14 @@ static struct drm_driver bochs_driver = {
.gem_free_object_unlocked = bochs_gem_free_object,
.dumb_create= bochs_dumb_create,
.dumb_map_offset= bochs_dumb_mmap_offset,
+
+   .gem_prime_export = drm_gem_prime_export,
+   .gem_prime_import = drm_gem_prime_import,
+   .gem_prime_pin = bochs_gem_prime_pin,
+   .gem_prime_unpin = bochs_gem_prime_unpin,
+   .gem_prime_vmap = bochs_gem_prime_vmap,
+   .gem_prime_vunmap = bochs_gem_prime_vunmap,
+   .gem_prime_mmap = bochs_gem_prime_mmap,
 };
 
 /* -- */
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index fcbf35456d..641a33f134 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -395,3 +395,52 @@ int bochs_dumb_mmap_offset(struct drm_file *file, struct 
drm_device *dev,
drm_gem_object_put_unlocked(obj);
return 0;
 }
+
+/* -- */
+
+int bochs_gem_prime_pin(struct drm_gem_object *obj)
+{
+   struct bochs_bo *bo = gem_to_bochs_bo(obj);
+
+   return bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
+}
+
+void bochs_gem_prime_unpin(struct drm_gem_object *obj)
+{
+   struct bochs_bo *bo = gem_to_bochs_bo(obj);
+
+   bochs_bo_unpin(bo);
+}
+
+void *bochs_gem_prime_vmap(struct drm_gem_object *obj)
+{
+   struct bochs_bo *bo = gem_to_bochs_bo(obj);
+   bool is_iomem;
+   int ret;
+
+   ret = bochs_bo_pin(bo, TTM_PL_FLAG_VRAM);
+   if (ret)
+   return NULL;
+   ret = ttm_bo_kmap(>bo, 0, bo->bo.num_pages, >kmap);
+   if (ret) {
+   bochs_bo_unpin(bo);
+   return NULL;
+   }
+   return ttm_kmap_obj_virtual(>kmap, _iomem);
+}
+
+void bochs_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
+{
+   struct bochs_bo *bo = gem_to_bochs_bo(obj);
+
+   ttm_bo_kunmap(>kmap);
+   bochs_bo_unpin(bo);
+}
+
+int bochs_gem_prime_mmap(struct drm_gem_object *obj,
+struct vm_area_struct *vma)
+{
+   struct bochs_bo *bo = gem_to_bochs_bo(obj);
+
+   return ttm_fbdev_mmap(vma, >bo);
+}
-- 
2.9.3

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


[PATCH v4 01/16] drm/bochs: encoder cleanup

2019-01-16 Thread Gerd Hoffmann
Most unused callbacks can be NULL pointers these days.
Drop a bunch of empty encoder callbacks.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 26 --
 1 file changed, 26 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index f87c284dd9..c8ce54498d 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -170,31 +170,6 @@ static void bochs_crtc_init(struct drm_device *dev)
drm_crtc_helper_add(crtc, _helper_funcs);
 }
 
-static void bochs_encoder_mode_set(struct drm_encoder *encoder,
-  struct drm_display_mode *mode,
-  struct drm_display_mode *adjusted_mode)
-{
-}
-
-static void bochs_encoder_dpms(struct drm_encoder *encoder, int state)
-{
-}
-
-static void bochs_encoder_prepare(struct drm_encoder *encoder)
-{
-}
-
-static void bochs_encoder_commit(struct drm_encoder *encoder)
-{
-}
-
-static const struct drm_encoder_helper_funcs bochs_encoder_helper_funcs = {
-   .dpms = bochs_encoder_dpms,
-   .mode_set = bochs_encoder_mode_set,
-   .prepare = bochs_encoder_prepare,
-   .commit = bochs_encoder_commit,
-};
-
 static const struct drm_encoder_funcs bochs_encoder_encoder_funcs = {
.destroy = drm_encoder_cleanup,
 };
@@ -207,7 +182,6 @@ static void bochs_encoder_init(struct drm_device *dev)
encoder->possible_crtcs = 0x1;
drm_encoder_init(dev, encoder, _encoder_encoder_funcs,
 DRM_MODE_ENCODER_DAC, NULL);
-   drm_encoder_helper_add(encoder, _encoder_helper_funcs);
 }
 
 
-- 
2.9.3

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


[PATCH v4 07/16] drm/bochs: atomic: use atomic page_flip helper

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, step five.
Use atomic page_flip helper for crtc.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 23 +--
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index 646f897cb2..67c3674609 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -104,27 +104,6 @@ static void bochs_crtc_commit(struct drm_crtc *crtc)
 {
 }
 
-static int bochs_crtc_page_flip(struct drm_crtc *crtc,
-   struct drm_framebuffer *fb,
-   struct drm_pending_vblank_event *event,
-   uint32_t page_flip_flags,
-   struct drm_modeset_acquire_ctx *ctx)
-{
-   struct bochs_device *bochs =
-   container_of(crtc, struct bochs_device, crtc);
-   struct drm_framebuffer *old_fb = crtc->primary->fb;
-   unsigned long irqflags;
-
-   drm_atomic_set_fb_for_plane(crtc->primary->state, fb);
-   bochs_crtc_mode_set_base(crtc, 0, 0, old_fb);
-   if (event) {
-   spin_lock_irqsave(>dev->event_lock, irqflags);
-   drm_crtc_send_vblank_event(crtc, event);
-   spin_unlock_irqrestore(>dev->event_lock, irqflags);
-   }
-   return 0;
-}
-
 static void bochs_crtc_atomic_enable(struct drm_crtc *crtc,
 struct drm_crtc_state *old_crtc_state)
 {
@@ -152,7 +131,7 @@ static void bochs_crtc_atomic_flush(struct drm_crtc *crtc,
 static const struct drm_crtc_funcs bochs_crtc_funcs = {
.set_config = drm_atomic_helper_set_config,
.destroy = drm_crtc_cleanup,
-   .page_flip = bochs_crtc_page_flip,
+   .page_flip = drm_atomic_helper_page_flip,
.reset = drm_atomic_helper_crtc_reset,
.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
-- 
2.9.3

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


[PATCH v4 06/16] drm/bochs: atomic: use atomic set_config helper

2019-01-16 Thread Gerd Hoffmann
Conversion to atomic modesetting, step four.
Use atomic set_config helper for crtc.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Oleksandr Andrushchenko 
Acked-by: Daniel Vetter 
---
 drivers/gpu/drm/bochs/bochs_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c 
b/drivers/gpu/drm/bochs/bochs_kms.c
index c6993c2d59..646f897cb2 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -150,7 +150,7 @@ static void bochs_crtc_atomic_flush(struct drm_crtc *crtc,
 
 /* These provide the minimum set of functions required to handle a CRTC */
 static const struct drm_crtc_funcs bochs_crtc_funcs = {
-   .set_config = drm_crtc_helper_set_config,
+   .set_config = drm_atomic_helper_set_config,
.destroy = drm_crtc_cleanup,
.page_flip = bochs_crtc_page_flip,
.reset = drm_atomic_helper_crtc_reset,
-- 
2.9.3

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


Re: [PATCH net-next] virtio_net: bulk free tx skbs

2019-01-16 Thread David Miller
From: "Michael S. Tsirkin" 
Date: Mon, 14 Jan 2019 20:34:26 -0500

> Use napi_consume_skb() to get bulk free.  Note that napi_consume_skb is
> safe to call in a non-napi context as long as the napi_budget flag is
> correct.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> My perf testing setup is down but it works fine on my devel box and
> should be fairly uncontroversial.

It would be uncontroversial if it compiled.

drivers/net/virtio_net.c: In function ‘free_old_xmit_skbs’:
drivers/net/virtio_net.c:1346:25: error: ‘use_napi’ undeclared (first use in 
this function); did you mean ‘used_math’?
   napi_consume_skb(skb, use_napi);
 ^~~~
 used_math
drivers/net/virtio_net.c:1346:25: note: each undeclared identifier is reported 
only once for each function it appears in
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Sam Ravnborg
Hi Daniel.

> v5: Actually try to sort them, and while at it, sort all the ones I
> touch.

Applied this variant on top of drm-misc and did a build test.
Looked good for ia64, x86 and alpha.

Took a closer look at the changes to atmel_hlcd - and they looked OK.

But I noticed that atmel_hlcdc uses only drm_kms_helper_poll_init() and
drm_kms_helper_poll_fini().
But there are no hits on DRM_CONNECTOR_POLL - so I think we maybe
have a driver here where we have plugged the drm_poll infrastructure,
but it is not in use.

>  include/drm/drm_crtc_helper.h | 16 ---

The list of include files in this file could be dropped and replaced by:
struct drm_connector;
struct drm_device;
struct drm_display_mode;
struct drm_encoder;
struct drm_framebuffer;
struct drm_mode_set;
struct drm_modeset_acquire_ctx;

I tried to do so on top of your patch.
But there were too many build errros and I somehow lost the motivation.


>  include/drm/drm_probe_helper.h| 27 +++
This on the other hand is fine - as expected as this is a new file.

But the above is just some random comments so:

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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Michael S. Tsirkin
On Tue, Jan 15, 2019 at 02:20:19PM +0100, Christoph Hellwig wrote:
> On Tue, Jan 15, 2019 at 09:37:42AM +0100, Joerg Roedel wrote:
> > On Mon, Jan 14, 2019 at 01:20:45PM -0500, Michael S. Tsirkin wrote:
> > > Which would be fine especially if we can manage not to introduce a bunch
> > > of indirect calls all over the place and hurt performance.
> > 
> > Which indirect calls? In case of unset dma_ops the DMA-API functions
> > call directly into the dma-direct implementation, no indirect calls at
> > all.
> 
> True.  But the NULL-ops dma direct case still has two issues that might
> not work for virtio:
> 
>  (a) if the architecture supports devices that are not DMA coherent it
>  will dip into a special allocator and do cache maintainance for
>  streaming mappings.  Although it would be a bit of a hack we could
>  work around this in virtio doings something like:
> 
> #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
> defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
> defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
>   dev->dma_coherent = true;
> #endif
> 
> except that won't work for mips, which has a mode where it does
> a system instead of device level coherency flag and thus doesn't
> look at this struct device field
> 
>  (b) we can still mangle the DMA address, either using the
>  dma_pfn_offset field in struct device, or by a full override
>  of __phys_to_dma / __dma_to_phys by the architecture code.
>  The first could be solved using a hack like the one above,
>  but the latter would be a little harder.  In the long run
>  I'd love to get rid of that hook and have everyone use the
>  generic offset code, but for that we first need to support
>  multiple ranges with different offset and do quite some
>  nasty arch code surgery.
>  
> So for the legacy virtio case I fear we need to keep local dma mapping
> implementation for now.  I just wish now recent hypervisor would ever
> offer devices in this broken legacy mode..

IIUC some emulated hardware has no reasonable way to specify that
some devices bypass the IOMMU, and no cheap way to cover all
memory with an IOMMU mapping.

So I suspect !ACCESS_PLATFORM will stay a supported configuration
forever :(



> > 
> > Regards,
> > 
> > Joerg
> ---end quoted text---
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Daniel Vetter
Having the probe helper stuff (which pretty much everyone needs) in
the drm_crtc_helper.h file (which atomic drivers should never need) is
confusing. Split them out.

To make sure I actually achieved the goal here I went through all
drivers. And indeed, all atomic drivers are now free of
drm_crtc_helper.h includes.

v2: Make it compile. There was so much compile fail on arm drivers
that I figured I'll better not include any of the acks on v1.

v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
there was still one, which this patch largely removes. Which means
rolling out lots more includes all over.

This will also conflict with ongoing drmP.h cleanup by others I
expect.

v3: Rebase on top of atomic bochs.

v4: Review from Laurent for bridge/rcar/omap/shmob/core bits:
- (re)move some of the added includes, use the better include files in
  other places (all suggested from Laurent adopted unchanged).
- sort alphabetically

v5: Actually try to sort them, and while at it, sort all the ones I
touch.

Cc: Sam Ravnborg 
Cc: Jani Nikula 
Cc: Laurent Pinchart 
Acked-by: Rodrigo Vivi 
Acked-by: Benjamin Gaignard 
Acked-by: Jani Nikula 
Acked-by: Neil Armstrong 
Acked-by: Oleksandr Andrushchenko 
Acked-by: CK Hu 
Acked-by: Alex Deucher 
Reviewed-by: Laurent Pinchart 
Acked-by: Liviu Dudau 
Signed-off-by: Daniel Vetter 
Cc: linux-arm-ker...@lists.infradead.org
Cc: virtualization@lists.linux-foundation.org
Cc: etna...@lists.freedesktop.org
Cc: linux-samsung-...@vger.kernel.org
Cc: intel-...@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: spice-de...@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-st...@st-md-mailman.stormreply.com
Cc: linux-te...@vger.kernel.org
Cc: xen-de...@lists.xen.org
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  1 +
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  |  2 +-
 .../display/amdgpu_dm/amdgpu_dm_services.c|  2 +-
 drivers/gpu/drm/arc/arcpgu_crtc.c |  2 +-
 drivers/gpu/drm/arc/arcpgu_drv.c  |  6 ++---
 drivers/gpu/drm/arc/arcpgu_sim.c  |  2 +-
 drivers/gpu/drm/arm/hdlcd_crtc.c  |  4 +--
 drivers/gpu/drm/arm/hdlcd_drv.c   |  4 +--
 drivers/gpu/drm/arm/malidp_crtc.c |  2 +-
 drivers/gpu/drm/arm/malidp_drv.c  |  2 +-
 drivers/gpu/drm/arm/malidp_mw.c   |  2 +-
 drivers/gpu/drm/armada/armada_510.c   |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c  |  2 +-
 drivers/gpu/drm/armada/armada_crtc.h  |  2 ++
 drivers/gpu/drm/armada/armada_drv.c   |  2 +-
 drivers/gpu/drm/armada/armada_fb.c|  2 +-
 drivers/gpu/drm/ast/ast_drv.c |  1 +
 drivers/gpu/drm/ast/ast_mode.c|  1 +
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  2 +-
 drivers/gpu/drm/bochs/bochs_drv.c |  1 +
 drivers/gpu/drm/bochs/bochs_kms.c |  1 +
 drivers/gpu/drm/bridge/adv7511/adv7511.h  |  4 ++-
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c  |  1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  2 +-
 .../drm/bridge/analogix/analogix_dp_core.c|  2 +-
 drivers/gpu/drm/bridge/cdns-dsi.c |  2 +-
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  2 +-
 .../bridge/megachips-stdp-ge-b850v3-fw.c  |  2 +-
 drivers/gpu/drm/bridge/nxp-ptn3460.c  |  2 +-
 drivers/gpu/drm/bridge/panel.c|  2 +-
 drivers/gpu/drm/bridge/parade-ps8622.c|  2 +-
 drivers/gpu/drm/bridge/sii902x.c  |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  2 +-
 drivers/gpu/drm/bridge/tc358764.c |  2 +-
 drivers/gpu/drm/bridge/tc358767.c |  2 +-
 drivers/gpu/drm/bridge/ti-sn65dsi86.c |  2 +-
 drivers/gpu/drm/bridge/ti-tfp410.c|  2 +-
 drivers/gpu/drm/cirrus/cirrus_drv.c   |  1 +
 drivers/gpu/drm/cirrus/cirrus_mode.c  |  1 +
 drivers/gpu/drm/drm_atomic_helper.c   |  1 -
 drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
 drivers/gpu/drm/drm_modeset_helper.c  |  2 +-
 drivers/gpu/drm/drm_probe_helper.c|  2 +-
 drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.h |  1 -
 drivers/gpu/drm/exynos/exynos_dp.c|  3 ++-
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  2 +-
 

Re: [PATCH 3/3] virtio-blk: Consider dma_max_mapping_size() for maximum segment size

2019-01-16 Thread Joerg Roedel
On Wed, Jan 16, 2019 at 09:05:40AM -0500, Michael S. Tsirkin wrote:
> On Tue, Jan 15, 2019 at 02:22:57PM +0100, Joerg Roedel wrote:
> > +   max_size = dma_max_mapping_size(>dev);
> > +
> 
> 
> Should this be limited to ACCESS_PLATFORM?
> 
> I see no reason to limit this without as guest can
> access any memory.

Actually, yes. This should be inside a use_dma_api check. I had it in
v1, but it went away without the vring layer for propagating the limit.
I'll add that again.

Thanks,

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


[PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Daniel Vetter
Having the probe helper stuff (which pretty much everyone needs) in
the drm_crtc_helper.h file (which atomic drivers should never need) is
confusing. Split them out.

To make sure I actually achieved the goal here I went through all
drivers. And indeed, all atomic drivers are now free of
drm_crtc_helper.h includes.

v2: Make it compile. There was so much compile fail on arm drivers
that I figured I'll better not include any of the acks on v1.

v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
there was still one, which this patch largely removes. Which means
rolling out lots more includes all over.

This will also conflict with ongoing drmP.h cleanup by others I
expect.

v3: Rebase on top of atomic bochs.

v4: Review from Laurent for bridge/rcar/omap/shmob/core bits:
- (re)move some of the added includes, use the better include files in
  other places (all suggested from Laurent adopted unchanged).
- sort alphabetically

Cc: Sam Ravnborg 
Cc: Jani Nikula 
Cc: Laurent Pinchart 
Acked-by: Rodrigo Vivi 
Acked-by: Benjamin Gaignard 
Acked-by: Jani Nikula 
Acked-by: Neil Armstrong 
Acked-by: Oleksandr Andrushchenko 
Acked-by: CK Hu 
Acked-by: Alex Deucher 
Reviewed-by: Laurent Pinchart 
Signed-off-by: Daniel Vetter 
Cc: linux-arm-ker...@lists.infradead.org
Cc: virtualization@lists.linux-foundation.org
Cc: etna...@lists.freedesktop.org
Cc: linux-samsung-...@vger.kernel.org
Cc: intel-...@lists.freedesktop.org
Cc: linux-media...@lists.infradead.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: nouv...@lists.freedesktop.org
Cc: spice-de...@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-st...@st-md-mailman.stormreply.com
Cc: linux-te...@vger.kernel.org
Cc: xen-de...@lists.xen.org
---
 .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  1 +
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  |  2 +-
 .../display/amdgpu_dm/amdgpu_dm_services.c|  2 +-
 drivers/gpu/drm/arc/arcpgu_crtc.c |  2 +-
 drivers/gpu/drm/arc/arcpgu_drv.c  |  2 +-
 drivers/gpu/drm/arc/arcpgu_sim.c  |  2 +-
 drivers/gpu/drm/arm/hdlcd_crtc.c  |  2 +-
 drivers/gpu/drm/arm/hdlcd_drv.c   |  2 +-
 drivers/gpu/drm/arm/malidp_crtc.c |  2 +-
 drivers/gpu/drm/arm/malidp_drv.c  |  2 +-
 drivers/gpu/drm/arm/malidp_mw.c   |  2 +-
 drivers/gpu/drm/armada/armada_510.c   |  2 +-
 drivers/gpu/drm/armada/armada_crtc.c  |  2 +-
 drivers/gpu/drm/armada/armada_crtc.h  |  2 ++
 drivers/gpu/drm/armada/armada_drv.c   |  2 +-
 drivers/gpu/drm/armada/armada_fb.c|  2 +-
 drivers/gpu/drm/ast/ast_drv.c |  1 +
 drivers/gpu/drm/ast/ast_mode.c|  1 +
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  2 +-
 drivers/gpu/drm/bochs/bochs_drv.c |  1 +
 drivers/gpu/drm/bochs/bochs_kms.c |  1 +
 drivers/gpu/drm/bridge/adv7511/adv7511.h  |  4 ++-
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c  |  1 +
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  2 +-
 .../drm/bridge/analogix/analogix_dp_core.c|  2 +-
 drivers/gpu/drm/bridge/cdns-dsi.c |  2 +-
 drivers/gpu/drm/bridge/dumb-vga-dac.c |  2 +-
 .../bridge/megachips-stdp-ge-b850v3-fw.c  |  2 +-
 drivers/gpu/drm/bridge/nxp-ptn3460.c  |  2 +-
 drivers/gpu/drm/bridge/panel.c|  2 +-
 drivers/gpu/drm/bridge/parade-ps8622.c|  2 +-
 drivers/gpu/drm/bridge/sii902x.c  |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  2 +-
 drivers/gpu/drm/bridge/tc358764.c |  2 +-
 drivers/gpu/drm/bridge/tc358767.c |  2 +-
 drivers/gpu/drm/bridge/ti-sn65dsi86.c |  2 +-
 drivers/gpu/drm/bridge/ti-tfp410.c|  2 +-
 drivers/gpu/drm/cirrus/cirrus_drv.c   |  1 +
 drivers/gpu/drm/cirrus/cirrus_mode.c  |  1 +
 drivers/gpu/drm/drm_atomic_helper.c   |  1 -
 drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
 drivers/gpu/drm/drm_modeset_helper.c  |  2 +-
 drivers/gpu/drm/drm_probe_helper.c|  2 +-
 drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.h |  1 -
 drivers/gpu/drm/exynos/exynos_dp.c|  3 ++-
 drivers/gpu/drm/exynos/exynos_drm_crtc.c  |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_dpi.c   |  2 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c   |  2 +-
 

Re: [PATCH 3/3] virtio-blk: Consider dma_max_mapping_size() for maximum segment size

2019-01-16 Thread Michael S. Tsirkin
On Tue, Jan 15, 2019 at 02:22:57PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> Segments can't be larger than the maximum DMA mapping size
> supported on the platform. Take that into account when
> setting the maximum segment size for a block device.
> 
> Signed-off-by: Joerg Roedel 
> ---
>  drivers/block/virtio_blk.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index b16a887bbd02..6193962a7fec 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -723,7 +723,7 @@ static int virtblk_probe(struct virtio_device *vdev)
>   struct request_queue *q;
>   int err, index;
>  
> - u32 v, blk_size, sg_elems, opt_io_size;
> + u32 v, blk_size, max_size, sg_elems, opt_io_size;
>   u16 min_io_size;
>   u8 physical_block_exp, alignment_offset;
>  
> @@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev)
>   /* No real sector limit. */
>   blk_queue_max_hw_sectors(q, -1U);
>  
> + max_size = dma_max_mapping_size(>dev);
> +


Should this be limited to ACCESS_PLATFORM?

I see no reason to limit this without as guest can
access any memory.


I'd like a bit of time to consider this point.

>   /* Host can optionally specify maximum segment size and number of
>* segments. */
>   err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
>  struct virtio_blk_config, size_max, );
>   if (!err)
> - blk_queue_max_segment_size(q, v);
> - else
> - blk_queue_max_segment_size(q, -1U);
> + max_size = min(max_size, v);
> +
> + blk_queue_max_segment_size(q, max_size);
>  
>   /* Host can optionally specify the block size of the device */
>   err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
> -- 
> 2.17.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions

2019-01-16 Thread Daniel Vetter
On Wed, Jan 16, 2019 at 12:28:00PM +0100, Gerd Hoffmann wrote:
> > > +static int qxl_check_mode(struct qxl_device *qdev,
> > > +   unsigned int width,
> > > +   unsigned int height)
> > > +{
> > > + if (width * height * 4 > qdev->vram_size)
> > 
> > Is someone checking for integer overflows already?
> 
> Need to have a look.  This is just ...

The addfb ioctl checks for integer overflows for you.
-Daniel

> 
> > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> > > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> > > - return -EINVAL;
> > > - }
> 
> ... that check moved into the new function.
> 
> cheers,
>   Gerd
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions

2019-01-16 Thread Gerd Hoffmann
> > +static int qxl_check_mode(struct qxl_device *qdev,
> > + unsigned int width,
> > + unsigned int height)
> > +{
> > +   if (width * height * 4 > qdev->vram_size)
> 
> Is someone checking for integer overflows already?

Need to have a look.  This is just ...

> > -   if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> > -   DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> > -   return -EINVAL;
> > -   }

... that check moved into the new function.

cheers,
  Gerd

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


[PATCH 3/3] drm/qxl: use kernel mode db

2019-01-16 Thread Gerd Hoffmann
Add all standard modes from the kernel's video mode data base.
Keep a few non-standard modes in the qxl mode list.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_display.c | 27 +++
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index b87f72f59e..9e49472872 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -256,34 +256,20 @@ static int qxl_add_monitors_config_modes(struct 
drm_connector *connector)
 static struct mode_size {
int w;
int h;
-} common_modes[] = {
-   { 640,  480},
+} extra_modes[] = {
{ 720,  480},
-   { 800,  600},
-   { 848,  480},
-   {1024,  768},
{1152,  768},
-   {1280,  720},
-   {1280,  800},
{1280,  854},
-   {1280,  960},
-   {1280, 1024},
-   {1440,  900},
-   {1400, 1050},
-   {1680, 1050},
-   {1600, 1200},
-   {1920, 1080},
-   {1920, 1200}
 };
 
-static int qxl_add_common_modes(struct drm_connector *connector)
+static int qxl_add_extra_modes(struct drm_connector *connector)
 {
int i, ret = 0;
 
-   for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+   for (i = 0; i < ARRAY_SIZE(extra_modes); i++)
ret += qxl_add_mode(connector,
-   common_modes[i].w,
-   common_modes[i].h,
+   extra_modes[i].w,
+   extra_modes[i].h,
false);
return ret;
 }
@@ -954,7 +940,8 @@ static int qxl_conn_get_modes(struct drm_connector 
*connector)
pheight = head->height;
}
 
-   ret += qxl_add_common_modes(connector);
+   ret += drm_add_modes_noedid(connector, 8192, 8192);
+   ret += qxl_add_extra_modes(connector);
ret += qxl_add_monitors_config_modes(connector);
drm_set_preferred_mode(connector, pwidth, pheight);
return ret;
-- 
2.9.3

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


[PATCH 2/3] drm/qxl: add qxl_add_mode helper function

2019-01-16 Thread Gerd Hoffmann
Add a helper function to add custom video modes to a connector.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_display.c | 84 +++
 1 file changed, 49 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c 
b/drivers/gpu/drm/qxl/qxl_display.c
index 07a37d52c4..b87f72f59e 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -206,15 +206,36 @@ static int qxl_check_framebuffer(struct qxl_device *qdev,
return qxl_check_mode(qdev, bo->surf.width, bo->surf.height);
 }
 
-static int qxl_add_monitors_config_modes(struct drm_connector *connector,
- unsigned *pwidth,
- unsigned *pheight)
+static int qxl_add_mode(struct drm_connector *connector,
+   unsigned int width,
+   unsigned int height,
+   bool preferred)
+{
+   struct drm_device *dev = connector->dev;
+   struct qxl_device *qdev = dev->dev_private;
+   struct drm_display_mode *mode = NULL;
+   int rc;
+
+   rc = qxl_check_mode(qdev, width, height);
+   if (rc != 0)
+   return 0;
+
+   mode = drm_cvt_mode(dev, width, height, 60, false, false, false);
+   if (preferred)
+   mode->type |= DRM_MODE_TYPE_PREFERRED;
+   mode->hdisplay = width;
+   mode->vdisplay = height;
+   drm_mode_set_name(mode);
+   drm_mode_probed_add(connector, mode);
+   return 1;
+}
+
+static int qxl_add_monitors_config_modes(struct drm_connector *connector)
 {
struct drm_device *dev = connector->dev;
struct qxl_device *qdev = dev->dev_private;
struct qxl_output *output = drm_connector_to_qxl_output(connector);
int h = output->index;
-   struct drm_display_mode *mode = NULL;
struct qxl_head *head;
 
if (!qdev->monitors_config)
@@ -229,19 +250,7 @@ static int qxl_add_monitors_config_modes(struct 
drm_connector *connector,
head = >client_monitors_config->heads[h];
DRM_DEBUG_KMS("head %d is %dx%d\n", h, head->width, head->height);
 
-   mode = drm_cvt_mode(dev, head->width, head->height, 60, false, false,
-   false);
-   mode->type |= DRM_MODE_TYPE_PREFERRED;
-   mode->hdisplay = head->width;
-   mode->vdisplay = head->height;
-   drm_mode_set_name(mode);
-   *pwidth = head->width;
-   *pheight = head->height;
-   drm_mode_probed_add(connector, mode);
-   /* remember the last custom size for mode validation */
-   qdev->monitors_config_width = mode->hdisplay;
-   qdev->monitors_config_height = mode->vdisplay;
-   return 1;
+   return qxl_add_mode(connector, head->width, head->height, true);
 }
 
 static struct mode_size {
@@ -267,22 +276,16 @@ static struct mode_size {
{1920, 1200}
 };
 
-static int qxl_add_common_modes(struct drm_connector *connector,
-unsigned int pwidth,
-unsigned int pheight)
+static int qxl_add_common_modes(struct drm_connector *connector)
 {
-   struct drm_device *dev = connector->dev;
-   struct drm_display_mode *mode = NULL;
-   int i;
+   int i, ret = 0;
 
-   for (i = 0; i < ARRAY_SIZE(common_modes); i++) {
-   mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
-   60, false, false, false);
-   if (common_modes[i].w == pwidth && common_modes[i].h == pheight)
-   mode->type |= DRM_MODE_TYPE_PREFERRED;
-   drm_mode_probed_add(connector, mode);
-   }
-   return i - 1;
+   for (i = 0; i < ARRAY_SIZE(common_modes); i++)
+   ret += qxl_add_mode(connector,
+   common_modes[i].w,
+   common_modes[i].h,
+   false);
+   return ret;
 }
 
 static void qxl_send_monitors_config(struct qxl_device *qdev)
@@ -935,14 +938,25 @@ static int qdev_crtc_init(struct drm_device *dev, int 
crtc_id)
 
 static int qxl_conn_get_modes(struct drm_connector *connector)
 {
+   struct drm_device *dev = connector->dev;
+   struct qxl_device *qdev = dev->dev_private;
+   struct qxl_output *output = drm_connector_to_qxl_output(connector);
unsigned int pwidth = 1024;
unsigned int pheight = 768;
int ret = 0;
 
-   ret = qxl_add_monitors_config_modes(connector, , );
-   if (ret < 0)
-   return ret;
-   ret += qxl_add_common_modes(connector, pwidth, pheight);
+   if (qdev->client_monitors_config) {
+   struct qxl_head *head;
+   head = >client_monitors_config->heads[output->index];
+   if (head->width)
+   pwidth = head->width;
+   if (head->height)
+   pheight = head->height;

Re: [PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Alex Deucher
On Tue, Jan 15, 2019 at 5:41 AM Daniel Vetter  wrote:
>
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
>
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.
>
> v2: Make it compile. There was so much compile fail on arm drivers
> that I figured I'll better not include any of the acks on v1.
>
> v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
> not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
> there was still one, which this patch largely removes. Which means
> rolling out lots more includes all over.
>
> This will also conflict with ongoing drmP.h cleanup by others I
> expect.
>
> v3: Rebase on top of atomic bochs.
>
> Cc: Sam Ravnborg 
> Cc: Jani Nikula 
> Cc: Laurent Pinchart 
> Acked-by: Rodrigo Vivi  (v2)
> Acked-by: Benjamin Gaignard  (v2)
> Signed-off-by: Daniel Vetter 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: etna...@lists.freedesktop.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: spice-de...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> Cc: linux-te...@vger.kernel.org
> Cc: xen-de...@lists.xen.org
> ---
> Merging this is going to be a bit a mess due to all the ongoing drmP.h
> cleanups. I think the following should work:
> - Apply Sam's prep patches for removing drmP.h from
>   drm_modeset_helper.h
> - Get the i915 drmP.h cleanup backmerged into drm-misc-next
> - Apply this patch.
> - Apply Sam's patch to remove drmP.h from drm_modeset_helper.h
> - All through drm-misc-next, which has some potential for trivial
>   conflicts around #includes with other drivers unfortunately.
>
> I hope there's no other driver who'll blow up accidentally because
> someone else is doing a drmP.h cleanup. Laurent maybe?
>
> Jani, ack on this?
> -Daniel

amdgpu and radeon:
Acked-by: Alex Deucher 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 2/3] dma: Introduce dma_max_mapping_size()

2019-01-16 Thread Christoph Hellwig
On Tue, Jan 15, 2019 at 05:23:22PM +0100, Joerg Roedel wrote:
> Right, I thought about that too, but didn't find a generic way to check
> for all the cases. There are various checks that could be done:
> 
>   1) Check if SWIOTLB is initialized at all, if not, return
>  SIZE_MAX as the limit. This can't be checked from dma-direct
>  code right now, but could be easily implemented.

Yes, this is the low hanging fruit.

>   2) Check for swiotlb=force needs to be done.
> 
>   3) Check whether the device can access all of available RAM. I
>  have no idea how to check that in an architecture independent
>  way. It also has to take memory hotplug into account as well
>  as the DMA mask of the device.
> 
>  An easy approximation could be to omit the limit if the
>  dma-mask covers all of the physical address bits available
>  on the platform. It would require to pass the dma-mask as an
>  additional parameter like it is done in dma_supported().
> 
> Any better ideas for how to implement 3)?

And yeah, this is hard.  So I'd just go for the low hanging fruit
for now and only implement 1) with a comment mentioning that
we are a little pessimistic.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Dave Chinner
On Tue, Jan 15, 2019 at 12:35:06AM -0500, Pankaj Gupta wrote:
> 
> > > > On Mon, Jan 14, 2019 at 02:15:40AM -0500, Pankaj Gupta wrote:
> > > > >
> > > > > > > Until you have images (and hence host page cache) shared between
> > > > > > > multiple guests. People will want to do this, because it means 
> > > > > > > they
> > > > > > > only need a single set of pages in host memory for executable
> > > > > > > binaries rather than a set of pages per guest. Then you have
> > > > > > > multiple guests being able to detect residency of the same set of
> > > > > > > pages. If the guests can then, in any way, control eviction of the
> > > > > > > pages from the host cache, then we have a guest-to-guest
> > > > > > > information
> > > > > > > leak channel.
> > > > > >
> > > > > > I don't think we should ever be considering something that would
> > > > > > allow a
> > > > > > guest to evict page's from the host's pagecache [1].  The guest
> > > > > > should
> > > > > > be able to kick its own references to the host's pagecache out of 
> > > > > > its
> > > > > > own pagecache, but not be able to influence whether the host or
> > > > > > another
> > > > > > guest has a read-only mapping cached.
> > > > > >
> > > > > > [1] Unless the guest is allowed to modify the host's file; obviously
> > > > > > truncation, holepunching, etc are going to evict pages from the
> > > > > > host's
> > > > > > page cache.
> > > > >
> > > > > This is so correct. Guest does not not evict host page cache pages
> > > > > directly.
> > > >
> > > > They don't right now.
> > > >
> > > > But someone is going to end up asking for discard to work so that
> > > > the guest can free unused space in the underlying spares image (i.e.
> > > > make use of fstrim or mount -o discard) because they have workloads
> > > > that have bursts of space usage and they need to trim the image
> > > > files afterwards to keep their overall space usage under control.
> > > >
> > > > And then
> > > 
> > > ...we reject / push back on that patch citing the above concern.
> > 
> > So at what point do we draw the line?
> > 
> > We're allowing writable DAX mappings, but as I've pointed out that
> > means we are going to be allowing  a potential information leak via
> > files with shared extents to be directly mapped and written to.
> > 
> > But we won't allow useful admin operations that allow better
> > management of host side storage space similar to how normal image
> > files are used by guests because it's an information leak vector?
> 
> First of all Thank you for all the useful discussions. 
> I am summarizing here:
> 
> - We have to live with the limitation to not support fstrim and 
>   mount -o discard options with virtio-pmem as they will evict 
>   host page cache pages. We cannot allow this for virtio-pmem
>   for security reasons. These filesystem commands will just zero out 
>   unused pages currently.

Not sure I follow you here - what pages are going to be zeroed and
when will they be zeroed? If discard is not allowed, filesystems
just don't issue such commands and the underlying device will never
seen them.

> - If alot of space is unused and not freed guest can request host 
>   Administrator for truncating the host backing image. 

You can't use truncate to free space in a disk image file. The only
way to do it safely in a generic, filesystem agnositic way is to
mount the disk image (e.g. on loopback) and run fstrim on it. The
loopback device will punches holes in the file where all the free
space is reported by the filesystem via discard requests.

Which is kinda my point - this could only be done if the guest is
shut down, which makes it very difficult for admins to manage. 

>   We are also planning to support qcow2 sparse image format at 
>   host side with virtio-pmem.

So you're going to be remapping a huge number of disjoint regions
into a linear pmem mapping? ISTR discussions about similar things
for virtio+fuse+dax that came up against "large numbers of mapped
regions don't scale" and so it wasn't a practical solution compared
to a just using raw sparse files

> - There is no existing solution for Qemu persistent memory 
>   emulation with write support currently. This solution provides 
>   us the paravartualized way of emulating persistent memory.

Sure, but the question is why do you need to create an emulation
that doesn't actually perform like pmem? The whole point of pmem is
performance, and emulating pmem by mmap() of a file on spinning
disks is going to be horrible for performance. Even on SSDs it's
going to be orders of magnitudes slower than real pmem.

So exactly what problem are you trying to solve with this driver?

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 2/3] dma: Introduce dma_max_mapping_size()

2019-01-16 Thread Joerg Roedel
From: Joerg Roedel 

The function returns the maximum size that can be mapped
using DMA-API functions. The patch also adds the
implementation for direct DMA and a new dma_map_ops pointer
so that other implementations can expose their limit.

Signed-off-by: Joerg Roedel 
---
 include/linux/dma-mapping.h | 16 
 kernel/dma/direct.c | 10 ++
 2 files changed, 26 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f6ded992c183..a3ca8a71a704 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -130,6 +130,7 @@ struct dma_map_ops {
enum dma_data_direction direction);
int (*dma_supported)(struct device *dev, u64 mask);
u64 (*get_required_mask)(struct device *dev);
+   size_t (*max_mapping_size)(struct device *dev);
 };
 
 #define DMA_MAPPING_ERROR  (~(dma_addr_t)0)
@@ -257,6 +258,8 @@ static inline void dma_direct_sync_sg_for_cpu(struct device 
*dev,
 }
 #endif
 
+size_t dma_direct_max_mapping_size(struct device *dev);
+
 #ifdef CONFIG_HAS_DMA
 #include 
 
@@ -440,6 +443,19 @@ static inline int dma_mapping_error(struct device *dev, 
dma_addr_t dma_addr)
return 0;
 }
 
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+   const struct dma_map_ops *ops = get_dma_ops(dev);
+   size_t size = SIZE_MAX;
+
+   if (dma_is_direct(ops))
+   size = dma_direct_max_mapping_size(dev);
+   else if (ops && ops->max_mapping_size)
+   size = ops->max_mapping_size(dev);
+
+   return size;
+}
+
 void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t flag, unsigned long attrs);
 void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..84917e1003c4 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -380,3 +380,13 @@ int dma_direct_supported(struct device *dev, u64 mask)
 */
return mask >= __phys_to_dma(dev, min_mask);
 }
+
+size_t dma_direct_max_mapping_size(struct device *dev)
+{
+   /*
+* Return the minimum of the direct DMA limit and the SWIOTLB limit.
+* Since direct DMA has no limit, it is fine to just return the SWIOTLB
+* limit.
+*/
+   return swiotlb_max_mapping_size(dev);
+}
-- 
2.17.1

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


Re: [PATCH 2/3] dma: Introduce dma_max_mapping_size()

2019-01-16 Thread Joerg Roedel
On Tue, Jan 15, 2019 at 02:37:54PM +0100, Christoph Hellwig wrote:
> > +size_t dma_direct_max_mapping_size(struct device *dev)
> > +{
> > +   /*
> > +* Return the minimum of the direct DMA limit and the SWIOTLB limit.
> > +* Since direct DMA has no limit, it is fine to just return the SWIOTLB
> > +* limit.
> > +*/
> > +   return swiotlb_max_mapping_size(dev);
> 
> Well, if we don't actually use the swiotlb buffers despite it being
> compiled in or even allocated we don't need the limit.

Right, I thought about that too, but didn't find a generic way to check
for all the cases. There are various checks that could be done:

1) Check if SWIOTLB is initialized at all, if not, return
   SIZE_MAX as the limit. This can't be checked from dma-direct
   code right now, but could be easily implemented.

2) Check for swiotlb=force needs to be done.

3) Check whether the device can access all of available RAM. I
   have no idea how to check that in an architecture independent
   way. It also has to take memory hotplug into account as well
   as the DMA mask of the device.

   An easy approximation could be to omit the limit if the
   dma-mask covers all of the physical address bits available
   on the platform. It would require to pass the dma-mask as an
   additional parameter like it is done in dma_supported().

Any better ideas for how to implement 3)?

Regards,

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


Re: [PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Laurent Pinchart
Hi Daniel,

Thank you for the patch.

On Tuesday, 15 January 2019 12:41:37 EET Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
> 
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.
> 
> v2: Make it compile. There was so much compile fail on arm drivers
> that I figured I'll better not include any of the acks on v1.
> 
> v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
> not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
> there was still one, which this patch largely removes. Which means
> rolling out lots more includes all over.
> 
> This will also conflict with ongoing drmP.h cleanup by others I
> expect.
> 
> v3: Rebase on top of atomic bochs.
> 
> Cc: Sam Ravnborg 
> Cc: Jani Nikula 
> Cc: Laurent Pinchart 
> Acked-by: Rodrigo Vivi  (v2)
> Acked-by: Benjamin Gaignard  (v2)
> Signed-off-by: Daniel Vetter 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: etna...@lists.freedesktop.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: spice-de...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> Cc: linux-te...@vger.kernel.org
> Cc: xen-de...@lists.xen.org
> ---
> Merging this is going to be a bit a mess due to all the ongoing drmP.h
> cleanups. I think the following should work:
> - Apply Sam's prep patches for removing drmP.h from
>   drm_modeset_helper.h
> - Get the i915 drmP.h cleanup backmerged into drm-misc-next
> - Apply this patch.
> - Apply Sam's patch to remove drmP.h from drm_modeset_helper.h
> - All through drm-misc-next, which has some potential for trivial
>   conflicts around #includes with other drivers unfortunately.
> 
> I hope there's no other driver who'll blow up accidentally because
> someone else is doing a drmP.h cleanup. Laurent maybe?

There's a drmP.h cleanup in the R-Car DU driver, but it doesn't conflict with 
this patch, the combination of both compiles fine.

> Jani, ack on this?
> -Daniel
> ---

[snip]

>  drivers/gpu/drm/bridge/adv7511/adv7511.h  |  5 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  2 +-
>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
>  drivers/gpu/drm/bridge/ti-tfp410.c|  2 +-
>  drivers/gpu/drm/drm_atomic_helper.c   |  1 -
>  drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
>  drivers/gpu/drm/drm_modeset_helper.c  |  2 +-
>  drivers/gpu/drm/drm_probe_helper.c|  2 +-
>  drivers/gpu/drm/drm_simple_kms_helper.c   |  2 +-
>  drivers/gpu/drm/omapdrm/omap_connector.c  |  2 +-
>  drivers/gpu/drm/omapdrm/omap_crtc.c   |  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c|  2 +-
>  drivers/gpu/drm/omapdrm/omap_drv.h|  2 +-
>  drivers/gpu/drm/omapdrm/omap_encoder.c|  2 +-
>  drivers/gpu/drm/omapdrm/omap_fb.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c|  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c   |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c |  2 +-
>  drivers/gpu/drm/rcar-du/rcar_lvds.c   |  2 +-
>  drivers/gpu/drm/shmobile/shmob_drm_crtc.c |  1 +
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c  |  1 +
>  drivers/gpu/drm/shmobile/shmob_drm_kms.c  |  1 +
>  include/drm/drm_crtc_helper.h | 16 --
>  include/drm/drm_probe_helper.h| 50 +++

For the above files, with the comments below addressed,

Reviewed-by: Laurent Pinchart 

>  227 files changed, 289 insertions(+), 200 deletions(-)
>  create mode 100644 include/drm/drm_probe_helper.h

[snip]

> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 73d8ccb97742..d52ffab41eb4
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -14,8 +14,11 @@
>  #include 
>  #include 
> 
> -#include 
> +#include 

The probe helpers are needed in adv7511_drv.c only, I would move the include 
there.

>  #include 
> +#include 
> +#include 
> +#include 

Please keep the headers alphabetically sorted, here and in all other drivers.

>  #define ADV7511_REG_CHIP_REVISION0x00
>  #define 

Re: [PATCH 2/3] dma: Introduce dma_max_mapping_size()

2019-01-16 Thread Christoph Hellwig
> +size_t dma_direct_max_mapping_size(struct device *dev)
> +{
> + /*
> +  * Return the minimum of the direct DMA limit and the SWIOTLB limit.
> +  * Since direct DMA has no limit, it is fine to just return the SWIOTLB
> +  * limit.
> +  */
> + return swiotlb_max_mapping_size(dev);

Well, if we don't actually use the swiotlb buffers despite it being
compiled in or even allocated we don't need the limit.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 0/3 v2] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Joerg Roedel
Hi,

here is the second version of my patch-set to fix a DMA
mapping size issue triggered by the virtio-blk driver.

The problem is a limitation of the SWIOTLB implementation,
which does not support allocations larger than 256kb.  When
the virtio-blk driver tries to read/write a block larger
than that, the allocation of the dma-handle fails and an IO
error is reported.

v1 of the patch-set can be found here:

https://lore.kernel.org/lkml/20190110134433.15672-1-j...@8bytes.org/

The change to v1 is that the maximum mapping size is now
officially propagated through the DMA-API, as suggested by
Christoph Hellwig.

Please review.

Thanks,

Joerg

Joerg Roedel (3):
  swiotlb: Introduce swiotlb_max_mapping_size()
  dma: Introduce dma_max_mapping_size()
  virtio-blk: Consider dma_max_mapping_size() for maximum segment size

 drivers/block/virtio_blk.c  | 10 ++
 include/linux/dma-mapping.h | 16 
 include/linux/swiotlb.h |  5 +
 kernel/dma/direct.c | 10 ++
 kernel/dma/swiotlb.c|  5 +
 5 files changed, 42 insertions(+), 4 deletions(-)

-- 
2.17.1

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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Christoph Hellwig
On Tue, Jan 15, 2019 at 09:37:42AM +0100, Joerg Roedel wrote:
> On Mon, Jan 14, 2019 at 01:20:45PM -0500, Michael S. Tsirkin wrote:
> > Which would be fine especially if we can manage not to introduce a bunch
> > of indirect calls all over the place and hurt performance.
> 
> Which indirect calls? In case of unset dma_ops the DMA-API functions
> call directly into the dma-direct implementation, no indirect calls at
> all.

True.  But the NULL-ops dma direct case still has two issues that might
not work for virtio:

 (a) if the architecture supports devices that are not DMA coherent it
 will dip into a special allocator and do cache maintainance for
 streaming mappings.  Although it would be a bit of a hack we could
 work around this in virtio doings something like:

#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
dev->dma_coherent = true;
#endif

except that won't work for mips, which has a mode where it does
a system instead of device level coherency flag and thus doesn't
look at this struct device field

 (b) we can still mangle the DMA address, either using the
 dma_pfn_offset field in struct device, or by a full override
 of __phys_to_dma / __dma_to_phys by the architecture code.
 The first could be solved using a hack like the one above,
 but the latter would be a little harder.  In the long run
 I'd love to get rid of that hook and have everyone use the
 generic offset code, but for that we first need to support
 multiple ranges with different offset and do quite some
 nasty arch code surgery.
 
So for the legacy virtio case I fear we need to keep local dma mapping
implementation for now.  I just wish now recent hypervisor would ever
offer devices in this broken legacy mode..

> 
> Regards,
> 
>   Joerg
---end quoted text---
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 3/3] virtio-blk: Consider dma_max_mapping_size() for maximum segment size

2019-01-16 Thread Joerg Roedel
From: Joerg Roedel 

Segments can't be larger than the maximum DMA mapping size
supported on the platform. Take that into account when
setting the maximum segment size for a block device.

Signed-off-by: Joerg Roedel 
---
 drivers/block/virtio_blk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b16a887bbd02..6193962a7fec 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -723,7 +723,7 @@ static int virtblk_probe(struct virtio_device *vdev)
struct request_queue *q;
int err, index;
 
-   u32 v, blk_size, sg_elems, opt_io_size;
+   u32 v, blk_size, max_size, sg_elems, opt_io_size;
u16 min_io_size;
u8 physical_block_exp, alignment_offset;
 
@@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev)
/* No real sector limit. */
blk_queue_max_hw_sectors(q, -1U);
 
+   max_size = dma_max_mapping_size(>dev);
+
/* Host can optionally specify maximum segment size and number of
 * segments. */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
   struct virtio_blk_config, size_max, );
if (!err)
-   blk_queue_max_segment_size(q, v);
-   else
-   blk_queue_max_segment_size(q, -1U);
+   max_size = min(max_size, v);
+
+   blk_queue_max_segment_size(q, max_size);
 
/* Host can optionally specify the block size of the device */
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE,
-- 
2.17.1

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


[PATCH 1/3] swiotlb: Introduce swiotlb_max_mapping_size()

2019-01-16 Thread Joerg Roedel
From: Joerg Roedel 

The function returns the maximum size that can be remapped
by the SWIOTLB implementation. This function will be later
exposed to users through the DMA-API.

Signed-off-by: Joerg Roedel 
---
 include/linux/swiotlb.h | 5 +
 kernel/dma/swiotlb.c| 5 +
 2 files changed, 10 insertions(+)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 7c007ed7505f..ceb623321f38 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -62,6 +62,7 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
 
 extern int
 swiotlb_dma_supported(struct device *hwdev, u64 mask);
+extern size_t swiotlb_max_mapping_size(struct device *dev);
 
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
@@ -95,6 +96,10 @@ static inline unsigned int swiotlb_max_segment(void)
 {
return 0;
 }
+static inline size_t swiotlb_max_mapping_size(struct device *dev)
+{
+   return SIZE_MAX;
+}
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d6361776dc5c..c950b3e9f683 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -660,3 +660,8 @@ swiotlb_dma_supported(struct device *hwdev, u64 mask)
 {
return __phys_to_dma(hwdev, io_tlb_end - 1) <= mask;
 }
+
+size_t swiotlb_max_mapping_size(struct device *dev)
+{
+   return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
+}
-- 
2.17.1

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


[PATCH v7 7/7] iommu/virtio: Add event queue

2019-01-16 Thread Jean-Philippe Brucker
The event queue offers a way for the device to report access faults from
endpoints. It is implemented on virtqueue #1. Whenever the host needs to
signal a fault, it fills one of the buffers offered by the guest and
interrupts it.

Tested-by: Bharat Bhushan 
Tested-by: Eric Auger 
Reviewed-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/virtio-iommu.c  | 115 +++---
 include/uapi/linux/virtio_iommu.h |  19 +
 2 files changed, 125 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 5e194493a531..4620dd221ffd 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -29,7 +29,8 @@
 #define MSI_IOVA_LENGTH0x10
 
 #define VIOMMU_REQUEST_VQ  0
-#define VIOMMU_NR_VQS  1
+#define VIOMMU_EVENT_VQ1
+#define VIOMMU_NR_VQS  2
 
 struct viommu_dev {
struct iommu_device iommu;
@@ -41,6 +42,7 @@ struct viommu_dev {
struct virtqueue*vqs[VIOMMU_NR_VQS];
spinlock_t  request_lock;
struct list_headrequests;
+   void*evts;
 
/* Device configuration */
struct iommu_domain_geometrygeometry;
@@ -82,6 +84,15 @@ struct viommu_request {
charbuf[];
 };
 
+#define VIOMMU_FAULT_RESV_MASK 0xff00
+
+struct viommu_event {
+   union {
+   u32 head;
+   struct virtio_iommu_fault fault;
+   };
+};
+
 #define to_viommu_domain(domain)   \
container_of(domain, struct viommu_domain, domain)
 
@@ -503,6 +514,68 @@ static int viommu_probe_endpoint(struct viommu_dev 
*viommu, struct device *dev)
return ret;
 }
 
+static int viommu_fault_handler(struct viommu_dev *viommu,
+   struct virtio_iommu_fault *fault)
+{
+   char *reason_str;
+
+   u8 reason   = fault->reason;
+   u32 flags   = le32_to_cpu(fault->flags);
+   u32 endpoint= le32_to_cpu(fault->endpoint);
+   u64 address = le64_to_cpu(fault->address);
+
+   switch (reason) {
+   case VIRTIO_IOMMU_FAULT_R_DOMAIN:
+   reason_str = "domain";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_MAPPING:
+   reason_str = "page";
+   break;
+   case VIRTIO_IOMMU_FAULT_R_UNKNOWN:
+   default:
+   reason_str = "unknown";
+   break;
+   }
+
+   /* TODO: find EP by ID and report_iommu_fault */
+   if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
+   dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx 
[%s%s%s]\n",
+   reason_str, endpoint, address,
+   flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" : 
"",
+   flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" : 
"",
+   flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" : 
"");
+   else
+   dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
+   reason_str, endpoint);
+   return 0;
+}
+
+static void viommu_event_handler(struct virtqueue *vq)
+{
+   int ret;
+   unsigned int len;
+   struct scatterlist sg[1];
+   struct viommu_event *evt;
+   struct viommu_dev *viommu = vq->vdev->priv;
+
+   while ((evt = virtqueue_get_buf(vq, )) != NULL) {
+   if (len > sizeof(*evt)) {
+   dev_err(viommu->dev,
+   "invalid event buffer (len %u != %zu)\n",
+   len, sizeof(*evt));
+   } else if (!(evt->head & VIOMMU_FAULT_RESV_MASK)) {
+   viommu_fault_handler(viommu, >fault);
+   }
+
+   sg_init_one(sg, evt, sizeof(*evt));
+   ret = virtqueue_add_inbuf(vq, sg, 1, evt, GFP_ATOMIC);
+   if (ret)
+   dev_err(viommu->dev, "could not add event buffer\n");
+   }
+
+   virtqueue_kick(vq);
+}
+
 /* IOMMU API */
 
 static struct iommu_domain *viommu_domain_alloc(unsigned type)
@@ -886,16 +959,35 @@ static struct iommu_ops viommu_ops = {
 static int viommu_init_vqs(struct viommu_dev *viommu)
 {
struct virtio_device *vdev = dev_to_virtio(viommu->dev);
-   const char *name = "request";
-   void *ret;
+   const char *names[] = { "request", "event" };
+   vq_callback_t *callbacks[] = {
+   NULL, /* No async requests */
+   viommu_event_handler,
+   };
 
-   ret = virtio_find_single_vq(vdev, NULL, name);
-   if (IS_ERR(ret)) {
-   dev_err(viommu->dev, "cannot find VQ\n");
-   return PTR_ERR(ret);
-   }
+   return virtio_find_vqs(vdev, VIOMMU_NR_VQS, viommu->vqs, 

Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Christoph Hellwig
On Mon, Jan 14, 2019 at 03:48:47PM -0500, Michael S. Tsirkin wrote:
> I think you are exaggerating a little here.  Limited access with e.g.
> need to flush caches is common on lower end but less common on higher
> end systems used for virtualization.  As you point out that changes with
> e.g. SEV but then SEV is a pretty niche thing so far.
> 
> So I would make that a SHOULD probably.

The problem is that without using DMA ops you can't just plug the device
into a random system, which is a total no-go for periphals.

And cache flushing is not that uncommmon, e.g. every sparc systems
needs it, many arm/arm64 ones, some power ones, lots of mips ones, etc.

But cache flushing is just one side of it - lots of systems have either
mandatory or option IOMMUs, and without the platform access flag that
doesn't work.  As does that case where you have a DMA offset, which
is extremly common on arm and power systems.  So you might find a few
non-x86 systems you can plug it in and it'll just work, but it won't
be all that many.  And even on x86 your device will be completely broken
if the users dares to use an IOMMU.

> > flags really needs some major updates in terms of DMA access.
> 
> Thanks for the reminder. Yes generally spec tends to still say e.g.
> "physical address" where it really means a "DMA address" in Linux terms.
> Whether changing that will make things much clearer I'm not sure. E.g.
> hardware designers don't much care I guess.

At least say bus address - as said above the CPU and bus concepts of
physicall addresses are different in many systems.  Including x86
when you use IOMMUs.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 1/3] swiotlb: Export maximum allocation size

2019-01-16 Thread Christoph Hellwig
On Mon, Jan 14, 2019 at 04:59:27PM -0500, Michael S. Tsirkin wrote:
> On Mon, Jan 14, 2019 at 03:49:07PM -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jan 11, 2019 at 10:12:31AM +0100, Joerg Roedel wrote:
> > > On Thu, Jan 10, 2019 at 12:02:05PM -0500, Konrad Rzeszutek Wilk wrote:
> > > > Why not use swiotlb_nr_tbl ? That is how drivers/gpu/drm use to figure 
> > > > if they
> > > > need to limit the size of pages.
> > > 
> > > That function just exports the overall size of the swiotlb aperture, no?
> > > What I need here is the maximum size for a single mapping.
> > 
> > Yes. The other drivers just assumed that if there is SWIOTLB they would use
> > the smaller size by default (as in they knew the limitation).
> > 
> > But I agree it would be better to have something smarter - and also convert 
> > the
> > DRM drivers to piggy back on this.
> > 
> > Or alternatively we could make SWIOTLB handle bigger sizes..
> 
> 
> Just a thought: is it a good idea to teach blk_queue_max_segment_size
> to get the dma size? This will help us find other devices
> possibly missing this check.

Yes, we should.  Both the existing DMA size communicated through dma_params
which is set by the driver, and this new DMA-ops exposed one which needs
to be added.  I'm working on some preliminary patches for the first part,
as I think I introduced a bug related to that in the SCSI layer in 5.0..
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Neil Armstrong
On 15/01/2019 11:41, Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
> 
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.
> 
> v2: Make it compile. There was so much compile fail on arm drivers
> that I figured I'll better not include any of the acks on v1.
> 
> v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
> not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
> there was still one, which this patch largely removes. Which means
> rolling out lots more includes all over.
> 
> This will also conflict with ongoing drmP.h cleanup by others I
> expect.
> 
> v3: Rebase on top of atomic bochs.
> 
> Cc: Sam Ravnborg 
> Cc: Jani Nikula 
> Cc: Laurent Pinchart 
> Acked-by: Rodrigo Vivi  (v2)
> Acked-by: Benjamin Gaignard  (v2)
> Signed-off-by: Daniel Vetter 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: etna...@lists.freedesktop.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: spice-de...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> Cc: linux-te...@vger.kernel.org
> Cc: xen-de...@lists.xen.org
> ---
> Merging this is going to be a bit a mess due to all the ongoing drmP.h
> cleanups. I think the following should work:
> - Apply Sam's prep patches for removing drmP.h from
>   drm_modeset_helper.h
> - Get the i915 drmP.h cleanup backmerged into drm-misc-next
> - Apply this patch.
> - Apply Sam's patch to remove drmP.h from drm_modeset_helper.h
> - All through drm-misc-next, which has some potential for trivial
>   conflicts around #includes with other drivers unfortunately.
> 
> I hope there's no other driver who'll blow up accidentally because
> someone else is doing a drmP.h cleanup. Laurent maybe?
> 
> Jani, ack on this?
> -Daniel
> ---
>  .../gpu/drm/amd/amdgpu/amdgpu_connectors.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  1 +
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  2 +-
>  .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c  |  2 +-
>  .../display/amdgpu_dm/amdgpu_dm_services.c|  2 +-
>  drivers/gpu/drm/arc/arcpgu_crtc.c |  2 +-
>  drivers/gpu/drm/arc/arcpgu_drv.c  |  2 +-
>  drivers/gpu/drm/arc/arcpgu_sim.c  |  2 +-
>  drivers/gpu/drm/arm/hdlcd_crtc.c  |  2 +-
>  drivers/gpu/drm/arm/hdlcd_drv.c   |  2 +-
>  drivers/gpu/drm/arm/malidp_crtc.c |  2 +-
>  drivers/gpu/drm/arm/malidp_drv.c  |  2 +-
>  drivers/gpu/drm/arm/malidp_mw.c   |  2 +-
>  drivers/gpu/drm/armada/armada_510.c   |  2 +-
>  drivers/gpu/drm/armada/armada_crtc.c  |  2 +-
>  drivers/gpu/drm/armada/armada_crtc.h  |  2 +
>  drivers/gpu/drm/armada/armada_drv.c   |  2 +-
>  drivers/gpu/drm/armada/armada_fb.c|  2 +-
>  drivers/gpu/drm/ast/ast_drv.c |  1 +
>  drivers/gpu/drm/ast/ast_mode.c|  1 +
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  2 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  2 +-
>  drivers/gpu/drm/bochs/bochs_drv.c |  1 +
>  drivers/gpu/drm/bochs/bochs_kms.c |  1 +
>  drivers/gpu/drm/bridge/adv7511/adv7511.h  |  5 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  2 +-
>  .../drm/bridge/analogix/analogix_dp_core.c|  2 +-
>  drivers/gpu/drm/bridge/cdns-dsi.c |  2 +-
>  drivers/gpu/drm/bridge/dumb-vga-dac.c |  2 +-
>  .../bridge/megachips-stdp-ge-b850v3-fw.c  |  2 +-
>  drivers/gpu/drm/bridge/nxp-ptn3460.c  |  2 +-
>  drivers/gpu/drm/bridge/panel.c|  2 +-
>  drivers/gpu/drm/bridge/parade-ps8622.c|  2 +-
>  drivers/gpu/drm/bridge/sii902x.c  |  2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c |  2 +-
>  drivers/gpu/drm/bridge/tc358764.c |  2 +-
>  drivers/gpu/drm/bridge/tc358767.c |  2 +-
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c |  2 +-
>  drivers/gpu/drm/bridge/ti-tfp410.c|  2 +-
>  drivers/gpu/drm/cirrus/cirrus_drv.c   |  1 +
>  drivers/gpu/drm/cirrus/cirrus_mode.c  |  1 +
>  drivers/gpu/drm/drm_atomic_helper.c   |  1 -
>  drivers/gpu/drm/drm_dp_mst_topology.c |  2 +-
>  

[PATCH v7 5/7] iommu: Add virtio-iommu driver

2019-01-16 Thread Jean-Philippe Brucker
The virtio IOMMU is a para-virtualized device, allowing to send IOMMU
requests such as map/unmap over virtio transport without emulating page
tables. This implementation handles ATTACH, DETACH, MAP and UNMAP
requests.

The bulk of the code transforms calls coming from the IOMMU API into
corresponding virtio requests. Mappings are kept in an interval tree
instead of page tables. A little more work is required for modular and x86
support, so for the moment the driver depends on CONFIG_VIRTIO=y and
CONFIG_ARM64.

Tested-by: Bharat Bhushan 
Tested-by: Eric Auger 
Reviewed-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 MAINTAINERS   |   7 +
 drivers/iommu/Kconfig |  11 +
 drivers/iommu/Makefile|   1 +
 drivers/iommu/virtio-iommu.c  | 916 ++
 include/uapi/linux/virtio_ids.h   |   1 +
 include/uapi/linux/virtio_iommu.h | 106 
 6 files changed, 1042 insertions(+)
 create mode 100644 drivers/iommu/virtio-iommu.c
 create mode 100644 include/uapi/linux/virtio_iommu.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d04cebb4a71..1ef06a1e0525 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16274,6 +16274,13 @@ S: Maintained
 F: drivers/virtio/virtio_input.c
 F: include/uapi/linux/virtio_input.h
 
+VIRTIO IOMMU DRIVER
+M: Jean-Philippe Brucker 
+L: virtualization@lists.linux-foundation.org
+S: Maintained
+F: drivers/iommu/virtio-iommu.c
+F: include/uapi/linux/virtio_iommu.h
+
 VIRTUAL BOX GUEST DEVICE DRIVER
 M: Hans de Goede 
 M: Arnd Bergmann 
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index d9a25715650e..d507fd754214 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -435,4 +435,15 @@ config QCOM_IOMMU
help
  Support for IOMMU on certain Qualcomm SoCs.
 
+config VIRTIO_IOMMU
+   bool "Virtio IOMMU driver"
+   depends on VIRTIO=y
+   depends on ARM64
+   select IOMMU_API
+   select INTERVAL_TREE
+   help
+ Para-virtualised IOMMU driver with virtio.
+
+ Say Y here if you intend to run this kernel as a guest.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index a158a68c8ea8..48d831a39281 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
 obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
+obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
new file mode 100644
index ..6fa012cd727e
--- /dev/null
+++ b/drivers/iommu/virtio-iommu.c
@@ -0,0 +1,916 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtio driver for the paravirtualized IOMMU
+ *
+ * Copyright (C) 2018 Arm Limited
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define MSI_IOVA_BASE  0x800
+#define MSI_IOVA_LENGTH0x10
+
+#define VIOMMU_REQUEST_VQ  0
+#define VIOMMU_NR_VQS  1
+
+struct viommu_dev {
+   struct iommu_device iommu;
+   struct device   *dev;
+   struct virtio_device*vdev;
+
+   struct ida  domain_ids;
+
+   struct virtqueue*vqs[VIOMMU_NR_VQS];
+   spinlock_t  request_lock;
+   struct list_headrequests;
+
+   /* Device configuration */
+   struct iommu_domain_geometrygeometry;
+   u64 pgsize_bitmap;
+   u8  domain_bits;
+};
+
+struct viommu_mapping {
+   phys_addr_t paddr;
+   struct interval_tree_node   iova;
+   u32 flags;
+};
+
+struct viommu_domain {
+   struct iommu_domain domain;
+   struct viommu_dev   *viommu;
+   struct mutexmutex; /* protects viommu pointer */
+   unsigned intid;
+
+   spinlock_t  mappings_lock;
+   struct rb_root_cached   mappings;
+
+   unsigned long   nr_endpoints;
+};
+
+struct viommu_endpoint {
+   struct viommu_dev   *viommu;
+   struct viommu_domain*vdomain;
+};
+
+struct viommu_request {
+   struct list_headlist;
+   void*writeback;
+   unsigned intwrite_offset;
+   unsigned intlen;
+   charbuf[];
+};
+
+#define to_viommu_domain(domain)   \
+   container_of(domain, struct viommu_domain, 

[PATCH v7 6/7] iommu/virtio: Add probe request

2019-01-16 Thread Jean-Philippe Brucker
When the device offers the probe feature, send a probe request for each
device managed by the IOMMU. Extract RESV_MEM information. When we
encounter a MSI doorbell region, set it up as a IOMMU_RESV_MSI region.
This will tell other subsystems that there is no need to map the MSI
doorbell in the virtio-iommu, because MSIs bypass it.

Tested-by: Bharat Bhushan 
Tested-by: Eric Auger 
Reviewed-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 drivers/iommu/virtio-iommu.c  | 157 --
 include/uapi/linux/virtio_iommu.h |  36 +++
 2 files changed, 187 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 6fa012cd727e..5e194493a531 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -46,6 +46,7 @@ struct viommu_dev {
struct iommu_domain_geometrygeometry;
u64 pgsize_bitmap;
u8  domain_bits;
+   u32 probe_size;
 };
 
 struct viommu_mapping {
@@ -67,8 +68,10 @@ struct viommu_domain {
 };
 
 struct viommu_endpoint {
+   struct device   *dev;
struct viommu_dev   *viommu;
struct viommu_domain*vdomain;
+   struct list_headresv_regions;
 };
 
 struct viommu_request {
@@ -119,6 +122,9 @@ static off_t viommu_get_write_desc_offset(struct viommu_dev 
*viommu,
 {
size_t tail_size = sizeof(struct virtio_iommu_req_tail);
 
+   if (req->type == VIRTIO_IOMMU_T_PROBE)
+   return len - viommu->probe_size - tail_size;
+
return len - tail_size;
 }
 
@@ -393,6 +399,110 @@ static int viommu_replay_mappings(struct viommu_domain 
*vdomain)
return ret;
 }
 
+static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
+  struct virtio_iommu_probe_resv_mem *mem,
+  size_t len)
+{
+   size_t size;
+   u64 start64, end64;
+   phys_addr_t start, end;
+   struct iommu_resv_region *region = NULL;
+   unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+   start = start64 = le64_to_cpu(mem->start);
+   end = end64 = le64_to_cpu(mem->end);
+   size = end64 - start64 + 1;
+
+   /* Catch any overflow, including the unlikely end64 - start64 + 1 = 0 */
+   if (start != start64 || end != end64 || size < end64 - start64)
+   return -EOVERFLOW;
+
+   if (len < sizeof(*mem))
+   return -EINVAL;
+
+   switch (mem->subtype) {
+   default:
+   dev_warn(vdev->dev, "unknown resv mem subtype 0x%x\n",
+mem->subtype);
+   /* Fall-through */
+   case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
+   region = iommu_alloc_resv_region(start, size, 0,
+IOMMU_RESV_RESERVED);
+   break;
+   case VIRTIO_IOMMU_RESV_MEM_T_MSI:
+   region = iommu_alloc_resv_region(start, size, prot,
+IOMMU_RESV_MSI);
+   break;
+   }
+   if (!region)
+   return -ENOMEM;
+
+   list_add(>resv_regions, >list);
+   return 0;
+}
+
+static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
+{
+   int ret;
+   u16 type, len;
+   size_t cur = 0;
+   size_t probe_len;
+   struct virtio_iommu_req_probe *probe;
+   struct virtio_iommu_probe_property *prop;
+   struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+   struct viommu_endpoint *vdev = fwspec->iommu_priv;
+
+   if (!fwspec->num_ids)
+   return -EINVAL;
+
+   probe_len = sizeof(*probe) + viommu->probe_size +
+   sizeof(struct virtio_iommu_req_tail);
+   probe = kzalloc(probe_len, GFP_KERNEL);
+   if (!probe)
+   return -ENOMEM;
+
+   probe->head.type = VIRTIO_IOMMU_T_PROBE;
+   /*
+* For now, assume that properties of an endpoint that outputs multiple
+* IDs are consistent. Only probe the first one.
+*/
+   probe->endpoint = cpu_to_le32(fwspec->ids[0]);
+
+   ret = viommu_send_req_sync(viommu, probe, probe_len);
+   if (ret)
+   goto out_free;
+
+   prop = (void *)probe->properties;
+   type = le16_to_cpu(prop->type) & VIRTIO_IOMMU_PROBE_T_MASK;
+
+   while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
+  cur < viommu->probe_size) {
+   len = le16_to_cpu(prop->length) + sizeof(*prop);
+
+   switch (type) {
+   case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
+   ret = viommu_add_resv_mem(vdev, (void *)prop, len);
+   break;
+   default:
+   dev_err(dev, "unknown viommu prop 0x%x\n", type);
+   }
+
+   if (ret)
+  

[PATCH v7 4/7] PCI: OF: Initialize dev->fwnode appropriately

2019-01-16 Thread Jean-Philippe Brucker
For PCI devices that have an OF node, set the fwnode as well. This way
drivers that rely on fwnode don't need the special case described by
commit f94277af03ea ("of/platform: Initialise dev->fwnode appropriately").

Acked-by: Bjorn Helgaas 
Signed-off-by: Jean-Philippe Brucker 
---
 drivers/pci/of.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 4c4217d0c3f1..c272ecfcd038 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -21,12 +21,15 @@ void pci_set_of_node(struct pci_dev *dev)
return;
dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
dev->devfn);
+   if (dev->dev.of_node)
+   dev->dev.fwnode = >dev.of_node->fwnode;
 }
 
 void pci_release_of_node(struct pci_dev *dev)
 {
of_node_put(dev->dev.of_node);
dev->dev.of_node = NULL;
+   dev->dev.fwnode = NULL;
 }
 
 void pci_set_bus_of_node(struct pci_bus *bus)
@@ -35,12 +38,16 @@ void pci_set_bus_of_node(struct pci_bus *bus)
bus->dev.of_node = pcibios_get_phb_of_node(bus);
else
bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+
+   if (bus->dev.of_node)
+   bus->dev.fwnode = >dev.of_node->fwnode;
 }
 
 void pci_release_bus_of_node(struct pci_bus *bus)
 {
of_node_put(bus->dev.of_node);
bus->dev.of_node = NULL;
+   bus->dev.fwnode = NULL;
 }
 
 struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
-- 
2.19.1

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


[PATCH v7 3/7] of: Allow the iommu-map property to omit untranslated devices

2019-01-16 Thread Jean-Philippe Brucker
In PCI root complex nodes, the iommu-map property describes the IOMMU that
translates each endpoint. On some platforms, the IOMMU itself is presented
as a PCI endpoint (e.g. AMD IOMMU and virtio-iommu). This isn't supported
by the current OF driver, which expects all endpoints to have an IOMMU.
Allow the iommu-map property to have gaps.

Relaxing of_map_rid() also allows the msi-map property to have gaps, which
is invalid since MSIs always reach an MSI controller. In that case
pci_msi_setup_msi_irqs() will return an error when attempting to find the
device's MSI domain.

Reviewed-by: Rob Herring 
Signed-off-by: Jean-Philippe Brucker 
---
 drivers/of/base.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 5226e898476e..4d12b1cab55f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2293,8 +2293,12 @@ int of_map_rid(struct device_node *np, u32 rid,
return 0;
}
 
-   pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
-   np, map_name, rid, target && *target ? *target : NULL);
-   return -EFAULT;
+   pr_info("%pOF: no %s translation for rid 0x%x on %pOF\n", np, map_name,
+   rid, target && *target ? *target : NULL);
+
+   /* Bypasses translation */
+   if (id_out)
+   *id_out = rid;
+   return 0;
 }
 EXPORT_SYMBOL_GPL(of_map_rid);
-- 
2.19.1

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


[PATCH v7 2/7] dt-bindings: virtio: Add virtio-pci-iommu node

2019-01-16 Thread Jean-Philippe Brucker
Some systems implement virtio-iommu as a PCI endpoint. The operating
system needs to discover the relationship between IOMMU and masters long
before the PCI endpoint gets probed. Add a PCI child node to describe the
virtio-iommu device.

The virtio-pci-iommu is conceptually split between a PCI programming
interface and a translation component on the parent bus. The latter
doesn't have a node in the device tree. The virtio-pci-iommu node
describes both, by linking the PCI endpoint to "iommus" property of DMA
master nodes and to "iommu-map" properties of bus nodes.

Reviewed-by: Rob Herring 
Reviewed-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 .../devicetree/bindings/virtio/iommu.txt  | 66 +++
 1 file changed, 66 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt

diff --git a/Documentation/devicetree/bindings/virtio/iommu.txt 
b/Documentation/devicetree/bindings/virtio/iommu.txt
new file mode 100644
index ..2407fea0651c
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/iommu.txt
@@ -0,0 +1,66 @@
+* virtio IOMMU PCI device
+
+When virtio-iommu uses the PCI transport, its programming interface is
+discovered dynamically by the PCI probing infrastructure. However the
+device tree statically describes the relation between IOMMU and DMA
+masters. Therefore, the PCI root complex that hosts the virtio-iommu
+contains a child node representing the IOMMU device explicitly.
+
+Required properties:
+
+- compatible:  Should be "virtio,pci-iommu"
+- reg: PCI address of the IOMMU. As defined in the PCI Bus
+   Binding reference [1], the reg property is a five-cell
+   address encoded as (phys.hi phys.mid phys.lo size.hi
+   size.lo). phys.hi should contain the device's BDF as
+   0b  dfff . The other cells
+   should be zero.
+- #iommu-cells:Each platform DMA master managed by the IOMMU is 
assigned
+   an endpoint ID, described by the "iommus" property [2].
+   For virtio-iommu, #iommu-cells must be 1.
+
+Notes:
+
+- DMA from the IOMMU device isn't managed by another IOMMU. Therefore the
+  virtio-iommu node doesn't have an "iommus" property, and is omitted from
+  the iommu-map property of the root complex.
+
+Example:
+
+pcie@1000 {
+   compatible = "pci-host-ecam-generic";
+   ...
+
+   /* The IOMMU programming interface uses slot 00:01.0 */
+   iommu0: iommu@0008 {
+   compatible = "virtio,pci-iommu";
+   reg = <0x0800 0 0 0 0>;
+   #iommu-cells = <1>;
+   };
+
+   /*
+* The IOMMU manages all functions in this PCI domain except
+* itself. Omit BDF 00:01.0.
+*/
+   iommu-map = <0x0  0x0 0x8>
+   <0x9  0x9 0xfff7>;
+};
+
+pcie@2000 {
+   compatible = "pci-host-ecam-generic";
+   ...
+   /*
+* The IOMMU also manages all functions from this domain,
+* with endpoint IDs 0x1 - 0x1
+*/
+   iommu-map = <0x0  0x1 0x1>;
+};
+
+ethernet@fe001000 {
+   ...
+   /* The IOMMU manages this platform device with endpoint ID 0x2 */
+   iommus = < 0x2>;
+};
+
+[1] Documentation/devicetree/bindings/pci/pci.txt
+[2] Documentation/devicetree/bindings/iommu/iommu.txt
-- 
2.19.1

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


[PATCH v7 1/7] dt-bindings: virtio-mmio: Add IOMMU description

2019-01-16 Thread Jean-Philippe Brucker
The nature of a virtio-mmio node is discovered by the virtio driver at
probe time. However the DMA relation between devices must be described
statically. When a virtio-mmio node is a virtio-iommu device, it needs an
"#iommu-cells" property as specified by bindings/iommu/iommu.txt.

Otherwise, the virtio-mmio device may perform DMA through an IOMMU, which
requires an "iommus" property. Describe these requirements in the
device-tree bindings documentation.

Reviewed-by: Rob Herring 
Reviewed-by: Eric Auger 
Signed-off-by: Jean-Philippe Brucker 
---
 .../devicetree/bindings/virtio/mmio.txt   | 30 +++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/virtio/mmio.txt 
b/Documentation/devicetree/bindings/virtio/mmio.txt
index 5069c1b8e193..21af30fbb81f 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.txt
+++ b/Documentation/devicetree/bindings/virtio/mmio.txt
@@ -8,10 +8,40 @@ Required properties:
 - reg: control registers base address and size including configuration 
space
 - interrupts:  interrupt generated by the device
 
+Required properties for virtio-iommu:
+
+- #iommu-cells:When the node corresponds to a virtio-iommu device, it 
is
+   linked to DMA masters using the "iommus" or "iommu-map"
+   properties [1][2]. #iommu-cells specifies the size of the
+   "iommus" property. For virtio-iommu #iommu-cells must be
+   1, each cell describing a single endpoint ID.
+
+Optional properties:
+
+- iommus:  If the device accesses memory through an IOMMU, it should
+   have an "iommus" property [1]. Since virtio-iommu itself
+   does not access memory through an IOMMU, the "virtio,mmio"
+   node cannot have both an "#iommu-cells" and an "iommus"
+   property.
+
 Example:
 
virtio_block@3000 {
compatible = "virtio,mmio";
reg = <0x3000 0x100>;
interrupts = <41>;
+
+   /* Device has endpoint ID 23 */
+   iommus = < 23>
}
+
+   viommu: iommu@3100 {
+   compatible = "virtio,mmio";
+   reg = <0x3100 0x100>;
+   interrupts = <42>;
+
+   #iommu-cells = <1>
+   }
+
+[1] Documentation/devicetree/bindings/iommu/iommu.txt
+[2] Documentation/devicetree/bindings/pci/pci-iommu.txt
-- 
2.19.1

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


[PATCH v7 0/7] Add virtio-iommu driver

2019-01-16 Thread Jean-Philippe Brucker
Implement the virtio-iommu driver, following specification v0.9 [1].

This is a simple rebase onto Linux v5.0-rc2. We now use the
dev_iommu_fwspec_get() helper introduced in v5.0 instead of accessing
dev->iommu_fwspec, but there aren't any functional change from v6 [2].

Our current goal for virtio-iommu is to get a paravirtual IOMMU working
on Arm, and enable device assignment to guest userspace. In this
use-case the mappings are static, and don't require optimal performance,
so this series tries to keep things simple. However there is plenty more
to do for features and optimizations, and having this base in v5.1 would
be good. Given that most of the changes are to drivers/iommu, I believe
the driver and future changes should go via the IOMMU tree.

You can find Linux driver and kvmtool device on v0.9.2 branches [3],
module and x86 support on virtio-iommu/devel. Also tested with Eric's
QEMU device [4]. Please note that the series depends on Robin's
probe-deferral fix [5], which will hopefully land in v5.0.

[1] Virtio-iommu specification v0.9, sources and pdf
git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf

[2] [PATCH v6 0/7] Add virtio-iommu driver
https://lists.linuxfoundation.org/pipermail/iommu/2018-December/032127.html

[3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.2
git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9.2

[4] [RFC v9 00/17] VIRTIO-IOMMU device
https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html

[5] [PATCH] iommu/of: Fix probe-deferral
https://www.spinics.net/lists/arm-kernel/msg698371.html

Jean-Philippe Brucker (7):
  dt-bindings: virtio-mmio: Add IOMMU description
  dt-bindings: virtio: Add virtio-pci-iommu node
  of: Allow the iommu-map property to omit untranslated devices
  PCI: OF: Initialize dev->fwnode appropriately
  iommu: Add virtio-iommu driver
  iommu/virtio: Add probe request
  iommu/virtio: Add event queue

 .../devicetree/bindings/virtio/iommu.txt  |   66 +
 .../devicetree/bindings/virtio/mmio.txt   |   30 +
 MAINTAINERS   |7 +
 drivers/iommu/Kconfig |   11 +
 drivers/iommu/Makefile|1 +
 drivers/iommu/virtio-iommu.c  | 1158 +
 drivers/of/base.c |   10 +-
 drivers/pci/of.c  |7 +
 include/uapi/linux/virtio_ids.h   |1 +
 include/uapi/linux/virtio_iommu.h |  161 +++
 10 files changed, 1449 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
 create mode 100644 drivers/iommu/virtio-iommu.c
 create mode 100644 include/uapi/linux/virtio_iommu.h

-- 
2.19.1

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


Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-16 Thread Joerg Roedel
On Mon, Jan 14, 2019 at 01:20:45PM -0500, Michael S. Tsirkin wrote:
> Which would be fine especially if we can manage not to introduce a bunch
> of indirect calls all over the place and hurt performance.

Which indirect calls? In case of unset dma_ops the DMA-API functions
call directly into the dma-direct implementation, no indirect calls at
all.

Regards,

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


Re: [PATCH] drm: Split out drm_probe_helper.h

2019-01-16 Thread Jani Nikula
On Tue, 15 Jan 2019, Daniel Vetter  wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
>
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.
>
> v2: Make it compile. There was so much compile fail on arm drivers
> that I figured I'll better not include any of the acks on v1.
>
> v3: Massive rebase because i915 has lost a lot of drmP.h includes, but
> not all: Through drm_crtc_helper.h > drm_modeset_helper.h -> drmP.h
> there was still one, which this patch largely removes. Which means
> rolling out lots more includes all over.
>
> This will also conflict with ongoing drmP.h cleanup by others I
> expect.
>
> v3: Rebase on top of atomic bochs.
>
> Cc: Sam Ravnborg 
> Cc: Jani Nikula 
> Cc: Laurent Pinchart 
> Acked-by: Rodrigo Vivi  (v2)
> Acked-by: Benjamin Gaignard  (v2)
> Signed-off-by: Daniel Vetter 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: virtualization@lists.linux-foundation.org
> Cc: etna...@lists.freedesktop.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-amlo...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: spice-de...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: linux-renesas-...@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-st...@st-md-mailman.stormreply.com
> Cc: linux-te...@vger.kernel.org
> Cc: xen-de...@lists.xen.org
> ---
> Merging this is going to be a bit a mess due to all the ongoing drmP.h
> cleanups. I think the following should work:
> - Apply Sam's prep patches for removing drmP.h from
>   drm_modeset_helper.h
> - Get the i915 drmP.h cleanup backmerged into drm-misc-next
> - Apply this patch.
> - Apply Sam's patch to remove drmP.h from drm_modeset_helper.h
> - All through drm-misc-next, which has some potential for trivial
>   conflicts around #includes with other drivers unfortunately.
>
> I hope there's no other driver who'll blow up accidentally because
> someone else is doing a drmP.h cleanup. Laurent maybe?
>
> Jani, ack on this?

Acked-by: Jani Nikula 



-- 
Jani Nikula, Intel Open Source Graphics Center
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net V3] vhost: log dirty page correctly

2019-01-16 Thread Jason Wang


On 2019/1/15 上午2:04, Michael S. Tsirkin wrote:

On Fri, Jan 11, 2019 at 12:00:36PM +0800, Jason Wang wrote:

Vhost dirty page logging API is designed to sync through GPA. But we
try to log GIOVA when device IOTLB is enabled. This is wrong and may
lead to missing data after migration.

To solve this issue, when logging with device IOTLB enabled, we will:

1) reuse the device IOTLB translation result of GIOVA->HVA mapping to
get HVA, for writable descriptor, get HVA through iovec. For used
ring update, translate its GIOVA to HVA
2) traverse the GPA->HVA mapping to get the possible GPA and log
through GPA. Pay attention this reverse mapping is not guaranteed
to be unique, so we should log each possible GPA in this case.

This fix the failure of scp to guest during migration. In -next, we
will probably support passing GIOVA->GPA instead of GIOVA->HVA.

Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Reported-by: Jintack Lim
Cc: Jintack Lim
Signed-off-by: Jason Wang
---
Changes from V2:
- check and log the case of range overlap
- remove unnecessary u64 cast
- use smp_wmb() for the case of device IOTLB as well
Changes from V1:
- return error instead of warn
---
  drivers/vhost/net.c   |  3 +-
  drivers/vhost/vhost.c | 88 ---
  drivers/vhost/vhost.h |  3 +-
  3 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 36f3d0f49e60..bca86bf7189f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1236,7 +1236,8 @@ static void handle_rx(struct vhost_net *net)
if (nvq->done_idx > VHOST_NET_BATCH)
vhost_net_signal_used(nvq);
if (unlikely(vq_log))
-   vhost_log_write(vq, vq_log, log, vhost_len);
+   vhost_log_write(vq, vq_log, log, vhost_len,
+   vq->iov, in);
total_len += vhost_len;
if (unlikely(vhost_exceeds_weight(++recv_pkts, total_len))) {
vhost_poll_queue(>poll);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9f7942cbcbb2..55a2e8f9f8ca 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1733,13 +1733,78 @@ static int log_write(void __user *log_base,
return r;
  }
  
+static int log_write_hva(struct vhost_virtqueue *vq, u64 hva, u64 len)

+{
+   struct vhost_umem *umem = vq->umem;
+   struct vhost_umem_node *u;
+   u64 start, end;
+   int r;
+   bool hit = false;
+
+   /* More than one GPAs can be mapped into a single HVA. So
+* iterate all possible umems here to be safe.
+*/
+   list_for_each_entry(u, >umem_list, link) {
+   if (u->userspace_addr > hva - 1 + len ||
+   u->userspace_addr - 1 + u->size < hva)
+   continue;
+   start = max(u->userspace_addr, hva);
+   end = min(u->userspace_addr - 1 + u->size, hva - 1 + len);
+   r = log_write(vq->log_base,
+ u->start + start - u->userspace_addr,
+ end - start + 1);
+   if (r < 0)
+   return r;
+   hit = true;
+   }
+
+   if (!hit)
+   return -EFAULT;
+
+   return 0;
+}
+

I definitely like the simplicity.

But there's one point left here: if len crosses a region boundary,
but doesn't all fall within a region, we don't consistently report -EFAULT.

So I suspect we need to start by finding a region that contains hva.
If there are many of these - move right to the end of the
leftmost one and then repeat until we run out of len
or fail to find a region and exit with -EFAULT.



Ok, will do it in V4.

Thanks






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

Re: [PATCH v3 2/5] virtio-pmem: Add virtio pmem driver

2019-01-16 Thread Pankaj Gupta


> > This patch adds virtio-pmem driver for KVM guest.
> > 
> > Guest reads the persistent memory range information from
> > Qemu over VIRTIO and registers it on nvdimm_bus. It also
> > creates a nd_region object with the persistent memory
> > range information so that existing 'nvdimm/pmem' driver
> > can reserve this into system memory map. This way
> > 'virtio-pmem' driver uses existing functionality of pmem
> > driver to register persistent memory compatible for DAX
> > capable filesystems.
> > 
> > This also provides function to perform guest flush over
> > VIRTIO from 'pmem' driver when userspace performs flush
> > on DAX memory range.
> > 
> > Signed-off-by: Pankaj Gupta 
> > ---
> >  drivers/nvdimm/virtio_pmem.c |  84 ++
> >  drivers/virtio/Kconfig   |  10 
> >  drivers/virtio/Makefile  |   1 +
> >  drivers/virtio/pmem.c| 124
> >  +++
> >  include/linux/virtio_pmem.h  |  60 +++
> >  include/uapi/linux/virtio_ids.h  |   1 +
> >  include/uapi/linux/virtio_pmem.h |  10 
> 
> As with any uapi change, you need to CC the virtio dev
> mailing list (subscribers only, sorry about that).

Sure, will add virtio dev mailing list while sending v4.

Thanks,
Pankaj

> 
> 
> >  7 files changed, 290 insertions(+)
> >  create mode 100644 drivers/nvdimm/virtio_pmem.c
> >  create mode 100644 drivers/virtio/pmem.c
> >  create mode 100644 include/linux/virtio_pmem.h
> >  create mode 100644 include/uapi/linux/virtio_pmem.h
> > 
> > diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> > new file mode 100644
> > index 000..2a1b1ba
> > --- /dev/null
> > +++ b/drivers/nvdimm/virtio_pmem.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * virtio_pmem.c: Virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + */
> > +#include 
> > +#include "nd.h"
> > +
> > + /* The interrupt handler */
> > +void host_ack(struct virtqueue *vq)
> > +{
> > +   unsigned int len;
> > +   unsigned long flags;
> > +   struct virtio_pmem_request *req, *req_buf;
> > +   struct virtio_pmem *vpmem = vq->vdev->priv;
> > +
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +   while ((req = virtqueue_get_buf(vq, )) != NULL) {
> > +   req->done = true;
> > +   wake_up(>host_acked);
> > +
> > +   if (!list_empty(>req_list)) {
> > +   req_buf = list_first_entry(>req_list,
> > +   struct virtio_pmem_request, list);
> > +   list_del(>req_list);
> > +   req_buf->wq_buf_avail = true;
> > +   wake_up(_buf->wq_buf);
> > +   }
> > +   }
> > +   spin_unlock_irqrestore(>pmem_lock, flags);
> > +}
> > +EXPORT_SYMBOL_GPL(host_ack);
> > +
> > + /* The request submission function */
> > +int virtio_pmem_flush(struct nd_region *nd_region)
> > +{
> > +   int err;
> > +   unsigned long flags;
> > +   struct scatterlist *sgs[2], sg, ret;
> > +   struct virtio_device *vdev = nd_region->provider_data;
> > +   struct virtio_pmem *vpmem = vdev->priv;
> > +   struct virtio_pmem_request *req;
> > +
> > +   might_sleep();
> > +   req = kmalloc(sizeof(*req), GFP_KERNEL);
> > +   if (!req)
> > +   return -ENOMEM;
> > +
> > +   req->done = req->wq_buf_avail = false;
> > +   strcpy(req->name, "FLUSH");
> > +   init_waitqueue_head(>host_acked);
> > +   init_waitqueue_head(>wq_buf);
> > +   sg_init_one(, req->name, strlen(req->name));
> > +   sgs[0] = 
> > +   sg_init_one(, >ret, sizeof(req->ret));
> > +   sgs[1] = 
> > +
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +   err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req, GFP_ATOMIC);
> > +   if (err) {
> > +   dev_err(>dev, "failed to send command to virtio pmem 
> > device\n");
> > +
> > +   list_add_tail(>req_list, >list);
> > +   spin_unlock_irqrestore(>pmem_lock, flags);
> > +
> > +   /* When host has read buffer, this completes via host_ack */
> > +   wait_event(req->wq_buf, req->wq_buf_avail);
> > +   spin_lock_irqsave(>pmem_lock, flags);
> > +   }
> > +   virtqueue_kick(vpmem->req_vq);
> > +   spin_unlock_irqrestore(>pmem_lock, flags);
> > +
> > +   /* When host has read buffer, this completes via host_ack */
> > +   wait_event(req->host_acked, req->done);
> > +   err = req->ret;
> > +   kfree(req);
> > +
> > +   return err;
> > +};
> > +EXPORT_SYMBOL_GPL(virtio_pmem_flush);
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> > index 3589764..9f634a2 100644
> > --- a/drivers/virtio/Kconfig
> > +++ b/drivers/virtio/Kconfig
> > @@ -42,6 +42,16 @@ config VIRTIO_PCI_LEGACY
> >  
> >   If unsure, say Y.
> >  
> > +config VIRTIO_PMEM
> > +   tristate "Support for virtio pmem driver"
> > +   depends on VIRTIO
> > +   depends on 

Re: [Qemu-devel] [PATCH v3 0/5] kvm "virtio pmem" device

2019-01-16 Thread Pankaj Gupta


> > > >
> > > > On Mon, Jan 14, 2019 at 02:15:40AM -0500, Pankaj Gupta wrote:
> > > > >
> > > > > > > Until you have images (and hence host page cache) shared between
> > > > > > > multiple guests. People will want to do this, because it means
> > > > > > > they
> > > > > > > only need a single set of pages in host memory for executable
> > > > > > > binaries rather than a set of pages per guest. Then you have
> > > > > > > multiple guests being able to detect residency of the same set of
> > > > > > > pages. If the guests can then, in any way, control eviction of
> > > > > > > the
> > > > > > > pages from the host cache, then we have a guest-to-guest
> > > > > > > information
> > > > > > > leak channel.
> > > > > >
> > > > > > I don't think we should ever be considering something that would
> > > > > > allow a
> > > > > > guest to evict page's from the host's pagecache [1].  The guest
> > > > > > should
> > > > > > be able to kick its own references to the host's pagecache out of
> > > > > > its
> > > > > > own pagecache, but not be able to influence whether the host or
> > > > > > another
> > > > > > guest has a read-only mapping cached.
> > > > > >
> > > > > > [1] Unless the guest is allowed to modify the host's file;
> > > > > > obviously
> > > > > > truncation, holepunching, etc are going to evict pages from the
> > > > > > host's
> > > > > > page cache.
> > > > >
> > > > > This is so correct. Guest does not not evict host page cache pages
> > > > > directly.
> > > >
> > > > They don't right now.
> > > >
> > > > But someone is going to end up asking for discard to work so that
> > > > the guest can free unused space in the underlying spares image (i.e.
> > > > make use of fstrim or mount -o discard) because they have workloads
> > > > that have bursts of space usage and they need to trim the image
> > > > files afterwards to keep their overall space usage under control.
> > > >
> > > > And then
> > > 
> > > ...we reject / push back on that patch citing the above concern.
> > 
> > So at what point do we draw the line?
> > 
> > We're allowing writable DAX mappings, but as I've pointed out that
> > means we are going to be allowing  a potential information leak via
> > files with shared extents to be directly mapped and written to.
> > 
> > But we won't allow useful admin operations that allow better
> > management of host side storage space similar to how normal image
> > files are used by guests because it's an information leak vector?
> > 
> > That's splitting some really fine hairs there...
> 
> May I summarize that th security implications need to
> be documented?
> 
> In fact that would make a fine security implications section
> in the device specification.

This is a very good suggestion. 

I will document the security implications in details in device specification
with details of what all filesystem features we don't support and why.

Best regards,
Pankaj

> 
> 
> 
> 
> 
> > > > > In case of virtio-pmem & DAX, guest clears guest page cache
> > > > > exceptional entries.
> > > > > Its solely decision of host to take action on the host page cache
> > > > > pages.
> > > > >
> > > > > In case of virtio-pmem, guest does not modify host file directly i.e
> > > > > don't
> > > > > perform hole punch & truncation operation directly on host file.
> > > >
> > > > ... this will no longer be true, and the nuclear landmine in this
> > > > driver interface will have been armed
> > > 
> > > I agree with the need to be careful when / if explicit cache control
> > > is added, but that's not the case today.
> > 
> > "if"?
> > 
> > I expect it to be "when", not if. Expect the worst, plan for it now.
> > 
> > Cheers,
> > 
> > Dave.
> > --
> > Dave Chinner
> > da...@fromorbit.com
> 
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 1/2] virtio: fix virtio_config_ops description

2019-01-16 Thread Michael S. Tsirkin
On Tue, Jan 15, 2019 at 10:15:39AM +0800, Wei Wang wrote:
> On 01/14/2019 10:09 PM, Cornelia Huck wrote:
> > On Thu,  3 Jan 2019 17:08:03 +0100
> > Cornelia Huck  wrote:
> > 
> > > - get_features has returned 64 bits since commit d025477368792
> > >("virtio: add support for 64 bit features.")
> > > - properly mark all optional callbacks
> > > 
> > > Signed-off-by: Cornelia Huck 
> > > ---
> > >   include/linux/virtio_config.h | 8 
> > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > index 32baf8e26735..7087ef946ba7 100644
> > > --- a/include/linux/virtio_config.h
> > > +++ b/include/linux/virtio_config.h
> > > @@ -22,7 +22,7 @@ struct irq_affinity;
> > >*  offset: the offset of the configuration field
> > >*  buf: the buffer to read the field value from.
> > >*  len: the length of the buffer
> > > - * @generation: config generation counter
> > > + * @generation: config generation counter (optional)
> > >*  vdev: the virtio_device
> > >*  Returns the config generation counter
> > >* @get_status: read the status byte
> > > @@ -48,17 +48,17 @@ struct irq_affinity;
> > >* @del_vqs: free virtqueues found by find_vqs().
> > >* @get_features: get the array of feature bits for this device.
> > >*  vdev: the virtio_device
> > > - *   Returns the first 32 feature bits (all we currently need).
> > > + *   Returns the first 64 feature bits (all we currently need).
> > >* @finalize_features: confirm what device features we'll be using.
> > >*  vdev: the virtio_device
> > >*  This gives the final feature bits for the device: it can change
> > >*  the dev->feature bits if it wants.
> > >*  Returns 0 on success or error status
> > > - * @bus_name: return the bus name associated with the device
> > > + * @bus_name: return the bus name associated with the device (optional)
> > >*  vdev: the virtio_device
> > >*  This returns a pointer to the bus name a la pci_name from which
> > >*  the caller can then copy.
> > > - * @set_vq_affinity: set the affinity for a virtqueue.
> > > + * @set_vq_affinity: set the affinity for a virtqueue (optional).
> > >* @get_vq_affinity: get the affinity for a virtqueue (optional).
> > >*/
> > >   typedef void vq_callback_t(struct virtqueue *);
> > Ping. Any feedback on that patch?
> 
> Reviewed-by: Wei Wang 
> 
> Best,
> Wei

Pushed already, can't add tags.
Sorry about that.

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


  1   2   >