From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751742AbcGRHyz (ORCPT ); Mon, 18 Jul 2016 03:54:55 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:39690 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751456AbcGRHyx convert rfc822-to-8bit (ORCPT ); Mon, 18 Jul 2016 03:54:53 -0400 X-IBM-Helo: d06dlp03.portsmouth.uk.ibm.com X-IBM-MailFrom: cornelia.huck@de.ibm.com X-IBM-RcptTo: kvm@vger.kernel.org;linux-kernel@vger.kernel.org Date: Mon, 18 Jul 2016 09:54:39 +0200 From: Cornelia Huck To: Namhyung Kim Cc: LKML , Paolo Bonzini , Radim =?UTF-8?B?S3LEjW3DocWZ?= , "Michael S. Tsirkin" , Anthony Liguori , Anton Vorontsov , Colin Cross , Kees Cook , Tony Luck , Steven Rostedt , Ingo Molnar , Minchan Kim , 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 In-Reply-To: <1468816661-6345-2-git-send-email-namhyung@kernel.org> References: <1468816661-6345-1-git-send-email-namhyung@kernel.org> <1468816661-6345-2-git-send-email-namhyung@kernel.org> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16071807-0040-0000-0000-000002AC0DF7 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16071807-0041-0000-0000-00001C3E9781 Message-Id: <20160718095439.1eabb340.cornelia.huck@de.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-18_04:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1607180095 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 18 Jul 2016 13:37:39 +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. Like the idea. > > 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? > operation of pstore is synchronous, it would be fine IMHO. However I > don't know how to make write operation synchronous since it's called > with a spinlock held (from any context including NMI). > > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: "Michael S. Tsirkin" > Cc: Anthony Liguori > Cc: Anton Vorontsov > Cc: Colin Cross > Cc: Kees Cook > Cc: Tony Luck > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: Minchan Kim > Cc: kvm@vger.kernel.org > Cc: qemu-devel@nongnu.org > Cc: virtualization@lists.linux-foundation.org > Signed-off-by: Namhyung Kim > --- > 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 > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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. (...) > 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. 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? > > #endif /* _LINUX_VIRTIO_IDS_H */