* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Michael S. Tsirkin @ 2016-07-28 0:17 UTC (permalink / raw)
To: Li, Liang Z
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
Hansen, Dave, qemu-devel@nongnu.org, Amit Shah,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, linux-mm@kvack.org,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, Mel Gorman,
dgilbert@redhat.com
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E04213C27@shsmsx102.ccr.corp.intel.com>
On Thu, Jul 28, 2016 at 12:10:16AM +0000, Li, Liang Z wrote:
> > Subject: Re: [PATCH v2 repost 6/7] mm: add the related functions to get free
> > page info
> >
> > On 07/26/2016 06:23 PM, Liang Li wrote:
> > > + for_each_migratetype_order(order, t) {
> > > + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> > > + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> > > + if (pfn >= start_pfn && pfn <= end_pfn) {
> > > + page_num = 1UL << order;
> > > + if (pfn + page_num > end_pfn)
> > > + page_num = end_pfn - pfn;
> > > + bitmap_set(bitmap, pfn - start_pfn,
> > page_num);
> > > + }
> > > + }
> > > + }
> >
> > Nit: The 'page_num' nomenclature really confused me here. It is the
> > number of bits being set in the bitmap. Seems like calling it nr_pages or
> > num_pages would be more appropriate.
> >
>
> You are right, will change.
>
> > Isn't this bitmap out of date by the time it's send up to the hypervisor? Is
> > there something that makes the inaccuracy OK here?
>
> Yes. The dirty page logging will be used to correct the inaccuracy.
> The dirty page logging should be started before getting the free page bitmap, then if some of the free pages become no free for writing, these pages will be tracked by the dirty page logging mechanism.
>
> Thanks!
> Liang
Right but this should be clear from code and naming.
^ permalink raw reply
* RE: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Li, Liang Z @ 2016-07-28 0:10 UTC (permalink / raw)
To: Hansen, Dave, linux-kernel@vger.kernel.org
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org, Amit Shah,
Michael S. Tsirkin, qemu-devel@nongnu.org, dgilbert@redhat.com,
linux-mm@kvack.org, Paolo Bonzini, Andrew Morton,
virtualization@lists.linux-foundation.org, Mel Gorman,
Vlastimil Babka
In-Reply-To: <5798E418.7080608@intel.com>
> Subject: Re: [PATCH v2 repost 6/7] mm: add the related functions to get free
> page info
>
> On 07/26/2016 06:23 PM, Liang Li wrote:
> > + for_each_migratetype_order(order, t) {
> > + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> > + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> > + if (pfn >= start_pfn && pfn <= end_pfn) {
> > + page_num = 1UL << order;
> > + if (pfn + page_num > end_pfn)
> > + page_num = end_pfn - pfn;
> > + bitmap_set(bitmap, pfn - start_pfn,
> page_num);
> > + }
> > + }
> > + }
>
> Nit: The 'page_num' nomenclature really confused me here. It is the
> number of bits being set in the bitmap. Seems like calling it nr_pages or
> num_pages would be more appropriate.
>
You are right, will change.
> Isn't this bitmap out of date by the time it's send up to the hypervisor? Is
> there something that makes the inaccuracy OK here?
Yes. The dirty page logging will be used to correct the inaccuracy.
The dirty page logging should be started before getting the free page bitmap, then if some of the free pages become no free for writing, these pages will be tracked by the dirty page logging mechanism.
Thanks!
Liang
^ permalink raw reply
* Re: [PATCH 6/7] qemu: Implement virtio-pstore device
From: Michael S. Tsirkin @ 2016-07-28 0:02 UTC (permalink / raw)
To: Namhyung Kim
Cc: Tony Luck, Kees Cook, kvm, Radim Krčmář,
Anton Vorontsov, LKML, Steven Rostedt, qemu-devel, Minchan Kim,
Anthony Liguori, Colin Cross, Paolo Bonzini, virtualization,
Ingo Molnar
In-Reply-To: <1469632111-23260-7-git-send-email-namhyung@kernel.org>
On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
> Add virtio pstore device to allow kernel log files saved on the host.
> It will save the log files on the directory given by pstore device
> option.
>
> $ qemu-system-x86_64 -device virtio-pstore,directory=dir-xx ...
>
> (guest) # echo c > /proc/sysrq-trigger
So if the point is handling system crashes, I suspect
virtio is the wrong protocol to use. ATM it's rather
elaborate for performance, it's likely not to work when
linux is crashing. I think you want something very very
simple that will still work when you crash. Like maybe
a serial device?
> $ ls dir-xx
> dmesg-1.enc.z dmesg-2.enc.z
>
> The log files are usually compressed using zlib. Users can see the log
> messages directly on the host or on the guest (using pstore filesystem).
So this lacks all management tools that a regular storage device
has, and these are necessary if people are to use this
in production.
For example, some kind of provision for limiting the amount of host disk
this can consume seems called for. Rate-limiting disk writes
on host also seems necessary. Handling host disk errors always
propagates error to guest but an option to e.g. stop vm might
be useful to avoid corruption.
>
> The 'directory' property is required for virtio-pstore device to work.
> It also adds 'bufsize' and 'console' (boolean) properties.
No idea what these do. Seem to be RW by guest but have no effect
otherwise.
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: qemu-devel@nongnu.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> hw/virtio/Makefile.objs | 2 +-
> hw/virtio/virtio-pci.c | 54 +++
> hw/virtio/virtio-pci.h | 14 +
> hw/virtio/virtio-pstore.c | 477 +++++++++++++++++++++++++
> include/hw/pci/pci.h | 1 +
> include/hw/virtio/virtio-pstore.h | 34 ++
> include/standard-headers/linux/virtio_ids.h | 1 +
> include/standard-headers/linux/virtio_pstore.h | 80 +++++
> qdev-monitor.c | 1 +
> 9 files changed, 663 insertions(+), 1 deletion(-)
> create mode 100644 hw/virtio/virtio-pstore.c
> create mode 100644 include/hw/virtio/virtio-pstore.h
> create mode 100644 include/standard-headers/linux/virtio_pstore.h
>
> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> index 3e2b175..aae7082 100644
> --- a/hw/virtio/Makefile.objs
> +++ b/hw/virtio/Makefile.objs
> @@ -4,4 +4,4 @@ common-obj-y += virtio-bus.o
> common-obj-y += virtio-mmio.o
>
> obj-y += virtio.o virtio-balloon.o
> -obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
> +obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o virtio-pstore.o
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index f0677b7..d99a405 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -2414,6 +2414,59 @@ static const TypeInfo virtio_host_pci_info = {
> };
> #endif
>
> +/* virtio-pstore-pci */
> +
> +static void virtio_pstore_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> +{
> + VirtIOPstorePCI *vps = VIRTIO_PSTORE_PCI(vpci_dev);
> + DeviceState *vdev = DEVICE(&vps->vdev);
> + Error *err = NULL;
> +
> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> + object_property_set_bool(OBJECT(vdev), true, "realized", &err);
> + if (err) {
> + error_propagate(errp, err);
> + return;
> + }
> +}
> +
> +static void virtio_pstore_pci_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> +
> + k->realize = virtio_pstore_pci_realize;
> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +
> + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_PSTORE;
> + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> + pcidev_k->class_id = PCI_CLASS_OTHERS;
> +}
> +
> +static void virtio_pstore_pci_instance_init(Object *obj)
> +{
> + VirtIOPstorePCI *dev = VIRTIO_PSTORE_PCI(obj);
> +
> + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> + TYPE_VIRTIO_PSTORE);
> + object_property_add_alias(obj, "directory", OBJECT(&dev->vdev),
> + "directory", &error_abort);
> + object_property_add_alias(obj, "bufsize", OBJECT(&dev->vdev),
> + "bufsize", &error_abort);
> + object_property_add_alias(obj, "console", OBJECT(&dev->vdev),
> + "console", &error_abort);
> +}
> +
> +static const TypeInfo virtio_pstore_pci_info = {
> + .name = TYPE_VIRTIO_PSTORE_PCI,
> + .parent = TYPE_VIRTIO_PCI,
> + .instance_size = sizeof(VirtIOPstorePCI),
> + .instance_init = virtio_pstore_pci_instance_init,
> + .class_init = virtio_pstore_pci_class_init,
> +};
> +
> /* virtio-pci-bus */
>
> static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
> @@ -2483,6 +2536,7 @@ static void virtio_pci_register_types(void)
> #ifdef CONFIG_VHOST_SCSI
> type_register_static(&vhost_scsi_pci_info);
> #endif
> + type_register_static(&virtio_pstore_pci_info);
> }
>
> type_init(virtio_pci_register_types)
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index e4548c2..b4c039f 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -31,6 +31,7 @@
> #ifdef CONFIG_VHOST_SCSI
> #include "hw/virtio/vhost-scsi.h"
> #endif
> +#include "hw/virtio/virtio-pstore.h"
>
> typedef struct VirtIOPCIProxy VirtIOPCIProxy;
> typedef struct VirtIOBlkPCI VirtIOBlkPCI;
> @@ -44,6 +45,7 @@ typedef struct VirtIOInputPCI VirtIOInputPCI;
> typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI;
> typedef struct VirtIOInputHostPCI VirtIOInputHostPCI;
> typedef struct VirtIOGPUPCI VirtIOGPUPCI;
> +typedef struct VirtIOPstorePCI VirtIOPstorePCI;
>
> /* virtio-pci-bus */
>
> @@ -311,6 +313,18 @@ struct VirtIOGPUPCI {
> VirtIOGPU vdev;
> };
>
> +/*
> + * virtio-pstore-pci: This extends VirtioPCIProxy.
> + */
> +#define TYPE_VIRTIO_PSTORE_PCI "virtio-pstore-pci"
> +#define VIRTIO_PSTORE_PCI(obj) \
> + OBJECT_CHECK(VirtIOPstorePCI, (obj), TYPE_VIRTIO_PSTORE_PCI)
> +
> +struct VirtIOPstorePCI {
> + VirtIOPCIProxy parent_obj;
> + VirtIOPstore vdev;
> +};
> +
> /* Virtio ABI version, if we increment this, we break the guest driver. */
> #define VIRTIO_PCI_ABI_VERSION 0
>
> diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c
> new file mode 100644
> index 0000000..2ca7786
> --- /dev/null
> +++ b/hw/virtio/virtio-pstore.c
> @@ -0,0 +1,477 @@
> +/*
> + * Virtio Pstore Device
> + *
> + * Copyright (C) 2016 LG Electronics
> + *
> + * Authors:
> + * Namhyung Kim <namhyung@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
We generally ask new code to be v2 or later.
> See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include <stdio.h>
> +
> +#include "qemu/osdep.h"
> +#include "qemu/iov.h"
> +#include "qemu-common.h"
> +#include "qemu/cutils.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/kvm.h"
> +#include "qapi/visitor.h"
> +#include "qapi-event.h"
> +#include "trace.h"
> +
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/virtio-bus.h"
> +#include "hw/virtio/virtio-access.h"
> +#include "hw/virtio/virtio-pstore.h"
> +
> +
> +static void virtio_pstore_to_filename(VirtIOPstore *s, char *buf, size_t sz,
> + struct virtio_pstore_req *req)
> +{
> + const char *basename;
> + unsigned long long id = 0;
> + unsigned int flags = le32_to_cpu(req->flags);
> +
> + switch (le16_to_cpu(req->type)) {
> + case VIRTIO_PSTORE_TYPE_DMESG:
> + basename = "dmesg";
> + id = s->id++;
> + break;
> + case VIRTIO_PSTORE_TYPE_CONSOLE:
> + basename = "console";
> + if (s->console_id) {
> + id = s->console_id;
> + } else {
> + id = s->console_id = s->id++;
> + }
> + break;
> + default:
> + basename = "unknown";
> + break;
> + }
> +
> + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, id,
> + flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : "");
> +}
> +
> +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> + char *buf, size_t sz,
> + struct virtio_pstore_fileinfo *info)
> +{
> + snprintf(buf, sz, "%s/%s", s->directory, name);
if this does not fit, buf will not be 0 terminated and can
generally be corrupted.
> +
> + if (g_str_has_prefix(name, "dmesg-")) {
> + info->type = VIRTIO_PSTORE_TYPE_DMESG;
> + name += strlen("dmesg-");
> + } else if (g_str_has_prefix(name, "console-")) {
> + info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
> + name += strlen("console-");
> + } else if (g_str_has_prefix(name, "unknown-")) {
> + info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> + name += strlen("unknown-");
> + }
> +
> + qemu_strtoull(name, NULL, 0, &info->id);
> +
> + info->flags = 0;
> + if (g_str_has_suffix(name, ".enc.z")) {
> + info->flags |= VIRTIO_PSTORE_FL_COMPRESSED;
> + }
> +}
> +
> +static ssize_t virtio_pstore_do_open(VirtIOPstore *s)
> +{
> + s->dirp = opendir(s->directory);
> + if (s->dirp == NULL) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, struct iovec *in_sg,
> + unsigned int in_num,
> + struct virtio_pstore_res *res)
> +{
> + char path[PATH_MAX];
> + int fd;
> + ssize_t len;
> + struct stat stbuf;
> + struct dirent *dent;
> + int sg_num = in_num;
> + struct iovec sg[sg_num];
> + struct virtio_pstore_fileinfo info;
> + size_t offset = sizeof(*res) + sizeof(info);
> +
> + if (s->dirp == NULL) {
> + return -1;
> + }
> +
> + dent = readdir(s->dirp);
> + while (dent) {
> + if (dent->d_name[0] != '.') {
> + break;
> + }
> + dent = readdir(s->dirp);
> + }
> +
> + if (dent == NULL) {
> + return 0;
> + }
> +
> + /* skip res and fileinfo */
> + sg_num = iov_copy(sg, sg_num, in_sg, in_num, offset,
> + iov_size(in_sg, in_num) - offset);
> +
> + virtio_pstore_from_filename(s, dent->d_name, path, sizeof(path), &info);
> + fd = open(path, O_RDONLY);
> + if (fd < 0) {
> + error_report("cannot open %s", path);
> + return -1;
> + }
> +
> + if (fstat(fd, &stbuf) < 0) {
> + len = -1;
> + goto out;
> + }
> +
> + len = readv(fd, sg, sg_num);
> + if (len < 0) {
> + if (errno == EAGAIN) {
> + len = 0;
> + }
> + goto out;
> + }
> +
> + info.id = cpu_to_le64(info.id);
> + info.type = cpu_to_le16(info.type);
> + info.flags = cpu_to_le32(info.flags);
> + info.len = cpu_to_le32(len);
> + info.time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec);
> + info.time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec);
> +
> + iov_from_buf(in_sg, in_num, sizeof(*res), &info, sizeof(info));
> + len += sizeof(info);
> +
> + out:
> + close(fd);
> + return len;
> +}
> +
> +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg,
> + unsigned int out_num,
> + struct virtio_pstore_req *req)
> +{
> + char path[PATH_MAX];
> + int fd;
> + ssize_t len;
> + unsigned short type;
> + int flags = O_WRONLY | O_CREAT;
> +
> + /* we already consume the req */
> + iov_discard_front(&out_sg, &out_num, sizeof(*req));
> +
> + virtio_pstore_to_filename(s, path, sizeof(path), req);
> +
> + type = le16_to_cpu(req->type);
> +
> + if (type == VIRTIO_PSTORE_TYPE_DMESG) {
> + flags |= O_TRUNC;
> + } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) {
> + flags |= O_APPEND;
> + }
> +
> + fd = open(path, flags, 0644);
> + if (fd < 0) {
> + error_report("cannot open %s", path);
> + return -1;
> + }
> + len = writev(fd, out_sg, out_num);
> + close(fd);
> +
> + return len;
All this is blocking VM until host io completes.
> +}
> +
> +static ssize_t virtio_pstore_do_close(VirtIOPstore *s)
> +{
> + if (s->dirp == NULL) {
> + return 0;
> + }
> +
> + closedir(s->dirp);
> + s->dirp = NULL;
> +
> + return 0;
> +}
> +
> +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s,
> + struct virtio_pstore_req *req)
> +{
> + char path[PATH_MAX];
> +
> + virtio_pstore_to_filename(s, path, sizeof(path), req);
> +
> + return unlink(path);
> +}
> +
> +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + VirtIOPstore *s = VIRTIO_PSTORE(vdev);
> + VirtQueueElement *elem;
> + struct virtio_pstore_req req;
> + struct virtio_pstore_res res;
> + ssize_t len = 0;
> + int ret;
> +
> + for (;;) {
> + elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> + if (!elem) {
> + return;
> + }
> +
> + if (elem->out_num < 1 || elem->in_num < 1) {
> + error_report("request or response buffer is missing");
> + exit(1);
> + }
> +
> + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> + if (len != (ssize_t)sizeof(req)) {
> + error_report("invalid request size: %ld", (long)len);
> + exit(1);
> + }
> + res.cmd = req.cmd;
> + res.type = req.type;
> +
> + switch (le16_to_cpu(req.cmd)) {
> + case VIRTIO_PSTORE_CMD_OPEN:
> + ret = virtio_pstore_do_open(s);
> + break;
> + case VIRTIO_PSTORE_CMD_READ:
> + ret = virtio_pstore_do_read(s, elem->in_sg, elem->in_num, &res);
> + if (ret > 0) {
> + len = ret;
> + ret = 0;
> + }
> + break;
> + case VIRTIO_PSTORE_CMD_WRITE:
> + ret = virtio_pstore_do_write(s, elem->out_sg, elem->out_num, &req);
> + break;
> + case VIRTIO_PSTORE_CMD_CLOSE:
> + ret = virtio_pstore_do_close(s);
> + break;
> + case VIRTIO_PSTORE_CMD_ERASE:
> + ret = virtio_pstore_do_erase(s, &req);
> + break;
> + default:
> + ret = -1;
> + break;
> + }
> +
> + res.ret = ret;
> +
> + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> + virtqueue_push(vq, elem, sizeof(res) + len);
this is wrong - len should be # of bytes written into guest memory.
> +
> + virtio_notify(vdev, vq);
> + g_free(elem);
> +
> + if (ret < 0) {
> + return;
> + }
> + }
> +}
> +
> +static void virtio_pstore_device_realize(DeviceState *dev, Error **errp)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> + VirtIOPstore *s = VIRTIO_PSTORE(dev);
> +
> + virtio_init(vdev, "virtio-pstore", VIRTIO_ID_PSTORE,
> + sizeof(struct virtio_pstore_config));
> +
> + s->id = 1;
> + s->console_id = 0;
> +
> + s->vq[0] = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> + s->vq[1] = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> +}
> +
> +static void virtio_pstore_device_unrealize(DeviceState *dev, Error **errp)
> +{
> + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +
> + virtio_cleanup(vdev);
> +}
> +
> +static void virtio_pstore_get_config(VirtIODevice *vdev, uint8_t *config_data)
> +{
> + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> + struct virtio_pstore_config config;
> +
> + config.bufsize = cpu_to_le32(dev->bufsize);
> + if (dev->console) {
> + config.flags |= cpu_to_le32(VIRTIO_PSTORE_CONFIG_FL_CONSOLE);
> + }
> +
> + memcpy(config_data, &config, sizeof(struct virtio_pstore_config));
> +}
> +
> +static void virtio_pstore_set_config(VirtIODevice *vdev,
> + const uint8_t *config_data)
> +{
> + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> + struct virtio_pstore_config config;
> +
> + memcpy(&config, config_data, sizeof(struct virtio_pstore_config));
> +
> + dev->bufsize = le32_to_cpu(config.bufsize);
> +}
> +
> +static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
> +{
> + return f;
> +}
> +
> +static void pstore_get_directory(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> +
> + visit_type_str(v, name, &s->directory, errp);
> +}
> +
> +static void pstore_set_directory(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> + Error *local_err = NULL;
> + char *value;
> +
> + visit_type_str(v, name, &value, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + g_free(s->directory);
> + s->directory = value;
> +}
> +
> +static void pstore_release_directory(Object *obj, const char *name,
> + void *opaque)
> +{
> + VirtIOPstore *s = opaque;
> +
> + g_free(s->directory);
> + s->directory = NULL;
> +}
> +
> +static void pstore_get_bufsize(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> + uint64_t value = s->bufsize;
> +
> + visit_type_size(v, name, &value, errp);
> +}
> +
> +static void pstore_set_bufsize(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> + Error *error = NULL;
> + uint64_t value;
> +
> + visit_type_size(v, name, &value, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> +
> + if (value < 4096) {
> + error_report("Warning: too small buffer size: %"PRIu64, value);
> + }
> +
> + s->bufsize = value;
> +}
> +
> +static void pstore_get_console(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> + bool value = s->console;
> +
> + visit_type_bool(v, name, &value, errp);
> +}
> +
> +static void pstore_set_console(Object *obj, Visitor *v,
> + const char *name, void *opaque,
> + Error **errp)
> +{
> + VirtIOPstore *s = opaque;
> + Error *error = NULL;
> + bool value;
> +
> + visit_type_bool(v, name, &value, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> +
> + s->console = value;
> +}
> +
> +static Property virtio_pstore_properties[] = {
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void virtio_pstore_instance_init(Object *obj)
> +{
> + VirtIOPstore *s = VIRTIO_PSTORE(obj);
> +
> + object_property_add(obj, "directory", "str",
> + pstore_get_directory, pstore_set_directory,
> + pstore_release_directory, s, NULL);
> + object_property_add(obj, "bufsize", "size",
> + pstore_get_bufsize, pstore_set_bufsize, NULL, s, NULL);
> + object_property_add(obj, "console", "bool",
> + pstore_get_console, pstore_set_console, NULL, s, NULL);
> +}
> +
> +static void virtio_pstore_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> +
> + dc->props = virtio_pstore_properties;
> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + vdc->realize = virtio_pstore_device_realize;
> + vdc->unrealize = virtio_pstore_device_unrealize;
> + vdc->get_config = virtio_pstore_get_config;
> + vdc->set_config = virtio_pstore_set_config;
> + vdc->get_features = get_features;
> +}
> +
> +static const TypeInfo virtio_pstore_info = {
> + .name = TYPE_VIRTIO_PSTORE,
> + .parent = TYPE_VIRTIO_DEVICE,
> + .instance_size = sizeof(VirtIOPstore),
> + .instance_init = virtio_pstore_instance_init,
> + .class_init = virtio_pstore_class_init,
> +};
> +
> +static void virtio_register_types(void)
> +{
> + type_register_static(&virtio_pstore_info);
> +}
> +
> +type_init(virtio_register_types)
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 74d797d..000e1e9 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -79,6 +79,7 @@
> #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
> #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
> #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> +#define PCI_DEVICE_ID_VIRTIO_PSTORE 0x100a
>
> #define PCI_VENDOR_ID_REDHAT 0x1b36
> #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> diff --git a/include/hw/virtio/virtio-pstore.h b/include/hw/virtio/virtio-pstore.h
> new file mode 100644
> index 0000000..d188a48
> --- /dev/null
> +++ b/include/hw/virtio/virtio-pstore.h
> @@ -0,0 +1,34 @@
> +/*
> + * Virtio Pstore Support
> + *
> + * Authors:
> + * Namhyung Kim <namhyung@gmail.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef _QEMU_VIRTIO_PSTORE_H
> +#define _QEMU_VIRTIO_PSTORE_H
> +
> +#include "standard-headers/linux/virtio_pstore.h"
> +#include "hw/virtio/virtio.h"
> +#include "hw/pci/pci.h"
> +
> +#define TYPE_VIRTIO_PSTORE "virtio-pstore-device"
> +#define VIRTIO_PSTORE(obj) \
> + OBJECT_CHECK(VirtIOPstore, (obj), TYPE_VIRTIO_PSTORE)
> +
> +typedef struct VirtIOPstore {
> + VirtIODevice parent_obj;
> + VirtQueue *vq[2];
> + char *directory;
> + uint64_t id;
> + uint64_t console_id;
> + DIR *dirp;
> + uint64_t bufsize;
> + bool console;
> +} VirtIOPstore;
> +
> +#endif
> diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
> index 77925f5..c72a9ab 100644
> --- a/include/standard-headers/linux/virtio_ids.h
> +++ b/include/standard-headers/linux/virtio_ids.h
> @@ -41,5 +41,6 @@
> #define VIRTIO_ID_CAIF 12 /* Virtio caif */
> #define VIRTIO_ID_GPU 16 /* virtio GPU */
> #define VIRTIO_ID_INPUT 18 /* virtio input */
> +#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
>
> #endif /* _LINUX_VIRTIO_IDS_H */
> diff --git a/include/standard-headers/linux/virtio_pstore.h b/include/standard-headers/linux/virtio_pstore.h
> new file mode 100644
> index 0000000..b893d15
> --- /dev/null
> +++ b/include/standard-headers/linux/virtio_pstore.h
> @@ -0,0 +1,80 @@
> +#ifndef _LINUX_VIRTIO_PSTORE_H
> +#define _LINUX_VIRTIO_PSTORE_H
> +/* This header is BSD licensed so anyone can use the definitions to implement
> + * compatible drivers/servers.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of IBM nor the names of its contributors
> + * may be used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE. */
> +#include "standard-headers/linux/types.h"
> +#include "standard-headers/linux/virtio_types.h"
> +#include "standard-headers/linux/virtio_ids.h"
> +#include "standard-headers/linux/virtio_config.h"
> +
> +#define VIRTIO_PSTORE_CMD_NULL 0
> +#define VIRTIO_PSTORE_CMD_OPEN 1
> +#define VIRTIO_PSTORE_CMD_READ 2
> +#define VIRTIO_PSTORE_CMD_WRITE 3
> +#define VIRTIO_PSTORE_CMD_ERASE 4
> +#define VIRTIO_PSTORE_CMD_CLOSE 5
> +
> +#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
> +#define VIRTIO_PSTORE_TYPE_DMESG 1
> +#define VIRTIO_PSTORE_TYPE_CONSOLE 2
> +
> +#define VIRTIO_PSTORE_FL_COMPRESSED 1
> +
> +#define VIRTIO_PSTORE_CONFIG_FL_CONSOLE (1 << 0)
> +
> +struct virtio_pstore_req {
> + __virtio16 cmd;
> + __virtio16 type;
> + __virtio32 flags;
> + __virtio64 id;
> + __virtio32 count;
> + __virtio32 reserved;
> +};
> +
> +struct virtio_pstore_res {
> + __virtio16 cmd;
> + __virtio16 type;
> + __virtio32 ret;
> +};
> +
> +struct virtio_pstore_fileinfo {
> + __virtio64 id;
> + __virtio32 count;
> + __virtio16 type;
> + __virtio16 unused;
> + __virtio32 flags;
> + __virtio32 len;
> + __virtio64 time_sec;
> + __virtio32 time_nsec;
> + __virtio32 reserved;
> +};
> +
> +struct virtio_pstore_config {
> + __virtio32 bufsize;
> + __virtio32 flags;
> +};
> +
> +#endif /* _LINUX_VIRTIO_PSTORE_H */
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index e19617f..e1df5a9 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -73,6 +73,7 @@ static const QDevAlias qdev_alias_table[] = {
> { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
> { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> + { "virtio-pstore-pci", "virtio-pstore" },
> { }
> };
>
> --
> 2.8.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Michael S. Tsirkin @ 2016-07-27 23:05 UTC (permalink / raw)
To: Dave Hansen
Cc: virtio-dev, kvm, qemu-devel, Amit Shah, Liang Li, linux-kernel,
virtualization, linux-mm, Vlastimil Babka, Paolo Bonzini,
Andrew Morton, Mel Gorman, dgilbert
In-Reply-To: <579932D9.6000106@intel.com>
On Wed, Jul 27, 2016 at 03:16:57PM -0700, Dave Hansen wrote:
> On 07/27/2016 03:05 PM, Michael S. Tsirkin wrote:
> > On Wed, Jul 27, 2016 at 09:40:56AM -0700, Dave Hansen wrote:
> >> On 07/26/2016 06:23 PM, Liang Li wrote:
> >>> + for_each_migratetype_order(order, t) {
> >>> + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> >>> + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> >>> + if (pfn >= start_pfn && pfn <= end_pfn) {
> >>> + page_num = 1UL << order;
> >>> + if (pfn + page_num > end_pfn)
> >>> + page_num = end_pfn - pfn;
> >>> + bitmap_set(bitmap, pfn - start_pfn, page_num);
> >>> + }
> >>> + }
> >>> + }
> >>
> >> Nit: The 'page_num' nomenclature really confused me here. It is the
> >> number of bits being set in the bitmap. Seems like calling it nr_pages
> >> or num_pages would be more appropriate.
> >>
> >> Isn't this bitmap out of date by the time it's send up to the
> >> hypervisor? Is there something that makes the inaccuracy OK here?
> >
> > Yes. Calling these free pages is unfortunate. It's likely to confuse
> > people thinking they can just discard these pages.
> >
> > Hypervisor sends a request. We respond with this list of pages, and
> > the guarantee hypervisor needs is that these were free sometime between request
> > and response, so they are safe to free if they are unmodified
> > since the request. hypervisor can detect modifications so
> > it can detect modifications itself and does not need guest help.
>
> Ahh, that makes sense.
>
> So the hypervisor is trying to figure out: "Which pages do I move?". It
> wants to know which pages the guest thinks have good data and need to
> move. But, the list of free pages is (likely) smaller than the list of
> pages with good data, so it asks for that instead.
>
> A write to a page means that it has valuable data, regardless of whether
> it was in the free list or not.
>
> The hypervisor only skips moving pages that were free *and* were never
> written to.
Right - except never is a long time, so we just need it "since the request
was received".
> So we never lose data, even if this "get free page info"
> stuff is totally out of date.
So if you include pages that were written to before the request
then yes data will be lost. This is why we do this scan
after we get the request and not e.g. on boot :)
> The patch description and code comments are, um, a _bit_ light for this
> level of subtlety. :)
Add to that, for any page it is safe to skip and not add it to list.
So the requirement is for when page must *not* be on this list:
it must not be there if it is needed by guest but was not
modified since the request.
Calling it "free" will just keep confusing people. Either use the
verbose "free or modified" or invent a new word like "discardable" and
add a comment explaining that page is always discardable unless it's
content is needed by Linux but was not modified since the request.
--
MST
^ permalink raw reply
* Re: [PATCH v2 repost 3/7] mm: add a function to get the max pfn
From: Dave Hansen @ 2016-07-27 22:52 UTC (permalink / raw)
To: Michael S. Tsirkin, Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <20160728010729-mutt-send-email-mst@kernel.org>
On 07/27/2016 03:08 PM, Michael S. Tsirkin wrote:
>> > +unsigned long get_max_pfn(void)
>> > +{
>> > + return max_pfn;
>> > +}
>> > +EXPORT_SYMBOL(get_max_pfn);
>> > +
>
> This needs a coment that this can change at any time.
> So it's only good as a hint e.g. for sizing data structures.
Or, if we limit the batches to 1GB like you suggested earlier, then we
might not even need this exported. It would mean that in the worst
case, we temporarily waste 28k out of the 32k allocation for a small VM
that had <128MB of memory.
That seems like a small price to pay for not having to track max_pfn
anywhere.
^ permalink raw reply
* [PATCH v4] virtio: new feature to detect IOMMU device quirk
From: Michael S. Tsirkin @ 2016-07-27 22:50 UTC (permalink / raw)
To: linux-kernel; +Cc: Christoph Hellwig, virtualization
The interaction between virtio and IOMMUs is messy.
On most systems with virtio, physical addresses match bus addresses,
and it doesn't particularly matter which one we use to program
the device.
On some systems, including Xen and any system with a physical device
that speaks virtio behind a physical IOMMU, we must program the IOMMU
for virtio DMA to work at all.
On other systems, including SPARC and PPC64, virtio-pci devices are
enumerated as though they are behind an IOMMU, but the virtio host
ignores the IOMMU, so we must either pretend that the IOMMU isn't
there or somehow map everything as the identity.
Add a feature bit to detect that quirk: VIRTIO_F_IOMMU_PLATFORM.
Any device with this feature bit set to 0 needs a quirk and has to be
passed physical addresses (as opposed to bus addresses) even though
the device is behind an IOMMU.
Note: it has to be a per-device quirk because for example, there could
be a mix of passed-through and virtual virtio devices. As another
example, some devices could be implemented by an out of process
hypervisor backend (in case of qemu vhost, or vhost-user) and so support
for an IOMMU needs to be coded up separately.
It would be cleanest to handle this in IOMMU core code, but that needs
per-device DMA ops. This should not be hard to do, and it will serve
to replace the existing Xen hacks, but doesn't seem practical for 4.8
and we don't want to defer hardware support until that's ready.
Add a TODO so we remember to clean it up for 4.9.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v3:
In response to Christoph's comments, add TODO so we remember to
move this into DMA core. Definitely something we want to do but doesn't
seem practical for this release cycle.
include/uapi/linux/virtio_config.h | 4 +++-
drivers/virtio/virtio_ring.c | 15 ++++++++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/virtio_config.h b/include/uapi/linux/virtio_config.h
index 4cb65bb..3f6195e 100644
--- a/include/uapi/linux/virtio_config.h
+++ b/include/uapi/linux/virtio_config.h
@@ -49,7 +49,7 @@
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 33
+#define VIRTIO_TRANSPORT_F_END 34
#ifndef VIRTIO_CONFIG_NO_LEGACY
/* Do we get callbacks when the ring is completely used, even if we've
@@ -63,4 +63,6 @@
/* v1.0 compliant. */
#define VIRTIO_F_VERSION_1 32
+/* Do not bypass the IOMMU (if configured) */
+#define VIRTIO_F_IOMMU_PLATFORM 33
#endif /* _UAPI_LINUX_VIRTIO_CONFIG_H */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index ca6bfdd..a0571f6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -117,7 +117,10 @@ struct vring_virtqueue {
#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
/*
- * The interaction between virtio and a possible IOMMU is a mess.
+ * Modern virtio devices might set feature bits to specify whether
+ * they use the platform IOMMU. If there, just use the DMA API.
+ *
+ * If not there, the interaction between virtio and DMA API is messy.
*
* On most systems with virtio, physical addresses match bus addresses,
* and it doesn't particularly matter whether we use the DMA API.
@@ -133,10 +136,18 @@ struct vring_virtqueue {
*
* For the time being, we preserve historic behavior and bypass the DMA
* API.
+ *
+ * TODO: install a per-device DMA ops structure that does the right thing
+ * taking into account all the above quirks, and use the DMA API
+ * unconditionally on data path.
*/
static bool vring_use_dma_api(struct virtio_device *vdev)
{
+ if (virtio_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM))
+ return true;
+
+ /* Otherwise, we are left to guess. */
/*
* In theory, it's possible to have a buggy QEMU-supposed
* emulated Q35 IOMMU and Xen enabled at the same time. On
@@ -1099,6 +1110,8 @@ void vring_transport_features(struct virtio_device *vdev)
break;
case VIRTIO_F_VERSION_1:
break;
+ case VIRTIO_F_IOMMU_PLATFORM:
+ break;
default:
/* We don't understand this bit. */
__virtio_clear_bit(vdev, i);
--
MST
^ permalink raw reply related
* Re: [RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2)
From: Michael S. Tsirkin @ 2016-07-27 22:18 UTC (permalink / raw)
To: Namhyung Kim
Cc: Tony Luck, Kees Cook, kvm, Radim Krčmář, LKML,
Anton Vorontsov, Will Deacon, qemu-devel, Steven Rostedt,
virtualization, Minchan Kim, Anthony Liguori, Colin Cross,
Paolo Bonzini, Ingo Molnar
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
On Thu, Jul 28, 2016 at 12:08:24AM +0900, Namhyung Kim wrote:
> Hello,
>
> This is v2 of the virtio-pstore work. In this patchset I addressed
> most of feedbacks from previous version. Limiting disk size is not
> implemented yet.
For some reason, only parts of the patchset were received.
Pls post all patches to all lists.
If you are changing the virtio interface with host,
like a new device, they you must copy the virtio TC
so make sure there are no objections from there.
https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio#feedback
> * changes in v2)
> - update VIRTIO_ID_PSTORE to 22 (Cornelia, Stefan)
> - make buffer size configurable (Cornelia)
> - support PSTORE_TYPE_CONSOLE (Kees)
> - use separate virtqueues for read and write
> - support concurrent async write
> - manage pstore (file) id in device side
> - fix various mistakes in qemu device (Stefan)
>
> It started from the fact that dumping ftrace buffer at kernel
> oops/panic takes too much time. Although there's a way to reduce the
> size of the original data, sometimes I want to have the information as
> many as possible. Maybe kexec/kdump can solve this problem but it
> consumes some portion of guest memory so I'd like to avoid it. And I
> know the qemu + crashtool can dump and analyze the whole guest memory
> including the ftrace buffer without wasting guest memory, but it adds
> one more layer and has some limitation as an out-of-tree tool like not
> being in sync with the kernel changes.
>
> So I think it'd be great using the pstore interface to dump guest
> kernel data on the host. One can read the data on the host directly
> or on the guest (at the next boot) using pstore filesystem as usual.
> While this patchset only implements dumping kernel log buffer, it can
> be extended to have ftrace buffer and probably some more..
>
> The patch 0001-0003 are preparation for pstore to support virtio
> device which requires async write. The patch 0004 implements virtio
> pstore driver. It has two virt queue for (sync) read and (async)
> write, pstore buffer and io request and response structure. The
> virtio_pstore_req struct is to give information about the current
> pstore operation. The result will be written to the virtio_pstore_res
> struct. For read operation it also uses virtio_pstore_fileinfo struct.
>
> The patch 0005 adds support for PSTORE_TYPE_CONSOLE which was
> requested by Kees. The console data is appended to a single file for
> now.
>
> The patch 0006 and 0007 implement virtio-pstore legacy PCI device on
> qemu-kvm and kvmtool respectively. I referenced virtio-baloon and
> virtio-rng implementations and I don't know whether kvmtool supports
> modern virtio 1.0+ spec. Other transports might be supported later.
>
> For example, using virtio-pstore on qemu looks like below:
>
> $ qemu-system-x86_64 -enable-kvm -device virtio-pstore,directory=xxx
>
> When guest kernel gets panic the log messages will be saved under the
> xxx directory.
>
> $ ls xxx
> dmesg-1.enc.z dmesg-2.enc.z
>
> As you can see the pstore subsystem compresses the log data using zlib
> (now supports lzo and lz4 too). The data can be extracted with the
> following command:
>
> $ cat xxx/dmesg-1.enc.z | \
> > python -c 'import sys, zlib; print(zlib.decompress(sys.stdin.read()))'
> Oops#1 Part1
> <5>[ 0.000000] Linux version 4.6.0kvm+ (namhyung@danjae) (gcc version 5.3.0 (GCC) ) #145 SMP Mon Jul 18 10:22:45 KST 2016
> <6>[ 0.000000] Command line: root=/dev/vda console=ttyS0
> <6>[ 0.000000] x86/fpu: Legacy x87 FPU detected.
> <6>[ 0.000000] x86/fpu: Using 'eager' FPU context switches.
> <6>[ 0.000000] e820: BIOS-provided physical RAM map:
> <6>[ 0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> <6>[ 0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> <6>[ 0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> <6>[ 0.000000] BIOS-e820: [mem 0x0000000000100000-0x0000000007fddfff] usable
> <6>[ 0.000000] BIOS-e820: [mem 0x0000000007fde000-0x0000000007ffffff] reserved
> <6>[ 0.000000] BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] reserved
> <6>[ 0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> <6>[ 0.000000] NX (Execute Disable) protection: active
> <6>[ 0.000000] SMBIOS 2.8 present.
> <7>[ 0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> ...
>
> To enable PSTORE_TYPE_CONSOLE, add 'console=true' to virtio-pstore
> device option. Also 'bufsize' option can set different size for
> pstore buffer (default is 16K). Maybe we can add a config option to
> control the compression later.
>
> Currently the kvmtool doesn't support any options except the directory
> the pstore saves the logs.
>
>
> Namhyung Kim (7):
> pstore: Split pstore fragile flags
> pstore/ram: Set pstore flags dynamically
> pstore: Manage buffer position for async write
> virtio: Basic implementation of virtio pstore driver
> virtio-pstore: Support PSTORE_TYPE_CONSOLE
> qemu: Implement virtio-pstore device
> kvmtool: Implement virtio-pstore device
>
> drivers/acpi/apei/erst.c | 2 +-
> drivers/firmware/efi/efi-pstore.c | 4 +-
> drivers/virtio/Kconfig | 10 +
> drivers/virtio/Makefile | 1 +
> drivers/virtio/virtio_pstore.c | 421 +++++++++++++++++++++
> fs/pstore/platform.c | 65 +++-
> fs/pstore/ram.c | 8 +
> include/linux/pstore.h | 9 +-
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/virtio_ids.h | 1 +
> include/uapi/linux/virtio_pstore.h | 78 +++-
> 11 files changed, 580 insertions(+), 20 deletions(-)
> create mode 100644 drivers/virtio/virtio_pstore.c
> create mode 100644 include/uapi/linux/virtio_pstore.h
>
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: kvm@vger.kernel.org
> Cc: qemu-devel@nongnu.org
> Cc: virtualization@lists.linux-foundation.org
>
> Thanks,
> Namhyung
>
>
> --
> 2.8.0
>
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Dave Hansen @ 2016-07-27 22:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, kvm, qemu-devel, Amit Shah, Liang Li, linux-kernel,
virtualization, linux-mm, Vlastimil Babka, Paolo Bonzini,
Andrew Morton, Mel Gorman, dgilbert
In-Reply-To: <20160728010030-mutt-send-email-mst@kernel.org>
On 07/27/2016 03:05 PM, Michael S. Tsirkin wrote:
> On Wed, Jul 27, 2016 at 09:40:56AM -0700, Dave Hansen wrote:
>> On 07/26/2016 06:23 PM, Liang Li wrote:
>>> + for_each_migratetype_order(order, t) {
>>> + list_for_each(curr, &zone->free_area[order].free_list[t]) {
>>> + pfn = page_to_pfn(list_entry(curr, struct page, lru));
>>> + if (pfn >= start_pfn && pfn <= end_pfn) {
>>> + page_num = 1UL << order;
>>> + if (pfn + page_num > end_pfn)
>>> + page_num = end_pfn - pfn;
>>> + bitmap_set(bitmap, pfn - start_pfn, page_num);
>>> + }
>>> + }
>>> + }
>>
>> Nit: The 'page_num' nomenclature really confused me here. It is the
>> number of bits being set in the bitmap. Seems like calling it nr_pages
>> or num_pages would be more appropriate.
>>
>> Isn't this bitmap out of date by the time it's send up to the
>> hypervisor? Is there something that makes the inaccuracy OK here?
>
> Yes. Calling these free pages is unfortunate. It's likely to confuse
> people thinking they can just discard these pages.
>
> Hypervisor sends a request. We respond with this list of pages, and
> the guarantee hypervisor needs is that these were free sometime between request
> and response, so they are safe to free if they are unmodified
> since the request. hypervisor can detect modifications so
> it can detect modifications itself and does not need guest help.
Ahh, that makes sense.
So the hypervisor is trying to figure out: "Which pages do I move?". It
wants to know which pages the guest thinks have good data and need to
move. But, the list of free pages is (likely) smaller than the list of
pages with good data, so it asks for that instead.
A write to a page means that it has valuable data, regardless of whether
it was in the free list or not.
The hypervisor only skips moving pages that were free *and* were never
written to. So we never lose data, even if this "get free page info"
stuff is totally out of date.
The patch description and code comments are, um, a _bit_ light for this
level of subtlety. :)
^ permalink raw reply
* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Michael S. Tsirkin @ 2016-07-27 22:13 UTC (permalink / raw)
To: Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <1469582616-5729-7-git-send-email-liang.z.li@intel.com>
On Wed, Jul 27, 2016 at 09:23:35AM +0800, Liang Li wrote:
> Save the free page info into a page bitmap, will be used in virtio
> balloon device driver.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> ---
> mm/page_alloc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7da61ad..3ad8b10 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4523,6 +4523,52 @@ unsigned long get_max_pfn(void)
> }
> EXPORT_SYMBOL(get_max_pfn);
>
> +static void mark_free_pages_bitmap(struct zone *zone, unsigned long start_pfn,
> + unsigned long end_pfn, unsigned long *bitmap, unsigned long len)
> +{
> + unsigned long pfn, flags, page_num;
> + unsigned int order, t;
> + struct list_head *curr;
> +
> + if (zone_is_empty(zone))
> + return;
> + end_pfn = min(start_pfn + len, end_pfn);
> + spin_lock_irqsave(&zone->lock, flags);
> +
> + for_each_migratetype_order(order, t) {
Why not do each order separately? This way you can
use a single bit to pass a huge page to host.
Not a requirement but hey.
Alternatively (and maybe that is a better idea0
if you wanted to, you could just skip lone 4K pages.
It's not clear that they are worth bothering with.
Add a flag to start with some reasonably large order and go from there.
> + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> + if (pfn >= start_pfn && pfn <= end_pfn) {
> + page_num = 1UL << order;
> + if (pfn + page_num > end_pfn)
> + page_num = end_pfn - pfn;
> + bitmap_set(bitmap, pfn - start_pfn, page_num);
> + }
> + }
> + }
> +
> + spin_unlock_irqrestore(&zone->lock, flags);
> +}
> +
> +int get_free_pages(unsigned long start_pfn, unsigned long end_pfn,
> + unsigned long *bitmap, unsigned long len)
> +{
> + struct zone *zone;
> + int ret = 0;
> +
> + if (bitmap == NULL || start_pfn > end_pfn || start_pfn >= max_pfn)
> + return 0;
> + if (end_pfn < max_pfn)
> + ret = 1;
> + if (end_pfn >= max_pfn)
> + ret = 0;
> +
> + for_each_populated_zone(zone)
> + mark_free_pages_bitmap(zone, start_pfn, end_pfn, bitmap, len);
> + return ret;
> +}
> +EXPORT_SYMBOL(get_free_pages);
> +
> static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
> {
> zoneref->zone = zone;
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 repost 3/7] mm: add a function to get the max pfn
From: Michael S. Tsirkin @ 2016-07-27 22:08 UTC (permalink / raw)
To: Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <1469582616-5729-4-git-send-email-liang.z.li@intel.com>
On Wed, Jul 27, 2016 at 09:23:32AM +0800, Liang Li wrote:
> Expose the function to get the max pfn, so it can be used in the
> virtio-balloon device driver.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> ---
> mm/page_alloc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8b3e134..7da61ad 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4517,6 +4517,12 @@ void show_free_areas(unsigned int filter)
> show_swap_cache_info();
> }
>
> +unsigned long get_max_pfn(void)
> +{
> + return max_pfn;
> +}
> +EXPORT_SYMBOL(get_max_pfn);
> +
This needs a coment that this can change at any time.
So it's only good as a hint e.g. for sizing data structures.
> static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
> {
> zoneref->zone = zone;
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 repost 4/7] virtio-balloon: speed up inflate/deflate process
From: Michael S. Tsirkin @ 2016-07-27 22:07 UTC (permalink / raw)
To: Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <1469582616-5729-5-git-send-email-liang.z.li@intel.com>
On Wed, Jul 27, 2016 at 09:23:33AM +0800, Liang Li wrote:
> The implementation of the current virtio-balloon is not very
> efficient, the time spends on different stages of inflating
> the balloon to 7GB of a 8GB idle guest:
>
> a. allocating pages (6.5%)
> b. sending PFNs to host (68.3%)
> c. address translation (6.1%)
> d. madvise (19%)
>
> It takes about 4126ms for the inflating process to complete.
> Debugging shows that the bottle neck are the stage b and stage d.
>
> If using a bitmap to send the page info instead of the PFNs, we
> can reduce the overhead in stage b quite a lot. Furthermore, we
> can do the address translation and call madvise() with a bulk of
> RAM pages, instead of the current page per page way, the overhead
> of stage c and stage d can also be reduced a lot.
>
> This patch is the kernel side implementation which is intended to
> speed up the inflating & deflating process by adding a new feature
> to the virtio-balloon device. With this new feature, inflating the
> balloon to 7GB of a 8GB idle guest only takes 590ms, the
> performance improvement is about 85%.
>
> TODO: optimize stage a by allocating/freeing a chunk of pages
> instead of a single page at a time.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/virtio/virtio_balloon.c | 184 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 162 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8d649a2..2d18ff6 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -41,10 +41,28 @@
> #define OOM_VBALLOON_DEFAULT_PAGES 256
> #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
>
> +/*
> + * VIRTIO_BALLOON_PFNS_LIMIT is used to limit the size of page bitmap
> + * to prevent a very large page bitmap, there are two reasons for this:
> + * 1) to save memory.
> + * 2) allocate a large bitmap may fail.
> + *
> + * The actual limit of pfn is determined by:
> + * pfn_limit = min(max_pfn, VIRTIO_BALLOON_PFNS_LIMIT);
> + *
> + * If system has more pages than VIRTIO_BALLOON_PFNS_LIMIT, we will scan
> + * the page list and send the PFNs with several times. To reduce the
> + * overhead of scanning the page list. VIRTIO_BALLOON_PFNS_LIMIT should
> + * be set with a value which can cover most cases.
> + */
> +#define VIRTIO_BALLOON_PFNS_LIMIT ((32 * (1ULL << 30)) >> PAGE_SHIFT) /* 32GB */
> +
> static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
>
> +extern unsigned long get_max_pfn(void);
> +
Please just include the correct header. No need for this hackery.
> struct virtio_balloon {
> struct virtio_device *vdev;
> struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> @@ -62,6 +80,15 @@ struct virtio_balloon {
>
> /* Number of balloon pages we've told the Host we're not using. */
> unsigned int num_pages;
> + /* Pointer of the bitmap header. */
> + void *bmap_hdr;
> + /* Bitmap and length used to tell the host the pages */
> + unsigned long *page_bitmap;
> + unsigned long bmap_len;
> + /* Pfn limit */
> + unsigned long pfn_limit;
> + /* Used to record the processed pfn range */
> + unsigned long min_pfn, max_pfn, start_pfn, end_pfn;
> /*
> * The pages we've told the Host we're not using are enqueued
> * at vb_dev_info->pages list.
> @@ -105,12 +132,45 @@ static void balloon_ack(struct virtqueue *vq)
> wake_up(&vb->acked);
> }
>
> +static inline void init_pfn_range(struct virtio_balloon *vb)
> +{
> + vb->min_pfn = ULONG_MAX;
> + vb->max_pfn = 0;
> +}
> +
> +static inline void update_pfn_range(struct virtio_balloon *vb,
> + struct page *page)
> +{
> + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> +
> + if (balloon_pfn < vb->min_pfn)
> + vb->min_pfn = balloon_pfn;
> + if (balloon_pfn > vb->max_pfn)
> + vb->max_pfn = balloon_pfn;
> +}
> +
> static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
> {
> struct scatterlist sg;
> unsigned int len;
>
> - sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP)) {
> + struct balloon_bmap_hdr *hdr = vb->bmap_hdr;
> + unsigned long bmap_len;
> +
> + /* cmd and req_id are not used here, set them to 0 */
> + hdr->cmd = cpu_to_virtio16(vb->vdev, 0);
> + hdr->page_shift = cpu_to_virtio16(vb->vdev, PAGE_SHIFT);
> + hdr->reserved = cpu_to_virtio16(vb->vdev, 0);
> + hdr->req_id = cpu_to_virtio64(vb->vdev, 0);
> + hdr->start_pfn = cpu_to_virtio64(vb->vdev, vb->start_pfn);
> + bmap_len = min(vb->bmap_len,
> + (vb->end_pfn - vb->start_pfn) / BITS_PER_BYTE);
> + hdr->bmap_len = cpu_to_virtio64(vb->vdev, bmap_len);
> + sg_init_one(&sg, hdr,
> + sizeof(struct balloon_bmap_hdr) + bmap_len);
> + } else
> + sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
>
> /* We should always be able to add one buffer to an empty queue. */
> virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> @@ -118,7 +178,6 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
>
> /* When host has read buffer, this completes via balloon_ack */
> wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> -
> }
>
> static void set_page_pfns(struct virtio_balloon *vb,
> @@ -133,13 +192,53 @@ static void set_page_pfns(struct virtio_balloon *vb,
> page_to_balloon_pfn(page) + i);
> }
>
> -static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> +static void set_page_bitmap(struct virtio_balloon *vb,
> + struct list_head *pages, struct virtqueue *vq)
> +{
> + unsigned long pfn;
> + struct page *page;
> + bool found;
> +
> + vb->min_pfn = rounddown(vb->min_pfn, BITS_PER_LONG);
> + vb->max_pfn = roundup(vb->max_pfn, BITS_PER_LONG);
> + for (pfn = vb->min_pfn; pfn < vb->max_pfn;
> + pfn += vb->pfn_limit) {
> + vb->start_pfn = pfn + vb->pfn_limit;
> + vb->end_pfn = pfn;
> + memset(vb->page_bitmap, 0, vb->bmap_len);
> + found = false;
> + list_for_each_entry(page, pages, lru) {
> + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> +
> + if (balloon_pfn < pfn ||
> + balloon_pfn >= pfn + vb->pfn_limit)
> + continue;
> + set_bit(balloon_pfn - pfn, vb->page_bitmap);
> + if (balloon_pfn > vb->end_pfn)
> + vb->end_pfn = balloon_pfn;
> + if (balloon_pfn < vb->start_pfn)
> + vb->start_pfn = balloon_pfn;
> + found = true;
> + }
> + if (found) {
> + vb->start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> + vb->end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> + tell_host(vb, vq);
> + }
> + }
> +}
> +
> +static unsigned int fill_balloon(struct virtio_balloon *vb, size_t num,
> + bool use_bmap)
> {
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> - unsigned num_allocated_pages;
> + unsigned int num_allocated_pages;
>
> - /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + if (use_bmap)
> + init_pfn_range(vb);
> + else
> + /* We can only do one array worth at a time. */
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> for (vb->num_pfns = 0; vb->num_pfns < num;
> @@ -154,7 +253,10 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> msleep(200);
> break;
> }
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (use_bmap)
> + update_pfn_range(vb, page);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> if (!virtio_has_feature(vb->vdev,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> @@ -163,8 +265,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>
> num_allocated_pages = vb->num_pfns;
> /* Did we get any? */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->inflate_vq);
> + if (vb->num_pfns != 0) {
> + if (use_bmap)
> + set_page_bitmap(vb, &vb_dev_info->pages,
> + vb->inflate_vq);
> + else
> + tell_host(vb, vb->inflate_vq);
> + }
> mutex_unlock(&vb->balloon_lock);
>
> return num_allocated_pages;
> @@ -184,15 +291,19 @@ static void release_pages_balloon(struct virtio_balloon *vb,
> }
> }
>
> -static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> +static unsigned int leak_balloon(struct virtio_balloon *vb, size_t num,
> + bool use_bmap)
> {
> - unsigned num_freed_pages;
> + unsigned int num_freed_pages;
> struct page *page;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> LIST_HEAD(pages);
>
> - /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + if (use_bmap)
> + init_pfn_range(vb);
> + else
> + /* We can only do one array worth at a time. */
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> for (vb->num_pfns = 0; vb->num_pfns < num;
> @@ -200,7 +311,10 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (use_bmap)
> + update_pfn_range(vb, page);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
> @@ -211,9 +325,14 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
> * is true, we *have* to do it in this order
> */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->deflate_vq);
> - release_pages_balloon(vb, &pages);
> + if (vb->num_pfns != 0) {
> + if (use_bmap)
> + set_page_bitmap(vb, &pages, vb->deflate_vq);
> + else
> + tell_host(vb, vb->deflate_vq);
> +
> + release_pages_balloon(vb, &pages);
> + }
> mutex_unlock(&vb->balloon_lock);
> return num_freed_pages;
> }
> @@ -347,13 +466,15 @@ static int virtballoon_oom_notify(struct notifier_block *self,
> struct virtio_balloon *vb;
> unsigned long *freed;
> unsigned num_freed_pages;
> + bool use_bmap;
>
> vb = container_of(self, struct virtio_balloon, nb);
> if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> return NOTIFY_OK;
>
> freed = parm;
> - num_freed_pages = leak_balloon(vb, oom_pages);
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> + num_freed_pages = leak_balloon(vb, oom_pages, use_bmap);
> update_balloon_size(vb);
> *freed += num_freed_pages;
>
> @@ -373,15 +494,17 @@ static void update_balloon_size_func(struct work_struct *work)
> {
> struct virtio_balloon *vb;
> s64 diff;
> + bool use_bmap;
>
> vb = container_of(work, struct virtio_balloon,
> update_balloon_size_work);
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> diff = towards_target(vb);
>
> if (diff > 0)
> - diff -= fill_balloon(vb, diff);
> + diff -= fill_balloon(vb, diff, use_bmap);
> else if (diff < 0)
> - diff += leak_balloon(vb, -diff);
> + diff += leak_balloon(vb, -diff, use_bmap);
> update_balloon_size(vb);
>
> if (diff)
> @@ -489,7 +612,7 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> static int virtballoon_probe(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb;
> - int err;
> + int err, hdr_len;
>
> if (!vdev->config->get) {
> dev_err(&vdev->dev, "%s failure: config access disabled\n",
> @@ -508,6 +631,18 @@ static int virtballoon_probe(struct virtio_device *vdev)
> spin_lock_init(&vb->stop_update_lock);
> vb->stop_update = false;
> vb->num_pages = 0;
> + vb->pfn_limit = VIRTIO_BALLOON_PFNS_LIMIT;
> + vb->pfn_limit = min(vb->pfn_limit, get_max_pfn());
> + vb->bmap_len = ALIGN(vb->pfn_limit, BITS_PER_LONG) /
> + BITS_PER_BYTE + 2 * sizeof(unsigned long);
> + hdr_len = sizeof(struct balloon_bmap_hdr);
> + vb->bmap_hdr = kzalloc(hdr_len + vb->bmap_len, GFP_KERNEL);
> +
> + /* Clear the feature bit if memory allocation fails */
> + if (!vb->bmap_hdr)
> + __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> + else
> + vb->page_bitmap = vb->bmap_hdr + hdr_len;
> mutex_init(&vb->balloon_lock);
> init_waitqueue_head(&vb->acked);
> vb->vdev = vdev;
> @@ -541,9 +676,12 @@ out:
>
> static void remove_common(struct virtio_balloon *vb)
> {
> + bool use_bmap;
> +
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> /* There might be pages left in the balloon: free them. */
> while (vb->num_pages)
> - leak_balloon(vb, vb->num_pages);
> + leak_balloon(vb, vb->num_pages, use_bmap);
> update_balloon_size(vb);
>
> /* Now we reset the device so we can clean up the queues. */
> @@ -565,6 +703,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
> cancel_work_sync(&vb->update_balloon_stats_work);
>
> remove_common(vb);
> + kfree(vb->page_bitmap);
> kfree(vb);
> }
>
> @@ -603,6 +742,7 @@ static unsigned int features[] = {
> VIRTIO_BALLOON_F_MUST_TELL_HOST,
> VIRTIO_BALLOON_F_STATS_VQ,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
> + VIRTIO_BALLOON_F_PAGE_BITMAP,
> };
>
> static struct virtio_driver virtio_balloon_driver = {
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Michael S. Tsirkin @ 2016-07-27 22:05 UTC (permalink / raw)
To: Dave Hansen
Cc: virtio-dev, kvm, qemu-devel, Amit Shah, Liang Li, linux-kernel,
virtualization, linux-mm, Vlastimil Babka, Paolo Bonzini,
Andrew Morton, Mel Gorman, dgilbert
In-Reply-To: <5798E418.7080608@intel.com>
On Wed, Jul 27, 2016 at 09:40:56AM -0700, Dave Hansen wrote:
> On 07/26/2016 06:23 PM, Liang Li wrote:
> > + for_each_migratetype_order(order, t) {
> > + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> > + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> > + if (pfn >= start_pfn && pfn <= end_pfn) {
> > + page_num = 1UL << order;
> > + if (pfn + page_num > end_pfn)
> > + page_num = end_pfn - pfn;
> > + bitmap_set(bitmap, pfn - start_pfn, page_num);
> > + }
> > + }
> > + }
>
> Nit: The 'page_num' nomenclature really confused me here. It is the
> number of bits being set in the bitmap. Seems like calling it nr_pages
> or num_pages would be more appropriate.
>
> Isn't this bitmap out of date by the time it's send up to the
> hypervisor? Is there something that makes the inaccuracy OK here?
Yes. Calling these free pages is unfortunate. It's likely to confuse
people thinking they can just discard these pages.
Hypervisor sends a request. We respond with this list of pages, and
the guarantee hypervisor needs is that these were free sometime between request
and response, so they are safe to free if they are unmodified
since the request. hypervisor can detect modifications so
it can detect modifications itself and does not need guest help.
Maybe just call these "free if unmodified" and reflect this
everywhere - verbose but hey. Better naming suggestions would be
welcome.
--
MST
^ permalink raw reply
* Re: [PATCH v2 repost 7/7] virtio-balloon: tell host vm's free page info
From: Michael S. Tsirkin @ 2016-07-27 22:00 UTC (permalink / raw)
To: Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <1469582616-5729-8-git-send-email-liang.z.li@intel.com>
On Wed, Jul 27, 2016 at 09:23:36AM +0800, Liang Li wrote:
> Support the request for vm's free page information, response with
> a page bitmap. QEMU can make use of this free page bitmap to speed
> up live migration process by skipping process the free pages.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/virtio/virtio_balloon.c | 104 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 98 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 2d18ff6..5ca4ad3 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -62,10 +62,13 @@ module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
>
> extern unsigned long get_max_pfn(void);
> +extern int get_free_pages(unsigned long start_pfn, unsigned long end_pfn,
> + unsigned long *bitmap, unsigned long len);
> +
>
> struct virtio_balloon {
> struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *misc_vq;
>
> /* The balloon servicing is delegated to a freezable workqueue. */
> struct work_struct update_balloon_stats_work;
> @@ -89,6 +92,8 @@ struct virtio_balloon {
> unsigned long pfn_limit;
> /* Used to record the processed pfn range */
> unsigned long min_pfn, max_pfn, start_pfn, end_pfn;
> + /* Request header */
> + struct balloon_req_hdr req_hdr;
> /*
> * The pages we've told the Host we're not using are enqueued
> * at vb_dev_info->pages list.
> @@ -373,6 +378,49 @@ static void update_balloon_stats(struct virtio_balloon *vb)
> pages_to_bytes(available));
> }
>
> +static void update_free_pages_stats(struct virtio_balloon *vb,
why _stats?
> + unsigned long req_id)
> +{
> + struct scatterlist sg_in, sg_out;
> + unsigned long pfn = 0, bmap_len, max_pfn;
> + struct virtqueue *vq = vb->misc_vq;
> + struct balloon_bmap_hdr *hdr = vb->bmap_hdr;
> + int ret = 1;
> +
> + max_pfn = get_max_pfn();
> + mutex_lock(&vb->balloon_lock);
> + while (pfn < max_pfn) {
> + memset(vb->page_bitmap, 0, vb->bmap_len);
> + ret = get_free_pages(pfn, pfn + vb->pfn_limit,
> + vb->page_bitmap, vb->bmap_len * BITS_PER_BYTE);
> + hdr->cmd = cpu_to_virtio16(vb->vdev, BALLOON_GET_FREE_PAGES);
> + hdr->page_shift = cpu_to_virtio16(vb->vdev, PAGE_SHIFT);
> + hdr->req_id = cpu_to_virtio64(vb->vdev, req_id);
> + hdr->start_pfn = cpu_to_virtio64(vb->vdev, pfn);
> + bmap_len = vb->pfn_limit / BITS_PER_BYTE;
> + if (!ret) {
> + hdr->flag = cpu_to_virtio16(vb->vdev,
> + BALLOON_FLAG_DONE);
> + if (pfn + vb->pfn_limit > max_pfn)
> + bmap_len = (max_pfn - pfn) / BITS_PER_BYTE;
> + } else
> + hdr->flag = cpu_to_virtio16(vb->vdev,
> + BALLOON_FLAG_CONT);
> + hdr->bmap_len = cpu_to_virtio64(vb->vdev, bmap_len);
> + sg_init_one(&sg_out, hdr,
> + sizeof(struct balloon_bmap_hdr) + bmap_len);
Wait a second. This adds the same buffer multiple times in a loop.
We will overwrite the buffer without waiting for
hypervisor to process it. What did I miss?
> +
> + virtqueue_add_outbuf(vq, &sg_out, 1, vb, GFP_KERNEL);
this can fail. you want to maybe make sure vq has enough space
before you use it or check error and wait.
> + virtqueue_kick(vq);
why kick here within loop? wait until done. in fact kick
outside lock is better for smp.
> + pfn += vb->pfn_limit;
> + }
> +
> + sg_init_one(&sg_in, &vb->req_hdr, sizeof(vb->req_hdr));
> + virtqueue_add_inbuf(vq, &sg_in, 1, &vb->req_hdr, GFP_KERNEL);
> + virtqueue_kick(vq);
> + mutex_unlock(&vb->balloon_lock);
> +}
> +
> /*
> * While most virtqueues communicate guest-initiated requests to the hypervisor,
> * the stats queue operates in reverse. The driver initializes the virtqueue
> @@ -511,18 +559,49 @@ static void update_balloon_size_func(struct work_struct *work)
> queue_work(system_freezable_wq, work);
> }
>
> +static void misc_handle_rq(struct virtio_balloon *vb)
> +{
> + struct balloon_req_hdr *ptr_hdr;
> + unsigned int len;
> +
> + ptr_hdr = virtqueue_get_buf(vb->misc_vq, &len);
> + if (!ptr_hdr || len != sizeof(vb->req_hdr))
> + return;
> +
> + switch (ptr_hdr->cmd) {
> + case BALLOON_GET_FREE_PAGES:
> + update_free_pages_stats(vb, ptr_hdr->param);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void misc_request(struct virtqueue *vq)
> +{
> + struct virtio_balloon *vb = vq->vdev->priv;
> +
> + misc_handle_rq(vb);
> +}
> +
> static int init_vqs(struct virtio_balloon *vb)
> {
> - struct virtqueue *vqs[3];
> - vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
> - static const char * const names[] = { "inflate", "deflate", "stats" };
> + struct virtqueue *vqs[4];
> + vq_callback_t *callbacks[] = { balloon_ack, balloon_ack,
> + stats_request, misc_request };
> + static const char * const names[] = { "inflate", "deflate", "stats",
> + "misc" };
> int err, nvqs;
>
> /*
> * We expect two virtqueues: inflate and deflate, and
> * optionally stat.
> */
> - nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_MISC_VQ))
> + nvqs = 4;
Does misc vq depend on stats vq feature then? if yes please validate that.
> + else
> + nvqs = virtio_has_feature(vb->vdev,
> + VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
Replace that ? with else too pls.
> err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
> if (err)
> return err;
> @@ -543,6 +622,16 @@ static int init_vqs(struct virtio_balloon *vb)
> BUG();
> virtqueue_kick(vb->stats_vq);
> }
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_MISC_VQ)) {
> + struct scatterlist sg_in;
> +
> + vb->misc_vq = vqs[3];
> + sg_init_one(&sg_in, &vb->req_hdr, sizeof(vb->req_hdr));
> + if (virtqueue_add_inbuf(vb->misc_vq, &sg_in, 1,
> + &vb->req_hdr, GFP_KERNEL) < 0)
> + BUG();
> + virtqueue_kick(vb->misc_vq);
> + }
> return 0;
> }
>
> @@ -639,8 +728,10 @@ static int virtballoon_probe(struct virtio_device *vdev)
> vb->bmap_hdr = kzalloc(hdr_len + vb->bmap_len, GFP_KERNEL);
>
> /* Clear the feature bit if memory allocation fails */
> - if (!vb->bmap_hdr)
> + if (!vb->bmap_hdr) {
> __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> + __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_MISC_VQ);
> + }
> else
> vb->page_bitmap = vb->bmap_hdr + hdr_len;
> mutex_init(&vb->balloon_lock);
> @@ -743,6 +834,7 @@ static unsigned int features[] = {
> VIRTIO_BALLOON_F_STATS_VQ,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
> VIRTIO_BALLOON_F_PAGE_BITMAP,
> + VIRTIO_BALLOON_F_MISC_VQ,
> };
>
> static struct virtio_driver virtio_balloon_driver = {
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 repost 4/7] virtio-balloon: speed up inflate/deflate process
From: Michael S. Tsirkin @ 2016-07-27 21:39 UTC (permalink / raw)
To: Dave Hansen
Cc: virtio-dev, kvm, qemu-devel, Amit Shah, Liang Li, linux-kernel,
virtualization, linux-mm, Vlastimil Babka, Paolo Bonzini,
Andrew Morton, Mel Gorman, dgilbert
In-Reply-To: <5798DB49.7030803@intel.com>
On Wed, Jul 27, 2016 at 09:03:21AM -0700, Dave Hansen wrote:
> On 07/26/2016 06:23 PM, Liang Li wrote:
> > + vb->pfn_limit = VIRTIO_BALLOON_PFNS_LIMIT;
> > + vb->pfn_limit = min(vb->pfn_limit, get_max_pfn());
> > + vb->bmap_len = ALIGN(vb->pfn_limit, BITS_PER_LONG) /
> > + BITS_PER_BYTE + 2 * sizeof(unsigned long);
> > + hdr_len = sizeof(struct balloon_bmap_hdr);
> > + vb->bmap_hdr = kzalloc(hdr_len + vb->bmap_len, GFP_KERNEL);
>
> This ends up doing a 1MB kmalloc() right? That seems a _bit_ big. How
> big was the pfn buffer before?
Yes I would limit this to 1G memory in a go, will result
in a 32KByte bitmap.
--
MST
^ permalink raw reply
* Re: [PATCH v2 repost 4/7] virtio-balloon: speed up inflate/deflate process
From: Michael S. Tsirkin @ 2016-07-27 21:36 UTC (permalink / raw)
To: Liang Li
Cc: virtio-dev, kvm, Amit Shah, qemu-devel, linux-kernel, linux-mm,
Vlastimil Babka, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, dgilbert
In-Reply-To: <1469582616-5729-5-git-send-email-liang.z.li@intel.com>
On Wed, Jul 27, 2016 at 09:23:33AM +0800, Liang Li wrote:
> The implementation of the current virtio-balloon is not very
> efficient, the time spends on different stages of inflating
> the balloon to 7GB of a 8GB idle guest:
>
> a. allocating pages (6.5%)
> b. sending PFNs to host (68.3%)
> c. address translation (6.1%)
> d. madvise (19%)
>
> It takes about 4126ms for the inflating process to complete.
> Debugging shows that the bottle neck are the stage b and stage d.
>
> If using a bitmap to send the page info instead of the PFNs, we
> can reduce the overhead in stage b quite a lot. Furthermore, we
> can do the address translation and call madvise() with a bulk of
> RAM pages, instead of the current page per page way, the overhead
> of stage c and stage d can also be reduced a lot.
>
> This patch is the kernel side implementation which is intended to
> speed up the inflating & deflating process by adding a new feature
> to the virtio-balloon device. With this new feature, inflating the
> balloon to 7GB of a 8GB idle guest only takes 590ms, the
> performance improvement is about 85%.
>
> TODO: optimize stage a by allocating/freeing a chunk of pages
> instead of a single page at a time.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/virtio/virtio_balloon.c | 184 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 162 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 8d649a2..2d18ff6 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -41,10 +41,28 @@
> #define OOM_VBALLOON_DEFAULT_PAGES 256
> #define VIRTBALLOON_OOM_NOTIFY_PRIORITY 80
>
> +/*
> + * VIRTIO_BALLOON_PFNS_LIMIT is used to limit the size of page bitmap
> + * to prevent a very large page bitmap, there are two reasons for this:
> + * 1) to save memory.
> + * 2) allocate a large bitmap may fail.
> + *
> + * The actual limit of pfn is determined by:
> + * pfn_limit = min(max_pfn, VIRTIO_BALLOON_PFNS_LIMIT);
> + *
> + * If system has more pages than VIRTIO_BALLOON_PFNS_LIMIT, we will scan
> + * the page list and send the PFNs with several times. To reduce the
> + * overhead of scanning the page list. VIRTIO_BALLOON_PFNS_LIMIT should
> + * be set with a value which can cover most cases.
So what if it covers 1/32 of the memory? We'll do 32 exits and not 1,
still not a big deal for a big guest.
> + */
> +#define VIRTIO_BALLOON_PFNS_LIMIT ((32 * (1ULL << 30)) >> PAGE_SHIFT) /* 32GB */
I already said this with a smaller limit.
2<< 30 is 2G but that is not a useful comment.
pls explain what is the reason for this selection.
Still applies here.
> +
> static int oom_pages = OOM_VBALLOON_DEFAULT_PAGES;
> module_param(oom_pages, int, S_IRUSR | S_IWUSR);
> MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
>
> +extern unsigned long get_max_pfn(void);
> +
> struct virtio_balloon {
> struct virtio_device *vdev;
> struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> @@ -62,6 +80,15 @@ struct virtio_balloon {
>
> /* Number of balloon pages we've told the Host we're not using. */
> unsigned int num_pages;
> + /* Pointer of the bitmap header. */
> + void *bmap_hdr;
> + /* Bitmap and length used to tell the host the pages */
> + unsigned long *page_bitmap;
> + unsigned long bmap_len;
> + /* Pfn limit */
> + unsigned long pfn_limit;
> + /* Used to record the processed pfn range */
> + unsigned long min_pfn, max_pfn, start_pfn, end_pfn;
> /*
> * The pages we've told the Host we're not using are enqueued
> * at vb_dev_info->pages list.
> @@ -105,12 +132,45 @@ static void balloon_ack(struct virtqueue *vq)
> wake_up(&vb->acked);
> }
>
> +static inline void init_pfn_range(struct virtio_balloon *vb)
> +{
> + vb->min_pfn = ULONG_MAX;
> + vb->max_pfn = 0;
> +}
> +
> +static inline void update_pfn_range(struct virtio_balloon *vb,
> + struct page *page)
> +{
> + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> +
> + if (balloon_pfn < vb->min_pfn)
> + vb->min_pfn = balloon_pfn;
> + if (balloon_pfn > vb->max_pfn)
> + vb->max_pfn = balloon_pfn;
> +}
> +
> static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
> {
> struct scatterlist sg;
> unsigned int len;
>
> - sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP)) {
> + struct balloon_bmap_hdr *hdr = vb->bmap_hdr;
> + unsigned long bmap_len;
> +
> + /* cmd and req_id are not used here, set them to 0 */
> + hdr->cmd = cpu_to_virtio16(vb->vdev, 0);
> + hdr->page_shift = cpu_to_virtio16(vb->vdev, PAGE_SHIFT);
> + hdr->reserved = cpu_to_virtio16(vb->vdev, 0);
> + hdr->req_id = cpu_to_virtio64(vb->vdev, 0);
no need to swap 0, just fill it in. in fact you allocated all 0s
so no need to touch these fields at all.
> + hdr->start_pfn = cpu_to_virtio64(vb->vdev, vb->start_pfn);
> + bmap_len = min(vb->bmap_len,
> + (vb->end_pfn - vb->start_pfn) / BITS_PER_BYTE);
> + hdr->bmap_len = cpu_to_virtio64(vb->vdev, bmap_len);
> + sg_init_one(&sg, hdr,
> + sizeof(struct balloon_bmap_hdr) + bmap_len);
> + } else
> + sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
>
> /* We should always be able to add one buffer to an empty queue. */
> virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> @@ -118,7 +178,6 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
>
> /* When host has read buffer, this completes via balloon_ack */
> wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> -
> }
>
> static void set_page_pfns(struct virtio_balloon *vb,
> @@ -133,13 +192,53 @@ static void set_page_pfns(struct virtio_balloon *vb,
> page_to_balloon_pfn(page) + i);
> }
>
> -static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> +static void set_page_bitmap(struct virtio_balloon *vb,
> + struct list_head *pages, struct virtqueue *vq)
> +{
> + unsigned long pfn;
> + struct page *page;
> + bool found;
> +
> + vb->min_pfn = rounddown(vb->min_pfn, BITS_PER_LONG);
> + vb->max_pfn = roundup(vb->max_pfn, BITS_PER_LONG);
> + for (pfn = vb->min_pfn; pfn < vb->max_pfn;
> + pfn += vb->pfn_limit) {
> + vb->start_pfn = pfn + vb->pfn_limit;
> + vb->end_pfn = pfn;
> + memset(vb->page_bitmap, 0, vb->bmap_len);
> + found = false;
> + list_for_each_entry(page, pages, lru) {
> + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> +
> + if (balloon_pfn < pfn ||
> + balloon_pfn >= pfn + vb->pfn_limit)
> + continue;
> + set_bit(balloon_pfn - pfn, vb->page_bitmap);
> + if (balloon_pfn > vb->end_pfn)
> + vb->end_pfn = balloon_pfn;
> + if (balloon_pfn < vb->start_pfn)
> + vb->start_pfn = balloon_pfn;
> + found = true;
> + }
> + if (found) {
> + vb->start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> + vb->end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> + tell_host(vb, vq);
> + }
> + }
> +}
> +
> +static unsigned int fill_balloon(struct virtio_balloon *vb, size_t num,
> + bool use_bmap)
> {
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> - unsigned num_allocated_pages;
> + unsigned int num_allocated_pages;
>
> - /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + if (use_bmap)
> + init_pfn_range(vb);
> + else
> + /* We can only do one array worth at a time. */
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> for (vb->num_pfns = 0; vb->num_pfns < num;
> @@ -154,7 +253,10 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> msleep(200);
> break;
> }
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (use_bmap)
> + update_pfn_range(vb, page);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
> if (!virtio_has_feature(vb->vdev,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> @@ -163,8 +265,13 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>
> num_allocated_pages = vb->num_pfns;
> /* Did we get any? */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->inflate_vq);
> + if (vb->num_pfns != 0) {
> + if (use_bmap)
> + set_page_bitmap(vb, &vb_dev_info->pages,
> + vb->inflate_vq);
> + else
> + tell_host(vb, vb->inflate_vq);
> + }
> mutex_unlock(&vb->balloon_lock);
>
> return num_allocated_pages;
> @@ -184,15 +291,19 @@ static void release_pages_balloon(struct virtio_balloon *vb,
> }
> }
>
> -static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> +static unsigned int leak_balloon(struct virtio_balloon *vb, size_t num,
> + bool use_bmap)
> {
> - unsigned num_freed_pages;
> + unsigned int num_freed_pages;
> struct page *page;
> struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
> LIST_HEAD(pages);
>
> - /* We can only do one array worth at a time. */
> - num = min(num, ARRAY_SIZE(vb->pfns));
> + if (use_bmap)
> + init_pfn_range(vb);
> + else
> + /* We can only do one array worth at a time. */
> + num = min(num, ARRAY_SIZE(vb->pfns));
>
> mutex_lock(&vb->balloon_lock);
> for (vb->num_pfns = 0; vb->num_pfns < num;
> @@ -200,7 +311,10 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> page = balloon_page_dequeue(vb_dev_info);
> if (!page)
> break;
> - set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> + if (use_bmap)
> + update_pfn_range(vb, page);
> + else
> + set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
> list_add(&page->lru, &pages);
> vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
> }
> @@ -211,9 +325,14 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
> * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
> * is true, we *have* to do it in this order
> */
> - if (vb->num_pfns != 0)
> - tell_host(vb, vb->deflate_vq);
> - release_pages_balloon(vb, &pages);
> + if (vb->num_pfns != 0) {
> + if (use_bmap)
> + set_page_bitmap(vb, &pages, vb->deflate_vq);
> + else
> + tell_host(vb, vb->deflate_vq);
> +
> + release_pages_balloon(vb, &pages);
> + }
> mutex_unlock(&vb->balloon_lock);
> return num_freed_pages;
> }
> @@ -347,13 +466,15 @@ static int virtballoon_oom_notify(struct notifier_block *self,
> struct virtio_balloon *vb;
> unsigned long *freed;
> unsigned num_freed_pages;
> + bool use_bmap;
>
> vb = container_of(self, struct virtio_balloon, nb);
> if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
> return NOTIFY_OK;
>
> freed = parm;
> - num_freed_pages = leak_balloon(vb, oom_pages);
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> + num_freed_pages = leak_balloon(vb, oom_pages, use_bmap);
> update_balloon_size(vb);
> *freed += num_freed_pages;
>
> @@ -373,15 +494,17 @@ static void update_balloon_size_func(struct work_struct *work)
> {
> struct virtio_balloon *vb;
> s64 diff;
> + bool use_bmap;
>
> vb = container_of(work, struct virtio_balloon,
> update_balloon_size_work);
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> diff = towards_target(vb);
>
> if (diff > 0)
> - diff -= fill_balloon(vb, diff);
> + diff -= fill_balloon(vb, diff, use_bmap);
> else if (diff < 0)
> - diff += leak_balloon(vb, -diff);
> + diff += leak_balloon(vb, -diff, use_bmap);
> update_balloon_size(vb);
>
> if (diff)
> @@ -489,7 +612,7 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> static int virtballoon_probe(struct virtio_device *vdev)
> {
> struct virtio_balloon *vb;
> - int err;
> + int err, hdr_len;
>
> if (!vdev->config->get) {
> dev_err(&vdev->dev, "%s failure: config access disabled\n",
> @@ -508,6 +631,18 @@ static int virtballoon_probe(struct virtio_device *vdev)
> spin_lock_init(&vb->stop_update_lock);
> vb->stop_update = false;
> vb->num_pages = 0;
> + vb->pfn_limit = VIRTIO_BALLOON_PFNS_LIMIT;
> + vb->pfn_limit = min(vb->pfn_limit, get_max_pfn());
> + vb->bmap_len = ALIGN(vb->pfn_limit, BITS_PER_LONG) /
> + BITS_PER_BYTE + 2 * sizeof(unsigned long);
What are these 2 longs in aid of?
> + hdr_len = sizeof(struct balloon_bmap_hdr);
> + vb->bmap_hdr = kzalloc(hdr_len + vb->bmap_len, GFP_KERNEL);
So it can go up to 1MByte but adding header size etc you need a higher
order allocation. This is a waste, there is no need to have a power of
two allocation. Start from the other side. Say "I want to allocate
32KBytes for the bitmap". Subtract the header and you get bitmap size.
Calculate the pfn limit from there.
> +
> + /* Clear the feature bit if memory allocation fails */
> + if (!vb->bmap_hdr)
> + __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> + else
> + vb->page_bitmap = vb->bmap_hdr + hdr_len;
> mutex_init(&vb->balloon_lock);
> init_waitqueue_head(&vb->acked);
> vb->vdev = vdev;
> @@ -541,9 +676,12 @@ out:
>
> static void remove_common(struct virtio_balloon *vb)
> {
> + bool use_bmap;
> +
> + use_bmap = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_PAGE_BITMAP);
> /* There might be pages left in the balloon: free them. */
> while (vb->num_pages)
> - leak_balloon(vb, vb->num_pages);
> + leak_balloon(vb, vb->num_pages, use_bmap);
> update_balloon_size(vb);
>
> /* Now we reset the device so we can clean up the queues. */
> @@ -565,6 +703,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
> cancel_work_sync(&vb->update_balloon_stats_work);
>
> remove_common(vb);
> + kfree(vb->page_bitmap);
> kfree(vb);
> }
>
> @@ -603,6 +742,7 @@ static unsigned int features[] = {
> VIRTIO_BALLOON_F_MUST_TELL_HOST,
> VIRTIO_BALLOON_F_STATS_VQ,
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
> + VIRTIO_BALLOON_F_PAGE_BITMAP,
> };
>
> static struct virtio_driver virtio_balloon_driver = {
> --
> 1.9.1
^ permalink raw reply
* Re: [PATCH v2 repost 6/7] mm: add the related functions to get free page info
From: Dave Hansen @ 2016-07-27 16:40 UTC (permalink / raw)
To: Liang Li, linux-kernel
Cc: virtio-dev, kvm, Amit Shah, Michael S. Tsirkin, qemu-devel,
dgilbert, linux-mm, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, Vlastimil Babka
In-Reply-To: <1469582616-5729-7-git-send-email-liang.z.li@intel.com>
On 07/26/2016 06:23 PM, Liang Li wrote:
> + for_each_migratetype_order(order, t) {
> + list_for_each(curr, &zone->free_area[order].free_list[t]) {
> + pfn = page_to_pfn(list_entry(curr, struct page, lru));
> + if (pfn >= start_pfn && pfn <= end_pfn) {
> + page_num = 1UL << order;
> + if (pfn + page_num > end_pfn)
> + page_num = end_pfn - pfn;
> + bitmap_set(bitmap, pfn - start_pfn, page_num);
> + }
> + }
> + }
Nit: The 'page_num' nomenclature really confused me here. It is the
number of bits being set in the bitmap. Seems like calling it nr_pages
or num_pages would be more appropriate.
Isn't this bitmap out of date by the time it's send up to the
hypervisor? Is there something that makes the inaccuracy OK here?
^ permalink raw reply
* Re: [PATCH v2 repost 4/7] virtio-balloon: speed up inflate/deflate process
From: Dave Hansen @ 2016-07-27 16:03 UTC (permalink / raw)
To: Liang Li, linux-kernel
Cc: virtio-dev, kvm, Amit Shah, Michael S. Tsirkin, qemu-devel,
dgilbert, linux-mm, Paolo Bonzini, Andrew Morton, virtualization,
Mel Gorman, Vlastimil Babka
In-Reply-To: <1469582616-5729-5-git-send-email-liang.z.li@intel.com>
On 07/26/2016 06:23 PM, Liang Li wrote:
> + vb->pfn_limit = VIRTIO_BALLOON_PFNS_LIMIT;
> + vb->pfn_limit = min(vb->pfn_limit, get_max_pfn());
> + vb->bmap_len = ALIGN(vb->pfn_limit, BITS_PER_LONG) /
> + BITS_PER_BYTE + 2 * sizeof(unsigned long);
> + hdr_len = sizeof(struct balloon_bmap_hdr);
> + vb->bmap_hdr = kzalloc(hdr_len + vb->bmap_len, GFP_KERNEL);
This ends up doing a 1MB kmalloc() right? That seems a _bit_ big. How
big was the pfn buffer before?
^ permalink raw reply
* Re: ext4 error when testing virtio-scsi & vhost-scsi
From: Jan Kara @ 2016-07-27 15:56 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Theodore Ts'o, kvm, Michael S. Tsirkin, qemu-devel,
virtualization, target-devel, linux-ext4
In-Reply-To: <CAMj5BkifLL=3C5f0VZKhpNmi81em8P5VNupr7d7_aD=uo2Hwow@mail.gmail.com>
Hi!
On Wed 27-07-16 15:58:55, Zhangfei Gao wrote:
> Hi, Michael
>
> I have met ext4 error when using vhost_scsi on arm64 platform, and
> suspect it is vhost_scsi issue.
>
> Ext4 error when testing virtio_scsi & vhost_scsi
>
>
> No issue:
> 1. virtio_scsi, ext4
> 2. vhost_scsi & virtio_scsi, ext2
> 3. Instead of vhost, also tried loopback and no problem.
> Using loopback, host can use the new block device, while vhost is used
> by guest (qemu).
> http://www.linux-iscsi.org/wiki/Tcm_loop
> Test directly in host, not find ext4 error.
>
>
>
> Have issue:
> 1. vhost_scsi & virtio_scsi, ext4
> a. iblock
> b, fileio, file located in /tmp (ram), no device based.
>
> 2, Have tried 4.7-r2 and 4.5-rc1 on D02 board, both have issue.
> Since I need kvm specific patch for D02, so it may not freely to switch
> to older version.
>
> 3. Also test with ext4, disabling journal
> mkfs.ext4 -O ^has_journal /dev/sda
>
>
> Do you have any suggestion?
So can you mount the filesystem with errors=remount-ro to avoid clobbering
the fs after the problem happens? And then run e2fsck on the problematic
filesystem and send the output here?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* [PATCH 7/7] kvmtool: Implement virtio-pstore device
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Kees Cook, Anton Vorontsov, Will Deacon, LKML,
Steven Rostedt, Colin Cross, Ingo Molnar
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
Add virtio pstore device to allow kernel log messages saved on the
host. With this patch, it will save the log files under directory given
by --pstore option.
$ lkvm run --pstore=dir-xx
(guest) # echo c > /proc/sysrq-trigger
$ ls dir-xx
dmesg-1.enc.z dmesg-2.enc.z
The log files are usually compressed using zlib. User can easily see
the messages on the host or on the guest (using pstore filesystem).
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
Makefile | 1 +
builtin-run.c | 2 +
include/kvm/kvm-config.h | 1 +
include/kvm/virtio-pci-dev.h | 2 +
include/kvm/virtio-pstore.h | 57 ++++++
include/linux/virtio_ids.h | 1 +
virtio/pstore.c | 459 +++++++++++++++++++++++++++++++++++++++++++
7 files changed, 523 insertions(+)
create mode 100644 include/kvm/virtio-pstore.h
create mode 100644 virtio/pstore.c
diff --git a/Makefile b/Makefile
index 1f0196f..d7462b9 100644
--- a/Makefile
+++ b/Makefile
@@ -67,6 +67,7 @@ OBJS += virtio/net.o
OBJS += virtio/rng.o
OBJS += virtio/balloon.o
OBJS += virtio/pci.o
+OBJS += virtio/pstore.o
OBJS += disk/blk.o
OBJS += disk/qcow.o
OBJS += disk/raw.o
diff --git a/builtin-run.c b/builtin-run.c
index 72b878d..08c12dd 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -128,6 +128,8 @@ void kvm_run_set_wrapper_sandbox(void)
" rootfs"), \
OPT_STRING('\0', "hugetlbfs", &(cfg)->hugetlbfs_path, "path", \
"Hugetlbfs path"), \
+ OPT_STRING('\0', "pstore", &(cfg)->pstore_path, "path", \
+ "pstore data path"), \
\
OPT_GROUP("Kernel options:"), \
OPT_STRING('k', "kernel", &(cfg)->kernel_filename, "kernel", \
diff --git a/include/kvm/kvm-config.h b/include/kvm/kvm-config.h
index 386fa8c..42b7651 100644
--- a/include/kvm/kvm-config.h
+++ b/include/kvm/kvm-config.h
@@ -45,6 +45,7 @@ struct kvm_config {
const char *hugetlbfs_path;
const char *custom_rootfs_name;
const char *real_cmdline;
+ const char *pstore_path;
struct virtio_net_params *net_params;
bool single_step;
bool vnc;
diff --git a/include/kvm/virtio-pci-dev.h b/include/kvm/virtio-pci-dev.h
index 48ae018..4339d94 100644
--- a/include/kvm/virtio-pci-dev.h
+++ b/include/kvm/virtio-pci-dev.h
@@ -15,6 +15,7 @@
#define PCI_DEVICE_ID_VIRTIO_BLN 0x1005
#define PCI_DEVICE_ID_VIRTIO_SCSI 0x1008
#define PCI_DEVICE_ID_VIRTIO_9P 0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE 0x100a
#define PCI_DEVICE_ID_VESA 0x2000
#define PCI_DEVICE_ID_PCI_SHMEM 0x0001
@@ -34,5 +35,6 @@
#define PCI_CLASS_RNG 0xff0000
#define PCI_CLASS_BLN 0xff0000
#define PCI_CLASS_9P 0xff0000
+#define PCI_CLASS_PSTORE 0xff0000
#endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 0000000..670d5e3
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,57 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+#include <kvm/virtio.h>
+#include <sys/types.h>
+
+#define VIRTIO_PSTORE_CMD_NULL 0
+#define VIRTIO_PSTORE_CMD_OPEN 1
+#define VIRTIO_PSTORE_CMD_READ 2
+#define VIRTIO_PSTORE_CMD_WRITE 3
+#define VIRTIO_PSTORE_CMD_ERASE 4
+#define VIRTIO_PSTORE_CMD_CLOSE 5
+
+#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
+#define VIRTIO_PSTORE_TYPE_DMESG 1
+#define VIRTIO_PSTORE_TYPE_CONSOLE 2
+
+#define VIRTIO_PSTORE_FL_COMPRESSED 1
+
+#define VIRTIO_PSTORE_CONFIG_FL_CONSOLE (1 << 0)
+
+struct virtio_pstore_req {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 flags;
+ __virtio64 id;
+ __virtio32 count;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_res {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 ret;
+};
+
+struct virtio_pstore_fileinfo {
+ __virtio64 id;
+ __virtio32 count;
+ __virtio16 type;
+ __virtio16 unused;
+ __virtio32 flags;
+ __virtio32 len;
+ __virtio64 time_sec;
+ __virtio32 time_nsec;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_config {
+ __virtio32 bufsize;
+ __virtio32 flags;
+};
+
+int virtio_pstore__init(struct kvm *kvm);
+int virtio_pstore__exit(struct kvm *kvm);
+
+#endif /* KVM__PSTORE_VIRTIO_H */
diff --git a/include/linux/virtio_ids.h b/include/linux/virtio_ids.h
index 5f60aa4..40eabf7 100644
--- a/include/linux/virtio_ids.h
+++ b/include/linux/virtio_ids.h
@@ -40,5 +40,6 @@
#define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */
#define VIRTIO_ID_CAIF 12 /* Virtio caif */
#define VIRTIO_ID_INPUT 18 /* virtio input */
+#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
#endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/virtio/pstore.c b/virtio/pstore.c
new file mode 100644
index 0000000..8290c14
--- /dev/null
+++ b/virtio/pstore.c
@@ -0,0 +1,459 @@
+#include "kvm/virtio-pstore.h"
+
+#include "kvm/virtio-pci-dev.h"
+
+#include "kvm/virtio.h"
+#include "kvm/util.h"
+#include "kvm/kvm.h"
+#include "kvm/threadpool.h"
+#include "kvm/guest_compat.h"
+
+#include <linux/virtio_ring.h>
+
+#include <linux/list.h>
+#include <fcntl.h>
+#include <dirent.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <pthread.h>
+#include <linux/kernel.h>
+#include <sys/eventfd.h>
+
+#define NUM_VIRT_QUEUES 2
+#define VIRTIO_PSTORE_QUEUE_SIZE 128
+
+struct io_thread_arg {
+ struct kvm *kvm;
+ struct pstore_dev *pdev;
+};
+
+struct pstore_dev {
+ struct list_head list;
+ struct virtio_device vdev;
+ pthread_t io_thread;
+ int io_efd;
+ int done;
+
+ struct virtio_pstore_config *config;
+
+ int fd;
+ DIR *dir;
+ u64 id;
+ u64 console_id;
+
+ /* virtio queue */
+ struct virt_queue vqs[NUM_VIRT_QUEUES];
+};
+
+static LIST_HEAD(pdevs);
+static int compat_id = -1;
+
+static u8 *get_config(struct kvm *kvm, void *dev)
+{
+ struct pstore_dev *pdev = dev;
+
+ return (u8*)pdev->config;
+}
+
+static u32 get_host_features(struct kvm *kvm, void *dev)
+{
+ /* Unused */
+ return 0;
+}
+
+static void set_guest_features(struct kvm *kvm, void *dev, u32 features)
+{
+ /* Unused */
+}
+
+static void virtio_pstore_to_filename(struct kvm *kvm, struct pstore_dev *pdev,
+ char *buf, size_t sz,
+ struct virtio_pstore_req *req)
+{
+ const char *basename;
+ unsigned long long id = 0;
+ unsigned int flags = virtio_host_to_guest_u64(pdev->vqs, req->flags);
+
+ switch (req->type) {
+ case VIRTIO_PSTORE_TYPE_DMESG:
+ basename = "dmesg";
+ id = pdev->id++;
+ break;
+ case VIRTIO_PSTORE_TYPE_CONSOLE:
+ basename = "console";
+ if (pdev->console_id)
+ id = pdev->console_id;
+ else
+ id = pdev->console_id = pdev->id++;
+ break;
+ default:
+ basename = "unknown";
+ break;
+ }
+
+ snprintf(buf, sz, "%s/%s-%llu%s", kvm->cfg.pstore_path, basename, id,
+ flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : "");
+}
+
+static void virtio_pstore_from_filename(struct kvm *kvm, char *name,
+ char *buf, size_t sz,
+ struct virtio_pstore_fileinfo *info)
+{
+ size_t len = strlen(name);
+
+ snprintf(buf, sz, "%s/%s", kvm->cfg.pstore_path, name);
+
+ info->flags = 0;
+ if (len > 6 && !strncmp(name + len - 6, ".enc.z", 6))
+ info->flags |= VIRTIO_PSTORE_FL_COMPRESSED;
+
+ if (!strncmp(name, "dmesg-", 6)) {
+ info->type = VIRTIO_PSTORE_TYPE_DMESG;
+ name += strlen("dmesg-");
+ } else if (!strncmp(name, "console-", 8)) {
+ info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
+ name += strlen("console-");
+ } else if (!strncmp(name, "unknown-", 8)) {
+ info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
+ name += strlen("unknown-");
+ }
+
+ info->id = strtoul(name, NULL, 0);
+}
+
+static int virtio_pstore_do_open(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virtio_pstore_req *req,
+ struct iovec *iov)
+{
+ pdev->dir = opendir(kvm->cfg.pstore_path);
+ if (pdev->dir == NULL)
+ return -errno;
+
+ return 0;
+}
+
+static int virtio_pstore_do_close(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virtio_pstore_req *req,
+ struct iovec *iov)
+{
+ if (pdev->dir == NULL)
+ return -1;
+
+ closedir(pdev->dir);
+ pdev->dir = NULL;
+
+ return 0;
+}
+
+static ssize_t virtio_pstore_do_read(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virtio_pstore_req *req,
+ struct iovec *iov,
+ struct virtio_pstore_fileinfo *info)
+{
+ char path[PATH_MAX];
+ FILE *fp;
+ ssize_t len = 0;
+ struct stat stbuf;
+ struct dirent *dent;
+
+ if (pdev->dir == NULL)
+ return 0;
+
+ dent = readdir(pdev->dir);
+ while (dent) {
+ if (dent->d_name[0] != '.')
+ break;
+ dent = readdir(pdev->dir);
+ }
+
+ if (dent == NULL)
+ return 0;
+
+ virtio_pstore_from_filename(kvm, dent->d_name, path, sizeof(path), info);
+ fp = fopen(path, "r");
+ if (fp == NULL)
+ return -1;
+
+ if (fstat(fileno(fp), &stbuf) < 0)
+ return -1;
+
+ len = fread(iov[3].iov_base, 1, iov[3].iov_len, fp);
+ if (len < 0 && errno == EAGAIN) {
+ len = 0;
+ goto out;
+ }
+
+ info->id = virtio_host_to_guest_u64(pdev->vqs, info->id);
+ info->type = virtio_host_to_guest_u64(pdev->vqs, info->type);
+ info->flags = virtio_host_to_guest_u32(pdev->vqs, info->flags);
+ info->len = virtio_host_to_guest_u32(pdev->vqs, len);
+
+ info->time_sec = virtio_host_to_guest_u64(pdev->vqs, stbuf.st_ctim.tv_sec);
+ info->time_nsec = virtio_host_to_guest_u32(pdev->vqs, stbuf.st_ctim.tv_nsec);
+
+ len += sizeof(*info);
+
+out:
+ fclose(fp);
+ return len;
+}
+
+static ssize_t virtio_pstore_do_write(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virtio_pstore_req *req,
+ struct iovec *iov)
+{
+ char path[PATH_MAX];
+ FILE *fp;
+ ssize_t len = 0;
+
+ virtio_pstore_to_filename(kvm, pdev, path, sizeof(path), req);
+
+ fp = fopen(path, "a");
+ if (fp == NULL)
+ return -1;
+
+ len = fwrite(iov[1].iov_base, 1, iov[1].iov_len, fp);
+ if (len < 0 && errno == EAGAIN)
+ len = 0;
+
+ fclose(fp);
+ return 0;
+}
+
+static ssize_t virtio_pstore_do_erase(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virtio_pstore_req *req,
+ struct iovec *iov)
+{
+ char path[PATH_MAX];
+
+ virtio_pstore_to_filename(kvm, pdev, path, sizeof(path), req);
+
+ return unlink(path);
+}
+
+static bool virtio_pstore_do_io_request(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virt_queue *vq)
+{
+ struct iovec iov[VIRTIO_PSTORE_QUEUE_SIZE];
+ struct virtio_pstore_req *req;
+ struct virtio_pstore_res *res;
+ struct virtio_pstore_fileinfo *info;
+ ssize_t len = 0;
+ u16 out, in, head;
+ int ret = 0;
+
+ head = virt_queue__get_iov(vq, iov, &out, &in, kvm);
+
+ if (iov[0].iov_len != sizeof(*req) || iov[out].iov_len != sizeof(*res)) {
+ return false;
+ }
+
+ req = iov[0].iov_base;
+ res = iov[out].iov_base;
+
+ switch (virtio_guest_to_host_u16(vq, req->cmd)) {
+ case VIRTIO_PSTORE_CMD_OPEN:
+ ret = virtio_pstore_do_open(kvm, pdev, req, iov);
+ break;
+ case VIRTIO_PSTORE_CMD_READ:
+ info = iov[out + 1].iov_base;
+ ret = virtio_pstore_do_read(kvm, pdev, req, iov, info);
+ if (ret > 0) {
+ len = ret;
+ ret = 0;
+ }
+ break;
+ case VIRTIO_PSTORE_CMD_WRITE:
+ ret = virtio_pstore_do_write(kvm, pdev, req, iov);
+ break;
+ case VIRTIO_PSTORE_CMD_CLOSE:
+ ret = virtio_pstore_do_close(kvm, pdev, req, iov);
+ break;
+ case VIRTIO_PSTORE_CMD_ERASE:
+ ret = virtio_pstore_do_erase(kvm, pdev, req, iov);
+ break;
+ default:
+ return false;
+ }
+
+ res->cmd = req->cmd;
+ res->type = req->type;
+ res->ret = virtio_host_to_guest_u32(vq, ret);
+
+ virt_queue__set_used_elem(vq, head, sizeof(*res) + len);
+
+ return ret == 0;
+}
+
+static void virtio_pstore_do_io(struct kvm *kvm, struct pstore_dev *pdev,
+ struct virt_queue *vq)
+{
+ bool done = false;
+
+ while (virt_queue__available(vq)) {
+ virtio_pstore_do_io_request(kvm, pdev, vq);
+ done = true;
+ }
+
+ if (done)
+ pdev->vdev.ops->signal_vq(kvm, &pdev->vdev, vq - pdev->vqs);
+}
+
+static void *virtio_pstore_io_thread(void *arg)
+{
+ struct io_thread_arg *io_arg = arg;
+ struct pstore_dev *pdev = io_arg->pdev;
+ struct kvm *kvm = io_arg->kvm;
+ u64 data;
+ int r;
+
+ kvm__set_thread_name("virtio-pstore-io");
+
+ while (!pdev->done) {
+ r = read(pdev->io_efd, &data, sizeof(u64));
+ if (r < 0)
+ continue;
+
+ virtio_pstore_do_io(kvm, pdev, &pdev->vqs[0]);
+ virtio_pstore_do_io(kvm, pdev, &pdev->vqs[1]);
+ }
+ free(io_arg);
+
+ pthread_exit(NULL);
+ return NULL;
+}
+
+static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align,
+ u32 pfn)
+{
+ struct pstore_dev *pdev = dev;
+ struct virt_queue *queue;
+ void *p;
+
+ compat__remove_message(compat_id);
+
+ queue = &pdev->vqs[vq];
+ queue->pfn = pfn;
+ p = virtio_get_vq(kvm, queue->pfn, page_size);
+
+ vring_init(&queue->vring, VIRTIO_PSTORE_QUEUE_SIZE, p, align);
+
+ return 0;
+}
+
+static int notify_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+ struct pstore_dev *pdev = dev;
+ u64 data = 1;
+ int r;
+
+ r = write(pdev->io_efd, &data, sizeof(data));
+ if (r < 0)
+ return r;
+
+ return 0;
+}
+
+static int get_pfn_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+ struct pstore_dev *pdev = dev;
+
+ return pdev->vqs[vq].pfn;
+}
+
+static int get_size_vq(struct kvm *kvm, void *dev, u32 vq)
+{
+ return VIRTIO_PSTORE_QUEUE_SIZE;
+}
+
+static int set_size_vq(struct kvm *kvm, void *dev, u32 vq, int size)
+{
+ /* FIXME: dynamic */
+ return size;
+}
+
+static struct virtio_ops pstore_dev_virtio_ops = {
+ .get_config = get_config,
+ .get_host_features = get_host_features,
+ .set_guest_features = set_guest_features,
+ .init_vq = init_vq,
+ .notify_vq = notify_vq,
+ .get_pfn_vq = get_pfn_vq,
+ .get_size_vq = get_size_vq,
+ .set_size_vq = set_size_vq,
+};
+
+int virtio_pstore__init(struct kvm *kvm)
+{
+ struct pstore_dev *pdev;
+ struct io_thread_arg *io_arg = NULL;
+ int r;
+
+ if (!kvm->cfg.pstore_path)
+ return 0;
+
+ pdev = calloc(1, sizeof(*pdev));
+ if (pdev == NULL)
+ return -ENOMEM;
+
+ pdev->config = calloc(1, sizeof(*pdev->config));
+ if (pdev->config == NULL) {
+ r = -ENOMEM;
+ goto cleanup;
+ }
+
+ pdev->id = 1;
+ pdev->console_id = 0;
+
+ io_arg = malloc(sizeof(*io_arg));
+ if (io_arg == NULL) {
+ r = -ENOMEM;
+ goto cleanup;
+ }
+
+ pdev->io_efd = eventfd(0, 0);
+
+ *io_arg = (struct io_thread_arg) {
+ .pdev = pdev,
+ .kvm = kvm,
+ };
+ r = pthread_create(&pdev->io_thread, NULL,
+ virtio_pstore_io_thread, io_arg);
+ if (r < 0)
+ goto cleanup;
+
+ r = virtio_init(kvm, pdev, &pdev->vdev, &pstore_dev_virtio_ops,
+ VIRTIO_DEFAULT_TRANS(kvm), PCI_DEVICE_ID_VIRTIO_PSTORE,
+ VIRTIO_ID_PSTORE, PCI_CLASS_PSTORE);
+ if (r < 0)
+ goto cleanup;
+
+ list_add_tail(&pdev->list, &pdevs);
+
+ if (compat_id == -1)
+ compat_id = virtio_compat_add_message("virtio-pstore", "CONFIG_VIRTIO_PSTORE");
+ return 0;
+
+cleanup:
+ free(io_arg);
+ free(pdev->config);
+ free(pdev);
+
+ return r;
+}
+virtio_dev_init(virtio_pstore__init);
+
+int virtio_pstore__exit(struct kvm *kvm)
+{
+ struct pstore_dev *pdev, *tmp;
+
+ list_for_each_entry_safe(pdev, tmp, &pdevs, list) {
+ list_del(&pdev->list);
+ close(pdev->io_efd);
+ pdev->vdev.ops->exit(kvm, &pdev->vdev);
+ free(pdev);
+ }
+
+ return 0;
+}
+virtio_dev_exit(virtio_pstore__exit);
--
2.8.0
^ permalink raw reply related
* [PATCH 6/7] qemu: Implement virtio-pstore device
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Radim Krčmář, Kees Cook,
Michael S. Tsirkin, Anton Vorontsov, LKML, Steven Rostedt,
Minchan Kim, Anthony Liguori, Colin Cross, Paolo Bonzini,
Ingo Molnar
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
Add virtio pstore device to allow kernel log files saved on the host.
It will save the log files on the directory given by pstore device
option.
$ qemu-system-x86_64 -device virtio-pstore,directory=dir-xx ...
(guest) # echo c > /proc/sysrq-trigger
$ ls dir-xx
dmesg-1.enc.z dmesg-2.enc.z
The log files are usually compressed using zlib. Users can see the log
messages directly on the host or on the guest (using pstore filesystem).
The 'directory' property is required for virtio-pstore device to work.
It also adds 'bufsize' and 'console' (boolean) properties.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: kvm@vger.kernel.org
Cc: qemu-devel@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
hw/virtio/Makefile.objs | 2 +-
hw/virtio/virtio-pci.c | 54 +++
hw/virtio/virtio-pci.h | 14 +
hw/virtio/virtio-pstore.c | 477 +++++++++++++++++++++++++
include/hw/pci/pci.h | 1 +
include/hw/virtio/virtio-pstore.h | 34 ++
include/standard-headers/linux/virtio_ids.h | 1 +
include/standard-headers/linux/virtio_pstore.h | 80 +++++
qdev-monitor.c | 1 +
9 files changed, 663 insertions(+), 1 deletion(-)
create mode 100644 hw/virtio/virtio-pstore.c
create mode 100644 include/hw/virtio/virtio-pstore.h
create mode 100644 include/standard-headers/linux/virtio_pstore.h
diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index 3e2b175..aae7082 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -4,4 +4,4 @@ common-obj-y += virtio-bus.o
common-obj-y += virtio-mmio.o
obj-y += virtio.o virtio-balloon.o
-obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o
+obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o virtio-pstore.o
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f0677b7..d99a405 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2414,6 +2414,59 @@ static const TypeInfo virtio_host_pci_info = {
};
#endif
+/* virtio-pstore-pci */
+
+static void virtio_pstore_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
+{
+ VirtIOPstorePCI *vps = VIRTIO_PSTORE_PCI(vpci_dev);
+ DeviceState *vdev = DEVICE(&vps->vdev);
+ Error *err = NULL;
+
+ qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
+ object_property_set_bool(OBJECT(vdev), true, "realized", &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
+ }
+}
+
+static void virtio_pstore_pci_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+ PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+
+ k->realize = virtio_pstore_pci_realize;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+
+ pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+ pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_PSTORE;
+ pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
+ pcidev_k->class_id = PCI_CLASS_OTHERS;
+}
+
+static void virtio_pstore_pci_instance_init(Object *obj)
+{
+ VirtIOPstorePCI *dev = VIRTIO_PSTORE_PCI(obj);
+
+ virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+ TYPE_VIRTIO_PSTORE);
+ object_property_add_alias(obj, "directory", OBJECT(&dev->vdev),
+ "directory", &error_abort);
+ object_property_add_alias(obj, "bufsize", OBJECT(&dev->vdev),
+ "bufsize", &error_abort);
+ object_property_add_alias(obj, "console", OBJECT(&dev->vdev),
+ "console", &error_abort);
+}
+
+static const TypeInfo virtio_pstore_pci_info = {
+ .name = TYPE_VIRTIO_PSTORE_PCI,
+ .parent = TYPE_VIRTIO_PCI,
+ .instance_size = sizeof(VirtIOPstorePCI),
+ .instance_init = virtio_pstore_pci_instance_init,
+ .class_init = virtio_pstore_pci_class_init,
+};
+
/* virtio-pci-bus */
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
@@ -2483,6 +2536,7 @@ static void virtio_pci_register_types(void)
#ifdef CONFIG_VHOST_SCSI
type_register_static(&vhost_scsi_pci_info);
#endif
+ type_register_static(&virtio_pstore_pci_info);
}
type_init(virtio_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index e4548c2..b4c039f 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -31,6 +31,7 @@
#ifdef CONFIG_VHOST_SCSI
#include "hw/virtio/vhost-scsi.h"
#endif
+#include "hw/virtio/virtio-pstore.h"
typedef struct VirtIOPCIProxy VirtIOPCIProxy;
typedef struct VirtIOBlkPCI VirtIOBlkPCI;
@@ -44,6 +45,7 @@ typedef struct VirtIOInputPCI VirtIOInputPCI;
typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI;
typedef struct VirtIOInputHostPCI VirtIOInputHostPCI;
typedef struct VirtIOGPUPCI VirtIOGPUPCI;
+typedef struct VirtIOPstorePCI VirtIOPstorePCI;
/* virtio-pci-bus */
@@ -311,6 +313,18 @@ struct VirtIOGPUPCI {
VirtIOGPU vdev;
};
+/*
+ * virtio-pstore-pci: This extends VirtioPCIProxy.
+ */
+#define TYPE_VIRTIO_PSTORE_PCI "virtio-pstore-pci"
+#define VIRTIO_PSTORE_PCI(obj) \
+ OBJECT_CHECK(VirtIOPstorePCI, (obj), TYPE_VIRTIO_PSTORE_PCI)
+
+struct VirtIOPstorePCI {
+ VirtIOPCIProxy parent_obj;
+ VirtIOPstore vdev;
+};
+
/* Virtio ABI version, if we increment this, we break the guest driver. */
#define VIRTIO_PCI_ABI_VERSION 0
diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c
new file mode 100644
index 0000000..2ca7786
--- /dev/null
+++ b/hw/virtio/virtio-pstore.c
@@ -0,0 +1,477 @@
+/*
+ * Virtio Pstore Device
+ *
+ * Copyright (C) 2016 LG Electronics
+ *
+ * Authors:
+ * Namhyung Kim <namhyung@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+
+#include "qemu/osdep.h"
+#include "qemu/iov.h"
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include "sysemu/kvm.h"
+#include "qapi/visitor.h"
+#include "qapi-event.h"
+#include "trace.h"
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-pstore.h"
+
+
+static void virtio_pstore_to_filename(VirtIOPstore *s, char *buf, size_t sz,
+ struct virtio_pstore_req *req)
+{
+ const char *basename;
+ unsigned long long id = 0;
+ unsigned int flags = le32_to_cpu(req->flags);
+
+ switch (le16_to_cpu(req->type)) {
+ case VIRTIO_PSTORE_TYPE_DMESG:
+ basename = "dmesg";
+ id = s->id++;
+ break;
+ case VIRTIO_PSTORE_TYPE_CONSOLE:
+ basename = "console";
+ if (s->console_id) {
+ id = s->console_id;
+ } else {
+ id = s->console_id = s->id++;
+ }
+ break;
+ default:
+ basename = "unknown";
+ break;
+ }
+
+ snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, id,
+ flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : "");
+}
+
+static void virtio_pstore_from_filename(VirtIOPstore *s, char *name,
+ char *buf, size_t sz,
+ struct virtio_pstore_fileinfo *info)
+{
+ snprintf(buf, sz, "%s/%s", s->directory, name);
+
+ if (g_str_has_prefix(name, "dmesg-")) {
+ info->type = VIRTIO_PSTORE_TYPE_DMESG;
+ name += strlen("dmesg-");
+ } else if (g_str_has_prefix(name, "console-")) {
+ info->type = VIRTIO_PSTORE_TYPE_CONSOLE;
+ name += strlen("console-");
+ } else if (g_str_has_prefix(name, "unknown-")) {
+ info->type = VIRTIO_PSTORE_TYPE_UNKNOWN;
+ name += strlen("unknown-");
+ }
+
+ qemu_strtoull(name, NULL, 0, &info->id);
+
+ info->flags = 0;
+ if (g_str_has_suffix(name, ".enc.z")) {
+ info->flags |= VIRTIO_PSTORE_FL_COMPRESSED;
+ }
+}
+
+static ssize_t virtio_pstore_do_open(VirtIOPstore *s)
+{
+ s->dirp = opendir(s->directory);
+ if (s->dirp == NULL) {
+ return -1;
+ }
+
+ return 0;
+}
+
+static ssize_t virtio_pstore_do_read(VirtIOPstore *s, struct iovec *in_sg,
+ unsigned int in_num,
+ struct virtio_pstore_res *res)
+{
+ char path[PATH_MAX];
+ int fd;
+ ssize_t len;
+ struct stat stbuf;
+ struct dirent *dent;
+ int sg_num = in_num;
+ struct iovec sg[sg_num];
+ struct virtio_pstore_fileinfo info;
+ size_t offset = sizeof(*res) + sizeof(info);
+
+ if (s->dirp == NULL) {
+ return -1;
+ }
+
+ dent = readdir(s->dirp);
+ while (dent) {
+ if (dent->d_name[0] != '.') {
+ break;
+ }
+ dent = readdir(s->dirp);
+ }
+
+ if (dent == NULL) {
+ return 0;
+ }
+
+ /* skip res and fileinfo */
+ sg_num = iov_copy(sg, sg_num, in_sg, in_num, offset,
+ iov_size(in_sg, in_num) - offset);
+
+ virtio_pstore_from_filename(s, dent->d_name, path, sizeof(path), &info);
+ fd = open(path, O_RDONLY);
+ if (fd < 0) {
+ error_report("cannot open %s", path);
+ return -1;
+ }
+
+ if (fstat(fd, &stbuf) < 0) {
+ len = -1;
+ goto out;
+ }
+
+ len = readv(fd, sg, sg_num);
+ if (len < 0) {
+ if (errno == EAGAIN) {
+ len = 0;
+ }
+ goto out;
+ }
+
+ info.id = cpu_to_le64(info.id);
+ info.type = cpu_to_le16(info.type);
+ info.flags = cpu_to_le32(info.flags);
+ info.len = cpu_to_le32(len);
+ info.time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec);
+ info.time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec);
+
+ iov_from_buf(in_sg, in_num, sizeof(*res), &info, sizeof(info));
+ len += sizeof(info);
+
+ out:
+ close(fd);
+ return len;
+}
+
+static ssize_t virtio_pstore_do_write(VirtIOPstore *s, struct iovec *out_sg,
+ unsigned int out_num,
+ struct virtio_pstore_req *req)
+{
+ char path[PATH_MAX];
+ int fd;
+ ssize_t len;
+ unsigned short type;
+ int flags = O_WRONLY | O_CREAT;
+
+ /* we already consume the req */
+ iov_discard_front(&out_sg, &out_num, sizeof(*req));
+
+ virtio_pstore_to_filename(s, path, sizeof(path), req);
+
+ type = le16_to_cpu(req->type);
+
+ if (type == VIRTIO_PSTORE_TYPE_DMESG) {
+ flags |= O_TRUNC;
+ } else if (type == VIRTIO_PSTORE_TYPE_CONSOLE) {
+ flags |= O_APPEND;
+ }
+
+ fd = open(path, flags, 0644);
+ if (fd < 0) {
+ error_report("cannot open %s", path);
+ return -1;
+ }
+ len = writev(fd, out_sg, out_num);
+ close(fd);
+
+ return len;
+}
+
+static ssize_t virtio_pstore_do_close(VirtIOPstore *s)
+{
+ if (s->dirp == NULL) {
+ return 0;
+ }
+
+ closedir(s->dirp);
+ s->dirp = NULL;
+
+ return 0;
+}
+
+static ssize_t virtio_pstore_do_erase(VirtIOPstore *s,
+ struct virtio_pstore_req *req)
+{
+ char path[PATH_MAX];
+
+ virtio_pstore_to_filename(s, path, sizeof(path), req);
+
+ return unlink(path);
+}
+
+static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VirtIOPstore *s = VIRTIO_PSTORE(vdev);
+ VirtQueueElement *elem;
+ struct virtio_pstore_req req;
+ struct virtio_pstore_res res;
+ ssize_t len = 0;
+ int ret;
+
+ for (;;) {
+ elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
+ if (!elem) {
+ return;
+ }
+
+ if (elem->out_num < 1 || elem->in_num < 1) {
+ error_report("request or response buffer is missing");
+ exit(1);
+ }
+
+ len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
+ if (len != (ssize_t)sizeof(req)) {
+ error_report("invalid request size: %ld", (long)len);
+ exit(1);
+ }
+ res.cmd = req.cmd;
+ res.type = req.type;
+
+ switch (le16_to_cpu(req.cmd)) {
+ case VIRTIO_PSTORE_CMD_OPEN:
+ ret = virtio_pstore_do_open(s);
+ break;
+ case VIRTIO_PSTORE_CMD_READ:
+ ret = virtio_pstore_do_read(s, elem->in_sg, elem->in_num, &res);
+ if (ret > 0) {
+ len = ret;
+ ret = 0;
+ }
+ break;
+ case VIRTIO_PSTORE_CMD_WRITE:
+ ret = virtio_pstore_do_write(s, elem->out_sg, elem->out_num, &req);
+ break;
+ case VIRTIO_PSTORE_CMD_CLOSE:
+ ret = virtio_pstore_do_close(s);
+ break;
+ case VIRTIO_PSTORE_CMD_ERASE:
+ ret = virtio_pstore_do_erase(s, &req);
+ break;
+ default:
+ ret = -1;
+ break;
+ }
+
+ res.ret = ret;
+
+ iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
+ virtqueue_push(vq, elem, sizeof(res) + len);
+
+ virtio_notify(vdev, vq);
+ g_free(elem);
+
+ if (ret < 0) {
+ return;
+ }
+ }
+}
+
+static void virtio_pstore_device_realize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+ VirtIOPstore *s = VIRTIO_PSTORE(dev);
+
+ virtio_init(vdev, "virtio-pstore", VIRTIO_ID_PSTORE,
+ sizeof(struct virtio_pstore_config));
+
+ s->id = 1;
+ s->console_id = 0;
+
+ s->vq[0] = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
+ s->vq[1] = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
+}
+
+static void virtio_pstore_device_unrealize(DeviceState *dev, Error **errp)
+{
+ VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+
+ virtio_cleanup(vdev);
+}
+
+static void virtio_pstore_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+ VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
+ struct virtio_pstore_config config;
+
+ config.bufsize = cpu_to_le32(dev->bufsize);
+ if (dev->console) {
+ config.flags |= cpu_to_le32(VIRTIO_PSTORE_CONFIG_FL_CONSOLE);
+ }
+
+ memcpy(config_data, &config, sizeof(struct virtio_pstore_config));
+}
+
+static void virtio_pstore_set_config(VirtIODevice *vdev,
+ const uint8_t *config_data)
+{
+ VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
+ struct virtio_pstore_config config;
+
+ memcpy(&config, config_data, sizeof(struct virtio_pstore_config));
+
+ dev->bufsize = le32_to_cpu(config.bufsize);
+}
+
+static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
+{
+ return f;
+}
+
+static void pstore_get_directory(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+
+ visit_type_str(v, name, &s->directory, errp);
+}
+
+static void pstore_set_directory(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+ Error *local_err = NULL;
+ char *value;
+
+ visit_type_str(v, name, &value, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ g_free(s->directory);
+ s->directory = value;
+}
+
+static void pstore_release_directory(Object *obj, const char *name,
+ void *opaque)
+{
+ VirtIOPstore *s = opaque;
+
+ g_free(s->directory);
+ s->directory = NULL;
+}
+
+static void pstore_get_bufsize(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+ uint64_t value = s->bufsize;
+
+ visit_type_size(v, name, &value, errp);
+}
+
+static void pstore_set_bufsize(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+ Error *error = NULL;
+ uint64_t value;
+
+ visit_type_size(v, name, &value, &error);
+ if (error) {
+ error_propagate(errp, error);
+ return;
+ }
+
+ if (value < 4096) {
+ error_report("Warning: too small buffer size: %"PRIu64, value);
+ }
+
+ s->bufsize = value;
+}
+
+static void pstore_get_console(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+ bool value = s->console;
+
+ visit_type_bool(v, name, &value, errp);
+}
+
+static void pstore_set_console(Object *obj, Visitor *v,
+ const char *name, void *opaque,
+ Error **errp)
+{
+ VirtIOPstore *s = opaque;
+ Error *error = NULL;
+ bool value;
+
+ visit_type_bool(v, name, &value, &error);
+ if (error) {
+ error_propagate(errp, error);
+ return;
+ }
+
+ s->console = value;
+}
+
+static Property virtio_pstore_properties[] = {
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_pstore_instance_init(Object *obj)
+{
+ VirtIOPstore *s = VIRTIO_PSTORE(obj);
+
+ object_property_add(obj, "directory", "str",
+ pstore_get_directory, pstore_set_directory,
+ pstore_release_directory, s, NULL);
+ object_property_add(obj, "bufsize", "size",
+ pstore_get_bufsize, pstore_set_bufsize, NULL, s, NULL);
+ object_property_add(obj, "console", "bool",
+ pstore_get_console, pstore_set_console, NULL, s, NULL);
+}
+
+static void virtio_pstore_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+
+ dc->props = virtio_pstore_properties;
+ set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+ vdc->realize = virtio_pstore_device_realize;
+ vdc->unrealize = virtio_pstore_device_unrealize;
+ vdc->get_config = virtio_pstore_get_config;
+ vdc->set_config = virtio_pstore_set_config;
+ vdc->get_features = get_features;
+}
+
+static const TypeInfo virtio_pstore_info = {
+ .name = TYPE_VIRTIO_PSTORE,
+ .parent = TYPE_VIRTIO_DEVICE,
+ .instance_size = sizeof(VirtIOPstore),
+ .instance_init = virtio_pstore_instance_init,
+ .class_init = virtio_pstore_class_init,
+};
+
+static void virtio_register_types(void)
+{
+ type_register_static(&virtio_pstore_info);
+}
+
+type_init(virtio_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 74d797d..000e1e9 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -79,6 +79,7 @@
#define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
#define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
#define PCI_DEVICE_ID_VIRTIO_9P 0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE 0x100a
#define PCI_VENDOR_ID_REDHAT 0x1b36
#define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
diff --git a/include/hw/virtio/virtio-pstore.h b/include/hw/virtio/virtio-pstore.h
new file mode 100644
index 0000000..d188a48
--- /dev/null
+++ b/include/hw/virtio/virtio-pstore.h
@@ -0,0 +1,34 @@
+/*
+ * Virtio Pstore Support
+ *
+ * Authors:
+ * Namhyung Kim <namhyung@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef _QEMU_VIRTIO_PSTORE_H
+#define _QEMU_VIRTIO_PSTORE_H
+
+#include "standard-headers/linux/virtio_pstore.h"
+#include "hw/virtio/virtio.h"
+#include "hw/pci/pci.h"
+
+#define TYPE_VIRTIO_PSTORE "virtio-pstore-device"
+#define VIRTIO_PSTORE(obj) \
+ OBJECT_CHECK(VirtIOPstore, (obj), TYPE_VIRTIO_PSTORE)
+
+typedef struct VirtIOPstore {
+ VirtIODevice parent_obj;
+ VirtQueue *vq[2];
+ char *directory;
+ uint64_t id;
+ uint64_t console_id;
+ DIR *dirp;
+ uint64_t bufsize;
+ bool console;
+} VirtIOPstore;
+
+#endif
diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
index 77925f5..c72a9ab 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -41,5 +41,6 @@
#define VIRTIO_ID_CAIF 12 /* Virtio caif */
#define VIRTIO_ID_GPU 16 /* virtio GPU */
#define VIRTIO_ID_INPUT 18 /* virtio input */
+#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
#endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/standard-headers/linux/virtio_pstore.h b/include/standard-headers/linux/virtio_pstore.h
new file mode 100644
index 0000000..b893d15
--- /dev/null
+++ b/include/standard-headers/linux/virtio_pstore.h
@@ -0,0 +1,80 @@
+#ifndef _LINUX_VIRTIO_PSTORE_H
+#define _LINUX_VIRTIO_PSTORE_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE. */
+#include "standard-headers/linux/types.h"
+#include "standard-headers/linux/virtio_types.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_config.h"
+
+#define VIRTIO_PSTORE_CMD_NULL 0
+#define VIRTIO_PSTORE_CMD_OPEN 1
+#define VIRTIO_PSTORE_CMD_READ 2
+#define VIRTIO_PSTORE_CMD_WRITE 3
+#define VIRTIO_PSTORE_CMD_ERASE 4
+#define VIRTIO_PSTORE_CMD_CLOSE 5
+
+#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
+#define VIRTIO_PSTORE_TYPE_DMESG 1
+#define VIRTIO_PSTORE_TYPE_CONSOLE 2
+
+#define VIRTIO_PSTORE_FL_COMPRESSED 1
+
+#define VIRTIO_PSTORE_CONFIG_FL_CONSOLE (1 << 0)
+
+struct virtio_pstore_req {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 flags;
+ __virtio64 id;
+ __virtio32 count;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_res {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 ret;
+};
+
+struct virtio_pstore_fileinfo {
+ __virtio64 id;
+ __virtio32 count;
+ __virtio16 type;
+ __virtio16 unused;
+ __virtio32 flags;
+ __virtio32 len;
+ __virtio64 time_sec;
+ __virtio32 time_nsec;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_config {
+ __virtio32 bufsize;
+ __virtio32 flags;
+};
+
+#endif /* _LINUX_VIRTIO_PSTORE_H */
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e19617f..e1df5a9 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -73,6 +73,7 @@ static const QDevAlias qdev_alias_table[] = {
{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+ { "virtio-pstore-pci", "virtio-pstore" },
{ }
};
--
2.8.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH 5/7] virtio-pstore: Support PSTORE_TYPE_CONSOLE
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Radim Krčmář, Kees Cook,
Michael S. Tsirkin, Anton Vorontsov, LKML, Steven Rostedt,
Minchan Kim, Anthony Liguori, Colin Cross, Paolo Bonzini,
Ingo Molnar
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
With help of buffer management functions, it now supports CONSOLE type
pstore write. The config space has flags field which defines the
VIRTIO_PSTORE_CONFIG_FL_CONSOLE. When it's set, the virtio-pstore
driver also sets PSTORE_FLAGS_ASYNC so that the buffer management is
enabled.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: kvm@vger.kernel.org
Cc: qemu-devel@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
drivers/virtio/virtio_pstore.c | 9 ++++++++-
include/uapi/linux/virtio_pstore.h | 4 ++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
index 4ee1c186f582..458c4d3ccbb1 100644
--- a/drivers/virtio/virtio_pstore.c
+++ b/drivers/virtio/virtio_pstore.c
@@ -33,6 +33,7 @@ struct type_table {
u16 virtio;
} type_table[] = {
TYPE_TABLE_ENTRY(DMESG),
+ TYPE_TABLE_ENTRY(CONSOLE),
};
#undef TYPE_TABLE_ENTRY
@@ -276,7 +277,8 @@ static int virt_pstore_init(struct virtio_pstore *vps)
psinfo->read = virt_pstore_read;
psinfo->erase = virt_pstore_erase;
psinfo->write = virt_pstore_write;
- psinfo->flags = PSTORE_FLAGS_DMESG;
+ /* preserve flags from config */
+ psinfo->flags |= PSTORE_FLAGS_DMESG;
psinfo->data = vps;
spin_lock_init(&psinfo->buf_lock);
@@ -313,10 +315,15 @@ static int virtpstore_init_vqs(struct virtio_pstore *vps)
static void virtpstore_init_config(struct virtio_pstore *vps)
{
u32 bufsize;
+ u32 flags;
virtio_cread(vps->vdev, struct virtio_pstore_config, bufsize, &bufsize);
+ virtio_cread(vps->vdev, struct virtio_pstore_config, flags, &flags);
vps->pstore.bufsize = PAGE_ALIGN(bufsize);
+
+ if (flags & VIRTIO_PSTORE_CONFIG_FL_CONSOLE)
+ vps->pstore.flags |= PSTORE_FLAGS_CONSOLE | PSTORE_FLAGS_ASYNC;
}
static void virtpstore_confirm_config(struct virtio_pstore *vps)
diff --git a/include/uapi/linux/virtio_pstore.h b/include/uapi/linux/virtio_pstore.h
index f4b0d204d8ae..56d0b1554231 100644
--- a/include/uapi/linux/virtio_pstore.h
+++ b/include/uapi/linux/virtio_pstore.h
@@ -37,9 +37,12 @@
#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
#define VIRTIO_PSTORE_TYPE_DMESG 1
+#define VIRTIO_PSTORE_TYPE_CONSOLE 2
#define VIRTIO_PSTORE_FL_COMPRESSED 1
+#define VIRTIO_PSTORE_CONFIG_FL_CONSOLE (1 << 0)
+
struct virtio_pstore_req {
__virtio16 cmd;
__virtio16 type;
@@ -69,6 +72,7 @@ struct virtio_pstore_fileinfo {
struct virtio_pstore_config {
__virtio32 bufsize;
+ __virtio32 flags;
};
#endif /* _LINUX_VIRTIO_PSTORE_H */
--
2.8.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH 4/7] virtio: Basic implementation of virtio pstore driver
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Radim Krčmář, Kees Cook,
Michael S. Tsirkin, Anton Vorontsov, LKML, Steven Rostedt,
Minchan Kim, Anthony Liguori, Colin Cross, Paolo Bonzini,
Ingo Molnar
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
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. It uses
two virtqueues - one for (sync) read and another for (async) write.
Since it cannot wait for write finished, it supports up to 128
concurrent IO. The buffer size is configurable now.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: kvm@vger.kernel.org
Cc: qemu-devel@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
drivers/virtio/Kconfig | 10 +
drivers/virtio/Makefile | 1 +
drivers/virtio/virtio_pstore.c | 414 +++++++++++++++++++++++++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/virtio_ids.h | 1 +
include/uapi/linux/virtio_pstore.h | 74 +++++++
6 files changed, 501 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 000000000000..4ee1c186f582
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,414 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pstore.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_pstore.h>
+
+#define VIRT_PSTORE_ORDER 2
+#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
+#define VIRT_PSTORE_NR_REQ 128
+
+struct virtio_pstore {
+ struct virtio_device *vdev;
+ struct virtqueue *vq[2];
+ struct pstore_info pstore;
+ struct virtio_pstore_req req[VIRT_PSTORE_NR_REQ];
+ struct virtio_pstore_res res[VIRT_PSTORE_NR_REQ];
+ unsigned int req_id;
+
+ /* Waiting for host to ack */
+ wait_queue_head_t acked;
+ int failed;
+};
+
+#define TYPE_TABLE_ENTRY(_entry) \
+ { PSTORE_TYPE_##_entry, VIRTIO_PSTORE_TYPE_##_entry }
+
+struct type_table {
+ int pstore;
+ u16 virtio;
+} type_table[] = {
+ TYPE_TABLE_ENTRY(DMESG),
+};
+
+#undef TYPE_TABLE_ENTRY
+
+
+static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(type_table); i++) {
+ if (type == type_table[i].pstore)
+ return cpu_to_virtio16(vps->vdev, type_table[i].virtio);
+ }
+
+ return cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
+}
+
+static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(type_table); i++) {
+ if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio)
+ return type_table[i].pstore;
+ }
+
+ return PSTORE_TYPE_UNKNOWN;
+}
+
+static void virtpstore_ack(struct virtqueue *vq)
+{
+ struct virtio_pstore *vps = vq->vdev->priv;
+
+ wake_up(&vps->acked);
+}
+
+static void virtpstore_check(struct virtqueue *vq)
+{
+ struct virtio_pstore *vps = vq->vdev->priv;
+ struct virtio_pstore_res *res;
+ unsigned int len;
+
+ res = virtqueue_get_buf(vq, &len);
+ if (res == NULL)
+ return;
+
+ if (virtio32_to_cpu(vq->vdev, res->ret) < 0)
+ vps->failed = 1;
+}
+
+static void virt_pstore_get_reqs(struct virtio_pstore *vps,
+ struct virtio_pstore_req **preq,
+ struct virtio_pstore_res **pres)
+{
+ unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ;
+
+ *preq = &vps->req[idx];
+ *pres = &vps->res[idx];
+
+ memset(*preq, 0, sizeof(**preq));
+ memset(*pres, 0, sizeof(**pres));
+}
+
+static int virt_pstore_open(struct pstore_info *psi)
+{
+ struct virtio_pstore *vps = psi->data;
+ struct virtio_pstore_req *req;
+ struct virtio_pstore_res *res;
+ struct scatterlist sgo[1], sgi[1];
+ struct scatterlist *sgs[2] = { sgo, sgi };
+ unsigned int len;
+
+ virt_pstore_get_reqs(vps, &req, &res);
+
+ req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
+
+ sg_init_one(sgo, req, sizeof(*req));
+ sg_init_one(sgi, res, sizeof(*res));
+ virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
+ virtqueue_kick(vps->vq[0]);
+
+ wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
+ return virtio32_to_cpu(vps->vdev, res->ret);
+}
+
+static int virt_pstore_close(struct pstore_info *psi)
+{
+ struct virtio_pstore *vps = psi->data;
+ struct virtio_pstore_req *req = &vps->req[vps->req_id];
+ struct virtio_pstore_res *res = &vps->res[vps->req_id];
+ struct scatterlist sgo[1], sgi[1];
+ struct scatterlist *sgs[2] = { sgo, sgi };
+ unsigned int len;
+
+ virt_pstore_get_reqs(vps, &req, &res);
+
+ req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_CLOSE);
+
+ sg_init_one(sgo, req, sizeof(*req));
+ sg_init_one(sgi, res, sizeof(*res));
+ virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
+ virtqueue_kick(vps->vq[0]);
+
+ wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
+ return virtio32_to_cpu(vps->vdev, res->ret);
+}
+
+static ssize_t virt_pstore_read(u64 *id, enum pstore_type_id *type,
+ int *count, struct timespec *time,
+ char **buf, bool *compressed,
+ ssize_t *ecc_notice_size,
+ struct pstore_info *psi)
+{
+ struct virtio_pstore *vps = psi->data;
+ struct virtio_pstore_req *req;
+ struct virtio_pstore_res *res;
+ struct virtio_pstore_fileinfo info;
+ struct scatterlist sgo[1], sgi[3];
+ struct scatterlist *sgs[2] = { sgo, sgi };
+ unsigned int len;
+ unsigned int flags;
+ int ret;
+ void *bf;
+
+ virt_pstore_get_reqs(vps, &req, &res);
+
+ req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_READ);
+
+ sg_init_one(sgo, req, sizeof(*req));
+ sg_init_table(sgi, 3);
+ sg_set_buf(&sgi[0], res, sizeof(*res));
+ sg_set_buf(&sgi[1], &info, sizeof(info));
+ sg_set_buf(&sgi[2], psi->buf, psi->bufsize);
+ virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
+ virtqueue_kick(vps->vq[0]);
+
+ wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
+ if (len < sizeof(*res) + sizeof(info))
+ return -1;
+
+ ret = virtio32_to_cpu(vps->vdev, res->ret);
+ if (ret < 0)
+ return ret;
+
+ len = virtio32_to_cpu(vps->vdev, info.len);
+
+ bf = kmalloc(len, GFP_KERNEL);
+ if (bf == NULL)
+ return -ENOMEM;
+
+ *id = virtio64_to_cpu(vps->vdev, info.id);
+ *type = from_virtio_type(vps, info.type);
+ *count = virtio32_to_cpu(vps->vdev, info.count);
+
+ flags = virtio32_to_cpu(vps->vdev, info.flags);
+ *compressed = flags & VIRTIO_PSTORE_FL_COMPRESSED;
+
+ time->tv_sec = virtio64_to_cpu(vps->vdev, info.time_sec);
+ time->tv_nsec = virtio32_to_cpu(vps->vdev, info.time_nsec);
+
+ memcpy(bf, psi->buf, len);
+ *buf = bf;
+
+ return len;
+}
+
+static int notrace virt_pstore_write(enum pstore_type_id type,
+ enum kmsg_dump_reason reason,
+ u64 *id, unsigned int part, int count,
+ bool compressed, size_t size,
+ struct pstore_info *psi)
+{
+ struct virtio_pstore *vps = psi->data;
+ struct virtio_pstore_req *req;
+ struct virtio_pstore_res *res;
+ struct scatterlist sgo[2], sgi[1];
+ struct scatterlist *sgs[2] = { sgo, sgi };
+ unsigned int flags = compressed ? VIRTIO_PSTORE_FL_COMPRESSED : 0;
+
+ *id = vps->req_id;
+ virt_pstore_get_reqs(vps, &req, &res);
+
+ req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_WRITE);
+ req->type = to_virtio_type(vps, type);
+ req->flags = cpu_to_virtio32(vps->vdev, flags);
+
+ sg_init_table(sgo, 2);
+ sg_set_buf(&sgo[0], req, sizeof(*req));
+ sg_set_buf(&sgo[1], pstore_get_buf(psi), size);
+ sg_init_one(sgi, res, sizeof(*res));
+ virtqueue_add_sgs(vps->vq[1], sgs, 1, 1, vps, GFP_ATOMIC);
+ virtqueue_kick(vps->vq[1]);
+
+ return 0;
+}
+
+static int virt_pstore_erase(enum pstore_type_id type, u64 id, int count,
+ struct timespec time, struct pstore_info *psi)
+{
+ struct virtio_pstore *vps = psi->data;
+ struct virtio_pstore_req *req;
+ struct virtio_pstore_res *res;
+ struct scatterlist sgo[1], sgi[1];
+ struct scatterlist *sgs[2] = { sgo, sgi };
+ unsigned int len;
+
+ virt_pstore_get_reqs(vps, &req, &res);
+
+ req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_ERASE);
+ req->type = to_virtio_type(vps, type);
+ req->id = cpu_to_virtio64(vps->vdev, id);
+ req->count = cpu_to_virtio32(vps->vdev, count);
+
+ sg_init_one(sgo, req, sizeof(*req));
+ sg_init_one(sgi, res, sizeof(*res));
+ virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
+ virtqueue_kick(vps->vq[0]);
+
+ wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
+ return virtio32_to_cpu(vps->vdev, res->ret);
+}
+
+static int virt_pstore_init(struct virtio_pstore *vps)
+{
+ struct pstore_info *psinfo = &vps->pstore;
+ int err;
+
+ if (!psinfo->bufsize)
+ psinfo->bufsize = VIRT_PSTORE_BUFSIZE;
+
+ psinfo->buf = alloc_pages_exact(psinfo->bufsize, GFP_KERNEL);
+ if (!psinfo->buf) {
+ pr_err("cannot allocate pstore buffer\n");
+ return -ENOMEM;
+ }
+
+ psinfo->owner = THIS_MODULE;
+ psinfo->name = "virtio";
+ psinfo->open = virt_pstore_open;
+ psinfo->close = virt_pstore_close;
+ psinfo->read = virt_pstore_read;
+ psinfo->erase = virt_pstore_erase;
+ psinfo->write = virt_pstore_write;
+ psinfo->flags = PSTORE_FLAGS_DMESG;
+
+ psinfo->data = vps;
+ spin_lock_init(&psinfo->buf_lock);
+
+ err = pstore_register(psinfo);
+ if (err)
+ kfree(psinfo->buf);
+
+ return err;
+}
+
+static int virt_pstore_exit(struct virtio_pstore *vps)
+{
+ struct pstore_info *psinfo = &vps->pstore;
+
+ pstore_unregister(psinfo);
+
+ free_pages_exact(psinfo->buf, psinfo->bufsize);
+ psinfo->buf = NULL;
+ psinfo->bufsize = 0;
+
+ return 0;
+}
+
+static int virtpstore_init_vqs(struct virtio_pstore *vps)
+{
+ vq_callback_t *callbacks[] = { virtpstore_ack, virtpstore_check };
+ const char *names[] = { "pstore_read", "pstore_write" };
+
+ return vps->vdev->config->find_vqs(vps->vdev, 2, vps->vq,
+ callbacks, names);
+}
+
+static void virtpstore_init_config(struct virtio_pstore *vps)
+{
+ u32 bufsize;
+
+ virtio_cread(vps->vdev, struct virtio_pstore_config, bufsize, &bufsize);
+
+ vps->pstore.bufsize = PAGE_ALIGN(bufsize);
+}
+
+static void virtpstore_confirm_config(struct virtio_pstore *vps)
+{
+ u32 bufsize = vps->pstore.bufsize;
+
+ virtio_cwrite(vps->vdev, struct virtio_pstore_config, bufsize,
+ &bufsize);
+}
+
+static int virtpstore_probe(struct virtio_device *vdev)
+{
+ struct virtio_pstore *vps;
+ int err;
+
+ if (!vdev->config->get) {
+ dev_err(&vdev->dev, "driver init: config access disabled\n");
+ return -EINVAL;
+ }
+
+ vdev->priv = vps = kzalloc(sizeof(*vps), GFP_KERNEL);
+ if (!vps) {
+ err = -ENOMEM;
+ goto out;
+ }
+ vps->vdev = vdev;
+
+ err = virtpstore_init_vqs(vps);
+ if (err < 0)
+ goto out_free;
+
+ virtpstore_init_config(vps);
+
+ err = virt_pstore_init(vps);
+ if (err)
+ goto out_del_vq;
+
+ virtpstore_confirm_config(vps);
+
+ init_waitqueue_head(&vps->acked);
+
+ virtio_device_ready(vdev);
+
+ dev_info(&vdev->dev, "driver init: ok (bufsize = %luK, flags = %x)\n",
+ vps->pstore.bufsize >> 10, vps->pstore.flags);
+
+ return 0;
+
+out_del_vq:
+ vdev->config->del_vqs(vdev);
+out_free:
+ kfree(vps);
+out:
+ dev_err(&vdev->dev, "driver init: failed with %d\n", err);
+ return err;
+}
+
+static void virtpstore_remove(struct virtio_device *vdev)
+{
+ struct virtio_pstore *vps = vdev->priv;
+
+ virt_pstore_exit(vps);
+
+ /* Now we reset the device so we can clean up the queues. */
+ vdev->config->reset(vdev);
+
+ vdev->config->del_vqs(vdev);
+
+ kfree(vps);
+}
+
+static unsigned int features[] = {
+};
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_PSTORE, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_pstore_driver = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .feature_table = features,
+ .feature_table_size = ARRAY_SIZE(features),
+ .id_table = id_table,
+ .probe = virtpstore_probe,
+ .remove = virtpstore_remove,
+};
+
+module_virtio_driver(virtio_pstore_driver);
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Namhyung Kim <namhyung@kernel.org>");
+MODULE_DESCRIPTION("Virtio pstore driver");
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index ec10cfef166a..c3a7d56599c5 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -449,6 +449,7 @@ header-y += virtio_ids.h
header-y += virtio_input.h
header-y += virtio_net.h
header-y += virtio_pci.h
+header-y += virtio_pstore.h
header-y += virtio_ring.h
header-y += virtio_rng.h
header-y += virtio_scsi.h
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 77925f587b15..c72a9ab588c0 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -41,5 +41,6 @@
#define VIRTIO_ID_CAIF 12 /* Virtio caif */
#define VIRTIO_ID_GPU 16 /* virtio GPU */
#define VIRTIO_ID_INPUT 18 /* virtio input */
+#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
#endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/uapi/linux/virtio_pstore.h b/include/uapi/linux/virtio_pstore.h
new file mode 100644
index 000000000000..f4b0d204d8ae
--- /dev/null
+++ b/include/uapi/linux/virtio_pstore.h
@@ -0,0 +1,74 @@
+#ifndef _LINUX_VIRTIO_PSTORE_H
+#define _LINUX_VIRTIO_PSTORE_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE. */
+#include <linux/types.h>
+#include <linux/virtio_types.h>
+
+#define VIRTIO_PSTORE_CMD_NULL 0
+#define VIRTIO_PSTORE_CMD_OPEN 1
+#define VIRTIO_PSTORE_CMD_READ 2
+#define VIRTIO_PSTORE_CMD_WRITE 3
+#define VIRTIO_PSTORE_CMD_ERASE 4
+#define VIRTIO_PSTORE_CMD_CLOSE 5
+
+#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
+#define VIRTIO_PSTORE_TYPE_DMESG 1
+
+#define VIRTIO_PSTORE_FL_COMPRESSED 1
+
+struct virtio_pstore_req {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 flags;
+ __virtio64 id;
+ __virtio32 count;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_res {
+ __virtio16 cmd;
+ __virtio16 type;
+ __virtio32 ret;
+};
+
+struct virtio_pstore_fileinfo {
+ __virtio64 id;
+ __virtio32 count;
+ __virtio16 type;
+ __virtio16 unused;
+ __virtio32 flags;
+ __virtio32 len;
+ __virtio64 time_sec;
+ __virtio32 time_nsec;
+ __virtio32 reserved;
+};
+
+struct virtio_pstore_config {
+ __virtio32 bufsize;
+};
+
+#endif /* _LINUX_VIRTIO_PSTORE_H */
--
2.8.0
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH 3/7] pstore: Manage buffer position for async write
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Kees Cook, Matt Fleming, Anton Vorontsov, LKML,
linux-efi, Colin Cross
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
Some pstore backend only supports async mode, so it's possible to do
some IO at the same time. In this case we need to manage the single
psinfo->buf not to broken by concurrent IO.
For example PSTORE_TYPE_CONSOLE IO is serialized by psinfo->buf_lock but
later IO can be issued before finishing previous request. In this case
it overwrites psinfo->buf so the previous result might be broken.
This patch adds psinfo->bufpos field to keep track of current position
in order to use psinfo->buf as a ring buffer. However it's just a
simple, best-effort way of doing that, and provides no 100% guarantee.
It's only effective for small concurrent IO like PSTORE_TYPE_CONSOLE
IMHO.
The new PSTORE_FLAGS_ASYNC flag enables management of buffer position.
The pstore_prepare_buf() is called before accessing the psinfo->buf and
the pstore_update_buf() is called after accessing the buf.
The pstore_get_buf() is provided for psinfo->write callback to determine
the current position of available buffer.
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-efi@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
drivers/firmware/efi/efi-pstore.c | 2 +-
fs/pstore/platform.c | 48 +++++++++++++++++++++++++++++++--------
include/linux/pstore.h | 4 ++++
3 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 4daa5acd9117..ae0ffe259e07 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -259,7 +259,7 @@ static int efi_pstore_write(enum pstore_type_id type,
efivar_entry_set_safe(efi_name, vendor, PSTORE_EFI_ATTRIBUTES,
!pstore_cannot_block_path(reason),
- size, psi->buf);
+ size, pstore_get_buf(psi));
if (reason == KMSG_DUMP_OOPS)
efivar_run_worker();
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 76dd604a0f2c..26e2808cf554 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -150,6 +150,27 @@ bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
}
EXPORT_SYMBOL_GPL(pstore_cannot_block_path);
+static void *pstore_prepare_buf(struct pstore_info *psi, size_t len)
+{
+ if (psi->bufpos + len > psi->bufsize ||
+ (psi->flags & PSTORE_FLAGS_ASYNC) == 0)
+ psi->bufpos = 0;
+
+ return psi->buf + psi->bufpos;
+}
+
+static void pstore_update_buf(struct pstore_info *psi, size_t len)
+{
+ if (psi->flags & PSTORE_FLAGS_ASYNC)
+ psi->bufpos += len;
+}
+
+void *pstore_get_buf(struct pstore_info *psi)
+{
+ return psi->buf + psi->bufpos;
+}
+EXPORT_SYMBOL_GPL(pstore_get_buf);
+
#ifdef CONFIG_PSTORE_ZLIB_COMPRESS
/* Derived from logfs_compress() */
static int compress_zlib(const void *in, void *out, size_t inlen, size_t outlen)
@@ -455,18 +476,21 @@ static size_t copy_kmsg_to_buffer(int hsize, size_t len)
{
size_t total_len;
size_t diff;
+ void *dst;
total_len = hsize + len;
+ dst = pstore_prepare_buf(psinfo, total_len);
if (total_len > psinfo->bufsize) {
diff = total_len - psinfo->bufsize + hsize;
- memcpy(psinfo->buf, big_oops_buf, hsize);
- memcpy(psinfo->buf + hsize, big_oops_buf + diff,
+ memcpy(dst, big_oops_buf, hsize);
+ memcpy(dst + hsize, big_oops_buf + diff,
psinfo->bufsize - hsize);
total_len = psinfo->bufsize;
} else
- memcpy(psinfo->buf, big_oops_buf, total_len);
+ memcpy(dst, big_oops_buf, total_len);
+ pstore_update_buf(psinfo, total_len);
return total_len;
}
@@ -500,7 +524,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
}
oopscount++;
while (total < kmsg_bytes) {
- char *dst;
+ char *dst, *buf;
unsigned long size;
int hsize;
int zipped_len = -1;
@@ -514,6 +538,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
} else {
dst = psinfo->buf;
size = psinfo->bufsize;
+ psinfo->bufpos = 0;
}
hsize = sprintf(dst, "%s#%d Part%u\n", why, oopscount, part);
@@ -524,8 +549,9 @@ static void pstore_dump(struct kmsg_dumper *dumper,
break;
if (big_oops_buf && is_locked) {
- zipped_len = pstore_compress(dst, psinfo->buf,
- hsize + len, psinfo->bufsize);
+ buf = pstore_prepare_buf(psinfo, hsize + len);
+ zipped_len = pstore_compress(dst, buf, hsize + len,
+ psinfo->bufsize - psinfo->bufpos);
if (zipped_len > 0) {
compressed = true;
@@ -543,6 +569,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
pstore_new_entry = 1;
total += total_len;
+ pstore_update_buf(psinfo, total_len);
part++;
}
if (is_locked)
@@ -573,6 +600,7 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c)
while (s < e) {
unsigned long flags;
+ void *dst;
u64 id;
if (c > psinfo->bufsize)
@@ -584,8 +612,10 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c)
} else {
spin_lock_irqsave(&psinfo->buf_lock, flags);
}
- memcpy(psinfo->buf, s, c);
+ dst = pstore_prepare_buf(psinfo, c);
+ memcpy(dst, s, c);
psinfo->write(PSTORE_TYPE_CONSOLE, 0, &id, 0, 0, 0, c, psinfo);
+ pstore_update_buf(psinfo, c);
spin_unlock_irqrestore(&psinfo->buf_lock, flags);
s += c;
c = e - s;
@@ -619,8 +649,8 @@ static int pstore_write_compat(enum pstore_type_id type,
bool compressed, size_t size,
struct pstore_info *psi)
{
- return psi->write_buf(type, reason, id, part, psinfo->buf, compressed,
- size, psi);
+ return psi->write_buf(type, reason, id, part, pstore_get_buf(psinfo),
+ compressed, size, psi);
}
/*
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 9790904de6d2..14f524177b1f 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -52,6 +52,7 @@ struct pstore_info {
spinlock_t buf_lock; /* serialize access to 'buf' */
char *buf;
size_t bufsize;
+ size_t bufpos;
struct mutex read_mutex; /* serialize open/read/close */
int flags;
int (*open)(struct pstore_info *psi);
@@ -79,8 +80,11 @@ struct pstore_info {
#define PSTORE_FLAGS_FTRACE (1 << 2)
#define PSTORE_FLAGS_PMSG (1 << 3)
+#define PSTORE_FLAGS_ASYNC (1 << 30)
+
extern int pstore_register(struct pstore_info *);
extern void pstore_unregister(struct pstore_info *);
extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason);
+extern void *pstore_get_buf(struct pstore_info *);
#endif /*_LINUX_PSTORE_H*/
--
2.8.0
^ permalink raw reply related
* [PATCH 2/7] pstore/ram: Set pstore flags dynamically
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Anton Vorontsov, LKML, Kees Cook, Colin Cross
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
The ramoops can be configured to enable each pstore type by setting
their size. In that case, it'd be better not to register disabled types
in the first place.
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
fs/pstore/ram.c | 8 +++++++-
include/linux/pstore.h | 2 --
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index ba19a74e95bc..6c93268f7ced 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -624,7 +624,13 @@ static int ramoops_probe(struct platform_device *pdev)
goto fail_clear;
}
- cxt->pstore.flags = PSTORE_FLAGS_ALL;
+ cxt->pstore.flags = PSTORE_FLAGS_DMESG;
+ if (cxt->console_size)
+ cxt->pstore.flags |= PSTORE_FLAGS_CONSOLE;
+ if (cxt->ftrace_size)
+ cxt->pstore.flags |= PSTORE_FLAGS_FTRACE;
+ if (cxt->pmsg_size)
+ cxt->pstore.flags |= PSTORE_FLAGS_PMSG;
err = pstore_register(&cxt->pstore);
if (err) {
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 069b96faf478..9790904de6d2 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -79,8 +79,6 @@ struct pstore_info {
#define PSTORE_FLAGS_FTRACE (1 << 2)
#define PSTORE_FLAGS_PMSG (1 << 3)
-#define PSTORE_FLAGS_ALL ((1 << 4) - 1)
-
extern int pstore_register(struct pstore_info *);
extern void pstore_unregister(struct pstore_info *);
extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason);
--
2.8.0
^ permalink raw reply related
* [PATCH 1/7] pstore: Split pstore fragile flags
From: Namhyung Kim @ 2016-07-27 15:08 UTC (permalink / raw)
To: kvm, qemu-devel, virtualization
Cc: Tony Luck, Kees Cook, Matt Fleming, Anton Vorontsov,
Rafael J. Wysocki, LKML, linux-acpi, linux-efi, Colin Cross,
Len Brown
In-Reply-To: <1469632111-23260-1-git-send-email-namhyung@kernel.org>
This patch adds new PSTORE_FLAGS for each pstore type so that they can
be enabled separately. This is a preparation for ongoing virtio-pstore
work to support those types flexibly.
The PSTORE_FLAGS_FRAGILE is changed to PSTORE_FLAGS_DMESG to preserve the
original behavior.
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-acpi@vger.kernel.org
Cc: linux-efi@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
drivers/acpi/apei/erst.c | 2 +-
drivers/firmware/efi/efi-pstore.c | 2 +-
fs/pstore/platform.c | 17 ++++++++++-------
fs/pstore/ram.c | 2 ++
include/linux/pstore.h | 7 ++++++-
5 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index f096ab3cb54d..ec4f507b524f 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -938,7 +938,7 @@ static int erst_clearer(enum pstore_type_id type, u64 id, int count,
static struct pstore_info erst_info = {
.owner = THIS_MODULE,
.name = "erst",
- .flags = PSTORE_FLAGS_FRAGILE,
+ .flags = PSTORE_FLAGS_DMESG,
.open = erst_open_pstore,
.close = erst_close_pstore,
.read = erst_reader,
diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index 30a24d09ea6c..4daa5acd9117 100644
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -362,7 +362,7 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count,
static struct pstore_info efi_pstore_info = {
.owner = THIS_MODULE,
.name = "efi",
- .flags = PSTORE_FLAGS_FRAGILE,
+ .flags = PSTORE_FLAGS_DMESG,
.open = efi_pstore_open,
.close = efi_pstore_close,
.read = efi_pstore_read,
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 16ecca5b72d8..76dd604a0f2c 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -659,13 +659,14 @@ int pstore_register(struct pstore_info *psi)
if (pstore_is_mounted())
pstore_get_records(0);
- pstore_register_kmsg();
-
- if ((psi->flags & PSTORE_FLAGS_FRAGILE) == 0) {
+ if (psi->flags & PSTORE_FLAGS_DMESG)
+ pstore_register_kmsg();
+ if (psi->flags & PSTORE_FLAGS_CONSOLE)
pstore_register_console();
+ if (psi->flags & PSTORE_FLAGS_FTRACE)
pstore_register_ftrace();
+ if (psi->flags & PSTORE_FLAGS_PMSG)
pstore_register_pmsg();
- }
if (pstore_update_ms >= 0) {
pstore_timer.expires = jiffies +
@@ -689,12 +690,14 @@ EXPORT_SYMBOL_GPL(pstore_register);
void pstore_unregister(struct pstore_info *psi)
{
- if ((psi->flags & PSTORE_FLAGS_FRAGILE) == 0) {
+ if (psi->flags & PSTORE_FLAGS_PMSG)
pstore_unregister_pmsg();
+ if (psi->flags & PSTORE_FLAGS_FTRACE)
pstore_unregister_ftrace();
+ if (psi->flags & PSTORE_FLAGS_CONSOLE)
pstore_unregister_console();
- }
- pstore_unregister_kmsg();
+ if (psi->flags & PSTORE_FLAGS_DMESG)
+ pstore_unregister_kmsg();
free_buf_for_compression();
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
index 47516a794011..ba19a74e95bc 100644
--- a/fs/pstore/ram.c
+++ b/fs/pstore/ram.c
@@ -624,6 +624,8 @@ static int ramoops_probe(struct platform_device *pdev)
goto fail_clear;
}
+ cxt->pstore.flags = PSTORE_FLAGS_ALL;
+
err = pstore_register(&cxt->pstore);
if (err) {
pr_err("registering with pstore failed\n");
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index 899e95e84400..069b96faf478 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -74,7 +74,12 @@ struct pstore_info {
void *data;
};
-#define PSTORE_FLAGS_FRAGILE 1
+#define PSTORE_FLAGS_DMESG (1 << 0)
+#define PSTORE_FLAGS_CONSOLE (1 << 1)
+#define PSTORE_FLAGS_FTRACE (1 << 2)
+#define PSTORE_FLAGS_PMSG (1 << 3)
+
+#define PSTORE_FLAGS_ALL ((1 << 4) - 1)
extern int pstore_register(struct pstore_info *);
extern void pstore_unregister(struct pstore_info *);
--
2.8.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox