Re: [PATCH] virtio: Work around frames incorrectly marked as gso

2020-02-10 Thread Anton Ivanov 

On 11/02/2020 02:51, Jason Wang wrote:


On 2020/2/11 上午12:55, Anton Ivanov wrote:



On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).

This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.

These frames are reported as invalid while they are in fact
gso-less frames.

This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.

Signed-off-by: Anton Ivanov 
---
  include/linux/virtio_net.h | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const 
struct sk_buff *skb,

  hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
  else if (sinfo->gso_type & SKB_GSO_TCPV6)
  hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
-    else
-    return -EINVAL;
+    else {
+    if (skb->data_len == 0)
+    hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+    else
+    return -EINVAL;
+    }
  if (sinfo->gso_type & SKB_GSO_TCP_ECN)
  hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
  } else



ping.



Do you mean gso_size is set but gso_type is not? Looks like a bug 
elsewhere.


Thanks



Yes.

I could not trace it where it is coming from.

I see it when doing recvmmsg on raw sockets in the UML vector network 
drivers.



--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2] tools/virtio: option to build an out of tree module

2020-02-10 Thread Jason Wang 


On 2020/2/7 下午3:35, Michael S. Tsirkin wrote:

Handy for testing with distro kernels.
Warn that the resulting module is completely unsupported,
and isn't intended for production use.

Usage:
 make oot # builds vhost_test.ko, vhost.ko
 make oot-clean # cleans out files created

Signed-off-by: Michael S. Tsirkin 
---

changes from v1:
lots of refactoring
disable all modules except vhost by default (more of a chance
 it'll build)
oot-clean target

  tools/virtio/Makefile | 27 ++-
  1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index 8e2a908115c2..f33f32f1d208 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -8,7 +8,32 @@ CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I 
../../usr/include/ -Wno-poin
  vpath %.c ../../drivers/virtio ../../drivers/vhost
  mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
-.PHONY: all test mod clean
+
+#oot: build vhost as an out of tree module for a distro kernel
+#no effort is taken to make it actually build or work, but tends to mostly work
+#if the distro kernel is very close to upstream
+#unsupported! this is a development tool only, don't use the
+#resulting modules in production!
+OOT_KSRC=/lib/modules/$$(uname -r)/build
+OOT_VHOST=`pwd`/../../drivers/vhost
+#Everyone depends on vhost
+#Tweak the below to enable more modules
+OOT_CONFIGS=\
+   CONFIG_VHOST=m \
+   CONFIG_VHOST_NET=n \
+   CONFIG_VHOST_SCSI=n \
+   CONFIG_VHOST_VSOCK=n
+OOT_BUILD=KCFLAGS="-I "${OOT_VHOST} ${MAKE} -C ${OOT_KSRC} V=${V}
+oot-build:
+   echo "UNSUPPORTED! Don't use the resulting modules in production!"
+   ${OOT_BUILD} M=`pwd`/vhost_test
+   ${OOT_BUILD} M=${OOT_VHOST} ${OOT_CONFIGS}
+
+oot-clean: oot-build
+oot: oot-build
+oot-clean: OOT_BUILD+=clean
+
+.PHONY: all test mod clean vhost oot oot-clean oot-build
  clean:
${RM} *.o vringh_test virtio_test vhost_test/*.o vhost_test/.*.cmd \
vhost_test/Module.symvers vhost_test/modules.order *.d



Acked-by: Jason Wang 


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

Re: [PATCH V2 5/5] vdpasim: vDPA device simulator

2020-02-10 Thread Jason Wang 


On 2020/2/10 下午7:23, Michael S. Tsirkin wrote:

On Mon, Feb 10, 2020 at 11:56:08AM +0800, Jason Wang wrote:

This patch implements a software vDPA networking device. The datapath
is implemented through vringh and workqueue. The device has an on-chip
IOMMU which translates IOVA to PA. For kernel virtio drivers, vDPA
simulator driver provides dma_ops. For vhost driers, set_map() methods
of vdpa_config_ops is implemented to accept mappings from vhost.

Currently, vDPA device simulator will loopback TX traffic to RX. So
the main use case for the device is vDPA feature testing, prototyping
and development.

Note, there's no management API implemented, a vDPA device will be
registered once the module is probed. We need to handle this in the
future development.

Signed-off-by: Jason Wang
---
  drivers/virtio/vdpa/Kconfig|  17 +
  drivers/virtio/vdpa/Makefile   |   1 +
  drivers/virtio/vdpa/vdpa_sim.c | 678 +
  3 files changed, 696 insertions(+)
  create mode 100644 drivers/virtio/vdpa/vdpa_sim.c

diff --git a/drivers/virtio/vdpa/Kconfig b/drivers/virtio/vdpa/Kconfig
index 7a99170e6c30..a7888974dda8 100644
--- a/drivers/virtio/vdpa/Kconfig
+++ b/drivers/virtio/vdpa/Kconfig
@@ -7,3 +7,20 @@ config VDPA
datapath which complies with virtio specifications with
vendor specific control path.
  
+menuconfig VDPA_MENU

+   bool "VDPA drivers"
+   default n
+
+if VDPA_MENU
+
+config VDPA_SIM
+   tristate "vDPA device simulator"
+select VDPA
+default n
+help
+  vDPA networking device simulator which loop TX traffic back
+  to RX. This device is used for testing, prototyping and
+  development of vDPA.

So how about we make this depend on RUNTIME_TESTING_MENU?




I'm not sure how much it can help but I can do that in next version.

Thanks

RUNTIME_TESTING_MENU

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

Re: [PATCH V2 4/5] virtio: introduce a vDPA based transport

2020-02-10 Thread Jason Wang 


On 2020/2/10 下午9:34, Jason Gunthorpe wrote:

On Mon, Feb 10, 2020 at 11:56:07AM +0800, Jason Wang wrote:

This patch introduces a vDPA transport for virtio. This is used to
use kernel virtio driver to drive the mediated device that is capable
of populating virtqueue directly.

Is this comment still right? Is there a mediated device still?

Jason



No, will fix.

Thanks






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

Re: [PATCH] virtio: Work around frames incorrectly marked as gso

2020-02-10 Thread Jason Wang 


On 2020/2/11 上午12:55, Anton Ivanov wrote:



On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).

This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.

These frames are reported as invalid while they are in fact
gso-less frames.

This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.

Signed-off-by: Anton Ivanov 
---
  include/linux/virtio_net.h | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const 
struct sk_buff *skb,

  hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
  else if (sinfo->gso_type & SKB_GSO_TCPV6)
  hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
-    else
-    return -EINVAL;
+    else {
+    if (skb->data_len == 0)
+    hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+    else
+    return -EINVAL;
+    }
  if (sinfo->gso_type & SKB_GSO_TCP_ECN)
  hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
  } else



ping.



Do you mean gso_size is set but gso_type is not? Looks like a bug elsewhere.

Thanks

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

Re: [PULL] vhost: cleanups and fixes

2020-02-10 Thread Linus Torvalds 
On Sun, Feb 9, 2020 at 10:03 PM Michael S. Tsirkin  wrote:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

Hmm? Pull request re-send? This already got merged on Friday as commit
e0f121c5cc2c, as far as I can tell.

It looks like the pr-tracker-bot didn't reply to that old email. Maybe
because the subject line only says "PULL", not "GIT PULL". But more
likely because it looks like lore.kernel.org doesn't have a record of
that email at all.

You might want to check your email settings. You have some odd headers
(including a completely broken name that has "Cc:" in it in the "To:"
field).

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

Re: [PATCH v2 2/2] drm/qxl: add drm_driver.release callback.

2020-02-10 Thread Noralf Trønnes 
(adding back Daniel)

Den 10.02.2020 17.57, skrev Noralf Trønnes:
> 
> 
> Den 10.02.2020 16.06, skrev Daniel Vetter:
>> On Mon, Feb 10, 2020 at 12:37:52PM +0100, Gerd Hoffmann wrote:
>>> Move final cleanups to qxl_drm_release() callback.
>>> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>>>
>>> Signed-off-by: Gerd Hoffmann 
>>> ---
>>>  drivers/gpu/drm/qxl/qxl_drv.c | 26 +++---
>>>  1 file changed, 19 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>>> index 1d601f57a6ba..4fda3f9b29f4 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>>> @@ -34,6 +34,7 @@
>>>  #include 
>>>  
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -132,21 +133,30 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
>>> pci_device_id *ent)
>>> return ret;
>>>  }
>>>  
>>> +static void qxl_drm_release(struct drm_device *dev)
>>> +{
>>> +   struct qxl_device *qdev = dev->dev_private;
>>> +
>>> +   /*
>>> +* TODO: qxl_device_fini() call should be in qxl_pci_remove(),
>>> +* reodering qxl_modeset_fini() + qxl_device_fini() calls is
>>> +* non-trivial though.
>>> +*/
>>> +   qxl_modeset_fini(qdev);
>>
>> So the drm_mode_config_cleanup call in here belongs in ->release, but the
>> qxl_destroy_monitors_object feels like should be perfectly fine in the
>> remove hook. You might need to sprinkle a few drm_dev_enter/exit around to
>> protect code paths thought.
>>
>> Aside from this lgtm, for the series
>>
>> Acked-by: Daniel Vetter 
>>
>> And up to you whether you want to fix this or not really.
>>
>> Btw for testing all these patches that add a ->release hook I think it'd
>> be good if the drm core checks that drm_device->dev is set to NULL, and
>> that we do this in the remove hook. Since that's guaranteed to be gone at
>> that point, so anything in ->release that still needs the device is
>> broken. Ofc maybe do that check only for drivers which have a ->release
>> hook, and we might need a few fixups.
>>
> 
> We take a ref on the parent device in drm_dev_init() and release it in
> drm_dev_fini(). I added this because of the DRM_DEV_* macros we have, to
> protect access to the device struct after it was unregistered. Setting
> drm_device->dev to NULL in drm_dev_unregister() instead will provide the
> same protection I think.
> 
> commit 56be6503aab2
> drm/drv: Hold ref on parent device during drm_device lifetime
> 
> Noralf.
> 
>> Cheers, Daniel
>>
>>> +   qxl_device_fini(qdev);
>>> +   dev->dev_private = NULL;
>>> +   kfree(qdev);
>>> +}
>>> +
>>>  static void
>>>  qxl_pci_remove(struct pci_dev *pdev)
>>>  {
>>> struct drm_device *dev = pci_get_drvdata(pdev);
>>> -   struct qxl_device *qdev = dev->dev_private;
>>>  
>>> drm_dev_unregister(dev);
>>> -
>>> -   qxl_modeset_fini(qdev);
>>> -   qxl_device_fini(qdev);
>>> +   drm_atomic_helper_shutdown(dev);
>>> if (is_vga(pdev))
>>> vga_put(pdev, VGA_RSRC_LEGACY_IO);
>>> -
>>> -   dev->dev_private = NULL;
>>> -   kfree(qdev);
>>> drm_dev_put(dev);
>>>  }
>>>  
>>> @@ -279,6 +289,8 @@ static struct drm_driver qxl_driver = {
>>> .major = 0,
>>> .minor = 1,
>>> .patchlevel = 0,
>>> +
>>> +   .release = qxl_drm_release,
>>>  };
>>>  
>>>  static int __init qxl_init(void)
>>> -- 
>>> 2.18.1
>>>
>>
> ___
> 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 v2 2/2] drm/qxl: add drm_driver.release callback.

2020-02-10 Thread Noralf Trønnes 



Den 10.02.2020 16.06, skrev Daniel Vetter:
> On Mon, Feb 10, 2020 at 12:37:52PM +0100, Gerd Hoffmann wrote:
>> Move final cleanups to qxl_drm_release() callback.
>> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>>
>> Signed-off-by: Gerd Hoffmann 
>> ---
>>  drivers/gpu/drm/qxl/qxl_drv.c | 26 +++---
>>  1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
>> index 1d601f57a6ba..4fda3f9b29f4 100644
>> --- a/drivers/gpu/drm/qxl/qxl_drv.c
>> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
>> @@ -34,6 +34,7 @@
>>  #include 
>>  
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -132,21 +133,30 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
>> pci_device_id *ent)
>>  return ret;
>>  }
>>  
>> +static void qxl_drm_release(struct drm_device *dev)
>> +{
>> +struct qxl_device *qdev = dev->dev_private;
>> +
>> +/*
>> + * TODO: qxl_device_fini() call should be in qxl_pci_remove(),
>> + * reodering qxl_modeset_fini() + qxl_device_fini() calls is
>> + * non-trivial though.
>> + */
>> +qxl_modeset_fini(qdev);
> 
> So the drm_mode_config_cleanup call in here belongs in ->release, but the
> qxl_destroy_monitors_object feels like should be perfectly fine in the
> remove hook. You might need to sprinkle a few drm_dev_enter/exit around to
> protect code paths thought.
> 
> Aside from this lgtm, for the series
> 
> Acked-by: Daniel Vetter 
> 
> And up to you whether you want to fix this or not really.
> 
> Btw for testing all these patches that add a ->release hook I think it'd
> be good if the drm core checks that drm_device->dev is set to NULL, and
> that we do this in the remove hook. Since that's guaranteed to be gone at
> that point, so anything in ->release that still needs the device is
> broken. Ofc maybe do that check only for drivers which have a ->release
> hook, and we might need a few fixups.
> 

We take a ref on the parent device in drm_dev_init() and release it in
drm_dev_fini(). I added this because of the DRM_DEV_* macros we have, to
protect access to the device struct after it was unregistered. Setting
drm_device->dev to NULL in drm_dev_unregister() instead will provide the
same protection I think.

commit 56be6503aab2
drm/drv: Hold ref on parent device during drm_device lifetime

Noralf.

> Cheers, Daniel
> 
>> +qxl_device_fini(qdev);
>> +dev->dev_private = NULL;
>> +kfree(qdev);
>> +}
>> +
>>  static void
>>  qxl_pci_remove(struct pci_dev *pdev)
>>  {
>>  struct drm_device *dev = pci_get_drvdata(pdev);
>> -struct qxl_device *qdev = dev->dev_private;
>>  
>>  drm_dev_unregister(dev);
>> -
>> -qxl_modeset_fini(qdev);
>> -qxl_device_fini(qdev);
>> +drm_atomic_helper_shutdown(dev);
>>  if (is_vga(pdev))
>>  vga_put(pdev, VGA_RSRC_LEGACY_IO);
>> -
>> -dev->dev_private = NULL;
>> -kfree(qdev);
>>  drm_dev_put(dev);
>>  }
>>  
>> @@ -279,6 +289,8 @@ static struct drm_driver qxl_driver = {
>>  .major = 0,
>>  .minor = 1,
>>  .patchlevel = 0,
>> +
>> +.release = qxl_drm_release,
>>  };
>>  
>>  static int __init qxl_init(void)
>> -- 
>> 2.18.1
>>
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio: Work around frames incorrectly marked as gso

2020-02-10 Thread Anton Ivanov 




On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).

This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.

These frames are reported as invalid while they are in fact
gso-less frames.

This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.

Signed-off-by: Anton Ivanov 
---
  include/linux/virtio_net.h | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const struct 
sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
-   else
-   return -EINVAL;
+   else {
+   if (skb->data_len == 0)
+   hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+   else
+   return -EINVAL;
+   }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else



ping.

--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] drm/qxl: add drm_driver.release callback.

2020-02-10 Thread Daniel Vetter 
On Mon, Feb 10, 2020 at 12:37:52PM +0100, Gerd Hoffmann wrote:
> Move final cleanups to qxl_drm_release() callback.
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/qxl/qxl_drv.c | 26 +++---
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d601f57a6ba..4fda3f9b29f4 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -34,6 +34,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -132,21 +133,30 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   return ret;
>  }
>  
> +static void qxl_drm_release(struct drm_device *dev)
> +{
> + struct qxl_device *qdev = dev->dev_private;
> +
> + /*
> +  * TODO: qxl_device_fini() call should be in qxl_pci_remove(),
> +  * reodering qxl_modeset_fini() + qxl_device_fini() calls is
> +  * non-trivial though.
> +  */
> + qxl_modeset_fini(qdev);

So the drm_mode_config_cleanup call in here belongs in ->release, but the
qxl_destroy_monitors_object feels like should be perfectly fine in the
remove hook. You might need to sprinkle a few drm_dev_enter/exit around to
protect code paths thought.

Aside from this lgtm, for the series

Acked-by: Daniel Vetter 

And up to you whether you want to fix this or not really.

Btw for testing all these patches that add a ->release hook I think it'd
be good if the drm core checks that drm_device->dev is set to NULL, and
that we do this in the remove hook. Since that's guaranteed to be gone at
that point, so anything in ->release that still needs the device is
broken. Ofc maybe do that check only for drivers which have a ->release
hook, and we might need a few fixups.

Cheers, Daniel

> + qxl_device_fini(qdev);
> + dev->dev_private = NULL;
> + kfree(qdev);
> +}
> +
>  static void
>  qxl_pci_remove(struct pci_dev *pdev)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
> - struct qxl_device *qdev = dev->dev_private;
>  
>   drm_dev_unregister(dev);
> -
> - qxl_modeset_fini(qdev);
> - qxl_device_fini(qdev);
> + drm_atomic_helper_shutdown(dev);
>   if (is_vga(pdev))
>   vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -
> - dev->dev_private = NULL;
> - kfree(qdev);
>   drm_dev_put(dev);
>  }
>  
> @@ -279,6 +289,8 @@ static struct drm_driver qxl_driver = {
>   .major = 0,
>   .minor = 1,
>   .patchlevel = 0,
> +
> + .release = qxl_drm_release,
>  };
>  
>  static int __init qxl_init(void)
> -- 
> 2.18.1
> 

-- 
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 v2] drm/bochs: add drm_driver.release callback.

2020-02-10 Thread Daniel Vetter 
On Mon, Feb 10, 2020 at 10:38:01AM +0100, Gerd Hoffmann wrote:
> Call drm_dev_unregister() first in bochs_pci_remove().  Hook
> bochs_unload() into the new .release callback, to make sure cleanup
> is done when all users are gone.
> 
> Add ready bool to state struct and move bochs_hw_fini() call from
> bochs_unload() to bochs_pci_remove() to make sure hardware is not
> touched after bochs_pci_remove returns.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  drivers/gpu/drm/bochs/bochs.h |  1 +
>  drivers/gpu/drm/bochs/bochs_drv.c |  6 +++---
>  drivers/gpu/drm/bochs/bochs_hw.c  | 14 ++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> index 917767173ee6..f6bce8669274 100644
> --- a/drivers/gpu/drm/bochs/bochs.h
> +++ b/drivers/gpu/drm/bochs/bochs.h
> @@ -57,6 +57,7 @@ struct bochs_device {
>   unsigned long  fb_base;
>   unsigned long  fb_size;
>   unsigned long  qext_size;
> + bool   ready;
>  
>   /* mode */
>   u16 xres;
> diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
> b/drivers/gpu/drm/bochs/bochs_drv.c
> index 10460878414e..60b5492739ef 100644
> --- a/drivers/gpu/drm/bochs/bochs_drv.c
> +++ b/drivers/gpu/drm/bochs/bochs_drv.c
> @@ -23,7 +23,6 @@ static void bochs_unload(struct drm_device *dev)
>  
>   bochs_kms_fini(bochs);
>   bochs_mm_fini(bochs);
> - bochs_hw_fini(dev);
>   kfree(bochs);
>   dev->dev_private = NULL;
>  }
> @@ -69,6 +68,7 @@ static struct drm_driver bochs_driver = {
>   .major  = 1,
>   .minor  = 0,
>   DRM_GEM_VRAM_DRIVER,
> + .release= bochs_unload,
>  };
>  
>  /* -- */
> @@ -148,9 +148,9 @@ static void bochs_pci_remove(struct pci_dev *pdev)
>  {
>   struct drm_device *dev = pci_get_drvdata(pdev);
>  
> - drm_atomic_helper_shutdown(dev);
>   drm_dev_unregister(dev);
> - bochs_unload(dev);
> + drm_atomic_helper_shutdown(dev);
> + bochs_hw_fini(dev);
>   drm_dev_put(dev);
>  }
>  
> diff --git a/drivers/gpu/drm/bochs/bochs_hw.c 
> b/drivers/gpu/drm/bochs/bochs_hw.c
> index b615b7dfdd9d..48c1a6a8b026 100644
> --- a/drivers/gpu/drm/bochs/bochs_hw.c
> +++ b/drivers/gpu/drm/bochs/bochs_hw.c
> @@ -168,6 +168,7 @@ int bochs_hw_init(struct drm_device *dev)
>   }
>   bochs->fb_base = addr;
>   bochs->fb_size = size;
> + bochs->ready = true;
>  
>   DRM_INFO("Found bochs VGA, ID 0x%x.\n", id);
>   DRM_INFO("Framebuffer size %ld kB @ 0x%lx, %s @ 0x%lx.\n",
> @@ -194,6 +195,10 @@ void bochs_hw_fini(struct drm_device *dev)
>  {
>   struct bochs_device *bochs = dev->dev_private;
>  
> + bochs->ready = false;
> +
> + /* TODO: shot down existing vram mappings */

Aside: I'm mildly hopefull that we could do this with a generic helper,
both punching out all current ptes and replacing them with something
dummy. Since replacing them with nothing and refusing to fault stuff is
probably not going to work out well - userspace will crash too much.

> +
>   if (bochs->mmio)
>   iounmap(bochs->mmio);
>   if (bochs->ioports)
> @@ -207,6 +212,9 @@ void bochs_hw_fini(struct drm_device *dev)
>  void bochs_hw_setmode(struct bochs_device *bochs,
> struct drm_display_mode *mode)
>  {
> + if (!bochs->ready)
> + return;

drm_dev_enter/exit is the primitive you're looking for I think. Don't hand
roll your own racy version of this. btw changelog in the patch missing.
Personally I'd split out the drm_dev_enter/exit in a 2nd patch, but up to
you.

The remove/release split looks correct to me now.
-Daniel


> +
>   bochs->xres = mode->hdisplay;
>   bochs->yres = mode->vdisplay;
>   bochs->bpp = 32;
> @@ -237,6 +245,9 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>  void bochs_hw_setformat(struct bochs_device *bochs,
>   const struct drm_format_info *format)
>  {
> + if (!bochs->ready)
> + return;
> +
>   DRM_DEBUG_DRIVER("format %c%c%c%c\n",
>(format->format >>  0) & 0xff,
>(format->format >>  8) & 0xff,
> @@ -264,6 +275,9 @@ void bochs_hw_setbase(struct bochs_device *bochs,
>   unsigned long offset;
>   unsigned int vx, vy, vwidth;
>  
> + if (!bochs->ready)
> + return;
> +
>   bochs->stride = stride;
>   offset = (unsigned long)addr +
>   y * bochs->stride +
> -- 
> 2.18.1
> 

-- 
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 V2 4/5] virtio: introduce a vDPA based transport

2020-02-10 Thread Jason Gunthorpe 
On Mon, Feb 10, 2020 at 11:56:07AM +0800, Jason Wang wrote:
> This patch introduces a vDPA transport for virtio. This is used to
> use kernel virtio driver to drive the mediated device that is capable
> of populating virtqueue directly.

Is this comment still right? Is there a mediated device still?

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

[PATCH v2 1/2] drm/qxl: reorder calls in qxl_device_fini().

2020-02-10 Thread Gerd Hoffmann 
Reorder calls in qxl_device_fini().  Cleaning up gem & ttm
might trigger qxl commands, so we should do that before
releaseing command rings.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/qxl/qxl_kms.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index bfc1631093e9..70b20ee4741a 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
 {
qxl_bo_unref(>current_release_bo[0]);
qxl_bo_unref(>current_release_bo[1]);
-   flush_work(>gc_work);
-   qxl_ring_free(qdev->command_ring);
-   qxl_ring_free(qdev->cursor_ring);
-   qxl_ring_free(qdev->release_ring);
qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
+   flush_work(>gc_work);
+   qxl_ring_free(qdev->command_ring);
+   qxl_ring_free(qdev->cursor_ring);
+   qxl_ring_free(qdev->release_ring);
io_mapping_free(qdev->surface_mapping);
io_mapping_free(qdev->vram_mapping);
iounmap(qdev->ram_header);
-- 
2.18.1

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

[PATCH v2 2/2] drm/qxl: add drm_driver.release callback.

2020-02-10 Thread Gerd Hoffmann 
Move final cleanups to qxl_drm_release() callback.
Add drm_atomic_helper_shutdown() call to qxl_pci_remove().

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

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1d601f57a6ba..4fda3f9b29f4 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -34,6 +34,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -132,21 +133,30 @@ qxl_pci_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
return ret;
 }
 
+static void qxl_drm_release(struct drm_device *dev)
+{
+   struct qxl_device *qdev = dev->dev_private;
+
+   /*
+* TODO: qxl_device_fini() call should be in qxl_pci_remove(),
+* reodering qxl_modeset_fini() + qxl_device_fini() calls is
+* non-trivial though.
+*/
+   qxl_modeset_fini(qdev);
+   qxl_device_fini(qdev);
+   dev->dev_private = NULL;
+   kfree(qdev);
+}
+
 static void
 qxl_pci_remove(struct pci_dev *pdev)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
-   struct qxl_device *qdev = dev->dev_private;
 
drm_dev_unregister(dev);
-
-   qxl_modeset_fini(qdev);
-   qxl_device_fini(qdev);
+   drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
-
-   dev->dev_private = NULL;
-   kfree(qdev);
drm_dev_put(dev);
 }
 
@@ -279,6 +289,8 @@ static struct drm_driver qxl_driver = {
.major = 0,
.minor = 1,
.patchlevel = 0,
+
+   .release = qxl_drm_release,
 };
 
 static int __init qxl_init(void)
-- 
2.18.1

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

Re: [PATCH V2 5/5] vdpasim: vDPA device simulator

2020-02-10 Thread Michael S. Tsirkin 
On Mon, Feb 10, 2020 at 11:56:08AM +0800, Jason Wang wrote:
> This patch implements a software vDPA networking device. The datapath
> is implemented through vringh and workqueue. The device has an on-chip
> IOMMU which translates IOVA to PA. For kernel virtio drivers, vDPA
> simulator driver provides dma_ops. For vhost driers, set_map() methods
> of vdpa_config_ops is implemented to accept mappings from vhost.
> 
> Currently, vDPA device simulator will loopback TX traffic to RX. So
> the main use case for the device is vDPA feature testing, prototyping
> and development.
> 
> Note, there's no management API implemented, a vDPA device will be
> registered once the module is probed. We need to handle this in the
> future development.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/virtio/vdpa/Kconfig|  17 +
>  drivers/virtio/vdpa/Makefile   |   1 +
>  drivers/virtio/vdpa/vdpa_sim.c | 678 +
>  3 files changed, 696 insertions(+)
>  create mode 100644 drivers/virtio/vdpa/vdpa_sim.c
> 
> diff --git a/drivers/virtio/vdpa/Kconfig b/drivers/virtio/vdpa/Kconfig
> index 7a99170e6c30..a7888974dda8 100644
> --- a/drivers/virtio/vdpa/Kconfig
> +++ b/drivers/virtio/vdpa/Kconfig
> @@ -7,3 +7,20 @@ config VDPA
>datapath which complies with virtio specifications with
>vendor specific control path.
>  
> +menuconfig VDPA_MENU
> + bool "VDPA drivers"
> + default n
> +
> +if VDPA_MENU
> +
> +config VDPA_SIM
> + tristate "vDPA device simulator"
> +select VDPA
> +default n
> +help
> +  vDPA networking device simulator which loop TX traffic back
> +  to RX. This device is used for testing, prototyping and
> +  development of vDPA.

So how about we make this depend on RUNTIME_TESTING_MENU?


> +
> +endif # VDPA_MENU
> +
> diff --git a/drivers/virtio/vdpa/Makefile b/drivers/virtio/vdpa/Makefile
> index ee6a35e8a4fb..5ec0e6ae3c57 100644
> --- a/drivers/virtio/vdpa/Makefile
> +++ b/drivers/virtio/vdpa/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_VDPA) += vdpa.o
> +obj-$(CONFIG_VDPA_SIM) += vdpa_sim.o
> diff --git a/drivers/virtio/vdpa/vdpa_sim.c b/drivers/virtio/vdpa/vdpa_sim.c
> new file mode 100644
> index ..89783a2b9773
> --- /dev/null
> +++ b/drivers/virtio/vdpa/vdpa_sim.c
> @@ -0,0 +1,678 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * VDPA networking device simulator.
> + *
> + * Copyright (c) 2020, Red Hat Inc. All rights reserved.
> + * Author: Jason Wang 
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_VERSION  "0.1"
> +#define DRV_AUTHOR   "Jason Wang "
> +#define DRV_DESC "vDPA Device Simulator"
> +#define DRV_LICENSE  "GPL v2"
> +
> +struct vdpasim_dev {
> + struct device   dev;
> +};
> +
> +struct vdpasim_dev *vdpasim_dev;
> +
> +struct vdpasim_virtqueue {
> + struct vringh vring;
> + struct vringh_kiov iov;
> + unsigned short head;
> + bool ready;
> + u64 desc_addr;
> + u64 device_addr;
> + u64 driver_addr;
> + u32 num;
> + void *private;
> + irqreturn_t (*cb)(void *data);
> +};
> +
> +#define VDPASIM_QUEUE_ALIGN PAGE_SIZE
> +#define VDPASIM_QUEUE_MAX 256
> +#define VDPASIM_DEVICE_ID 0x1
> +#define VDPASIM_VENDOR_ID 0
> +#define VDPASIM_VQ_NUM 0x2
> +#define VDPASIM_NAME "vdpasim-netdev"
> +
> +static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
> +   (1ULL << VIRTIO_F_VERSION_1)  |
> +   (1ULL << VIRTIO_F_IOMMU_PLATFORM);
> +
> +/* State of each vdpasim device */
> +struct vdpasim {
> + struct vdpasim_virtqueue vqs[2];
> + struct work_struct work;
> + /* spinlock to synchronize virtqueue state */
> + spinlock_t lock;
> + struct vdpa_device vdpa;
> + struct virtio_net_config config;
> + struct vhost_iotlb *iommu;
> + void *buffer;
> + u32 status;
> + u32 generation;
> + u64 features;
> +};
> +
> +struct vdpasim *vdpa_sim;
> +
> +static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
> +{
> + return container_of(vdpa, struct vdpasim, vdpa);
> +}
> +
> +static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
> +{
> + struct vdpasim_virtqueue *vq = >vqs[idx];
> + int ret;
> +
> + ret = vringh_init_iotlb(>vring, vdpasim_features,
> + VDPASIM_QUEUE_MAX, false,
> + (struct vring_desc *)(uintptr_t)vq->desc_addr,
> + (struct vring_avail *)
> + (uintptr_t)vq->driver_addr,
> + (struct vring_used *)
> + (uintptr_t)vq->device_addr);
> +}
> +
> +static void

Re: vhost changes (batched) in linux-next after 12/13 trigger random crashes in KVM guests after reboot

2020-02-10 Thread Christian Borntraeger 



On 10.02.20 10:47, Eugenio Perez Martin wrote:
> Hi Christian.
> 
> I'm not able to reproduce the failure with 
> eccb852f1fe6bede630e2e4f1a121a81e34354ab commit. Could you add more data? 
> Your configuration (libvirt or qemu line), and host's dmesg output if any?
> 
> Thanks!

If it was not obvious, this is on s390x, a big endian system.

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

Re: [PATCH 2/6] drm: Add drm_simple_encoder_{init,create}()

2020-02-10 Thread kbuild test robot 
Hi Thomas,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.6-rc1 next-20200210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Provide-a-simple-encoder/20200210-153139
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
reproduce: make htmldocs

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All warnings (new ones prefixed by >>):

   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'getprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'setprocattr' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'locked_down' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'perf_event_open' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'perf_event_alloc' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'perf_event_free' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'perf_event_read' not described in 'security_list_options'
   include/linux/lsm_hooks.h:1830: warning: Function parameter or member 
'perf_event_write' not described in 'security_list_options'
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
   drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not 
found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_unregister_notifier' 
not found
   drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not 
found
   include/linux/regulator/machine.h:196: warning: Function parameter or member 
'max_uV_step' not described in 'regulation_constraints'
   include/linux/regulator/driver.h:223: warning: Function parameter or member 
'resume' not described in 'regulator_ops'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'dev_scratch' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 'list' not 
described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'ip_defrag_offset' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'skb_mstamp_ns' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'__cloned_offset' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'head_frag' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'__pkt_type_offset' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'encapsulation' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'encap_hdr_csum' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'csum_valid' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'__pkt_vlan_present_offset' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'vlan_present' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'csum_complete_sw' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'csum_level' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'inner_protocol_type' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'remcsum_offload' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'sender_cpu' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'reserved_tailroom' not described in 'sk_buff'
   include/linux/skbuff.h:890: warning: Function parameter or member 
'inner_ipproto' not described in 'sk_buff'
   include/net/sock.h:232: warning: Function parameter or member 'skc_addrpair' 
not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_portpair' 
not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or member 'skc_ipv6only' 
not described in 'sock_common'
   include/net/sock.h:232: warning: Function parameter or

Re: vhost changes (batched) in linux-next after 12/13 trigger random crashes in KVM guests after reboot

2020-02-10 Thread Christian Borntraeger 


On 10.02.20 10:40, Eugenio Perez Martin wrote:
> Hi Christian.
> 
> I'm not able to reproduce the failure with 
> eccb852f1fe6bede630e2e4f1a121a81e34354ab commit. Could you add more data? 
> Your configuration (libvirt or qemu line), and host's dmesg output if any?

I do the following in the guest:
ping -c 200 -f somevalidip; reboot
sometimes I need to do that multiple times and sometimes I do not get a guest 
crash but host dmesg like

Guest moved used index from 0 to 292

xml is pretty simple


  
  
  
  
  



Reverting this patch seems to make both problems go away.


> 
> Thanks!
> 
> On Fri, Feb 7, 2020 at 9:13 AM Christian Borntraeger  > wrote:
> 
> 
> 
> On 07.02.20 08:58, Michael S. Tsirkin wrote:
> > On Fri, Feb 07, 2020 at 08:47:14AM +0100, Christian Borntraeger wrote:
> >> Also adding Cornelia.
> >>
> >>
> >> On 06.02.20 23:17, Michael S. Tsirkin wrote:
> >>> On Thu, Feb 06, 2020 at 04:12:21PM +0100, Christian Borntraeger wrote:
> 
> 
>  On 06.02.20 15:22, epere...@redhat.com  
> wrote:
> > Hi Christian.
> >
> > Could you try this patch on top of ("38ced0208491 vhost: use 
> batched version by default")?
> >
> > It will not solve your first random crash but it should help with 
> the lost of network connectivity.
> >
> > Please let me know how does it goes.
> 
> 
>  38ced0208491 + this seem to be ok.
> 
>  Not sure if you can make out anything of this (and the previous git 
> bisect log)
> >>>
> >>> Yes it does - that this is just bad split-up of patches, and there's
> >>> still a real bug that caused worse crashes :)
> >>>
> >>> So I just pushed batch-v4.
> >>> I expect that will fail, and bisect to give us
> >>>     vhost: batching fetches
> >>> Can you try that please?
> >>>
> >>
> >> yes.
> >>
> >> eccb852f1fe6bede630e2e4f1a121a81e34354ab is the first bad commit
> >> commit eccb852f1fe6bede630e2e4f1a121a81e34354ab
> >> Author: Michael S. Tsirkin mailto:m...@redhat.com>>
> >> Date:   Mon Oct 7 06:11:18 2019 -0400
> >>
> >>     vhost: batching fetches
> >>     
> >>     With this patch applied, new and old code perform identically.
> >>     
> >>     Lots of extra optimizations are now possible, e.g.
> >>     we can fetch multiple heads with copy_from/to_user now.
> >>     We can get rid of maintaining the log array.  Etc etc.
> >>     
> >>     Signed-off-by: Michael S. Tsirkin  >
> >>
> >>  drivers/vhost/test.c  |  2 +-
> >>  drivers/vhost/vhost.c | 39 ++-
> >>  drivers/vhost/vhost.h |  4 +++-
> >>  3 files changed, 38 insertions(+), 7 deletions(-)
> >>
> >
> >
> > And the symptom is still the same - random crashes
> > after a bit of traffic, right?
> 
> random guest crashes after a reboot of the guests. As if vhost would still
> write into now stale buffers.
> 

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

[PATCH v2] drm/virtio: add drm_driver.release callback.

2020-02-10 Thread Gerd Hoffmann 
Split virtio_gpu_deinit(), move the drm shutdown and release to
virtio_gpu_release().  Also free vbufs in case we can't queue them.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/virtio/virtgpu_drv.h | 1 +
 drivers/gpu/drm/virtio/virtgpu_display.c | 1 -
 drivers/gpu/drm/virtio/virtgpu_drv.c | 4 
 drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 9 -
 5 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h 
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index d278c8c50f39..09a485b001e7 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -217,6 +217,7 @@ extern struct drm_ioctl_desc 
virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
 /* virtio_kms.c */
 int virtio_gpu_init(struct drm_device *dev);
 void virtio_gpu_deinit(struct drm_device *dev);
+void virtio_gpu_release(struct drm_device *dev);
 int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file);
 void virtio_gpu_driver_postclose(struct drm_device *dev, struct drm_file 
*file);
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c 
b/drivers/gpu/drm/virtio/virtgpu_display.c
index 7b0f0643bb2d..af953db4a0c9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -368,6 +368,5 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device 
*vgdev)
 
for (i = 0 ; i < vgdev->num_scanouts; ++i)
kfree(vgdev->outputs[i].edid);
-   drm_atomic_helper_shutdown(vgdev->ddev);
drm_mode_config_cleanup(vgdev->ddev);
 }
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c 
b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 8cf27af3ad53..664a741a3b0b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -31,6 +31,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
@@ -136,6 +137,7 @@ static void virtio_gpu_remove(struct virtio_device *vdev)
struct drm_device *dev = vdev->priv;
 
drm_dev_unregister(dev);
+   drm_atomic_helper_shutdown(dev);
virtio_gpu_deinit(dev);
drm_dev_put(dev);
 }
@@ -214,4 +216,6 @@ static struct drm_driver driver = {
.major = DRIVER_MAJOR,
.minor = DRIVER_MINOR,
.patchlevel = DRIVER_PATCHLEVEL,
+
+   .release = virtio_gpu_release,
 };
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c 
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index c1086df49816..b45d12e3db2a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -240,6 +240,11 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(>config_changed_work);
vgdev->vdev->config->reset(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
+}
+
+void virtio_gpu_release(struct drm_device *dev)
+{
+   struct virtio_gpu_device *vgdev = dev->dev_private;
 
virtio_gpu_modeset_fini(vgdev);
virtio_gpu_free_vbufs(vgdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index cc02fc4bab2a..cc674b45f904 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -330,6 +330,11 @@ static void virtio_gpu_queue_ctrl_sgs(struct 
virtio_gpu_device *vgdev,
bool notify = false;
int ret;
 
+   if (!vgdev->vqs_ready) {
+   free_vbuf(vgdev, vbuf);
+   return;
+   }
+
if (vgdev->has_indirect)
elemcnt = 1;
 
@@ -462,8 +467,10 @@ static void virtio_gpu_queue_cursor(struct 
virtio_gpu_device *vgdev,
int ret;
int outcnt;
 
-   if (!vgdev->vqs_ready)
+   if (!vgdev->vqs_ready) {
+   free_vbuf(vgdev, vbuf);
return;
+   }
 
sg_init_one(, vbuf->buf, vbuf->size);
sgs[0] = 
-- 
2.18.1

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

[PATCH v2] drm/cirrus: add drm_driver.release callback.

2020-02-10 Thread Gerd Hoffmann 
Move final cleanups from cirrus_pci_remove() to the new callback.
Add drm_atomic_helper_shutdown() call to cirrus_pci_remove().

Set pointers to NULL after iounmap() and check them before using
them to make sure we don't touch released hardware.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/cirrus/cirrus.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
index a91fb0d7282c..128db11ed4d3 100644
--- a/drivers/gpu/drm/cirrus/cirrus.c
+++ b/drivers/gpu/drm/cirrus/cirrus.c
@@ -154,6 +154,9 @@ static void cirrus_set_start_address(struct cirrus_device 
*cirrus, u32 offset)
u32 addr;
u8 tmp;
 
+   if (!cirrus->mmio)
+   return;
+
addr = offset >> 2;
wreg_crt(cirrus, 0x0c, (u8)((addr >> 8) & 0xff));
wreg_crt(cirrus, 0x0d, (u8)(addr & 0xff));
@@ -179,6 +182,9 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
int tmp;
int sr07 = 0, hdr = 0;
 
+   if (!cirrus->mmio)
+   return -1;
+
htotal = mode->htotal / 8;
hsyncend = mode->hsync_end / 8;
hsyncstart = mode->hsync_start / 8;
@@ -301,6 +307,9 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
struct cirrus_device *cirrus = fb->dev->dev_private;
void *vmap;
 
+   if (!cirrus->vram)
+   return -ENODEV;
+
vmap = drm_gem_shmem_vmap(fb->obj[0]);
if (!vmap)
return -ENOMEM;
@@ -502,6 +511,14 @@ static void cirrus_mode_config_init(struct cirrus_device 
*cirrus)
 
 /* -- */
 
+static void cirrus_release(struct drm_device *dev)
+{
+   struct cirrus_device *cirrus = dev->dev_private;
+
+   drm_mode_config_cleanup(dev);
+   kfree(cirrus);
+}
+
 DEFINE_DRM_GEM_FOPS(cirrus_fops);
 
 static struct drm_driver cirrus_driver = {
@@ -515,6 +532,7 @@ static struct drm_driver cirrus_driver = {
 
.fops= _fops,
DRM_GEM_SHMEM_DRIVER_OPS,
+   .release = cirrus_release,
 };
 
 static int cirrus_pci_probe(struct pci_dev *pdev,
@@ -599,11 +617,12 @@ static void cirrus_pci_remove(struct pci_dev *pdev)
struct cirrus_device *cirrus = dev->dev_private;
 
drm_dev_unregister(dev);
-   drm_mode_config_cleanup(dev);
+   drm_atomic_helper_shutdown(dev);
iounmap(cirrus->mmio);
+   cirrus->mmio = NULL;
iounmap(cirrus->vram);
+   cirrus->vram = NULL;
drm_dev_put(dev);
-   kfree(cirrus);
pci_release_regions(pdev);
 }
 
-- 
2.18.1

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

[PATCH v2] drm/bochs: add drm_driver.release callback.

2020-02-10 Thread Gerd Hoffmann 
Call drm_dev_unregister() first in bochs_pci_remove().  Hook
bochs_unload() into the new .release callback, to make sure cleanup
is done when all users are gone.

Add ready bool to state struct and move bochs_hw_fini() call from
bochs_unload() to bochs_pci_remove() to make sure hardware is not
touched after bochs_pci_remove returns.

Signed-off-by: Gerd Hoffmann 
---
 drivers/gpu/drm/bochs/bochs.h |  1 +
 drivers/gpu/drm/bochs/bochs_drv.c |  6 +++---
 drivers/gpu/drm/bochs/bochs_hw.c  | 14 ++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
index 917767173ee6..f6bce8669274 100644
--- a/drivers/gpu/drm/bochs/bochs.h
+++ b/drivers/gpu/drm/bochs/bochs.h
@@ -57,6 +57,7 @@ struct bochs_device {
unsigned long  fb_base;
unsigned long  fb_size;
unsigned long  qext_size;
+   bool   ready;
 
/* mode */
u16 xres;
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index 10460878414e..60b5492739ef 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -23,7 +23,6 @@ static void bochs_unload(struct drm_device *dev)
 
bochs_kms_fini(bochs);
bochs_mm_fini(bochs);
-   bochs_hw_fini(dev);
kfree(bochs);
dev->dev_private = NULL;
 }
@@ -69,6 +68,7 @@ static struct drm_driver bochs_driver = {
.major  = 1,
.minor  = 0,
DRM_GEM_VRAM_DRIVER,
+   .release= bochs_unload,
 };
 
 /* -- */
@@ -148,9 +148,9 @@ static void bochs_pci_remove(struct pci_dev *pdev)
 {
struct drm_device *dev = pci_get_drvdata(pdev);
 
-   drm_atomic_helper_shutdown(dev);
drm_dev_unregister(dev);
-   bochs_unload(dev);
+   drm_atomic_helper_shutdown(dev);
+   bochs_hw_fini(dev);
drm_dev_put(dev);
 }
 
diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
index b615b7dfdd9d..48c1a6a8b026 100644
--- a/drivers/gpu/drm/bochs/bochs_hw.c
+++ b/drivers/gpu/drm/bochs/bochs_hw.c
@@ -168,6 +168,7 @@ int bochs_hw_init(struct drm_device *dev)
}
bochs->fb_base = addr;
bochs->fb_size = size;
+   bochs->ready = true;
 
DRM_INFO("Found bochs VGA, ID 0x%x.\n", id);
DRM_INFO("Framebuffer size %ld kB @ 0x%lx, %s @ 0x%lx.\n",
@@ -194,6 +195,10 @@ void bochs_hw_fini(struct drm_device *dev)
 {
struct bochs_device *bochs = dev->dev_private;
 
+   bochs->ready = false;
+
+   /* TODO: shot down existing vram mappings */
+
if (bochs->mmio)
iounmap(bochs->mmio);
if (bochs->ioports)
@@ -207,6 +212,9 @@ void bochs_hw_fini(struct drm_device *dev)
 void bochs_hw_setmode(struct bochs_device *bochs,
  struct drm_display_mode *mode)
 {
+   if (!bochs->ready)
+   return;
+
bochs->xres = mode->hdisplay;
bochs->yres = mode->vdisplay;
bochs->bpp = 32;
@@ -237,6 +245,9 @@ void bochs_hw_setmode(struct bochs_device *bochs,
 void bochs_hw_setformat(struct bochs_device *bochs,
const struct drm_format_info *format)
 {
+   if (!bochs->ready)
+   return;
+
DRM_DEBUG_DRIVER("format %c%c%c%c\n",
 (format->format >>  0) & 0xff,
 (format->format >>  8) & 0xff,
@@ -264,6 +275,9 @@ void bochs_hw_setbase(struct bochs_device *bochs,
unsigned long offset;
unsigned int vx, vy, vwidth;
 
+   if (!bochs->ready)
+   return;
+
bochs->stride = stride;
offset = (unsigned long)addr +
y * bochs->stride +
-- 
2.18.1

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

Re: [PATCH] virtio_balloon: Fix unused label warning

2020-02-10 Thread David Hildenbrand 
On 10.02.20 10:33, Borislav Petkov wrote:
> From: Borislav Petkov 
> 
> Fix
> 
>   drivers/virtio/virtio_balloon.c: In function ‘virtballoon_probe’:
>   drivers/virtio/virtio_balloon.c:963:1: warning: label ‘out_del_vqs’ defined 
> but not used [-Wunused-label]
> 963 | out_del_vqs:
> | ^~
> 
> The CONFIG_BALLOON_COMPACTION ifdeffery should enclose it too.
> 
> Signed-off-by: Borislav Petkov 
> Cc: David Hildenbrand 
> ---
>  drivers/virtio/virtio_balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7bfe365d9372..b6ed5f8bccb1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -959,9 +959,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
>   iput(vb->vb_dev_info.inode);
>  out_kern_unmount:
>   kern_unmount(balloon_mnt);
> -#endif
>  out_del_vqs:
>   vdev->config->del_vqs(vdev);
> +#endif
>  out_free_vb:
>   kfree(vb);
>  out:
> 

Oh, right, thanks!

Reviewed-by: David Hildenbrand 

-- 
Thanks,

David / dhildenb

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

[PATCH] virtio_balloon: Fix unused label warning

2020-02-10 Thread Borislav Petkov 
From: Borislav Petkov 

Fix

  drivers/virtio/virtio_balloon.c: In function ‘virtballoon_probe’:
  drivers/virtio/virtio_balloon.c:963:1: warning: label ‘out_del_vqs’ defined 
but not used [-Wunused-label]
963 | out_del_vqs:
| ^~

The CONFIG_BALLOON_COMPACTION ifdeffery should enclose it too.

Signed-off-by: Borislav Petkov 
Cc: David Hildenbrand 
---
 drivers/virtio/virtio_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7bfe365d9372..b6ed5f8bccb1 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -959,9 +959,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
iput(vb->vb_dev_info.inode);
 out_kern_unmount:
kern_unmount(balloon_mnt);
-#endif
 out_del_vqs:
vdev->config->del_vqs(vdev);
+#endif
 out_free_vb:
kfree(vb);
 out:
-- 
2.21.0

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

Re: [PATCH 5/6] drm/qxl: Use simple encoder

2020-02-10 Thread kbuild test robot 
Hi Thomas,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.6-rc1 next-20200210]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Provide-a-simple-encoder/20200210-153139
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
config: x86_64-randconfig-f002-20200210 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/qxl/qxl_display.c: In function 'qdev_output_init':
>> drivers/gpu/drm/qxl/qxl_display.c:1103:31: error: incompatible type for 
>> argument 2 of 'drm_simple_encoder_init'
 drm_simple_encoder_init(dev, qxl_output->enc, DRM_MODE_ENCODER_VIRTUAL,
  ^~
   In file included from include/drm/drm_modeset_helper_vtables.h:33:0,
from include/drm/drm_atomic_helper.h:32,
from drivers/gpu/drm/qxl/qxl_display.c:30:
   include/drm/drm_encoder.h:194:5: note: expected 'struct drm_encoder *' but 
argument is of type 'struct drm_encoder'
int drm_simple_encoder_init(struct drm_device *dev,
^~~

vim +/drm_simple_encoder_init +1103 drivers/gpu/drm/qxl/qxl_display.c

  1084  
  1085  static int qdev_output_init(struct drm_device *dev, int num_output)
  1086  {
  1087  struct qxl_device *qdev = dev->dev_private;
  1088  struct qxl_output *qxl_output;
  1089  struct drm_connector *connector;
  1090  struct drm_encoder *encoder;
  1091  
  1092  qxl_output = kzalloc(sizeof(struct qxl_output), GFP_KERNEL);
  1093  if (!qxl_output)
  1094  return -ENOMEM;
  1095  
  1096  qxl_output->index = num_output;
  1097  
  1098  connector = _output->base;
  1099  encoder = _output->enc;
  1100  drm_connector_init(dev, _output->base,
  1101 _connector_funcs, 
DRM_MODE_CONNECTOR_VIRTUAL);
  1102  
> 1103  drm_simple_encoder_init(dev, qxl_output->enc, 
> DRM_MODE_ENCODER_VIRTUAL,
  1104  NULL);
  1105  
  1106  /* we get HPD via client monitors config */
  1107  connector->polled = DRM_CONNECTOR_POLL_HPD;
  1108  encoder->possible_crtcs = 1 << num_output;
  1109  drm_connector_attach_encoder(_output->base,
  1110_output->enc);
    drm_connector_helper_add(connector, 
_connector_helper_funcs);
  1112  
  1113  drm_object_attach_property(>base,
  1114 qdev->hotplug_mode_update_property, 
0);
  1115  drm_object_attach_property(>base,
  1116 
dev->mode_config.suggested_x_property, 0);
  1117  drm_object_attach_property(>base,
  1118 
dev->mode_config.suggested_y_property, 0);
  1119  return 0;
  1120  }
  1121  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization