Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH 1/1] virtio/s390: handle failures of READ_VQ_CONF ccw
From: Cornelia Huck @ 2015-09-10 14:35 UTC (permalink / raw)
  To: mst; +Cc: linux-s390, kvm, Pierre Morel, virtualization, borntraeger
In-Reply-To: <1441895708-70657-1-git-send-email-cornelia.huck@de.ibm.com>

From: Pierre Morel <pmorel@linux.vnet.ibm.com>

In virtio_ccw_read_vq_conf() the return value of ccw_io_helper()
was not checked.
If the configuration could not be read properly, we'd wrongly assume a
queue size of 0.

Let's propagate any I/O error to virtio_ccw_setup_vq() so it may
properly fail.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/s390/virtio/virtio_ccw.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index f8d8fdb..e9fae30 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -400,12 +400,16 @@ static bool virtio_ccw_kvm_notify(struct virtqueue *vq)
 static int virtio_ccw_read_vq_conf(struct virtio_ccw_device *vcdev,
 				   struct ccw1 *ccw, int index)
 {
+	int ret;
+
 	vcdev->config_block->index = index;
 	ccw->cmd_code = CCW_CMD_READ_VQ_CONF;
 	ccw->flags = 0;
 	ccw->count = sizeof(struct vq_config_block);
 	ccw->cda = (__u32)(unsigned long)(vcdev->config_block);
-	ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_VQ_CONF);
+	ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_VQ_CONF);
+	if (ret)
+		return ret;
 	return vcdev->config_block->num;
 }
 
@@ -503,6 +507,10 @@ static struct virtqueue *virtio_ccw_setup_vq(struct virtio_device *vdev,
 		goto out_err;
 	}
 	info->num = virtio_ccw_read_vq_conf(vcdev, ccw, i);
+	if (info->num < 0) {
+		err = info->num;
+		goto out_err;
+	}
 	size = PAGE_ALIGN(vring_size(info->num, KVM_VIRTIO_CCW_RING_ALIGN));
 	info->queue = alloc_pages_exact(size, GFP_KERNEL | __GFP_ZERO);
 	if (info->queue == NULL) {
-- 
2.3.8

^ permalink raw reply related

* Re: [RFC PATCH 0/2] virtio nvme
From: Stefan Hajnoczi @ 2015-09-10 14:38 UTC (permalink / raw)
  To: Ming Lin; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <1441864112-12765-1-git-send-email-mlin@kernel.org>

On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
> These 2 patches added virtio-nvme to kernel and qemu,
> basically modified from virtio-blk and nvme code.
>
> As title said, request for your comments.
>
> Play it in Qemu with:
> -drive file=disk.img,format=raw,if=none,id=D22 \
> -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
>
> The goal is to have a full NVMe stack from VM guest(virtio-nvme)
> to host(vhost_nvme) to LIO NVMe-over-fabrics target.

Why is a virtio-nvme guest device needed?  I guess there must either
be NVMe-only features that you want to pass through, or you think the
performance will be significantly better than virtio-blk/virtio-scsi?

At first glance it seems like the virtio_nvme guest driver is just
another block driver like virtio_blk, so I'm not clear why a
virtio-nvme device makes sense.

> Now there are lots of duplicated code with linux/nvme-core.c and qemu/nvme.c.
> The ideal result is to have a multi level NVMe stack(similar as SCSI).
> So we can re-use the nvme code, for example
>
>                         .-------------------------.
>                         | NVMe device register    |
>   Upper level           | NVMe protocol process   |
>                         |                         |
>                         '-------------------------'
>
>
>
>               .-----------.    .-----------.    .------------------.
>  Lower level  |   PCIe    |    | VIRTIO    |    |NVMe over Fabrics |
>               |           |    |           |    |initiator         |
>               '-----------'    '-----------'    '------------------'

You mentioned LIO and SCSI.  How will NVMe over Fabrics be integrated
into LIO?  If it is mapped to SCSI then using virtio_scsi in the guest
and tcm_vhost should work.

Please also post virtio draft specifications documenting the virtio device.

Stefan

^ permalink raw reply

* Re: [PATCH 3/5] update virtio gpu driver: add 3d/virgl support
From: Gerd Hoffmann @ 2015-09-10 14:45 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Michael S. Tsirkin, open list:ABI/API, open list, ML dri-devel,
	open list:VIRTIO GPU DRIVER, Dave Airlie
In-Reply-To: <CACvgo51Y7j0ka46Rfq2eBvXzP4q8Jc6x4j-JrYHDLkH6_xOh7w@mail.gmail.com>

  Hi,

> Just a FYI - Daniel Vetter has a series in flight which deprecates
> DRM_UNLOCKED for KMS drivers.

Thanks for the heads up.

> 
> > --- /dev/null
> > +++ b/include/uapi/drm/virtgpu_drm.h
> > @@ -0,0 +1,163 @@
> 
> > +
> > +struct drm_virtgpu_3d_box {
> > +       uint32_t x, y, z;
> > +       uint32_t w, h, d;
> > +};
> > +
> There was a similar case (multiple variables declared on a single
> line) in drm core that caused confusion and we broke the 32bit compat.
> I thought I mention it - not advocating for/against the above declaration.

I highly doubt we'll ever change that.
But we can give each struct field its own row, sure.

> > +struct drm_virtgpu_3d_transfer_to_host {
> > +       uint32_t bo_handle;
> > +       struct drm_virtgpu_3d_box box;
> > +       uint32_t level;
> > +       uint32_t offset;
> > +};
> > +
> > +struct drm_virtgpu_3d_transfer_from_host {
> > +       uint32_t bo_handle;
> > +       struct drm_virtgpu_3d_box box;
> > +       uint32_t level;
> > +       uint32_t offset;
> > +};
> > +
> Afaics these seems to be used by the ioctls. If so the current
> declarations are not 32bit compat safe.

Why?  As long as we have only 32bit fields in the struct the struct
itself gets a 32bit alignment too.  So no layout differences between
32bit and 64bit.

> Things will also go badly if you consider expanding 
> struct drm_virtgpu_3d_box in the distant future.

See above, very unlikely.  And should that really happen we have bigger
problems anyway because it is quite likely that we have to touch the
virtio wire protocol too.

> A u32 pad after bo_handle and a 'pointer' to struct
> drm_virtgpu_3d_box might be the more flexible solution.

IMO this makes things more complicated for no good reason.

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH 5/5] virtgpu: mark as a render gpu
From: Gerd Hoffmann @ 2015-09-10 14:52 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Dave Airlie, open list, ML dri-devel, open list:VIRTIO GPU DRIVER
In-Reply-To: <CACvgo50SU46SYXBbVe+VGcAb6uFSj-iBY0E4=Vuuzf5RKbE+2Q@mail.gmail.com>

  Hi,

> > Dave?  Looking at the ioctls they are all fine for render nodes, there
> > isn't anything modesetting related in the device-specific ioctls.
> >
> > Correct?
> >
> Unless I've overdone the coffee this time - modesetting is done via
> the card# node, while render via either card# or renderD#.

Exactly, thats why anything modesetting-related must be disabled for
renderD#.  Looking at the virtio-gpu device-specific ioctls I don't
think there is anything doing modesetting (which we would have to leave
out), so we can apply DRM_RENDER_ALLOW everythere I think.  Or maybe
there is a global switch to flip DRM_RENDER_ALLOW for the whole list ...

cheers,
  Gerd

^ permalink raw reply

* Re: [PATCH 0/1] virtio/s390: one bugfix
From: Michael S. Tsirkin @ 2015-09-10 14:58 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: borntraeger, linux-s390, kvm, virtualization
In-Reply-To: <1441895708-70657-1-git-send-email-cornelia.huck@de.ibm.com>

On Thu, Sep 10, 2015 at 04:35:07PM +0200, Cornelia Huck wrote:
> Michael,
> 
> here's a bugfix for the virtio-ccw driver.
> 
> Question: How would you like to get patches? This one is formatted
> against your current linux-next branch; I can prepare a branch for
> you to pull from as well.

No need, patches are fine.

> Pierre Morel (1):
>   virtio/s390: handle failures of READ_VQ_CONF ccw
> 
>  drivers/s390/virtio/virtio_ccw.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> -- 
> 2.3.8

^ permalink raw reply

* Re: [PATCH 5/5] virtgpu: mark as a render gpu
From: Emil Velikov @ 2015-09-10 15:04 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Dave Airlie, open list, ML dri-devel, open list:VIRTIO GPU DRIVER
In-Reply-To: <1441896765.11233.25.camel@redhat.com>

On 10 September 2015 at 15:52, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> > Dave?  Looking at the ioctls they are all fine for render nodes, there
>> > isn't anything modesetting related in the device-specific ioctls.
>> >
>> > Correct?
>> >
>> Unless I've overdone the coffee this time - modesetting is done via
>> the card# node, while render via either card# or renderD#.
>
> Exactly, thats why anything modesetting-related must be disabled for
> renderD#.  Looking at the virtio-gpu device-specific ioctls I don't
> think there is anything doing modesetting (which we would have to leave
> out), so we can apply DRM_RENDER_ALLOW everythere I think.  Or maybe
> there is a global switch to flip DRM_RENDER_ALLOW for the whole list ...
>
IMHO the idea of having a 'global' switch sounds quite good, yet there
isn't one atm :-( It will be quite useful as we get more render only
devices.
DRIVER_RENDER doesn't do that unfortunately (which I think was the
original assumption), it only instructs drm core to create the
renderD# device/node.

Hope this clears up any ambiguity from my earlier replies :-)
Emil

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Ming Lin @ 2015-09-10 17:02 UTC (permalink / raw)
  To: Keith Busch; +Cc: Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <alpine.LNX.2.00.1509101356200.23840@localhost.lm.intel.com>

On Thu, 2015-09-10 at 14:02 +0000, Keith Busch wrote:
> On Wed, 9 Sep 2015, Ming Lin wrote:
> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
> >
> > Now there are lots of duplicated code with linux/nvme-core.c and qemu/nvme.c.
> > The ideal result is to have a multi level NVMe stack(similar as SCSI).
> > So we can re-use the nvme code, for example
> >
> >                        .-------------------------.
> >                        | NVMe device register    |
> >  Upper level           | NVMe protocol process   |
> >                        |                         |
> >                        '-------------------------'
> >
> >
> >
> >              .-----------.    .-----------.    .------------------.
> > Lower level  |   PCIe    |    | VIRTIO    |    |NVMe over Fabrics |
> >              |           |    |           |    |initiator         |
> >              '-----------'    '-----------'    '------------------'
> >
> > todo:
> > - tune performance. Should be as good as virtio-blk/virtio-scsi
> > - support discard/flush/integrity
> > - need Redhat's help for the VIRTIO_ID_NVME pci id
> > - multi level NVMe stack
> 
> Hi Ming,

Hi Keith,

> 
> I'll be out for travel for the next week, so I won't have much time to
> do a proper review till the following week.
> 
> I think it'd be better to get this hierarchy setup to make the most reuse
> possible than to have this much code duplication between the existing
> driver and emulated qemu nvme. For better or worse, I think the generic
> nvme layer is where things are going. Are you signed up with the fabrics
> contributors?

No. How to sign up?

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Ming Lin @ 2015-09-10 17:28 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <CAJSP0QUmtgoh23G3u64xRnBbJkAoO19AEi=vkyPPyg9n9jqC2Q@mail.gmail.com>

On Thu, 2015-09-10 at 15:38 +0100, Stefan Hajnoczi wrote:
> On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
> > These 2 patches added virtio-nvme to kernel and qemu,
> > basically modified from virtio-blk and nvme code.
> >
> > As title said, request for your comments.
> >
> > Play it in Qemu with:
> > -drive file=disk.img,format=raw,if=none,id=D22 \
> > -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
> >
> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
> 
> Why is a virtio-nvme guest device needed?  I guess there must either
> be NVMe-only features that you want to pass through, or you think the
> performance will be significantly better than virtio-blk/virtio-scsi?

It simply passes through NVMe commands.

Right now performance is poor. Performance tunning is on my todo list.
It should be as good as virtio-blk/virtio-scsi.

> 
> At first glance it seems like the virtio_nvme guest driver is just
> another block driver like virtio_blk, so I'm not clear why a
> virtio-nvme device makes sense.

I think the future "LIO NVMe target" only speaks NVMe protocol.

Nick(CCed), could you correct me if I'm wrong?

For SCSI stack, we have:
virtio-scsi(guest)
tcm_vhost(or vhost_scsi, host)
LIO-scsi-target

For NVMe stack, we'll have similar components:
virtio-nvme(guest)
vhost_nvme(host)
LIO-NVMe-target

> 
> > Now there are lots of duplicated code with linux/nvme-core.c and qemu/nvme.c.
> > The ideal result is to have a multi level NVMe stack(similar as SCSI).
> > So we can re-use the nvme code, for example
> >
> >                         .-------------------------.
> >                         | NVMe device register    |
> >   Upper level           | NVMe protocol process   |
> >                         |                         |
> >                         '-------------------------'
> >
> >
> >
> >               .-----------.    .-----------.    .------------------.
> >  Lower level  |   PCIe    |    | VIRTIO    |    |NVMe over Fabrics |
> >               |           |    |           |    |initiator         |
> >               '-----------'    '-----------'    '------------------'
> 
> You mentioned LIO and SCSI.  How will NVMe over Fabrics be integrated
> into LIO?  If it is mapped to SCSI then using virtio_scsi in the guest
> and tcm_vhost should work.

I think it's not mapped to SCSI.

Nick, would you share more here?

> 
> Please also post virtio draft specifications documenting the virtio device.

I'll do this later.

> 
> Stefan

^ permalink raw reply

* Re: [PATCH 2/4] vhost: move features to core
From: Sergei Shtylyov @ 2015-09-10 19:15 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel; +Cc: netdev, Xie, Huawei, kvm, virtualization
In-Reply-To: <1441869802-15847-3-git-send-email-mst@redhat.com>

Hello.

On 09/10/2015 10:23 AM, Michael S. Tsirkin wrote:

> virtio 1 and any layout are core features, move them
> there. This fixes vhost test.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

[...]
> diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
> index d9c501e..f2882ac 100644
> --- a/drivers/vhost/test.c
> +++ b/drivers/vhost/test.c
> @@ -277,10 +277,13 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl,
>   			return -EFAULT;
>   		return 0;
>   	case VHOST_SET_FEATURES:
> +		printk(KERN_ERR "1\n");
>   		if (copy_from_user(&features, featurep, sizeof features))
>   			return -EFAULT;
> +		printk(KERN_ERR "2\n");
>   		if (features & ~VHOST_FEATURES)
>   			return -EOPNOTSUPP;
> +		printk(KERN_ERR "3\n");
>   		return vhost_test_set_features(n, features);
>   	case VHOST_RESET_OWNER:
>   		return vhost_test_reset_owner(n);

    Debugging printk's remained?

MBR, Sergei

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Ming Lin @ 2015-09-11  4:55 UTC (permalink / raw)
  To: Ming Lin; +Cc: Keith Busch, Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <1441904521.18716.4.camel@ssi>

On Thu, Sep 10, 2015 at 10:02 AM, Ming Lin <mlin@kernel.org> wrote:
> On Thu, 2015-09-10 at 14:02 +0000, Keith Busch wrote:
>> On Wed, 9 Sep 2015, Ming Lin wrote:
>> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
>> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
>> >
>> > Now there are lots of duplicated code with linux/nvme-core.c and qemu/nvme.c.
>> > The ideal result is to have a multi level NVMe stack(similar as SCSI).
>> > So we can re-use the nvme code, for example
>> >
>> >                        .-------------------------.
>> >                        | NVMe device register    |
>> >  Upper level           | NVMe protocol process   |
>> >                        |                         |
>> >                        '-------------------------'
>> >
>> >
>> >
>> >              .-----------.    .-----------.    .------------------.
>> > Lower level  |   PCIe    |    | VIRTIO    |    |NVMe over Fabrics |
>> >              |           |    |           |    |initiator         |
>> >              '-----------'    '-----------'    '------------------'
>> >
>> > todo:
>> > - tune performance. Should be as good as virtio-blk/virtio-scsi
>> > - support discard/flush/integrity
>> > - need Redhat's help for the VIRTIO_ID_NVME pci id
>> > - multi level NVMe stack
>>
>> Hi Ming,
>
> Hi Keith,
>
>>
>> I'll be out for travel for the next week, so I won't have much time to
>> do a proper review till the following week.
>>
>> I think it'd be better to get this hierarchy setup to make the most reuse
>> possible than to have this much code duplication between the existing
>> driver and emulated qemu nvme. For better or worse, I think the generic
>> nvme layer is where things are going. Are you signed up with the fabrics
>> contributors?
>
> No. How to sign up?

Got it. Just saw your previous mail: A message on NVMe Over Fabrics

^ permalink raw reply

* Re: [PATCH 5/5] virtgpu: mark as a render gpu
From: Dave Airlie @ 2015-09-11  6:32 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Dave Airlie, open list:VIRTIO GPU DRIVER, ML dri-devel, open list
In-Reply-To: <CACvgo51skbWdw8E8RftZZjA2wUV0_RHPCiPbSR42XPY9aCfg6w@mail.gmail.com>

On 11 September 2015 at 01:04, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 10 September 2015 at 15:52, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>   Hi,
>>
>>> > Dave?  Looking at the ioctls they are all fine for render nodes, there
>>> > isn't anything modesetting related in the device-specific ioctls.
>>> >
>>> > Correct?
>>> >
>>> Unless I've overdone the coffee this time - modesetting is done via
>>> the card# node, while render via either card# or renderD#.
>>
>> Exactly, thats why anything modesetting-related must be disabled for
>> renderD#.  Looking at the virtio-gpu device-specific ioctls I don't
>> think there is anything doing modesetting (which we would have to leave
>> out), so we can apply DRM_RENDER_ALLOW everythere I think.  Or maybe
>> there is a global switch to flip DRM_RENDER_ALLOW for the whole list ...
>>
> IMHO the idea of having a 'global' switch sounds quite good, yet there
> isn't one atm :-( It will be quite useful as we get more render only
> devices.
> DRIVER_RENDER doesn't do that unfortunately (which I think was the
> original assumption), it only instructs drm core to create the
> renderD# device/node.

doh, yes we need to add DRM_RENDER_ALLOW to the ioctls, can you do that?

Dave.

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Stefan Hajnoczi @ 2015-09-11  7:48 UTC (permalink / raw)
  To: Ming Lin; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <1441906098.18716.21.camel@ssi>

On Thu, Sep 10, 2015 at 6:28 PM, Ming Lin <mlin@kernel.org> wrote:
> On Thu, 2015-09-10 at 15:38 +0100, Stefan Hajnoczi wrote:
>> On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
>> > These 2 patches added virtio-nvme to kernel and qemu,
>> > basically modified from virtio-blk and nvme code.
>> >
>> > As title said, request for your comments.
>> >
>> > Play it in Qemu with:
>> > -drive file=disk.img,format=raw,if=none,id=D22 \
>> > -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
>> >
>> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
>> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
>>
>> Why is a virtio-nvme guest device needed?  I guess there must either
>> be NVMe-only features that you want to pass through, or you think the
>> performance will be significantly better than virtio-blk/virtio-scsi?
>
> It simply passes through NVMe commands.

I understand that.  My question is why the guest needs to send NVMe commands?

If the virtio_nvme.ko guest driver only sends read/write/flush then
there's no advantage over virtio-blk.

There must be something you are trying to achieve which is not
possible with virtio-blk or virtio-scsi.  What is that?

Stefan

^ permalink raw reply

* Re: [PATCH 5/5] virtgpu: mark as a render gpu
From: Gerd Hoffmann @ 2015-09-11  9:16 UTC (permalink / raw)
  To: Dave Airlie
  Cc: Dave Airlie, Emil Velikov, open list, ML dri-devel,
	open list:VIRTIO GPU DRIVER
In-Reply-To: <CAPM=9tz_4NSXnmmT6EuXGWp5wUK0vZfUhk-zhnhQ6QTgtEuVfQ@mail.gmail.com>

On Fr, 2015-09-11 at 16:32 +1000, Dave Airlie wrote:
> doh, yes we need to add DRM_RENDER_ALLOW to the ioctls, can you do
> that?

Done.

cheers,
  Gerd

^ permalink raw reply

* [PATCH RFC 0/2] virtio-ccw: new status accessor in device
From: Cornelia Huck @ 2015-09-11 13:58 UTC (permalink / raw)
  To: virtio-dev, virtualization, qemu-devel

These patches implement the new read status command in qemu and bump
the revision to 2 (as this ccw is currently the only thing new with
that revision).

Patches are on top of v2 of "virtio-1/virtio-ccw related patches".

Pierre Morel (2):
  s390x/virtio-ccw: respond to READ_STATUS command
  s390x/virtio-ccw: set revision 2 as maximum revision number

 hw/s390x/virtio-ccw.c | 20 ++++++++++++++++++++
 hw/s390x/virtio-ccw.h |  3 ++-
 2 files changed, 22 insertions(+), 1 deletion(-)

-- 
2.3.8

^ permalink raw reply

* [PATCH RFC 1/2] s390x/virtio-ccw: respond to READ_STATUS command
From: Cornelia Huck @ 2015-09-11 13:58 UTC (permalink / raw)
  To: virtio-dev, virtualization, qemu-devel; +Cc: Pierre Morel
In-Reply-To: <1441979895-36990-1-git-send-email-cornelia.huck@de.ibm.com>

From: Pierre Morel <pmorel@linux.vnet.ibm.com>

This patch adds the response to the READ_STATUS CCW command.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c | 20 ++++++++++++++++++++
 hw/s390x/virtio-ccw.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index fb103b7..dcd0fff 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -586,6 +586,26 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
             }
         }
         break;
+    case CCW_CMD_READ_STATUS:
+        if (check_len) {
+            if (ccw.count != sizeof(status)) {
+                ret = -EINVAL;
+                break;
+            }
+        } else if (ccw.count < sizeof(status)) {
+            /* Can't execute command. */
+            ret = -EINVAL;
+            break;
+        }
+        if (!ccw.cda) {
+            ret = -EFAULT;
+        } else {
+            address_space_stb(&address_space_memory, ccw.cda, vdev->status,
+                                        MEMTXATTRS_UNSPECIFIED, NULL);
+            sch->curr_status.scsw.count = ccw.count - sizeof(vdev->status);;
+            ret = 0;
+        }
+        break;
     case CCW_CMD_WRITE_STATUS:
         if (check_len) {
             if (ccw.count != sizeof(status)) {
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 7ab8367..453fdf7 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -40,6 +40,7 @@
 #define CCW_CMD_SET_IND      0x43
 #define CCW_CMD_SET_CONF_IND 0x53
 #define CCW_CMD_READ_VQ_CONF 0x32
+#define CCW_CMD_READ_STATUS  0x72
 #define CCW_CMD_SET_IND_ADAPTER 0x73
 #define CCW_CMD_SET_VIRTIO_REV 0x83
 
-- 
2.3.8

^ permalink raw reply related

* [PATCH RFC 2/2] s390x/virtio-ccw: set revision 2 as maximum revision number
From: Cornelia Huck @ 2015-09-11 13:58 UTC (permalink / raw)
  To: virtio-dev, virtualization, qemu-devel; +Cc: Pierre Morel
In-Reply-To: <1441979895-36990-1-git-send-email-cornelia.huck@de.ibm.com>

From: Pierre Morel <pmorel@linux.vnet.ibm.com>

We have everything needed for virtio-ccw revision 2 wired up now. Bump
the maximum supported revision reported to the guest so they can make
use of it.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 453fdf7..da07bfa 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -104,7 +104,7 @@ struct VirtioCcwDevice {
 };
 
 /* The maximum virtio revision we support. */
-#define VIRTIO_CCW_MAX_REV 1
+#define VIRTIO_CCW_MAX_REV 2
 static inline int virtio_ccw_rev_max(VirtioCcwDevice *dev)
 {
     return dev->max_rev;
-- 
2.3.8

^ permalink raw reply related

* [PATCH RFC 0/1] virtio_ccw: new status accessor in driver
From: Cornelia Huck @ 2015-09-11 13:58 UTC (permalink / raw)
  To: virtio-dev, virtualization, qemu-devel

This patch (on top of "virtio/s390: handle failures of READ_VQ_CONF ccw")
implements the guest side of the new status ccw.

Pierre Morel (1):
  virtio/s390: support READ_STATUS command for virtio-ccw

 drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

-- 
2.3.8

^ permalink raw reply

* [PATCH RFC 1/1] virtio/s390: support READ_STATUS command for virtio-ccw
From: Cornelia Huck @ 2015-09-11 13:58 UTC (permalink / raw)
  To: virtio-dev, virtualization, qemu-devel; +Cc: Pierre Morel
In-Reply-To: <1441979907-37174-1-git-send-email-cornelia.huck@de.ibm.com>

From: Pierre Morel <pmorel@linux.vnet.ibm.com>

As virtio-1 introduced the possibility of the device manipulating the
status byte, revision 2 of the virtio-ccw transport introduced a means
of getting the status byte from the device via READ_STATUS. Let's wire
it up for revisions >= 2 and fall back to returning the stored status
byte if not supported.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 drivers/s390/virtio/virtio_ccw.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index e9fae30..1cda784 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -144,6 +144,7 @@ static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
 #define CCW_CMD_WRITE_CONF 0x21
 #define CCW_CMD_WRITE_STATUS 0x31
 #define CCW_CMD_READ_VQ_CONF 0x32
+#define CCW_CMD_READ_STATUS 0x72
 #define CCW_CMD_SET_IND_ADAPTER 0x73
 #define CCW_CMD_SET_VIRTIO_REV 0x83
 
@@ -159,6 +160,7 @@ static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
 #define VIRTIO_CCW_DOING_SET_CONF_IND 0x04000000
 #define VIRTIO_CCW_DOING_SET_IND_ADAPTER 0x08000000
 #define VIRTIO_CCW_DOING_SET_VIRTIO_REV 0x10000000
+#define VIRTIO_CCW_DOING_READ_STATUS 0x20000000
 #define VIRTIO_CCW_INTPARM_MASK 0xffff0000
 
 static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev)
@@ -889,6 +891,28 @@ out_free:
 static u8 virtio_ccw_get_status(struct virtio_device *vdev)
 {
 	struct virtio_ccw_device *vcdev = to_vc_device(vdev);
+	u8 old_status = *vcdev->status;
+	struct ccw1 *ccw;
+
+	if (vcdev->revision < 1)
+		return *vcdev->status;
+
+	ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
+	if (!ccw)
+		return old_status;
+
+	ccw->cmd_code = CCW_CMD_READ_STATUS;
+	ccw->flags = 0;
+	ccw->count = sizeof(*vcdev->status);
+	ccw->cda = (__u32)(unsigned long)vcdev->status;
+	ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_STATUS);
+/*
+ * If the channel program failed (should only happen if the device
+ * was hotunplugged, and then we clean up via the machine check
+ * handler anyway), vcdev->status was not overwritten and we just
+ * return the old status, which is fine.
+*/
+	kfree(ccw);
 
 	return *vcdev->status;
 }
@@ -1008,6 +1032,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
 		case VIRTIO_CCW_DOING_READ_CONFIG:
 		case VIRTIO_CCW_DOING_WRITE_CONFIG:
 		case VIRTIO_CCW_DOING_WRITE_STATUS:
+		case VIRTIO_CCW_DOING_READ_STATUS:
 		case VIRTIO_CCW_DOING_SET_VQ:
 		case VIRTIO_CCW_DOING_SET_IND:
 		case VIRTIO_CCW_DOING_SET_CONF_IND:
-- 
2.3.8

^ permalink raw reply related

* Re: rfc: vhost user enhancements for vm2vm communication
From: Claudio Fontana @ 2015-09-11 15:39 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: opnfv-tech-discuss, virtio-dev, Jan Kiszka, qemu-devel,
	virtualization
In-Reply-To: <20150909095258-mutt-send-email-mst@redhat.com>

On 09.09.2015 09:06, Michael S. Tsirkin wrote:
> On Mon, Sep 07, 2015 at 02:38:34PM +0200, Claudio Fontana wrote:
>> Coming late to the party, 
>>
>> On 31.08.2015 16:11, Michael S. Tsirkin wrote:
>>> Hello!
>>> During the KVM forum, we discussed supporting virtio on top
>>> of ivshmem. I have considered it, and came up with an alternative
>>> that has several advantages over that - please see below.
>>> Comments welcome.
>>
>> as Jan mentioned we actually discussed a virtio-shmem device which would incorporate the advantages of ivshmem (so no need for a separate ivshmem device), which would use the well known virtio interface, taking advantage of the new virtio-1 virtqueue layout to split r/w and read-only rings as seen from the two sides, and make use also of BAR0 which has been freed up for use by the device.
>>
>> This way it would be possible to share the rings and the actual memory for the buffers in the PCI bars. The guest VMs could decide to use the shared memory regions directly as prepared by the hypervisor (in the jailhouse case) or QEMU/KVM, or perform their own validation on the input depending on the use case.
>>
>> Of course the communication between VMs needs in this case to be pre-configured and is quite static (which is actually beneficial in our use case).
>>
>> But still in your proposed solution, each VM needs to be pre-configured to communicate with a specific other VM using a separate device right?
>>
>> But I wonder if we are addressing the same problem.. in your case you are looking at having a shared memory pool for all VMs potentially visible to all VMs (the vhost-user case), while in the virtio-shmem proposal we discussed we were assuming specific different regions for every channel.
>>
>> Ciao,
>>
>> Claudio
> 
> The problem, as I see it, is to allow inter-vm communication with
> polling (to get very low latencies) but polling within VMs only, without
> need to run a host thread (which when polling uses up a host CPU).
> 
> What was proposed was to simply change virtio to allow
> "offset within BAR" instead of PA.

There are many consequences to this, offset within BAR alone is not enough, there are multiple things at the virtio level that need sorting out.
Also we need to consider virtio-mmio etc.

> This would allow VM2VM communication if there are only 2 VMs,
> but if data needs to be sent to multiple VMs, you
> must copy it.

Not necessarily, however getting it to work (sharing the backend window and arbitrating the multicast) is really hard.

> 
> Additionally, it's a single-purpose feature: you can use it from
> a userspace PMD but linux will never use it.
> 
> 
> My proposal is a superset: don't require that BAR memory is
> used, use IOMMU translation tables.
> This way, data can be sent to multiple VMs by sharing the same
> memory with them all.

Can you describe in detail how your proposal deals with the arbitration necessary for multicast handling?

> 
> It is still possible to put data in some device BAR if that's
> what the guest wants to do: just program the IOMMU to limit
> virtio to the memory range that is within this BAR.
> 
> Another advantage here is that the feature is more generally useful.
> 
> 
>>>
>>> -----
>>>
>>> Existing solutions to userspace switching between VMs on the
>>> same host are vhost-user and ivshmem.
>>>
>>> vhost-user works by mapping memory of all VMs being bridged into the
>>> switch memory space.
>>>
>>> By comparison, ivshmem works by exposing a shared region of memory to all VMs.
>>> VMs are required to use this region to store packets. The switch only
>>> needs access to this region.
>>>
>>> Another difference between vhost-user and ivshmem surfaces when polling
>>> is used. With vhost-user, the switch is required to handle
>>> data movement between VMs, if using polling, this means that 1 host CPU
>>> needs to be sacrificed for this task.
>>>
>>> This is easiest to understand when one of the VMs is
>>> used with VF pass-through. This can be schematically shown below:
>>>
>>> +-- VM1 --------------+            +---VM2-----------+
>>> | virtio-pci          +-vhost-user-+ virtio-pci -- VF | -- VFIO -- IOMMU -- NIC
>>> +---------------------+            +-----------------+
>>>
>>>
>>> With ivshmem in theory communication can happen directly, with two VMs
>>> polling the shared memory region.
>>>
>>>
>>> I won't spend time listing advantages of vhost-user over ivshmem.
>>> Instead, having identified two advantages of ivshmem over vhost-user,
>>> below is a proposal to extend vhost-user to gain the advantages
>>> of ivshmem.
>>>
>>>
>>> 1: virtio in guest can be extended to allow support
>>> for IOMMUs. This provides guest with full flexibility
>>> about memory which is readable or write able by each device.
>>> By setting up a virtio device for each other VM we need to
>>> communicate to, guest gets full control of its security, from
>>> mapping all memory (like with current vhost-user) to only
>>> mapping buffers used for networking (like ivshmem) to
>>> transient mappings for the duration of data transfer only.
>>> This also allows use of VFIO within guests, for improved
>>> security.
>>>
>>> vhost user would need to be extended to send the
>>> mappings programmed by guest IOMMU.
>>>
>>> 2. qemu can be extended to serve as a vhost-user client:
>>> remote VM mappings over the vhost-user protocol, and
>>> map them into another VM's memory.
>>> This mapping can take, for example, the form of
>>> a BAR of a pci device, which I'll call here vhost-pci - 
>>> with bus address allowed
>>> by VM1's IOMMU mappings being translated into
>>> offsets within this BAR within VM2's physical
>>> memory space.
>>>
>>> Since the translation can be a simple one, VM2
>>> can perform it within its vhost-pci device driver.
>>>
>>> While this setup would be the most useful with polling,
>>> VM1's ioeventfd can also be mapped to
>>> another VM2's irqfd, and vice versa, such that VMs
>>> can trigger interrupts to each other without need
>>> for a helper thread on the host.
>>>
>>>
>>> The resulting channel might look something like the following:
>>>
>>> +-- VM1 --------------+  +---VM2-----------+
>>> | virtio-pci -- iommu +--+ vhost-pci -- VF | -- VFIO -- IOMMU -- NIC
>>> +---------------------+  +-----------------+
>>>
>>> comparing the two diagrams, a vhost-user thread on the host is
>>> no longer required, reducing the host CPU utilization when
>>> polling is active.  At the same time, VM2 can not access all of VM1's
>>> memory - it is limited by the iommu configuration setup by VM1.
>>>
>>>
>>> Advantages over ivshmem:
>>>
>>> - more flexibility, endpoint VMs do not have to place data at any
>>>   specific locations to use the device, in practice this likely
>>>   means less data copies.
>>> - better standardization/code reuse
>>>   virtio changes within guests would be fairly easy to implement
>>>   and would also benefit other backends, besides vhost-user
>>>   standard hotplug interfaces can be used to add and remove these
>>>   channels as VMs are added or removed.
>>> - migration support
>>>   It's easy to implement since ownership of memory is well defined.
>>>   For example, during migration VM2 can notify hypervisor of VM1
>>>   by updating dirty bitmap each time is writes into VM1 memory.
>>>
>>> Thanks,
>>>
>>
>>

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Ming Lin @ 2015-09-11 17:21 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <CAJSP0QXbjY5kbz0jTJSDyo=Zh9B+DRZorTY0m3bRC8yhJT=Nvw@mail.gmail.com>

On Fri, 2015-09-11 at 08:48 +0100, Stefan Hajnoczi wrote:
> On Thu, Sep 10, 2015 at 6:28 PM, Ming Lin <mlin@kernel.org> wrote:
> > On Thu, 2015-09-10 at 15:38 +0100, Stefan Hajnoczi wrote:
> >> On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
> >> > These 2 patches added virtio-nvme to kernel and qemu,
> >> > basically modified from virtio-blk and nvme code.
> >> >
> >> > As title said, request for your comments.
> >> >
> >> > Play it in Qemu with:
> >> > -drive file=disk.img,format=raw,if=none,id=D22 \
> >> > -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
> >> >
> >> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
> >> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
> >>
> >> Why is a virtio-nvme guest device needed?  I guess there must either
> >> be NVMe-only features that you want to pass through, or you think the
> >> performance will be significantly better than virtio-blk/virtio-scsi?
> >
> > It simply passes through NVMe commands.
> 
> I understand that.  My question is why the guest needs to send NVMe commands?
> 
> If the virtio_nvme.ko guest driver only sends read/write/flush then
> there's no advantage over virtio-blk.
> 
> There must be something you are trying to achieve which is not
> possible with virtio-blk or virtio-scsi.  What is that?

I actually learned from your virtio-scsi work.
http://www.linux-kvm.org/images/f/f5/2011-forum-virtio-scsi.pdf

Then I thought a full NVMe stack from guest to host to target seems
reasonable.

Trying to achieve similar things as virtio-scsi, but all NVMe protocol.

- Effective NVMe passthrough
- Multiple target choices: QEMU, LIO-NVMe(vhost_nvme)
- Almost unlimited scalability. Thousands of namespaces per PCI device
- True NVMe device
- End-to-end Protection Information
- ....

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: J Freyensee @ 2015-09-11 17:46 UTC (permalink / raw)
  To: Ming Lin, Keith Busch; +Cc: Christoph Hellwig, linux-nvme, virtualization
In-Reply-To: <1441904521.18716.4.camel@ssi>

On Thu, 2015-09-10 at 10:02 -0700, Ming Lin wrote:
> On Thu, 2015-09-10 at 14:02 +0000, Keith Busch wrote:
> > On Wed, 9 Sep 2015, Ming Lin wrote:
> > > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
> > > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
> > > 
> > > Now there are lots of duplicated code with linux/nvme-core.c and 
> > > qemu/nvme.c.
> > > The ideal result is to have a multi level NVMe stack(similar as 
> > > SCSI).
> > > So we can re-use the nvme code, for example
> > > 
> > >                        .-------------------------.
> > >                        | NVMe device register    |
> > >  Upper level           | NVMe protocol process   |
> > >                        |                         |
> > >                        '-------------------------'
> > > 
> > > 
> > > 
> > >              .-----------.    .-----------.    .-----------------
> > > -.
> > > Lower level  |   PCIe    |    | VIRTIO    |    |NVMe over Fabrics 
> > > |
> > >              |           |    |           |    |initiator        
> > >  |
> > >              '-----------'    '-----------'    '-----------------
> > > -'
> > > 
> > > todo:
> > > - tune performance. Should be as good as virtio-blk/virtio-scsi
> > > - support discard/flush/integrity
> > > - need Redhat's help for the VIRTIO_ID_NVME pci id
> > > - multi level NVMe stack
> > 
> > Hi Ming,
> 
> Hi Keith,
> 
> > 
> > I'll be out for travel for the next week, so I won't have much time 
> > to
> > do a proper review till the following week.
> > 
> > I think it'd be better to get this hierarchy setup to make the most 
> > reuse
> > possible than to have this much code duplication between the 
> > existing
> > driver and emulated qemu nvme. For better or worse, I think the 
> > generic
> > nvme layer is where things are going. Are you signed up with the 
> > fabrics
> > contributors?
> 
> No. How to sign up?

Ming,

Here is the email Keith sent out on this list that says how to sign up:

http://lists.infradead.org/pipermail/linux-nvme/2015
-September/002331.html

Jay



> 
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Stefan Hajnoczi @ 2015-09-11 17:53 UTC (permalink / raw)
  To: Ming Lin; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <1441992101.802.11.camel@ssi>

On Fri, Sep 11, 2015 at 6:21 PM, Ming Lin <mlin@kernel.org> wrote:
> On Fri, 2015-09-11 at 08:48 +0100, Stefan Hajnoczi wrote:
>> On Thu, Sep 10, 2015 at 6:28 PM, Ming Lin <mlin@kernel.org> wrote:
>> > On Thu, 2015-09-10 at 15:38 +0100, Stefan Hajnoczi wrote:
>> >> On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
>> >> > These 2 patches added virtio-nvme to kernel and qemu,
>> >> > basically modified from virtio-blk and nvme code.
>> >> >
>> >> > As title said, request for your comments.
>> >> >
>> >> > Play it in Qemu with:
>> >> > -drive file=disk.img,format=raw,if=none,id=D22 \
>> >> > -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
>> >> >
>> >> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
>> >> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
>> >>
>> >> Why is a virtio-nvme guest device needed?  I guess there must either
>> >> be NVMe-only features that you want to pass through, or you think the
>> >> performance will be significantly better than virtio-blk/virtio-scsi?
>> >
>> > It simply passes through NVMe commands.
>>
>> I understand that.  My question is why the guest needs to send NVMe commands?
>>
>> If the virtio_nvme.ko guest driver only sends read/write/flush then
>> there's no advantage over virtio-blk.
>>
>> There must be something you are trying to achieve which is not
>> possible with virtio-blk or virtio-scsi.  What is that?
>
> I actually learned from your virtio-scsi work.
> http://www.linux-kvm.org/images/f/f5/2011-forum-virtio-scsi.pdf
>
> Then I thought a full NVMe stack from guest to host to target seems
> reasonable.
>
> Trying to achieve similar things as virtio-scsi, but all NVMe protocol.
>
> - Effective NVMe passthrough
> - Multiple target choices: QEMU, LIO-NVMe(vhost_nvme)
> - Almost unlimited scalability. Thousands of namespaces per PCI device
> - True NVMe device
> - End-to-end Protection Information
> - ....

The advantages you mentioned are already available in virtio-scsi,
except for the NVMe command set.

I don't understand what unique problem virtio-nvme solves yet.  If
someone asked me to explain why NVMe-over-virtio makes sense compared
to the existing virtio-blk/virtio-scsi or NVMe SR-IOV options, I
wouldn't know the answer.  I'd like to learn that from you or anyone
else on CC.

Do you have a use case in mind?

Stefan

^ permalink raw reply

* Re: [RFC PATCH 0/2] virtio nvme
From: Ming Lin @ 2015-09-11 18:54 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Christoph Hellwig, linux-nvme, Linux Virtualization
In-Reply-To: <CAJSP0QXz_w+FasdZ2zZsVmjuBmJLgAnrG5vV+c2eLr=pdnRm5Q@mail.gmail.com>

On Fri, Sep 11, 2015 at 10:53 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Fri, Sep 11, 2015 at 6:21 PM, Ming Lin <mlin@kernel.org> wrote:
>> On Fri, 2015-09-11 at 08:48 +0100, Stefan Hajnoczi wrote:
>>> On Thu, Sep 10, 2015 at 6:28 PM, Ming Lin <mlin@kernel.org> wrote:
>>> > On Thu, 2015-09-10 at 15:38 +0100, Stefan Hajnoczi wrote:
>>> >> On Thu, Sep 10, 2015 at 6:48 AM, Ming Lin <mlin@kernel.org> wrote:
>>> >> > These 2 patches added virtio-nvme to kernel and qemu,
>>> >> > basically modified from virtio-blk and nvme code.
>>> >> >
>>> >> > As title said, request for your comments.
>>> >> >
>>> >> > Play it in Qemu with:
>>> >> > -drive file=disk.img,format=raw,if=none,id=D22 \
>>> >> > -device virtio-nvme-pci,drive=D22,serial=1234,num_queues=4
>>> >> >
>>> >> > The goal is to have a full NVMe stack from VM guest(virtio-nvme)
>>> >> > to host(vhost_nvme) to LIO NVMe-over-fabrics target.
>>> >>
>>> >> Why is a virtio-nvme guest device needed?  I guess there must either
>>> >> be NVMe-only features that you want to pass through, or you think the
>>> >> performance will be significantly better than virtio-blk/virtio-scsi?
>>> >
>>> > It simply passes through NVMe commands.
>>>
>>> I understand that.  My question is why the guest needs to send NVMe commands?
>>>
>>> If the virtio_nvme.ko guest driver only sends read/write/flush then
>>> there's no advantage over virtio-blk.
>>>
>>> There must be something you are trying to achieve which is not
>>> possible with virtio-blk or virtio-scsi.  What is that?
>>
>> I actually learned from your virtio-scsi work.
>> http://www.linux-kvm.org/images/f/f5/2011-forum-virtio-scsi.pdf
>>
>> Then I thought a full NVMe stack from guest to host to target seems
>> reasonable.
>>
>> Trying to achieve similar things as virtio-scsi, but all NVMe protocol.
>>
>> - Effective NVMe passthrough
>> - Multiple target choices: QEMU, LIO-NVMe(vhost_nvme)
>> - Almost unlimited scalability. Thousands of namespaces per PCI device
>> - True NVMe device
>> - End-to-end Protection Information
>> - ....
>
> The advantages you mentioned are already available in virtio-scsi,
> except for the NVMe command set.
>
> I don't understand what unique problem virtio-nvme solves yet.  If
> someone asked me to explain why NVMe-over-virtio makes sense compared
> to the existing virtio-blk/virtio-scsi or NVMe SR-IOV options, I
> wouldn't know the answer.  I'd like to learn that from you or anyone
> else on CC.
>
> Do you have a use case in mind?

One use case is for All NVMe storage array. There is no SCSI device at all.

Samsung demoed one in flash memory summit 2015.
http://www.tomsitpro.com/articles/best-of-flash-memory-summit,1-2806.html

>
> Stefan

^ permalink raw reply

* Re: rfc: vhost user enhancements for vm2vm communication
From: Michael S. Tsirkin @ 2015-09-13  9:12 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: opnfv-tech-discuss, virtio-dev, Jan Kiszka, qemu-devel,
	virtualization
In-Reply-To: <55F2F59B.4030401@huawei.com>

On Fri, Sep 11, 2015 at 05:39:07PM +0200, Claudio Fontana wrote:
> On 09.09.2015 09:06, Michael S. Tsirkin wrote:
> > On Mon, Sep 07, 2015 at 02:38:34PM +0200, Claudio Fontana wrote:
> >> Coming late to the party, 
> >>
> >> On 31.08.2015 16:11, Michael S. Tsirkin wrote:
> >>> Hello!
> >>> During the KVM forum, we discussed supporting virtio on top
> >>> of ivshmem. I have considered it, and came up with an alternative
> >>> that has several advantages over that - please see below.
> >>> Comments welcome.
> >>
> >> as Jan mentioned we actually discussed a virtio-shmem device which would incorporate the advantages of ivshmem (so no need for a separate ivshmem device), which would use the well known virtio interface, taking advantage of the new virtio-1 virtqueue layout to split r/w and read-only rings as seen from the two sides, and make use also of BAR0 which has been freed up for use by the device.
> >>
> >> This way it would be possible to share the rings and the actual memory for the buffers in the PCI bars. The guest VMs could decide to use the shared memory regions directly as prepared by the hypervisor (in the jailhouse case) or QEMU/KVM, or perform their own validation on the input depending on the use case.
> >>
> >> Of course the communication between VMs needs in this case to be pre-configured and is quite static (which is actually beneficial in our use case).
> >>
> >> But still in your proposed solution, each VM needs to be pre-configured to communicate with a specific other VM using a separate device right?
> >>
> >> But I wonder if we are addressing the same problem.. in your case you are looking at having a shared memory pool for all VMs potentially visible to all VMs (the vhost-user case), while in the virtio-shmem proposal we discussed we were assuming specific different regions for every channel.
> >>
> >> Ciao,
> >>
> >> Claudio
> > 
> > The problem, as I see it, is to allow inter-vm communication with
> > polling (to get very low latencies) but polling within VMs only, without
> > need to run a host thread (which when polling uses up a host CPU).
> > 
> > What was proposed was to simply change virtio to allow
> > "offset within BAR" instead of PA.
> 
> There are many consequences to this, offset within BAR alone is not enough, there are multiple things at the virtio level that need sorting out.
> Also we need to consider virtio-mmio etc.
> 
> > This would allow VM2VM communication if there are only 2 VMs,
> > but if data needs to be sent to multiple VMs, you
> > must copy it.
> 
> Not necessarily, however getting it to work (sharing the backend window and arbitrating the multicast) is really hard.
> 
> > 
> > Additionally, it's a single-purpose feature: you can use it from
> > a userspace PMD but linux will never use it.
> > 
> > 
> > My proposal is a superset: don't require that BAR memory is
> > used, use IOMMU translation tables.
> > This way, data can be sent to multiple VMs by sharing the same
> > memory with them all.
> 
> Can you describe in detail how your proposal deals with the arbitration necessary for multicast handling?

Basically it falls out naturally. Consider linux guest as an example,
and assume dynamic mappings for simplicity.

Multicast is done by a bridge on the guest side. That code clones the
skb (reference-counting page fragments) and passes it to multiple ports.
Each of these will program the IOMMU to allow read access to the
fragments to the relevant device.



> > 
> > It is still possible to put data in some device BAR if that's
> > what the guest wants to do: just program the IOMMU to limit
> > virtio to the memory range that is within this BAR.
> > 
> > Another advantage here is that the feature is more generally useful.
> > 
> > 
> >>>
> >>> -----
> >>>
> >>> Existing solutions to userspace switching between VMs on the
> >>> same host are vhost-user and ivshmem.
> >>>
> >>> vhost-user works by mapping memory of all VMs being bridged into the
> >>> switch memory space.
> >>>
> >>> By comparison, ivshmem works by exposing a shared region of memory to all VMs.
> >>> VMs are required to use this region to store packets. The switch only
> >>> needs access to this region.
> >>>
> >>> Another difference between vhost-user and ivshmem surfaces when polling
> >>> is used. With vhost-user, the switch is required to handle
> >>> data movement between VMs, if using polling, this means that 1 host CPU
> >>> needs to be sacrificed for this task.
> >>>
> >>> This is easiest to understand when one of the VMs is
> >>> used with VF pass-through. This can be schematically shown below:
> >>>
> >>> +-- VM1 --------------+            +---VM2-----------+
> >>> | virtio-pci          +-vhost-user-+ virtio-pci -- VF | -- VFIO -- IOMMU -- NIC
> >>> +---------------------+            +-----------------+
> >>>
> >>>
> >>> With ivshmem in theory communication can happen directly, with two VMs
> >>> polling the shared memory region.
> >>>
> >>>
> >>> I won't spend time listing advantages of vhost-user over ivshmem.
> >>> Instead, having identified two advantages of ivshmem over vhost-user,
> >>> below is a proposal to extend vhost-user to gain the advantages
> >>> of ivshmem.
> >>>
> >>>
> >>> 1: virtio in guest can be extended to allow support
> >>> for IOMMUs. This provides guest with full flexibility
> >>> about memory which is readable or write able by each device.
> >>> By setting up a virtio device for each other VM we need to
> >>> communicate to, guest gets full control of its security, from
> >>> mapping all memory (like with current vhost-user) to only
> >>> mapping buffers used for networking (like ivshmem) to
> >>> transient mappings for the duration of data transfer only.
> >>> This also allows use of VFIO within guests, for improved
> >>> security.
> >>>
> >>> vhost user would need to be extended to send the
> >>> mappings programmed by guest IOMMU.
> >>>
> >>> 2. qemu can be extended to serve as a vhost-user client:
> >>> remote VM mappings over the vhost-user protocol, and
> >>> map them into another VM's memory.
> >>> This mapping can take, for example, the form of
> >>> a BAR of a pci device, which I'll call here vhost-pci - 
> >>> with bus address allowed
> >>> by VM1's IOMMU mappings being translated into
> >>> offsets within this BAR within VM2's physical
> >>> memory space.
> >>>
> >>> Since the translation can be a simple one, VM2
> >>> can perform it within its vhost-pci device driver.
> >>>
> >>> While this setup would be the most useful with polling,
> >>> VM1's ioeventfd can also be mapped to
> >>> another VM2's irqfd, and vice versa, such that VMs
> >>> can trigger interrupts to each other without need
> >>> for a helper thread on the host.
> >>>
> >>>
> >>> The resulting channel might look something like the following:
> >>>
> >>> +-- VM1 --------------+  +---VM2-----------+
> >>> | virtio-pci -- iommu +--+ vhost-pci -- VF | -- VFIO -- IOMMU -- NIC
> >>> +---------------------+  +-----------------+
> >>>
> >>> comparing the two diagrams, a vhost-user thread on the host is
> >>> no longer required, reducing the host CPU utilization when
> >>> polling is active.  At the same time, VM2 can not access all of VM1's
> >>> memory - it is limited by the iommu configuration setup by VM1.
> >>>
> >>>
> >>> Advantages over ivshmem:
> >>>
> >>> - more flexibility, endpoint VMs do not have to place data at any
> >>>   specific locations to use the device, in practice this likely
> >>>   means less data copies.
> >>> - better standardization/code reuse
> >>>   virtio changes within guests would be fairly easy to implement
> >>>   and would also benefit other backends, besides vhost-user
> >>>   standard hotplug interfaces can be used to add and remove these
> >>>   channels as VMs are added or removed.
> >>> - migration support
> >>>   It's easy to implement since ownership of memory is well defined.
> >>>   For example, during migration VM2 can notify hypervisor of VM1
> >>>   by updating dirty bitmap each time is writes into VM1 memory.
> >>>
> >>> Thanks,
> >>>
> >>
> >>

^ permalink raw reply

* RE: [opnfv-tech-discuss] rfc: vhost user enhancements for vm2vm communication
From: Zhang, Yang Z @ 2015-09-14  0:43 UTC (permalink / raw)
  To: Michael S. Tsirkin, Claudio Fontana
  Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	opnfv-tech-discuss@lists.opnfv.org, Jan Kiszka
In-Reply-To: <20150913120821-mutt-send-email-mst@redhat.com>

Michael S. Tsirkin wrote on 2015-09-13:
> On Fri, Sep 11, 2015 at 05:39:07PM +0200, Claudio Fontana wrote:
>> On 09.09.2015 09:06, Michael S. Tsirkin wrote:
>> 
>> There are many consequences to this, offset within BAR alone is not
>> enough, there are multiple things at the virtio level that need sorting
>> out. Also we need to consider virtio-mmio etc.
>> 
>>> This would allow VM2VM communication if there are only 2 VMs, but
>>> if data needs to be sent to multiple VMs, you must copy it.
>> 
>> Not necessarily, however getting it to work (sharing the backend window
>> and arbitrating the multicast) is really hard.
>> 
>>> 
>>> Additionally, it's a single-purpose feature: you can use it from a
>>> userspace PMD but linux will never use it.
>>> 
>>> 
>>> My proposal is a superset: don't require that BAR memory is used,
>>> use IOMMU translation tables.
>>> This way, data can be sent to multiple VMs by sharing the same
>>> memory with them all.
>> 
>> Can you describe in detail how your proposal deals with the
>> arbitration
> necessary for multicast handling?
> 
> Basically it falls out naturally. Consider linux guest as an example,
> and assume dynamic mappings for simplicity.
> 
> Multicast is done by a bridge on the guest side. That code clones the
> skb (reference-counting page fragments) and passes it to multiple ports.
> Each of these will program the IOMMU to allow read access to the
> fragments to the relevant device.

How to work with vswitch in host side like OVS? Since the flow table is inside host, but guest cannot see it.

Best regards,
Yang

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox