* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Jiri Slaby @ 2016-11-15 8:55 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: David Airlie, virtualization, Linux kernel mailing list,
dri-devel, Michael S. Tsirkin
In-Reply-To: <1479199588.32639.16.camel@redhat.com>
On 11/15/2016, 09:46 AM, Gerd Hoffmann wrote:
> On Fr, 2016-11-11 at 17:28 +0100, Jiri Slaby wrote:
>> On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote:
>>> On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote:
>>>> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
>>>>> Hi,
>>>>>
>>>>> I can relatively easily reproduce this bug:
>>>
>>> How?
>>
>> Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output.
>
> fbcon? Or xorg/wayland with terminal app?
Ah, just console, so fbcon. No X server running.
thanks,
--
js
suse labs
^ permalink raw reply
* [PATCH for-4.9] drm/virtio: allocate some extra bufs
From: Gerd Hoffmann @ 2016-11-15 8:53 UTC (permalink / raw)
To: David Airlie, dri-devel; +Cc: open list, open list:VIRTIO GPU DRIVER
virtio-gpu guest driver appearently can run out of buffers.
allocate some extra buffers, as quick stopgap for 4.9.
analyzing root cause and fixing it properly is TBD.
Reported-by: Jiri Slaby <jslaby@suse.cz>
Tested-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..974f941 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -75,7 +75,7 @@ void virtio_gpu_cursor_ack(struct virtqueue *vq)
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
{
struct virtio_gpu_vbuffer *vbuf;
- int i, size, count = 0;
+ int i, size, count = 16;
void *ptr;
INIT_LIST_HEAD(&vgdev->free_vbufs);
--
1.8.3.1
^ permalink raw reply related
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Gerd Hoffmann @ 2016-11-15 8:46 UTC (permalink / raw)
To: Jiri Slaby
Cc: David Airlie, virtualization, Linux kernel mailing list,
dri-devel, Michael S. Tsirkin
In-Reply-To: <f006e3a6-51d2-4092-57b6-eea7ff64f58f@suse.cz>
On Fr, 2016-11-11 at 17:28 +0100, Jiri Slaby wrote:
> On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote:
> > On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote:
> >> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> >>> Hi,
> >>>
> >>> I can relatively easily reproduce this bug:
> >
> > How?
>
> Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output.
fbcon? Or xorg/wayland with terminal app?
> Run pps [1] without exit(0); on e.g. serial console.
> Wait a bit. The lot of output causes the BUG.
>
> [1] https://github.com/jirislaby/collected_sources/blob/master/pps.c
>
> >>> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> >
> >> The following might be helpful for debugging - if kernel still will
> >> not stop panicing, we are looking at some kind
> >> of memory corruption.
> >
> > Looking carefully through the code I think it isn't impossible to
> > trigger this, but you need for that:
> >
> > (1) command queue full (quite possible),
> > (2) cursor queue full too (unlikely), and
> > (3) multiple threads trying to submit commands and waiting for free
> > space in the command queue (possible with virgl enabled).
>
> I use -vga virtio with no -display option, so no virtgl, I suppose:
> [drm] virgl 3d acceleration not available
>
> > Do things improve if you allocate some extra bufs?
> >
> > int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
> > {
> > struct virtio_gpu_vbuffer *vbuf;
> > - int i, size, count = 0;
> > + int i, size, count = 16;
>
> This seems to help.
>
> thanks,
^ permalink raw reply
* Re: [PATCH 2/3] qemu: Implement virtio-pstore device
From: Namhyung Kim @ 2016-11-15 6:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, Tony Luck, Daniel P . Berrange, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20161111004710-mutt-send-email-mst@kernel.org>
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
^ permalink raw reply
* Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
From: Namhyung Kim @ 2016-11-15 5:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, Tony Luck, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20161115065658-mutt-send-email-mst@kernel.org>
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
^ permalink raw reply
* Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
From: Michael S. Tsirkin @ 2016-11-15 5:06 UTC (permalink / raw)
To: Namhyung Kim
Cc: virtio-dev, Tony Luck, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20161115045021.GA15992@danjae.aot.lge.com>
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.
> 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 <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 | 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 000000000000..0a63c7db4278
> > > --- /dev/null
> > > +++ b/drivers/virtio/virtio_pstore.c
> > > @@ -0,0 +1,417 @@
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pstore.h>
> > > +#include <linux/virtio.h>
> > > +#include <linux/virtio_config.h>
> > > +#include <uapi/linux/virtio_ids.h>
> > > +#include <uapi/linux/virtio_pstore.h>
> > > +
> > > +#define VIRT_PSTORE_ORDER 2
> > > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
> > > +#define VIRT_PSTORE_NR_REQ 128
> > > +
> > > +struct virtio_pstore {
> > > + struct virtio_device *vdev;
> > > + struct virtqueue *vq[2];
> >
> > 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;
> > > + 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
> >
> > let's avoid macros for now pls. In fact, I would just open-code this
> > in to_virtio_type below. We can always change our minds later if
> > lots of types are added.
>
> Yep.
>
> >
> > > +
> > > +
> >
> > single emoty line pls
>
> Ok.
>
> >
> > > +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);
> >
> > This assigns u16 to __virtio type, sparse will warn
> > if you enable endian-ness checks.
> > Pls fix that and generally, please make sure this is
> > clean from sparse warnings.
>
> I'll run sparse before sending patch next time.
>
> >
> > > +}
> > > +
> > > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type)
> > > +{
> > > + unsigned int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(type_table); i++) {
> > > + if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio)
> > > + return type_table[i].pstore;
> > > + }
> > > +
> > > + return PSTORE_TYPE_UNKNOWN;
> > > +}
> > > +
> > > +static void virtpstore_ack(struct virtqueue *vq)
> > > +{
> > > + struct virtio_pstore *vps = vq->vdev->priv;
> > > +
> > > + wake_up(&vps->acked);
> > > +}
> > > +
> > > +static void virtpstore_check(struct virtqueue *vq)
> > > +{
> > > + struct virtio_pstore *vps = vq->vdev->priv;
> > > + struct virtio_pstore_res *res;
> > > + unsigned int len;
> > > +
> > > + res = virtqueue_get_buf(vq, &len);
> > > + if (res == NULL)
> > > + return;
> > > +
> > > + if (virtio32_to_cpu(vq->vdev, res->ret) < 0)
> > > + vps->failed = 1;
> > > +}
> > > +
> > > +static void virt_pstore_get_reqs(struct virtio_pstore *vps,
> > > + struct virtio_pstore_req **preq,
> > > + struct virtio_pstore_res **pres)
> > > +{
> > > + unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ;
> > > +
> > > + *preq = &vps->req[idx];
> > > + *pres = &vps->res[idx];
> > > +
> > > + memset(*preq, 0, sizeof(**preq));
> > > + memset(*pres, 0, sizeof(**pres));
> > > +}
> > > +
> > > +static int virt_pstore_open(struct pstore_info *psi)
> > > +{
> > > + struct virtio_pstore *vps = psi->data;
> > > + struct virtio_pstore_req *req;
> > > + struct virtio_pstore_res *res;
> > > + struct scatterlist sgo[1], sgi[1];
> > > + struct scatterlist *sgs[2] = { sgo, sgi };
> > > + unsigned int len;
> > > +
> > > + virt_pstore_get_reqs(vps, &req, &res);
> > > +
> > > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
> > > +
> > > + sg_init_one(sgo, req, sizeof(*req));
> > > + sg_init_one(sgi, res, sizeof(*res));
> > > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > > + virtqueue_kick(vps->vq[0]);
> > > +
> > > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> >
> > Does this block userspace in an uninterruptible wait if
> > hardware is slow? That's not nice.
>
> Yes, but it's not a common operation and I just wanted to make it
> simple.
>
>
> >
> > > + return virtio32_to_cpu(vps->vdev, res->ret);
> > > +}
> > > +
>
> [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.
>
> Thanks for your review!
> Namhyung
>
> >
> > > +#endif /* _LINUX_VIRTIO_PSTORE_H */
> > > --
> > > 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
From: Namhyung Kim @ 2016-11-15 4:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtio-dev, Tony Luck, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20161110182611-mutt-send-email-mst@kernel.org>
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 <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 | 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 000000000000..0a63c7db4278
> > --- /dev/null
> > +++ b/drivers/virtio/virtio_pstore.c
> > @@ -0,0 +1,417 @@
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/pstore.h>
> > +#include <linux/virtio.h>
> > +#include <linux/virtio_config.h>
> > +#include <uapi/linux/virtio_ids.h>
> > +#include <uapi/linux/virtio_pstore.h>
> > +
> > +#define VIRT_PSTORE_ORDER 2
> > +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
> > +#define VIRT_PSTORE_NR_REQ 128
> > +
> > +struct virtio_pstore {
> > + struct virtio_device *vdev;
> > + struct virtqueue *vq[2];
>
> 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;
> > + 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
>
> let's avoid macros for now pls. In fact, I would just open-code this
> in to_virtio_type below. We can always change our minds later if
> lots of types are added.
Yep.
>
> > +
> > +
>
> single emoty line pls
Ok.
>
> > +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);
>
> This assigns u16 to __virtio type, sparse will warn
> if you enable endian-ness checks.
> Pls fix that and generally, please make sure this is
> clean from sparse warnings.
I'll run sparse before sending patch next time.
>
> > +}
> > +
> > +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(type_table); i++) {
> > + if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio)
> > + return type_table[i].pstore;
> > + }
> > +
> > + return PSTORE_TYPE_UNKNOWN;
> > +}
> > +
> > +static void virtpstore_ack(struct virtqueue *vq)
> > +{
> > + struct virtio_pstore *vps = vq->vdev->priv;
> > +
> > + wake_up(&vps->acked);
> > +}
> > +
> > +static void virtpstore_check(struct virtqueue *vq)
> > +{
> > + struct virtio_pstore *vps = vq->vdev->priv;
> > + struct virtio_pstore_res *res;
> > + unsigned int len;
> > +
> > + res = virtqueue_get_buf(vq, &len);
> > + if (res == NULL)
> > + return;
> > +
> > + if (virtio32_to_cpu(vq->vdev, res->ret) < 0)
> > + vps->failed = 1;
> > +}
> > +
> > +static void virt_pstore_get_reqs(struct virtio_pstore *vps,
> > + struct virtio_pstore_req **preq,
> > + struct virtio_pstore_res **pres)
> > +{
> > + unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ;
> > +
> > + *preq = &vps->req[idx];
> > + *pres = &vps->res[idx];
> > +
> > + memset(*preq, 0, sizeof(**preq));
> > + memset(*pres, 0, sizeof(**pres));
> > +}
> > +
> > +static int virt_pstore_open(struct pstore_info *psi)
> > +{
> > + struct virtio_pstore *vps = psi->data;
> > + struct virtio_pstore_req *req;
> > + struct virtio_pstore_res *res;
> > + struct scatterlist sgo[1], sgi[1];
> > + struct scatterlist *sgs[2] = { sgo, sgi };
> > + unsigned int len;
> > +
> > + virt_pstore_get_reqs(vps, &req, &res);
> > +
> > + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
> > +
> > + sg_init_one(sgo, req, sizeof(*req));
> > + sg_init_one(sgi, res, sizeof(*res));
> > + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> > + virtqueue_kick(vps->vq[0]);
> > +
> > + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
>
> Does this block userspace in an uninterruptible wait if
> hardware is slow? That's not nice.
Yes, but it's not a common operation and I just wanted to make it
simple.
>
> > + return virtio32_to_cpu(vps->vdev, res->ret);
> > +}
> > +
[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.
Thanks for your review!
Namhyung
>
> > +#endif /* _LINUX_VIRTIO_PSTORE_H */
> > --
> > 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH] virtio_ring: fix description of virtqueue_get_buf
From: Jason Wang @ 2016-11-15 3:21 UTC (permalink / raw)
To: Felipe Franciosi, Michael S. Tsirkin; +Cc: linux-kernel, virtualization
In-Reply-To: <1479132975-8517-1-git-send-email-felipe@nutanix.com>
On 2016年11月14日 22:16, Felipe Franciosi wrote:
> The device (not the driver) populates the used ring and includes the len
> of how much data was written.
>
> Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
> ---
> drivers/virtio/virtio_ring.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 489bfc6..8a0d6a9 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -649,7 +649,7 @@ static inline bool more_used(const struct vring_virtqueue *vq)
> * @vq: the struct virtqueue we're talking about.
> * @len: the length written into the buffer
> *
> - * If the driver wrote data into the buffer, @len will be set to the
> + * If the device wrote data into the buffer, @len will be set to the
> * amount written. This means you don't need to clear the buffer
> * beforehand to ensure there's no data leakage in the case of short
> * writes.
Reviewed-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH] virtio_ring: fix description of virtqueue_get_buf
From: Felipe Franciosi @ 2016-11-14 14:16 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: Felipe Franciosi, linux-kernel, virtualization
The device (not the driver) populates the used ring and includes the len
of how much data was written.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
---
drivers/virtio/virtio_ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 489bfc6..8a0d6a9 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -649,7 +649,7 @@ static inline bool more_used(const struct vring_virtqueue *vq)
* @vq: the struct virtqueue we're talking about.
* @len: the length written into the buffer
*
- * If the driver wrote data into the buffer, @len will be set to the
+ * If the device wrote data into the buffer, @len will be set to the
* amount written. This means you don't need to clear the buffer
* beforehand to ensure there's no data leakage in the case of short
* writes.
--
1.9.4
^ permalink raw reply related
* [PATCH] crypto: add virtio-crypto driver
From: Gonglei @ 2016-11-14 6:47 UTC (permalink / raw)
To: qemu-devel, virtio-dev, virtualization, linux-crypto
Cc: weidong.huang, claudio.fontana, mst, luonengjun, hanweidong,
peter.huangpeng, xuquan8, Gonglei, stefanha, jianjay.zhou,
arei.gonglei, davem, wu.wubin, herbert
This patch introduces virtio-crypto driver for Linux Kernel.
The virtio crypto device is a virtual cryptography device
as well as a kind of virtual hardware accelerator for
virtual machines. The encryption anddecryption requests
are placed in the data queue and are ultimately handled by
thebackend crypto accelerators. The second queue is the
control queue used to create or destroy sessions for
symmetric algorithms and will control some advanced features
in the future. The virtio crypto device provides the following
cryptoservices: CIPHER, MAC, HASH, and AEAD.
For more information about virtio-crypto device, please see:
http://qemu-project.org/Features/VirtioCrypto
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Cornelia Huck <cornelia.huck@de.ibm.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: Halil Pasic <pasic@linux.vnet.ibm.com>
CC: David S. Miller <davem@davemloft.net>
CC: Zeng Xin <xin.zeng@intel.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
MAINTAINERS | 8 +
drivers/crypto/Kconfig | 2 +
drivers/crypto/Makefile | 1 +
drivers/crypto/virtio/Kconfig | 10 +
drivers/crypto/virtio/Makefile | 5 +
drivers/crypto/virtio/virtio_crypto.c | 437 +++++++++++++++++++++++
drivers/crypto/virtio/virtio_crypto_algs.c | 496 +++++++++++++++++++++++++++
drivers/crypto/virtio/virtio_crypto_common.h | 115 +++++++
drivers/crypto/virtio/virtio_crypto_mgr.c | 261 ++++++++++++++
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/virtio_crypto.h | 436 +++++++++++++++++++++++
include/uapi/linux/virtio_ids.h | 1 +
12 files changed, 1773 insertions(+)
create mode 100644 drivers/crypto/virtio/Kconfig
create mode 100644 drivers/crypto/virtio/Makefile
create mode 100644 drivers/crypto/virtio/virtio_crypto.c
create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
create mode 100644 include/uapi/linux/virtio_crypto.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b8..d6b18bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12844,6 +12844,14 @@ S: Maintained
F: drivers/virtio/virtio_input.c
F: include/uapi/linux/virtio_input.h
+VIRTIO CRYPTO DRIVER
+M: Gonglei <arei.gonglei@huawei.com>
+L: virtualization@lists.linux-foundation.org
+L: linux-crypto@vger.kernel.org
+S: Maintained
+F: drivers/crypto/virtio/
+F: include/uapi/linux/virtio_crypto.h
+
VIA RHINE NETWORK DRIVER
S: Orphan
F: drivers/net/ethernet/via/via-rhine.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..7956478 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP
source "drivers/crypto/chelsio/Kconfig"
+source "drivers/crypto/virtio/Kconfig"
+
endif # CRYPTO_HW
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index ad7250f..bc53cb8 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig
new file mode 100644
index 0000000..ceae88c
--- /dev/null
+++ b/drivers/crypto/virtio/Kconfig
@@ -0,0 +1,10 @@
+config CRYPTO_DEV_VIRTIO
+ tristate "VirtIO crypto driver"
+ depends on VIRTIO
+ select CRYPTO_AEAD
+ select CRYPTO_AUTHENC
+ select CRYPTO_BLKCIPHER
+ default m
+ help
+ This driver provides support for virtio crypto device. If you
+ choose 'M' here, this module will be called virtio-crypto.
diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile
new file mode 100644
index 0000000..a316e92
--- /dev/null
+++ b/drivers/crypto/virtio/Makefile
@@ -0,0 +1,5 @@
+obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio-crypto.o
+virtio-crypto-objs := \
+ virtio_crypto_algs.o \
+ virtio_crypto_mgr.o \
+ virtio_crypto.o
diff --git a/drivers/crypto/virtio/virtio_crypto.c b/drivers/crypto/virtio/virtio_crypto.c
new file mode 100644
index 0000000..6c9de18
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto.c
@@ -0,0 +1,437 @@
+ /* Driver for Virtio crypto device.
+ *
+ * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/virtio_config.h>
+#include <linux/cpu.h>
+
+#include <uapi/linux/virtio_crypto.h>
+#include "virtio_crypto_common.h"
+
+
+static void virtcrypto_dataq_callback(struct virtqueue *vq)
+{
+ struct virtio_crypto *vcrypto = vq->vdev->priv;
+ struct virtio_crypto_request *vc_req;
+ unsigned long flags;
+ unsigned int len;
+ struct ablkcipher_request *ablk_req;
+ int error;
+
+ spin_lock_irqsave(&vcrypto->lock, flags);
+ do {
+ virtqueue_disable_cb(vq);
+ while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
+ if (vc_req->type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
+ switch (vc_req->status) {
+ case VIRTIO_CRYPTO_OK:
+ error = 0;
+ break;
+ case VIRTIO_CRYPTO_INVSESS:
+ case VIRTIO_CRYPTO_ERR:
+ error = -EINVAL;
+ break;
+ case VIRTIO_CRYPTO_BADMSG:
+ error = -EBADMSG;
+ break;
+ default:
+ error = -EIO;
+ break;
+ }
+ ablk_req = vc_req->ablkcipher_req;
+ /* Finish the encrypt or decrypt process */
+ ablk_req->base.complete(&ablk_req->base, error);
+ }
+
+ kfree(vc_req->req_data);
+ kfree(vc_req->sgs);
+ }
+ } while (!virtqueue_enable_cb(vq));
+ spin_unlock_irqrestore(&vcrypto->lock, flags);
+}
+
+static int virtcrypto_find_vqs(struct virtio_crypto *vi)
+{
+ vq_callback_t **callbacks;
+ struct virtqueue **vqs;
+ int ret = -ENOMEM;
+ int i, total_vqs;
+ const char **names;
+
+ /* We expect 1 data virtqueue, followed by
+ * possible N-1 data queues used in multiqueue mode, followed by
+ * control vq.
+ */
+ total_vqs = vi->max_data_queues + 1;
+
+ /* Allocate space for find_vqs parameters */
+ vqs = kcalloc(total_vqs, sizeof(*vqs), GFP_KERNEL);
+ if (!vqs)
+ goto err_vq;
+ callbacks = kcalloc(total_vqs, sizeof(*callbacks), GFP_KERNEL);
+ if (!callbacks)
+ goto err_callback;
+ names = kcalloc(total_vqs, sizeof(*names), GFP_KERNEL);
+ if (!names)
+ goto err_names;
+
+ /* Parameters for control virtqueue */
+ callbacks[total_vqs - 1] = NULL;
+ names[total_vqs - 1] = "controlq";
+
+ /* Allocate/initialize parameters for data virtqueues */
+ for (i = 0; i < vi->max_data_queues; i++) {
+ callbacks[i] = virtcrypto_dataq_callback;
+ snprintf(vi->data_vq[i].name, sizeof(vi->data_vq[i].name),
+ "dataq.%d", i);
+ names[i] = vi->data_vq[i].name;
+ }
+
+ ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
+ names);
+ if (ret)
+ goto err_find;
+
+ vi->ctrl_vq = vqs[total_vqs - 1];
+
+ for (i = 0; i < vi->max_data_queues; i++)
+ vi->data_vq[i].vq = vqs[i];
+
+ kfree(names);
+ kfree(callbacks);
+ kfree(vqs);
+
+ return 0;
+
+err_find:
+ kfree(names);
+err_names:
+ kfree(callbacks);
+err_callback:
+ kfree(vqs);
+err_vq:
+ return ret;
+}
+
+static int virtcrypto_alloc_queues(struct virtio_crypto *vi)
+{
+ vi->data_vq = kcalloc(vi->max_data_queues, sizeof(*vi->data_vq),
+ GFP_KERNEL);
+ if (!vi->data_vq)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void virtcrypto_clean_affinity(struct virtio_crypto *vi, long hcpu)
+{
+ int i;
+
+ if (vi->affinity_hint_set) {
+ for (i = 0; i < vi->max_data_queues; i++)
+ virtqueue_set_affinity(vi->data_vq[i].vq, -1);
+
+ vi->affinity_hint_set = false;
+ }
+}
+
+static void virtcrypto_set_affinity(struct virtio_crypto *vi)
+{
+ int i;
+ int cpu;
+
+ /*
+ * In multiqueue mode, when the number of cpu is equal to the number
+ * of queue, we let the queue to be private to one cpu by
+ * setting the affinity hint to eliminate the contention.
+ */
+ if (vi->curr_queue == 1 ||
+ vi->max_data_queues != num_online_cpus()) {
+ virtcrypto_clean_affinity(vi, -1);
+ return;
+ }
+
+ i = 0;
+ for_each_online_cpu(cpu) {
+ virtqueue_set_affinity(vi->data_vq[i].vq, cpu);
+ i++;
+ }
+
+ vi->affinity_hint_set = true;
+}
+
+static void virtcrypto_free_queues(struct virtio_crypto *vi)
+{
+ kfree(vi->data_vq);
+}
+
+static int virtcrypto_init_vqs(struct virtio_crypto *vi)
+{
+ int ret;
+
+ /* Allocate send & receive queues */
+ ret = virtcrypto_alloc_queues(vi);
+ if (ret)
+ goto err;
+
+ ret = virtcrypto_find_vqs(vi);
+ if (ret)
+ goto err_free;
+
+ get_online_cpus();
+ virtcrypto_set_affinity(vi);
+ put_online_cpus();
+
+ return 0;
+
+err_free:
+ virtcrypto_free_queues(vi);
+err:
+ return ret;
+}
+
+static void virtcrypto_update_status(struct virtio_crypto *vcrypto)
+{
+ u32 v;
+
+ virtio_cread(vcrypto->vdev, struct virtio_crypto_config, status, &v);
+
+ /* Ignore unknown (future) status bits */
+ v &= VIRTIO_CRYPTO_S_HW_READY;
+
+ if (vcrypto->status == v)
+ return;
+
+ vcrypto->status = v;
+
+ if (vcrypto->status & VIRTIO_CRYPTO_S_HW_READY)
+ pr_info("virtio_crypto: accelerator is ready\n");
+ else
+ pr_info("virtio_crypto: accelerator is not ready\n");
+}
+
+static int virtcrypto_probe(struct virtio_device *vdev)
+{
+ int err = -EFAULT;
+ struct virtio_crypto *vcrypto;
+ u32 max_data_queues = 0, max_cipher_key_len = 0;
+ u32 max_auth_key_len = 0;
+ u64 max_size = 0;
+
+ if (!vdev->config->get) {
+ dev_err(&vdev->dev, "%s failure: config access disabled\n",
+ __func__);
+ return -EINVAL;
+ }
+
+ if (num_possible_nodes() > 1 && dev_to_node(&vdev->dev) < 0) {
+ /*
+ * If the accelerator is connected to a node with no memory
+ * there is no point in using the accelerator since the remote
+ * memory transaction will be very slow.
+ */
+ dev_err(&vdev->dev, "Invalid NUMA configuration.\n");
+ return -EINVAL;
+ }
+
+ virtio_cread(vdev, struct virtio_crypto_config,
+ max_dataqueues, &max_data_queues);
+ if (max_data_queues < 1)
+ max_data_queues = 1;
+
+ dev_info(&vdev->dev, "max_queues: %u\n", max_data_queues);
+
+ virtio_cread(vdev, struct virtio_crypto_config,
+ max_cipher_key_len, &max_cipher_key_len);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ max_auth_key_len, &max_auth_key_len);
+ virtio_cread(vdev, struct virtio_crypto_config,
+ max_size, &max_size);
+
+ vcrypto = kzalloc_node(sizeof(*vcrypto), GFP_KERNEL,
+ dev_to_node(&vdev->dev));
+ if (!vcrypto)
+ return -ENOMEM;
+
+ /* Add virtio crypto device to global table */
+ err = virtcrypto_devmgr_add_dev(vcrypto);
+ if (err) {
+ dev_err(&vdev->dev, "Failed to add new virtio crypto device.\n");
+ goto free;
+ }
+ vcrypto->owner = THIS_MODULE;
+ vcrypto = vdev->priv = vcrypto;
+ vcrypto->vdev = vdev;
+ spin_lock_init(&vcrypto->lock);
+
+ /* Use sigle data queue as default */
+ vcrypto->curr_queue = 1;
+ vcrypto->max_data_queues = max_data_queues;
+ vcrypto->max_cipher_key_len = max_cipher_key_len;
+ vcrypto->max_auth_key_len = max_auth_key_len;
+ vcrypto->max_size = max_size;
+
+ err = virtcrypto_init_vqs(vcrypto);
+ if (err) {
+ dev_err(&vdev->dev, "Failed to initialize vqs.\n");
+ goto free_dev;
+ }
+ virtio_device_ready(vdev);
+
+ virtcrypto_update_status(vcrypto);
+
+ if (vcrypto->status & VIRTIO_CRYPTO_S_HW_READY) {
+ err = virtcrypto_dev_start(vcrypto);
+ if (err) {
+ dev_err(&vdev->dev, "Failed to start virtio crypto device.\n");
+ goto free_start;
+ }
+ }
+
+ return 0;
+
+free_start:
+ virtcrypto_dev_stop(vcrypto);
+free_dev:
+ virtcrypto_devmgr_rm_dev(vcrypto);
+free:
+ kfree(vcrypto);
+ return err;
+}
+
+static void virtcrypto_del_vqs(struct virtio_crypto *vcrypto)
+{
+ struct virtio_device *vdev = vcrypto->vdev;
+
+ virtcrypto_clean_affinity(vcrypto, -1);
+
+ vdev->config->del_vqs(vdev);
+
+ virtcrypto_free_queues(vcrypto);
+}
+
+static void virtcrypto_free_unused_reqs(struct virtio_crypto *vcrypto)
+{
+ struct virtio_crypto_request *vc_req;
+ int i;
+ struct virtqueue *vq;
+
+ for (i = 0; i < vcrypto->max_data_queues; i++) {
+ vq = vcrypto->data_vq[i].vq;
+ while ((vc_req = virtqueue_detach_unused_buf(vq)) != NULL) {
+ kfree(vc_req->req_data);
+ kfree(vc_req->sgs);
+ }
+ }
+}
+
+static void virtcrypto_remove(struct virtio_device *vdev)
+{
+ struct virtio_crypto *vcrypto = vdev->priv;
+
+ dev_info(&vdev->dev, "Start virtcrypto_remove.\n");
+
+ if (virtcrypto_dev_started(vcrypto))
+ virtcrypto_dev_stop(vcrypto);
+ vdev->config->reset(vdev);
+ virtcrypto_free_unused_reqs(vcrypto);
+ virtcrypto_del_vqs(vcrypto);
+ virtcrypto_devmgr_rm_dev(vcrypto);
+ kfree(vcrypto);
+}
+
+static void virtcrypto_config_changed(struct virtio_device *vdev)
+{
+ struct virtio_crypto *vcrypto = vdev->priv;
+
+ virtcrypto_update_status(vcrypto);
+
+ if (vcrypto->status & VIRTIO_CRYPTO_S_HW_READY) {
+ /* Set started status bit for device */
+ vcrypto->status |= VIRTIO_CRYPTO_S_STARTED;
+ }
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int virtcrypto_freeze(struct virtio_device *vdev)
+{
+ struct virtio_crypto *vcrypto = vdev->priv;
+ unsigned long flags;
+
+ virtcrypto_free_unused_reqs(vcrypto);
+ spin_lock_irqsave(&vcrypto->lock, flags);
+ if (virtcrypto_dev_started(vcrypto))
+ virtcrypto_dev_stop(vcrypto);
+ spin_unlock_irqrestore(&vcrypto->lock, flags);
+
+ virtcrypto_del_vqs(vcrypto);
+ return 0;
+}
+
+static int virtcrypto_restore(struct virtio_device *vdev)
+{
+ struct virtio_crypto *vcrypto = vdev->priv;
+ int err;
+
+ err = virtcrypto_init_vqs(vcrypto);
+ if (err)
+ return err;
+
+ virtio_device_ready(vdev);
+ err = virtcrypto_dev_start(vcrypto);
+ if (err) {
+ dev_err(&vdev->dev, "Failed to start virtio crypto device.\n");
+ return -EFAULT;
+ }
+
+ return 0;
+}
+#endif
+
+
+static unsigned int features[] = {
+ /* none */
+};
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_CRYPTO, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_crypto_driver = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .feature_table = features,
+ .feature_table_size = ARRAY_SIZE(features),
+ .id_table = id_table,
+ .probe = virtcrypto_probe,
+ .remove = virtcrypto_remove,
+ .config_changed = virtcrypto_config_changed,
+#ifdef CONFIG_PM_SLEEP
+ .freeze = virtcrypto_freeze,
+ .restore = virtcrypto_restore,
+#endif
+};
+
+module_virtio_driver(virtio_crypto_driver);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("virtio crypto device driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Gonglei <arei.gonglei@huawei.com>");
diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
new file mode 100644
index 0000000..83c077f
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto_algs.c
@@ -0,0 +1,496 @@
+ /* Algorithms supported by virtio crypto device
+ *
+ * Authors: Gonglei <arei.gonglei@huawei.com>
+ *
+ * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/scatterlist.h>
+#include <crypto/algapi.h>
+#include <linux/err.h>
+#include <crypto/scatterwalk.h>
+#include <linux/atomic.h>
+
+#include <uapi/linux/virtio_crypto.h>
+#include "virtio_crypto_common.h"
+
+static DEFINE_MUTEX(algs_lock);
+static unsigned int virtio_crypto_active_devs;
+
+static u64 sg_nents_length(struct scatterlist *sg)
+{
+ u64 total = 0;
+
+ for (total = 0; sg; sg = sg_next(sg))
+ total += sg->length;
+
+ return total;
+}
+
+static int virtio_crypto_alg_validate_key(int key_len, int *alg)
+{
+ switch (key_len) {
+ case AES_KEYSIZE_128:
+ case AES_KEYSIZE_192:
+ case AES_KEYSIZE_256:
+ *alg = VIRTIO_CRYPTO_CIPHER_AES_CBC;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int virtio_crypto_alg_ablkcipher_init_session(
+ struct virtio_crypto_ablkcipher_ctx *ctx,
+ int alg, const uint8_t *key,
+ unsigned int keylen,
+ int encrypt)
+{
+ struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
+ unsigned int tmp;
+ struct virtio_crypto_session_input input;
+ struct virtio_crypto_op_ctrl_req ctrl;
+ struct virtio_crypto *vcrypto = ctx->vcrypto;
+ int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
+ int err;
+ unsigned int num_out = 0, num_in = 0;
+
+ memset(&ctrl, 0, sizeof(ctrl));
+ memset(&input, 0, sizeof(input));
+ /* Pad ctrl header */
+ ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
+ ctrl.header.algo = cpu_to_le32((uint32_t)alg);
+ /* Set the default dataqueue id to 0 */
+ ctrl.header.queue_id = 0;
+
+ input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
+ /* Pad cipher's parameters */
+ ctrl.u.sym_create_session.op_type =
+ cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
+ ctrl.u.sym_create_session.u.cipher.para.algo = ctrl.header.algo;
+ ctrl.u.sym_create_session.u.cipher.para.keylen = cpu_to_le32(keylen);
+ ctrl.u.sym_create_session.u.cipher.para.op = cpu_to_le32(op);
+
+ sg_init_one(&outhdr, &ctrl, sizeof(ctrl));
+ sgs[num_out++] = &outhdr;
+
+ /* Set key */
+ sg_init_one(&key_sg, key, keylen);
+ sgs[num_out++] = &key_sg;
+
+ /* Return status and session id back */
+ sg_init_one(&inhdr, &input, sizeof(input));
+ sgs[num_out + num_in++] = &inhdr;
+
+ err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
+ num_in, vcrypto, GFP_ATOMIC);
+ if (err < 0)
+ return err;
+ virtqueue_kick(vcrypto->ctrl_vq);
+
+ /*
+ * Spin for a response, the kick causes an ioport write, trapping
+ * into the hypervisor, so the request should be handled immediately.
+ */
+ while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
+ !virtqueue_is_broken(vcrypto->ctrl_vq))
+ cpu_relax();
+
+ if (le32_to_cpu(input.status) != VIRTIO_CRYPTO_OK) {
+ pr_err("virtio_crypto: Create session failed status: %u, session_id: 0x%llx\n",
+ input.status, input.session_id);
+ return -EINVAL;
+ }
+
+ spin_lock(&ctx->lock);
+ if (encrypt)
+ ctx->enc_sess_info.session_id = le32_to_cpu(input.session_id);
+ else
+ ctx->dec_sess_info.session_id = le32_to_cpu(input.session_id);
+ spin_unlock(&ctx->lock);
+
+ return 0;
+}
+
+static int virtio_crypto_alg_ablkcipher_close_session(
+ struct virtio_crypto_ablkcipher_ctx *ctx,
+ int encrypt)
+{
+ struct scatterlist outhdr, status_sg, *sgs[2];
+ unsigned int tmp;
+ struct virtio_crypto_destroy_session_req *destroy_session;
+ struct virtio_crypto_op_ctrl_req ctrl;
+ struct virtio_crypto *vcrypto = ctx->vcrypto;
+ int err;
+ unsigned int num_out = 0, num_in = 0;
+ uint8_t status = VIRTIO_CRYPTO_ERR;
+
+ memset(&ctrl, 0, sizeof(ctrl));
+
+ /* Pad ctrl header */
+ ctrl.header.opcode = cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
+ /* Set the default virtqueue id to 0 */
+ ctrl.header.queue_id = 0;
+
+ destroy_session = &ctrl.u.destroy_session;
+
+ if (encrypt)
+ destroy_session->session_id =
+ cpu_to_le64(ctx->enc_sess_info.session_id);
+ else
+ destroy_session->session_id =
+ cpu_to_le64(ctx->dec_sess_info.session_id);
+
+ pr_debug("virtio_crypto: Close session, session_id=0x%llx\n",
+ destroy_session->session_id);
+
+ sg_init_one(&outhdr, &ctrl, sizeof(ctrl));
+ sgs[num_out++] = &outhdr;
+
+ /* Return status and session id back */
+ sg_init_one(&status_sg, &status, sizeof(status));
+ sgs[num_out + num_in++] = &status_sg;
+
+ err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
+ num_in, vcrypto, GFP_ATOMIC);
+ if (err < 0)
+ return err;
+ virtqueue_kick(vcrypto->ctrl_vq);
+
+ while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
+ !virtqueue_is_broken(vcrypto->ctrl_vq))
+ cpu_relax();
+
+ if (status != VIRTIO_CRYPTO_OK) {
+ pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
+ status, destroy_session->session_id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int virtio_crypto_alg_ablkcipher_init_sessions(
+ struct virtio_crypto_ablkcipher_ctx *ctx,
+ const uint8_t *key, unsigned int keylen)
+{
+ int alg;
+ int ret;
+ struct virtio_crypto *vcrypto = ctx->vcrypto;
+
+ if (keylen > vcrypto->max_cipher_key_len) {
+ pr_err("virtio_crypto: the key is too long\n");
+ goto bad_key;
+ }
+
+ if (virtio_crypto_alg_validate_key(keylen, &alg))
+ goto bad_key;
+
+ /* Create encryption session */
+ ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
+ alg, key, keylen, 1);
+ if (ret)
+ return ret;
+ /* Create decryption session */
+ ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
+ alg, key, keylen, 0);
+ if (ret) {
+ virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
+ return ret;
+ }
+ return 0;
+
+bad_key:
+ crypto_tfm_set_flags(ctx->tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
+ return -EINVAL;
+}
+
+/* Note: kernel crypto API realization */
+static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm,
+ const uint8_t *key,
+ unsigned int keylen)
+{
+ struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
+ int ret;
+
+ spin_lock(&ctx->lock);
+
+ if (!ctx->vcrypto) {
+ /* New key */
+ int node = virtio_crypto_get_current_node();
+ struct virtio_crypto *vcrypto =
+ virtcrypto_get_dev_node(node);
+ if (!vcrypto) {
+ vcrypto = virtcrypto_devmgr_get_first();
+ if (!vcrypto) {
+ pr_err("virtio_crypto: Could not find a virtio device in the system");
+ spin_unlock(&ctx->lock);
+ return -ENODEV;
+ }
+ }
+
+ ctx->vcrypto = vcrypto;
+ }
+ spin_unlock(&ctx->lock);
+
+ ret = virtio_crypto_alg_ablkcipher_init_sessions(ctx, key, keylen);
+ if (ret) {
+ virtcrypto_dev_put(ctx->vcrypto);
+ ctx->vcrypto = NULL;
+
+ return ret;
+ }
+
+ return 0;
+}
+
+static int
+__virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req,
+ struct ablkcipher_request *req, __u8 op)
+{
+ struct virtio_crypto_ablkcipher_ctx *ctx = vc_req->ablkcipher_ctx;
+ struct virtio_crypto *vcrypto = ctx->vcrypto;
+ struct virtio_crypto_op_data_req *req_data;
+ int src_nents, dst_nents;
+ int err;
+ unsigned long flags;
+ struct scatterlist outhdr, iv_sg, status_sg, **sgs;
+ int i;
+ u64 dst_len;
+ unsigned int num_out = 0, num_in = 0;
+ int sg_total;
+
+ /* Use the first data virtqueue as default */
+ struct data_queue *data_vq = &vcrypto->data_vq[0];
+
+ src_nents = sg_nents_for_len(req->src, req->nbytes);
+ dst_nents = sg_nents(req->dst);
+
+ pr_debug("virtio_crypto: The number of scatterlist (src_nents: %d, dst_nents: %d)\n",
+ src_nents, dst_nents);
+
+ /* Why 3? outhdr + iv + inhdr */
+ sg_total = src_nents + dst_nents + 3;
+ sgs = kzalloc_node(sg_total * sizeof(*sgs), GFP_ATOMIC,
+ dev_to_node(&vcrypto->vdev->dev));
+ if (!sgs)
+ return -ENOMEM;
+
+ req_data = kzalloc_node(sizeof(*req_data), GFP_ATOMIC,
+ dev_to_node(&vcrypto->vdev->dev));
+ if (!req_data) {
+ kfree(sgs);
+ return -ENOMEM;
+ }
+
+ vc_req->req_data = req_data;
+ vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER;
+ /* Head of operation */
+ if (op) {
+ req_data->header.session_id =
+ cpu_to_le64(ctx->enc_sess_info.session_id);
+ req_data->header.opcode =
+ cpu_to_le32(VIRTIO_CRYPTO_CIPHER_ENCRYPT);
+ } else {
+ req_data->header.session_id =
+ cpu_to_le64(ctx->dec_sess_info.session_id);
+ req_data->header.opcode =
+ cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DECRYPT);
+ }
+ req_data->u.sym_req.op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
+ req_data->u.sym_req.u.cipher.para.iv_len = cpu_to_le32(AES_BLOCK_SIZE);
+ req_data->u.sym_req.u.cipher.para.src_data_len =
+ cpu_to_le32(req->nbytes);
+
+ dst_len = sg_nents_length(req->dst);
+ if (unlikely(dst_len > U32_MAX)) {
+ pr_err("virtio_crypto: The dst_len is beyond U32_MAX\n");
+ err = -EINVAL;
+ goto free;
+ }
+
+ pr_debug("virtio_crypto: src_len: %u, dst_len: %llu\n",
+ req->nbytes, dst_len);
+
+ if (unlikely(req->nbytes + dst_len + AES_BLOCK_SIZE +
+ sizeof(vc_req->status) > vcrypto->max_size)) {
+ pr_err("virtio_crypto: The length is too big\n");
+ err = -EINVAL;
+ goto free;
+ }
+
+ req_data->u.sym_req.u.cipher.para.dst_data_len =
+ cpu_to_le32((uint32_t)dst_len);
+
+ /* Outhdr */
+ sg_init_one(&outhdr, req_data, sizeof(*req_data));
+ sgs[num_out++] = &outhdr;
+
+ /* IV */
+ sg_init_one(&iv_sg, req->info,
+ req_data->u.sym_req.u.cipher.para.iv_len);
+ sgs[num_out++] = &iv_sg;
+
+ /* Source data */
+ for (i = 0; i < src_nents; i++)
+ sgs[num_out++] = &req->src[i];
+
+ /* Destination data */
+ for (i = 0; i < dst_nents; i++)
+ sgs[num_out + num_in++] = &req->dst[i];
+
+ /* Status */
+ sg_init_one(&status_sg, &vc_req->status, sizeof(vc_req->status));
+ sgs[num_out + num_in++] = &status_sg;
+
+ vc_req->sgs = sgs;
+
+ spin_lock_irqsave(&vcrypto->lock, flags);
+ err = virtqueue_add_sgs(data_vq->vq, sgs, num_out,
+ num_in, vc_req, GFP_ATOMIC);
+ spin_unlock_irqrestore(&vcrypto->lock, flags);
+ if (unlikely(err < 0))
+ goto free;
+
+ return 0;
+
+free:
+ kfree(req_data);
+ kfree(sgs);
+ return err;
+}
+
+static int virtio_crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
+{
+ struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
+ struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
+ struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
+ int ret;
+
+ vc_req->ablkcipher_ctx = ctx;
+ vc_req->ablkcipher_req = req;
+ ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, 1);
+ if (ret < 0) {
+ pr_err("virtio_crypto: Encryption failed!\n");
+ return ret;
+ }
+ virtqueue_kick(ctx->vcrypto->data_vq->vq);
+
+ return -EINPROGRESS;
+}
+
+static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
+{
+ struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
+ struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
+ struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
+ int ret;
+
+ vc_req->ablkcipher_ctx = ctx;
+ vc_req->ablkcipher_req = req;
+
+ ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, 0);
+ if (ret < 0) {
+ pr_err("virtio_crypto: Decryption failed!\n");
+ return ret;
+ }
+ virtqueue_kick(ctx->vcrypto->data_vq->vq);
+
+ return -EINPROGRESS;
+}
+
+static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm)
+{
+ struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ spin_lock_init(&ctx->lock);
+ tfm->crt_ablkcipher.reqsize = sizeof(struct virtio_crypto_request);
+ ctx->tfm = tfm;
+
+ return 0;
+}
+
+static void virtio_crypto_ablkcipher_exit(struct crypto_tfm *tfm)
+{
+ struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
+
+ if (!ctx->vcrypto)
+ return;
+
+ virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
+ virtio_crypto_alg_ablkcipher_close_session(ctx, 0);
+ virtcrypto_dev_put(ctx->vcrypto);
+ ctx->vcrypto = NULL;
+}
+
+static struct crypto_alg virtio_crypto_algs[] = { {
+ .cra_name = "cbc(aes)",
+ .cra_driver_name = "virtio_crypto_aes_cbc",
+ .cra_priority = 4001,
+ .cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
+ .cra_blocksize = AES_BLOCK_SIZE,
+ .cra_ctxsize = sizeof(struct virtio_crypto_ablkcipher_ctx),
+ .cra_alignmask = 0,
+ .cra_module = THIS_MODULE,
+ .cra_type = &crypto_ablkcipher_type,
+ .cra_init = virtio_crypto_ablkcipher_init,
+ .cra_exit = virtio_crypto_ablkcipher_exit,
+ .cra_u = {
+ .ablkcipher = {
+ .setkey = virtio_crypto_ablkcipher_setkey,
+ .decrypt = virtio_crypto_ablkcipher_decrypt,
+ .encrypt = virtio_crypto_ablkcipher_encrypt,
+ .min_keysize = AES_MIN_KEY_SIZE,
+ .max_keysize = AES_MAX_KEY_SIZE,
+ .ivsize = AES_BLOCK_SIZE,
+ },
+ },
+} };
+
+int virtio_crypto_algs_register(void)
+{
+ int ret = 0, i;
+
+ mutex_lock(&algs_lock);
+ if (++virtio_crypto_active_devs != 1)
+ goto unlock;
+
+ for (i = 0; i < ARRAY_SIZE(virtio_crypto_algs); i++) {
+ virtio_crypto_algs[i].cra_flags =
+ CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC;
+ }
+
+ ret = crypto_register_algs(virtio_crypto_algs,
+ ARRAY_SIZE(virtio_crypto_algs));
+
+unlock:
+ mutex_unlock(&algs_lock);
+ return ret;
+}
+
+void virtio_crypto_algs_unregister(void)
+{
+ mutex_lock(&algs_lock);
+ if (--virtio_crypto_active_devs != 0)
+ goto unlock;
+
+ crypto_unregister_algs(virtio_crypto_algs,
+ ARRAY_SIZE(virtio_crypto_algs));
+
+unlock:
+ mutex_unlock(&algs_lock);
+}
diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
new file mode 100644
index 0000000..f19e2fb
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto_common.h
@@ -0,0 +1,115 @@
+/* Common header for Virtio crypto device.
+ *
+ * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _VIRITO_CRYPTO_COMMON_H
+#define _VIRITO_CRYPTO_COMMON_H
+
+#include <linux/virtio.h>
+#include <linux/crypto.h>
+#include <linux/spinlock.h>
+#include <crypto/aead.h>
+#include <crypto/aes.h>
+#include <crypto/authenc.h>
+
+
+/* Internal representation of a data virtqueue */
+struct data_queue {
+ /* Virtqueue associated with this send _queue */
+ struct virtqueue *vq;
+
+ /* Name of the tx queue: dataq.$index */
+ char name[32];
+};
+
+struct virtio_crypto {
+ struct virtio_device *vdev;
+ struct virtqueue *ctrl_vq;
+ struct data_queue *data_vq;
+
+ spinlock_t lock;
+
+ /* Maximum of data queues supported by the device */
+ u32 max_data_queues;
+
+ /* Number of queue currently used by the driver */
+ u32 curr_queue;
+
+ /* Maximum length of cipher key */
+ u32 max_cipher_key_len;
+ /* Maximum length of authenticated key */
+ u32 max_auth_key_len;
+ /* Maximum size of per request */
+ u64 max_size;
+
+ unsigned long status;
+ atomic_t ref_count;
+ struct list_head list;
+ struct module *owner;
+ uint8_t dev_id;
+
+ /* Does the affinity hint is set for virtqueues? */
+ bool affinity_hint_set;
+};
+
+struct virtio_crypto_sym_session_info {
+ /* Backend session id, which come from the host side */
+ __u64 session_id;
+};
+
+struct virtio_crypto_ablkcipher_ctx {
+ struct virtio_crypto *vcrypto;
+ struct crypto_tfm *tfm;
+
+ struct virtio_crypto_sym_session_info enc_sess_info;
+ struct virtio_crypto_sym_session_info dec_sess_info;
+
+ /* Protects virtio_crypto_ablkcipher_ctx struct */
+ spinlock_t lock;
+};
+
+struct virtio_crypto_request {
+ /* Cipher or aead */
+ uint32_t type;
+ uint8_t status;
+ struct virtio_crypto_ablkcipher_ctx *ablkcipher_ctx;
+ struct ablkcipher_request *ablkcipher_req;
+ struct virtio_crypto_op_data_req *req_data;
+ struct scatterlist **sgs;
+};
+
+int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev);
+struct list_head *virtcrypto_devmgr_get_head(void);
+void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev);
+struct virtio_crypto *virtcrypto_devmgr_get_first(void);
+int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev);
+int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev);
+void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev);
+int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev);
+struct virtio_crypto *virtcrypto_get_dev_node(int node);
+int virtcrypto_dev_start(struct virtio_crypto *vcrypto);
+void virtcrypto_dev_stop(struct virtio_crypto *vcrypto);
+
+static inline int virtio_crypto_get_current_node(void)
+{
+ return topology_physical_package_id(smp_processor_id());
+}
+
+int virtio_crypto_algs_register(void);
+void virtio_crypto_algs_unregister(void);
+
+#endif /* _VIRITO_CRYPTO_COMMON_H */
diff --git a/drivers/crypto/virtio/virtio_crypto_mgr.c b/drivers/crypto/virtio/virtio_crypto_mgr.c
new file mode 100644
index 0000000..e4b6c6d
--- /dev/null
+++ b/drivers/crypto/virtio/virtio_crypto_mgr.c
@@ -0,0 +1,261 @@
+ /* Management for virtio crypto devices (refer to adf_dev_mgr.c)
+ *
+ * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/mutex.h>
+#include <linux/list.h>
+#include <linux/module.h>
+
+#include <uapi/linux/virtio_crypto.h>
+#include "virtio_crypto_common.h"
+
+static LIST_HEAD(virtio_crypto_table);
+static DEFINE_MUTEX(table_lock);
+static uint32_t num_devices;
+
+#define VIRTIO_CRYPTO_MAX_DEVICES 32
+
+
+/*
+ * virtcrypto_devmgr_add_dev() - Add vcrypto_dev to the acceleration
+ * framework.
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * Function adds virtio crypto device to the global list.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: 0 on success, error code othewise.
+ */
+int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev)
+{
+ struct list_head *itr;
+
+ if (num_devices == VIRTIO_CRYPTO_MAX_DEVICES) {
+ pr_info("virtio_crypto: only support up to %d devices\n",
+ VIRTIO_CRYPTO_MAX_DEVICES);
+ return -EFAULT;
+ }
+
+ mutex_lock(&table_lock);
+ list_for_each(itr, &virtio_crypto_table) {
+ struct virtio_crypto *ptr =
+ list_entry(itr, struct virtio_crypto, list);
+
+ if (ptr == vcrypto_dev) {
+ mutex_unlock(&table_lock);
+ return -EEXIST;
+ }
+ }
+ atomic_set(&vcrypto_dev->ref_count, 0);
+ list_add_tail(&vcrypto_dev->list, &virtio_crypto_table);
+ vcrypto_dev->dev_id = num_devices++;
+ mutex_unlock(&table_lock);
+ return 0;
+}
+
+struct list_head *virtcrypto_devmgr_get_head(void)
+{
+ return &virtio_crypto_table;
+}
+
+/*
+ * virtcrypto_devmgr_rm_dev() - Remove vcrypto_dev from the acceleration
+ * framework.
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * Function removes virtio crypto device from the acceleration framework.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: void
+ */
+void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev)
+{
+ mutex_lock(&table_lock);
+ list_del(&vcrypto_dev->list);
+ num_devices--;
+ mutex_unlock(&table_lock);
+}
+
+/*
+ * virtcrypto_devmgr_get_first()
+ *
+ * Function returns the first virtio crypto device from the acceleration
+ * framework.
+ *
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: pointer to vcrypto_dev or NULL if not found.
+ */
+struct virtio_crypto *virtcrypto_devmgr_get_first(void)
+{
+ struct virtio_crypto *dev = NULL;
+
+ if (!list_empty(&virtio_crypto_table))
+ dev = list_first_entry(&virtio_crypto_table,
+ struct virtio_crypto,
+ list);
+ return dev;
+}
+
+/*
+ * virtcrypto_dev_in_use() - Check whether vcrypto_dev is currently in use
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: 1 when device is in use, 0 otherwise.
+ */
+int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev)
+{
+ return atomic_read(&vcrypto_dev->ref_count) != 0;
+}
+
+/*
+ * virtcrypto_dev_get() - Increment vcrypto_dev reference count
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * Increment the vcrypto_dev refcount and if this is the first time
+ * incrementing it during this period the vcrypto_dev is in use,
+ * increment the module refcount too.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: 0 when successful, EFAULT when fail to bump module refcount
+ */
+int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev)
+{
+ if (atomic_add_return(1, &vcrypto_dev->ref_count) == 1)
+ if (!try_module_get(vcrypto_dev->owner))
+ return -EFAULT;
+ return 0;
+}
+
+/*
+ * virtcrypto_dev_put() - Decrement vcrypto_dev reference count
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * Decrement the vcrypto_dev refcount and if this is the last time
+ * decrementing it during this period the vcrypto_dev is in use,
+ * decrement the module refcount too.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: void
+ */
+void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev)
+{
+ if (atomic_sub_return(1, &vcrypto_dev->ref_count) == 0)
+ module_put(vcrypto_dev->owner);
+}
+
+/*
+ * virtcrypto_dev_started() - Check whether device has started
+ * @vcrypto_dev: Pointer to virtio crypto device.
+ *
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: 1 when the device has started, 0 otherwise
+ */
+int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev)
+{
+ return (vcrypto_dev->status & VIRTIO_CRYPTO_S_STARTED);
+}
+
+/*
+ * virtcrypto_get_dev_node() - Get vcrypto_dev on the node.
+ * @node: Node id the driver works.
+ *
+ * Function returns the virtio crypto device used fewest on the node.
+ *
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: pointer to vcrypto_dev or NULL if not found.
+ */
+struct virtio_crypto *virtcrypto_get_dev_node(int node)
+{
+ struct virtio_crypto *vcrypto_dev = NULL, *tmp_dev;
+ unsigned long best = ~0;
+ unsigned long ctr;
+
+ list_for_each_entry(tmp_dev, virtcrypto_devmgr_get_head(), list) {
+
+ if ((node == dev_to_node(&tmp_dev->vdev->dev) ||
+ dev_to_node(&tmp_dev->vdev->dev) < 0) &&
+ virtcrypto_dev_started(tmp_dev)) {
+ ctr = atomic_read(&tmp_dev->ref_count);
+ if (best > ctr) {
+ vcrypto_dev = tmp_dev;
+ best = ctr;
+ }
+ }
+ }
+
+ if (!vcrypto_dev) {
+ pr_info("virtio_crypto: Could not find a device on node %d\n",
+ node);
+ /* Get any started device */
+ list_for_each_entry(tmp_dev,
+ virtcrypto_devmgr_get_head(), list) {
+ if (virtcrypto_dev_started(tmp_dev)) {
+ vcrypto_dev = tmp_dev;
+ break;
+ }
+ }
+ }
+
+ if (!vcrypto_dev)
+ return NULL;
+
+ virtcrypto_dev_get(vcrypto_dev);
+ return vcrypto_dev;
+}
+
+/*
+ * virtcrypto_dev_start() - Start virtio crypto device
+ * @vcrypto: Pointer to virtio crypto device.
+ *
+ * Function notifies all the registered services that the virtio crypto device
+ * is ready to be used.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: 0 on success, EFAULT when fail to register algorithms
+ */
+int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
+{
+ if (virtio_crypto_algs_register()) {
+ pr_err("virtio_crypto: Failed to register crypto algs\n");
+ return -EFAULT;
+ }
+
+ vcrypto->status |= VIRTIO_CRYPTO_S_STARTED;
+ return 0;
+}
+
+/*
+ * virtcrypto_dev_stop() - Stop virtio crypto device
+ * @vcrypto: Pointer to virtio crypto device.
+ *
+ * Function notifies all the registered services that the virtio crypto device
+ * is ready to be used.
+ * To be used by virtio crypto device specific drivers.
+ *
+ * Return: void
+ */
+void virtcrypto_dev_stop(struct virtio_crypto *vcrypto)
+{
+ virtio_crypto_algs_unregister();
+
+ vcrypto->status &= ~VIRTIO_CRYPTO_S_STARTED;
+}
\ No newline at end of file
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index cd2be1c..4bdb84c 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -460,6 +460,7 @@ header-y += virtio_rng.h
header-y += virtio_scsi.h
header-y += virtio_types.h
header-y += virtio_vsock.h
+header-y += virtio_crypto.h
header-y += vm_sockets.h
header-y += vt.h
header-y += vtpm_proxy.h
diff --git a/include/uapi/linux/virtio_crypto.h b/include/uapi/linux/virtio_crypto.h
new file mode 100644
index 0000000..f0933af
--- /dev/null
+++ b/include/uapi/linux/virtio_crypto.h
@@ -0,0 +1,436 @@
+#ifndef _VIRTIO_CRYPTO_H
+#define _VIRTIO_CRYPTO_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <linux/types.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+
+
+#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
+#define VIRTIO_CRYPTO_SERVICE_HASH 1
+#define VIRTIO_CRYPTO_SERVICE_MAC 2
+#define VIRTIO_CRYPTO_SERVICE_AEAD 3
+
+#define VIRTIO_CRYPTO_OPCODE(service, op) (((service) << 8) | (op))
+
+struct virtio_crypto_ctrl_header {
+#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
+#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
+#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
+#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
+#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
+#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
+#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
+#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
+ __virtio32 opcode;
+ __virtio32 algo;
+ __virtio32 flag;
+ /* data virtqueue id */
+ __virtio32 queue_id;
+};
+
+struct virtio_crypto_cipher_session_para {
+#define VIRTIO_CRYPTO_NO_CIPHER 0
+#define VIRTIO_CRYPTO_CIPHER_ARC4 1
+#define VIRTIO_CRYPTO_CIPHER_AES_ECB 2
+#define VIRTIO_CRYPTO_CIPHER_AES_CBC 3
+#define VIRTIO_CRYPTO_CIPHER_AES_CTR 4
+#define VIRTIO_CRYPTO_CIPHER_DES_ECB 5
+#define VIRTIO_CRYPTO_CIPHER_DES_CBC 6
+#define VIRTIO_CRYPTO_CIPHER_3DES_ECB 7
+#define VIRTIO_CRYPTO_CIPHER_3DES_CBC 8
+#define VIRTIO_CRYPTO_CIPHER_3DES_CTR 9
+#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8 10
+#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2 11
+#define VIRTIO_CRYPTO_CIPHER_AES_F8 12
+#define VIRTIO_CRYPTO_CIPHER_AES_XTS 13
+#define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3 14
+ __virtio32 algo;
+ /* length of key */
+ __virtio32 keylen;
+
+#define VIRTIO_CRYPTO_OP_ENCRYPT 1
+#define VIRTIO_CRYPTO_OP_DECRYPT 2
+ /* encrypt or decrypt */
+ __virtio32 op;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_session_input {
+ /* Device-writable part */
+ __virtio64 session_id;
+ __virtio32 status;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_cipher_session_req {
+ struct virtio_crypto_cipher_session_para para;
+};
+
+struct virtio_crypto_hash_session_para {
+#define VIRTIO_CRYPTO_NO_HASH 0
+#define VIRTIO_CRYPTO_HASH_MD5 1
+#define VIRTIO_CRYPTO_HASH_SHA1 2
+#define VIRTIO_CRYPTO_HASH_SHA_224 3
+#define VIRTIO_CRYPTO_HASH_SHA_256 4
+#define VIRTIO_CRYPTO_HASH_SHA_384 5
+#define VIRTIO_CRYPTO_HASH_SHA_512 6
+#define VIRTIO_CRYPTO_HASH_SHA3_224 7
+#define VIRTIO_CRYPTO_HASH_SHA3_256 8
+#define VIRTIO_CRYPTO_HASH_SHA3_384 9
+#define VIRTIO_CRYPTO_HASH_SHA3_512 10
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128 11
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256 12
+ __virtio32 algo;
+ /* hash result length */
+ __virtio32 hash_result_len;
+};
+
+struct virtio_crypto_hash_create_session_req {
+ struct virtio_crypto_hash_session_para para;
+};
+
+struct virtio_crypto_mac_session_para {
+#define VIRTIO_CRYPTO_NO_MAC 0
+#define VIRTIO_CRYPTO_MAC_HMAC_MD5 1
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA1 2
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224 3
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256 4
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384 5
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512 6
+#define VIRTIO_CRYPTO_MAC_CMAC_3DES 25
+#define VIRTIO_CRYPTO_MAC_CMAC_AES 26
+#define VIRTIO_CRYPTO_MAC_KASUMI_F9 27
+#define VIRTIO_CRYPTO_MAC_SNOW3G_UIA2 28
+#define VIRTIO_CRYPTO_MAC_GMAC_AES 41
+#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH 42
+#define VIRTIO_CRYPTO_MAC_CBCMAC_AES 49
+#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9 50
+#define VIRTIO_CRYPTO_MAC_XCBC_AES 53
+ __virtio32 algo;
+ /* hash result length */
+ __virtio32 hash_result_len;
+ /* length of authenticated key */
+ __virtio32 auth_key_len;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_mac_create_session_req {
+ struct virtio_crypto_mac_session_para para;
+};
+
+struct virtio_crypto_aead_session_para {
+#define VIRTIO_CRYPTO_NO_AEAD 0
+#define VIRTIO_CRYPTO_AEAD_GCM 1
+#define VIRTIO_CRYPTO_AEAD_CCM 2
+#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305 3
+ __virtio32 algo;
+ /* length of key */
+ __virtio32 key_len;
+ /* hash result length */
+ __virtio32 hash_result_len;
+ /* length of the additional authenticated data (AAD) in bytes */
+ __virtio32 aad_len;
+ /* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
+ __virtio32 op;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_aead_create_session_req {
+ struct virtio_crypto_aead_session_para para;
+};
+
+struct virtio_crypto_alg_chain_session_para {
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER 1
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH 2
+ __virtio32 alg_chain_order;
+/* Plain hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN 1
+/* Authenticated hash (mac) */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH 2
+/* Nested hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED 3
+ __virtio32 hash_mode;
+ struct virtio_crypto_cipher_session_para cipher_param;
+ union {
+ struct virtio_crypto_hash_session_para hash_param;
+ struct virtio_crypto_mac_session_para mac_param;
+ } u;
+ /* length of the additional authenticated data (AAD) in bytes */
+ __virtio32 aad_len;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_alg_chain_session_req {
+ struct virtio_crypto_alg_chain_session_para para;
+};
+
+struct virtio_crypto_sym_create_session_req {
+ union {
+ struct virtio_crypto_cipher_session_req cipher;
+ struct virtio_crypto_alg_chain_session_req chain;
+ } u;
+
+ /* Device-readable part */
+
+/* No operation */
+#define VIRTIO_CRYPTO_SYM_OP_NONE 0
+/* Cipher only operation on the data */
+#define VIRTIO_CRYPTO_SYM_OP_CIPHER 1
+/*
+ * Chain any cipher with any hash or mac operation. The order
+ * depends on the value of alg_chain_order param
+ */
+#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING 2
+ __virtio32 op_type;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_destroy_session_req {
+ /* Device-readable part */
+ __virtio64 session_id;
+};
+
+/* The request of the control viritqueue's packet */
+struct virtio_crypto_op_ctrl_req {
+ struct virtio_crypto_ctrl_header header;
+
+ union {
+ struct virtio_crypto_sym_create_session_req
+ sym_create_session;
+ struct virtio_crypto_hash_create_session_req
+ hash_create_session;
+ struct virtio_crypto_mac_create_session_req
+ mac_create_session;
+ struct virtio_crypto_aead_create_session_req
+ aead_create_session;
+ struct virtio_crypto_destroy_session_req
+ destroy_session;
+ } u;
+};
+
+struct virtio_crypto_op_header {
+#define VIRTIO_CRYPTO_CIPHER_ENCRYPT \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
+#define VIRTIO_CRYPTO_CIPHER_DECRYPT \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
+#define VIRTIO_CRYPTO_HASH \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
+#define VIRTIO_CRYPTO_MAC \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
+#define VIRTIO_CRYPTO_AEAD_ENCRYPT \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
+#define VIRTIO_CRYPTO_AEAD_DECRYPT \
+ VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
+ __virtio32 opcode;
+ /* algo should be service-specific algorithms */
+ __virtio32 algo;
+ /* session_id should be service-specific algorithms */
+ __virtio64 session_id;
+ /* control flag to control the request */
+ __virtio32 flag;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_cipher_para {
+ /*
+ * Byte Length of valid IV/Counter
+ *
+ * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
+ * SNOW3G in UEA2 mode, this is the length of the IV (which
+ * must be the same as the block length of the cipher).
+ * For block ciphers in CTR mode, this is the length of the counter
+ * (which must be the same as the block length of the cipher).
+ * For AES-XTS, this is the 128bit tweak, i, from IEEE Std 1619-2007.
+ *
+ * The IV/Counter will be updated after every partial cryptographic
+ * operation.
+ */
+ __virtio32 iv_len;
+ /* length of source data */
+ __virtio32 src_data_len;
+ /* length of dst data */
+ __virtio32 dst_data_len;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_hash_para {
+ /* length of source data */
+ __virtio32 src_data_len;
+ /* hash result length */
+ __virtio32 hash_result_len;
+};
+
+struct virtio_crypto_mac_para {
+ struct virtio_crypto_hash_para hash;
+};
+
+struct virtio_crypto_aead_para {
+ /*
+ * Byte Length of valid IV data pointed to by the below iv_addr
+ * parameter.
+ *
+ * For GCM mode, this is either 12 (for 96-bit IVs) or 16, in which
+ * case iv_addr points to J0.
+ * For CCM mode, this is the length of the nonce, which can be in the
+ * range 7 to 13 inclusive.
+ */
+ __virtio32 iv_len;
+ /* length of additional auth data */
+ __virtio32 aad_len;
+ /* length of source data */
+ __virtio32 src_data_len;
+ /* length of dst data */
+ __virtio32 dst_data_len;
+};
+
+struct virtio_crypto_cipher_data_req {
+ /* Device-readable part */
+ struct virtio_crypto_cipher_para para;
+};
+
+struct virtio_crypto_hash_data_req {
+ /* Device-readable part */
+ struct virtio_crypto_hash_para para;
+};
+
+struct virtio_crypto_mac_data_req {
+ /* Device-readable part */
+ struct virtio_crypto_mac_para para;
+};
+
+struct virtio_crypto_alg_chain_data_para {
+ __virtio32 iv_len;
+ /* Length of source data */
+ __virtio32 src_data_len;
+ /* Length of destination data */
+ __virtio32 dst_data_len;
+ /* Starting point for cipher processing in source data */
+ __virtio32 cipher_start_src_offset;
+ /* Length of the source data that the cipher will be computed on */
+ __virtio32 len_to_cipher;
+ /* Starting point for hash processing in source data */
+ __virtio32 hash_start_src_offset;
+ /* Length of the source data that the hash will be computed on */
+ __virtio32 len_to_hash;
+ /* Length of the additional auth data */
+ __virtio32 aad_len;
+ /* Length of the hash result */
+ __virtio32 hash_result_len;
+ __virtio32 reserved;
+};
+
+struct virtio_crypto_alg_chain_data_req {
+ /* Device-readable part */
+ struct virtio_crypto_alg_chain_data_para para;
+};
+
+struct virtio_crypto_sym_data_req {
+ union {
+ struct virtio_crypto_cipher_data_req cipher;
+ struct virtio_crypto_alg_chain_data_req chain;
+ } u;
+
+ /* See above VIRTIO_CRYPTO_SYM_OP_* */
+ __virtio32 op_type;
+ __virtio32 padding;
+};
+
+struct virtio_crypto_aead_data_req {
+ /* Device-readable part */
+ struct virtio_crypto_aead_para para;
+};
+
+/* The request of the data viritqueue's packet */
+struct virtio_crypto_op_data_req {
+ struct virtio_crypto_op_header header;
+
+ union {
+ struct virtio_crypto_sym_data_req sym_req;
+ struct virtio_crypto_hash_data_req hash_req;
+ struct virtio_crypto_mac_data_req mac_req;
+ struct virtio_crypto_aead_data_req aead_req;
+ } u;
+};
+
+#define VIRTIO_CRYPTO_OK 0
+#define VIRTIO_CRYPTO_ERR 1
+#define VIRTIO_CRYPTO_BADMSG 2
+#define VIRTIO_CRYPTO_NOTSUPP 3
+#define VIRTIO_CRYPTO_INVSESS 4 /* Invaild session id */
+
+/* The accelerator hardware is ready */
+#define VIRTIO_CRYPTO_S_HW_READY (1 << 0)
+#define VIRTIO_CRYPTO_S_STARTED (1 << 1)
+
+struct virtio_crypto_config {
+ /* See VIRTIO_CRYPTO_OP_* above */
+ __virtio32 status;
+
+ /*
+ * Maximum number of data queue legal values are between 1 and 0x8000
+ */
+ __virtio32 max_dataqueues;
+
+ /*
+ * Specifies the services mask which the devcie support,
+ * see VIRTIO_CRYPTO_SERVICE_* above
+ */
+ __virtio32 crypto_services;
+
+ /* Detailed algorithms mask */
+ __virtio32 cipher_algo_l;
+ __virtio32 cipher_algo_h;
+ __virtio32 hash_algo;
+ __virtio32 mac_algo_l;
+ __virtio32 mac_algo_h;
+ __virtio32 aead_algo;
+ /* Maximum length of cipher key */
+ __virtio32 max_cipher_key_len;
+ /* Maximum length of authenticated key */
+ __virtio32 max_auth_key_len;
+ __virtio32 reserve;
+ /* Maximum size of each crypto request's content */
+ __virtio64 max_size;
+};
+
+struct virtio_crypto_inhdr {
+ /* See VIRTIO_CRYPTO_* above */
+ u8 status;
+};
+#endif
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 3228d58..6d5c3b2 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -42,5 +42,6 @@
#define VIRTIO_ID_GPU 16 /* virtio GPU */
#define VIRTIO_ID_INPUT 18 /* virtio input */
#define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
+#define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
#endif /* _LINUX_VIRTIO_IDS_H */
--
1.8.3.1
^ permalink raw reply related
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Jiri Slaby @ 2016-11-11 16:28 UTC (permalink / raw)
To: Gerd Hoffmann, Michael S. Tsirkin
Cc: David Airlie, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <1478678517.2078.12.camel@redhat.com>
On 11/09/2016, 09:01 AM, Gerd Hoffmann wrote:
> On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote:
>> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
>>> Hi,
>>>
>>> I can relatively easily reproduce this bug:
>
> How?
Run dmesg -w in the qemu window (virtio_gpu) to see a lot of output.
Run pps [1] without exit(0); on e.g. serial console.
Wait a bit. The lot of output causes the BUG.
[1] https://github.com/jirislaby/collected_sources/blob/master/pps.c
>>> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
>
>> The following might be helpful for debugging - if kernel still will
>> not stop panicing, we are looking at some kind
>> of memory corruption.
>
> Looking carefully through the code I think it isn't impossible to
> trigger this, but you need for that:
>
> (1) command queue full (quite possible),
> (2) cursor queue full too (unlikely), and
> (3) multiple threads trying to submit commands and waiting for free
> space in the command queue (possible with virgl enabled).
I use -vga virtio with no -display option, so no virtgl, I suppose:
[drm] virgl 3d acceleration not available
> Do things improve if you allocate some extra bufs?
>
> int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
> {
> struct virtio_gpu_vbuffer *vbuf;
> - int i, size, count = 0;
> + int i, size, count = 16;
This seems to help.
thanks,
--
js
suse labs
^ permalink raw reply
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Jiri Slaby @ 2016-11-11 14:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Airlie, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <20161108223153-mutt-send-email-mst@kernel.org>
On 11/08/2016, 09:37 PM, Michael S. Tsirkin wrote:
> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> The following might be helpful for debugging - if kernel still will
> not stop panicing, we are looking at some kind
> of memory corruption.
>
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 5a0f8a7..d5e1e72 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf;
>
> spin_lock(&vgdev->free_vbufs_lock);
> - BUG_ON(list_empty(&vgdev->free_vbufs));
> + WARN_ON(list_empty(&vgdev->free_vbufs));
> + if (list_empty(&vgdev->free_vbufs)) {
> + spin_unlock(&vgdev->free_vbufs_lock);
> + return ERR_PTR(-EINVAL);
> + }
Yeah, I already tried that, but it dies immediately after that:
WARNING: '1' is true!
------------[ cut here ]------------
WARNING: CPU: 2 PID: 5019 at
/home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130
virtio_gpu_get_vbuf+0x415/0x6a0
Modules linked in:
CPU: 2 PID: 5019 Comm: kworker/2:3 Not tainted 4.9.0-rc2-next-20161028+ #33
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
Workqueue: events drm_fb_helper_dirty_work
Call Trace:
dump_stack+0xcd/0x134
? _atomic_dec_and_lock+0xcc/0xcc
? vprintk_default+0x1f/0x30
? printk+0x99/0xb5
__warn+0x19e/0x1d0
warn_slowpath_null+0x1d/0x20
virtio_gpu_get_vbuf+0x415/0x6a0
? lock_pin_lock+0x4a0/0x4a0
? virtio_gpu_cmd_capset_cb+0x460/0x460
? debug_check_no_locks_freed+0x350/0x350
virtio_gpu_cmd_resource_flush+0x8d/0x2d0
? virtio_gpu_cmd_set_scanout+0x310/0x310
virtio_gpu_surface_dirty+0x364/0x930
? mark_held_locks+0xff/0x290
? virtio_gpufb_create+0xab0/0xab0
? _raw_spin_unlock_irqrestore+0x53/0x70
? trace_hardirqs_on_caller+0x46c/0x6b0
virtio_gpu_framebuffer_surface_dirty+0x14/0x20
drm_fb_helper_dirty_work+0x27a/0x400
? drm_fb_helper_is_bound+0x300/0x300
process_one_work+0x834/0x1c90
? process_one_work+0x7a5/0x1c90
? pwq_dec_nr_in_flight+0x3a0/0x3a0
? worker_thread+0x1b2/0x1540
worker_thread+0x650/0x1540
? process_one_work+0x1c90/0x1c90
? process_one_work+0x1c90/0x1c90
kthread+0x206/0x310
? kthread_create_on_node+0xa0/0xa0
? trace_hardirqs_on+0xd/0x10
? kthread_create_on_node+0xa0/0xa0
? kthread_create_on_node+0xa0/0xa0
ret_from_fork+0x2a/0x40
---[ end trace c723c98d382423f4 ]---
BUG: unable to handle kernel paging request at fffffc0000000000
IP: check_memory_region+0x7f/0x1a0
PGD 0
Oops: 0000 [#1] PREEMPT SMP KASAN
Modules linked in:
CPU: 2 PID: 5019 Comm: kworker/2:3 Tainted: G W
4.9.0-rc2-next-20161028+ #33
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
Workqueue: events drm_fb_helper_dirty_work
task: ffff8800455f4980 task.stack: ffff88001fd78000
RIP: 0010:check_memory_region+0x7f/0x1a0
RSP: 0018:ffff88001fd7f938 EFLAGS: 00010282
RAX: fffffc0000000000 RBX: dffffc0000000001 RCX: ffffffff8260afb3
RDX: 0000000000000001 RSI: 0000000000000030 RDI: fffffffffffffff4
RBP: ffff88001fd7f948 R08: fffffc0000000001 R09: dffffc0000000004
R10: 0000000000000023 R11: dffffc0000000005 R12: 0000000000000030
R13: 0000000000000000 R14: 0000000000000050 R15: 0000000000000001
FS: 0000000000000000(0000) GS:ffff88007dd00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: fffffc0000000000 CR3: 00000000773a0000 CR4: 00000000000006e0
Call Trace:
Code: 83 fb 10 7f 3f 4d 85 db 74 34 48 bb 01 00 00 00 00 fc ff df 49 01
c3 49 01 d8 80 38 00 75 13 4d 39 c3 4c 89 c0 74 17 49 83 c0 01 <41> 80
78 ff 00 74 ed 49 89 c0 4d 85 c0 0f 85 8f 00 00 00 5b 41
RIP: check_memory_region+0x7f/0x1a0 RSP: ffff88001fd7f938
CR2: fffffc0000000000
thanks,
--
js
suse labs
^ permalink raw reply
* [PATCH] vhost/scsi: Remove unused but set variable
From: Tobias Klauser @ 2016-11-11 13:27 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, kvm, virtualization
Remove the unused but set variable se_tpg in vhost_scsi_nexus_cb() to
fix the following GCC warning when building with 'W=1':
drivers/vhost/scsi.c:1752:26: warning: variable ‘se_tpg’ set but not used
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/vhost/scsi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 6e29d053843d..e2be447752c2 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1749,7 +1749,6 @@ static int vhost_scsi_nexus_cb(struct se_portal_group *se_tpg,
static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
const char *name)
{
- struct se_portal_group *se_tpg;
struct vhost_scsi_nexus *tv_nexus;
mutex_lock(&tpg->tv_tpg_mutex);
@@ -1758,7 +1757,6 @@ static int vhost_scsi_make_nexus(struct vhost_scsi_tpg *tpg,
pr_debug("tpg->tpg_nexus already exists\n");
return -EEXIST;
}
- se_tpg = &tpg->se_tpg;
tv_nexus = kzalloc(sizeof(struct vhost_scsi_nexus), GFP_KERNEL);
if (!tv_nexus) {
--
2.11.0.rc0.7.gbe5a750
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* [PATCH] vhost/vsock: Remove unused but set variable
From: Tobias Klauser @ 2016-11-11 13:26 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
Remove the unused but set variable vq in vhost_transport_send_pkt() to
fix the following GCC warning when building with 'W=1':
drivers/vhost/vsock.c:198:26: warning: variable ‘vq’ set but not used
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
drivers/vhost/vsock.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index e3b30ea9ece5..9c3c68b9a49e 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -195,7 +195,6 @@ static int
vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
{
struct vhost_vsock *vsock;
- struct vhost_virtqueue *vq;
int len = pkt->len;
/* Find the vhost_vsock according to guest context id */
@@ -205,8 +204,6 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
return -ENODEV;
}
- vq = &vsock->vqs[VSOCK_VQ_RX];
-
if (pkt->reply)
atomic_inc(&vsock->queued_replies);
--
2.11.0.rc0.7.gbe5a750
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* Re: [PATCH 2/3] qemu: Implement virtio-pstore device
From: Michael S. Tsirkin @ 2016-11-10 22:50 UTC (permalink / raw)
To: Namhyung Kim
Cc: virtio-dev, Tony Luck, Daniel P . Berrange, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20160916100547.GC2474@danjae.aot.lge.com>
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:
> > > 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 <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: Daniel P. Berrange <berrange@redhat.com>
> > > Cc: kvm@vger.kernel.org
> > > Cc: qemu-devel@nongnu.org
> > > Cc: virtualization@lists.linux-foundation.org
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > hw/virtio/Makefile.objs | 2 +-
> > > hw/virtio/virtio-pci.c | 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(&vps->vdev);
> > > + Error *err = NULL;
> > > +
> > > + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> > > + object_property_set_bool(OBJECT(vdev), true, "realized", &err);
> > > + if (err) {
> > > + error_propagate(errp, err);
> > > + return;
> > > + }
> > > +}
> > > +
> > > +static void virtio_pstore_pci_class_init(ObjectClass *klass, void *data)
> > > +{
> > > + DeviceClass *dc = DEVICE_CLASS(klass);
> > > + VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> > > + PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> > > +
> > > + k->realize = virtio_pstore_pci_realize;
> > > + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > > +
> > > + pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> > > + pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_PSTORE;
> > > + pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
> > > + pcidev_k->class_id = PCI_CLASS_OTHERS;
> > > +}
> > > +
> > > +static void virtio_pstore_pci_instance_init(Object *obj)
> > > +{
> > > + VirtIOPstorePCI *dev = VIRTIO_PSTORE_PCI(obj);
> > > +
> > > + virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> > > + TYPE_VIRTIO_PSTORE);
> > > + object_property_add_alias(obj, "directory", OBJECT(&dev->vdev),
> > > + "directory", &error_abort);
> > > + object_property_add_alias(obj, "bufsize", OBJECT(&dev->vdev),
> > > + "bufsize", &error_abort);
> > > +}
> > > +
> > > +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(&vhost_scsi_pci_info);
> > > #endif
> > > + type_register_static(&virtio_pstore_pci_info);
> > > }
> > >
> > > type_init(virtio_pci_register_types)
> > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> > > index 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/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 */
> > >
> > > @@ -324,6 +326,18 @@ struct VirtIOGPUPCI {
> > > VirtIOGPU vdev;
> > > };
> > >
> > > +/*
> > > + * virtio-pstore-pci: This extends VirtioPCIProxy.
> > > + */
> > > +#define TYPE_VIRTIO_PSTORE_PCI "virtio-pstore-pci"
> > > +#define VIRTIO_PSTORE_PCI(obj) \
> > > + OBJECT_CHECK(VirtIOPstorePCI, (obj), TYPE_VIRTIO_PSTORE_PCI)
> > > +
> > > +struct VirtIOPstorePCI {
> > > + VirtIOPCIProxy parent_obj;
> > > + VirtIOPstore vdev;
> > > +};
> > > +
> > > /* Virtio ABI version, if we increment this, we break the guest driver. */
> > > #define VIRTIO_PCI_ABI_VERSION 0
> > >
> > > diff --git a/hw/virtio/virtio-pstore.c b/hw/virtio/virtio-pstore.c
> > > new file mode 100644
> > > index 0000000..b8fb4be
> > > --- /dev/null
> > > +++ b/hw/virtio/virtio-pstore.c
> > > @@ -0,0 +1,699 @@
> > > +/*
> > > + * Virtio Pstore Device
> > > + *
> > > + * Copyright (C) 2016 LG Electronics
> > > + *
> > > + * Authors:
> > > + * Namhyung Kim <namhyung@gmail.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > > + * See the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#include <stdio.h>
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "qemu/iov.h"
> > > +#include "qemu-common.h"
> > > +#include "qemu/cutils.h"
> > > +#include "qemu/error-report.h"
> > > +#include "sysemu/kvm.h"
> > > +#include "qapi/visitor.h"
> > > +#include "qapi-event.h"
> > > +#include "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 */
> >
> > 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.
> >
> >
> > > + "dmesg-", /* VIRTIO_PSTORE_TYPE_DMESG */
> >
> > use named initializers for this instead of comments.
>
> Ok.
>
> >
> > > +};
> > > +
> > > +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, &info->id);
> >
> > What if this fails?
>
> Hmm.. will add a check for return value then.
>
> >
> > > +
> > > + 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;
> > This does not work properly if there are multiple instances
> > of it. Pls move everything into device state.
>
> Kernel (currently?) allows only a single pstore device active. But I
> think it'd be better to move them into device state anyway.
>
> >
> > > +
> > > +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, &id_a);
> > > + qemu_strtoull((*b)->d_name + prefix_len, NULL, 0, &id_b);
> > > +
> > > + return id_a - id_b;
> > > +}
> > > +
> > > +static int rotate_pstore_file(VirtIOPstore *s, unsigned short type)
> > > +{
> > > + int ret = 0;
> > > + int i, num;
> > > + char *filename;
> > > + struct dirent **files;
> > > +
> > > + if (type >= ARRAY_SIZE(virtio_pstore_file_prefix)) {
> > > + type = VIRTIO_PSTORE_TYPE_UNKNOWN;
> > > + }
> > > +
> > > + prefix_idx = type;
> > > + prefix_len = strlen(virtio_pstore_file_prefix[type]);
> > > + prefix_count = 1; /* only scan current type */
> > > +
> > > + /* delete the oldest file in the same type */
> > > + num = scandir(s->directory, &files, filter_pstore, sort_pstore);
> > > + if (num < 0)
> > > + return num;
> > > + if (num < (int)s->file_max)
> > > + goto out;
> > > +
> > > + filename = g_strdup_printf("%s/%s", s->directory, files[0]->d_name);
> > > + if (filename == NULL) {
> > > + ret = -1;
> > > + goto out;
> > > + }
> > > +
> > > + ret = unlink(filename);
> > > +
> > > +out:
> > > + for (i = 0; i < num; i++) {
> > > + g_free(files[i]);
> > > + }
> > > + g_free(files);
> > > +
> > > + return ret;
> > > +}
> >
> > Pls prefix everything with virtio_pstore or another
> > unique prefix. also below.
>
> Ok.
>
> >
> > > +
> > > +static ssize_t virtio_pstore_do_open(VirtIOPstore *s)
> > > +{
> > > + /* scan all pstore files */
> > > + prefix_idx = 0;
> > > + prefix_count = ARRAY_SIZE(virtio_pstore_file_prefix);
> > > +
> > > + s->file_idx = 0;
> > > + s->num_file = scandir(s->directory, &s->files, filter_pstore, alphasort);
> > > +
> > > + return s->num_file >= 0 ? 0 : -1;
> > > +}
> > > +
> > > +static ssize_t virtio_pstore_do_close(VirtIOPstore *s)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < s->num_file; i++) {
> > > + g_free(s->files[i]);
> > > + }
> > > + g_free(s->files);
> > > + s->files = NULL;
> > > +
> > > + s->num_file = 0;
> > > + return 0;
> > > +}
> > > +
> > > +static ssize_t virtio_pstore_do_erase(VirtIOPstore *s,
> > > + struct virtio_pstore_req *req)
> > > +{
> > > + char *filename;
> > > + int ret;
> > > +
> > > + filename = virtio_pstore_to_filename(s, req);
> > > + if (filename == NULL)
> > > + return -1;
> >
> > this can't happen.
>
> Why? The virtio_pstore_to_filename() calls g_strdup_printf(). That
> means I don't need to worry about the memory allocation failure?
>
> >
> > also this is a coding style violation.
>
> Oh, I missed the add {}, will fix.
>
> >
> > > +
> > > + ret = unlink(filename);
> > > +
> > > + g_free(filename);
> > > + return ret;
> > > +}
> > > +
> > > +struct pstore_read_arg {
> > > + VirtIOPstore *vps;
> > > + VirtQueueElement *elem;
> > > + struct virtio_pstore_fileinfo info;
> > > + QIOChannel *ioc;
> > > +};
> > > +
> > > +static gboolean pstore_async_read_fn(QIOChannel *ioc, GIOCondition condition,
> > > + gpointer data)
> > > +{
> > > + struct pstore_read_arg *rarg = data;
> > > + struct virtio_pstore_fileinfo *info = &rarg->info;
> > > + VirtIOPstore *vps = rarg->vps;
> > > + VirtQueueElement *elem = rarg->elem;
> > > + struct virtio_pstore_res res;
> > > + size_t offset = sizeof(res) + sizeof(*info);
> > > + struct iovec *sg = elem->in_sg;
> > > + unsigned int sg_num = elem->in_num;
> > > + Error *err = NULL;
> > > + ssize_t len;
> > > + int ret;
> > > +
> > > + /* skip res and fileinfo */
> > > + iov_discard_front(&sg, &sg_num, sizeof(res) + sizeof(*info));
> > > +
> > > + len = qio_channel_readv(rarg->ioc, sg, sg_num, &err);
> > > + if (len < 0) {
> > > + if (errno == EAGAIN) {
> > > + len = 0;
> > > + }
> > > + ret = -1;
> > > + } else {
> > > + info->len = cpu_to_le32(len);
> > > + ret = 0;
> > > + }
> > > +
> > > + res.cmd = cpu_to_le16(VIRTIO_PSTORE_CMD_READ);
> > > + res.type = cpu_to_le16(VIRTIO_PSTORE_TYPE_UNKNOWN);
> > > + res.ret = cpu_to_le32(ret);
> > > +
> > > + /* now copy res and fileinfo */
> > > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > > + iov_from_buf(elem->in_sg, elem->in_num, sizeof(res), info, sizeof(*info));
> > > +
> > > + len += offset;
> > > + virtqueue_push(vps->rvq, elem, len);
> > > + virtio_notify(VIRTIO_DEVICE(vps), vps->rvq);
> > > +
> > > + return G_SOURCE_REMOVE;
> > > +}
> > > +
> > > +static void free_rarg_fn(gpointer data)
> > > +{
> > > + struct pstore_read_arg *rarg = data;
> > > +
> > > + qio_channel_close(rarg->ioc, NULL);
> > > +
> > > + g_free(rarg->elem);
> > > + g_free(rarg);
> > > +}
> > > +
> > > +static ssize_t virtio_pstore_do_read(VirtIOPstore *s, VirtQueueElement *elem)
> > > +{
> > > + char *filename = NULL;
> > > + int fd, idx;
> > > + struct stat stbuf;
> > > + struct pstore_read_arg *rarg = NULL;
> > > + Error *err = NULL;
> > > + int ret = -1;
> > > +
> > > + if (s->file_idx >= s->num_file) {
> > > + return 0;
> > > + }
> > > +
> > > + rarg = g_malloc(sizeof(*rarg));
> > > + if (rarg == NULL) {
> > > + return -1;
> > > + }
> > > +
> > > + idx = s->file_idx++;
> > > + filename = virtio_pstore_from_filename(s, s->files[idx]->d_name,
> > > + &rarg->info);
> > > + if (filename == NULL) {
> > > + goto out;
> > > + }
> > > +
> > > + fd = open(filename, O_RDONLY);
> > > + if (fd < 0) {
> > > + error_report("cannot open %s", filename);
> > > + goto out;
> > > + }
> >
> > I see open here but close nowhere. Does this leak fds?
>
> I guess so. But this is changed to use qio_channel_file API in v5 and
> I hope doing it right.
>
> >
> > > +
> > > + if (fstat(fd, &stbuf) < 0) {
> >
> > So we can stat, but can we e.g. read?
>
> It's just being a paranoid, I think it should succeed, no?
>
> >
> >
> > > + goto out;
> > > + }
> > > +
> > > + rarg->vps = s;
> > > + rarg->elem = elem;
> > > + rarg->info.id = cpu_to_le64(rarg->info.id);
> > > + rarg->info.type = cpu_to_le16(rarg->info.type);
> > > + rarg->info.flags = cpu_to_le32(rarg->info.flags);
> > > + rarg->info.time_sec = cpu_to_le64(stbuf.st_ctim.tv_sec);
> >
> > Is this seconds since epoch?
> > Why ctim specifically?
> > Pls add comments.
>
> I think it doesn't matter either ctim or mtim.
>
> >
> > > + rarg->info.time_nsec = cpu_to_le32(stbuf.st_ctim.tv_nsec);
> >
> > Not all hosts support nanosecond precision.
> > Do we need some way to tell guest what's reliable?
>
> In fact I'm not sure how much it affects users. The pstore messages
> are occasional and AFAIK pstore keeps it only for users' information.
>
> >
> > Unless you limit this to linux host, you should care about things
> > like this (in man fstat)
> >
> > Since kernel 2.5.48, the stat structure supports nanosecond
> > resolution for the three file timestamp fields. The nanosecond compo‐
> > nents of each timestamp are available via names of the form
> > st_atim.tv_nsec if the _BSD_SOURCE or _SVID_SOURCE feature test macro
> > is defined. Nanosecond timestamps are nowadays standardized,
> > starting with POSIX.1-2008, and, starting with version 2.12, glibc also
> > exposes the nanosecond component names if _POSIX_C_SOURCE is defined
> > with the value 200809L or greater, or _XOPEN_SOURCE is defined with
> > the value 700 or greater. If none of the aforementioned macros are
> > defined, then the nanosecond values are exposed with names of the form
> > st_atimensec.
>
> Thanks for the info.
>
> >
> >
> >
> >
> > > +
> > > + rarg->ioc = qio_channel_new_fd(fd, &err);
> > > + if (err) {
> > > + error_reportf_err(err, "cannot create io channel: ");
> > > + goto out;
> > > + }
> > > +
> > > + qio_channel_set_blocking(rarg->ioc, false, &err);
> > > + qio_channel_add_watch(rarg->ioc, G_IO_IN, pstore_async_read_fn, rarg,
> > > + free_rarg_fn);
> > > + g_free(filename);
> > > + return 1;
> > > +
> > > +out:
> > > + g_free(filename);
> > > + g_free(rarg);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +struct pstore_write_arg {
> > > + VirtIOPstore *vps;
> > > + VirtQueueElement *elem;
> > > + struct virtio_pstore_req *req;
> > > + QIOChannel *ioc;
> > > +};
> > > +
> > > +static gboolean pstore_async_write_fn(QIOChannel *ioc, GIOCondition condition,
> > > + gpointer data)
> > > +{
> > > + struct pstore_write_arg *warg = data;
> > > + VirtIOPstore *vps = warg->vps;
> > > + VirtQueueElement *elem = warg->elem;
> > > + struct iovec *sg = elem->out_sg;
> > > + unsigned int sg_num = elem->out_num;
> > > + struct virtio_pstore_res res;
> > > + Error *err = NULL;
> > > + ssize_t len;
> > > + int ret;
> > > +
> > > + /* we already consumed the req */
> > > + iov_discard_front(&sg, &sg_num, sizeof(*warg->req));
> > > +
> > > + len = qio_channel_writev(warg->ioc, sg, sg_num, &err);
> > > + if (len < 0) {
> > > + ret = -1;
> > > + } else {
> > > + ret = 0;
> > > + }
> >
> > This can discard part of the data written.
> > Don't we care?
>
> Doing partial write is better than failing out. But if it's
> meaningful to add a retry loop, I'd like to do so.
>
> >
> > > +
> > > + res.cmd = cpu_to_le16(VIRTIO_PSTORE_CMD_WRITE);
> > > + res.type = warg->req->type;
> > > + res.ret = cpu_to_le32(ret);
> > > +
> > > + /* tell the result to guest */
> > > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > > +
> > > + virtqueue_push(vps->wvq, elem, sizeof(res));
> > > + virtio_notify(VIRTIO_DEVICE(vps), vps->wvq);
> > > +
> > > + return G_SOURCE_REMOVE;
> > > +}
> > > +
> > > +static void free_warg_fn(gpointer data)
> > > +{
> > > + struct pstore_write_arg *warg = data;
> > > +
> > > + qio_channel_close(warg->ioc, NULL);
> > > +
> > > + g_free(warg->elem);
> > > + g_free(warg);
> > > +}
> > > +
> > > +static ssize_t virtio_pstore_do_write(VirtIOPstore *s, VirtQueueElement *elem,
> > > + struct virtio_pstore_req *req)
> > > +{
> > > + unsigned short type = le16_to_cpu(req->type);
> > > + char *filename = NULL;
> > > + int fd;
> > > + int flags = O_WRONLY | O_CREAT | O_TRUNC;
> > > + struct pstore_write_arg *warg = NULL;
> > > + Error *err = NULL;
> > > + int ret = -1;
> > > +
> > > + /* do not keep same type of files more than 'file-max' */
> > > + rotate_pstore_file(s, type);
> >
> > If you don't care about failures, should this function
> > return a value? How about reporting it to the user?
>
> Did you mean when it failed to delete the oldest file (FYI it's not
> really 'rotate'). Hmm.. will add error check and report.
>
> >
> >
> > > +
> > > + filename = virtio_pstore_to_filename(s, req);
> > > + if (filename == NULL) {
> > > + return -1;
> > > + }
> >
> > this can't happen
> >
> > > +
> > > + warg = g_malloc(sizeof(*warg));
> > > + if (warg == NULL) {
> > > + goto out;
> > > + }
> > > +
> > > + fd = open(filename, flags, 0644);
> > > + if (fd < 0) {
> > > + error_report("cannot open %s", filename);
> > > + ret = fd;
> > > + goto out;
> > > + }
> > > +
> > > + warg->vps = s;
> > > + warg->elem = elem;
> > > + warg->req = req;
> > > +
> > > + warg->ioc = qio_channel_new_fd(fd, &err);
> > > + if (err) {
> > > + error_reportf_err(err, "cannot create io channel: ");
> > > + goto out;
> > > + }
> > > +
> > > + qio_channel_set_blocking(warg->ioc, false, &err);
> > > + qio_channel_add_watch(warg->ioc, G_IO_OUT, pstore_async_write_fn, warg,
> > > + free_warg_fn);
> > > + g_free(filename);
> > > + return 1;
> > > +
> > > +out:
> > > + g_free(filename);
> > > + g_free(warg);
> > > + return ret;
> > > +}
> > > +
> > > +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);
> > > + }
> > > +
> > > + if (elem->out_num > 2 || elem->in_num > 3) {
> > > + error_report("invalid number of input/output buffer");
> > > + exit(1);
> > > + }
> > > +
> > > + len = iov_to_buf(elem->out_sg, elem->out_num, 0, &req, sizeof(req));
> > > + if (len != (ssize_t)sizeof(req)) {
> > > + error_report("invalid request size: %ld", (long)len);
> > > + exit(1);
> > > + }
> > > + res.cmd = req.cmd;
> > > + res.type = req.type;
> > > +
> > > + switch (le16_to_cpu(req.cmd)) {
> > > + case VIRTIO_PSTORE_CMD_OPEN:
> > > + ret = virtio_pstore_do_open(s);
> > > + break;
> > > + case VIRTIO_PSTORE_CMD_CLOSE:
> > > + ret = virtio_pstore_do_close(s);
> > > + break;
> > > + case VIRTIO_PSTORE_CMD_ERASE:
> > > + ret = virtio_pstore_do_erase(s, &req);
> > > + 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, &req);
> > > + if (ret == 1) {
> > > + /* async channel io */
> > > + continue;
> > > + }
> > > + break;
> > > + default:
> > > + ret = -1;
> > > + break;
> > > + }
> > > +
> > > + res.ret = ret;
> > > +
> > > + iov_from_buf(elem->in_sg, elem->in_num, 0, &res, sizeof(res));
> > > + virtqueue_push(vq, elem, sizeof(res) + len);
> > > +
> > > + virtio_notify(vdev, vq);
> > > + g_free(elem);
> > > +
> > > + if (ret < 0) {
> > > + return;
> >
> > what does this do?
>
> If it failed on any processing, reports it to the kernel and stop
> processing later commands. The kernel won't send same kind of command
> later.
>
> >
> > > + }
> > > + }
> > > +}
> > > +
> > > +static void virtio_pstore_device_realize(DeviceState *dev, Error **errp)
> > > +{
> > > + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > > + VirtIOPstore *s = VIRTIO_PSTORE(dev);
> > > +
> > > + virtio_init(vdev, "virtio-pstore", VIRTIO_ID_PSTORE,
> > > + sizeof(struct virtio_pstore_config));
> > > +
> > > + s->id = 1;
> > > +
> > > + if (!s->bufsize)
> > > + s->bufsize = PSTORE_DEFAULT_BUFSIZE;
> > > + if (!s->file_max)
> > > + s->file_max = PSTORE_DEFAULT_FILE_MAX;
> > > +
> > > + s->rvq = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> > > + s->wvq = virtio_add_queue(vdev, 128, virtio_pstore_handle_io);
> > > +}
> > > +
> > > +static void virtio_pstore_device_unrealize(DeviceState *dev, Error **errp)
> > > +{
> > > + VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> > > +
> > > + virtio_cleanup(vdev);
> > > +}
> > > +
> > > +static void virtio_pstore_get_config(VirtIODevice *vdev, uint8_t *config_data)
> > > +{
> > > + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> > > + struct virtio_pstore_config config;
> >
> > Add {} here - you want all fields initialized
> > if you add them, to avoid leaking them to guest.
>
> Ok.
>
> >
> > > +
> > > + config.bufsize = cpu_to_le32(dev->bufsize);
> > > +
> > > + memcpy(config_data, &config, sizeof(struct virtio_pstore_config));
> > > +}
> > > +
> > > +static void virtio_pstore_set_config(VirtIODevice *vdev,
> > > + const uint8_t *config_data)
> > > +{
> > > + VirtIOPstore *dev = VIRTIO_PSTORE(vdev);
> > > + struct virtio_pstore_config config;
> > > +
> > > + memcpy(&config, config_data, sizeof(struct virtio_pstore_config));
> > > +
> > > + dev->bufsize = le32_to_cpu(config.bufsize);
> > > +}
> > > +
> > > +static uint64_t get_features(VirtIODevice *vdev, uint64_t f, Error **errp)
> > > +{
> > > + return f;
> > > +}
> > > +
> > > +static void pstore_get_directory(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > +
> > > + visit_type_str(v, name, &s->directory, errp);
> > > +}
> > > +
> > > +static void pstore_set_directory(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > + Error *local_err = NULL;
> > > + char *value;
> > > +
> > > + visit_type_str(v, name, &value, &local_err);
> > > + if (local_err) {
> > > + error_propagate(errp, local_err);
> > > + return;
> > > + }
> > > +
> > > + g_free(s->directory);
> > > + s->directory = value;
> > > +}
> > > +
> > > +static void pstore_release_directory(Object *obj, const char *name,
> > > + void *opaque)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > +
> > > + g_free(s->directory);
> > > + s->directory = NULL;
> > > +}
> > > +
> > > +static void pstore_get_bufsize(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > + uint64_t value = s->bufsize;
> > > +
> > > + visit_type_size(v, name, &value, errp);
> > > +}
> > > +
> > > +static void pstore_set_bufsize(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > + Error *error = NULL;
> > > + uint64_t value;
> > > +
> > > + visit_type_size(v, name, &value, &error);
> > > + if (error) {
> > > + error_propagate(errp, error);
> > > + return;
> > > + }
> > > +
> > > + if (value < 4096) {
> > > + error_setg(&error, "Warning: too small buffer size: %"PRIu64, value);
> > > + error_propagate(errp, error);
> > > + return;
> > > + }
> > > +
> > > + s->bufsize = value;
> > > +}
> > > +
> > > +static void pstore_get_file_max(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > + int64_t value = s->file_max;
> > > +
> > > + visit_type_int(v, name, &value, errp);
> > > +}
> > > +
> > > +static void pstore_set_file_max(Object *obj, Visitor *v,
> > > + const char *name, void *opaque,
> > > + Error **errp)
> > > +{
> > > + VirtIOPstore *s = opaque;
> > > + Error *error = NULL;
> > > + int64_t value;
> > > +
> > > + visit_type_int(v, name, &value, &error);
> > > + if (error) {
> > > + error_propagate(errp, error);
> > > + return;
> > > + }
> > > +
> > > + s->file_max = value;
> > > +}
> >
> > Do you need dynamic properties? There are easier ways
> > to define an int property. Same for others.
>
> It was due to my insufficient knowledge about the qemu code base. I
> don't think it needs to be dynamic.
>
> Thanks,
> Namhyung
>
> >
> > > +
> > > +static Property virtio_pstore_properties[] = {
> > > + DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void virtio_pstore_instance_init(Object *obj)
> > > +{
> > > + VirtIOPstore *s = VIRTIO_PSTORE(obj);
> > > +
> > > + object_property_add(obj, "directory", "str",
> > > + pstore_get_directory, pstore_set_directory,
> > > + pstore_release_directory, s, NULL);
> > > + object_property_add(obj, "bufsize", "size",
> > > + pstore_get_bufsize, pstore_set_bufsize, NULL, s, NULL);
> > > + object_property_add(obj, "file-max", "int",
> > > + pstore_get_file_max, pstore_set_file_max, NULL, s, NULL);
> > > +}
> > > +
> > > +static void virtio_pstore_class_init(ObjectClass *klass, void *data)
> > > +{
> > > + DeviceClass *dc = DEVICE_CLASS(klass);
> > > + VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
> > > +
> > > + dc->props = virtio_pstore_properties;
> > > + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > > + vdc->realize = virtio_pstore_device_realize;
> > > + vdc->unrealize = virtio_pstore_device_unrealize;
> > > + vdc->get_config = virtio_pstore_get_config;
> > > + vdc->set_config = virtio_pstore_set_config;
> > > + vdc->get_features = get_features;
> > > +}
> > > +
> > > +static const TypeInfo virtio_pstore_info = {
> > > + .name = TYPE_VIRTIO_PSTORE,
> > > + .parent = TYPE_VIRTIO_DEVICE,
> > > + .instance_size = sizeof(VirtIOPstore),
> > > + .instance_init = virtio_pstore_instance_init,
> > > + .class_init = virtio_pstore_class_init,
> > > +};
> > > +
> > > +static void virtio_register_types(void)
> > > +{
> > > + type_register_static(&virtio_pstore_info);
> > > +}
> > > +
> > > +type_init(virtio_register_types)
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index 929ec2f..b31774a 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -79,6 +79,7 @@
> > > #define PCI_DEVICE_ID_VIRTIO_SCSI 0x1004
> > > #define PCI_DEVICE_ID_VIRTIO_RNG 0x1005
> > > #define PCI_DEVICE_ID_VIRTIO_9P 0x1009
> > > +#define PCI_DEVICE_ID_VIRTIO_PSTORE 0x100a
> > >
> > > #define PCI_VENDOR_ID_REDHAT 0x1b36
> > > #define PCI_DEVICE_ID_REDHAT_BRIDGE 0x0001
> > > diff --git a/include/hw/virtio/virtio-pstore.h b/include/hw/virtio/virtio-pstore.h
> > > new file mode 100644
> > > index 0000000..85b1828
> > > --- /dev/null
> > > +++ b/include/hw/virtio/virtio-pstore.h
> > > @@ -0,0 +1,36 @@
> > > +/*
> > > + * Virtio Pstore Support
> > > + *
> > > + * Authors:
> > > + * Namhyung Kim <namhyung@gmail.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2. See
> > > + * the COPYING file in the top-level directory.
> > > + *
> > > + */
> > > +
> > > +#ifndef _QEMU_VIRTIO_PSTORE_H
> > > +#define _QEMU_VIRTIO_PSTORE_H
> > > +
> > > +#include "standard-headers/linux/virtio_pstore.h"
> > > +#include "hw/virtio/virtio.h"
> > > +#include "hw/pci/pci.h"
> > > +
> > > +#define TYPE_VIRTIO_PSTORE "virtio-pstore-device"
> > > +#define VIRTIO_PSTORE(obj) \
> > > + OBJECT_CHECK(VirtIOPstore, (obj), TYPE_VIRTIO_PSTORE)
> > > +
> > > +typedef struct VirtIOPstore {
> > > + VirtIODevice parent_obj;
> > > + VirtQueue *rvq;
> > > + VirtQueue *wvq;
> > > + char *directory;
> > > + int file_idx;
> > > + int num_file;
> > > + struct dirent **files;
> > > + uint64_t id;
> > > + uint64_t bufsize;
> > > + uint64_t file_max;
> > > +} VirtIOPstore;
> > > +
> > > +#endif
> > > diff --git a/include/standard-headers/linux/virtio_ids.h b/include/standard-headers/linux/virtio_ids.h
> > > index 77925f5..c72a9ab 100644
> > > --- a/include/standard-headers/linux/virtio_ids.h
> > > +++ b/include/standard-headers/linux/virtio_ids.h
> > > @@ -41,5 +41,6 @@
> > > #define VIRTIO_ID_CAIF 12 /* Virtio caif */
> > > #define VIRTIO_ID_GPU 16 /* virtio GPU */
> > > #define VIRTIO_ID_INPUT 18 /* virtio input */
> > > +#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
> > >
> > > #endif /* _LINUX_VIRTIO_IDS_H */
> > > diff --git a/include/standard-headers/linux/virtio_pstore.h b/include/standard-headers/linux/virtio_pstore.h
> > > new file mode 100644
> > > index 0000000..2f91839
> > > --- /dev/null
> > > +++ b/include/standard-headers/linux/virtio_pstore.h
> > > @@ -0,0 +1,76 @@
> > > +#ifndef _LINUX_VIRTIO_PSTORE_H
> > > +#define _LINUX_VIRTIO_PSTORE_H
> > > +/* This header is BSD licensed so anyone can use the definitions to implement
> > > + * compatible drivers/servers.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or without
> > > + * modification, are permitted provided that the following conditions
> > > + * are met:
> > > + * 1. Redistributions of source code must retain the above copyright
> > > + * notice, this list of conditions and the following disclaimer.
> > > + * 2. Redistributions in binary form must reproduce the above copyright
> > > + * notice, this list of conditions and the following disclaimer in the
> > > + * documentation and/or other materials provided with the distribution.
> > > + * 3. Neither the name of IBM nor the names of its contributors
> > > + * may be used to endorse or promote products derived from this software
> > > + * without specific prior written permission.
> > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> > > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> > > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> > > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> > > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> > > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> > > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > > + * SUCH DAMAGE. */
> > > +#include "standard-headers/linux/types.h"
> > > +#include "standard-headers/linux/virtio_types.h"
> > > +#include "standard-headers/linux/virtio_ids.h"
> > > +#include "standard-headers/linux/virtio_config.h"
> > > +
> > > +#define VIRTIO_PSTORE_CMD_NULL 0
> > > +#define VIRTIO_PSTORE_CMD_OPEN 1
> > > +#define VIRTIO_PSTORE_CMD_READ 2
> > > +#define VIRTIO_PSTORE_CMD_WRITE 3
> > > +#define VIRTIO_PSTORE_CMD_ERASE 4
> > > +#define VIRTIO_PSTORE_CMD_CLOSE 5
> > > +
> > > +#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
> > > +#define VIRTIO_PSTORE_TYPE_DMESG 1
> > > +
> > > +#define VIRTIO_PSTORE_FL_COMPRESSED 1
> > > +
> > > +struct virtio_pstore_req {
> > > + __virtio16 cmd;
> > > + __virtio16 type;
> > > + __virtio32 flags;
> > > + __virtio64 id;
> > > + __virtio32 count;
> > > + __virtio32 reserved;
> > > +};
> > > +
> > > +struct virtio_pstore_res {
> > > + __virtio16 cmd;
> > > + __virtio16 type;
> > > + __virtio32 ret;
> > > +};
> > > +
> > > +struct virtio_pstore_fileinfo {
> > > + __virtio64 id;
> > > + __virtio32 count;
> > > + __virtio16 type;
> > > + __virtio16 unused;
> > > + __virtio32 flags;
> > > + __virtio32 len;
> > > + __virtio64 time_sec;
> > > + __virtio32 time_nsec;
> > > + __virtio32 reserved;
> > > +};
> > > +
> > > +struct virtio_pstore_config {
> > > + __virtio32 bufsize;
> > > +};
> > > +
> > > +#endif /* _LINUX_VIRTIO_PSTORE_H */
> > > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > > index e19617f..e1df5a9 100644
> > > --- a/qdev-monitor.c
> > > +++ b/qdev-monitor.c
> > > @@ -73,6 +73,7 @@ static const QDevAlias qdev_alias_table[] = {
> > > { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> > > { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
> > > { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> > > + { "virtio-pstore-pci", "virtio-pstore" },
> > > { }
> > > };
> > >
> > > --
> > > 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Last call - WorldCIST'2017 - Deadline: November 27
From: ML @ 2016-11-10 19:20 UTC (permalink / raw)
To: virtualization
[-- Attachment #1: Type: text/plain, Size: 7984 bytes --]
* Best papers published in SCI/SSCI-indexed journals
** Proceedings by Springer, indexed in ISI, Scopus, DBLP, EI-Compendex, etc.
----------------------------------------------------------------------------
WorldCIST'17 - 5th World Conference on Information Systems and Technologies
Porto Santo Island, Madeira, Portugal
11th-13th of April 2017
http://www.worldcist.org/
---------------------------------------------------------------------
SCOPE
The WorldCist'17 - 5th World Conference on Information Systems and Technologies, to be held at Porto Santo Island, Madeira, Portugal, 11 - 13 April 2017, is a global forum for researchers and practitioners to present and discuss the most recent innovations, trends, results, experiences and concerns in the several perspectives of Information Systems and Technologies.
We are pleased to invite you to submit your papers to WorldCist'17 (http://www.worldcist.org/). All submissions will be reviewed on the basis of relevance, originality, importance and clarity.
THEMES
Submitted papers should be related with one or more of the main themes proposed for the Conference:
A) Information and Knowledge Management (IKM);
B) Organizational Models and Information Systems (OMIS);
C) Software and Systems Modeling (SSM);
D) Software Systems, Architectures, Applications and Tools (SSAAT);
E) Multimedia Systems and Applications (MSA);
F) Computer Networks, Mobility and Pervasive Systems (CNMPS);
G) Intelligent and Decision Support Systems (IDSS);
H) Big Data Analytics and Applications (BDAA);
I) Human-Computer Interaction (HCI);
J) Ethics, Computers and Security (ECS)
K) Health Informatics (HIS);
L) Information Technologies in Education (ITE);
M) Information Technologies in Radiocommunications (ITR).
TYPES of SUBMISSIONS AND DECISIONS
Four types of papers can be submitted:
- Full paper: Finished or consolidated R&D works, to be included in one of the Conference themes. These papers are assigned a 10-page limit.
- Short paper: Ongoing works with relevant preliminary results, open to discussion. These papers are assigned a 7-page limit.
- Poster paper: Initial work with relevant ideas, open to discussion. These papers are assigned to a 4-page limit.
- Company paper: Companies' papers that show practical experience, R & D, tools, etc., focused on some topics of the conference. These papers are assigned to a 4-page limit.
Submitted papers must comply with the format of Advances in Intelligent Systems and Computing Series (see Instructions for Authors at Springer Website or download a DOC example) be written in English, must not have been published before, not be under review for any other conference or publication and not include any information leading to the authors’ identification. Therefore, the authors’ names, affiliations and bibliographic references should not be included in the version for evaluation by the Program Committee. This information should only be included in the camera-ready version, saved in Word or Latex format and also in PDF format. These files must be accompanied by the Consent to Publication form filled out, in a ZIP file, and uploaded at the conference management system.
All papers will be subjected to a “double-blind review” by at least two members of the Program Committee.
Based on Program Committee evaluation, a paper can be rejected or accepted by the Conference Chairs. In the later case, it can be accepted as the type originally submitted or as another type. Thus, full papers can be accepted as short papers or poster papers only. Similarly, short papers can be accepted as poster papers only. In these cases, the authors will be allowed to maintain the original number of pages in the camera-ready version.
The authors of accepted poster papers must also build and print a poster to be exhibited during the Conference. This poster must follow an A1 or A2 vertical format. The Conference can includes Work Sessions where these posters are presented and orally discussed, with a 5 minute limit per poster.
The authors of accepted full papers will have 15 minutes to present their work in a Conference Work Session; approximately 5 minutes of discussion will follow each presentation. The authors of accepted short papers and company papers will have 11 minutes to present their work in a Conference Work Session; approximately 4 minutes of discussion will follow each presentation.
PUBLICATION & INDEXING
To ensure that a full paper, short paper, poster paper or company paper is published in the Proceedings, at least one of the authors must be fully registered by the 8th of January 2017, and the paper must comply with the suggested layout and page-limit. Additionally, all recommended changes must be addressed by the authors before they submit the camera-ready version.
No more than one paper per registration will be published in the Conference Proceedings. An extra fee must be paid for publication of additional papers, with a maximum of one additional paper per registration. One registration permits only the participation of one author in the conference.
Full and short papers will be published in Proceedings by Springer, in Advances in Intelligent Systems and Computing Series. Poster and company papers will be published by AISTI.
Published full and short papers will be submitted for indexation by ISI, EI-Compendex, SCOPUS, DBLP and Google Scholar, among others, and will be available in the SpringerLink Digital Library.
The authors of the best selected papers will be invited to extend them for publication in international journals indexed by ISI/SCI, SCOPUS and DBLP, among others, such as:
- International Journal of Neural Systems (IF: 6.085 / Q1)
- Integrated Computer-Aided Engineering (IF: 4.981 / Q1)
- International Journal of Information Management (IF: 2.692 / Q1)
- Telematics & Informatics (IF: 2.261 / Q1) - Special Issue on "Disruption of Higher Education in the 21st Century due to ICTs"
- Electronic Commerce Research and Applications (IF: 2.139 / Q1)
- Computers, Environment and Urban Systems (IF: 2.092 / Q1)
- Data Mining and Knowledge Discovery (IF: 1.759 / Q1)
- Journal of Medical Systems (IF: 2.213 / Q2)
- Journal of Business Research (IF: 2.129 / Q2) - Special Issue on "Strategic Knowledge Management in the Digital Age"
- Pervasive and Mobile Computing (IF: 1.719 / Q2)
- Knowledge and Information Systems (IF: 1.702 / Q2)
- Journal of Grid Computing (IF: 1.561 / Q2) - Special Issue on "Big Data"
- Cluster Computing (IF:1.514 / Q2) - Special Issue on "Advanced Machine Learning in Parallel and Distributed Knowledge Discovery"
- International Journal of Critical Infrastructure Protection (IF: 1.351 / Q2)
- Expert Systems - Journal of Knowledge Engineering (IF: 0.947 / Q3)
- Concurrency and Computation: Practice and Experience (IF: 0.942 / Q3)
- Ethics and Information Technology (IF: 0.739 / Q3)
- Annals of Telecommunications (IF: 0.722 / Q3)
- Engineering Computations (IF: 0.691 / Q3)
- Advances in Complex Systems (IF: 0.461 / Q3)
- Computing and Informatics (IF: 0.504 / Q4)
- AI Communications (IF: 0.364 / Q4)
- Journal of Hospitality and Tourism Technology (SR: 0.672 / Q2)
- Transforming Government: People, Process and Policy (SR: 0.642 / Q2)
- TEM Journal - Technology, Education, Management, Informatics (ISI - Emerging Sources Citation Index)
- Computer Methods in Biomechanics and Biomedical Engineering - Imaging & Visualization (ISI - Emerging Sources Citation Index)
- Journal of Information Systems Engineering & Management
IMPORTANT DATES
Paper Submission: November 27, 2016
Notification of Acceptance: December 25, 20156
Payment of Registration, to ensure the inclusion of an accepted paper in the conference proceedings: January 8, 2017.
Camera-ready Submission: January 8, 2017
-
Website of the WorldCIST'17
http://www.worldcist.org/
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
From: Michael S. Tsirkin @ 2016-11-10 16:39 UTC (permalink / raw)
To: Namhyung Kim
Cc: virtio-dev, Tony Luck, Kees Cook, kvm,
Radim Krčmář, Anton Vorontsov, LKML,
Steven Rostedt, qemu-devel, Minchan Kim, Anthony Liguori,
Colin Cross, Paolo Bonzini, virtualization, Ingo Molnar
In-Reply-To: <20160820080744.10344-2-namhyung@kernel.org>
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.
> It uses
> two virtqueues - one for (sync) read and another for (async) write.
> Since it cannot wait for write finished, it supports up to 128
> concurrent IO. The buffer size is configurable now.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: qemu-devel@nongnu.org
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> drivers/virtio/Kconfig | 10 +
> drivers/virtio/Makefile | 1 +
> drivers/virtio/virtio_pstore.c | 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 000000000000..0a63c7db4278
> --- /dev/null
> +++ b/drivers/virtio/virtio_pstore.c
> @@ -0,0 +1,417 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pstore.h>
> +#include <linux/virtio.h>
> +#include <linux/virtio_config.h>
> +#include <uapi/linux/virtio_ids.h>
> +#include <uapi/linux/virtio_pstore.h>
> +
> +#define VIRT_PSTORE_ORDER 2
> +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
> +#define VIRT_PSTORE_NR_REQ 128
> +
> +struct virtio_pstore {
> + struct virtio_device *vdev;
> + struct virtqueue *vq[2];
I'd add named fields instead of an array here, vq[0]
vq[1] all over the place is hard to read.
> + 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
let's avoid macros for now pls. In fact, I would just open-code this
in to_virtio_type below. We can always change our minds later if
lots of types are added.
> +
> +
single emoty line pls
> +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);
This assigns u16 to __virtio type, sparse will warn
if you enable endian-ness checks.
Pls fix that and generally, please make sure this is
clean from sparse warnings.
> +}
> +
> +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16 type)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(type_table); i++) {
> + if (virtio16_to_cpu(vps->vdev, type) == type_table[i].virtio)
> + return type_table[i].pstore;
> + }
> +
> + return PSTORE_TYPE_UNKNOWN;
> +}
> +
> +static void virtpstore_ack(struct virtqueue *vq)
> +{
> + struct virtio_pstore *vps = vq->vdev->priv;
> +
> + wake_up(&vps->acked);
> +}
> +
> +static void virtpstore_check(struct virtqueue *vq)
> +{
> + struct virtio_pstore *vps = vq->vdev->priv;
> + struct virtio_pstore_res *res;
> + unsigned int len;
> +
> + res = virtqueue_get_buf(vq, &len);
> + if (res == NULL)
> + return;
> +
> + if (virtio32_to_cpu(vq->vdev, res->ret) < 0)
> + vps->failed = 1;
> +}
> +
> +static void virt_pstore_get_reqs(struct virtio_pstore *vps,
> + struct virtio_pstore_req **preq,
> + struct virtio_pstore_res **pres)
> +{
> + unsigned int idx = vps->req_id++ % VIRT_PSTORE_NR_REQ;
> +
> + *preq = &vps->req[idx];
> + *pres = &vps->res[idx];
> +
> + memset(*preq, 0, sizeof(**preq));
> + memset(*pres, 0, sizeof(**pres));
> +}
> +
> +static int virt_pstore_open(struct pstore_info *psi)
> +{
> + struct virtio_pstore *vps = psi->data;
> + struct virtio_pstore_req *req;
> + struct virtio_pstore_res *res;
> + struct scatterlist sgo[1], sgi[1];
> + struct scatterlist *sgs[2] = { sgo, sgi };
> + unsigned int len;
> +
> + virt_pstore_get_reqs(vps, &req, &res);
> +
> + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_OPEN);
> +
> + sg_init_one(sgo, req, sizeof(*req));
> + sg_init_one(sgi, res, sizeof(*res));
> + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> + virtqueue_kick(vps->vq[0]);
> +
> + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
Does this block userspace in an uninterruptible wait if
hardware is slow? That's not nice.
> + return virtio32_to_cpu(vps->vdev, res->ret);
> +}
> +
> +static int virt_pstore_close(struct pstore_info *psi)
> +{
> + struct virtio_pstore *vps = psi->data;
> + struct virtio_pstore_req *req = &vps->req[vps->req_id];
> + struct virtio_pstore_res *res = &vps->res[vps->req_id];
> + struct scatterlist sgo[1], sgi[1];
> + struct scatterlist *sgs[2] = { sgo, sgi };
> + unsigned int len;
> +
> + virt_pstore_get_reqs(vps, &req, &res);
> +
> + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_CLOSE);
> +
> + sg_init_one(sgo, req, sizeof(*req));
> + sg_init_one(sgi, res, sizeof(*res));
> + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> + virtqueue_kick(vps->vq[0]);
> +
> + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> + return virtio32_to_cpu(vps->vdev, res->ret);
> +}
> +
> +static ssize_t virt_pstore_read(u64 *id, enum pstore_type_id *type,
> + int *count, struct timespec *time,
> + char **buf, bool *compressed,
> + ssize_t *ecc_notice_size,
> + struct pstore_info *psi)
> +{
> + struct virtio_pstore *vps = psi->data;
> + struct virtio_pstore_req *req;
> + struct virtio_pstore_res *res;
> + struct virtio_pstore_fileinfo info;
> + struct scatterlist sgo[1], sgi[3];
> + struct scatterlist *sgs[2] = { sgo, sgi };
> + unsigned int len;
> + unsigned int flags;
> + int ret;
> + void *bf;
> +
> + virt_pstore_get_reqs(vps, &req, &res);
> +
> + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_READ);
> +
> + sg_init_one(sgo, req, sizeof(*req));
> + sg_init_table(sgi, 3);
> + sg_set_buf(&sgi[0], res, sizeof(*res));
> + sg_set_buf(&sgi[1], &info, sizeof(info));
> + sg_set_buf(&sgi[2], psi->buf, psi->bufsize);
> + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> + virtqueue_kick(vps->vq[0]);
> +
> + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> + if (len < sizeof(*res) + sizeof(info))
> + return -1;
> +
> + ret = virtio32_to_cpu(vps->vdev, res->ret);
> + if (ret < 0)
> + return ret;
> +
> + len = virtio32_to_cpu(vps->vdev, info.len);
> +
> + bf = kmalloc(len, GFP_KERNEL);
> + if (bf == NULL)
> + return -ENOMEM;
> +
> + *id = virtio64_to_cpu(vps->vdev, info.id);
> + *type = from_virtio_type(vps, info.type);
> + *count = virtio32_to_cpu(vps->vdev, info.count);
> +
> + flags = virtio32_to_cpu(vps->vdev, info.flags);
> + *compressed = flags & VIRTIO_PSTORE_FL_COMPRESSED;
> +
> + time->tv_sec = virtio64_to_cpu(vps->vdev, info.time_sec);
> + time->tv_nsec = virtio32_to_cpu(vps->vdev, info.time_nsec);
> +
> + memcpy(bf, psi->buf, len);
> + *buf = bf;
> +
> + return len;
> +}
> +
> +static int notrace virt_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason,
> + u64 *id, unsigned int part, int count,
> + bool compressed, size_t size,
> + struct pstore_info *psi)
> +{
> + struct virtio_pstore *vps = psi->data;
> + struct virtio_pstore_req *req;
> + struct virtio_pstore_res *res;
> + struct scatterlist sgo[2], sgi[1];
> + struct scatterlist *sgs[2] = { sgo, sgi };
> + unsigned int flags = compressed ? VIRTIO_PSTORE_FL_COMPRESSED : 0;
> +
> + if (vps->failed)
> + return -1;
> +
> + *id = vps->req_id;
> + virt_pstore_get_reqs(vps, &req, &res);
> +
> + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_WRITE);
> + req->type = to_virtio_type(vps, type);
> + req->flags = cpu_to_virtio32(vps->vdev, flags);
> +
> + sg_init_table(sgo, 2);
> + sg_set_buf(&sgo[0], req, sizeof(*req));
> + sg_set_buf(&sgo[1], pstore_get_buf(psi), size);
> + sg_init_one(sgi, res, sizeof(*res));
> + virtqueue_add_sgs(vps->vq[1], sgs, 1, 1, vps, GFP_ATOMIC);
> + virtqueue_kick(vps->vq[1]);
> +
> + return 0;
> +}
> +
> +static int virt_pstore_erase(enum pstore_type_id type, u64 id, int count,
> + struct timespec time, struct pstore_info *psi)
> +{
> + struct virtio_pstore *vps = psi->data;
> + struct virtio_pstore_req *req;
> + struct virtio_pstore_res *res;
> + struct scatterlist sgo[1], sgi[1];
> + struct scatterlist *sgs[2] = { sgo, sgi };
> + unsigned int len;
> +
> + virt_pstore_get_reqs(vps, &req, &res);
> +
> + req->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_ERASE);
> + req->type = to_virtio_type(vps, type);
> + req->id = cpu_to_virtio64(vps->vdev, id);
> + req->count = cpu_to_virtio32(vps->vdev, count);
> +
> + sg_init_one(sgo, req, sizeof(*req));
> + sg_init_one(sgi, res, sizeof(*res));
> + virtqueue_add_sgs(vps->vq[0], sgs, 1, 1, vps, GFP_KERNEL);
> + virtqueue_kick(vps->vq[0]);
> +
> + wait_event(vps->acked, virtqueue_get_buf(vps->vq[0], &len));
> + return virtio32_to_cpu(vps->vdev, res->ret);
> +}
> +
> +static int virt_pstore_init(struct virtio_pstore *vps)
> +{
> + struct pstore_info *psinfo = &vps->pstore;
> + int err;
> +
> + if (!psinfo->bufsize)
> + psinfo->bufsize = VIRT_PSTORE_BUFSIZE;
> +
> + psinfo->buf = alloc_pages_exact(psinfo->bufsize, GFP_KERNEL);
> + if (!psinfo->buf) {
> + pr_err("cannot allocate pstore buffer\n");
> + return -ENOMEM;
> + }
> +
> + psinfo->owner = THIS_MODULE;
> + psinfo->name = "virtio";
> + psinfo->open = virt_pstore_open;
> + psinfo->close = virt_pstore_close;
> + psinfo->read = virt_pstore_read;
> + psinfo->erase = virt_pstore_erase;
> + psinfo->write = virt_pstore_write;
> + psinfo->flags = PSTORE_FLAGS_DMESG;
> +
> + psinfo->data = vps;
> + spin_lock_init(&psinfo->buf_lock);
> +
> + err = pstore_register(psinfo);
> + if (err)
> + kfree(psinfo->buf);
> +
> + return err;
> +}
> +
> +static int virt_pstore_exit(struct virtio_pstore *vps)
> +{
> + struct pstore_info *psinfo = &vps->pstore;
> +
> + pstore_unregister(psinfo);
> +
> + free_pages_exact(psinfo->buf, psinfo->bufsize);
> + psinfo->buf = NULL;
> + psinfo->bufsize = 0;
> +
> + return 0;
> +}
> +
> +static int virtpstore_init_vqs(struct virtio_pstore *vps)
> +{
> + vq_callback_t *callbacks[] = { virtpstore_ack, virtpstore_check };
> + const char *names[] = { "pstore_read", "pstore_write" };
> +
> + return vps->vdev->config->find_vqs(vps->vdev, 2, vps->vq,
> + callbacks, names);
> +}
> +
> +static void virtpstore_init_config(struct virtio_pstore *vps)
> +{
> + u32 bufsize;
> +
> + virtio_cread(vps->vdev, struct virtio_pstore_config, bufsize, &bufsize);
> +
> + vps->pstore.bufsize = PAGE_ALIGN(bufsize);
> +}
> +
> +static void virtpstore_confirm_config(struct virtio_pstore *vps)
> +{
> + u32 bufsize = vps->pstore.bufsize;
> +
> + virtio_cwrite(vps->vdev, struct virtio_pstore_config, bufsize,
> + &bufsize);
> +}
> +
> +static int virtpstore_probe(struct virtio_device *vdev)
> +{
> + struct virtio_pstore *vps;
> + int err;
> +
> + if (!vdev->config->get) {
> + dev_err(&vdev->dev, "driver init: config access disabled\n");
> + return -EINVAL;
> + }
> +
> + vdev->priv = vps = kzalloc(sizeof(*vps), GFP_KERNEL);
> + if (!vps) {
> + err = -ENOMEM;
> + goto out;
> + }
> + vps->vdev = vdev;
> +
> + err = virtpstore_init_vqs(vps);
> + if (err < 0)
> + goto out_free;
> +
> + virtpstore_init_config(vps);
> +
> + err = virt_pstore_init(vps);
> + if (err)
> + goto out_del_vq;
> +
> + virtpstore_confirm_config(vps);
> +
> + init_waitqueue_head(&vps->acked);
> +
> + virtio_device_ready(vdev);
> +
> + dev_info(&vdev->dev, "driver init: ok (bufsize = %luK, flags = %x)\n",
> + vps->pstore.bufsize >> 10, vps->pstore.flags);
> +
> + return 0;
> +
> +out_del_vq:
> + vdev->config->del_vqs(vdev);
> +out_free:
> + kfree(vps);
> +out:
> + dev_err(&vdev->dev, "driver init: failed with %d\n", err);
> + return err;
> +}
> +
> +static void virtpstore_remove(struct virtio_device *vdev)
> +{
> + struct virtio_pstore *vps = vdev->priv;
> +
> + virt_pstore_exit(vps);
> +
> + /* Now we reset the device so we can clean up the queues. */
> + vdev->config->reset(vdev);
> +
> + vdev->config->del_vqs(vdev);
> +
> + kfree(vps);
> +}
> +
> +static unsigned int features[] = {
> +};
> +
> +static struct virtio_device_id id_table[] = {
> + { VIRTIO_ID_PSTORE, VIRTIO_DEV_ANY_ID },
> + { 0 },
> +};
> +
> +static struct virtio_driver virtio_pstore_driver = {
> + .driver.name = KBUILD_MODNAME,
> + .driver.owner = THIS_MODULE,
> + .feature_table = features,
> + .feature_table_size = ARRAY_SIZE(features),
> + .id_table = id_table,
> + .probe = virtpstore_probe,
> + .remove = virtpstore_remove,
> +};
> +
> +module_virtio_driver(virtio_pstore_driver);
> +MODULE_DEVICE_TABLE(virtio, id_table);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Namhyung Kim <namhyung@kernel.org>");
> +MODULE_DESCRIPTION("Virtio pstore driver");
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 6d4e92ccdc91..9bbb1554d8b2 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -449,6 +449,7 @@ header-y += virtio_ids.h
> header-y += virtio_input.h
> header-y += virtio_net.h
> header-y += virtio_pci.h
> +header-y += virtio_pstore.h
> header-y += virtio_ring.h
> header-y += virtio_rng.h
> header-y += virtio_scsi.h
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index 77925f587b15..c72a9ab588c0 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -41,5 +41,6 @@
> #define VIRTIO_ID_CAIF 12 /* Virtio caif */
> #define VIRTIO_ID_GPU 16 /* virtio GPU */
> #define VIRTIO_ID_INPUT 18 /* virtio input */
> +#define VIRTIO_ID_PSTORE 22 /* virtio pstore */
>
> #endif /* _LINUX_VIRTIO_IDS_H */
> diff --git a/include/uapi/linux/virtio_pstore.h b/include/uapi/linux/virtio_pstore.h
> new file mode 100644
> index 000000000000..f4b0d204d8ae
> --- /dev/null
> +++ b/include/uapi/linux/virtio_pstore.h
> @@ -0,0 +1,74 @@
> +#ifndef _LINUX_VIRTIO_PSTORE_H
> +#define _LINUX_VIRTIO_PSTORE_H
> +/* This header is BSD licensed so anyone can use the definitions to implement
> + * compatible drivers/servers.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of IBM nor the names of its contributors
> + * may be used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE. */
> +#include <linux/types.h>
> +#include <linux/virtio_types.h>
> +
> +#define VIRTIO_PSTORE_CMD_NULL 0
> +#define VIRTIO_PSTORE_CMD_OPEN 1
> +#define VIRTIO_PSTORE_CMD_READ 2
> +#define VIRTIO_PSTORE_CMD_WRITE 3
> +#define VIRTIO_PSTORE_CMD_ERASE 4
> +#define VIRTIO_PSTORE_CMD_CLOSE 5
> +
> +#define VIRTIO_PSTORE_TYPE_UNKNOWN 0
> +#define VIRTIO_PSTORE_TYPE_DMESG 1
> +
> +#define VIRTIO_PSTORE_FL_COMPRESSED 1
> +
> +struct virtio_pstore_req {
> + __virtio16 cmd;
> + __virtio16 type;
> + __virtio32 flags;
> + __virtio64 id;
> + __virtio32 count;
> + __virtio32 reserved;
> +};
> +
> +struct virtio_pstore_res {
> + __virtio16 cmd;
> + __virtio16 type;
> + __virtio32 ret;
> +};
> +
> +struct virtio_pstore_fileinfo {
> + __virtio64 id;
> + __virtio32 count;
> + __virtio16 type;
> + __virtio16 unused;
> + __virtio32 flags;
> + __virtio32 len;
> + __virtio64 time_sec;
> + __virtio32 time_nsec;
> + __virtio32 reserved;
> +};
> +
> +struct virtio_pstore_config {
> + __virtio32 bufsize;
> +};
> +
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.
> +#endif /* _LINUX_VIRTIO_PSTORE_H */
> --
> 2.9.3
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Gerd Hoffmann @ 2016-11-09 8:01 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Airlie, Jiri Slaby, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <20161108223153-mutt-send-email-mst@kernel.org>
On Di, 2016-11-08 at 22:37 +0200, Michael S. Tsirkin wrote:
> On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> > Hi,
> >
> > I can relatively easily reproduce this bug:
How?
> > BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> The following might be helpful for debugging - if kernel still will
> not stop panicing, we are looking at some kind
> of memory corruption.
Looking carefully through the code I think it isn't impossible to
trigger this, but you need for that:
(1) command queue full (quite possible),
(2) cursor queue full too (unlikely), and
(3) multiple threads trying to submit commands and waiting for free
space in the command queue (possible with virgl enabled).
Do things improve if you allocate some extra bufs?
int virtio_gpu_alloc_vbufs(struct virtio_gpu_device *vgdev)
{
struct virtio_gpu_vbuffer *vbuf;
- int i, size, count = 0;
+ int i, size, count = 16;
void *ptr;
INIT_LIST_HEAD(&vgdev->free_vbufs);
Memory corruption sounds plausible too.
Redirect console to ttyS0 for trouble-shooting, trying to dump the oops
to the display device which triggered the oops in the first place isn't
going to work very well ...
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH kernel v4 7/7] virtio-balloon: tell host vm's unused page info
From: Michael S. Tsirkin @ 2016-11-08 21:07 UTC (permalink / raw)
To: Dave Hansen
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Li, Liang Z, qemu-devel@nongnu.org,
dgilbert@redhat.com, linux-mm@kvack.org, amit.shah@redhat.com,
pbonzini@redhat.com, virtualization@lists.linux-foundation.org,
mgorman@techsingularity.net
In-Reply-To: <281acd8d-fd94-6318-35e5-9eb130303dc6@intel.com>
On Mon, Nov 07, 2016 at 09:23:38AM -0800, Dave Hansen wrote:
> On 11/06/2016 07:37 PM, Li, Liang Z wrote:
> >> Let's say we do a 32k bitmap that can hold ~1M pages. That's 4GB of RAM.
> >> On a 1TB system, that's 256 passes through the top-level loop.
> >> The bottom-level lists have tens of thousands of pages in them, even on my
> >> laptop. Only 1/256 of these pages will get consumed in a given pass.
> >>
> > Your description is not exactly.
> > A 32k bitmap is used only when there is few free memory left in the system and when
> > the extend_page_bitmap() failed to allocate more memory for the bitmap. Or dozens of
> > 32k split bitmap will be used, this version limit the bitmap count to 32, it means we can use
> > at most 32*32 kB for the bitmap, which can cover 128GB for RAM. We can increase the bitmap
> > count limit to a larger value if 32 is not big enough.
>
> OK, so it tries to allocate a large bitmap. But, if it fails, it will
> try to work with a smaller bitmap. Correct?
>
> So, what's the _worst_ case? It sounds like it is even worse than I was
> positing.
>
> >> That's an awfully inefficient way of doing it. This patch essentially changed
> >> the data structure without changing the algorithm to populate it.
> >>
> >> Please change the *algorithm* to use the new data structure efficiently.
> >> Such a change would only do a single pass through each freelist, and would
> >> choose whether to use the extent-based (pfn -> range) or bitmap-based
> >> approach based on the contents of the free lists.
> >
> > Save the free page info to a raw bitmap first and then process the raw bitmap to
> > get the proper ' extent-based ' and 'bitmap-based' is the most efficient way I can
> > come up with to save the virtio data transmission. Do you have some better idea?
>
> That's kinda my point. This patch *does* processing to try to pack the
> bitmaps full of pages from the various pfn ranges. It's a form of
> processing that gets *REALLY*, *REALLY* bad in some (admittedly obscure)
> cases.
>
> Let's not pretend that making an essentially unlimited number of passes
> over the free lists is not processing.
>
> 1. Allocate as large of a bitmap as you can. (what you already do)
> 2. Iterate from the largest freelist order. Store those pages in the
> bitmap.
> 3. If you can no longer fit pages in the bitmap, return the list that
> you have.
> 4. Make an approximation about where the bitmap does not make any more,
> and fall back to listing individual PFNs. This would make sens, for
> instance in a large zone with very few free order-0 pages left.
In practice, a single PFN using the bitmap format
only takes up twice the size: I think it's 128 instead of 64 bit
per entry.
So it's not a a given that point 4 is worth it at any point,
just packing multiple bitmaps might be good enough.
>
> > It seems the benefit we get for this feature is not as big as that in fast balloon inflating/deflating.
> >>
> >> You should not be using get_max_pfn(). Any patch set that continues to use
> >> it is not likely to be using a proper algorithm.
> >
> > Do you have any suggestion about how to avoid it?
>
> Yes: get the pfns from the page free lists alone. Don't derive them
> from the pfn limits of the system or zones.
^ permalink raw reply
* Re: BUG: 'list_empty(&vgdev->free_vbufs)' is true!
From: Michael S. Tsirkin @ 2016-11-08 20:37 UTC (permalink / raw)
To: Jiri Slaby
Cc: David Airlie, Linux kernel mailing list, dri-devel,
virtualization
In-Reply-To: <bfe29853-694b-6cb5-02e7-6986a9927438@suse.cz>
On Mon, Nov 07, 2016 at 09:43:24AM +0100, Jiri Slaby wrote:
> Hi,
>
> I can relatively easily reproduce this bug:
> BUG: 'list_empty(&vgdev->free_vbufs)' is true!
> ------------[ cut here ]------------
> kernel BUG at /home/latest/linux/drivers/gpu/drm/virtio/virtgpu_vq.c:130!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> Modules linked in:
> CPU: 1 PID: 355 Comm: kworker/1:2 Not tainted 4.9.0-rc2-next-20161028+ #32
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
> Workqueue: events drm_fb_helper_dirty_work
> task: ffff88007b124980 task.stack: ffff88007b8a0000
> RIP: 0010:virtio_gpu_get_vbuf+0x32e/0x630
> RSP: 0018:ffff88007b8a78c0 EFLAGS: 00010286
> RAX: 000000000000002e RBX: 1ffff1000f714f1d RCX: 0000000000000000
> RDX: 000000000000002e RSI: 0000000000000001 RDI: ffffed000f714f0e
> RBP: ffff88007b8a7970 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000002 R11: 0000000000000001 R12: 0000000000000030
> R13: ffff88007caeaba8 R14: 0000000000000018 R15: ffff88007cae0000
> FS: 0000000000000000(0000) GS:ffff88007dc80000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000601028 CR3: 000000007740d000 CR4: 00000000000006e0
> Call Trace:
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bb 01 00 00 4c 89 69 e8
> eb 9e 48 c7 c6 e0 d2 d1 83 48 c7 c7 20 d3 d1 83 e8 6c fb 04 ff <0f> 0b
> 48 c7 c7 a0 fb b0 85 e8 09 95 86 ff 48 c7 c6 c0 d3 d1 83
> RIP: virtio_gpu_get_vbuf+0x32e/0x630 RSP: ffff88007b8a78c0
>
>
> There is no stacktrace, as the kernel starts panicing all over the place
> during its generation. Any ideas?
>
> thanks,
CC maintainers.
The following might be helpful for debugging - if kernel still will
not stop panicing, we are looking at some kind
of memory corruption.
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a7..d5e1e72 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -127,7 +127,11 @@ virtio_gpu_get_vbuf(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
spin_lock(&vgdev->free_vbufs_lock);
- BUG_ON(list_empty(&vgdev->free_vbufs));
+ WARN_ON(list_empty(&vgdev->free_vbufs));
+ if (list_empty(&vgdev->free_vbufs)) {
+ spin_unlock(&vgdev->free_vbufs_lock);
+ return ERR_PTR(-EINVAL);
+ }
vbuf = list_first_entry(&vgdev->free_vbufs,
struct virtio_gpu_vbuffer, list);
list_del(&vbuf->list);
^ permalink raw reply related
* Re: [PATCH kernel v4 7/7] virtio-balloon: tell host vm's unused page info
From: Dave Hansen @ 2016-11-08 18:30 UTC (permalink / raw)
To: Li, Liang Z, mst@redhat.com
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
dgilbert@redhat.com, linux-mm@kvack.org, amit.shah@redhat.com,
pbonzini@redhat.com, virtualization@lists.linux-foundation.org,
mgorman@techsingularity.net
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E3A108196@shsmsx102.ccr.corp.intel.com>
On 11/07/2016 09:50 PM, Li, Liang Z wrote:
> Sounds good. Should we ignore some of the order-0 pages in step 4 if the bitmap is full?
> Or should retry to get a complete list of order-0 pages?
I think that's a pretty reasonable thing to do.
>>> It seems the benefit we get for this feature is not as big as that in fast
>> balloon inflating/deflating.
>>>>
>>>> You should not be using get_max_pfn(). Any patch set that continues
>>>> to use it is not likely to be using a proper algorithm.
>>>
>>> Do you have any suggestion about how to avoid it?
>>
>> Yes: get the pfns from the page free lists alone. Don't derive
>> them from the pfn limits of the system or zones.
>
> The ' get_max_pfn()' can be avoid in this patch, but I think we can't
> avoid it completely. We need it as a hint for allocating a proper
> size bitmap. No?
If you start with higher-order pages, you'll be unlikely to get anywhere
close to filling up a bitmap that was sized to hold all possible order-0
pages on the system. Any use of max_pfn also means that you'll
completely mis-size bitmaps on sparse systems with large holes.
I think you should size it based on the size of the free lists, if anything.
^ permalink raw reply
* RE: [PATCH kernel v4 7/7] virtio-balloon: tell host vm's unused page info
From: Li, Liang Z @ 2016-11-08 5:50 UTC (permalink / raw)
To: Hansen, Dave, mst@redhat.com
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
dgilbert@redhat.com, linux-mm@kvack.org, amit.shah@redhat.com,
pbonzini@redhat.com, virtualization@lists.linux-foundation.org,
mgorman@techsingularity.net
In-Reply-To: <281acd8d-fd94-6318-35e5-9eb130303dc6@intel.com>
> On 11/06/2016 07:37 PM, Li, Liang Z wrote:
> >> Let's say we do a 32k bitmap that can hold ~1M pages. That's 4GB of RAM.
> >> On a 1TB system, that's 256 passes through the top-level loop.
> >> The bottom-level lists have tens of thousands of pages in them, even
> >> on my laptop. Only 1/256 of these pages will get consumed in a given pass.
> >>
> > Your description is not exactly.
> > A 32k bitmap is used only when there is few free memory left in the
> > system and when the extend_page_bitmap() failed to allocate more
> > memory for the bitmap. Or dozens of 32k split bitmap will be used,
> > this version limit the bitmap count to 32, it means we can use at most
> > 32*32 kB for the bitmap, which can cover 128GB for RAM. We can increase
> the bitmap count limit to a larger value if 32 is not big enough.
>
> OK, so it tries to allocate a large bitmap. But, if it fails, it will try to work with a
> smaller bitmap. Correct?
>
Yes.
> So, what's the _worst_ case? It sounds like it is even worse than I was
> positing.
>
Only a 32KB bitmap can be allocated, and there are a huge amount of low order (<3) free pages is the worst case.
> >> That's an awfully inefficient way of doing it. This patch
> >> essentially changed the data structure without changing the algorithm to
> populate it.
> >>
> >> Please change the *algorithm* to use the new data structure efficiently.
> >> Such a change would only do a single pass through each freelist, and
> >> would choose whether to use the extent-based (pfn -> range) or
> >> bitmap-based approach based on the contents of the free lists.
> >
> > Save the free page info to a raw bitmap first and then process the raw
> > bitmap to get the proper ' extent-based ' and 'bitmap-based' is the
> > most efficient way I can come up with to save the virtio data transmission.
> Do you have some better idea?
>
> That's kinda my point. This patch *does* processing to try to pack the
> bitmaps full of pages from the various pfn ranges. It's a form of processing
> that gets *REALLY*, *REALLY* bad in some (admittedly obscure) cases.
>
> Let's not pretend that making an essentially unlimited number of passes over
> the free lists is not processing.
>
> 1. Allocate as large of a bitmap as you can. (what you already do) 2. Iterate
> from the largest freelist order. Store those pages in the
> bitmap.
> 3. If you can no longer fit pages in the bitmap, return the list that
> you have.
> 4. Make an approximation about where the bitmap does not make any more,
> and fall back to listing individual PFNs. This would make sens, for
> instance in a large zone with very few free order-0 pages left.
>
Sounds good. Should we ignore some of the order-0 pages in step 4 if the bitmap is full?
Or should retry to get a complete list of order-0 pages?
>
> > It seems the benefit we get for this feature is not as big as that in fast
> balloon inflating/deflating.
> >>
> >> You should not be using get_max_pfn(). Any patch set that continues
> >> to use it is not likely to be using a proper algorithm.
> >
> > Do you have any suggestion about how to avoid it?
>
> Yes: get the pfns from the page free lists alone. Don't derive them from the
> pfn limits of the system or zones.
The ' get_max_pfn()' can be avoid in this patch, but I think we can't avoid it completely.
We need it as a hint for allocating a proper size bitmap. No?
Thanks!
Liang
^ permalink raw reply
* Re: [PATCH] virtio-net: drop legacy features in virtio 1 mode
From: David Miller @ 2016-11-08 1:36 UTC (permalink / raw)
To: mst; +Cc: netdev, linux-kernel, stable, virtualization
In-Reply-To: <1478256865-29003-1-git-send-email-mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 4 Nov 2016 12:55:36 +0200
> Virtio 1.0 spec says VIRTIO_F_ANY_LAYOUT and VIRTIO_NET_F_GSO are
> legacy-only feature bits. Do not negotiate them in virtio 1 mode. Note
> this is a spec violation so we need to backport it to stable/downstream
> kernels.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH kernel v4 7/7] virtio-balloon: tell host vm's unused page info
From: Dave Hansen @ 2016-11-07 17:23 UTC (permalink / raw)
To: Li, Liang Z, mst@redhat.com
Cc: virtio-dev@lists.oasis-open.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
dgilbert@redhat.com, linux-mm@kvack.org, amit.shah@redhat.com,
pbonzini@redhat.com, virtualization@lists.linux-foundation.org,
mgorman@techsingularity.net
In-Reply-To: <F2CBF3009FA73547804AE4C663CAB28E3A106DAA@shsmsx102.ccr.corp.intel.com>
On 11/06/2016 07:37 PM, Li, Liang Z wrote:
>> Let's say we do a 32k bitmap that can hold ~1M pages. That's 4GB of RAM.
>> On a 1TB system, that's 256 passes through the top-level loop.
>> The bottom-level lists have tens of thousands of pages in them, even on my
>> laptop. Only 1/256 of these pages will get consumed in a given pass.
>>
> Your description is not exactly.
> A 32k bitmap is used only when there is few free memory left in the system and when
> the extend_page_bitmap() failed to allocate more memory for the bitmap. Or dozens of
> 32k split bitmap will be used, this version limit the bitmap count to 32, it means we can use
> at most 32*32 kB for the bitmap, which can cover 128GB for RAM. We can increase the bitmap
> count limit to a larger value if 32 is not big enough.
OK, so it tries to allocate a large bitmap. But, if it fails, it will
try to work with a smaller bitmap. Correct?
So, what's the _worst_ case? It sounds like it is even worse than I was
positing.
>> That's an awfully inefficient way of doing it. This patch essentially changed
>> the data structure without changing the algorithm to populate it.
>>
>> Please change the *algorithm* to use the new data structure efficiently.
>> Such a change would only do a single pass through each freelist, and would
>> choose whether to use the extent-based (pfn -> range) or bitmap-based
>> approach based on the contents of the free lists.
>
> Save the free page info to a raw bitmap first and then process the raw bitmap to
> get the proper ' extent-based ' and 'bitmap-based' is the most efficient way I can
> come up with to save the virtio data transmission. Do you have some better idea?
That's kinda my point. This patch *does* processing to try to pack the
bitmaps full of pages from the various pfn ranges. It's a form of
processing that gets *REALLY*, *REALLY* bad in some (admittedly obscure)
cases.
Let's not pretend that making an essentially unlimited number of passes
over the free lists is not processing.
1. Allocate as large of a bitmap as you can. (what you already do)
2. Iterate from the largest freelist order. Store those pages in the
bitmap.
3. If you can no longer fit pages in the bitmap, return the list that
you have.
4. Make an approximation about where the bitmap does not make any more,
and fall back to listing individual PFNs. This would make sens, for
instance in a large zone with very few free order-0 pages left.
> It seems the benefit we get for this feature is not as big as that in fast balloon inflating/deflating.
>>
>> You should not be using get_max_pfn(). Any patch set that continues to use
>> it is not likely to be using a proper algorithm.
>
> Do you have any suggestion about how to avoid it?
Yes: get the pfns from the page free lists alone. Don't derive them
from the pfn limits of the system or zones.
^ permalink raw reply
* Re: [PATCH] virtio-net: drop legacy features in virtio 1 mode
From: Jason Wang @ 2016-11-07 9:17 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel; +Cc: netdev, stable, virtualization
In-Reply-To: <1478256865-29003-1-git-send-email-mst@redhat.com>
On 2016年11月04日 18:55, Michael S. Tsirkin wrote:
> Virtio 1.0 spec says VIRTIO_F_ANY_LAYOUT and VIRTIO_NET_F_GSO are
> legacy-only feature bits. Do not negotiate them in virtio 1 mode. Note
> this is a spec violation so we need to backport it to stable/downstream
> kernels.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/virtio_net.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7a00365..b19fb4d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2089,23 +2089,33 @@ static struct virtio_device_id id_table[] = {
> { 0 },
> };
>
> +#define VIRTNET_FEATURES \
> + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, \
> + VIRTIO_NET_F_MAC, \
> + VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, \
> + VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, \
> + VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO, \
> + VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ, \
> + VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> + VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> + VIRTIO_NET_F_CTRL_MAC_ADDR, \
> + VIRTIO_NET_F_MTU
> +
> static unsigned int features[] = {
> - VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
> - VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
> - VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
> - VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
> - VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
> - VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
> - VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
> - VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> - VIRTIO_NET_F_CTRL_MAC_ADDR,
> + VIRTNET_FEATURES,
> +};
> +
> +static unsigned int features_legacy[] = {
> + VIRTNET_FEATURES,
> + VIRTIO_NET_F_GSO,
> VIRTIO_F_ANY_LAYOUT,
> - VIRTIO_NET_F_MTU,
> };
>
> static struct virtio_driver virtio_net_driver = {
> .feature_table = features,
> .feature_table_size = ARRAY_SIZE(features),
> + .feature_table_legacy = features_legacy,
> + .feature_table_size_legacy = ARRAY_SIZE(features_legacy),
> .driver.name = KBUILD_MODNAME,
> .driver.owner = THIS_MODULE,
> .id_table = id_table,
Acked-by: Jason Wang <jasowang@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox