[PATCH v2] virtio_blk: Fix a slient kernel panic

2016-07-17 Thread Minfei Huang



0001-virtio_blk-Fix-a-slient-kernel-panic.patch
Description: Binary data
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver

2016-07-17 Thread Kees Cook
On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim  wrote:
> The virtio pstore driver provides interface to the pstore subsystem so
> that the guest kernel's log/dump message can be saved on the host
> machine.  Users can access the log file directly on the host, or on the
> guest at the next boot using pstore filesystem.  It currently deals with
> kernel log (printk) buffer only, but we can extend it to have other
> information (like ftrace dump) later.
>
> It supports legacy PCI device using single order-2 page buffer.  As all
> operation of pstore is synchronous, it would be fine IMHO.  However I
> don't know how to make write operation synchronous since it's called
> with a spinlock held (from any context including NMI).
>
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: "Michael S. Tsirkin" 
> Cc: Anthony Liguori 
> Cc: Anton Vorontsov 
> Cc: Colin Cross 
> Cc: Kees Cook 
> Cc: Tony Luck 
> Cc: Steven Rostedt 
> Cc: Ingo Molnar 
> Cc: Minchan Kim 
> Cc: k...@vger.kernel.org
> Cc: qemu-de...@nongnu.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Namhyung Kim 

This looks great to me! I'd love to use this in qemu. (Right now I go
through hoops to use the ramoops backend for testing.)

Reviewed-by: Kees Cook 

Notes below...

> ---
>  drivers/virtio/Kconfig |  10 ++
>  drivers/virtio/Makefile|   1 +
>  drivers/virtio/virtio_pstore.c | 317 
> +
>  include/uapi/linux/Kbuild  |   1 +
>  include/uapi/linux/virtio_ids.h|   1 +
>  include/uapi/linux/virtio_pstore.h |  53 +++
>  6 files changed, 383 insertions(+)
>  create mode 100644 drivers/virtio/virtio_pstore.c
>  create mode 100644 include/uapi/linux/virtio_pstore.h
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 77590320d44c..8f0e6c796c12 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -58,6 +58,16 @@ config VIRTIO_INPUT
>
>  If unsure, say M.
>
> +config VIRTIO_PSTORE
> +   tristate "Virtio pstore driver"
> +   depends on VIRTIO
> +   depends on PSTORE
> +   ---help---
> +This driver supports virtio pstore devices to save/restore
> +panic and oops messages on the host.
> +
> +If unsure, say M.
> +
>   config VIRTIO_MMIO
> tristate "Platform bus driver for memory mapped virtio devices"
> depends on HAS_IOMEM && HAS_DMA
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index 41e30e3dc842..bee68cb26d48 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
>  virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
>  obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
>  obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> +obj-$(CONFIG_VIRTIO_PSTORE) += virtio_pstore.o
> diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
> new file mode 100644
> index ..6fe62c0f1508
> --- /dev/null
> +++ b/drivers/virtio/virtio_pstore.c
> @@ -0,0 +1,317 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define VIRT_PSTORE_ORDER2
> +#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
> +
> +struct virtio_pstore {
> +   struct virtio_device*vdev;
> +   struct virtqueue*vq;
> +   struct pstore_info   pstore;
> +   struct virtio_pstore_hdr hdr;
> +   size_t   buflen;
> +   u64  id;
> +
> +   /* Waiting for host to ack */
> +   wait_queue_head_t   acked;
> +};
> +
> +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id 
> type)
> +{
> +   u16 ret;
> +
> +   switch (type) {
> +   case PSTORE_TYPE_DMESG:
> +   ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_DMESG);
> +   break;
> +   default:
> +   ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
> +   break;
> +   }

I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
relatively easy to add: I think it'd just be another virtio command?

> +
> +   return ret;
> +}
> +
> +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 
> type)
> +{
> +   enum pstore_type_id ret;
> +
> +   switch (virtio16_to_cpu(vps->vdev, type)) {
> +   case VIRTIO_PSTORE_TYPE_DMESG:
> +   ret = PSTORE_TYPE_DMESG;
> +   break;
> +   default:
> +   ret = PSTORE_TYPE_UNKNOWN;
> +   break;
> +   }
> +
> +   return ret;
> +}
> +
> +static void virtpstore_ack(struct virtqueue *vq)
> +{
> +   struct virtio_pstore *vps = vq->vdev->priv;
> +
> +   wake_up(&vps->acked);
> +}
> +
> +static int virt_pstore_open(struct pstore_info *psi)
> +{
> +   struct virtio_pstore *vps = psi->data;
> +   stru