From: Namhyung Kim <namhyung@kernel.org> To: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: LKML <linux-kernel@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Radim Kr??m???? <rkrcmar@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Anthony Liguori <aliguori@amazon.com>, Anton Vorontsov <anton@enomsg.org>, Colin Cross <ccross@android.com>, Kees Cook <keescook@chromium.org>, Tony Luck <tony.luck@intel.com>, Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@kernel.org>, Minchan Kim <minchan@kernel.org>, kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org Subject: Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Date: Mon, 18 Jul 2016 17:29:55 +0900 [thread overview] Message-ID: <20160718082955.GA12086@danjae.aot.lge.com> (raw) In-Reply-To: <20160718095439.1eabb340.cornelia.huck@de.ibm.com> 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 <namhyung@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 <pbonzini@redhat.com> > > Cc: Radim Kr??m???? <rkrcmar@redhat.com> > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Anthony Liguori <aliguori@amazon.com> > > Cc: Anton Vorontsov <anton@enomsg.org> > > Cc: Colin Cross <ccross@android.com> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Tony Luck <tony.luck@intel.com> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Minchan Kim <minchan@kernel.org> > > Cc: kvm@vger.kernel.org > > Cc: qemu-devel@nongnu.org > > Cc: virtualization@lists.linux-foundation.org > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > drivers/virtio/Kconfig | 10 ++ > > drivers/virtio/Makefile | 1 + > > drivers/virtio/virtio_pstore.c | 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 000000000000..6fe62c0f1508 > > --- /dev/null > > +++ b/drivers/virtio/virtio_pstore.c > > @@ -0,0 +1,317 @@ > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/pstore.h> > > +#include <linux/virtio.h> > > +#include <linux/virtio_config.h> > > +#include <uapi/linux/virtio_ids.h> > > +#include <uapi/linux/virtio_pstore.h> > > + > > +#define VIRT_PSTORE_ORDER 2 > > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER) > > 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_CAIF 12 /* Virtio caif */ > > #define VIRTIO_ID_GPU 16 /* virtio GPU */ > > #define VIRTIO_ID_INPUT 18 /* virtio input */ > > +#define VIRTIO_ID_PSTORE 19 /* virtio pstore */ > > 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
WARNING: multiple messages have this Message-ID (diff)
From: Namhyung Kim <namhyung@kernel.org> To: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: LKML <linux-kernel@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Radim Kr??m???? <rkrcmar@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Anthony Liguori <aliguori@amazon.com>, Anton Vorontsov <anton@enomsg.org>, Colin Cross <ccross@android.com>, Kees Cook <keescook@chromium.org>, Tony Luck <tony.luck@intel.com>, Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@kernel.org>, Minchan Kim <minchan@kernel.org>, kvm@vger.kernel.org, qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org Subject: Re: [Qemu-devel] [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Date: Mon, 18 Jul 2016 17:29:55 +0900 [thread overview] Message-ID: <20160718082955.GA12086@danjae.aot.lge.com> (raw) In-Reply-To: <20160718095439.1eabb340.cornelia.huck@de.ibm.com> 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 <namhyung@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 <pbonzini@redhat.com> > > Cc: Radim Kr??m???? <rkrcmar@redhat.com> > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Anthony Liguori <aliguori@amazon.com> > > Cc: Anton Vorontsov <anton@enomsg.org> > > Cc: Colin Cross <ccross@android.com> > > Cc: Kees Cook <keescook@chromium.org> > > Cc: Tony Luck <tony.luck@intel.com> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Minchan Kim <minchan@kernel.org> > > Cc: kvm@vger.kernel.org > > Cc: qemu-devel@nongnu.org > > Cc: virtualization@lists.linux-foundation.org > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > drivers/virtio/Kconfig | 10 ++ > > drivers/virtio/Makefile | 1 + > > drivers/virtio/virtio_pstore.c | 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 000000000000..6fe62c0f1508 > > --- /dev/null > > +++ b/drivers/virtio/virtio_pstore.c > > @@ -0,0 +1,317 @@ > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/pstore.h> > > +#include <linux/virtio.h> > > +#include <linux/virtio_config.h> > > +#include <uapi/linux/virtio_ids.h> > > +#include <uapi/linux/virtio_pstore.h> > > + > > +#define VIRT_PSTORE_ORDER 2 > > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER) > > 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_CAIF 12 /* Virtio caif */ > > #define VIRTIO_ID_GPU 16 /* virtio GPU */ > > #define VIRTIO_ID_INPUT 18 /* virtio input */ > > +#define VIRTIO_ID_PSTORE 19 /* virtio pstore */ > > 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
next prev parent reply other threads:[~2016-07-18 8:31 UTC|newest] Thread overview: 111+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-18 4:37 [RFC/PATCHSET 0/3] virtio-pstore: Implement virtio pstore device Namhyung Kim 2016-07-18 4:37 ` [Qemu-devel] " Namhyung Kim 2016-07-18 4:37 ` [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Namhyung Kim 2016-07-18 4:37 ` Namhyung Kim 2016-07-18 4:37 ` [Qemu-devel] " Namhyung Kim 2016-07-18 5:12 ` Kees Cook 2016-07-18 5:12 ` Kees Cook 2016-07-18 5:12 ` [Qemu-devel] " Kees Cook 2016-07-18 5:50 ` Namhyung Kim 2016-07-18 5:50 ` Namhyung Kim 2016-07-18 5:50 ` [Qemu-devel] " Namhyung Kim 2016-07-18 5:50 ` Namhyung Kim 2016-07-18 17:50 ` Kees Cook 2016-07-18 17:50 ` [Qemu-devel] " Kees Cook 2016-07-18 17:50 ` Kees Cook 2016-07-19 13:43 ` Namhyung Kim 2016-07-19 13:43 ` [Qemu-devel] " Namhyung Kim 2016-07-19 13:43 ` Namhyung Kim 2016-07-19 15:32 ` Namhyung Kim 2016-07-19 15:32 ` [Qemu-devel] " Namhyung Kim 2016-07-19 15:32 ` Namhyung Kim 2016-07-20 12:56 ` Namhyung Kim 2016-07-20 12:56 ` [Qemu-devel] " Namhyung Kim 2016-07-20 12:56 ` Namhyung Kim 2016-07-18 7:54 ` Cornelia Huck 2016-07-18 7:54 ` Cornelia Huck 2016-07-18 7:54 ` [Qemu-devel] " Cornelia Huck 2016-07-18 8:29 ` Namhyung Kim 2016-07-18 8:29 ` Namhyung Kim [this message] 2016-07-18 8:29 ` [Qemu-devel] " Namhyung Kim 2016-07-18 9:02 ` Cornelia Huck 2016-07-18 9:02 ` [Qemu-devel] " Cornelia Huck 2016-07-18 9:02 ` Cornelia Huck 2016-07-18 4:37 ` [PATCH 2/3] qemu: Implement virtio-pstore device Namhyung Kim 2016-07-18 4:37 ` [Qemu-devel] " Namhyung Kim 2016-07-18 7:28 ` Christian Borntraeger 2016-07-18 7:28 ` [Qemu-devel] " Christian Borntraeger 2016-07-18 7:28 ` Christian Borntraeger 2016-07-18 8:33 ` Namhyung Kim 2016-07-18 8:33 ` [Qemu-devel] " Namhyung Kim 2016-07-18 8:33 ` Namhyung Kim 2016-07-18 10:03 ` Stefan Hajnoczi 2016-07-18 10:03 ` [Qemu-devel] " Stefan Hajnoczi 2016-07-18 10:03 ` Stefan Hajnoczi 2016-07-18 14:21 ` Namhyung Kim 2016-07-18 14:21 ` Namhyung Kim 2016-07-18 14:21 ` [Qemu-devel] " Namhyung Kim 2016-07-20 8:29 ` Stefan Hajnoczi 2016-07-20 8:29 ` [Qemu-devel] " Stefan Hajnoczi 2016-07-20 8:29 ` Stefan Hajnoczi 2016-07-20 12:46 ` Namhyung Kim 2016-07-20 12:46 ` [Qemu-devel] " Namhyung Kim 2016-07-20 12:46 ` Namhyung Kim 2016-07-19 15:48 ` Namhyung Kim 2016-07-19 15:48 ` Namhyung Kim 2016-07-19 15:48 ` [Qemu-devel] " Namhyung Kim 2016-07-20 8:21 ` Stefan Hajnoczi 2016-07-20 8:21 ` [Qemu-devel] " Stefan Hajnoczi 2016-07-20 8:21 ` Stefan Hajnoczi 2016-07-20 12:30 ` Namhyung Kim 2016-07-20 12:30 ` [Qemu-devel] " Namhyung Kim 2016-07-20 12:30 ` Namhyung Kim 2016-07-18 4:37 ` Namhyung Kim 2016-07-18 4:37 ` [PATCH 3/3] kvmtool: " Namhyung Kim 2016-07-18 4:37 ` Namhyung Kim 2016-08-20 8:07 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v3) Namhyung Kim 2016-08-20 8:07 ` [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Namhyung Kim 2016-08-20 8:07 ` Namhyung Kim 2016-09-13 15:19 ` Michael S. Tsirkin 2016-09-16 9:05 ` Namhyung Kim 2016-09-16 9:05 ` Namhyung Kim 2016-09-13 15:19 ` Michael S. Tsirkin 2016-11-10 16:39 ` Michael S. Tsirkin 2016-11-15 4:50 ` Namhyung Kim 2016-11-15 4:50 ` Namhyung Kim 2016-11-15 5:06 ` Michael S. Tsirkin 2016-11-15 5:06 ` Michael S. Tsirkin 2016-11-15 5:50 ` Namhyung Kim 2016-11-15 5:50 ` Namhyung Kim 2016-11-15 14:35 ` Michael S. Tsirkin 2016-11-15 14:35 ` Michael S. Tsirkin 2016-11-15 9:57 ` Paolo Bonzini 2016-11-15 9:57 ` Paolo Bonzini 2016-11-15 14:36 ` Namhyung Kim 2016-11-15 14:36 ` Namhyung Kim 2016-11-15 14:38 ` Paolo Bonzini 2016-11-15 14:38 ` Paolo Bonzini 2016-11-16 7:04 ` Namhyung Kim 2016-11-16 7:04 ` Namhyung Kim 2016-11-16 12:10 ` Paolo Bonzini 2016-11-16 12:10 ` Paolo Bonzini 2016-11-18 3:32 ` Namhyung Kim 2016-11-18 3:32 ` Namhyung Kim 2016-11-18 4:07 ` Michael S. Tsirkin 2016-11-18 4:07 ` Michael S. Tsirkin 2016-11-18 9:45 ` Paolo Bonzini 2016-11-18 9:45 ` Paolo Bonzini 2016-11-10 16:39 ` Michael S. Tsirkin 2016-08-31 8:07 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v4) Namhyung Kim 2016-08-31 8:08 ` [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Namhyung Kim 2016-08-31 8:08 ` Namhyung Kim 2016-08-31 14:54 ` Michael S. Tsirkin 2016-08-31 14:54 ` Michael S. Tsirkin 2016-09-01 0:03 ` Namhyung Kim 2016-09-01 0:03 ` Namhyung Kim 2016-09-04 14:38 [RFC/PATCHSET 0/3] virtio: Implement virtio pstore device (v5) Namhyung Kim 2016-09-04 14:38 ` [PATCH 1/3] virtio: Basic implementation of virtio pstore driver Namhyung Kim 2016-09-04 14:38 ` Namhyung Kim 2016-09-08 20:49 ` Kees Cook 2016-09-08 20:49 ` Kees Cook 2016-09-22 11:57 ` Stefan Hajnoczi 2016-09-22 11:57 ` Stefan Hajnoczi 2016-09-23 5:48 ` Namhyung Kim 2016-09-23 5:48 ` Namhyung Kim
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160718082955.GA12086@danjae.aot.lge.com \ --to=namhyung@kernel.org \ --cc=aliguori@amazon.com \ --cc=anton@enomsg.org \ --cc=ccross@android.com \ --cc=cornelia.huck@de.ibm.com \ --cc=keescook@chromium.org \ --cc=kvm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=minchan@kernel.org \ --cc=mingo@kernel.org \ --cc=mst@redhat.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=rkrcmar@redhat.com \ --cc=rostedt@goodmis.org \ --cc=tony.luck@intel.com \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.