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

[Namhyung Kim]

Hi,

Thanks for your detailed information,

On Wed, Nov 16, 2016 at 07:10:36AM -0500, Paolo Bonzini wrote:
> > Not sure how independent ERST is from ACPI and other specs.  It looks
> > like referencing UEFI spec at least.
> 
> It is just the format of error records that comes from the UEFI spec
> (include/linux/cper.h) but you can ignore it, I think.  It should be
> handled by tools on the host side.  For you, the error log address
> range contains a CPER header followed by a binary blob.  In practice,
> you only need the record length field (bytes 20-23 of the header),
> though it may be a good idea to validate the signature at the beginning
> of the header.
> 
> > Btw, is the ERST used for pstore only (in Linux)?
> 
> Yes.  It can store various records, including dmesg and MCE.
> 
> There are other examples in QEMU of interfaces with ACPI.  They all use the
> DSDT, but the logic is similar.  For example, docs/specs/acpi_mem_hotplug.txt
> documents the memory hotplug interface. In all cases, ACPI tables contain 
> small
> programs that talk to specialized hardware registers, typically allocated to
> hard-coded I/O ports.
> 
> In your case, the registers could occupy 16 consecutive I/O ports, like the
> following:
> 
>  0x00   read/write   operation type (0=write,1=read,2=clear,3=dummy 
> write)
> 
>  0x01   read-onlybit 7: if set, operation in progress
> 
>  bit 0-6: operation status, see "Command Status 
> Definition" in
>  the ACPI spec
> 
>  0x02   read-onlywhen read:
> 
>  - read a 64-bit record id from the store to 
> memory,
>from the address that was last written to 0x08.
> 
>  - if the id is valid and is not the last id in 
> the store,
>write the next 64-bit record id to the same 
> address
> 
>  - otherwise, write the first record id to the 
> same address,
>or 0x if the store is empty
> 
>  0x03unused, read as zero
> 
>  0x04-0x07  read/write   offset of the error record into the error log 
> address range
> 
>  0x08-0x0b  read/write   when read, return number of stored records
> 
>  when written, the written value is a 32-bit 
> memory address,
>  which points to a 64-bit location used to 
> communicate record ids.
> 
>  0x0c-0x0f  read/write   when read, always return -1 (together with the 
> "mask" field
>  and READ_REGISTER, this lets ERST instructions 
> return any value!)
> 
>  when written, trigger the pstore operation:
> 
>  - if the current operation is a dummy write, do 
> nothing
> 
>  - if the current operation is a write, write a 
> new record, using
>  the written value as the base of the error log 
> address range.  The
>  length must be parsed from the CPER header.
> 
>  - if the current operation is a clear, read the 
> record id
>  from the memory location that was last written 
> to 0x08 and do the
>  operation.  the value written is ignored.
> 
>  - if the current operation is a read, read the 
> record id from the
>  memory location that was last written to 0x08, 
> using the written
>  value as the base of the error log address range.
> 
> In addition, the firmware will need to reserve a few KB of RAM for the error 
> log
> address range (I checked a real system and it reserves 8KB).  The first eight
> bytes are needed for the record identifier interface, because there's no such
> thing as 64-bit I/O ports, and the rest can be used for the actual buffer.

Is there a limit on the size?  It'd be great if it can use a few MB..

> 
> QEMU already has an interface to allocate RAM and patch the address into an
> ACPI table (bios_linker_loader_alloc).  Because this interface is actually 
> meant
> to load data from QEMU into the firmware (using the "fw_cfg" interface), you
> would have to add a dummy 8KB file to fw_cfg using fw_cfg_add_file (for
> example "etc/erst-memory"), it can be just full of zeros.
> 
> QEMU supports two chipsets, PIIX and ICH9, and the free I/O port ranges are
> different.  You could use 0xa20 for ICH9 and 0xae20 for PIIX.
> 
> All in all, the contents of the ERST table would not be very different from a
> non-virtual system, except that on real hardware the firmware would use SMIs
> as the trap mechanism.  You almost have a one-to-one mapping between ERST
> actions and registers accesses:
> 
>BEGIN_WRITE_OPERATION  write value 0 to 

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

[Namhyung Kim]

Hi,

On Tue, Nov 15, 2016 at 11:38 PM, Paolo Bonzini <pbonz...@redhat.com> wrote:
>
>
> On 15/11/2016 15:36, Namhyung Kim wrote:
>> Hi,
>>
>> On Tue, Nov 15, 2016 at 10:57:29AM +0100, Paolo Bonzini wrote:
>>>
>>>
>>> On 15/11/2016 06:06, Michael S. Tsirkin wrote:
>>>> On Tue, Nov 15, 2016 at 01:50:21PM +0900, Namhyung Kim wrote:
>>>>> Hi Michael,
>>>>>
>>>>> On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote:
>>>>>> On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote:
>>>>>>> The virtio pstore driver provides interface to the pstore subsystem so
>>>>>>> that the guest kernel's log/dump message can be saved on the host
>>>>>>> machine.  Users can access the log file directly on the host, or on the
>>>>>>> guest at the next boot using pstore filesystem.  It currently deals with
>>>>>>> kernel log (printk) buffer only, but we can extend it to have other
>>>>>>> information (like ftrace dump) later.
>>>>>>>
>>>>>>> It supports legacy PCI device using single order-2 page buffer.
>>>>>>
>>>>>> Do you mean a legacy virtio device? I don't see why
>>>>>> you would want to support pre-1.0 mode.
>>>>>> If you drop that, you can drop all cpu_to_virtio things
>>>>>> and just use __le accessors.
>>>>>
>>>>> I was thinking about the kvmtools which lacks 1.0 support AFAIK.
>>>>
>>>> Unless kvmtools wants to be left behind it has to go 1.0.
>>>
>>> And it also has to go ACPI.  Is there any reason, apart from kvmtool, to
>>> make a completely new virtio device, with no support in existing guests,
>>> rather than implement ACPI ERST?
>>
>> Well, I know nothing about ACPI.  It looks like a huge spec and I
>> don't want to dig into it just for this.
>
> ERST (error record serialization table) is a small subset of the ACPI spec.

Not sure how independent ERST is from ACPI and other specs.  It looks
like referencing UEFI spec at least.  Btw, is the ERST used for pstore
only (in Linux)?

Also I need to control pstore driver like using bigger buffer,
enabling specific message types and so on if ERST supports.  Is it
possible for ERST to provide such information?

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

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

[Namhyung Kim]

Hi,

On Tue, Nov 15, 2016 at 10:57:29AM +0100, Paolo Bonzini wrote:
> 
> 
> On 15/11/2016 06:06, Michael S. Tsirkin wrote:
> > On Tue, Nov 15, 2016 at 01:50:21PM +0900, Namhyung Kim wrote:
> >> Hi Michael,
> >>
> >> On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote:
> >>> On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote:
> >>>> The virtio pstore driver provides interface to the pstore subsystem so
> >>>> that the guest kernel's log/dump message can be saved on the host
> >>>> machine.  Users can access the log file directly on the host, or on the
> >>>> guest at the next boot using pstore filesystem.  It currently deals with
> >>>> kernel log (printk) buffer only, but we can extend it to have other
> >>>> information (like ftrace dump) later.
> >>>>
> >>>> It supports legacy PCI device using single order-2 page buffer.
> >>>
> >>> Do you mean a legacy virtio device? I don't see why
> >>> you would want to support pre-1.0 mode.
> >>> If you drop that, you can drop all cpu_to_virtio things
> >>> and just use __le accessors.
> >>
> >> I was thinking about the kvmtools which lacks 1.0 support AFAIK.
> > 
> > Unless kvmtools wants to be left behind it has to go 1.0.
> 
> And it also has to go ACPI.  Is there any reason, apart from kvmtool, to
> make a completely new virtio device, with no support in existing guests,
> rather than implement ACPI ERST?

Well, I know nothing about ACPI.  It looks like a huge spec and I
don't want to dig into it just for this.

What I want is to speed up dumping guest kernel message (especially
for ftrace dump).

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

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

On Fri, Nov 11, 2016 at 12:50:03AM +0200, Michael S. Tsirkin wrote:
> On Fri, Sep 16, 2016 at 07:05:47PM +0900, Namhyung Kim wrote:
> > On Tue, Sep 13, 2016 at 06:57:10PM +0300, Michael S. Tsirkin wrote:
> > > On Sat, Aug 20, 2016 at 05:07:43PM +0900, Namhyung Kim wrote:
> > > > +
> > > > +/* the index should match to the type value */
> > > > +static const char *virtio_pstore_file_prefix[] = {
> > > > +"unknown-",/* VIRTIO_PSTORE_TYPE_UNKNOWN */
> > > 
> > > Is there value in treating everything unexpected as "unknown"
> > > and rotating them as if they were logs?
> > > It might be better to treat everything that's not known
> > > as guest error.
> > 
> > I was thinking about the version mismatch between the kernel and qemu.
> > I'd like to make the device can deal with a new kernel version which
> > might implement a new pstore message type.  It will be saved as
> > unknown but the kernel can read it properly later.
> 
> Well it'll have a different prefix. E.g. if kernel has
> two different types they will end up in the same
> file, hardly what was wanted.

Right, I think it needs to add 'type' info to the filename for unknown
type.

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

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

[Namhyung Kim]

On Tue, Nov 15, 2016 at 07:06:28AM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 15, 2016 at 01:50:21PM +0900, Namhyung Kim wrote:
> > On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote:
> > [SNIP]
> > > > +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;
> > > > +};
> > > > +
> > > 
> > > What exactly does each field mean? I'm especially
> > > interested in time fields - maintaining a consistent
> > > time between host and guest is not a simple problem.
> > 
> > These are required by pstore and will be used to create corresponding
> > files in the pstore filesystem.  The time fields are for mtime and
> > ctime and, I think, it's just a hint for user and doesn't require
> > strict consistency.
> 
> Pls add documentation. I would just drop hints for now.

Well, I'll add docmentation.  But I think just dropping might not good
since they all have host time and it's helpful to know their relative
difference in guest.

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

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

[Namhyung Kim]

Hi Michael,

On Thu, Nov 10, 2016 at 06:39:55PM +0200, Michael S. Tsirkin wrote:
> On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote:
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine.  Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem.  It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> > 
> > It supports legacy PCI device using single order-2 page buffer.
> 
> Do you mean a legacy virtio device? I don't see why
> you would want to support pre-1.0 mode.
> If you drop that, you can drop all cpu_to_virtio things
> and just use __le accessors.

I was thinking about the kvmtools which lacks 1.0 support AFAIK.  But
I think it'd be better to always use __le type anyway.  Will change.


> 
> > 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 <pbonz...@redhat.com>
> > Cc: Radim Krčmář <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---
> >  drivers/virtio/Kconfig |  10 +
> >  drivers/virtio/Makefile|   1 +
> >  drivers/virtio/virtio_pstore.c | 417 
> > +
> >  include/uapi/linux/Kbuild  |   1 +
> >  include/uapi/linux/virtio_ids.h|   1 +
> >  include/uapi/linux/virtio_pstore.h |  74 +++
> >  6 files changed, 504 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 ..0a63c7db4278
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pstore.c
> > @@ -0,0 +1,417 @@
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define VIRT_PSTORE_ORDER2
> > +#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
> > +#define VIRT_PSTORE_NR_REQ   128
> > +
> > +struct virtio_pstore {
> > +   struct virtio_device*vdev;
> > +   struct virtqueue*vq[2];
> 
> I'd add named fields instead of an array here, vq[0]
> vq[1] all over the place is hard to read.

Will change.

> 
> > +   struct pstore_info   pstore;
&

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

On Thu, Sep 22, 2016 at 01:23:16PM +0100, Stefan Hajnoczi wrote:
> On Sun, Sep 04, 2016 at 11:38:59PM +0900, Namhyung Kim wrote:
> > +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);
> 
> The new virtio_error() function might be available, depending on when
> this patch series is merged.  virtio_error() should be used instead of
> exit(1).  See "[PATCH v5 0/9] virtio: avoid exit() when device enters
> invalid states" on qemu-devel.

Thanks for the info, will take a look.

> 
> > +}
> > +
> > +if (elem->out_num > 2 || elem->in_num > 3) {
> > +error_report("invalid number of input/output buffer");
> > +exit(1);
> > +}
> 
> The VIRTIO specification requires that flexible framing is supported.
> The device cannot make assumptions about the scatter-gather list.  It
> must support any layout (e.g. even multiple 1-byte iovecs making up the
> buffer).

Ok.

> 
> > +
> > +len = iov_to_buf(elem->out_sg, elem->out_num, 0, , 
> > 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_CLOSE:
> > +ret = virtio_pstore_do_close(s);
> > +break;
> > +case VIRTIO_PSTORE_CMD_ERASE:
> > +ret = virtio_pstore_do_erase(s, );
> > +break;
> > +case VIRTIO_PSTORE_CMD_READ:
> > +ret = virtio_pstore_do_read(s, elem);
> > +if (ret == 1) {
> > +/* async channel io */
> > +continue;
> > +}
> > +break;
> > +case VIRTIO_PSTORE_CMD_WRITE:
> > +ret = virtio_pstore_do_write(s, elem, );
> > +if (ret == 1) {
> > +/* async channel io */
> > +continue;
> > +}
> > +break;
> > +default:
> > +ret = -1;
> > +break;
> > +}
> > +
> > +res.ret = ret;
> 
> Missing cpu_to_le()?

Right!

> 
> > +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, , );
> > +if (error) {
> > +error_propagate(errp, error);
> > +return;
> > +}
> > +
> > +if (value < 4096) {
> > +error_setg(, "Warning: too small buffer size: %"PRIu64, 
> > value);
> 
> This is an error, not a warning.  Please remove "Warning:" so it's clear
> to the user that this message caused QEMU to fail.

Will do.

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

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

[Namhyung Kim]

Hi Stefan,

On Thu, Sep 22, 2016 at 12:57:44PM +0100, Stefan Hajnoczi wrote:
> On Sun, Sep 04, 2016 at 11:38:58PM +0900, Namhyung Kim wrote:
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine.  Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem.  It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> > 
> > It supports legacy PCI device using a 16K buffer by default and it's
> > configurable.  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.
> 
> Please document the locks that this code relies on.  It is generally not
> safe to call virtqueue_*() from multiple threads.  I also wonder about
> locking for virtio_pstore->req_id and other fields.  Are locks missing
> or is something in pstore ensuring safety?

Ok, I should use atomic inc for pstore->req_id.  The open-read-close
are serialized by the read_mutex of pstore_info.  Write can happend
anytime so I gave it a dedicate queue.

Erase is a problem though, normally it's only doable after mount
operation is done so no contention to the open-read-close.  But if the
pstore_update_ms is set, timer routine can schedule a work to do the
open-read-close loop which might contend to erase.

I'm not sure how useful pstore_update_ms is and the descriptoin saids

  "milliseconds before pstore updates its content "
  "(default is -1, which means runtime updates are disabled; "
  "enabling this option is not safe, it may lead to further "
  "corruption on Oopses)")


> 
> > +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->cmd = cpu_to_le16(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], ));
> > +   return le32_to_cpu(res->ret);
> 
> This assumes the device puts compatible Linux errno values in res->ret.
> The function doesn't need to return -errno if I'm reading fs/pstore/
> code correctly.  You could return -1 on error to avoid making this
> assumption.  The same applies to other res->ret usage below.

Ok.

> 
> > +}
> > +
> > +static int virt_pstore_close(struct pstore_info *psi)
> > +{
> > +   struct virtio_pstore *vps = psi->data;
> > +   struct virtio_pstore_req *req = >req[vps->req_id];
> > +   struct virtio_pstore_res *res = >res[vps->req_id];
> 
> Assigning >req[vps->req_id]/>res[vps->req_id] is unnecessary,
> virt_pstore_get_reqs() handles that below.

Ah, right.

> 
> > +   struct scatterlist sgo[1], sgi[1];
> > +   struct scatterlist *sgs[2] = { sgo, sgi };
> > +   unsigned int len;
> > +
> > +   virt_pstore_get_reqs(vps, , );
> > +
> > +   req->cmd = cpu_to_le16(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], ));
> > +   return le32_to_cpu(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;
> > +   

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

On Tue, Sep 13, 2016 at 06:57:10PM +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 20, 2016 at 05:07:43PM +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
> > 
> >   $ 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' property to set size of pstore bufer.
> > 
> > Cc: Paolo Bonzini <pbonz...@redhat.com>
> > Cc: Radim Krčmář <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: Daniel P. Berrange <berra...@redhat.com>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---
> >  hw/virtio/Makefile.objs|   2 +-
> >  hw/virtio/virtio-pci.c |  52 ++
> >  hw/virtio/virtio-pci.h |  14 +
> >  hw/virtio/virtio-pstore.c  | 699 
> > +
> >  include/hw/pci/pci.h   |   1 +
> >  include/hw/virtio/virtio-pstore.h  |  36 ++
> >  include/standard-headers/linux/virtio_ids.h|   1 +
> >  include/standard-headers/linux/virtio_pstore.h |  76 +++
> >  qdev-monitor.c |   1 +
> >  9 files changed, 881 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 755f921..c184823 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -2416,6 +2416,57 @@ 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(>vdev);
> > +Error *err = NULL;
> > +
> > +qdev_set_parent_bus(vdev, BUS(_dev->bus));
> > +object_property_set_bool(OBJECT(vdev), true, "realized", );
> > +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);
> > +
> > +   

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

[Namhyung Kim]

Hello Michael,

Thanks for your detailed review.  Btw are you ok with the overall
direction of the patch?


On Tue, Sep 13, 2016 at 06:19:41PM +0300, Michael S. Tsirkin wrote:
> On Sat, Aug 20, 2016 at 05:07:42PM +0900, Namhyung Kim wrote:
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine.  Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem.  It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> > 
> > It supports legacy PCI device using single order-2 page buffer.  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 <pbonz...@redhat.com>
> > Cc: Radim Krčmář <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---
> >  drivers/virtio/Kconfig |  10 +
> >  drivers/virtio/Makefile|   1 +
> >  drivers/virtio/virtio_pstore.c | 417 
> > +
> >  include/uapi/linux/Kbuild  |   1 +
> >  include/uapi/linux/virtio_ids.h|   1 +
> >  include/uapi/linux/virtio_pstore.h |  74 +++
> >  6 files changed, 504 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 ..0a63c7db4278
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pstore.c
> > @@ -0,0 +1,417 @@
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define VIRT_PSTORE_ORDER2
> > +#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
> > +#define VIRT_PSTORE_NR_REQ   128
> 
> where are these numbers from?

The buffer size was chosen to be larger than default kmsg_bytes
(10240) in the pstore platform code and the request count is a
arbitrary value.

> 
> 
> > +
> > +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;
> > +
> > +  

[PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@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-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' property to set size of pstore bufer.

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: Daniel P. Berrange <berra...@redhat.com>
Signed-off-by: Namhyung Kim <namhy...@gmail.com>
---
 hw/virtio/Makefile.objs|   2 +-
 hw/virtio/virtio-pci.c |  52 ++
 hw/virtio/virtio-pci.h |  14 +
 hw/virtio/virtio-pstore.c  | 689 +
 include/hw/pci/pci.h   |   1 +
 include/hw/virtio/virtio-pstore.h  |  36 ++
 include/standard-headers/linux/virtio_ids.h|   1 +
 include/standard-headers/linux/virtio_pstore.h |  76 +++
 qdev-monitor.c |   1 +
 9 files changed, 871 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 755f921..c184823 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2416,6 +2416,57 @@ 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(>vdev);
+Error *err = NULL;
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus));
+object_property_set_bool(OBJECT(vdev), true, "realized", );
+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, >vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_PSTORE);
+object_property_add_alias(obj, "directory", OBJECT(>vdev),
+  "directory", _abort);
+object_property_add_alias(obj, "bufsize", OBJECT(>vdev),
+  "bufsize", _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 +2536,7 @@ static void virtio_pci_register_types(void)
 #ifdef CONFIG_VHOST_SCSI
 type_register_static(_scsi_pci_info);
 #endif
+type_register_static(_pstore_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 25fbf8a..354b2b7 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -31,6 +31,7 @@
 #ifdef CONFIG_VHOST_SCSI
 #include "hw/vir

[PATCH 3/3] kvmtool: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@gmail.com>

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: Will Deacon <will.dea...@arm.com>
Signed-off-by: Namhyung Kim <namhy...@gmail.com>
---
 Makefile |   1 +
 builtin-run.c|   2 +
 include/kvm/kvm-config.h |   1 +
 include/kvm/virtio-pci-dev.h |   2 +
 include/kvm/virtio-pstore.h  |  53 +
 include/linux/virtio_ids.h   |   1 +
 virtio/pstore.c  | 447 +++
 7 files changed, 507 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_9P0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE0x100a
 #define PCI_DEVICE_ID_VESA 0x2000
 #define PCI_DEVICE_ID_PCI_SHMEM0x0001
 
@@ -34,5 +35,6 @@
 #define PCI_CLASS_RNG  0xff
 #define PCI_CLASS_BLN  0xff
 #define PCI_CLASS_9P   0xff
+#define PCI_CLASS_PSTORE   0xff
 
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 000..9f52ffd
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,53 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+#include 
+#include 
+
+#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_DMESG1
+
+#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;
+};
+
+int virtio_pstore__init(struct kvm *kvm);
+int virtio_pstore__

[RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v5)

[Namhyung Kim]

Hello,

This is another iteration of the virtio-pstore work.  I've addressed
comments from Michael S. Tsirkin on the kernel code.

 * changes in v5)
  - convert __virtioXX to __leXX  (Michael)

 * changes in v4)
  - use qio_channel_file_new_path()  (Daniel)
  - rename to delete_old_pstore_file  (Daniel)
  - convert G_REMOVE_SOURCE to FALSE  (Daniel)

 * changes in v3)
  - use QIOChannel API  (Stefan, Daniel)
  - add bound check for malcious guests  (Daniel)
  - drop support PSTORE_TYPE_CONSOLE for now
  - update license to allow GPL v2 or later  (Michael)
  - limit number of pstore files on qemu

 * 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 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 0002 and 0003 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.00] 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.00] Command line: root=/dev/vda console=ttyS0
  <6>[0.00] x86/fpu: Legacy x87 FPU detected.
  <6>[0.00] x86/fpu: Using 'eager' FPU context switches.
  <6>[0.00] e820: BIOS-provided physical RAM map:
  <6>[0.00] BIOS-e820: [mem 0x-0x0009fbff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x000f-0x000f] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x0010-0x07fddfff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x07fde000-0x07ff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfffc-0x] 
reserved
  <6>[0.00] NX (Execute Disable) protection: active
  <6>[0.00] SMBIOS 2.8 present.
  <7>[0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
  ...


Namhyung Kim (3):
  virtio: Basic implementation of virtio pstore driver
  qemu: Implement virtio-pstore device
  kvmtool: Implement virtio-pstore device


 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 insertions(+)
 create mode 100644 drivers/virtio/virtio_pstore.c
 create mode 100644 include/uapi/linux/virtio_pstore.h


Cc: Paolo Bonzini <pbonz...@redhat.com>
C

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

[Namhyung Kim]

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 a 16K buffer by default and it's
configurable.  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.

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: Will Deacon <will.dea...@arm.com>
Cc: virtio-...@lists.oasis-open.org
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 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 ..c8fd8e39d1b8
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,417 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIRT_PSTORE_ORDER2
+#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
+#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 __le16 to_virtio_type(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_le16(type_table[i].virtio);
+   }
+
+   return cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN);
+}
+
+static enum pstore_type_id from_virtio_type(__le16 type)
+{
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(type_table); i++) {
+   if (le16_to_cpu(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(>acked);
+}
+
+static void virtpstore_check(struct virtqueue *vq)
+{
+   struct virtio_pstore *vps 

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

[Namhyung Kim]

Hi Michael,

On Wed, Aug 31, 2016 at 05:54:04PM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 31, 2016 at 05:08:00PM +0900, Namhyung Kim wrote:
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine.  Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem.  It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> > 
> > It supports legacy PCI device using single order-2 page buffer.  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 <pbonz...@redhat.com>
> > Cc: Radim Krčmář <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: Will Deacon <will.dea...@arm.com>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: virtio-...@lists.oasis-open.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---

[SNIP]
> > +#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);
> 
> Does this pass sparse checks? If yes I'm surprised - this clearly
> returns a virtio16 type.

Ah, didn't run sparse.  Will change it to return a __le16 type
(according to your comment below).

> 
> 
> > +   }
> > +
> > +   return cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
> > +}
> > +
> > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 
> > type)

This one should be '__le16 type' as well.


> > +{
> > +   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;
> > +}
> > +

[SNIP]
> > +
> > +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;
> > +};
> 
> Is there a reason to support legacy endian-ness?
> If not, you can just use __le formats.

I just didn't know what's the preferred type.  Will change!

Thanks,
Namhyung

> 
> 
> > +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.9.3
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH 3/3] kvmtool: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@gmail.com>

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: Will Deacon <will.dea...@arm.com>
Signed-off-by: Namhyung Kim <namhy...@gmail.com>
---
 Makefile |   1 +
 builtin-run.c|   2 +
 include/kvm/kvm-config.h |   1 +
 include/kvm/virtio-pci-dev.h |   2 +
 include/kvm/virtio-pstore.h  |  53 +
 include/linux/virtio_ids.h   |   1 +
 virtio/pstore.c  | 447 +++
 7 files changed, 507 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_9P0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE0x100a
 #define PCI_DEVICE_ID_VESA 0x2000
 #define PCI_DEVICE_ID_PCI_SHMEM0x0001
 
@@ -34,5 +35,6 @@
 #define PCI_CLASS_RNG  0xff
 #define PCI_CLASS_BLN  0xff
 #define PCI_CLASS_9P   0xff
+#define PCI_CLASS_PSTORE   0xff
 
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 000..9f52ffd
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,53 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+#include 
+#include 
+
+#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_DMESG1
+
+#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;
+};
+
+int virtio_pstore__init(struct kvm *kvm);
+int virtio_pstore__

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

[Namhyung Kim]

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 <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: Will Deacon <will.dea...@arm.com>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Cc: virtio-...@lists.oasis-open.org
Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 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 ..ec41f0d2f0b7
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,417 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIRT_PSTORE_ORDER2
+#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
+#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(

[PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@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-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' property to set size of pstore bufer.

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: Daniel P. Berrange <berra...@redhat.com>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@gmail.com>
---
 hw/virtio/Makefile.objs|   2 +-
 hw/virtio/virtio-pci.c |  52 ++
 hw/virtio/virtio-pci.h |  14 +
 hw/virtio/virtio-pstore.c  | 689 +
 include/hw/pci/pci.h   |   1 +
 include/hw/virtio/virtio-pstore.h  |  36 ++
 include/standard-headers/linux/virtio_ids.h|   1 +
 include/standard-headers/linux/virtio_pstore.h |  76 +++
 qdev-monitor.c |   1 +
 9 files changed, 871 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 755f921..c184823 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2416,6 +2416,57 @@ 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(>vdev);
+Error *err = NULL;
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus));
+object_property_set_bool(OBJECT(vdev), true, "realized", );
+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, >vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_PSTORE);
+object_property_add_alias(obj, "directory", OBJECT(>vdev),
+  "directory", _abort);
+object_property_add_alias(obj, "bufsize", OBJECT(>vdev),
+  "bufsize", _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 +2536,7 @@ static void virtio_pci_register_types(void)
 #ifdef CONFIG_VHOST_SCSI
 type_register_static(_scsi_pci_info);
 #endif
+type_register_static(_pstore_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 25fbf8a..354b2b7 100644
--- a/hw/virtio/virtio-pci.h
+++

[RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v4)

[Namhyung Kim]

Hello,

This is another iteration of the virtio-pstore work.  I've addressed
all comments from Daniel Berrange on the qemu side.

 * changes in v4)
  - use qio_channel_file_new_path()  (Daniel)
  - rename to delete_old_pstore_file  (Daniel)
  - convert G_REMOVE_SOURCE to FALSE  (Daniel)

 * changes in v3)
  - use QIOChannel API  (Stefan, Daniel)
  - add bound check for malcious guests  (Daniel)
  - drop support PSTORE_TYPE_CONSOLE for now
  - update license to allow GPL v2 or later  (Michael)
  - limit number of pstore files on qemu

 * 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 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 0002 and 0003 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.00] 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.00] Command line: root=/dev/vda console=ttyS0
  <6>[0.00] x86/fpu: Legacy x87 FPU detected.
  <6>[0.00] x86/fpu: Using 'eager' FPU context switches.
  <6>[0.00] e820: BIOS-provided physical RAM map:
  <6>[0.00] BIOS-e820: [mem 0x-0x0009fbff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x000f-0x000f] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x0010-0x07fddfff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x07fde000-0x07ff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfffc-0x] 
reserved
  <6>[0.00] NX (Execute Disable) protection: active
  <6>[0.00] SMBIOS 2.8 present.
  <7>[0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
  ...


Namhyung Kim (3):
  virtio: Basic implementation of virtio pstore driver
  qemu: Implement virtio-pstore device
  kvmtool: Implement virtio-pstore device


 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 insertions(+)
 create mode 100644 drivers/virtio/virtio_pstore.c
 create mode 100644 include/uapi/linux/virtio_pstore.h


Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. 

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

Hi Daniel,

On Wed, Aug 24, 2016 at 06:00:51PM -0400, Daniel P. Berrange wrote:
> 
> > diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c
> > new file mode 100644
> > index 000..b8fb4be
> > --- /dev/null
> > +++ b/hw/virtio/virtio-pstore.c
> > @@ -0,0 +1,699 @@
> > +/*
> > + * Virtio Pstore Device
> > + *
> > + * Copyright (C) 2016  LG Electronics
> > + *
> > + * Authors:
> > + *  Namhyung Kim  <namhy...@gmail.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include 
> > +
> > +#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 "io/channel-util.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"
> > +
> > +#define PSTORE_DEFAULT_BUFSIZE   (16 * 1024)
> > +#define PSTORE_DEFAULT_FILE_MAX  5
> > +
> > +/* the index should match to the type value */
> > +static const char *virtio_pstore_file_prefix[] = {
> > +"unknown-",/* VIRTIO_PSTORE_TYPE_UNKNOWN */
> > +"dmesg-",  /* VIRTIO_PSTORE_TYPE_DMESG */
> > +};
> > +
> > +static char *virtio_pstore_to_filename(VirtIOPstore *s,
> > +   struct virtio_pstore_req *req)
> > +{
> > +const char *basename;
> > +unsigned long long id;
> > +unsigned int type = le16_to_cpu(req->type);
> > +unsigned int flags = le32_to_cpu(req->flags);
> > +
> > +if (type < ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > +basename = virtio_pstore_file_prefix[type];
> > +} else {
> > +basename = "unknown-";
> > +}
> > +
> > +id = s->id++;
> > +return g_strdup_printf("%s/%s%llu%s", s->directory, basename, id,
> > +flags & VIRTIO_PSTORE_FL_COMPRESSED ? ".enc.z" 
> > : "");
> > +}
> > +
> > +static char *virtio_pstore_from_filename(VirtIOPstore *s, char *name,
> > + struct virtio_pstore_fileinfo 
> > *info)
> > +{
> > +char *filename;
> > +unsigned int idx;
> > +
> > +filename = g_strdup_printf("%s/%s", s->directory, name);
> > +if (filename == NULL)
> > +return NULL;
> > +
> > +for (idx = 0; idx < ARRAY_SIZE(virtio_pstore_file_prefix); idx++) {
> > +if (g_str_has_prefix(name, virtio_pstore_file_prefix[idx])) {
> > +info->type = idx;
> > +name += strlen(virtio_pstore_file_prefix[idx]);
> > +break;
> > +}
> > +}
> > +
> > +if (idx == ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > +g_free(filename);
> > +return NULL;
> > +}
> > +
> > +qemu_strtoull(name, NULL, 0, >id);
> > +
> > +info->flags = 0;
> > +if (g_str_has_suffix(name, ".enc.z")) {
> > +info->flags |= VIRTIO_PSTORE_FL_COMPRESSED;
> > +}
> > +
> > +return filename;
> > +}
> > +
> > +static int prefix_idx;
> > +static int prefix_count;
> > +static int prefix_len;
> > +
> > +static int filter_pstore(const struct dirent *de)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < prefix_count; i++) {
> > +const char *prefix = virtio_pstore_file_prefix[prefix_idx + i];
> > +
> > +if (g_str_has_prefix(de->d_name, prefix)) {
> > +return 1;
> > +}
> > +}
> > +return 0;
> > +}
> > +
> > +static int sort_pstore(const struct dirent **a, const struct dirent **b)
> > +{
> > +uint64_t id_a, id_b;
> > +
> > +qemu_strtoull((*a)->d_name + prefix_len, NULL, 0, _a);
> > +qemu_strtoull((*b)->d_name + prefix_len, NULL, 0, _b);
> > +
> > +return id_a - id_b;
> > +}
> > +
&

[PATCH 3/3] kvmtool: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@gmail.com>

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: Will Deacon <will.dea...@arm.com>
Signed-off-by: Namhyung Kim <namhy...@gmail.com>
---
 Makefile |   1 +
 builtin-run.c|   2 +
 include/kvm/kvm-config.h |   1 +
 include/kvm/virtio-pci-dev.h |   2 +
 include/kvm/virtio-pstore.h  |  53 +
 include/linux/virtio_ids.h   |   1 +
 virtio/pstore.c  | 447 +++
 7 files changed, 507 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_9P0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE0x100a
 #define PCI_DEVICE_ID_VESA 0x2000
 #define PCI_DEVICE_ID_PCI_SHMEM0x0001
 
@@ -34,5 +35,6 @@
 #define PCI_CLASS_RNG  0xff
 #define PCI_CLASS_BLN  0xff
 #define PCI_CLASS_9P   0xff
+#define PCI_CLASS_PSTORE   0xff
 
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 000..9f52ffd
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,53 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+#include 
+#include 
+
+#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_DMESG1
+
+#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;
+};
+
+int virtio_pstore__init(struct kvm *kvm);
+int virtio_pstore__

[PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

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' property to set size of pstore bufer.

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: Daniel P. Berrange <berra...@redhat.com>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 hw/virtio/Makefile.objs|   2 +-
 hw/virtio/virtio-pci.c |  52 ++
 hw/virtio/virtio-pci.h |  14 +
 hw/virtio/virtio-pstore.c  | 699 +
 include/hw/pci/pci.h   |   1 +
 include/hw/virtio/virtio-pstore.h  |  36 ++
 include/standard-headers/linux/virtio_ids.h|   1 +
 include/standard-headers/linux/virtio_pstore.h |  76 +++
 qdev-monitor.c |   1 +
 9 files changed, 881 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 755f921..c184823 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2416,6 +2416,57 @@ 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(>vdev);
+Error *err = NULL;
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus));
+object_property_set_bool(OBJECT(vdev), true, "realized", );
+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, >vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_PSTORE);
+object_property_add_alias(obj, "directory", OBJECT(>vdev),
+  "directory", _abort);
+object_property_add_alias(obj, "bufsize", OBJECT(>vdev),
+  "bufsize", _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 +2536,7 @@ static void virtio_pci_register_types(void)
 #ifdef CONFIG_VHOST_SCSI
 type_register_static(_scsi_pci_info);
 #endif
+type_register_static(_pstore_pci_info);
 }
 
 type_init(virtio_pci_register_types)
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 25fbf8a..354b2b7 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -31,6 +31

[RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v3)

[Namhyung Kim]

Hello,

This is another iteration of the virtio-pstore work.  In this patchset
I addressed most of feedbacks from previous version and drooped the
support for PSTORE_TYPE_CONSOLE for simplicity.  It'll be added once the basic 
implementation

 * changes in v3)
  - use QIOChannel API  (Stefan, Daniel)
  - add bound check for malcious guests  (Daniel)
  - drop support PSTORE_TYPE_CONSOLE for now
  - update license to allow GPL v2 or later  (Michael)
  - limit number of pstore files on qemu

 * 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 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 0002 and 0003 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.00] 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.00] Command line: root=/dev/vda console=ttyS0
  <6>[0.00] x86/fpu: Legacy x87 FPU detected.
  <6>[0.00] x86/fpu: Using 'eager' FPU context switches.
  <6>[0.00] e820: BIOS-provided physical RAM map:
  <6>[0.00] BIOS-e820: [mem 0x-0x0009fbff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x000f-0x000f] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x0010-0x07fddfff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x07fde000-0x07ff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfffc-0x] 
reserved
  <6>[0.00] NX (Execute Disable) protection: active
  <6>[0.00] SMBIOS 2.8 present.
  <7>[0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
  ...


Namhyung Kim (3):
  virtio: Basic implementation of virtio pstore driver
  qemu: Implement virtio-pstore device
  kvmtool: Implement virtio-pstore device

 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 insertions(+)
 create mode 100644 drivers/virtio/virtio_pstore.c
 create mode 100644 include/uapi/linux/virtio_pstore.h


Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthon

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

[Namhyung Kim]

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 <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 drivers/virtio/Kconfig |  10 +
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 417 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  74 +++
 6 files changed, 504 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 ..0a63c7db4278
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,417 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIRT_PSTORE_ORDER2
+#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
+#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(>acked);
+}
+
+static void virtpstore_check(struct virtqueue *vq)
+{
+  

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

[Namhyung Kim]

On Mon, Aug 01, 2016 at 10:24:39AM +0100, Daniel P. Berrange wrote:
> On Sat, Jul 30, 2016 at 05:57:02PM +0900, Namhyung Kim wrote:
> > On Thu, Jul 28, 2016 at 02:22:39PM +0100, Daniel P. Berrange wrote:
> > > > +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-");
> > > > +}
> 
> [snip]
> 
> > > > +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;
> > > > +}
> > > 
> > > So this seems to just be picking the first filename reported by
> > > readdir that isn't starting with '.'. Surely this can't the right
> > > logic when your corresponding do_write method can pick several
> > > different filenames, its potluck which do_read will give back.
> > 
> > Do you mean that it'd be better to check a list of known filenames and
> > fail if not?
> 
> No, I mean that you have several different VIRTIO_PSTORE_TYPE_nnn and
> use a different file for each constant. When reading this directory
> though you're not looking for the file corresponding to any given
> VIRTIO_PSTORE_TYPE_nnn - you're simply reading whichever file is found
> first. So you might have just read a TYPE_CONSOLE or have read a
> TYPE_DMESG - it surely doesn't make sense to randonly read either
> TYPE_CONSOLE or TYPE_DMESG based on whatever order readdir() lists
> the files.

In fact, each VIRTIO_PSTORE_TYPE_xxx can have more than one files and
I don't care about the ordering between them.  They will be fed to the
pstore filesystem on the guest.

But I also think it'd be better to scan files of known types only
rather than read out whatever readdir() gives.

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

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

[Namhyung Kim]

Hi Daniel,

On Mon, Aug 01, 2016 at 10:21:30AM +0100, Daniel P. Berrange wrote:
> On Sat, Jul 30, 2016 at 05:38:27PM +0900, Namhyung Kim wrote:
> > Hello,
> > 
> > On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote:
> > > On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote:
> > > > On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote:
> > > > > On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
> > > > > > > +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(_sg, _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.
> > > > > 
> > > > > Hmm.. I don't know about the internals of qemu.  So does it make guest
> > > > > stop?  If so, that's what I want to do for _DMESG. :)  As it's called
> > > > > only on kernel oops I think it's admittable.  But for _CONSOLE, it
> > > > > needs to do asynchronously.  Maybe I can add a thread to do the work.
> > > > 
> > > > Please look at include/io/channel.h.  QEMU is event-driven and tends to
> > > > use asynchronous I/O instead of spawning threads.  The include/io/ APIs
> > > > allow you to do asynchronous I/O in the event loop.
> > > 
> > > That is true, except for I/O to/from plain files - the QIOChannelFile
> > > impl doesn't do anything special (yet) to make that work correctly in
> > > non-blocking mode. Of course that could be fixed...
> > 
> > Yep, I don't know how I can use the QIOChannelFile for async IO.
> > AFAICS it's just a wrapper for normal readv/writev.  Who is
> > responsible to do these IO when I use the IO channel API?  Also does
> > it guarantee that all IOs are processed in order?
> 
> I'd suggest just using QIOChannelFile regardless - we need to fix the
> blocking behaviour already for sake of the qemu chardev code, and your
> code just adds more pressure to fix it. I/O will be done in the order
> in which you make the calls, as with regular POSIX I/O funcs you're
> currently using.

Thanks for the info.

So can I simply replace regular IO funcs to QIOChannel API like below?

  ioc = qio_channel_file_new_path(path, flags, 0644, );
  qio_channel_set_blocking(ioc, false, );
  len = qio_channel_writev(ioc, out_sg, out_num, );
  qio_channel_file_close(ioc, );
  return len;

Or do I need to call qio_channel_add_watch() or something?

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

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

[Namhyung Kim]

On Thu, Jul 28, 2016 at 02:22:39PM +0100, Daniel P. Berrange wrote:
> 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
> > 
> >   $ 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 <pbonz...@redhat.com>
> > Cc: Radim Kr??m <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---

[SNIP]
> > +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" : "");
> 
> Please use g_strdup_printf() instead of splattering into a pre-allocated
> buffer than may or may not be large enough.

OK.

> 
> > +}
> > +
> > +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, >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];
> 
> Don't declare PATH_MAX sized variables

Will change to use g_strdup_printf() as you said.

> 
> >

Re: [Qemu-devel] [PATCH 6/7] qemu: Implement virtio-pstore device

[Namhyung Kim]

Hello,

On Thu, Jul 28, 2016 at 02:08:41PM +0100, Daniel P. Berrange wrote:
> On Thu, Jul 28, 2016 at 01:56:07PM +0100, Stefan Hajnoczi wrote:
> > On Thu, Jul 28, 2016 at 02:39:53PM +0900, Namhyung Kim wrote:
> > > On Thu, Jul 28, 2016 at 03:02:54AM +0300, Michael S. Tsirkin wrote:
> > > > On Thu, Jul 28, 2016 at 12:08:30AM +0900, Namhyung Kim wrote:
> > > > > +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(_sg, _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.
> > > 
> > > Hmm.. I don't know about the internals of qemu.  So does it make guest
> > > stop?  If so, that's what I want to do for _DMESG. :)  As it's called
> > > only on kernel oops I think it's admittable.  But for _CONSOLE, it
> > > needs to do asynchronously.  Maybe I can add a thread to do the work.
> > 
> > Please look at include/io/channel.h.  QEMU is event-driven and tends to
> > use asynchronous I/O instead of spawning threads.  The include/io/ APIs
> > allow you to do asynchronous I/O in the event loop.
> 
> That is true, except for I/O to/from plain files - the QIOChannelFile
> impl doesn't do anything special (yet) to make that work correctly in
> non-blocking mode. Of course that could be fixed...

Yep, I don't know how I can use the QIOChannelFile for async IO.
AFAICS it's just a wrapper for normal readv/writev.  Who is
responsible to do these IO when I use the IO channel API?  Also does
it guarantee that all IOs are processed in order?

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

Re: [RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2)

[Namhyung Kim]

Hello,

On Thu, Jul 28, 2016 at 01:18:42AM +0300, Michael S. Tsirkin wrote:
> 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

Hmm.. I think I've CC-ed all the kvm, qemu and virtualization lists
but missed the virtio list, sorry.  Will add next time.

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

[PATCH 7/7] kvmtool: Implement virtio-pstore device

[Namhyung Kim]

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 <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Will Deacon <will.dea...@arm.com>
Signed-off-by: Namhyung Kim <namhy...@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_9P0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE0x100a
 #define PCI_DEVICE_ID_VESA 0x2000
 #define PCI_DEVICE_ID_PCI_SHMEM0x0001
 
@@ -34,5 +35,6 @@
 #define PCI_CLASS_RNG  0xff
 #define PCI_CLASS_BLN  0xff
 #define PCI_CLASS_9P   0xff
+#define PCI_CLASS_PSTORE   0xff
 
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 000..670d5e3
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,57 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+#include 
+#include 
+
+#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_DMESG1
+#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;

[PATCH 6/7] qemu: Implement virtio-pstore device

[Namhyung Kim]

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 <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@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(>vdev);
+Error *err = NULL;
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus));
+object_property_set_bool(OBJECT(vdev), true, "realized", );
+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, >vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_PSTORE);
+object_property_add_alias(obj, "directory", OBJECT(>vdev),
+  "directory", _abort);
+object_property_add_alias(obj, "bufsize", OBJECT(>vdev),
+  "bufsize", _abort);
+object_property_add_alias(obj, "console", OBJECT(>vdev),
+  "console", _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(_scsi_pci_info);
 #endif
+type_register_static(_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..b4

[PATCH 5/7] virtio-pstore: Support PSTORE_TYPE_CONSOLE

[Namhyung Kim]

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 <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@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(>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, );
+   virtio_cread(vps->vdev, struct virtio_pstore_config, 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_DMESG1
+#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

[PATCH 4/7] virtio: Basic implementation of virtio pstore driver

[Namhyung Kim]

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 <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@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 ..4ee1c186f582
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,414 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIRT_PSTORE_ORDER2
+#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
+#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(>acked);
+}
+
+static void virtpstore_check(struct virtqueue *vq)
+{
+  

[PATCH 3/7] pstore: Manage buffer position for async write

[Namhyung Kim]

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 <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Matt Fleming <m...@codeblueprint.co.uk>
Cc: linux-...@vger.kernel.org
Signed-off-by: Namhyung Kim <namhy...@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, psinf

[RFC/PATCHSET 0/7] virtio: Implement virtio pstore device (v2)

[Namhyung Kim]

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.

 * 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.00] 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.00] Command line: root=/dev/vda console=ttyS0
  <6>[0.00] x86/fpu: Legacy x87 FPU detected.
  <6>[0.00] x86/fpu: Using 'eager' FPU context switches.
  <6>[0.00] e820: BIOS-provided physical RAM map:
  <6>[0.00] BIOS-e820: [mem 0x-0x0009fbff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x000f-0x000f] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x0010-0x07fddfff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x07fde000-0x07ff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfffc-0x] 
reserved
  <6>[0.00] NX (Execute Disable) protection: active
  <6>[0.00] SMBIOS 2.8 present.
  <7>[0.00] 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  | 

[PATCH 2/7] pstore/ram: Set pstore flags dynamically

[Namhyung Kim]

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 <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Signed-off-by: Namhyung Kim <namhy...@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(>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

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

[PATCH 1/7] pstore: Split pstore fragile flags

[Namhyung Kim]

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 <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: "Rafael J. Wysocki" <r...@rjwysocki.net>
Cc: Len Brown <l...@kernel.org>
Cc: Matt Fleming <m...@codeblueprint.co.uk>
Cc: linux-a...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Namhyung Kim <namhy...@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(>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;
 };
 
-#definePSTORE_FLAGS_FRAGILE1
+#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

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

[PATCH] content: Reserve virtio device ID for pstore

[Namhyung Kim]

From: Namhyung Kim <namhy...@kernel.org>

This patch just reserve next available device ID for pstore device
type.  The device specification for pstore will come later.

Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 content.tex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/content.tex b/content.tex
index d989d98..10bacc5 100644
--- a/content.tex
+++ b/content.tex
@@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device\\
 \hline
 18 &   Input device \\
 \hline
+22 &   pstore device \\
+\hline
 \end{tabular}
 
 Some of the devices above are unspecified by this document,
-- 
2.8.0

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

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

[Namhyung Kim]

On Tue, Jul 19, 2016 at 10:43 PM, Namhyung Kim <namhy...@kernel.org> wrote:
> The other one is the file management on the host side.  I am thinking
> of a simple way that the log file is splitted when it exceeds the half
> of the allowed max size.  It would be configurable and might allow
> unlimited logs if user requests it explicitly (if qemu guys say ok)..

On a second thought, I think that size of log file should not exceed
the pstore buffer size in order to be read back later.  When total size
of log files becomes larger then the limit, the oldest file of same
type can be deleted. This way looks simpler to manage IMHO.

Also I think the pstore id should be managed by host device rather
than guest driver to handle splitted files properly.

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

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

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 <namhy...@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


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

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

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


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

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

[Namhyung Kim]

On Tue, Jul 19, 2016 at 10:43 PM, Namhyung Kim <namhy...@kernel.org> wrote:
> Hi Kees,
>
> On Mon, Jul 18, 2016 at 10:50:06AM -0700, Kees Cook wrote:
>> On Sun, Jul 17, 2016 at 10:50 PM, Namhyung Kim <namhy...@kernel.org> wrote:
>> > Hello,
>> >
>> > On Sun, Jul 17, 2016 at 10:12:26PM -0700, Kees Cook wrote:
>> >> On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim <namhy...@kernel.org> wrote:
>> > [SNIP]
>> >> > +static u16 to_virtio_type(struct virtio_pstore *vps, enum 
>> >> > pstore_type_id type)
>> >> > +{
>> >> > +   u16 ret;
>> >> > +
>> >> > +   switch (type) {
>> >> > +   case PSTORE_TYPE_DMESG:
>> >> > +   ret = cpu_to_virtio16(vps->vdev, 
>> >> > VIRTIO_PSTORE_TYPE_DMESG);
>> >> > +   break;
>> >> > +   default:
>> >> > +   ret = cpu_to_virtio16(vps->vdev, 
>> >> > VIRTIO_PSTORE_TYPE_UNKNOWN);
>> >> > +   break;
>> >> > +   }
>> >>
>> >> I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
>> >> relatively easy to add: I think it'd just be another virtio command?
>> >
>> > Do you want to append the data to the host file as guest does
>> > printk()?  I think it needs some kind of buffer management, but it's
>> > not hard to add IMHO.
>>
>> Well, with most pstore backends, the buffer size is limited, so it
>> tends to be a circular buffer of some sort. I think whatever you
>> choose to do is fine (I saw the various mentions of resource limits in
>> the qemu part of this thread), as long as the last N bytes of console
>> can be seen on the host side, where N is some portion of the memory
>> set aside for the log. (I don't mind the idea of an unlimited console
>> log either, but I suspect that will not be accepted on the qemu
>> side...)
>
> I think it needs two kinds of buffer management.
>
> The first one is the psinfo->buf (or something similar).  IIUC the
> PSTORE_TYPE_CONSOLE is different than PSTORE_TYPE_DMESG as it is
> emitted every time printk() sends messages to console.  So I think the
> it should remain in async mode due to performance reason.  To do that,
> the message should be copied to psinfo->buf and then sent via virtio.
> Then it needs to keep track of the available buffer position IMHO.

Looking at the code, I found myself confused with the PSTORE_TYPE_FTRACE
and PSTORE_TYPE_CONSOLE.  It seems that handling of
PSTORE_TYPE_CONSOLE is basically same as PSTORE_TYPE_DMESG..

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

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

[Namhyung Kim]

Hi Kees,

On Mon, Jul 18, 2016 at 10:50:06AM -0700, Kees Cook wrote:
> On Sun, Jul 17, 2016 at 10:50 PM, Namhyung Kim <namhy...@kernel.org> wrote:
> > Hello,
> >
> > On Sun, Jul 17, 2016 at 10:12:26PM -0700, Kees Cook wrote:
> >> On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim <namhy...@kernel.org> wrote:
> > [SNIP]
> >> > +static u16 to_virtio_type(struct virtio_pstore *vps, enum 
> >> > pstore_type_id type)
> >> > +{
> >> > +   u16 ret;
> >> > +
> >> > +   switch (type) {
> >> > +   case PSTORE_TYPE_DMESG:
> >> > +   ret = cpu_to_virtio16(vps->vdev, 
> >> > VIRTIO_PSTORE_TYPE_DMESG);
> >> > +   break;
> >> > +   default:
> >> > +   ret = cpu_to_virtio16(vps->vdev, 
> >> > VIRTIO_PSTORE_TYPE_UNKNOWN);
> >> > +   break;
> >> > +   }
> >>
> >> I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
> >> relatively easy to add: I think it'd just be another virtio command?
> >
> > Do you want to append the data to the host file as guest does
> > printk()?  I think it needs some kind of buffer management, but it's
> > not hard to add IMHO.
> 
> Well, with most pstore backends, the buffer size is limited, so it
> tends to be a circular buffer of some sort. I think whatever you
> choose to do is fine (I saw the various mentions of resource limits in
> the qemu part of this thread), as long as the last N bytes of console
> can be seen on the host side, where N is some portion of the memory
> set aside for the log. (I don't mind the idea of an unlimited console
> log either, but I suspect that will not be accepted on the qemu
> side...)

I think it needs two kinds of buffer management.

The first one is the psinfo->buf (or something similar).  IIUC the
PSTORE_TYPE_CONSOLE is different than PSTORE_TYPE_DMESG as it is
emitted every time printk() sends messages to console.  So I think the
it should remain in async mode due to performance reason.  To do that,
the message should be copied to psinfo->buf and then sent via virtio.
Then it needs to keep track of the available buffer position IMHO.

The other one is the file management on the host side.  I am thinking
of a simple way that the log file is splitted when it exceeds the half
of the allowed max size.  It would be configurable and might allow
unlimited logs if user requests it explicitly (if qemu guys say ok)..

Maybe we need to use 'part' or 'count' for filenames to identify
the splitted files.

> 
> > [SNIP]
> >> > +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_hdr *hdr = >hdr;
> >> > +   struct scatterlist sg[2];
> >> > +   unsigned int flags = compressed ? VIRTIO_PSTORE_FL_COMPRESSED : 
> >> > 0;
> >> > +
> >> > +   *id = vps->id++;
> >> > +
> >> > +   hdr->cmd   = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_WRITE);
> >> > +   hdr->id= cpu_to_virtio64(vps->vdev, *id);
> >> > +   hdr->flags = cpu_to_virtio32(vps->vdev, flags);
> >> > +   hdr->type  = to_virtio_type(vps, type);
> >> > +
> >> > +   sg_init_table(sg, 2);
> >> > +   sg_set_buf([0], hdr, sizeof(*hdr));
> >> > +   sg_set_buf([1], psi->buf, size);
> >> > +   virtqueue_add_outbuf(vps->vq, sg, 2, vps, GFP_ATOMIC);
> >> > +   virtqueue_kick(vps->vq);
> >> > +
> >> > +   /* TODO: make it synchronous */
> >> > +   return 0;
> >>
> >> The down side to this being asynchronous is the lack of error
> >> reporting. Perhaps this could check hdr->type before queuing and error
> >> for any VIRTIO_PSTORE_TYPE_UNKNOWN message instead of trying to send
> >> it?
> >
> > I cannot follow, sorry.  Could you please elaborate it more?
> 
> The mention you have here of "TODO: make it synchronous" made me think
> about what effects that could have. If a pstore_write() were issued
> for a type

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

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 <namhy...@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 <pbonz...@redhat.com>
> > Cc: Radim Kr??m <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@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, >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,
&g

Re: [PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

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(>vdev);
> > +Error *err = NULL;
> > +
> > +qdev_set_parent_bus(vdev, BUS(_dev->bus));
> > +object_property_set_bool(OBJECT(vdev), true, "realized", );
> > +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, >vdev, sizeof(dev->vdev),
> > +TYPE_VIRTIO_PSTORE);
> > +object_property_add_alias(obj, "directory", OBJECT(>vdev),
> > +  "directory", _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(_scsi_pci_info);
> >  #endif
> > +type_register_static(_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
> > 
> 
> [...]
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

[Namhyung Kim]

Hello,

On Mon, Jul 18, 2016 at 09:54:39AM +0200, Cornelia Huck wrote:
> On Mon, 18 Jul 2016 13:37:39 +0900
> Namhyung Kim <namhy...@kernel.org> wrote:
> 
> > The virtio pstore driver provides interface to the pstore subsystem so
> > that the guest kernel's log/dump message can be saved on the host
> > machine.  Users can access the log file directly on the host, or on the
> > guest at the next boot using pstore filesystem.  It currently deals with
> > kernel log (printk) buffer only, but we can extend it to have other
> > information (like ftrace dump) later.
> 
> Like the idea.

Thanks!

> 
> > 
> > It supports legacy PCI device using single order-2 page buffer.  As all
> 
> There should not be anything in there that limits this to pci, no?

Yep, there's no restriction AFAIK.  I just choose it to implement the poc
code quickly.

> 
> > operation of pstore is synchronous, it would be fine IMHO.  However I
> > don't know how to make write operation synchronous since it's called
> > with a spinlock held (from any context including NMI).
> > 
> > Cc: Paolo Bonzini <pbonz...@redhat.com>
> > Cc: Radim Kr??m <rkrc...@redhat.com>
> > Cc: "Michael S. Tsirkin" <m...@redhat.com>
> > Cc: Anthony Liguori <aligu...@amazon.com>
> > Cc: Anton Vorontsov <an...@enomsg.org>
> > Cc: Colin Cross <ccr...@android.com>
> > Cc: Kees Cook <keesc...@chromium.org>
> > Cc: Tony Luck <tony.l...@intel.com>
> > Cc: Steven Rostedt <rost...@goodmis.org>
> > Cc: Ingo Molnar <mi...@kernel.org>
> > Cc: Minchan Kim <minc...@kernel.org>
> > Cc: k...@vger.kernel.org
> > Cc: qemu-de...@nongnu.org
> > Cc: virtualization@lists.linux-foundation.org
> > Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> > ---
> >  drivers/virtio/Kconfig |  10 ++
> >  drivers/virtio/Makefile|   1 +
> >  drivers/virtio/virtio_pstore.c | 317 
> > +
> >  include/uapi/linux/Kbuild  |   1 +
> >  include/uapi/linux/virtio_ids.h|   1 +
> >  include/uapi/linux/virtio_pstore.h |  53 +++
> >  6 files changed, 383 insertions(+)
> >  create mode 100644 drivers/virtio/virtio_pstore.c
> >  create mode 100644 include/uapi/linux/virtio_pstore.h
> > 
> 
> (...)
> 
> > diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
> > new file mode 100644
> > index ..6fe62c0f1508
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pstore.c
> > @@ -0,0 +1,317 @@
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define VIRT_PSTORE_ORDER2
> > +#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
> 
> It may make sense to make the size of the buffer configurable through
> the config space.

Right.  I'm considering it too, but it needs a buffer larger than
kmsg_bytes (= 10K) to work properly in the current implementation.  As
this version is just to verify the idea is sane and useful, I used a
fixed size buffer.  Will change in the next version.

> 
> (...)
> 
> > diff --git a/include/uapi/linux/virtio_ids.h 
> > b/include/uapi/linux/virtio_ids.h
> > index 77925f587b15..cba63225d85a 100644
> > --- a/include/uapi/linux/virtio_ids.h
> > +++ b/include/uapi/linux/virtio_ids.h
> > @@ -41,5 +41,6 @@
> >  #define VIRTIO_ID_CAIF12 /* Virtio caif */
> >  #define VIRTIO_ID_GPU  16 /* virtio GPU */
> >  #define VIRTIO_ID_INPUT18 /* virtio input */
> > +#define VIRTIO_ID_PSTORE   19 /* virtio pstore */
> 
> This id is already used by one of the new device types queued but not
> yet in the standard. IIRC, 22 is the next free one.

Ok, will update.

> 
> Speaking of the standard: I think it makes sense to at least reserve a
> device id for pstore, as the idea is sound. Maybe prepare a patch to
> the standard as well if you have time?

I'd love to.  As I mentioned earlier, I don't have enough knowledge in
this area.  Could you please provide some links about how can I do that?

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

[PATCH 3/3] kvmtool: Implement virtio-pstore device

[Namhyung Kim]

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-0.enc.z  dmesg-1.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: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@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  |  31 
 include/linux/virtio_ids.h   |   1 +
 virtio/pstore.c  | 359 +++
 7 files changed, 397 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_9P0x1009
+#define PCI_DEVICE_ID_VIRTIO_PSTORE0x100a
 #define PCI_DEVICE_ID_VESA 0x2000
 #define PCI_DEVICE_ID_PCI_SHMEM0x0001
 
@@ -34,5 +35,6 @@
 #define PCI_CLASS_RNG  0xff
 #define PCI_CLASS_BLN  0xff
 #define PCI_CLASS_9P   0xff
+#define PCI_CLASS_PSTORE   0xff
 
 #endif /* VIRTIO_PCI_DEV_H_ */
diff --git a/include/kvm/virtio-pstore.h b/include/kvm/virtio-pstore.h
new file mode 100644
index 000..293ab57
--- /dev/null
+++ b/include/kvm/virtio-pstore.h
@@ -0,0 +1,31 @@
+#ifndef KVM__PSTORE_VIRTIO_H
+#define KVM__PSTORE_VIRTIO_H
+
+struct kvm;
+
+#define VIRTIO_PSTORE_TYPE_UNKNOWN  0
+#define VIRTIO_PSTORE_TYPE_DMESG1
+
+#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
+
+struct pstore_hdr {
+   u64 id;
+   u32 flags;
+   u16 cmd;
+   u16 type;
+   u64 time_sec;
+   u32 time_nsec;
+   u32 unused;
+};
+
+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/in

[PATCH 2/3] qemu: Implement virtio-pstore device

[Namhyung Kim]

From: Namhyung Kim <namhy...@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).

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@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(>vdev);
+Error *err = NULL;
+
+qdev_set_parent_bus(vdev, BUS(_dev->bus));
+object_property_set_bool(OBJECT(vdev), true, "realized", );
+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, >vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_PSTORE);
+object_property_add_alias(obj, "directory", OBJECT(>vdev),
+  "directory", _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(_scsi_pci_info);
 #endif
+type_register_static(_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,

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

[Namhyung Kim]

The virtio pstore driver provides interface to the pstore subsystem so
that the guest kernel's log/dump message can be saved on the host
machine.  Users can access the log file directly on the host, or on the
guest at the next boot using pstore filesystem.  It currently deals with
kernel log (printk) buffer only, but we can extend it to have other
information (like ftrace dump) later.

It supports legacy PCI device using single order-2 page buffer.  As all
operation of pstore is synchronous, it would be fine IMHO.  However I
don't know how to make write operation synchronous since it's called
with a spinlock held (from any context including NMI).

Cc: Paolo Bonzini <pbonz...@redhat.com>
Cc: Radim Krčmář <rkrc...@redhat.com>
Cc: "Michael S. Tsirkin" <m...@redhat.com>
Cc: Anthony Liguori <aligu...@amazon.com>
Cc: Anton Vorontsov <an...@enomsg.org>
Cc: Colin Cross <ccr...@android.com>
Cc: Kees Cook <keesc...@chromium.org>
Cc: Tony Luck <tony.l...@intel.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Ingo Molnar <mi...@kernel.org>
Cc: Minchan Kim <minc...@kernel.org>
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Namhyung Kim <namhy...@kernel.org>
---
 drivers/virtio/Kconfig |  10 ++
 drivers/virtio/Makefile|   1 +
 drivers/virtio/virtio_pstore.c | 317 +
 include/uapi/linux/Kbuild  |   1 +
 include/uapi/linux/virtio_ids.h|   1 +
 include/uapi/linux/virtio_pstore.h |  53 +++
 6 files changed, 383 insertions(+)
 create mode 100644 drivers/virtio/virtio_pstore.c
 create mode 100644 include/uapi/linux/virtio_pstore.h

diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 77590320d44c..8f0e6c796c12 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -58,6 +58,16 @@ config VIRTIO_INPUT
 
 If unsure, say M.
 
+config VIRTIO_PSTORE
+   tristate "Virtio pstore driver"
+   depends on VIRTIO
+   depends on PSTORE
+   ---help---
+This driver supports virtio pstore devices to save/restore
+panic and oops messages on the host.
+
+If unsure, say M.
+
  config VIRTIO_MMIO
tristate "Platform bus driver for memory mapped virtio devices"
depends on HAS_IOMEM && HAS_DMA
diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
index 41e30e3dc842..bee68cb26d48 100644
--- a/drivers/virtio/Makefile
+++ b/drivers/virtio/Makefile
@@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
 virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
 obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
 obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
+obj-$(CONFIG_VIRTIO_PSTORE) += virtio_pstore.o
diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
new file mode 100644
index ..6fe62c0f1508
--- /dev/null
+++ b/drivers/virtio/virtio_pstore.c
@@ -0,0 +1,317 @@
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define VIRT_PSTORE_ORDER2
+#define VIRT_PSTORE_BUFSIZE  (4096 << VIRT_PSTORE_ORDER)
+
+struct virtio_pstore {
+   struct virtio_device*vdev;
+   struct virtqueue*vq;
+   struct pstore_info   pstore;
+   struct virtio_pstore_hdr hdr;
+   size_t   buflen;
+   u64  id;
+
+   /* Waiting for host to ack */
+   wait_queue_head_t   acked;
+};
+
+static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id type)
+{
+   u16 ret;
+
+   switch (type) {
+   case PSTORE_TYPE_DMESG:
+   ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_DMESG);
+   break;
+   default:
+   ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
+   break;
+   }
+
+   return ret;
+}
+
+static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 
type)
+{
+   enum pstore_type_id ret;
+
+   switch (virtio16_to_cpu(vps->vdev, type)) {
+   case VIRTIO_PSTORE_TYPE_DMESG:
+   ret = PSTORE_TYPE_DMESG;
+   break;
+   default:
+   ret = PSTORE_TYPE_UNKNOWN;
+   break;
+   }
+
+   return ret;
+}
+
+static void virtpstore_ack(struct virtqueue *vq)
+{
+   struct virtio_pstore *vps = vq->vdev->priv;
+
+   wake_up(>acked);
+}
+
+static int virt_pstore_open(struct pstore_info *psi)
+{
+   struct virtio_pstore *vps = psi->data;
+   struct virtio_pstore_hdr *hdr = >hdr;
+   struct scatterlist sg[1];
+   unsigned int len;
+
+   hdr->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
+
+   sg_init_one(sg, hdr, sizeof(*hdr));
+   virtqueue_add_outbuf(vps->v

[RFC/PATCHSET 0/3] virtio-pstore: Implement virtio pstore device

[Namhyung Kim]

Hello,

This patchset is a proof of concept of virtio-pstore idea [1].  It has
some rough edges and I'm not familiar with this area, so please give
me feedbacks and advices if I'm going to a wrong direction.

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 implements virtio pstore driver.  It has a single virt
queue, pstore buffer and header structure.  The virtio_pstore_hdr
struct is to give information about the current pstore operation.

The patch 0002 and 0003 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.

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-0.enc.z  dmesg-1.enc.z

As you can see the pstore subsystem compresses the log data using
zlib.  The data can be extracted with the following command:

  $ cat xxx/dmesg-0.enc.z | \
  > python -c 'import sys, zlib; print(zlib.decompress(sys.stdin.read()))'
  Oops#1 Part1
  <5>[0.00] 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.00] Command line: root=/dev/vda console=ttyS0
  <6>[0.00] x86/fpu: Legacy x87 FPU detected.
  <6>[0.00] x86/fpu: Using 'eager' FPU context switches.
  <6>[0.00] e820: BIOS-provided physical RAM map:
  <6>[0.00] BIOS-e820: [mem 0x-0x0009fbff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x000f-0x000f] 
reserved
  <6>[0.00] BIOS-e820: [mem 0x0010-0x07fddfff] 
usable
  <6>[0.00] BIOS-e820: [mem 0x07fde000-0x07ff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] 
reserved
  <6>[0.00] BIOS-e820: [mem 0xfffc-0x] 
reserved
  <6>[0.00] NX (Execute Disable) protection: active
  <6>[0.00] SMBIOS 2.8 present.
  <7>[0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
  ...

Maybe we can add a config option to control the compression later.


Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Cc: "Michael S. Tsirkin" 
Cc: Anthony Liguori 
Cc: Anton Vorontsov 
Cc: Colin Cross 
Cc: Kees Cook 
Cc: Tony Luck 
Cc: Steven Rostedt 
Cc: Ingo Molnar 
Cc: Minchan Kim 
Cc: k...@vger.kernel.org
Cc: qemu-de...@nongnu.org
Cc: virtualization@lists.linux-foundation.org

[1] https://lkml.org/lkml/2016/7/1/6


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