| Message ID | 1468816661-6345-3-git-send-email-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/18/2016 06:37 AM, Namhyung Kim wrote: Can you do the virtio-mmio and virtio-ccw plumbing as well, or do you need help with that? [...] > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 2b34b43..8281b80 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -2416,6 +2416,55 @@ 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); > +} > + > +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, > @@ -2485,6 +2534,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 > [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Mon, Jul 18, 2016 at 09:28:42AM +0200, Christian Borntraeger wrote: > On 07/18/2016 06:37 AM, Namhyung Kim wrote: > > Can you do the virtio-mmio and virtio-ccw plumbing as well, or > do you need help with that? Any help would be greatly appreciated! Thanks, Namhyung > > [...] > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index 2b34b43..8281b80 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -2416,6 +2416,55 @@ 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); > > +} > > + > > +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, > > @@ -2485,6 +2534,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 > > > > [...] > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > From: Namhyung Kim <namhyung@gmail.com> > > 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-0.enc.z dmesg-1.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 implementation is synchronous (i.e. can pause guest code execution), does not handle write errors, and does not limit the amount of data the guest can write. This is sufficient for ad-hoc debugging and usage with trusted guests. If you want this to be available in environments where the guest isn't trusted then there must be a limit on how much the guest can write or some kind of log rotation. > > 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@gmail.com> > --- > hw/virtio/Makefile.objs | 2 +- > hw/virtio/virtio-pci.c | 50 ++++ > hw/virtio/virtio-pci.h | 14 + > hw/virtio/virtio-pstore.c | 328 +++++++++++++++++++++ > include/hw/pci/pci.h | 1 + > include/hw/virtio/virtio-pstore.h | 30 ++ > include/standard-headers/linux/virtio_ids.h | 1 + > .../linux/{virtio_ids.h => virtio_pstore.h} | 48 +-- > qdev-monitor.c | 1 + > 9 files changed, 455 insertions(+), 20 deletions(-) > create mode 100644 hw/virtio/virtio-pstore.c > create mode 100644 include/hw/virtio/virtio-pstore.h > copy include/standard-headers/linux/{virtio_ids.h => virtio_pstore.h} (63%) > > 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 2b34b43..8281b80 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -2416,6 +2416,55 @@ 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); > +} > + > +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, > @@ -2485,6 +2534,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..98cee7f > --- /dev/null > +++ b/hw/virtio/virtio-pstore.c > @@ -0,0 +1,328 @@ > +/* > + * 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_hdr *hdr) > +{ > + const char *basename; > + > + switch (hdr->type) { Missing le16_to_cpu()? > + case VIRTIO_PSTORE_TYPE_DMESG: > + basename = "dmesg"; > + break; > + default: > + basename = "unknown"; > + break; > + } > + > + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, > + (unsigned long long) hdr->id, Missing le64_to_cpu()? > + hdr->flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : ""); Missing le32_to_cpu()? > +} > + > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name, > + char *buf, size_t sz, > + struct virtio_pstore_hdr *hdr) > +{ > + size_t len = strlen(name); > + > + hdr->flags = 0; > + if (!strncmp(name + len - 6, ".enc.z", 6)) { Please use g_str_has_suffix(name, ".enc.z") to avoid accessing before the beginning of the string if the filename is shorter than 6 characters. > + hdr->flags |= VIRTIO_PSTORE_FL_COMPRESSED; > + } > + > + snprintf(buf, sz, "%s/%s", s->directory, name); > + > + if (!strncmp(name, "dmesg-", 6)) { g_str_has_prefix(name, "dmesg-") > + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_DMESG); > + name += 6; > + } else if (!strncmp(name, "unknown-", 8)) { g_str_has_prefix(name, "unknown-") > + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN); > + name += 8; > + } > + > + qemu_strtoull(name, NULL, 0, &hdr->id); > +} > + > +static ssize_t virtio_pstore_do_open(VirtIOPstore *s) > +{ > + s->dir = opendir(s->directory); > + if (s->dir == NULL) { > + return -1; > + } > + > + return 0; > +} > + > +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, void *buf, size_t sz, > + struct virtio_pstore_hdr *hdr) > +{ > + char path[PATH_MAX]; > + FILE *fp; > + ssize_t len; > + struct stat stbuf; > + struct dirent *dent; > + > + if (s->dir == NULL) { > + return -1; > + } > + > + dent = readdir(s->dir); > + while (dent) { > + if (dent->d_name[0] != '.') { > + break; > + } > + dent = readdir(s->dir); > + } > + > + if (dent == NULL) { > + return 0; > + } > + > + virtio_pstore_from_filename(s, dent->d_name, path, sizeof(path), hdr); > + if (stat(path, &stbuf) < 0) { > + return -1; > + } Please use fstat(fileno(fp), &stbuf) after opening the file instead. The race condition doesn't matter in this case but the race-free code is just as simple so it's one less thing someone reading the code has to worry about. > + > + fp = fopen(path, "r"); > + if (fp == NULL) { > + error_report("cannot open %s (%p %p)", path, s, s->directory); > + return -1; > + } > + > + len = fread(buf, 1, sz, fp); > + if (len < 0 && errno == EAGAIN) { > + len = 0; > + } > + > + hdr->id = cpu_to_le64(hdr->id); > + hdr->flags = cpu_to_le32(hdr->flags); > + hdr->time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec); > + hdr->time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec); > + > + fclose(fp); > + return len; > +} > + > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, void *buf, size_t sz, > + struct virtio_pstore_hdr *hdr) > +{ > + char path[PATH_MAX]; > + FILE *fp; > + > + virtio_pstore_to_filename(s, path, sizeof(path), hdr); > + > + fp = fopen(path, "w"); > + if (fp == NULL) { > + error_report("cannot open %s (%p %p)", path, s, s->directory); > + return -1; > + } > + fwrite(buf, 1, sz, fp); > + fclose(fp); > + > + return sz; > +} > + > +static ssize_t virtio_pstore_do_close(VirtIOPstore *s) > +{ > + if (s->dir == NULL) { > + return 0; > + } > + > + closedir(s->dir); > + s->dir = NULL; > + > + return 0; > +} > + > +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s, > + struct virtio_pstore_hdr *hdr) > +{ > + char path[PATH_MAX]; > + > + virtio_pstore_to_filename(s, path, sizeof(path), hdr); > + > + return unlink(path); > +} > + > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > +{ > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > + VirtQueueElement *elem; > + struct virtio_pstore_hdr *hdr; > + ssize_t len; > + > + for (;;) { > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > + if (!elem) { > + return; > + } > + > + hdr = elem->out_sg[0].iov_base; > + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { > + error_report("invalid header size: %u", > + (unsigned)elem->out_sg[0].iov_len); > + exit(1); > + } Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio devices are not supposed to assume a particular iovec layout. In other words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. You must also copy in data (similar to Linux syscall implementations) to prevent the guest from modifying data while the command is processed. Such race conditions could lead to security bugs. > + > + switch (hdr->cmd) { > + case VIRTIO_PSTORE_CMD_OPEN: > + len = virtio_pstore_do_open(s); > + break; > + case VIRTIO_PSTORE_CMD_READ: > + len = virtio_pstore_do_read(s, elem->in_sg[0].iov_base, > + elem->in_sg[0].iov_len, hdr); Same issue with iovec layout for in_sg[] here. The guest driver must be able to submit any in_sg[] iovec array and the device cannot assume in_sg[0] is the only iovec to fill. > + break; > + case VIRTIO_PSTORE_CMD_WRITE: > + len = virtio_pstore_do_write(s, elem->out_sg[1].iov_base, > + elem->out_sg[1].iov_len, hdr); > + break; > + case VIRTIO_PSTORE_CMD_CLOSE: > + len = virtio_pstore_do_close(s); > + break; > + case VIRTIO_PSTORE_CMD_ERASE: > + len = virtio_pstore_do_erase(s, hdr); > + break; > + default: > + len = -1; > + break; > + } > + > + if (len < 0) { > + return; > + } > + > + virtqueue_push(vq, elem, len); > + > + virtio_notify(vdev, vq); > + g_free(elem); > + } > +} > + > +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, 0); > + > + s->vq = 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 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 = strdup(value); Please use g_strdup() since this is paired with g_free(). Or even simpler would be s->directory = value and do not g_free(value) below. > + > + g_free(value); > +} > + > +static void pstore_release_directory(Object *obj, const char *name, > + void *opaque) > +{ > + VirtIOPstore *s = opaque; > + > + g_free(s->directory); > + s->directory = NULL; > +} > + > +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); > +} > + > +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_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 9ed1624..5689c6f 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..74cd1f6 > --- /dev/null > +++ b/include/hw/virtio/virtio-pstore.h > @@ -0,0 +1,30 @@ > +/* > + * 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; > + char *directory; > + DIR *dir; > +} VirtIOPstore; > + > +#endif > diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h > index 77925f5..cba6322 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 19 /* virtio pstore */ 19 has already been reserved. 22 is the next free ID (vsock, crypto, and sdm are currently under review and already use 19, 20, and 21). Please send a VIRTIO draft specification to virtio-dev@lists.oasis-open.org. You can find information on the VIRTIO standards process here: https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio
Hello, On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > From: Namhyung Kim <namhyung@gmail.com> > > > > 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-0.enc.z dmesg-1.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 implementation is synchronous (i.e. can pause guest code execution), > does not handle write errors, and does not limit the amount of data the > guest can write. This is sufficient for ad-hoc debugging and usage with > trusted guests. > > If you want this to be available in environments where the guest isn't > trusted then there must be a limit on how much the guest can write or > some kind of log rotation. Right. The synchronous IO is required by the pstore subsystem implementation AFAIK (it uses a single psinfo->buf in the loop). And I agree that it should have a way to handle write errors and to limit amount of data. > > > > > 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@gmail.com> > > --- [SNIP] > > + > > +static void virtio_pstore_to_filename(VirtIOPstore *s, char *buf, size_t sz, > > + struct virtio_pstore_hdr *hdr) > > +{ > > + const char *basename; > > + > > + switch (hdr->type) { > > Missing le16_to_cpu()? > > > + case VIRTIO_PSTORE_TYPE_DMESG: > > + basename = "dmesg"; > > + break; > > + default: > > + basename = "unknown"; > > + break; > > + } > > + > > + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, > > + (unsigned long long) hdr->id, > > Missing le64_to_cpu()? > > > + hdr->flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" : ""); > > Missing le32_to_cpu()? Oops, will fix. > > > +} > > + > > +static void virtio_pstore_from_filename(VirtIOPstore *s, char *name, > > + char *buf, size_t sz, > > + struct virtio_pstore_hdr *hdr) > > +{ > > + size_t len = strlen(name); > > + > > + hdr->flags = 0; > > + if (!strncmp(name + len - 6, ".enc.z", 6)) { > > Please use g_str_has_suffix(name, ".enc.z") to avoid accessing before > the beginning of the string if the filename is shorter than 6 > characters. Ah, ok. > > > + hdr->flags |= VIRTIO_PSTORE_FL_COMPRESSED; > > + } > > + > > + snprintf(buf, sz, "%s/%s", s->directory, name); > > + > > + if (!strncmp(name, "dmesg-", 6)) { > > g_str_has_prefix(name, "dmesg-") > > > + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_DMESG); > > + name += 6; > > + } else if (!strncmp(name, "unknown-", 8)) { > > g_str_has_prefix(name, "unknown-") Will change. > > > + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN); > > + name += 8; > > + } > > + > > + qemu_strtoull(name, NULL, 0, &hdr->id); > > +} > > + > > +static ssize_t virtio_pstore_do_open(VirtIOPstore *s) > > +{ > > + s->dir = opendir(s->directory); > > + if (s->dir == NULL) { > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, void *buf, size_t sz, > > + struct virtio_pstore_hdr *hdr) > > +{ > > + char path[PATH_MAX]; > > + FILE *fp; > > + ssize_t len; > > + struct stat stbuf; > > + struct dirent *dent; > > + > > + if (s->dir == NULL) { > > + return -1; > > + } > > + > > + dent = readdir(s->dir); > > + while (dent) { > > + if (dent->d_name[0] != '.') { > > + break; > > + } > > + dent = readdir(s->dir); > > + } > > + > > + if (dent == NULL) { > > + return 0; > > + } > > + > > + virtio_pstore_from_filename(s, dent->d_name, path, sizeof(path), hdr); > > + if (stat(path, &stbuf) < 0) { > > + return -1; > > + } > > Please use fstat(fileno(fp), &stbuf) after opening the file instead. > The race condition doesn't matter in this case but the race-free code is > just as simple so it's one less thing someone reading the code has to > worry about. Fair enough. > > > + > > + fp = fopen(path, "r"); > > + if (fp == NULL) { > > + error_report("cannot open %s (%p %p)", path, s, s->directory); > > + return -1; > > + } > > + > > + len = fread(buf, 1, sz, fp); > > + if (len < 0 && errno == EAGAIN) { > > + len = 0; > > + } > > + > > + hdr->id = cpu_to_le64(hdr->id); > > + hdr->flags = cpu_to_le32(hdr->flags); > > + hdr->time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec); > > + hdr->time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec); > > + > > + fclose(fp); > > + return len; > > +} > > + [SNIP] > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > > +{ > > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > > + VirtQueueElement *elem; > > + struct virtio_pstore_hdr *hdr; > > + ssize_t len; > > + > > + for (;;) { > > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > > + if (!elem) { > > + return; > > + } > > + > > + hdr = elem->out_sg[0].iov_base; > > + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { > > + error_report("invalid header size: %u", > > + (unsigned)elem->out_sg[0].iov_len); > > + exit(1); > > + } > > Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio > devices are not supposed to assume a particular iovec layout. In other > words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. I got it. > > You must also copy in data (similar to Linux syscall implementations) to > prevent the guest from modifying data while the command is processed. > Such race conditions could lead to security bugs. Ok, but this assumes the operation is synchronous. I agree on your opinion if I could make it async. > > > + > > + switch (hdr->cmd) { > > + case VIRTIO_PSTORE_CMD_OPEN: > > + len = virtio_pstore_do_open(s); > > + break; > > + case VIRTIO_PSTORE_CMD_READ: > > + len = virtio_pstore_do_read(s, elem->in_sg[0].iov_base, > > + elem->in_sg[0].iov_len, hdr); > > Same issue with iovec layout for in_sg[] here. The guest driver must be > able to submit any in_sg[] iovec array and the device cannot assume > in_sg[0] is the only iovec to fill. Ok. > > > + break; > > + case VIRTIO_PSTORE_CMD_WRITE: > > + len = virtio_pstore_do_write(s, elem->out_sg[1].iov_base, > > + elem->out_sg[1].iov_len, hdr); > > + break; > > + case VIRTIO_PSTORE_CMD_CLOSE: > > + len = virtio_pstore_do_close(s); > > + break; > > + case VIRTIO_PSTORE_CMD_ERASE: > > + len = virtio_pstore_do_erase(s, hdr); > > + break; > > + default: > > + len = -1; > > + break; > > + } > > + > > + if (len < 0) { > > + return; > > + } > > + > > + virtqueue_push(vq, elem, len); > > + > > + virtio_notify(vdev, vq); > > + g_free(elem); > > + } > > +} > > + > > +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, 0); > > + > > + s->vq = 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 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 = strdup(value); > > Please use g_strdup() since this is paired with g_free(). > > Or even simpler would be s->directory = value and do not g_free(value) > below. Ok, I was not sure whether I could use it without alloc/free pair. Will do it simpler way then. :) > > > + > > + g_free(value); > > +} > > + > > +static void pstore_release_directory(Object *obj, const char *name, > > + void *opaque) > > +{ > > + VirtIOPstore *s = opaque; > > + > > + g_free(s->directory); > > + s->directory = NULL; > > +} > > + > > +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); > > +} > > + > > +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_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 9ed1624..5689c6f 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..74cd1f6 > > --- /dev/null > > +++ b/include/hw/virtio/virtio-pstore.h > > @@ -0,0 +1,30 @@ > > +/* > > + * 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; > > + char *directory; > > + DIR *dir; > > +} VirtIOPstore; > > + > > +#endif > > diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h > > index 77925f5..cba6322 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 19 /* virtio pstore */ > > 19 has already been reserved. 22 is the next free ID (vsock, crypto, > and sdm are currently under review and already use 19, 20, and 21). I wasn't aware of the ongoing works but Cornelia already told me about it. Will update. > > Please send a VIRTIO draft specification to > virtio-dev@lists.oasis-open.org. You can find information on the VIRTIO > standards process here: > https://www.oasis-open.org/committees/tc_home.php?wg_abbrev=virtio Thank you very much for this information and your detailed review! I'll take a look at the virtio standards process too. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > > +{ > > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > > + VirtQueueElement *elem; > > + struct virtio_pstore_hdr *hdr; > > + ssize_t len; > > + > > + for (;;) { > > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > > + if (!elem) { > > + return; > > + } > > + > > + hdr = elem->out_sg[0].iov_base; > > + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { > > + error_report("invalid header size: %u", > > + (unsigned)elem->out_sg[0].iov_len); > > + exit(1); > > + } > > Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio > devices are not supposed to assume a particular iovec layout. In other > words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. > > You must also copy in data (similar to Linux syscall implementations) to > prevent the guest from modifying data while the command is processed. > Such race conditions could lead to security bugs. By accessing elem->out_sg[0].iov_base directly, I abused it as an in-and-out buffer. But it seems not allowed by the virtio spec, do I have to use separate buffers for request and response? Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 20, 2016 at 12:48:39AM +0900, Namhyung Kim wrote: > Hello, > > On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > > > +{ > > > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > > > + VirtQueueElement *elem; > > > + struct virtio_pstore_hdr *hdr; > > > + ssize_t len; > > > + > > > + for (;;) { > > > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > > > + if (!elem) { > > > + return; > > > + } > > > + > > > + hdr = elem->out_sg[0].iov_base; > > > + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { > > > + error_report("invalid header size: %u", > > > + (unsigned)elem->out_sg[0].iov_len); > > > + exit(1); > > > + } > > > > Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio > > devices are not supposed to assume a particular iovec layout. In other > > words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. > > > > You must also copy in data (similar to Linux syscall implementations) to > > prevent the guest from modifying data while the command is processed. > > Such race conditions could lead to security bugs. > > By accessing elem->out_sg[0].iov_base directly, I abused it as an > in-and-out buffer. But it seems not allowed by the virtio spec, do I > have to use separate buffers for request and response? Yes, a virtqueue element has (host read-only) out buffers followed by (host write-only) in buffers. Stefan
On Mon, Jul 18, 2016 at 11:21:18PM +0900, Namhyung Kim wrote: > On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > > From: Namhyung Kim <namhyung@gmail.com> > > > > > > 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-0.enc.z dmesg-1.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 implementation is synchronous (i.e. can pause guest code execution), > > does not handle write errors, and does not limit the amount of data the > > guest can write. This is sufficient for ad-hoc debugging and usage with > > trusted guests. > > > > If you want this to be available in environments where the guest isn't > > trusted then there must be a limit on how much the guest can write or > > some kind of log rotation. > > Right. The synchronous IO is required by the pstore subsystem > implementation AFAIK (it uses a single psinfo->buf in the loop). The pstore subsystem in Linux may be synchronous but the QEMU device emulation does not have to be synchronous. Synchronous device emulation means that no other vcpu or QEMU main loop processing can occur while device emulation is blocked in a syscall. This can make the QEMU monitor unavailable for libvirt and management tools. The guest can experience jitter since vcpus freeze if they vmexit while device emulation is blocked (it holds the QEMU global mutex and prevents other QEMU threads from making progress). You could use include/io.h for asynchronous I/O (qio_channel_add_watch()). Stefan
On Wed, Jul 20, 2016 at 09:21:08AM +0100, Stefan Hajnoczi wrote: > On Wed, Jul 20, 2016 at 12:48:39AM +0900, Namhyung Kim wrote: > > Hello, > > > > On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > > > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > > > +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) > > > > +{ > > > > + VirtIOPstore *s = VIRTIO_PSTORE(vdev); > > > > + VirtQueueElement *elem; > > > > + struct virtio_pstore_hdr *hdr; > > > > + ssize_t len; > > > > + > > > > + for (;;) { > > > > + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); > > > > + if (!elem) { > > > > + return; > > > > + } > > > > + > > > > + hdr = elem->out_sg[0].iov_base; > > > > + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { > > > > + error_report("invalid header size: %u", > > > > + (unsigned)elem->out_sg[0].iov_len); > > > > + exit(1); > > > > + } > > > > > > Please use iov_to_buf() instead of directly accessing out_sg[]. Virtio > > > devices are not supposed to assume a particular iovec layout. In other > > > words, virtio_pstore_hdr could be split across multiple out_sg[] iovecs. > > > > > > You must also copy in data (similar to Linux syscall implementations) to > > > prevent the guest from modifying data while the command is processed. > > > Such race conditions could lead to security bugs. > > > > By accessing elem->out_sg[0].iov_base directly, I abused it as an > > in-and-out buffer. But it seems not allowed by the virtio spec, do I > > have to use separate buffers for request and response? > > Yes, a virtqueue element has (host read-only) out buffers followed by > (host write-only) in buffers. Thanks for clarification. I'll split them. Thanks, Namhyung -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 20, 2016 at 09:29:06AM +0100, Stefan Hajnoczi wrote: > On Mon, Jul 18, 2016 at 11:21:18PM +0900, Namhyung Kim wrote: > > On Mon, Jul 18, 2016 at 11:03:53AM +0100, Stefan Hajnoczi wrote: > > > On Mon, Jul 18, 2016 at 01:37:40PM +0900, Namhyung Kim wrote: > > > > From: Namhyung Kim <namhyung@gmail.com> > > > > > > > > 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-0.enc.z dmesg-1.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 implementation is synchronous (i.e. can pause guest code execution), > > > does not handle write errors, and does not limit the amount of data the > > > guest can write. This is sufficient for ad-hoc debugging and usage with > > > trusted guests. > > > > > > If you want this to be available in environments where the guest isn't > > > trusted then there must be a limit on how much the guest can write or > > > some kind of log rotation. > > > > Right. The synchronous IO is required by the pstore subsystem > > implementation AFAIK (it uses a single psinfo->buf in the loop). > > The pstore subsystem in Linux may be synchronous but the QEMU device > emulation does not have to be synchronous. > > Synchronous device emulation means that no other vcpu or QEMU main loop > processing can occur while device emulation is blocked in a syscall. > This can make the QEMU monitor unavailable for libvirt and management > tools. The guest can experience jitter since vcpus freeze if they > vmexit while device emulation is blocked (it holds the QEMU global > mutex and prevents other QEMU threads from making progress). Thanks for your detailed explanation. I'll try to change pstore implementation to deal with async devices. Thanks, Namhyung > > You could use include/io.h for asynchronous I/O (qio_channel_add_watch()). > > Stefan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 2b34b43..8281b80 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -2416,6 +2416,55 @@ 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); +} + +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, @@ -2485,6 +2534,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..98cee7f --- /dev/null +++ b/hw/virtio/virtio-pstore.c @@ -0,0 +1,328 @@ +/* + * 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_hdr *hdr) +{ + const char *basename; + + switch (hdr->type) { + case VIRTIO_PSTORE_TYPE_DMESG: + basename = "dmesg"; + break; + default: + basename = "unknown"; + break; + } + + snprintf(buf, sz, "%s/%s-%llu%s", s->directory, basename, + (unsigned long long) hdr->id, + hdr->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_hdr *hdr) +{ + size_t len = strlen(name); + + hdr->flags = 0; + if (!strncmp(name + len - 6, ".enc.z", 6)) { + hdr->flags |= VIRTIO_PSTORE_FL_COMPRESSED; + } + + snprintf(buf, sz, "%s/%s", s->directory, name); + + if (!strncmp(name, "dmesg-", 6)) { + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_DMESG); + name += 6; + } else if (!strncmp(name, "unknown-", 8)) { + hdr->type = cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN); + name += 8; + } + + qemu_strtoull(name, NULL, 0, &hdr->id); +} + +static ssize_t virtio_pstore_do_open(VirtIOPstore *s) +{ + s->dir = opendir(s->directory); + if (s->dir == NULL) { + return -1; + } + + return 0; +} + +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, void *buf, size_t sz, + struct virtio_pstore_hdr *hdr) +{ + char path[PATH_MAX]; + FILE *fp; + ssize_t len; + struct stat stbuf; + struct dirent *dent; + + if (s->dir == NULL) { + return -1; + } + + dent = readdir(s->dir); + while (dent) { + if (dent->d_name[0] != '.') { + break; + } + dent = readdir(s->dir); + } + + if (dent == NULL) { + return 0; + } + + virtio_pstore_from_filename(s, dent->d_name, path, sizeof(path), hdr); + if (stat(path, &stbuf) < 0) { + return -1; + } + + fp = fopen(path, "r"); + if (fp == NULL) { + error_report("cannot open %s (%p %p)", path, s, s->directory); + return -1; + } + + len = fread(buf, 1, sz, fp); + if (len < 0 && errno == EAGAIN) { + len = 0; + } + + hdr->id = cpu_to_le64(hdr->id); + hdr->flags = cpu_to_le32(hdr->flags); + hdr->time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec); + hdr->time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec); + + fclose(fp); + return len; +} + +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, void *buf, size_t sz, + struct virtio_pstore_hdr *hdr) +{ + char path[PATH_MAX]; + FILE *fp; + + virtio_pstore_to_filename(s, path, sizeof(path), hdr); + + fp = fopen(path, "w"); + if (fp == NULL) { + error_report("cannot open %s (%p %p)", path, s, s->directory); + return -1; + } + fwrite(buf, 1, sz, fp); + fclose(fp); + + return sz; +} + +static ssize_t virtio_pstore_do_close(VirtIOPstore *s) +{ + if (s->dir == NULL) { + return 0; + } + + closedir(s->dir); + s->dir = NULL; + + return 0; +} + +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s, + struct virtio_pstore_hdr *hdr) +{ + char path[PATH_MAX]; + + virtio_pstore_to_filename(s, path, sizeof(path), hdr); + + return unlink(path); +} + +static void virtio_pstore_handle_io(VirtIODevice *vdev, VirtQueue *vq) +{ + VirtIOPstore *s = VIRTIO_PSTORE(vdev); + VirtQueueElement *elem; + struct virtio_pstore_hdr *hdr; + ssize_t len; + + for (;;) { + elem = virtqueue_pop(vq, sizeof(VirtQueueElement)); + if (!elem) { + return; + } + + hdr = elem->out_sg[0].iov_base; + if (elem->out_sg[0].iov_len != sizeof(*hdr)) { + error_report("invalid header size: %u", + (unsigned)elem->out_sg[0].iov_len); + exit(1); + } + + switch (hdr->cmd) { + case VIRTIO_PSTORE_CMD_OPEN: + len = virtio_pstore_do_open(s); + break; + case VIRTIO_PSTORE_CMD_READ: + len = virtio_pstore_do_read(s, elem->in_sg[0].iov_base, + elem->in_sg[0].iov_len, hdr); + break; + case VIRTIO_PSTORE_CMD_WRITE: + len = virtio_pstore_do_write(s, elem->out_sg[1].iov_base, + elem->out_sg[1].iov_len, hdr); + break; + case VIRTIO_PSTORE_CMD_CLOSE: + len = virtio_pstore_do_close(s); + break; + case VIRTIO_PSTORE_CMD_ERASE: + len = virtio_pstore_do_erase(s, hdr); + break; + default: + len = -1; + break; + } + + if (len < 0) { + return; + } + + virtqueue_push(vq, elem, len); + + virtio_notify(vdev, vq); + g_free(elem); + } +} + +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, 0); + + s->vq = 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 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 = strdup(value); + + g_free(value); +} + +static void pstore_release_directory(Object *obj, const char *name, + void *opaque) +{ + VirtIOPstore *s = opaque; + + g_free(s->directory); + s->directory = NULL; +} + +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); +} + +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_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 9ed1624..5689c6f 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..74cd1f6 --- /dev/null +++ b/include/hw/virtio/virtio-pstore.h @@ -0,0 +1,30 @@ +/* + * 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; + char *directory; + DIR *dir; +} VirtIOPstore; + +#endif diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h index 77925f5..cba6322 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 19 /* virtio pstore */ #endif /* _LINUX_VIRTIO_IDS_H */ diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_pstore.h similarity index 63% copy from include/standard-headers/linux/virtio_ids.h copy to include/standard-headers/linux/virtio_pstore.h index 77925f5..1b89cad 100644 --- a/include/standard-headers/linux/virtio_ids.h +++ b/include/standard-headers/linux/virtio_pstore.h @@ -1,9 +1,6 @@ -#ifndef _LINUX_VIRTIO_IDS_H -#define _LINUX_VIRTIO_IDS_H -/* - * Virtio IDs - * - * This header is BSD licensed so anyone can use the definitions to implement +#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 @@ -28,18 +25,31 @@ * 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_TYPE_UNKNOWN 0 +#define VIRTIO_PSTORE_TYPE_DMESG 1 + +#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_FL_COMPRESSED 1 -#define VIRTIO_ID_NET 1 /* virtio net */ -#define VIRTIO_ID_BLOCK 2 /* virtio block */ -#define VIRTIO_ID_CONSOLE 3 /* virtio console */ -#define VIRTIO_ID_RNG 4 /* virtio rng */ -#define VIRTIO_ID_BALLOON 5 /* virtio balloon */ -#define VIRTIO_ID_RPMSG 7 /* virtio remote processor messaging */ -#define VIRTIO_ID_SCSI 8 /* virtio scsi */ -#define VIRTIO_ID_9P 9 /* 9p virtio console */ -#define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */ -#define VIRTIO_ID_CAIF 12 /* Virtio caif */ -#define VIRTIO_ID_GPU 16 /* virtio GPU */ -#define VIRTIO_ID_INPUT 18 /* virtio input */ +struct virtio_pstore_hdr { + __virtio64 id; + __virtio32 flags; + __virtio16 cmd; + __virtio16 type; + __virtio64 time_sec; + __virtio32 time_nsec; + __virtio32 unused; +}; -#endif /* _LINUX_VIRTIO_IDS_H */ +#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" }, { } };