Re: [PATCH v6 00/27] x86: PIE support and option to extend KASLR randomization

2019-01-31 Thread Konrad Rzeszutek Wilk
On Thu, Jan 31, 2019 at 11:24:07AM -0800, Thomas Garnier wrote:
> There has been no major concern in the latest iterations. I am interested on
> what would be the best way to slowly integrate this patchset upstream.

One question that I was somehow expected in this cover letter - what
about all those lovely speculative bugs? As in say some one hasn't
updated their machine with the Spectre v3a microcode - wouldn't they
be able to get the kernel virtual address space?

In effect rendering all this hard-work not needed?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames

2019-01-31 Thread Michael S. Tsirkin
On Thu, Jan 31, 2019 at 08:40:30PM +0900, Toshiaki Makita wrote:
> Previously virtnet_xdp_xmit() did not account for device tx counters,
> which caused confusions.
> To be consistent with SKBs, account them on freeing xdp_frames.
> 
> Reported-by: David Ahern 
> Signed-off-by: Toshiaki Makita 

Well we count them on receive so I guess it makes sense for consistency

Acked-by: Michael S. Tsirkin 

however, I really wonder whether adding more and more standard net stack
things like this will end up costing most of XDP its speed.

Should we instead make sure *not* to account XDP packets
in any counters at all? XDP programs can use maps
to do their own counting...


> ---
>  drivers/net/virtio_net.c | 20 
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 2594481..4cfceb7 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -503,6 +503,8 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   struct bpf_prog *xdp_prog;
>   struct send_queue *sq;
>   unsigned int len;
> + int packets = 0;
> + int bytes = 0;
>   int drops = 0;
>   int kicks = 0;
>   int ret, err;
> @@ -526,10 +528,18 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  
>   /* Free up any pending old buffers before queueing new ones. */
>   while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
> - if (likely(is_xdp_frame(ptr)))
> - xdp_return_frame(ptr_to_xdp(ptr));
> - else
> - napi_consume_skb(ptr, false);
> + if (likely(is_xdp_frame(ptr))) {
> + struct xdp_frame *frame = ptr_to_xdp(ptr);
> +
> + bytes += frame->len;
> + xdp_return_frame(frame);
> + } else {
> + struct sk_buff *skb = ptr;
> +
> + bytes += skb->len;
> + napi_consume_skb(skb, false);
> + }
> + packets++;
>   }
>  
>   for (i = 0; i < n; i++) {
> @@ -549,6 +559,8 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>   }
>  out:
>   u64_stats_update_begin(&sq->stats.syncp);
> + sq->stats.bytes += bytes;
> + sq->stats.packets += packets;
>   sq->stats.xdp_tx += n;
>   sq->stats.xdp_tx_drops += drops;
>   sq->stats.kicks += kicks;
> -- 
> 1.8.3.1
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC] virtio: hint if callbacks surprisingly might sleep

2019-01-31 Thread Cornelia Huck
On Thu, 31 Jan 2019 10:27:53 -0500
"Michael S. Tsirkin"  wrote:

> On Thu, Jan 31, 2019 at 01:53:14PM +0100, Cornelia Huck wrote:
> > A virtio transport is free to implement some of the callbacks in
> > virtio_config_ops in a matter that they cannot be called from
> > atomic context (e.g. virtio-ccw, which maps a lot of the callbacks
> > to channel I/O, which is an inherently asynchronous mechanism).
> > This can be very surprising for developers using the much more
> > common virtio-pci transport, just to find out that things break
> > when used on s390.
> > 
> > The documentation for virtio_config_ops now contains a comment
> > explaining this, but it makes sense to add a might_sleep() annotation
> > to various wrapper functions in the virtio core to avoid surprises
> > later.
> > 
> > Note that annotations are NOT added to two classes of calls:
> > - direct calls from device drivers (all current callers should be
> >   fine, however)
> > - calls which clearly won't be made from atomic context (such as
> >   those ultimately coming in via the driver core)
> > 
> > Signed-off-by: Cornelia Huck   
> 
> 
> Makes sense to me. I don't think we should push our luck in
> this release though, better defer until the merge window.

Nod, that's definitely something for the next release.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] dma: Uninline dma_max_mapping_size()

2019-01-31 Thread Michael S. Tsirkin
On Thu, Jan 31, 2019 at 03:37:23PM +0100, Christoph Hellwig wrote:
> On Thu, Jan 31, 2019 at 02:01:27PM +0100, Joerg Roedel wrote:
> > On Thu, Jan 31, 2019 at 11:41:29AM +0100, Christoph Hellwig wrote:
> > > Sorry for not noticing last time, but since 5.0 we keep all non-fast
> > > path DMA mapping interfaces out of line, so this should move to
> > > kernel/dma/mapping.c.
> > 
> > Okay, attached patch does that. It applies on-top of this patch-set.
> > 
> > Michael, feel free to either apply this on-top of the patch-set or merge
> > the diff into patch 3, whatever you prefer.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig 
> 
> I'd prefer it to be squashed if possible.

OK. Joerg can you repost the series with this squashed
and all acks applied?

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


Re: [PATCH] dma: Uninline dma_max_mapping_size()

2019-01-31 Thread Christoph Hellwig
On Thu, Jan 31, 2019 at 02:01:27PM +0100, Joerg Roedel wrote:
> On Thu, Jan 31, 2019 at 11:41:29AM +0100, Christoph Hellwig wrote:
> > Sorry for not noticing last time, but since 5.0 we keep all non-fast
> > path DMA mapping interfaces out of line, so this should move to
> > kernel/dma/mapping.c.
> 
> Okay, attached patch does that. It applies on-top of this patch-set.
> 
> Michael, feel free to either apply this on-top of the patch-set or merge
> the diff into patch 3, whatever you prefer.

Looks good:

Reviewed-by: Christoph Hellwig 

I'd prefer it to be squashed if possible.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC] virtio: hint if callbacks surprisingly might sleep

2019-01-31 Thread Michael S. Tsirkin
On Thu, Jan 31, 2019 at 01:53:14PM +0100, Cornelia Huck wrote:
> A virtio transport is free to implement some of the callbacks in
> virtio_config_ops in a matter that they cannot be called from
> atomic context (e.g. virtio-ccw, which maps a lot of the callbacks
> to channel I/O, which is an inherently asynchronous mechanism).
> This can be very surprising for developers using the much more
> common virtio-pci transport, just to find out that things break
> when used on s390.
> 
> The documentation for virtio_config_ops now contains a comment
> explaining this, but it makes sense to add a might_sleep() annotation
> to various wrapper functions in the virtio core to avoid surprises
> later.
> 
> Note that annotations are NOT added to two classes of calls:
> - direct calls from device drivers (all current callers should be
>   fine, however)
> - calls which clearly won't be made from atomic context (such as
>   those ultimately coming in via the driver core)
> 
> Signed-off-by: Cornelia Huck 


Makes sense to me. I don't think we should push our luck in
this release though, better defer until the merge window.

> ---
> 
> I think it is safe to add this now that the issues with the balloon
> have been fixed.
> 
> Note that this is not bulletproof (nor is it inteded to be). The
> intention is to make it easier for people to catch problems earlier.
> 
> ---
>  drivers/virtio/virtio.c   |  2 ++
>  include/linux/virtio_config.h | 13 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 59e36ef4920f..98b30f54342c 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -161,6 +161,7 @@ EXPORT_SYMBOL_GPL(virtio_config_enable);
>  
>  void virtio_add_status(struct virtio_device *dev, unsigned int status)
>  {
> + might_sleep();
>   dev->config->set_status(dev, dev->config->get_status(dev) | status);
>  }
>  EXPORT_SYMBOL_GPL(virtio_add_status);
> @@ -170,6 +171,7 @@ int virtio_finalize_features(struct virtio_device *dev)
>   int ret = dev->config->finalize_features(dev);
>   unsigned status;
>  
> + might_sleep();
>   if (ret)
>   return ret;
>  
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 987b6491b946..bb4cc4910750 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -290,6 +290,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
> virtio_device *vdev, u64 val)
>  /* Config space accessors. */
>  #define virtio_cread(vdev, structname, member, ptr)  \
>   do {\
> + might_sleep();  \
>   /* Must match the member's type, and be integer */  \
>   if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
>   (*ptr) = 1; \
> @@ -319,6 +320,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
> virtio_device *vdev, u64 val)
>  /* Config space accessors. */
>  #define virtio_cwrite(vdev, structname, member, ptr) \
>   do {\
> + might_sleep();  \
>   /* Must match the member's type, and be integer */  \
>   if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
>   BUG_ON((*ptr) == 1);\
> @@ -358,6 +360,7 @@ static inline void __virtio_cread_many(struct 
> virtio_device *vdev,
>   vdev->config->generation(vdev) : 0;
>   int i;
>  
> + might_sleep();
>   do {
>   old = gen;
>  
> @@ -380,6 +383,8 @@ static inline void virtio_cread_bytes(struct 
> virtio_device *vdev,
>  static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int 
> offset)
>  {
>   u8 ret;
> +
> + might_sleep();
>   vdev->config->get(vdev, offset, &ret, sizeof(ret));
>   return ret;
>  }
> @@ -387,6 +392,7 @@ static inline u8 virtio_cread8(struct virtio_device 
> *vdev, unsigned int offset)
>  static inline void virtio_cwrite8(struct virtio_device *vdev,
> unsigned int offset, u8 val)
>  {
> + might_sleep();
>   vdev->config->set(vdev, offset, &val, sizeof(val));
>  }
>  
> @@ -394,6 +400,8 @@ static inline u16 virtio_cread16(struct virtio_device 
> *vdev,
>unsigned int offset)
>  {
>   u16 ret;
> +
> + might_sleep();
>   vdev->config->get(vdev, offset, &ret, sizeof(ret));
>   return virtio16_to_cpu(vdev, (__force __virtio16)ret);
>  }
> @@ -401,6 +409,7 @@ static inline u16 virtio_cread16(struct virtio_device 
> *vdev,
>  static inline void virtio_cwrite16(struct virtio_device *vdev,
>  unsigned int offset, u16 val)
>  {
> + might_sleep();

[PATCH] dma: Uninline dma_max_mapping_size()

2019-01-31 Thread Joerg Roedel
On Thu, Jan 31, 2019 at 11:41:29AM +0100, Christoph Hellwig wrote:
> Sorry for not noticing last time, but since 5.0 we keep all non-fast
> path DMA mapping interfaces out of line, so this should move to
> kernel/dma/mapping.c.

Okay, attached patch does that. It applies on-top of this patch-set.

Michael, feel free to either apply this on-top of the patch-set or merge
the diff into patch 3, whatever you prefer.

>From 2bb95d2136280c79de9553852ee3370f6d42d7b3 Mon Sep 17 00:00:00 2001
From: Joerg Roedel 
Date: Thu, 31 Jan 2019 13:55:27 +0100
Subject: [PATCH] dma: Uninline dma_max_mapping_size()

The function is not performance sensitive and doesn't need
to be inlined at every call-site. Move it out of the header
into the appropriate C source file.

Signed-off-by: Joerg Roedel 
---
 include/linux/dma-mapping.h | 18 +-
 kernel/dma/direct.c |  1 -
 kernel/dma/mapping.c| 14 ++
 3 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index a3ca8a71a704..5b21f14802e1 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -443,19 +443,6 @@ 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,
@@ -476,6 +463,7 @@ int dma_supported(struct device *dev, u64 mask);
 int dma_set_mask(struct device *dev, u64 mask);
 int dma_set_coherent_mask(struct device *dev, u64 mask);
 u64 dma_get_required_mask(struct device *dev);
+size_t dma_max_mapping_size(struct device *dev);
 #else /* CONFIG_HAS_DMA */
 static inline dma_addr_t dma_map_page_attrs(struct device *dev,
struct page *page, size_t offset, size_t size,
@@ -577,6 +565,10 @@ static inline u64 dma_get_required_mask(struct device *dev)
 {
return 0;
 }
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+   return 0;
+}
 #endif /* CONFIG_HAS_DMA */
 
 static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 81ca8170b928..6310ad01f915 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -391,4 +391,3 @@ size_t dma_direct_max_mapping_size(struct device *dev)
 
return size;
 }
-EXPORT_SYMBOL(dma_direct_max_mapping_size);
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index a11006b6d8e8..5753008ab286 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -357,3 +357,17 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
size_t size,
ops->cache_sync(dev, vaddr, size, dir);
 }
 EXPORT_SYMBOL(dma_cache_sync);
+
+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;
+}
+EXPORT_SYMBOL_GPL(dma_max_mapping_size);
-- 
2.16.4

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


Re: [PATCH v2 3/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource()

2019-01-31 Thread Noralf Trønnes


Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Add format, width and height fields to the virtio_gpu_object_params
> struct.  With that in place we can use the parameter struct for
> virtio_gpu_cmd_create_resource() calls too.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

Acked-by: Noralf Trønnes 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 net 0/7] virtio_net: Fix problems around XDP tx and napi_tx

2019-01-31 Thread David Miller
From: Toshiaki Makita 
Date: Tue, 29 Jan 2019 09:45:52 +0900

> While I'm looking into how to account standard tx counters on XDP tx
> processing, I found several bugs around XDP tx and napi_tx.
> 
> Patch1: Fix oops on error path. Patch2 depends on this.
> Patch2: Fix memory corruption on freeing xdp_frames with napi_tx enabled.
> Patch3: Minor fix patch5 depends on.
> Patch4: Fix memory corruption on processing xdp_frames when XDP is disabled.
>   Also patch5 depends on this.
> Patch5: Fix memory corruption on processing xdp_frames while XDP is being
>   disabled.
> Patch6: Minor fix patch7 depends on.
> Patch7: Fix memory corruption on freeing sk_buff or xdp_frames when a normal
>   queue is reused for XDP and vise versa.
> 
> v2:
> - patch5: Make rcu_assign_pointer/synchronize_net conditional instead of
>   _virtnet_set_queues.
> - patch7: Use napi_consume_skb() instead of dev_consume_skb_any()
> 
> Signed-off-by: Toshiaki Makita 

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


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

2019-01-31 Thread Christoph Hellwig
> +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;
> +}

Sorry for not noticing last time, but since 5.0 we keep all non-fast
path DMA mapping interfaces out of line, so this should move to
kernel/dma/mapping.c.

> +EXPORT_SYMBOL(dma_direct_max_mapping_size);

And then there is no need to export this one.

The dma_max_mapping_size export should be EXPORT_SYMBOL_GPL like all
new dma-mapping interfaces.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v6 00/27] x86: PIE support and option to extend KASLR randomization

2019-01-31 Thread Kees Cook
On Fri, Feb 1, 2019 at 8:28 AM Thomas Garnier  wrote:
> These patches make the changes necessary to build the kernel as Position
> Independent Executable (PIE) on x86_64. A PIE kernel can be relocated below
> the top 2G of the virtual address space. It allows to optionally extend the
> KASLR randomization range from 1G to 3G. The chosen range is the one currently
> available, future changes will allow the kernel module to have a wider
> randomization range.

This also lays the groundwork for doing compilation-unit-granularity
KASLR, as Kristen has been working on. With PIE working, the
relocations are more sane and boot-time reordering becomes possible
(or at least, it becomes the same logically as doing the work on
modules, etc).

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


Re: [PATCH v2 1/6] drm/virtio: move virtio_gpu_object_{attach, detach} calls.

2019-01-31 Thread Noralf Trønnes


Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Drop the dummy ttm backend implementation, add a real one for
> TTM_PL_FLAG_TT objects.  The bin/unbind callbacks will call
> virtio_gpu_object_{attach,detach}, to update the object state
> on the host side, instead of invoking those calls from the
> move_notify() callback.
> 
> With that in place the move and move_notify callbacks are not
> needed any more, so drop them.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

Acked-by: Noralf Trønnes 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames

2019-01-31 Thread Toshiaki Makita
Previously virtnet_xdp_xmit() did not account for device tx counters,
which caused confusions.
To be consistent with SKBs, account them on freeing xdp_frames.

Reported-by: David Ahern 
Signed-off-by: Toshiaki Makita 
---
 drivers/net/virtio_net.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 2594481..4cfceb7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -503,6 +503,8 @@ static int virtnet_xdp_xmit(struct net_device *dev,
struct bpf_prog *xdp_prog;
struct send_queue *sq;
unsigned int len;
+   int packets = 0;
+   int bytes = 0;
int drops = 0;
int kicks = 0;
int ret, err;
@@ -526,10 +528,18 @@ static int virtnet_xdp_xmit(struct net_device *dev,
 
/* Free up any pending old buffers before queueing new ones. */
while ((ptr = virtqueue_get_buf(sq->vq, &len)) != NULL) {
-   if (likely(is_xdp_frame(ptr)))
-   xdp_return_frame(ptr_to_xdp(ptr));
-   else
-   napi_consume_skb(ptr, false);
+   if (likely(is_xdp_frame(ptr))) {
+   struct xdp_frame *frame = ptr_to_xdp(ptr);
+
+   bytes += frame->len;
+   xdp_return_frame(frame);
+   } else {
+   struct sk_buff *skb = ptr;
+
+   bytes += skb->len;
+   napi_consume_skb(skb, false);
+   }
+   packets++;
}
 
for (i = 0; i < n; i++) {
@@ -549,6 +559,8 @@ static int virtnet_xdp_xmit(struct net_device *dev,
}
 out:
u64_stats_update_begin(&sq->stats.syncp);
+   sq->stats.bytes += bytes;
+   sq->stats.packets += packets;
sq->stats.xdp_tx += n;
sq->stats.xdp_tx_drops += drops;
sq->stats.kicks += kicks;
-- 
1.8.3.1


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


Re: [PATCH v2 4/6] drm/virtio: params struct for virtio_gpu_cmd_create_resource_3d()

2019-01-31 Thread Noralf Trønnes



Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Add 3d resource parameters to virtio_gpu_object_params struct.  With
> that in place we can use it for virtio_gpu_cmd_resource_create_3d()
> calls.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

You don't remove the struct virtio_gpu_resource_create_3d definition,
but it looks like there's no users left?

Noralf.

>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 10 +-
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 25 ++---
>  drivers/gpu/drm/virtio/virtgpu_vq.c| 16 +---
>  3 files changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index a40215c10e..3265e62725 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -56,6 +56,14 @@ struct virtio_gpu_object_params {
>   uint32_t height;
>   unsigned long size;
>   bool pinned;
> + /* 3d */
> + uint32_t target;
> + uint32_t bind;
> + uint32_t depth;
> + uint32_t array_size;
> + uint32_t last_level;
> + uint32_t nr_samples;
> + uint32_t flags;
>  };
>  
>  struct virtio_gpu_object {
> @@ -310,7 +318,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct 
> virtio_gpu_device *vgdev,
>  void
>  virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object *bo,
> -   struct virtio_gpu_resource_create_3d *rc_3d);
> +   struct virtio_gpu_object_params *params);
>  void virtio_gpu_ctrl_ack(struct virtqueue *vq);
>  void virtio_gpu_cursor_ack(struct virtqueue *vq);
>  void virtio_gpu_fence_ack(struct virtqueue *vq);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c 
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 84c2216fd4..431e5d767e 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -283,7 +283,6 @@ static int virtio_gpu_resource_create_ioctl(struct 
> drm_device *dev, void *data,
>   struct ttm_validate_buffer mainbuf;
>   struct virtio_gpu_fence *fence = NULL;
>   struct ww_acquire_ctx ticket;
> - struct virtio_gpu_resource_create_3d rc_3d;
>   struct virtio_gpu_object_params params = { 0 };
>  
>   if (vgdev->has_virgl_3d == false) {
> @@ -307,7 +306,15 @@ static int virtio_gpu_resource_create_ioctl(struct 
> drm_device *dev, void *data,
>   params.width = rc->width;
>   params.height = rc->height;
>   params.size = rc->size;
> -
> + if (vgdev->has_virgl_3d) {
> + params.target = rc->target;
> + params.bind = rc->bind;
> + params.depth = rc->depth;
> + params.array_size = rc->array_size;
> + params.last_level = rc->last_level;
> + params.nr_samples = rc->nr_samples;
> + params.flags = rc->flags;
> + }
>   /* allocate a single page size object */
>   if (params.size == 0)
>   params.size = PAGE_SIZE;
> @@ -333,25 +340,13 @@ static int virtio_gpu_resource_create_ioctl(struct 
> drm_device *dev, void *data,
>   goto fail_unref;
>   }
>  
> - rc_3d.resource_id = cpu_to_le32(qobj->hw_res_handle);
> - rc_3d.target = cpu_to_le32(rc->target);
> - rc_3d.format = cpu_to_le32(rc->format);
> - rc_3d.bind = cpu_to_le32(rc->bind);
> - rc_3d.width = cpu_to_le32(rc->width);
> - rc_3d.height = cpu_to_le32(rc->height);
> - rc_3d.depth = cpu_to_le32(rc->depth);
> - rc_3d.array_size = cpu_to_le32(rc->array_size);
> - rc_3d.last_level = cpu_to_le32(rc->last_level);
> - rc_3d.nr_samples = cpu_to_le32(rc->nr_samples);
> - rc_3d.flags = cpu_to_le32(rc->flags);
> -
>   fence = virtio_gpu_fence_alloc(vgdev);
>   if (!fence) {
>   ret = -ENOMEM;
>   goto fail_backoff;
>   }
>  
> - virtio_gpu_cmd_resource_create_3d(vgdev, qobj, &rc_3d);
> + virtio_gpu_cmd_resource_create_3d(vgdev, qobj, ¶ms);
>   ret = virtio_gpu_object_attach(vgdev, qobj, fence);
>   if (ret) {
>   dma_fence_put(&fence->f);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
> b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 363b8b8577..ca93ec6ca3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -826,7 +826,7 @@ void virtio_gpu_cmd_context_detach_resource(struct 
> virtio_gpu_device *vgdev,
>  void
>  virtio_gpu_cmd_resource_create_3d(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object *bo,
> -   struct virtio_gpu_resource_create_3d *rc_3d)
> +   struct virtio_gpu_object_params *params)
>  {
>   st

Re: [PATCH v2 5/6] drm/virtio: drop fencing in virtio_gpu_resource_create_ioctl

2019-01-31 Thread Noralf Trønnes


Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> There is no need to wait for completion here.
> 
> The host will process commands in submit order, so commands can
> reference the new resource just fine even when queued up before
> completion.
> 
> On the guest side there is no need to wait for completion too.  Which
> btw is different from resource destroy, where we have to make sure the
> host has seen the destroy and thus doesn't use it any more before
> releasing the pages backing the resource.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

Acked-by: Noralf Trønnes 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net] vhost: fix OOB in get_rx_bufs()

2019-01-31 Thread David Miller
From: "Michael S. Tsirkin" 
Date: Tue, 29 Jan 2019 20:36:31 -0500

> If it helps I can include most virtio stuff in my pull requests instead.
> Or if that can't work since there's too often a dependency on net-next,
> maybe Jason wants to create a tree and send pull requests to you.  Let
> us know if that will help, and which of the options looks better from
> your POV.

Thanks for offering Michael, I really appreciate it.

Let me think about the logistics of that and how it may or may not
help me with my backlog.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH RFC] virtio: hint if callbacks surprisingly might sleep

2019-01-31 Thread Cornelia Huck
A virtio transport is free to implement some of the callbacks in
virtio_config_ops in a matter that they cannot be called from
atomic context (e.g. virtio-ccw, which maps a lot of the callbacks
to channel I/O, which is an inherently asynchronous mechanism).
This can be very surprising for developers using the much more
common virtio-pci transport, just to find out that things break
when used on s390.

The documentation for virtio_config_ops now contains a comment
explaining this, but it makes sense to add a might_sleep() annotation
to various wrapper functions in the virtio core to avoid surprises
later.

Note that annotations are NOT added to two classes of calls:
- direct calls from device drivers (all current callers should be
  fine, however)
- calls which clearly won't be made from atomic context (such as
  those ultimately coming in via the driver core)

Signed-off-by: Cornelia Huck 
---

I think it is safe to add this now that the issues with the balloon
have been fixed.

Note that this is not bulletproof (nor is it inteded to be). The
intention is to make it easier for people to catch problems earlier.

---
 drivers/virtio/virtio.c   |  2 ++
 include/linux/virtio_config.h | 13 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 59e36ef4920f..98b30f54342c 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -161,6 +161,7 @@ EXPORT_SYMBOL_GPL(virtio_config_enable);
 
 void virtio_add_status(struct virtio_device *dev, unsigned int status)
 {
+   might_sleep();
dev->config->set_status(dev, dev->config->get_status(dev) | status);
 }
 EXPORT_SYMBOL_GPL(virtio_add_status);
@@ -170,6 +171,7 @@ int virtio_finalize_features(struct virtio_device *dev)
int ret = dev->config->finalize_features(dev);
unsigned status;
 
+   might_sleep();
if (ret)
return ret;
 
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 987b6491b946..bb4cc4910750 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -290,6 +290,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
 /* Config space accessors. */
 #define virtio_cread(vdev, structname, member, ptr)\
do {\
+   might_sleep();  \
/* Must match the member's type, and be integer */  \
if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
(*ptr) = 1; \
@@ -319,6 +320,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
 /* Config space accessors. */
 #define virtio_cwrite(vdev, structname, member, ptr)   \
do {\
+   might_sleep();  \
/* Must match the member's type, and be integer */  \
if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
BUG_ON((*ptr) == 1);\
@@ -358,6 +360,7 @@ static inline void __virtio_cread_many(struct virtio_device 
*vdev,
vdev->config->generation(vdev) : 0;
int i;
 
+   might_sleep();
do {
old = gen;
 
@@ -380,6 +383,8 @@ static inline void virtio_cread_bytes(struct virtio_device 
*vdev,
 static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int offset)
 {
u8 ret;
+
+   might_sleep();
vdev->config->get(vdev, offset, &ret, sizeof(ret));
return ret;
 }
@@ -387,6 +392,7 @@ static inline u8 virtio_cread8(struct virtio_device *vdev, 
unsigned int offset)
 static inline void virtio_cwrite8(struct virtio_device *vdev,
  unsigned int offset, u8 val)
 {
+   might_sleep();
vdev->config->set(vdev, offset, &val, sizeof(val));
 }
 
@@ -394,6 +400,8 @@ static inline u16 virtio_cread16(struct virtio_device *vdev,
 unsigned int offset)
 {
u16 ret;
+
+   might_sleep();
vdev->config->get(vdev, offset, &ret, sizeof(ret));
return virtio16_to_cpu(vdev, (__force __virtio16)ret);
 }
@@ -401,6 +409,7 @@ static inline u16 virtio_cread16(struct virtio_device *vdev,
 static inline void virtio_cwrite16(struct virtio_device *vdev,
   unsigned int offset, u16 val)
 {
+   might_sleep();
val = (__force u16)cpu_to_virtio16(vdev, val);
vdev->config->set(vdev, offset, &val, sizeof(val));
 }
@@ -409,6 +418,8 @@ static inline u32 virtio_cread32(struct virtio_device *vdev,
 unsigned int offset)
 {
u32 ret;
+
+   might_sleep();
vdev->config->get(vdev, offset, &ret, sizeof

Re: [PATCH v2 6/6] drm/virtio: move virtio_gpu_cmd_create_resource call into virtio_gpu_object_create

2019-01-31 Thread Noralf Trønnes


Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Specifically call virtio_gpu_object_create() before ttm_bo_init(), so
> the object is already created when ttm calls the
> virtio_gpu_ttm_tt_bind() callback (which in turn calls
> virtio_gpu_object_attach()).
> 
> With that in place virtio_gpu_object_attach() will never be called with
> an object which is not yet created, so the extra
> virtio_gpu_object_attach() calls done after
> virtio_gpu_cmd_create_resource() is not needed any more.
> 
> Signed-off-by: Gerd Hoffmann 
> ---

Acked-by: Noralf Trønnes 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/6] drm/virtio: use struct to pass params to virtio_gpu_object_create()

2019-01-31 Thread Noralf Trønnes


Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Create virtio_gpu_object_params, use that to pass object parameters to
> virtio_gpu_object_create.  This is just the first step, followup patches
> will add more parameters to the struct.  The plan is to use the struct
> for all object parameters.
> 
> Also drop unused "kernel" parameter for virtio_gpu_alloc_object(), it is
> unused and always false.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h| 15 ++-
>  drivers/gpu/drm/virtio/virtgpu_gem.c| 17 ++---
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c  | 11 ++-
>  drivers/gpu/drm/virtio/virtgpu_object.c | 22 +-
>  4 files changed, 35 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
> b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index d577cb76f5..40928980a2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -50,6 +50,11 @@
>  #define DRIVER_MINOR 1
>  #define DRIVER_PATCHLEVEL 0
>  
> +struct virtio_gpu_object_params {
> + unsigned long size;
> + bool pinned;
> +};
> +
>  struct virtio_gpu_object {
>   struct drm_gem_object gem_base;
>   uint32_t hw_res_handle;
> @@ -217,16 +222,16 @@ int virtio_gpu_gem_init(struct virtio_gpu_device 
> *vgdev);
>  void virtio_gpu_gem_fini(struct virtio_gpu_device *vgdev);
>  int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> -   uint64_t size,
> +   struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p);
>  int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
>  struct drm_file *file);
>  void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
>struct drm_file *file);
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -   size_t size, bool kernel,
> -   bool pinned);
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params);
>  int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>   struct drm_device *dev,
>   struct drm_mode_create_dumb *args);
> @@ -342,7 +347,7 @@ void virtio_gpu_fence_event_process(struct 
> virtio_gpu_device *vdev,
>  
>  /* virtio_gpu_object */
>  int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> -  unsigned long size, bool kernel, bool pinned,
> +  struct virtio_gpu_object_params *params,
>struct virtio_gpu_object **bo_ptr);
>  void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo);
>  int virtio_gpu_object_kmap(struct virtio_gpu_object *bo);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c 
> b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index f065863939..b5f2d94ce5 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -34,15 +34,15 @@ void virtio_gpu_gem_free_object(struct drm_gem_object 
> *gem_obj)
>   virtio_gpu_object_unref(&obj);
>  }
>  
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> -   size_t size, bool kernel,
> -   bool pinned)
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params)
>  {
>   struct virtio_gpu_device *vgdev = dev->dev_private;
>   struct virtio_gpu_object *obj;
>   int ret;
>  
> - ret = virtio_gpu_object_create(vgdev, size, kernel, pinned, &obj);
> + ret = virtio_gpu_object_create(vgdev, params, &obj);
>   if (ret)
>   return ERR_PTR(ret);
>  
> @@ -51,7 +51,7 @@ struct virtio_gpu_object *virtio_gpu_alloc_object(struct 
> drm_device *dev,
>  
>  int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> -   uint64_t size,
> +   struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p)
>  {
> @@ -59,7 +59,7 @@ int virtio_gpu_gem_create(struct drm_file *file,
>   int ret;
>   u32 handle;
>  
> - obj = virtio_gpu_alloc_object(dev, size, false, false);
> + obj = virtio_gpu_alloc_object(dev, params);
>   if (IS_ERR(obj))
>   return PTR_ERR(obj);
>  
> @@ -85,6 +85,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
>   struct virtio_gpu_device *vgdev = dev->dev_private;
>   struct drm_gem_object *gobj;
>   struct virtio_gpu_obje

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

2019-01-31 Thread Michael S. Tsirkin
On Wed, Jan 30, 2019 at 01:35:13PM -0500, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 30, 2019 at 05:40:02PM +0100, Joerg Roedel wrote:
> > Hi,
> > 
> > here is the next version of this patch-set. Previous
> > versions can be found here:
> > 
> > V1: https://lore.kernel.org/lkml/20190110134433.15672-1-j...@8bytes.org/
> > 
> > V2: https://lore.kernel.org/lkml/20190115132257.6426-1-j...@8bytes.org/
> > 
> > V3: https://lore.kernel.org/lkml/20190123163049.24863-1-j...@8bytes.org/
> > 
> > V4: https://lore.kernel.org/lkml/20190129084342.26030-1-j...@8bytes.org/
> > 
> > The problem solved here 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.
> > 
> > Changes to v4 are:
> > 
> > - Added Reviewed-by tags from Christoph
> > 
> > - Added missing EXPORT_SYMBOL(_GPL) lines
> > 
> > Please review.
> 
> I can put it in my tree and send it to Linus .. unless folks want
> to do it through a different tree?

I queued it in my tree as it seems virtio specific.

> 
> > 
> > Thanks,
> > 
> > Joerg
> > Joerg Roedel (5):
> >   swiotlb: Introduce swiotlb_max_mapping_size()
> >   swiotlb: Add is_swiotlb_active() function
> >   dma: Introduce dma_max_mapping_size()
> >   virtio: Introduce virtio_max_dma_size()
> >   virtio-blk: Consider virtio_max_dma_size() for maximum segment size
> > 
> >  Documentation/DMA-API.txt|  8 
> >  drivers/block/virtio_blk.c   | 10 ++
> >  drivers/virtio/virtio_ring.c | 11 +++
> >  include/linux/dma-mapping.h  | 16 
> >  include/linux/swiotlb.h  | 11 +++
> >  include/linux/virtio.h   |  2 ++
> >  kernel/dma/direct.c  | 12 
> >  kernel/dma/swiotlb.c | 14 ++
> >  8 files changed, 80 insertions(+), 4 deletions(-)
> > 
> > -- 
> > 2.17.1
> > 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames

2019-01-31 Thread Jesper Dangaard Brouer
On Thu, 31 Jan 2019 09:45:23 -0800 (PST)
David Miller  wrote:

> From: "Michael S. Tsirkin" 
> Date: Thu, 31 Jan 2019 10:25:17 -0500
> 
> > On Thu, Jan 31, 2019 at 08:40:30PM +0900, Toshiaki Makita wrote:  
> >> Previously virtnet_xdp_xmit() did not account for device tx counters,
> >> which caused confusions.
> >> To be consistent with SKBs, account them on freeing xdp_frames.
> >> 
> >> Reported-by: David Ahern 
> >> Signed-off-by: Toshiaki Makita   
> > 
> > Well we count them on receive so I guess it makes sense for consistency
> > 
> > Acked-by: Michael S. Tsirkin 
> > 
> > however, I really wonder whether adding more and more standard net stack
> > things like this will end up costing most of XDP its speed.
> > 
> > Should we instead make sure *not* to account XDP packets
> > in any counters at all? XDP programs can use maps
> > to do their own counting...  
> 
> This has been definitely a discussion point, and something we should
> develop a clear, strong, policy on.
> 
> David, Jesper, care to chime in where we ended up in that last thread
> discussion this?

IHMO packets RX and TX on a device need to be accounted, in standard
counters, regardless of XDP.  For XDP RX the packet is counted as RX,
regardless if XDP choose to XDP_DROP.  On XDP TX which is via
XDP_REDIRECT or XDP_TX, the driver that transmit the packet need to
account the packet in a TX counter (this if often delayed to DMA TX
completion handling).  We cannot break the expectation that RX and TX
counter are visible to userspace stats tools. XDP should not make these
packets invisible.

Performance wise, I don't see an issue. As updating these counters
(packets and bytes) can be done as a bulk, either when driver NAPI RX
func ends, or in TX DMA completion, like most drivers already do today.
Further more, most drivers save this in per RX ring data-area, which
are only summed when userspace read these.


A separate question (and project) raised by David Ahern, was if we
should have more detailed stats on the different XDP action return
codes, as an easy means for sysadms to diagnose running XDP programs.
That is something that require more discussions, as it can impact
performance, and likely need to be opt-in.  My opinion is yes we should
do this for the sake of better User eXperience, BUT *only* if we can find
a technical solution that does not hurt performance.

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


Re: [PATCH net] virtio_net: Account for tx bytes and packets on sending xdp_frames

2019-01-31 Thread David Miller
From: "Michael S. Tsirkin" 
Date: Thu, 31 Jan 2019 10:25:17 -0500

> On Thu, Jan 31, 2019 at 08:40:30PM +0900, Toshiaki Makita wrote:
>> Previously virtnet_xdp_xmit() did not account for device tx counters,
>> which caused confusions.
>> To be consistent with SKBs, account them on freeing xdp_frames.
>> 
>> Reported-by: David Ahern 
>> Signed-off-by: Toshiaki Makita 
> 
> Well we count them on receive so I guess it makes sense for consistency
> 
> Acked-by: Michael S. Tsirkin 
> 
> however, I really wonder whether adding more and more standard net stack
> things like this will end up costing most of XDP its speed.
> 
> Should we instead make sure *not* to account XDP packets
> in any counters at all? XDP programs can use maps
> to do their own counting...

This has been definitely a discussion point, and something we should
develop a clear, strong, policy on.

David, Jesper, care to chime in where we ended up in that last thread
discussion this?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] dma: Uninline dma_max_mapping_size()

2019-01-31 Thread Joerg Roedel
On Thu, Jan 31, 2019 at 09:43:51AM -0500, Michael S. Tsirkin wrote:
> OK. Joerg can you repost the series with this squashed
> and all acks applied?

Sure, sent out now as v6.

Regards,

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


[PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size

2019-01-31 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.

Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Christoph Hellwig 
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..4bc083b7c9b5 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 = virtio_max_dma_size(vdev);
+
/* 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, &v);
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 3/5] dma: Introduce dma_max_mapping_size()

2019-01-31 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.

Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Joerg Roedel 
---
 Documentation/DMA-API.txt   |  8 
 include/linux/dma-mapping.h |  8 
 kernel/dma/direct.c | 11 +++
 kernel/dma/mapping.c| 14 ++
 4 files changed, 41 insertions(+)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index e133ccd60228..acfe3d0f78d1 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -195,6 +195,14 @@ Requesting the required mask does not alter the current 
mask.  If you
 wish to take advantage of it, you should issue a dma_set_mask()
 call to set the mask to the value returned.
 
+::
+
+   size_t
+   dma_direct_max_mapping_size(struct device *dev);
+
+Returns the maximum size of a mapping for the device. The size parameter
+of the mapping functions like dma_map_single(), dma_map_page() and
+others should not be larger than the returned value.
 
 Part Id - Streaming DMA mappings
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f6ded992c183..5b21f14802e1 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 
 
@@ -460,6 +463,7 @@ int dma_supported(struct device *dev, u64 mask);
 int dma_set_mask(struct device *dev, u64 mask);
 int dma_set_coherent_mask(struct device *dev, u64 mask);
 u64 dma_get_required_mask(struct device *dev);
+size_t dma_max_mapping_size(struct device *dev);
 #else /* CONFIG_HAS_DMA */
 static inline dma_addr_t dma_map_page_attrs(struct device *dev,
struct page *page, size_t offset, size_t size,
@@ -561,6 +565,10 @@ static inline u64 dma_get_required_mask(struct device *dev)
 {
return 0;
 }
+static inline size_t dma_max_mapping_size(struct device *dev)
+{
+   return 0;
+}
 #endif /* CONFIG_HAS_DMA */
 
 static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 355d16acee6d..6310ad01f915 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -380,3 +380,14 @@ 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)
+{
+   size_t size = SIZE_MAX;
+
+   /* If SWIOTLB is active, use its maximum mapping size */
+   if (is_swiotlb_active())
+   size = swiotlb_max_mapping_size(dev);
+
+   return size;
+}
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index a11006b6d8e8..5753008ab286 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -357,3 +357,17 @@ void dma_cache_sync(struct device *dev, void *vaddr, 
size_t size,
ops->cache_sync(dev, vaddr, size, dir);
 }
 EXPORT_SYMBOL(dma_cache_sync);
+
+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;
+}
+EXPORT_SYMBOL_GPL(dma_max_mapping_size);
-- 
2.17.1

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


[PATCH 4/5] virtio: Introduce virtio_max_dma_size()

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

This function returns the maximum segment size for a single
dma transaction of a virtio device. The possible limit comes
from the SWIOTLB implementation in the Linux kernel, that
has an upper limit of (currently) 256kb of contiguous
memory it can map. Other DMA-API implementations might also
have limits.

Use the new dma_max_mapping_size() function to determine the
maximum mapping size when DMA-API is in use for virtio.

Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Joerg Roedel 
---
 drivers/virtio/virtio_ring.c | 11 +++
 include/linux/virtio.h   |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..8a31c6862b2b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -266,6 +266,17 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
return false;
 }
 
+size_t virtio_max_dma_size(struct virtio_device *vdev)
+{
+   size_t max_segment_size = SIZE_MAX;
+
+   if (vring_use_dma_api(vdev))
+   max_segment_size = dma_max_mapping_size(&vdev->dev);
+
+   return max_segment_size;
+}
+EXPORT_SYMBOL_GPL(virtio_max_dma_size);
+
 static void *vring_alloc_queue(struct virtio_device *vdev, size_t size,
  dma_addr_t *dma_handle, gfp_t flag)
 {
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index fa1b5da2804e..673fe3ef3607 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -157,6 +157,8 @@ int virtio_device_freeze(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);
 #endif
 
+size_t virtio_max_dma_size(struct virtio_device *vdev);
+
 #define virtio_device_for_each_vq(vdev, vq) \
list_for_each_entry(vq, &vdev->vqs, list)
 
-- 
2.17.1

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


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

2019-01-31 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.

Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Christoph Hellwig 
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..1c22d96e1742 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);
+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 1fb6fd68b9c7..9cb21259cb0b 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -662,3 +662,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 2/5] swiotlb: Add is_swiotlb_active() function

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

This function will be used from dma_direct code to determine
the maximum segment size of a dma mapping.

Reviewed-by: Konrad Rzeszutek Wilk 
Reviewed-by: Christoph Hellwig 
Signed-off-by: Joerg Roedel 
---
 include/linux/swiotlb.h | 6 ++
 kernel/dma/swiotlb.c| 9 +
 2 files changed, 15 insertions(+)

diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 1c22d96e1742..e9e786b4b598 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -63,6 +63,7 @@ extern void swiotlb_tbl_sync_single(struct device *hwdev,
 extern int
 swiotlb_dma_supported(struct device *hwdev, u64 mask);
 size_t swiotlb_max_mapping_size(struct device *dev);
+bool is_swiotlb_active(void);
 
 #ifdef CONFIG_SWIOTLB
 extern enum swiotlb_force swiotlb_force;
@@ -100,6 +101,11 @@ static inline size_t swiotlb_max_mapping_size(struct 
device *dev)
 {
return SIZE_MAX;
 }
+
+static inline bool is_swiotlb_active(void)
+{
+   return false;
+}
 #endif /* CONFIG_SWIOTLB */
 
 extern void swiotlb_print_info(void);
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 9cb21259cb0b..c873f9cc2146 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -667,3 +667,12 @@ size_t swiotlb_max_mapping_size(struct device *dev)
 {
return ((size_t)1 << IO_TLB_SHIFT) * IO_TLB_SEGSIZE;
 }
+
+bool is_swiotlb_active(void)
+{
+   /*
+* When SWIOTLB is initialized, even if io_tlb_start points to physical
+* address zero, io_tlb_end surely doesn't.
+*/
+   return io_tlb_end != 0;
+}
-- 
2.17.1

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


[PATCH 0/5 v6] Fix virtio-blk issue with SWIOTLB

2019-01-31 Thread Joerg Roedel
Hi,

here is the next version of this patch-set. Previous
versions can be found here:

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

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

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

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

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

The problem solved here 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.

Changes to v5 are:

- Changed patch 3 to uninline dma_max_mapping_size()

Please review.

Thanks,

Joerg

Joerg Roedel (5):
  swiotlb: Introduce swiotlb_max_mapping_size()
  swiotlb: Add is_swiotlb_active() function
  dma: Introduce dma_max_mapping_size()
  virtio: Introduce virtio_max_dma_size()
  virtio-blk: Consider virtio_max_dma_size() for maximum segment size

 Documentation/DMA-API.txt|  8 
 drivers/block/virtio_blk.c   | 10 ++
 drivers/virtio/virtio_ring.c | 11 +++
 include/linux/dma-mapping.h  |  8 
 include/linux/swiotlb.h  | 11 +++
 include/linux/virtio.h   |  2 ++
 kernel/dma/direct.c  | 11 +++
 kernel/dma/mapping.c | 14 ++
 kernel/dma/swiotlb.c | 14 ++
 9 files changed, 85 insertions(+), 4 deletions(-)

-- 
2.17.1

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