* [PATCH] vhost-scsi: remove incorrect memory barrier
From: Paolo Bonzini @ 2019-04-16 8:56 UTC (permalink / raw)
To: linux-kernel; +Cc: Andrea Parri, virtualization, stefanha, mst
At this point, vs_tpg is not public at all; tv_tpg_vhost_count
is accessed under tpg->tv_tpg_mutex; tpg->vhost_scsi is
accessed under vhost_scsi_mutex. Therefor there are no atomic
operations involved at all here, just remove the barrier.
Reported-by: Andrea Parri <andrea.parri@amarulasolutions.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
drivers/vhost/scsi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 618fb6461017..c090d177bd75 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1443,7 +1443,6 @@ static void vhost_scsi_flush(struct vhost_scsi *vs)
tpg->tv_tpg_vhost_count++;
tpg->vhost_scsi = vs;
vs_tpg[tpg->tport_tpgt] = tpg;
- smp_mb__after_atomic();
match = true;
}
mutex_unlock(&tpg->tv_tpg_mutex);
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC 3/3] RDMA/virtio-rdma: VirtIO rdma driver
From: Bart Van Assche @ 2019-04-16 1:07 UTC (permalink / raw)
To: Yuval Shaia, virtualization, qemu-devel, mst, cohuck,
marcel.apfelbaum, linux-rdma, jgg
In-Reply-To: <20190411110157.14252-4-yuval.shaia@oracle.com>
On 4/11/19 4:01 AM, Yuval Shaia wrote:
> +++ b/drivers/infiniband/hw/virtio/Kconfig
> @@ -0,0 +1,6 @@
> +config INFINIBAND_VIRTIO_RDMA
> + tristate "VirtIO Paravirtualized RDMA Driver"
> + depends on NETDEVICES && ETHERNET && PCI && INET
> + ---help---
> + This driver provides low-level support for VirtIO Paravirtual
> + RDMA adapter.
Does this driver really depend on Ethernet, or does it also work with
Ethernet support disabled?
> +static inline struct virtio_rdma_info *to_vdev(struct ib_device *ibdev)
> +{
> + return container_of(ibdev, struct virtio_rdma_info, ib_dev);
> +}
Is it really worth to introduce this function? Have you considered to
use container_of(ibdev, struct virtio_rdma_info, ib_dev) directly instead
of to_vdev()?
> +static void rdma_ctrl_ack(struct virtqueue *vq)
> +{
> + struct virtio_rdma_info *dev = vq->vdev->priv;
> +
> + wake_up(&dev->acked);
> +
> + printk("%s\n", __func__);
> +}
Should that printk() be changed into pr_debug()? The same comment holds for
all other printk() calls.
> +#define VIRTIO_RDMA_BOARD_ID 1
> +#define VIRTIO_RDMA_HW_NAME "virtio-rdma"
> +#define VIRTIO_RDMA_HW_REV 1
> +#define VIRTIO_RDMA_DRIVER_VER "1.0"
Is a driver version number useful in an upstream driver?
> +struct ib_cq *virtio_rdma_create_cq(struct ib_device *ibdev,
> + const struct ib_cq_init_attr *attr,
> + struct ib_ucontext *context,
> + struct ib_udata *udata)
> +{
> + struct scatterlist in, out;
> + struct virtio_rdma_ib_cq *vcq;
> + struct cmd_create_cq *cmd;
> + struct rsp_create_cq *rsp;
> + struct ib_cq *cq = NULL;
> + int rc;
> +
> + /* TODO: Check MAX_CQ */
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd)
> + return ERR_PTR(-ENOMEM);
> +
> + rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
> + if (!rsp) {
> + kfree(cmd);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + vcq = kzalloc(sizeof(*vcq), GFP_KERNEL);
> + if (!vcq)
> + goto out;
Are you sure that you want to mix GFP_ATOMIC and GFP_KERNEL in a single
function?
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: Daniel Vetter @ 2019-04-15 19:17 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied@linux.ie, puck.chen@hisilicon.com, Zhang, Jerry,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, zourongrong@gmail.com,
Koenig, Christian
In-Reply-To: <b9ba305f-8140-faf9-3954-464d9f38852a@suse.de>
On Mon, Apr 15, 2019 at 6:21 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 15.04.19 um 17:54 schrieb Daniel Vetter:
> > On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote:
> >> Hi
> >>
> >> Am 09.04.19 um 09:12 schrieb kraxel@redhat.com:
> >>> Hi,
> >>>
> >>>> If not for TTM, what would be the alternative? One VMA manager per
> >>>> memory region per device?
> >>>
> >>> Depends pretty much on the device.
> >>>
> >>> The cirrus is a display device with only 4 MB of vram. You can't fit
> >>> much in there. A single 1024x768 @ 24bpp framebuffer needs more 50%
> >>> of the video memory already. Which is why the cirrus driver (before the
> >>> rewrite) had to migrate buffers from/to vram on every page flip[1]. Which
> >>> is one[2] of the reasons why cirrus (after rewrite) doesn't ttm-manage the
> >>> vram any more. gem objects are managed with the shmem helpers instead
> >>> and the active framebuffer is blitted to vram.
> >>>
> >>> The qemu stdvga (bochs driver) has 16 MB vram by default and can be
> >>> configured to have up to 256 MB. Plenty of room even for multiple 4k
> >>> framebuffers if needed. So for the bochs driver all the ttm bo
> >>> migration logic is not needed, it could just store everything in vram.
> >>>
> >>> A set of drm_gem_vram_* helpers would do the job for bochs.
> >>
> >> Thanks for clarifying. drm_gem_vram_* (and drm_vram_mm for Simple TTM)
> >> is probably a better name for the data structures.
> >
> > +1 on drm_gem_vram_* naming convention - we want to describe what it's
> > for, not how it's implemented.
>
> OK, great.
>
> >>> I'd expect the same applies to the vbox driver.
> >>>
> >>> Dunno about the other drm drivers and the fbdev drivers you plan to
> >>> migrate over.
> >>
> >> The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic
> >> for a server. It's similar with mgag200 HW. The old fbdev-supported
> >> device are all somewhere in the range between cirrus and bochs. Some
> >> drivers would probably benefit from the cirrus approach, some could use
> >> VRAM directly.
> >
> > I think for dumb scanout with vram all we need is:
> > - pin framebuffers, which potentially moves the underlying bo into vram
> > - unpin framebuffers (which is just accounting, we don't want to move the
> > bo on every flip!)
> > - if a pin doesn't find enough space, move one of the unpinned bo still
> > resident in vram out
>
> For dumb buffers, I'd expect userspace to have a working set of only a
> front and back buffer (plus maybe a third one). This working set has to
> reside in VRAM for performance reasons; non-WS BOs from other userspace
> programs don't have to be.
>
> So we could simplify even more: if there's not enough free space in
> vram, remove all unpinned BO's. This would avoid the need to implement
> an LRU algorithm or another eviction strategy. Userspace with a WS
> larger than the absolute VRAM would see degraded performance but
> otherwise still work.
You still need a list of unpinned bo, and the lru scan algorithm is
just a few lines of code more than unpinning everything. Plus it'd be
a neat example of the drm_mm scan logic. Given that some folks might
think that not having lru evict si a problem and get them to type
their own, I'd just add it. But up to you. Plus with ttm you get it no
matter what.
-Daniel
> Best regards
> Thomas
>
> > - no pipelining, no support for dma engines (it's all cpu copies anway)
> > - a simple drm_mm should be good enough to manage the vram, no need for
> > the ttm style abstraction over how memory is manged
> > - also just bake in the lru eviction list and algorithm
> > - probably good to have built-in support for redirecting the mmap between
> > shmem and iomem.
> > - anything that needs pipelining or copy engines would be out of scope for
> > these helpers
> >
> > I think for starting points we can go with a copypasted version of the
> > various ttm implementations we already have, and then simplify from there
> > as needed. Or just start fresh if that's too complicated, due to the issue
> > Christian highlighted.
> > -Daniel
> >
> >> Best regards
> >> Thomas
> >>
> >>>
> >>> cheers,
> >>> Gerd
> >>>
> >>> [1] Note: The page-flip migration logic is present in some of the other
> >>> drivers too, not sure whenever they actually need that due to being low
> >>> on vram too or whenever they just copied the old cirrus code ...
> >>>
> >>> [2] The other reason is that this allow to convert formats at blit time,
> >>> which helps to deal with some cirrus hardware limitations.
> >>>
> >>
> >> --
> >> Thomas Zimmermann
> >> Graphics Driver Developer
> >> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> >> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> >> HRB 21284 (AG Nürnberg)
> >>
> >
> >
> >
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: Daniel Vetter @ 2019-04-15 15:57 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied@linux.ie, puck.chen@hisilicon.com,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, daniel@ffwll.ch,
zourongrong@gmail.com, Zhang, Jerry, Koenig, Christian
In-Reply-To: <20190415155430.GB2665@phenom.ffwll.local>
On Mon, Apr 15, 2019 at 05:54:30PM +0200, Daniel Vetter wrote:
> On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 09.04.19 um 09:12 schrieb kraxel@redhat.com:
> > > Hi,
> > >
> > >> If not for TTM, what would be the alternative? One VMA manager per
> > >> memory region per device?
> > >
> > > Depends pretty much on the device.
> > >
> > > The cirrus is a display device with only 4 MB of vram. You can't fit
> > > much in there. A single 1024x768 @ 24bpp framebuffer needs more 50%
> > > of the video memory already. Which is why the cirrus driver (before the
> > > rewrite) had to migrate buffers from/to vram on every page flip[1]. Which
> > > is one[2] of the reasons why cirrus (after rewrite) doesn't ttm-manage the
> > > vram any more. gem objects are managed with the shmem helpers instead
> > > and the active framebuffer is blitted to vram.
> > >
> > > The qemu stdvga (bochs driver) has 16 MB vram by default and can be
> > > configured to have up to 256 MB. Plenty of room even for multiple 4k
> > > framebuffers if needed. So for the bochs driver all the ttm bo
> > > migration logic is not needed, it could just store everything in vram.
> > >
> > > A set of drm_gem_vram_* helpers would do the job for bochs.
> >
> > Thanks for clarifying. drm_gem_vram_* (and drm_vram_mm for Simple TTM)
> > is probably a better name for the data structures.
>
> +1 on drm_gem_vram_* naming convention - we want to describe what it's
> for, not how it's implemented.
>
> > > I'd expect the same applies to the vbox driver.
> > >
> > > Dunno about the other drm drivers and the fbdev drivers you plan to
> > > migrate over.
> >
> > The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic
> > for a server. It's similar with mgag200 HW. The old fbdev-supported
> > device are all somewhere in the range between cirrus and bochs. Some
> > drivers would probably benefit from the cirrus approach, some could use
> > VRAM directly.
>
> I think for dumb scanout with vram all we need is:
> - pin framebuffers, which potentially moves the underlying bo into vram
> - unpin framebuffers (which is just accounting, we don't want to move the
> bo on every flip!)
> - if a pin doesn't find enough space, move one of the unpinned bo still
> resident in vram out
> - no pipelining, no support for dma engines (it's all cpu copies anway)
> - a simple drm_mm should be good enough to manage the vram, no need for
> the ttm style abstraction over how memory is manged
> - also just bake in the lru eviction list and algorithm
> - probably good to have built-in support for redirecting the mmap between
> shmem and iomem.
> - anything that needs pipelining or copy engines would be out of scope for
> these helpers
One more:
- Only bother supporting hw that needs contiguous scanout buffers in VRAM.
I think hw that has pagetables for scanout (and everything else really)
is sufficiently different that cramming them into the same helpers
doesn't make much sense: You want to manage your VRAM with a buddy
allocator at a page level, plus you need to manage your pagetables, and
all is likely very hw specific anyway. Plus I haven't seen such hw that
doesn't come with a real gpu attached :-)
-Daniel
>
> I think for starting points we can go with a copypasted version of the
> various ttm implementations we already have, and then simplify from there
> as needed. Or just start fresh if that's too complicated, due to the issue
> Christian highlighted.
> -Daniel
>
> > Best regards
> > Thomas
> >
> > >
> > > cheers,
> > > Gerd
> > >
> > > [1] Note: The page-flip migration logic is present in some of the other
> > > drivers too, not sure whenever they actually need that due to being low
> > > on vram too or whenever they just copied the old cirrus code ...
> > >
> > > [2] The other reason is that this allow to convert formats at blit time,
> > > which helps to deal with some cirrus hardware limitations.
> > >
> >
> > --
> > Thomas Zimmermann
> > Graphics Driver Developer
> > SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> > HRB 21284 (AG Nürnberg)
> >
>
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH 00/15] Share TTM code among framebuffer drivers
From: Daniel Vetter @ 2019-04-15 15:54 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: airlied@linux.ie, puck.chen@hisilicon.com,
dri-devel@lists.freedesktop.org,
virtualization@lists.linux-foundation.org,
z.liuxinliang@hisilicon.com, hdegoede@redhat.com,
kong.kongxinwei@hisilicon.com, Huang, Ray, daniel@ffwll.ch,
zourongrong@gmail.com, Zhang, Jerry, Koenig, Christian
In-Reply-To: <a2f778c3-904b-5ee2-215e-df534f14b928@suse.de>
On Tue, Apr 09, 2019 at 09:50:40AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 09.04.19 um 09:12 schrieb kraxel@redhat.com:
> > Hi,
> >
> >> If not for TTM, what would be the alternative? One VMA manager per
> >> memory region per device?
> >
> > Depends pretty much on the device.
> >
> > The cirrus is a display device with only 4 MB of vram. You can't fit
> > much in there. A single 1024x768 @ 24bpp framebuffer needs more 50%
> > of the video memory already. Which is why the cirrus driver (before the
> > rewrite) had to migrate buffers from/to vram on every page flip[1]. Which
> > is one[2] of the reasons why cirrus (after rewrite) doesn't ttm-manage the
> > vram any more. gem objects are managed with the shmem helpers instead
> > and the active framebuffer is blitted to vram.
> >
> > The qemu stdvga (bochs driver) has 16 MB vram by default and can be
> > configured to have up to 256 MB. Plenty of room even for multiple 4k
> > framebuffers if needed. So for the bochs driver all the ttm bo
> > migration logic is not needed, it could just store everything in vram.
> >
> > A set of drm_gem_vram_* helpers would do the job for bochs.
>
> Thanks for clarifying. drm_gem_vram_* (and drm_vram_mm for Simple TTM)
> is probably a better name for the data structures.
+1 on drm_gem_vram_* naming convention - we want to describe what it's
for, not how it's implemented.
> > I'd expect the same applies to the vbox driver.
> >
> > Dunno about the other drm drivers and the fbdev drivers you plan to
> > migrate over.
>
> The AST HW can support up to 512 MiB, but 32-64 MiB seems more realistic
> for a server. It's similar with mgag200 HW. The old fbdev-supported
> device are all somewhere in the range between cirrus and bochs. Some
> drivers would probably benefit from the cirrus approach, some could use
> VRAM directly.
I think for dumb scanout with vram all we need is:
- pin framebuffers, which potentially moves the underlying bo into vram
- unpin framebuffers (which is just accounting, we don't want to move the
bo on every flip!)
- if a pin doesn't find enough space, move one of the unpinned bo still
resident in vram out
- no pipelining, no support for dma engines (it's all cpu copies anway)
- a simple drm_mm should be good enough to manage the vram, no need for
the ttm style abstraction over how memory is manged
- also just bake in the lru eviction list and algorithm
- probably good to have built-in support for redirecting the mmap between
shmem and iomem.
- anything that needs pipelining or copy engines would be out of scope for
these helpers
I think for starting points we can go with a copypasted version of the
various ttm implementations we already have, and then simplify from there
as needed. Or just start fresh if that's too complicated, due to the issue
Christian highlighted.
-Daniel
> Best regards
> Thomas
>
> >
> > cheers,
> > Gerd
> >
> > [1] Note: The page-flip migration logic is present in some of the other
> > drivers too, not sure whenever they actually need that due to being low
> > on vram too or whenever they just copied the old cirrus code ...
> >
> > [2] The other reason is that this allow to convert formats at blit time,
> > which helps to deal with some cirrus hardware limitations.
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
> HRB 21284 (AG Nürnberg)
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-15 10:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: mst, linux-rdma, qemu-devel, virtualization, jgg
In-Reply-To: <20190411190215.2163572e.cohuck@redhat.com>
On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> On Thu, 11 Apr 2019 14:01:54 +0300
> Yuval Shaia <yuval.shaia@oracle.com> wrote:
>
> > Data center backends use more and more RDMA or RoCE devices and more and
> > more software runs in virtualized environment.
> > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> >
> > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > technology and also because the Virtio specification
> > allows Hardware Vendors to support Virtio protocol natively in order to
> > achieve bare metal performance.
> >
> > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > Virtio Specification and a look forward on possible implementation
> > techniques.
> >
> > Open issues/Todo list:
> > List is huge, this is only start point of the project.
> > Anyway, here is one example of item in the list:
> > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > that this is reasonable so one option is to have one for all and
> > multiplex the traffic on it. This is not good approach as by design it
> > introducing an optional starvation. Another approach would be multi
> > queues and round-robin (for example) between them.
> >
> > Expectations from this posting:
> > In general, any comment is welcome, starting from hey, drop this as it is a
> > very bad idea, to yeah, go ahead, we really want it.
> > Idea here is that since it is not a minor effort i first want to know if
> > there is some sort interest in the community for such device.
>
> My first reaction is: Sounds sensible, but it would be good to have a
> spec for this :)
>
> You'll need a spec if you want this to go forward anyway, so at least a
> sketch would be good to answer questions such as how many virtqueues
> you use for which purpose, what is actually put on the virtqueues,
> whether there are negotiable features, and what the expectations for
> the device and the driver are. It also makes it easier to understand
> how this is supposed to work in practice.
>
> If folks agree that this sounds useful, the next step would be to
> reserve an id for the device type.
Thanks for the tips, will sure do that, it is that first i wanted to make
sure there is a use case here.
Waiting for any feedback from the community.
>
> >
> > The scope of the implementation is limited to probing the device and doing
> > some basic ibverbs commands. Data-path is not yet implemented. So with this
> > one can expect only that driver is (partialy) loaded and basic queries and
> > resource allocation is done.
> >
> > One note regarding the patchset.
> > I know it is not standard to collaps patches from several repos as i did
> > here (qemu and linux) but decided to do it anyway so the whole picture can
> > be seen.
> >
> > patch 1: virtio-net: Move some virtio-net-pci decl to include/hw/virtio
> > This is a prelimenary patch just as a hack so i will not need to
> > impelement new netdev
> > patch 2: hw/virtio-rdma: VirtIO rdma device
> > The implementation of the device
> > patch 3: RDMA/virtio-rdma: VirtIO rdma driver
> > The device driver
> >
>
^ permalink raw reply
* Re: [Qemu-devel] [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-15 10:27 UTC (permalink / raw)
To: Devesh Sharma
Cc: mst@redhat.com, linux-rdma@vger.kernel.org, Cornelia Huck,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org,
Jason Gunthorpe
In-Reply-To: <CANjDDBj0rqZEmHzMH+2461_DvjV5K4hT=hJ_usBuucV4Xwh84g@mail.gmail.com>
On Fri, Apr 12, 2019 at 03:21:56PM +0530, Devesh Sharma wrote:
> On Thu, Apr 11, 2019 at 11:11 PM Yuval Shaia <yuval.shaia@oracle.com> wrote:
> >
> > On Thu, Apr 11, 2019 at 08:34:20PM +0300, Yuval Shaia wrote:
> > > On Thu, Apr 11, 2019 at 05:24:08PM +0000, Jason Gunthorpe wrote:
> > > > On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> > > > > On Thu, 11 Apr 2019 14:01:54 +0300
> > > > > Yuval Shaia <yuval.shaia@oracle.com> wrote:
> > > > >
> > > > > > Data center backends use more and more RDMA or RoCE devices and more and
> > > > > > more software runs in virtualized environment.
> > > > > > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > > > > >
> > > > > > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > > > > > technology and also because the Virtio specification
> > > > > > allows Hardware Vendors to support Virtio protocol natively in order to
> > > > > > achieve bare metal performance.
> > > > > >
> > > > > > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > > > > > Virtio Specification and a look forward on possible implementation
> > > > > > techniques.
> > > > > >
> > > > > > Open issues/Todo list:
> > > > > > List is huge, this is only start point of the project.
> > > > > > Anyway, here is one example of item in the list:
> > > > > > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > > > > > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > > > > > that this is reasonable so one option is to have one for all and
> > > > > > multiplex the traffic on it. This is not good approach as by design it
> > > > > > introducing an optional starvation. Another approach would be multi
> > > > > > queues and round-robin (for example) between them.
> > > > > >
> > > > > > Expectations from this posting:
> > > > > > In general, any comment is welcome, starting from hey, drop this as it is a
> > > > > > very bad idea, to yeah, go ahead, we really want it.
> > > > > > Idea here is that since it is not a minor effort i first want to know if
> > > > > > there is some sort interest in the community for such device.
> > > > >
> > > > > My first reaction is: Sounds sensible, but it would be good to have a
> > > > > spec for this :)
> > > >
> > > > I'm unclear why you'd want to have a virtio queue for anything other
> > > > that some kind of command channel.
> > > >
> > > > I'm not sure a QP or CQ benefits from this??
> > >
> > > Virtqueue is a standard mechanism to pass data from guest to host. By
> >
> > And vice versa (CQ?)
> >
> > > saying that - it really sounds like QP send and recv rings. So my thought
> > > is to use a standard way for rings. As i've learned this is how it is used
> > > by other virtio devices ex virtio-net.
> > >
> > > >
> > > > Jason
> > >
> I would like to ask more basic question, how virtio queue will glue
> with actual h/w qps? I may be to naive though.
Have to admit - I have no idea.
This work is based on emulated device so i'm my case - the emulated device
is creating the virtqueue. I guess that HW device will create a QP and
expose a virtqueue interface to it.
The same driver should serve both the SW and HW devices.
One of the objectives of this RFC is to collaborate an effort and
implementation notes/ideas from HW vendors.
>
> -Regards
> Devesh
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-15 10:04 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: mst@redhat.com, linux-rdma@vger.kernel.org, Cornelia Huck,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org
In-Reply-To: <20190411174022.GA26227@mellanox.com>
On Thu, Apr 11, 2019 at 05:40:26PM +0000, Jason Gunthorpe wrote:
> On Thu, Apr 11, 2019 at 08:34:20PM +0300, Yuval Shaia wrote:
> > On Thu, Apr 11, 2019 at 05:24:08PM +0000, Jason Gunthorpe wrote:
> > > On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> > > > On Thu, 11 Apr 2019 14:01:54 +0300
> > > > Yuval Shaia <yuval.shaia@oracle.com> wrote:
> > > >
> > > > > Data center backends use more and more RDMA or RoCE devices and more and
> > > > > more software runs in virtualized environment.
> > > > > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > > > >
> > > > > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > > > > technology and also because the Virtio specification
> > > > > allows Hardware Vendors to support Virtio protocol natively in order to
> > > > > achieve bare metal performance.
> > > > >
> > > > > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > > > > Virtio Specification and a look forward on possible implementation
> > > > > techniques.
> > > > >
> > > > > Open issues/Todo list:
> > > > > List is huge, this is only start point of the project.
> > > > > Anyway, here is one example of item in the list:
> > > > > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > > > > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > > > > that this is reasonable so one option is to have one for all and
> > > > > multiplex the traffic on it. This is not good approach as by design it
> > > > > introducing an optional starvation. Another approach would be multi
> > > > > queues and round-robin (for example) between them.
> > > > >
> > > > > Expectations from this posting:
> > > > > In general, any comment is welcome, starting from hey, drop this as it is a
> > > > > very bad idea, to yeah, go ahead, we really want it.
> > > > > Idea here is that since it is not a minor effort i first want to know if
> > > > > there is some sort interest in the community for such device.
> > > >
> > > > My first reaction is: Sounds sensible, but it would be good to have a
> > > > spec for this :)
> > >
> > > I'm unclear why you'd want to have a virtio queue for anything other
> > > that some kind of command channel.
> > >
> > > I'm not sure a QP or CQ benefits from this??
> >
> > Virtqueue is a standard mechanism to pass data from guest to host. By
> > saying that - it really sounds like QP send and recv rings. So my thought
> > is to use a standard way for rings. As i've learned this is how it is used
> > by other virtio devices ex virtio-net.
>
> I doubt you can use virtio queues from userspace securely? Usually
> needs a dedicated page for each user space process
Not yet started to do any work on datapath, i guess you are right but need
further work on this area.
Thanks for raising the concern.
As i said, there are many open issues at this stage.
>
> Jason
^ permalink raw reply
* Re: [RFC 3/3] RDMA/virtio-rdma: VirtIO rdma driver
From: Yuval Shaia @ 2019-04-14 5:20 UTC (permalink / raw)
To: Yanjun Zhu; +Cc: mst, linux-rdma, cohuck, qemu-devel, virtualization, jgg
In-Reply-To: <199a70a8-5926-66af-d2d7-b19a0a0f46a2@oracle.com>
> > +
> > + wake_up(&dev->acked);
> > +
> > + printk("%s\n", __func__);
>
> Cool:-)
>
> this line should be for debug?
Yes
>
> Zhu Yanjun
>
^ permalink raw reply
* Re: [RFC 3/3] RDMA/virtio-rdma: VirtIO rdma driver
From: Yanjun Zhu @ 2019-04-13 7:58 UTC (permalink / raw)
To: Yuval Shaia, virtualization, qemu-devel, mst, cohuck,
marcel.apfelbaum, linux-rdma, jgg
In-Reply-To: <20190411110157.14252-4-yuval.shaia@oracle.com>
On 2019/4/11 19:01, Yuval Shaia wrote:
> Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
> ---
> drivers/infiniband/Kconfig | 1 +
> drivers/infiniband/hw/Makefile | 1 +
> drivers/infiniband/hw/virtio/Kconfig | 6 +
> drivers/infiniband/hw/virtio/Makefile | 4 +
> drivers/infiniband/hw/virtio/virtio_rdma.h | 40 +
> .../infiniband/hw/virtio/virtio_rdma_device.c | 59 ++
> .../infiniband/hw/virtio/virtio_rdma_device.h | 32 +
> drivers/infiniband/hw/virtio/virtio_rdma_ib.c | 711 ++++++++++++++++++
> drivers/infiniband/hw/virtio/virtio_rdma_ib.h | 48 ++
> .../infiniband/hw/virtio/virtio_rdma_main.c | 149 ++++
> .../infiniband/hw/virtio/virtio_rdma_netdev.c | 44 ++
> .../infiniband/hw/virtio/virtio_rdma_netdev.h | 33 +
> include/uapi/linux/virtio_ids.h | 1 +
> 13 files changed, 1129 insertions(+)
> create mode 100644 drivers/infiniband/hw/virtio/Kconfig
> create mode 100644 drivers/infiniband/hw/virtio/Makefile
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma.h
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_device.c
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_device.h
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_ib.c
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_ib.h
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_main.c
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
> create mode 100644 drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
>
> diff --git a/drivers/infiniband/Kconfig b/drivers/infiniband/Kconfig
> index a1fb840de45d..218a47d4cecf 100644
> --- a/drivers/infiniband/Kconfig
> +++ b/drivers/infiniband/Kconfig
> @@ -107,6 +107,7 @@ source "drivers/infiniband/hw/hfi1/Kconfig"
> source "drivers/infiniband/hw/qedr/Kconfig"
> source "drivers/infiniband/sw/rdmavt/Kconfig"
> source "drivers/infiniband/sw/rxe/Kconfig"
> +source "drivers/infiniband/hw/virtio/Kconfig"
> endif
>
> source "drivers/infiniband/ulp/ipoib/Kconfig"
> diff --git a/drivers/infiniband/hw/Makefile b/drivers/infiniband/hw/Makefile
> index e4f31c1be8f7..10ffb2c421e4 100644
> --- a/drivers/infiniband/hw/Makefile
> +++ b/drivers/infiniband/hw/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_INFINIBAND_HFI1) += hfi1/
> obj-$(CONFIG_INFINIBAND_HNS) += hns/
> obj-$(CONFIG_INFINIBAND_QEDR) += qedr/
> obj-$(CONFIG_INFINIBAND_BNXT_RE) += bnxt_re/
> +obj-$(CONFIG_INFINIBAND_VIRTIO_RDMA) += virtio/
> diff --git a/drivers/infiniband/hw/virtio/Kconfig b/drivers/infiniband/hw/virtio/Kconfig
> new file mode 100644
> index 000000000000..92e41691cf5d
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/Kconfig
> @@ -0,0 +1,6 @@
> +config INFINIBAND_VIRTIO_RDMA
> + tristate "VirtIO Paravirtualized RDMA Driver"
> + depends on NETDEVICES && ETHERNET && PCI && INET
> + ---help---
> + This driver provides low-level support for VirtIO Paravirtual
> + RDMA adapter.
> diff --git a/drivers/infiniband/hw/virtio/Makefile b/drivers/infiniband/hw/virtio/Makefile
> new file mode 100644
> index 000000000000..fb637e467167
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/Makefile
> @@ -0,0 +1,4 @@
> +obj-$(CONFIG_INFINIBAND_VIRTIO_RDMA) += virtio_rdma.o
> +
> +virtio_rdma-y := virtio_rdma_main.o virtio_rdma_device.o virtio_rdma_ib.o \
> + virtio_rdma_netdev.o
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma.h b/drivers/infiniband/hw/virtio/virtio_rdma.h
> new file mode 100644
> index 000000000000..7896a2dfb812
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma.h
> @@ -0,0 +1,40 @@
> +/*
> + * Virtio RDMA device: Driver main data types
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef __VIRTIO_RDMA__
> +#define __VIRTIO_RDMA__
> +
> +#include <linux/virtio.h>
> +#include <rdma/ib_verbs.h>
> +
> +struct virtio_rdma_info {
> + struct ib_device ib_dev;
> + struct virtio_device *vdev;
> + struct virtqueue *ctrl_vq;
> + wait_queue_head_t acked; /* arm on send to host, release on recv */
> + struct net_device *netdev;
> +};
> +
> +static inline struct virtio_rdma_info *to_vdev(struct ib_device *ibdev)
> +{
> + return container_of(ibdev, struct virtio_rdma_info, ib_dev);
> +}
> +
> +#endif
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_device.c b/drivers/infiniband/hw/virtio/virtio_rdma_device.c
> new file mode 100644
> index 000000000000..ae41e530644f
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_device.c
> @@ -0,0 +1,59 @@
> +/*
> + * Virtio RDMA device: Device related functions and data
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/virtio_config.h>
> +
> +#include "virtio_rdma.h"
> +
> +static void rdma_ctrl_ack(struct virtqueue *vq)
> +{
> + struct virtio_rdma_info *dev = vq->vdev->priv;
> +
> + wake_up(&dev->acked);
> +
> + printk("%s\n", __func__);
Cool:-)
this line should be for debug?
Zhu Yanjun
> +}
> +
> +int init_device(struct virtio_rdma_info *dev)
> +{
> +#define TMP_MAX_VQ 1
> + int rc;
> + struct virtqueue *vqs[TMP_MAX_VQ];
> + vq_callback_t *cbs[TMP_MAX_VQ];
> + const char *names[TMP_MAX_VQ];
> +
> + names[0] = "ctrl";
> + cbs[0] = rdma_ctrl_ack;
> + cbs[0] = NULL;
> +
> + rc = virtio_find_vqs(dev->vdev, TMP_MAX_VQ, vqs, cbs, names, NULL);
> + if (rc)
> + return rc;
> +
> + dev->ctrl_vq = vqs[0];
> +
> + return 0;
> +}
> +
> +void fini_device(struct virtio_rdma_info *dev)
> +{
> + dev->vdev->config->reset(dev->vdev);
> + dev->vdev->config->del_vqs(dev->vdev);
> +}
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_device.h b/drivers/infiniband/hw/virtio/virtio_rdma_device.h
> new file mode 100644
> index 000000000000..d9b1240daf92
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_device.h
> @@ -0,0 +1,32 @@
> +/*
> + * Virtio RDMA device: Device related functions and data
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef __VIRTIO_RDMA_DEVICE__
> +#define __VIRTIO_RDMA_DEVICE__
> +
> +#define VIRTIO_RDMA_BOARD_ID 1
> +#define VIRTIO_RDMA_HW_NAME "virtio-rdma"
> +#define VIRTIO_RDMA_HW_REV 1
> +#define VIRTIO_RDMA_DRIVER_VER "1.0"
> +
> +int init_device(struct virtio_rdma_info *dev);
> +void fini_device(struct virtio_rdma_info *dev);
> +
> +#endif
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_ib.c b/drivers/infiniband/hw/virtio/virtio_rdma_ib.c
> new file mode 100644
> index 000000000000..02bf4a332611
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_ib.c
> @@ -0,0 +1,711 @@
> +/*
> + * Virtio RDMA device: IB related functions and data
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/scatterlist.h>
> +#include <linux/virtio.h>
> +#include <rdma/ib_mad.h>
> +
> +#include "virtio_rdma.h"
> +#include "virtio_rdma_device.h"
> +#include "virtio_rdma_ib.h"
> +
> +/* TODO: Move to uapi header file */
> +
> +/*
> + * Control virtqueue data structures
> + *
> + * The control virtqueue expects a header in the first sg entry
> + * and an ack/status response in the last entry. Data for the
> + * command goes in between.
> + */
> +
> +#define VIRTIO_RDMA_CTRL_OK 0
> +#define VIRTIO_RDMA_CTRL_ERR 1
> +
> +struct control_buf {
> + __u8 cmd;
> + __u8 status;
> +};
> +
> +enum {
> + VIRTIO_CMD_QUERY_DEVICE = 10,
> + VIRTIO_CMD_QUERY_PORT,
> + VIRTIO_CMD_CREATE_CQ,
> + VIRTIO_CMD_DESTROY_CQ,
> + VIRTIO_CMD_CREATE_PD,
> + VIRTIO_CMD_DESTROY_PD,
> + VIRTIO_CMD_GET_DMA_MR,
> +};
> +
> +struct cmd_query_port {
> + __u8 port;
> +};
> +
> +struct cmd_create_cq {
> + __u32 cqe;
> +};
> +
> +struct rsp_create_cq {
> + __u32 cqn;
> +};
> +
> +struct cmd_destroy_cq {
> + __u32 cqn;
> +};
> +
> +struct rsp_create_pd {
> + __u32 pdn;
> +};
> +
> +struct cmd_destroy_pd {
> + __u32 pdn;
> +};
> +
> +struct cmd_get_dma_mr {
> + __u32 pdn;
> + __u32 access_flags;
> +};
> +
> +struct rsp_get_dma_mr {
> + __u32 mrn;
> + __u32 lkey;
> + __u32 rkey;
> +};
> +
> +/* TODO: Move to uapi header file */
> +
> +struct virtio_rdma_ib_cq {
> + struct ib_cq ibcq;
> + u32 cq_handle;
> +};
> +
> +/* TODO: For the scope fof the RFC i'm utilizing ib*_*_attr structures */
> +
> +static int virtio_rdma_exec_cmd(struct virtio_rdma_info *di, int cmd,
> + struct scatterlist *in, struct scatterlist *out)
> +{
> + struct scatterlist *sgs[4], hdr, status;
> + struct control_buf *ctrl;
> + unsigned tmp;
> + int rc;
> +
> + ctrl = kmalloc(sizeof(*ctrl), GFP_ATOMIC);
> + ctrl->cmd = cmd;
> + ctrl->status = ~0;
> +
> + sg_init_one(&hdr, &ctrl->cmd, sizeof(ctrl->cmd));
> + sgs[0] = &hdr;
> + sgs[1] = in;
> + sgs[2] = out;
> + sg_init_one(&status, &ctrl->status, sizeof(ctrl->status));
> + sgs[3] = &status;
> +
> + rc = virtqueue_add_sgs(di->ctrl_vq, sgs, 2, 2, di, GFP_ATOMIC);
> + if (rc)
> + goto out;
> +
> + if (unlikely(!virtqueue_kick(di->ctrl_vq))) {
> + goto out_with_status;
> + }
> +
> + /* 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(di->ctrl_vq, &tmp) &&
> + !virtqueue_is_broken(di->ctrl_vq))
> + cpu_relax();
> +
> +out_with_status:
> + printk("%s: cmd %d, status %d\n", __func__, ctrl->cmd, ctrl->status);
> + rc = ctrl->status == VIRTIO_RDMA_CTRL_OK ? 0 : 1;
> +
> +out:
> + kfree(ctrl);
> +
> + return rc;
> +}
> +
> +static int virtio_rdma_port_immutable(struct ib_device *ibdev, u8 port_num,
> + struct ib_port_immutable *immutable)
> +{
> + struct ib_port_attr attr;
> + int rc;
> +
> + rc = ib_query_port(ibdev, port_num, &attr);
> + if (rc)
> + return rc;
> +
> + immutable->core_cap_flags |= RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
> + immutable->pkey_tbl_len = attr.pkey_tbl_len;
> + immutable->gid_tbl_len = attr.gid_tbl_len;
> + immutable->max_mad_size = IB_MGMT_MAD_SIZE;
> +
> + return 0;
> +}
> +
> +static int virtio_rdma_query_device(struct ib_device *ibdev,
> + struct ib_device_attr *props,
> + struct ib_udata *uhw)
> +{
> + struct scatterlist data;
> + int offs;
> + int rc;
> +
> + if (uhw->inlen || uhw->outlen)
> + return -EINVAL;
> +
> + /* We start with sys_image_guid because of inconsistency beween ib_
> + * and ibv_ */
> + offs = offsetof(struct ib_device_attr, sys_image_guid);
> + sg_init_one(&data, (void *)props + offs, sizeof(*props) - offs);
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_QUERY_DEVICE, NULL,
> + &data);
> +
> + printk("%s: sys_image_guid 0x%llx\n", __func__,
> + be64_to_cpu(props->sys_image_guid));
> +
> + return rc;
> +}
> +
> +static int virtio_rdma_query_port(struct ib_device *ibdev, u8 port,
> + struct ib_port_attr *props)
> +{
> + struct scatterlist in, out;
> + struct cmd_query_port *cmd;
> + int offs;
> + int rc;
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd)
> + return -ENOMEM;
> +
> + /* We start with state because of inconsistency beween ib and ibv */
> + offs = offsetof(struct ib_port_attr, state);
> + sg_init_one(&out, (void *)props + offs, sizeof(*props) - offs);
> +
> + cmd->port = port;
> + sg_init_one(&in, cmd, sizeof(*cmd));
> + printk("%s: port %d\n", __func__, cmd->port);
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_QUERY_PORT, &in,
> + &out);
> +
> + printk("%s: gid_tbl_len %d\n", __func__, props->gid_tbl_len);
> +
> + kfree(cmd);
> +
> + return rc;
> +}
> +
> +static struct net_device *virtio_rdma_get_netdev(struct ib_device *ibdev,
> + u8 port_num)
> +{
> + struct virtio_rdma_info *ri = to_vdev(ibdev);
> +
> + printk("%s:\n", __func__);
> +
> + return ri->netdev;
> +}
> +
> +struct ib_cq *virtio_rdma_create_cq(struct ib_device *ibdev,
> + const struct ib_cq_init_attr *attr,
> + struct ib_ucontext *context,
> + struct ib_udata *udata)
> +{
> + struct scatterlist in, out;
> + struct virtio_rdma_ib_cq *vcq;
> + struct cmd_create_cq *cmd;
> + struct rsp_create_cq *rsp;
> + struct ib_cq *cq = NULL;
> + int rc;
> +
> + /* TODO: Check MAX_CQ */
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd)
> + return ERR_PTR(-ENOMEM);
> +
> + rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
> + if (!rsp) {
> + kfree(cmd);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + vcq = kzalloc(sizeof(*vcq), GFP_KERNEL);
> + if (!vcq)
> + goto out;
> +
> + cmd->cqe = attr->cqe;
> + sg_init_one(&in, cmd, sizeof(*cmd));
> + printk("%s: cqe %d\n", __func__, cmd->cqe);
> +
> + sg_init_one(&out, rsp, sizeof(*rsp));
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_CREATE_CQ, &in,
> + &out);
> + if (rc)
> + goto out_err;
> +
> + printk("%s: cqn 0x%x\n", __func__, rsp->cqn);
> + vcq->cq_handle = rsp->cqn;
> + vcq->ibcq.cqe = attr->cqe;
> + cq = &vcq->ibcq;
> +
> + goto out;
> +
> +out_err:
> + kfree(vcq);
> + return ERR_PTR(rc);
> +
> +out:
> + kfree(rsp);
> + kfree(cmd);
> + return cq;
> +}
> +
> +int virtio_rdma_destroy_cq(struct ib_cq *cq)
> +{
> + struct virtio_rdma_ib_cq *vcq;
> + struct scatterlist in;
> + struct cmd_destroy_cq *cmd;
> + int rc;
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd)
> + return -ENOMEM;
> +
> + vcq = container_of(cq, struct virtio_rdma_ib_cq, ibcq);
> +
> + cmd->cqn = vcq->cq_handle;
> + sg_init_one(&in, cmd, sizeof(*cmd));
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(cq->device), VIRTIO_CMD_DESTROY_CQ,
> + &in, NULL);
> +
> + kfree(cmd);
> +
> + kfree(vcq);
> +
> + return rc;
> +}
> +
> +int virtio_rdma_alloc_pd(struct ib_pd *ibpd, struct ib_ucontext *context,
> + struct ib_udata *udata)
> +{
> + struct virtio_rdma_pd *pd = to_vpd(ibpd);
> + struct ib_device *ibdev = ibpd->device;
> + struct rsp_create_pd *rsp;
> + struct scatterlist out;
> + int rc;
> +
> + /* TODO: Check MAX_PD */
> +
> + rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
> + if (!rsp)
> + return -ENOMEM;
> +
> + sg_init_one(&out, rsp, sizeof(*rsp));
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_CREATE_PD, NULL,
> + &out);
> + if (rc)
> + goto out;
> +
> + pd->pd_handle = rsp->pdn;
> +
> + printk("%s: pd_handle=%d\n", __func__, pd->pd_handle);
> +
> +out:
> + kfree(rsp);
> +
> + printk("%s: rc=%d\n", __func__, rc);
> + return rc;
> +}
> +
> +void virtio_rdma_dealloc_pd(struct ib_pd *pd)
> +{
> + struct virtio_rdma_pd *vpd = to_vpd(pd);
> + struct ib_device *ibdev = pd->device;
> + struct cmd_destroy_pd *cmd;
> + struct scatterlist in;
> +
> + printk("%s:\n", __func__);
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd)
> + return;
> +
> + cmd->pdn = vpd->pd_handle;
> + sg_init_one(&in, cmd, sizeof(*cmd));
> +
> + virtio_rdma_exec_cmd(to_vdev(ibdev), VIRTIO_CMD_DESTROY_PD, &in, NULL);
> +
> + kfree(cmd);
> +}
> +
> +struct ib_mr *virtio_rdma_get_dma_mr(struct ib_pd *pd, int acc)
> +
> +{
> + struct virtio_rdma_user_mr *mr;
> + struct scatterlist in, out;
> + struct cmd_get_dma_mr *cmd = NULL;
> + struct rsp_get_dma_mr *rsp = NULL;
> + int rc;
> +
> + mr = kzalloc(sizeof(*mr), GFP_KERNEL);
> + if (!mr)
> + return ERR_PTR(-ENOMEM);
> +
> + cmd = kmalloc(sizeof(*cmd), GFP_ATOMIC);
> + if (!cmd) {
> + kfree(mr);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + rsp = kmalloc(sizeof(*rsp), GFP_ATOMIC);
> + if (!cmd) {
> + kfree(mr);
> + kfree(cmd);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + cmd->pdn = to_vpd(pd)->pd_handle;
> + cmd->access_flags = acc;
> + sg_init_one(&in, cmd, sizeof(*cmd));
> +
> + sg_init_one(&out, rsp, sizeof(*rsp));
> +
> + rc = virtio_rdma_exec_cmd(to_vdev(pd->device), VIRTIO_CMD_GET_DMA_MR,
> + &in, &out);
> + if (rc) {
> + kfree(mr);
> + kfree(cmd);
> + return ERR_PTR(rc);
> + }
> +
> + mr->mr_handle = rsp->mrn;
> + mr->ibmr.lkey = rsp->lkey;
> + mr->ibmr.rkey = rsp->rkey;
> +
> + printk("%s: mr_handle=0x%x\n", __func__, mr->mr_handle);
> +
> + kfree(cmd);
> + kfree(rsp);
> +
> + return &mr->ibmr;
> +}
> +
> +struct ib_qp *virtio_rdma_create_qp(struct ib_pd *pd,
> + struct ib_qp_init_attr *init_attr,
> + struct ib_udata *udata)
> +{
> + /* struct pvrdma_dev *dev = to_vdev(pd->device); */
> + struct virtio_rdma_qp *qp;
> +
> + printk("%s:\n", __func__);
> +
> + qp = kzalloc(sizeof(*qp), GFP_KERNEL);
> + if (!qp)
> + return ERR_PTR(-ENOMEM);
> +
> + return &qp->ibqp;
> +}
> +
> +int virtio_rdma_query_gid(struct ib_device *ibdev, u8 port, int index,
> + union ib_gid *gid)
> +{
> + memset(gid, 0, sizeof(union ib_gid));
> +
> + printk("%s: port %d, index %d\n", __func__, port, index);
> +
> + return 0;
> +}
> +
> +static int virtio_rdma_add_gid(const struct ib_gid_attr *attr, void **context)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +struct ib_mr *virtio_rdma_alloc_mr(struct ib_pd *pd, enum ib_mr_type mr_type,
> + u32 max_num_sg)
> +{
> + printk("%s: mr_type %d, max_num_sg %d\n", __func__, mr_type,
> + max_num_sg);
> +
> + return NULL;
> +}
> +
> +int virtio_rdma_alloc_ucontext(struct ib_ucontext *uctx, struct ib_udata *udata)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +struct ib_ah *virtio_rdma_create_ah(struct ib_pd *pd,
> + struct rdma_ah_attr *ah_attr, u32 flags,
> + struct ib_udata *udata)
> +{
> + printk("%s:\n", __func__);
> +
> + return NULL;
> +}
> +
> +void virtio_rdma_dealloc_ucontext(struct ib_ucontext *ibcontext)
> +
> +{
> +}
> +
> +static int virtio_rdma_del_gid(const struct ib_gid_attr *attr, void **context)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_dereg_mr(struct ib_mr *ibmr)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_destroy_ah(struct ib_ah *ah, u32 flags)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +struct virtio_rdma_cq {
> + struct ib_cq ibcq;
> +};
> +
> +int virtio_rdma_destroy_qp(struct ib_qp *qp)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +static void virtio_rdma_get_fw_ver_str(struct ib_device *device, char *str)
> +{
> + printk("%s:\n", __func__);
> +}
> +
> +enum rdma_link_layer virtio_rdma_port_link_layer(struct ib_device *ibdev,
> + u8 port)
> +{
> + return IB_LINK_LAYER_ETHERNET;
> +}
> +
> +int virtio_rdma_map_mr_sg(struct ib_mr *ibmr, struct scatterlist *sg,
> + int sg_nents, unsigned int *sg_offset)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_mmap(struct ib_ucontext *ibcontext, struct vm_area_struct *vma)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_modify_port(struct ib_device *ibdev, u8 port, int mask,
> + struct ib_port_modify *props)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
> + int attr_mask, struct ib_udata *udata)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *wr,
> + const struct ib_recv_wr **bad_wr)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_post_send(struct ib_qp *ibqp, const struct ib_send_wr *wr,
> + const struct ib_send_wr **bad_wr)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
> + u16 *pkey)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +int virtio_rdma_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
> + int attr_mask, struct ib_qp_init_attr *init_attr)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +struct ib_mr *virtio_rdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> + u64 virt_addr, int access_flags,
> + struct ib_udata *udata)
> +{
> + printk("%s:\n", __func__);
> +
> + return NULL;
> +}
> +
> +int virtio_rdma_req_notify_cq(struct ib_cq *ibcq,
> + enum ib_cq_notify_flags notify_flags)
> +{
> + printk("%s:\n", __func__);
> +
> + return 0;
> +}
> +
> +static const struct ib_device_ops virtio_rdma_dev_ops = {
> + .get_port_immutable = virtio_rdma_port_immutable,
> + .query_device = virtio_rdma_query_device,
> + .query_port = virtio_rdma_query_port,
> + .get_netdev = virtio_rdma_get_netdev,
> + .create_cq = virtio_rdma_create_cq,
> + .destroy_cq = virtio_rdma_destroy_cq,
> + .alloc_pd = virtio_rdma_alloc_pd,
> + .dealloc_pd = virtio_rdma_dealloc_pd,
> + .get_dma_mr = virtio_rdma_get_dma_mr,
> + .create_qp = virtio_rdma_create_qp,
> + .query_gid = virtio_rdma_query_gid,
> + .add_gid = virtio_rdma_add_gid,
> + .alloc_mr = virtio_rdma_alloc_mr,
> + .alloc_ucontext = virtio_rdma_alloc_ucontext,
> + .create_ah = virtio_rdma_create_ah,
> + .dealloc_ucontext = virtio_rdma_dealloc_ucontext,
> + .del_gid = virtio_rdma_del_gid,
> + .dereg_mr = virtio_rdma_dereg_mr,
> + .destroy_ah = virtio_rdma_destroy_ah,
> + .destroy_qp = virtio_rdma_destroy_qp,
> + .get_dev_fw_str = virtio_rdma_get_fw_ver_str,
> + .get_link_layer = virtio_rdma_port_link_layer,
> + .get_port_immutable = virtio_rdma_port_immutable,
> + .map_mr_sg = virtio_rdma_map_mr_sg,
> + .mmap = virtio_rdma_mmap,
> + .modify_port = virtio_rdma_modify_port,
> + .modify_qp = virtio_rdma_modify_qp,
> + .poll_cq = virtio_rdma_poll_cq,
> + .post_recv = virtio_rdma_post_recv,
> + .post_send = virtio_rdma_post_send,
> + .query_device = virtio_rdma_query_device,
> + .query_pkey = virtio_rdma_query_pkey,
> + .query_port = virtio_rdma_query_port,
> + .query_qp = virtio_rdma_query_qp,
> + .reg_user_mr = virtio_rdma_reg_user_mr,
> + .req_notify_cq = virtio_rdma_req_notify_cq,
> + INIT_RDMA_OBJ_SIZE(ib_pd, virtio_rdma_pd, ibpd),
> +};
> +
> +static ssize_t hca_type_show(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%s-%s\n", VIRTIO_RDMA_HW_NAME,
> + VIRTIO_RDMA_DRIVER_VER);
> +}
> +static DEVICE_ATTR_RO(hca_type);
> +
> +static ssize_t hw_rev_show(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", VIRTIO_RDMA_HW_REV);
> +}
> +static DEVICE_ATTR_RO(hw_rev);
> +
> +static ssize_t board_id_show(struct device *device,
> + struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", VIRTIO_RDMA_BOARD_ID);
> +}
> +static DEVICE_ATTR_RO(board_id);
> +
> +static struct attribute *virtio_rdmaa_class_attributes[] = {
> + &dev_attr_hw_rev.attr,
> + &dev_attr_hca_type.attr,
> + &dev_attr_board_id.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group virtio_rdmaa_attr_group = {
> + .attrs = virtio_rdmaa_class_attributes,
> +};
> +
> +int init_ib(struct virtio_rdma_info *ri)
> +{
> + int rc;
> +
> + ri->ib_dev.owner = THIS_MODULE;
> + ri->ib_dev.num_comp_vectors = 1;
> + ri->ib_dev.dev.parent = &ri->vdev->dev;
> + ri->ib_dev.node_type = RDMA_NODE_IB_CA;
> + ri->ib_dev.phys_port_cnt = 1;
> + ri->ib_dev.uverbs_cmd_mask =
> + (1ull << IB_USER_VERBS_CMD_QUERY_DEVICE) |
> + (1ull << IB_USER_VERBS_CMD_QUERY_PORT) |
> + (1ull << IB_USER_VERBS_CMD_CREATE_CQ) |
> + (1ull << IB_USER_VERBS_CMD_DESTROY_CQ) |
> + (1ull << IB_USER_VERBS_CMD_ALLOC_PD) |
> + (1ull << IB_USER_VERBS_CMD_DEALLOC_PD);
> +
> + rdma_set_device_sysfs_group(&ri->ib_dev, &virtio_rdmaa_attr_group);
> +
> + ib_set_device_ops(&ri->ib_dev, &virtio_rdma_dev_ops);
> +
> + rc = ib_register_device(&ri->ib_dev, "virtio_rdma%d");
> +
> + return rc;
> +}
> +
> +void fini_ib(struct virtio_rdma_info *ri)
> +{
> + ib_unregister_device(&ri->ib_dev);
> +}
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_ib.h b/drivers/infiniband/hw/virtio/virtio_rdma_ib.h
> new file mode 100644
> index 000000000000..7b82a60581ff
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_ib.h
> @@ -0,0 +1,48 @@
> +/*
> + * Virtio RDMA device: IB related functions and data
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef __VIRTIO_RDMA_IB__
> +#define __VIRTIO_RDMA_IB__
> +
> +#include <rdma/ib_verbs.h>
> +
> +struct virtio_rdma_pd {
> + struct ib_pd ibpd;
> + u32 pd_handle;
> +};
> +
> +struct virtio_rdma_user_mr {
> + struct ib_mr ibmr;
> + u32 mr_handle;
> +};
> +
> +struct virtio_rdma_qp {
> + struct ib_qp ibqp;
> +};
> +
> +static inline struct virtio_rdma_pd *to_vpd(struct ib_pd *ibpd)
> +{
> + return container_of(ibpd, struct virtio_rdma_pd, ibpd);
> +}
> +
> +int init_ib(struct virtio_rdma_info *ri);
> +void fini_ib(struct virtio_rdma_info *ri);
> +
> +#endif
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_main.c b/drivers/infiniband/hw/virtio/virtio_rdma_main.c
> new file mode 100644
> index 000000000000..811533d63160
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_main.c
> @@ -0,0 +1,149 @@
> +/*
> + * Virtio RDMA device
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/err.h>
> +#include <linux/scatterlist.h>
> +#include <linux/spinlock.h>
> +#include <linux/virtio.h>
> +#include <linux/module.h>
> +#include <uapi/linux/virtio_ids.h>
> +
> +#include "virtio_rdma.h"
> +#include "virtio_rdma_device.h"
> +#include "virtio_rdma_ib.h"
> +#include "virtio_rdma_netdev.h"
> +
> +/* TODO:
> + * - How to hook to unload driver, we need to undo all the stuff with did
> + * for all the devices that probed
> + * -
> + */
> +
> +static int virtio_rdma_probe(struct virtio_device *vdev)
> +{
> + struct virtio_rdma_info *ri;
> + int rc = -EIO;
> +
> + ri = ib_alloc_device(virtio_rdma_info, ib_dev);
> + if (!ri) {
> + pr_err("Fail to allocate IB device\n");
> + rc = -ENOMEM;
> + goto out;
> + }
> + vdev->priv = ri;
> +
> + ri->vdev = vdev;
> +
> + rc = init_device(ri);
> + if (rc) {
> + pr_err("Fail to connect to device\n");
> + goto out_dealloc_ib_device;
> + }
> +
> + rc = init_netdev(ri);
> + if (rc) {
> + pr_err("Fail to connect to NetDev layer\n");
> + goto out_fini_device;
> + }
> +
> + rc = init_ib(ri);
> + if (rc) {
> + pr_err("Fail to connect to IB layer\n");
> + goto out_fini_netdev;
> + }
> +
> + pr_info("VirtIO RDMA device %d probed\n", vdev->index);
> +
> + goto out;
> +
> +out_fini_netdev:
> + fini_netdev(ri);
> +
> +out_fini_device:
> + fini_device(ri);
> +
> +out_dealloc_ib_device:
> + ib_dealloc_device(&ri->ib_dev);
> +
> + vdev->priv = NULL;
> +
> +out:
> + return rc;
> +}
> +
> +static void virtio_rdma_remove(struct virtio_device *vdev)
> +{
> + struct virtio_rdma_info *ri = vdev->priv;
> +
> + if (!ri)
> + return;
> +
> + vdev->priv = NULL;
> +
> + fini_ib(ri);
> +
> + fini_netdev(ri);
> +
> + fini_device(ri);
> +
> + ib_dealloc_device(&ri->ib_dev);
> +
> + pr_info("VirtIO RDMA device %d removed\n", vdev->index);
> +}
> +
> +static struct virtio_device_id id_table[] = {
> + { VIRTIO_ID_RDMA, VIRTIO_DEV_ANY_ID },
> + { 0 },
> +};
> +
> +static struct virtio_driver virtio_rdma_driver = {
> + .driver.name = KBUILD_MODNAME,
> + .driver.owner = THIS_MODULE,
> + .id_table = id_table,
> + .probe = virtio_rdma_probe,
> + .remove = virtio_rdma_remove,
> +};
> +
> +static int __init virtio_rdma_init(void)
> +{
> + int rc;
> +
> + rc = register_virtio_driver(&virtio_rdma_driver);
> + if (rc) {
> + pr_err("%s: Fail to register virtio driver (%d)\n", __func__,
> + rc);
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> +static void __exit virtio_rdma_fini(void)
> +{
> + unregister_virtio_driver(&virtio_rdma_driver);
> +}
> +
> +module_init(virtio_rdma_init);
> +module_exit(virtio_rdma_fini);
> +
> +MODULE_DEVICE_TABLE(virtio, id_table);
> +MODULE_AUTHOR("Yuval Shaia");
> +MODULE_DESCRIPTION("Virtio RDMA driver");
> +MODULE_LICENSE("Dual BSD/GPL");
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
> new file mode 100644
> index 000000000000..001f30b3e0b9
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.c
> @@ -0,0 +1,44 @@
> +/*
> + * Virtio RDMA device
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "virtio_rdma_netdev.h"
> +
> +int init_netdev(struct virtio_rdma_info *ri)
> +{
> + struct net_device *dev;
> + struct virtio_rdma_netdev_info *vrndi;
> +
> + dev = alloc_etherdev(sizeof(struct virtio_rdma_netdev_info));
> + if (!dev) {
> + return -ENOMEM;
> + }
> +
> + SET_NETDEV_DEV(dev, &ri->vdev->dev);
> + vrndi = netdev_priv(dev);
> + vrndi->ri = ri;
> + ri->netdev = dev;
> +
> + return 0;
> +}
> +
> +void fini_netdev(struct virtio_rdma_info *ri)
> +{
> + unregister_netdev(ri->netdev);
> +}
> diff --git a/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
> new file mode 100644
> index 000000000000..e7e5d276d8ec
> --- /dev/null
> +++ b/drivers/infiniband/hw/virtio/virtio_rdma_netdev.h
> @@ -0,0 +1,33 @@
> +/*
> + * Virtio RDMA device: Netdev related functions and data
> + *
> + * Copyright (C) 2019 Yuval Shaia Oracle Corporation
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef __VIRTIO_RDMA_NETDEV__
> +#define __VIRTIO_RDMA_NETDEV__
> +
> +#include "virtio_rdma.h"
> +
> +struct virtio_rdma_netdev_info {
> + struct virtio_rdma_info *ri;
> +};
> +
> +int init_netdev(struct virtio_rdma_info *ri);
> +void fini_netdev(struct virtio_rdma_info *ri);
> +
> +#endif
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index 6d5c3b2d4f4d..288ee6fec8d3 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -43,5 +43,6 @@
> #define VIRTIO_ID_INPUT 18 /* virtio input */
> #define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
> #define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
> +#define VIRTIO_ID_RDMA 26 /* RDMA */
>
> #endif /* _LINUX_VIRTIO_IDS_H */
^ permalink raw reply
* Re: [PATCH 3/3] virtio-gpu api: VIRTIO_GPU_F_RESSOURCE_V2
From: Chia-I Wu @ 2019-04-12 23:34 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: virtio, Tomeu Vizoso, Michael S. Tsirkin, David Airlie, open list,
ML dri-devel, Gurchetan Singh, David Airlie,
open list:VIRTIO CORE, NET AND BLOCK DRIVERS
In-Reply-To: <20190412054924.dvh6bfxfrbgvezxr@sirius.home.kraxel.org>
[-- Attachment #1.1: Type: text/plain, Size: 7120 bytes --]
Hi,
I am still new to virgl, and missed the last round of discussion about
resource_create_v2.
From the discussion below, semantically resource_create_v2 creates a host
resource object _without_ any storage; memory_create creates a host memory
object which provides the storage. Is that correct?
And this version of memory_create is probably the most special one among
its other potential variants, because it is the only(?) one who imports the
pre-allocated guest pages.
Do we expect these new commands to be supported by OpenGL, which does not
separate resources and memories?
On Thu, Apr 11, 2019 at 10:49 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Thu, Apr 11, 2019 at 06:36:15PM -0700, Gurchetan Singh wrote:
> > On Wed, Apr 10, 2019 at 10:03 PM Gerd Hoffmann <kraxel@redhat.com>
> wrote:
> > >
> > > > > +/* VIRTIO_GPU_CMD_RESOURCE_CREATE_V2 */
> > > > > +struct virtio_gpu_cmd_resource_create_v2 {
> > > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > > + __le32 resource_id;
> > > > > + __le32 format;
> > > > > + __le32 width;
> > > > > + __le32 height;
> > > > > + /* 3d only */
> > > > > + __le32 target;
> > > > > + __le32 bind;
> > > > > + __le32 depth;
> > > > > + __le32 array_size;
> > > > > + __le32 last_level;
> > > > > + __le32 nr_samples;
> > > > > + __le32 flags;
> > > > > +};
> > > >
> > > >
> > > > I assume this is always backed by some host side allocation, without
> any
> > > > guest side pages associated with it?
> > >
> > > No. It is not backed at all yet. Workflow would be like this:
> > >
> > > (1) VIRTIO_GPU_CMD_RESOURCE_CREATE_V2
> > > (2) VIRTIO_GPU_CMD_MEMORY_CREATE (see patch 2)
> > > (3) VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH (see patch 2)
> >
> > Thanks for the clarification.
> >
> > >
> > > You could also create a larger pool with VIRTIO_GPU_CMD_MEMORY_CREATE,
> > > then go attach multiple resources to it.
> > >
> > > > If so, do we want the option for the guest allocate?
> > >
> > > Allocation options are handled by VIRTIO_GPU_CMD_MEMORY_CREATE
> > > (initially guest allocated only, i.e. what virtio-gpu supports today,
> > > the plan is to add other allocation types later on).
> >
> > You want to cover Vulkan, host-allocated dma-bufs, and guest-allocated
> > dma-bufs with this, correct? Let me know if it's a non-goal :-)
>
> Yes, even though it is not clear yet how we are going to handle
> host-allocated buffers in the vhost-user case ...
This might be another dumb question, but is this only an issue for
vhost-user(-gpu) case? What mechanisms are used to map host dma-buf into
the guest address space?
>
> > If so, we might want to distinguish between memory types (kind of like
> > memoryTypeIndex in Vulkan). [Assuming memory_id is like resource_id]
>
> For the host-allocated buffers we surely want that, yes.
> For guest-allocated memory regions it isn't useful I think ...
>
Guest-allocated memory regions can be just another memory type.
But one needs to create the resource first to know which memory types can
be attached to it. I think the metadata needs to be returned with
resource_create_v2.
>
> > 1) Vulkan seems the most straightforward
> >
> > virtio_gpu_cmd_memory_create --> create kernel data structure,
> > vkAllocateMemory on the host or import guest memory into Vulkan,
> > depending on the memory type
> > virtio_gpu_cmd_resource_create_v2 --> vkCreateImage +
> > vkGetImageMemoryRequirements on host
> > virtio_gpu_cmd_resource_attach_memory --> vkBindImageMemory on host
>
> Yes.
>
> Note 1: virtio_gpu_cmd_memory_create + virtio_gpu_cmd_resource_create_v2
> ordering doesn't matter, so you can virtio_gpu_cmd_resource_create_v2
> first to figure stride and size, then adjust memory size accordingly.
>
> Note 2: The old virtio_gpu_cmd_resource_create variants can be used
> too if you don't need the _v2 features.
>
> Note 3: If I understand things correctly it would be valid to create a
> memory pool (allocate one big chunk of memory) with vkAllocateMemory,
> then bind multiple images at different offsets to it.
>
> > 2) With a guest allocated dma-buf using some new allocation library,
> >
> > virtio_gpu_cmd_resource_create_v2 --> host returns metadata describing
> > optimal allocation
> > virtio_gpu_cmd_memory_create --> allocate guest memory pages since
> > it's guest memory type
> > virtio_gpu_cmd_resource_attach_memory --> associate guest pages with
> > resource in kernel, send iovecs to host for bookkeeping
>
> virtio_gpu_cmd_memory_create sends the iovecs. Otherwise correct.
>
> > 3) With gbm it's a little trickier,
> >
> > virtio_gpu_cmd_resource_create_v2 --> gbm_bo_create_with_modifiers,
> > get metadata in return
>
> Only get metadata in return.
>
> > virtio_gpu_cmd_memory_create --> create kernel data structure, but
> > don't allocate pages, nothing on the host
>
> Memory allocation happens here. Probably makes sense to have a
> virtio_gpu_cmd_memory_create_host command here, because the parameters
> we need are quite different from the guest-allocated case.
>
If we follow Vulkan, we only need the size and the memory type in most
cases. The current version of memory_create is a special case because it
is an import operation and needs the guest mem_entry. Perhaps
memory_create (for host) and memory_import_guest (to replace the current
version)?
>
> Maybe we even need a virtio_gpu_cmd_memory_create_host_for_resource
> variant, given that gbm doesn't have raw memory buffers without any
> format attached to it.
>
And the memory will only be attachable to the given (or compatible)
resource, right?
Vulkan is much more explicit than any pre-existing API. I guess we will
have to add this to cover APIs beyond Vulkan.
>
> > > > > +/* VIRTIO_GPU_RESP_OK_RESOURCE_INFO */
> > > > > +struct virtio_gpu_resp_resource_info {
> > > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > > + __le32 stride[4];
> > > > > + __le32 size[4];
> > > > > +};
> > > >
> > > > offsets[4] needed too
> > >
> > > That is in VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH ...
> >
> > I assume the offsets aren't returned by
> > VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH.
>
> Yes, they are send by the guest.
>
Gurchetan probably means alignment[4].
>
> > How does the guest know at which offsets in memory will be compatible
> > to share with display, camera, etc?
>
> Is is good enough to align offsets to page boundaries?
>
That should be good enough. But by returning alignments, we can minimize
the gaps when attaching multiple resources, especially when the resources
are only used by GPU.
>
> > Also, do you want to cover the case where the resource is backed by
> > three separate memory regions (VK_IMAGE_CREATE_DISJOINT_BIT)?
>
> Good point. I guess we should make memory_id in
> virtio_gpu_cmd_resource_attach_memory an array then,
> so you can specify a different memory region for each plane.
>
> cheers,
> Gerd
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[-- Attachment #1.2: Type: text/html, Size: 9825 bytes --]
[-- 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: [RFC PATCH 00/12] s390: virtio: support protected virtualization
From: David Hildenbrand @ 2019-04-12 13:47 UTC (permalink / raw)
To: Halil Pasic, kvm, linux-s390, Cornelia Huck, Martin Schwidefsky,
Sebastian Ott
Cc: Eric Farman, Viktor Mihajlovski, Janosch Frank, Vasily Gorbik,
Farhan Ali, virtualization, Claudio Imbrenda
In-Reply-To: <20190404231622.52531-1-pasic@linux.ibm.com>
On 05.04.19 01:16, Halil Pasic wrote:
> Enhanced virtualization protection technology may require the use of
> bounce buffers for I/O. While support for this was built into the virtio
> core, virtio-ccw wasn't changed accordingly.
Can you elaborate some more about the general approach (Enhanced
virtualization protection technology, ultravisor, concept, issues, how
to squeeze it into QEMU/KVM/kernel) etc =
For my taste, this cover letter misses some important context :)
>
> Thus what needs to be done to bring virtio-ccw up to speed with respect
> to this is:
> * use some 'new' common virtio stuff
> * make sure that virtio-ccw specific stuff uses shared memory when
> talking to the hypervisor (except communication blocks like ORB, these
> are handled by the hypervisor)
> * make sure the DMA API does what is necessary to talk through shared
> memory if we are a protected virtualization guest.
> * make sure the common IO layer plays along as well (airqs, sense).
>
> The series is structured in incremental fashion: some of the changes are
> overridden by following patches. The main reason why is that this is how I
> developed. But I think it ain't bad for the didactic and we are a bit more
> flexible with regards to throwing out some of the stuff in the end.
>
> Important notes:
>
> * This is an early (WIP) RFC that does not add any function to the
> kernel at his stage, as the ultravisor interactions are left out.
> The purpose is getting some early feedback ASAP.
>
> * In future these patches will depend on some code interacting with the
> ultravisor (WIP by Vasily and Janosch).
>
> * The s390 names are by no means final, and are not properly explained. Should
> not hamper understanding too much. If it does please ask.
>
> * The existing naming in the common infrastructure (kernel internal
> interfaces) is pretty much based on the AMD SEV terminology. Thus the
> names aren't always perfect. There might be merit to changing these
> names to more abstract ones. I did not put much thought into that at
> the current stage.
>
>
> Testing:
>
> Please use iommu_platform=on for any virtio devices you are going
> to test this code with (so virtio actually uses the DMA API).
>
> Looking forward to your review or any other type of input.
>
> Halil Pasic (12):
> virtio/s390: use vring_create_virtqueue
> virtio/s390: DMA support for virtio-ccw
> s390/mm: force swiotlb for protected virtualization
> s390/cio: introduce cio DMA pool
> s390/cio: add protected virtualization support to cio
> s390/airq: use DMA memory for adapter interrupts
> virtio/s390: use DMA memory for ccw I/O
> virtio/s390: add indirection to indicators access
> virtio/s390: use DMA memory for notifiers
> virtio/s390: consolidate DMA allocations
> virtio/s390: use the cio DMA pool
> virtio/s390: make airq summary indicators DMA
>
> arch/s390/Kconfig | 5 +
> arch/s390/include/asm/Kbuild | 1 -
> arch/s390/include/asm/airq.h | 2 +
> arch/s390/include/asm/cio.h | 4 +
> arch/s390/include/asm/dma-mapping.h | 13 ++
> arch/s390/include/asm/mem_encrypt.h | 18 +++
> arch/s390/mm/init.c | 44 +++++
> drivers/s390/cio/airq.c | 18 ++-
> drivers/s390/cio/ccwreq.c | 8 +-
> drivers/s390/cio/css.c | 63 ++++++++
> drivers/s390/cio/device.c | 46 ++++--
> drivers/s390/cio/device_fsm.c | 40 ++---
> drivers/s390/cio/device_id.c | 18 +--
> drivers/s390/cio/device_ops.c | 4 +-
> drivers/s390/cio/device_pgid.c | 20 +--
> drivers/s390/cio/device_status.c | 24 +--
> drivers/s390/cio/io_sch.h | 19 ++-
> drivers/s390/virtio/virtio_ccw.c | 310 ++++++++++++++++++++----------------
> 18 files changed, 444 insertions(+), 213 deletions(-)
> create mode 100644 arch/s390/include/asm/dma-mapping.h
> create mode 100644 arch/s390/include/asm/mem_encrypt.h
>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Jeff Moyer @ 2019-04-12 13:12 UTC (permalink / raw)
To: Jan Kara
Cc: Pankaj Gupta, cohuck, KVM list, Michael S. Tsirkin, david,
Qemu Developers, virtualization, Andreas Dilger, Ross Zwisler,
Andrea Arcangeli, Dave Jiang, linux-nvdimm, Vishal L Verma,
Matthew Wilcox, Christoph Hellwig, Linux ACPI, linux-ext4,
Len Brown, kilobyte, Rik van Riel, yuval.shaia, Stefan Hajnoczi,
Paolo Bonzini, Dan Williams, lcapitulino
In-Reply-To: <20190412083230.GA29850@quack2.suse.cz>
Jan Kara <jack@suse.cz> writes:
> On Thu 11-04-19 07:51:48, Dan Williams wrote:
>> On Tue, Apr 9, 2019 at 9:09 PM Pankaj Gupta <pagupta@redhat.com> wrote:
>> > + } else {
>> > + if (nd_region->flush(nd_region))
>> > + rc = -EIO;
>>
>> Given the common case wants to be fast and synchronous I think we
>> should try to avoid retpoline overhead by default. So something like
>> this:
>>
>> if (nd_region->flush == generic_nvdimm_flush)
>> rc = generic_nvdimm_flush(...);
>
> I'd either add a comment about avoiding retpoline overhead here or just
> make ->flush == NULL mean generic_nvdimm_flush(). Just so that people don't
> get confused by the code.
Isn't this premature optimization? I really don't like adding things
like this without some numbers to show it's worth it.
-Jeff
^ permalink raw reply
* Re: [RFC PATCH 04/12] s390/cio: introduce cio DMA pool
From: Sebastian Ott @ 2019-04-12 12:12 UTC (permalink / raw)
To: Halil Pasic
Cc: Farhan Ali, linux-s390, Eric Farman, Claudio Imbrenda,
Vasily Gorbik, kvm, Cornelia Huck, virtualization,
Martin Schwidefsky, Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190412132010.3c74cb63@oc2783563651>
On Fri, 12 Apr 2019, Halil Pasic wrote:
> On Thu, 11 Apr 2019 20:25:01 +0200 (CEST)
> Sebastian Ott <sebott@linux.ibm.com> wrote:
> > I don't think we should use this global DMA pool. I guess it's OK for
> > stuff like airq (where we don't have a struct device at hand) but for
> > CCW we should use the device we have. Yes, this way we waste some memory
> > but all dma memory a device uses should fit in a page - so the wastage
> > is not too much.
> >
>
> Is what you envision an own gen_pool on for each subchannel (e.g. a
> struct io_subchannel member)?
Either that or if that's too much overhead simply map a page and create
a struct containing the few dma areas for that device.
> I'm struggling with understanding the expected benefits of a
> per-subchannel pool/allocator. Can you please tell me what benefits do
> you expect (over the current approach)?
Logically DMA is a capability of a device and the whole DMA API is build
around devices. Working around that just feels wrong. For practical
matters: DMA debugging will complain about misuse of a specific device or
driver.
> I understand you idea is to keep the CIO global pool for stuff that can
> not be tied to a single device (i.e. ariq). So the per device stuff would
> also mean more code. Would you be OK with postponing this alleged
> enhancement (i.e. implement it as a patch on top of this series)?
I don't like it but it's just in-kernel usage which we can change at any
time. So if it helps you to do it that way, why not..
^ permalink raw reply
* Re: [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Jan Kara @ 2019-04-12 8:32 UTC (permalink / raw)
To: Dan Williams
Cc: Pankaj Gupta, cohuck, Jan Kara, KVM list, Michael S. Tsirkin,
david, Qemu Developers, virtualization, Andreas Dilger,
Ross Zwisler, Andrea Arcangeli, Dave Jiang, linux-nvdimm,
Vishal L Verma, Matthew Wilcox, Christoph Hellwig, Linux ACPI,
jmoyer, linux-ext4, Len Brown, kilobyte, Rik van Riel,
yuval.shaia, Stefan Hajnoczi, Paolo Bonzini, lcapitul
In-Reply-To: <CAPcyv4i1FjBzfcWtzeCvj-giHa9xaX0dMQ==q4LZC_L8VEqkHg@mail.gmail.com>
On Thu 11-04-19 07:51:48, Dan Williams wrote:
> On Tue, Apr 9, 2019 at 9:09 PM Pankaj Gupta <pagupta@redhat.com> wrote:
> > + } else {
> > + if (nd_region->flush(nd_region))
> > + rc = -EIO;
>
> Given the common case wants to be fast and synchronous I think we
> should try to avoid retpoline overhead by default. So something like
> this:
>
> if (nd_region->flush == generic_nvdimm_flush)
> rc = generic_nvdimm_flush(...);
I'd either add a comment about avoiding retpoline overhead here or just
make ->flush == NULL mean generic_nvdimm_flush(). Just so that people don't
get confused by the code.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH 3/3] virtio-gpu api: VIRTIO_GPU_F_RESSOURCE_V2
From: Gerd Hoffmann @ 2019-04-12 5:49 UTC (permalink / raw)
To: Gurchetan Singh
Cc: Tomeu Vizoso, Michael S. Tsirkin, David Airlie, open list,
ML dri-devel, open list:VIRTIO CORE, NET AND BLOCK DRIVERS,
Marc-André Lureau, David Airlie, virtio
In-Reply-To: <CAAfnVB=v+daY0Q+mtdK2Q=F7iS_Ra7jGYfic3twfnUVo=tz3Yg@mail.gmail.com>
On Thu, Apr 11, 2019 at 06:36:15PM -0700, Gurchetan Singh wrote:
> On Wed, Apr 10, 2019 at 10:03 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > > > +/* VIRTIO_GPU_CMD_RESOURCE_CREATE_V2 */
> > > > +struct virtio_gpu_cmd_resource_create_v2 {
> > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > + __le32 resource_id;
> > > > + __le32 format;
> > > > + __le32 width;
> > > > + __le32 height;
> > > > + /* 3d only */
> > > > + __le32 target;
> > > > + __le32 bind;
> > > > + __le32 depth;
> > > > + __le32 array_size;
> > > > + __le32 last_level;
> > > > + __le32 nr_samples;
> > > > + __le32 flags;
> > > > +};
> > >
> > >
> > > I assume this is always backed by some host side allocation, without any
> > > guest side pages associated with it?
> >
> > No. It is not backed at all yet. Workflow would be like this:
> >
> > (1) VIRTIO_GPU_CMD_RESOURCE_CREATE_V2
> > (2) VIRTIO_GPU_CMD_MEMORY_CREATE (see patch 2)
> > (3) VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH (see patch 2)
>
> Thanks for the clarification.
>
> >
> > You could also create a larger pool with VIRTIO_GPU_CMD_MEMORY_CREATE,
> > then go attach multiple resources to it.
> >
> > > If so, do we want the option for the guest allocate?
> >
> > Allocation options are handled by VIRTIO_GPU_CMD_MEMORY_CREATE
> > (initially guest allocated only, i.e. what virtio-gpu supports today,
> > the plan is to add other allocation types later on).
>
> You want to cover Vulkan, host-allocated dma-bufs, and guest-allocated
> dma-bufs with this, correct? Let me know if it's a non-goal :-)
Yes, even though it is not clear yet how we are going to handle
host-allocated buffers in the vhost-user case ...
> If so, we might want to distinguish between memory types (kind of like
> memoryTypeIndex in Vulkan). [Assuming memory_id is like resource_id]
For the host-allocated buffers we surely want that, yes.
For guest-allocated memory regions it isn't useful I think ...
> 1) Vulkan seems the most straightforward
>
> virtio_gpu_cmd_memory_create --> create kernel data structure,
> vkAllocateMemory on the host or import guest memory into Vulkan,
> depending on the memory type
> virtio_gpu_cmd_resource_create_v2 --> vkCreateImage +
> vkGetImageMemoryRequirements on host
> virtio_gpu_cmd_resource_attach_memory --> vkBindImageMemory on host
Yes.
Note 1: virtio_gpu_cmd_memory_create + virtio_gpu_cmd_resource_create_v2
ordering doesn't matter, so you can virtio_gpu_cmd_resource_create_v2
first to figure stride and size, then adjust memory size accordingly.
Note 2: The old virtio_gpu_cmd_resource_create variants can be used
too if you don't need the _v2 features.
Note 3: If I understand things correctly it would be valid to create a
memory pool (allocate one big chunk of memory) with vkAllocateMemory,
then bind multiple images at different offsets to it.
> 2) With a guest allocated dma-buf using some new allocation library,
>
> virtio_gpu_cmd_resource_create_v2 --> host returns metadata describing
> optimal allocation
> virtio_gpu_cmd_memory_create --> allocate guest memory pages since
> it's guest memory type
> virtio_gpu_cmd_resource_attach_memory --> associate guest pages with
> resource in kernel, send iovecs to host for bookkeeping
virtio_gpu_cmd_memory_create sends the iovecs. Otherwise correct.
> 3) With gbm it's a little trickier,
>
> virtio_gpu_cmd_resource_create_v2 --> gbm_bo_create_with_modifiers,
> get metadata in return
Only get metadata in return.
> virtio_gpu_cmd_memory_create --> create kernel data structure, but
> don't allocate pages, nothing on the host
Memory allocation happens here. Probably makes sense to have a
virtio_gpu_cmd_memory_create_host command here, because the parameters
we need are quite different from the guest-allocated case.
Maybe we even need a virtio_gpu_cmd_memory_create_host_for_resource
variant, given that gbm doesn't have raw memory buffers without any
format attached to it.
> > > > +/* VIRTIO_GPU_RESP_OK_RESOURCE_INFO */
> > > > +struct virtio_gpu_resp_resource_info {
> > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > + __le32 stride[4];
> > > > + __le32 size[4];
> > > > +};
> > >
> > > offsets[4] needed too
> >
> > That is in VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH ...
>
> I assume the offsets aren't returned by
> VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH.
Yes, they are send by the guest.
> How does the guest know at which offsets in memory will be compatible
> to share with display, camera, etc?
Is is good enough to align offsets to page boundaries?
> Also, do you want to cover the case where the resource is backed by
> three separate memory regions (VK_IMAGE_CREATE_DISJOINT_BIT)?
Good point. I guess we should make memory_id in
virtio_gpu_cmd_resource_attach_memory an array then,
so you can specify a different memory region for each plane.
cheers,
Gerd
^ permalink raw reply
* Re: [RFC PATCH 04/12] s390/cio: introduce cio DMA pool
From: Sebastian Ott @ 2019-04-11 18:25 UTC (permalink / raw)
To: Halil Pasic
Cc: Farhan Ali, linux-s390, Eric Farman, Claudio Imbrenda,
Vasily Gorbik, kvm, Cornelia Huck, virtualization,
Martin Schwidefsky, Viktor Mihajlovski, Janosch Frank
In-Reply-To: <20190404231622.52531-5-pasic@linux.ibm.com>
On Fri, 5 Apr 2019, Halil Pasic wrote:
> To support protected virtualization cio will need to make sure the
> memory used for communication with the hypervisor is DMA memory.
>
> Let us introduce a DMA pool to cio that will help us in allocating
> deallocating those chunks of memory.
>
> We use a gen_pool backed with DMA pages to avoid each allocation
> effectively wasting a page, as we typically allocate much less
> than PAGE_SIZE.
I don't think we should use this global DMA pool. I guess it's OK for
stuff like airq (where we don't have a struct device at hand) but for
CCW we should use the device we have. Yes, this way we waste some memory
but all dma memory a device uses should fit in a page - so the wastage
is not too much.
Sebastian
^ permalink raw reply
* Re: [Qemu-devel] [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-11 17:41 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma@vger.kernel.org, Cornelia Huck,
virtualization@lists.linux-foundation.org, qemu-devel@nongnu.org,
mst@redhat.com
In-Reply-To: <20190411173419.GB6259@lap1>
On Thu, Apr 11, 2019 at 08:34:20PM +0300, Yuval Shaia wrote:
> On Thu, Apr 11, 2019 at 05:24:08PM +0000, Jason Gunthorpe wrote:
> > On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> > > On Thu, 11 Apr 2019 14:01:54 +0300
> > > Yuval Shaia <yuval.shaia@oracle.com> wrote:
> > >
> > > > Data center backends use more and more RDMA or RoCE devices and more and
> > > > more software runs in virtualized environment.
> > > > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > > >
> > > > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > > > technology and also because the Virtio specification
> > > > allows Hardware Vendors to support Virtio protocol natively in order to
> > > > achieve bare metal performance.
> > > >
> > > > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > > > Virtio Specification and a look forward on possible implementation
> > > > techniques.
> > > >
> > > > Open issues/Todo list:
> > > > List is huge, this is only start point of the project.
> > > > Anyway, here is one example of item in the list:
> > > > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > > > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > > > that this is reasonable so one option is to have one for all and
> > > > multiplex the traffic on it. This is not good approach as by design it
> > > > introducing an optional starvation. Another approach would be multi
> > > > queues and round-robin (for example) between them.
> > > >
> > > > Expectations from this posting:
> > > > In general, any comment is welcome, starting from hey, drop this as it is a
> > > > very bad idea, to yeah, go ahead, we really want it.
> > > > Idea here is that since it is not a minor effort i first want to know if
> > > > there is some sort interest in the community for such device.
> > >
> > > My first reaction is: Sounds sensible, but it would be good to have a
> > > spec for this :)
> >
> > I'm unclear why you'd want to have a virtio queue for anything other
> > that some kind of command channel.
> >
> > I'm not sure a QP or CQ benefits from this??
>
> Virtqueue is a standard mechanism to pass data from guest to host. By
And vice versa (CQ?)
> saying that - it really sounds like QP send and recv rings. So my thought
> is to use a standard way for rings. As i've learned this is how it is used
> by other virtio devices ex virtio-net.
>
> >
> > Jason
>
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Jason Gunthorpe @ 2019-04-11 17:40 UTC (permalink / raw)
To: Yuval Shaia
Cc: mst@redhat.com, linux-rdma@vger.kernel.org, Cornelia Huck,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org
In-Reply-To: <20190411173419.GB6259@lap1>
On Thu, Apr 11, 2019 at 08:34:20PM +0300, Yuval Shaia wrote:
> On Thu, Apr 11, 2019 at 05:24:08PM +0000, Jason Gunthorpe wrote:
> > On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> > > On Thu, 11 Apr 2019 14:01:54 +0300
> > > Yuval Shaia <yuval.shaia@oracle.com> wrote:
> > >
> > > > Data center backends use more and more RDMA or RoCE devices and more and
> > > > more software runs in virtualized environment.
> > > > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > > >
> > > > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > > > technology and also because the Virtio specification
> > > > allows Hardware Vendors to support Virtio protocol natively in order to
> > > > achieve bare metal performance.
> > > >
> > > > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > > > Virtio Specification and a look forward on possible implementation
> > > > techniques.
> > > >
> > > > Open issues/Todo list:
> > > > List is huge, this is only start point of the project.
> > > > Anyway, here is one example of item in the list:
> > > > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > > > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > > > that this is reasonable so one option is to have one for all and
> > > > multiplex the traffic on it. This is not good approach as by design it
> > > > introducing an optional starvation. Another approach would be multi
> > > > queues and round-robin (for example) between them.
> > > >
> > > > Expectations from this posting:
> > > > In general, any comment is welcome, starting from hey, drop this as it is a
> > > > very bad idea, to yeah, go ahead, we really want it.
> > > > Idea here is that since it is not a minor effort i first want to know if
> > > > there is some sort interest in the community for such device.
> > >
> > > My first reaction is: Sounds sensible, but it would be good to have a
> > > spec for this :)
> >
> > I'm unclear why you'd want to have a virtio queue for anything other
> > that some kind of command channel.
> >
> > I'm not sure a QP or CQ benefits from this??
>
> Virtqueue is a standard mechanism to pass data from guest to host. By
> saying that - it really sounds like QP send and recv rings. So my thought
> is to use a standard way for rings. As i've learned this is how it is used
> by other virtio devices ex virtio-net.
I doubt you can use virtio queues from userspace securely? Usually
needs a dedicated page for each user space process
Jason
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Yuval Shaia @ 2019-04-11 17:34 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: mst@redhat.com, linux-rdma@vger.kernel.org, Cornelia Huck,
qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org
In-Reply-To: <20190411172402.GA14509@mellanox.com>
On Thu, Apr 11, 2019 at 05:24:08PM +0000, Jason Gunthorpe wrote:
> On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> > On Thu, 11 Apr 2019 14:01:54 +0300
> > Yuval Shaia <yuval.shaia@oracle.com> wrote:
> >
> > > Data center backends use more and more RDMA or RoCE devices and more and
> > > more software runs in virtualized environment.
> > > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> > >
> > > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > > technology and also because the Virtio specification
> > > allows Hardware Vendors to support Virtio protocol natively in order to
> > > achieve bare metal performance.
> > >
> > > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > > Virtio Specification and a look forward on possible implementation
> > > techniques.
> > >
> > > Open issues/Todo list:
> > > List is huge, this is only start point of the project.
> > > Anyway, here is one example of item in the list:
> > > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > > that this is reasonable so one option is to have one for all and
> > > multiplex the traffic on it. This is not good approach as by design it
> > > introducing an optional starvation. Another approach would be multi
> > > queues and round-robin (for example) between them.
> > >
> > > Expectations from this posting:
> > > In general, any comment is welcome, starting from hey, drop this as it is a
> > > very bad idea, to yeah, go ahead, we really want it.
> > > Idea here is that since it is not a minor effort i first want to know if
> > > there is some sort interest in the community for such device.
> >
> > My first reaction is: Sounds sensible, but it would be good to have a
> > spec for this :)
>
> I'm unclear why you'd want to have a virtio queue for anything other
> that some kind of command channel.
>
> I'm not sure a QP or CQ benefits from this??
Virtqueue is a standard mechanism to pass data from guest to host. By
saying that - it really sounds like QP send and recv rings. So my thought
is to use a standard way for rings. As i've learned this is how it is used
by other virtio devices ex virtio-net.
>
> Jason
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Jason Gunthorpe @ 2019-04-11 17:24 UTC (permalink / raw)
To: Cornelia Huck
Cc: mst@redhat.com, linux-rdma@vger.kernel.org, qemu-devel@nongnu.org,
Yuval Shaia, virtualization@lists.linux-foundation.org
In-Reply-To: <20190411190215.2163572e.cohuck@redhat.com>
On Thu, Apr 11, 2019 at 07:02:15PM +0200, Cornelia Huck wrote:
> On Thu, 11 Apr 2019 14:01:54 +0300
> Yuval Shaia <yuval.shaia@oracle.com> wrote:
>
> > Data center backends use more and more RDMA or RoCE devices and more and
> > more software runs in virtualized environment.
> > There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
> >
> > Virtio is the optimal solution since is the de-facto para-virtualizaton
> > technology and also because the Virtio specification
> > allows Hardware Vendors to support Virtio protocol natively in order to
> > achieve bare metal performance.
> >
> > This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> > Virtio Specification and a look forward on possible implementation
> > techniques.
> >
> > Open issues/Todo list:
> > List is huge, this is only start point of the project.
> > Anyway, here is one example of item in the list:
> > - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> > in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> > that this is reasonable so one option is to have one for all and
> > multiplex the traffic on it. This is not good approach as by design it
> > introducing an optional starvation. Another approach would be multi
> > queues and round-robin (for example) between them.
> >
> > Expectations from this posting:
> > In general, any comment is welcome, starting from hey, drop this as it is a
> > very bad idea, to yeah, go ahead, we really want it.
> > Idea here is that since it is not a minor effort i first want to know if
> > there is some sort interest in the community for such device.
>
> My first reaction is: Sounds sensible, but it would be good to have a
> spec for this :)
I'm unclear why you'd want to have a virtio queue for anything other
that some kind of command channel.
I'm not sure a QP or CQ benefits from this??
Jason
^ permalink raw reply
* Re: [RFC 0/3] VirtIO RDMA
From: Cornelia Huck @ 2019-04-11 17:02 UTC (permalink / raw)
To: Yuval Shaia; +Cc: mst, linux-rdma, qemu-devel, virtualization, jgg
In-Reply-To: <20190411110157.14252-1-yuval.shaia@oracle.com>
On Thu, 11 Apr 2019 14:01:54 +0300
Yuval Shaia <yuval.shaia@oracle.com> wrote:
> Data center backends use more and more RDMA or RoCE devices and more and
> more software runs in virtualized environment.
> There is a need for a standard to enable RDMA/RoCE on Virtual Machines.
>
> Virtio is the optimal solution since is the de-facto para-virtualizaton
> technology and also because the Virtio specification
> allows Hardware Vendors to support Virtio protocol natively in order to
> achieve bare metal performance.
>
> This RFC is an effort to addresses challenges in defining the RDMA/RoCE
> Virtio Specification and a look forward on possible implementation
> techniques.
>
> Open issues/Todo list:
> List is huge, this is only start point of the project.
> Anyway, here is one example of item in the list:
> - Multi VirtQ: Every QP has two rings and every CQ has one. This means that
> in order to support for example 32K QPs we will need 64K VirtQ. Not sure
> that this is reasonable so one option is to have one for all and
> multiplex the traffic on it. This is not good approach as by design it
> introducing an optional starvation. Another approach would be multi
> queues and round-robin (for example) between them.
>
> Expectations from this posting:
> In general, any comment is welcome, starting from hey, drop this as it is a
> very bad idea, to yeah, go ahead, we really want it.
> Idea here is that since it is not a minor effort i first want to know if
> there is some sort interest in the community for such device.
My first reaction is: Sounds sensible, but it would be good to have a
spec for this :)
You'll need a spec if you want this to go forward anyway, so at least a
sketch would be good to answer questions such as how many virtqueues
you use for which purpose, what is actually put on the virtqueues,
whether there are negotiable features, and what the expectations for
the device and the driver are. It also makes it easier to understand
how this is supposed to work in practice.
If folks agree that this sounds useful, the next step would be to
reserve an id for the device type.
>
> The scope of the implementation is limited to probing the device and doing
> some basic ibverbs commands. Data-path is not yet implemented. So with this
> one can expect only that driver is (partialy) loaded and basic queries and
> resource allocation is done.
>
> One note regarding the patchset.
> I know it is not standard to collaps patches from several repos as i did
> here (qemu and linux) but decided to do it anyway so the whole picture can
> be seen.
>
> patch 1: virtio-net: Move some virtio-net-pci decl to include/hw/virtio
> This is a prelimenary patch just as a hack so i will not need to
> impelement new netdev
> patch 2: hw/virtio-rdma: VirtIO rdma device
> The implementation of the device
> patch 3: RDMA/virtio-rdma: VirtIO rdma driver
> The device driver
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Pankaj Gupta @ 2019-04-11 16:23 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, KVM list, Michael S. Tsirkin, david, Qemu Developers,
virtualization, Andreas Dilger, Ross Zwisler, Andrea Arcangeli,
Dave Jiang, linux-nvdimm, Vishal L Verma, Matthew Wilcox,
Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4, Len Brown,
kilobyte, Rik van Riel, yuval shaia, Stefan Hajnoczi,
Igor Mammedov, lcapitulino, Nitesh Narayan Lal <nila>
In-Reply-To: <CAPcyv4gOPtgmVyk+zayRBtkbSivEvBNn1tyKL2ycT_q_nTHUUA@mail.gmail.com>
> > > > This patch adds functionality to perform flush from guest
> > > > to host over VIRTIO. We are registering a callback based
> > > > on 'nd_region' type. virtio_pmem driver requires this special
> > > > flush function. For rest of the region types we are registering
> > > > existing flush function. Report error returned by host fsync
> > > > failure to userspace.
> > > >
> > > > This also handles asynchronous flush requests from the block layer
> > > > by creating a child bio and chaining it with parent bio.
> > > >
> > > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > > ---bio_chain Dan williams
> > > [..]
> > > > diff --git a/drivers/nvdimm/region_devs.c
> > > > b/drivers/nvdimm/region_devs.c
> > > > index b4ef7d9ff22e..fb1041ab32a6 100644
> > > > --- a/drivers/nvdimm/region_devs.c
> > > > +++ b/drivers/nvdimm/region_devs.c
> > > > @@ -295,7 +295,9 @@ static ssize_t deep_flush_store(struct device *dev,
> > > > struct device_attribute *att
> > > > return rc;
> > > > if (!flush)
> > > > return -EINVAL;
> > > > - nvdimm_flush(nd_region);
> > > > + rc = nvdimm_flush(nd_region, NULL, false);
> > > > + if (rc)
> > > > + return rc;
> > > >
> > > > return len;
> > > > }
> > > > @@ -1085,6 +1087,11 @@ static struct nd_region *nd_region_create(struct
> > > > nvdimm_bus *nvdimm_bus,
> > > > dev->of_node = ndr_desc->of_node;
> > > > nd_region->ndr_size = resource_size(ndr_desc->res);
> > > > nd_region->ndr_start = ndr_desc->res->start;
> > > > + if (ndr_desc->flush)
> > > > + nd_region->flush = ndr_desc->flush;
> > > > + else
> > > > + nd_region->flush = generic_nvdimm_flush;
> > > > +
> > > > nd_device_register(dev);
> > > >
> > > > return nd_region;
> > > > @@ -1125,11 +1132,36 @@ struct nd_region
> > > > *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> > > > }
> > > > EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
> > > >
> > > > +int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool
> > > > async)
> > > > +{
> > >
> > > I don't quite see the point of the 'async' argument. All the usages of
> > > this routine are either
> > >
> > > nvdimm_flush(nd_region, bio, true)
> > > ...or:
> > > nvdimm_flush(nd_region, NULL, false)
> >
> > Agree.
> >
> > >
> > > ...so why not gate async behavior on the presence of the 'bio' argument?
> >
> > Sure.
> >
> > >
> > >
> > > > + int rc = 0;
> > > > +
> > > > + /* Create child bio for asynchronous flush and chain with
> > > > + * parent bio. Otherwise directly call nd_region flush.
> > > > + */
> > > > + if (async && bio->bi_iter.bi_sector != -1) {
> > > > +
> > > > + struct bio *child = bio_alloc(GFP_ATOMIC, 0);
> > > > +
> > > > + if (!child)
> > > > + return -ENOMEM;
> > > > + bio_copy_dev(child, bio);
> > > > + child->bi_opf = REQ_PREFLUSH;
> > > > + child->bi_iter.bi_sector = -1;
> > > > + bio_chain(child, bio);
> > > > + submit_bio(child);
> > >
> > > I understand how this works, but it's a bit too "magical" for my
> > > taste. I would prefer that all flush implementations take an optional
> > > 'bio' argument rather than rely on the make_request implementation to
> > > stash the bio away on a driver specific list.
> >
> > I did this to make use of "bio_chain" for chaining child bio for async
> > flush
> > suggested [1]. Are you saying to remove this and just call "flush" based on
> > bio argument? Or I implemented the 'bio_chain' request entirely wrong?
>
> No, I think you implemented it correctly. I'm just asking for the
> chaining to be performed internal to the ->flush() callback rather
> than in the common nvdimm_flush() front-end.
Sure. Perfect!
Thank you very much for all the suggestions.
Best regards,
Pankaj
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Dan Williams @ 2019-04-11 16:09 UTC (permalink / raw)
To: Pankaj Gupta
Cc: Jan Kara, KVM list, Michael S. Tsirkin, david, Qemu Developers,
virtualization, Andreas Dilger, Ross Zwisler, Andrea Arcangeli,
Dave Jiang, linux-nvdimm, Vishal L Verma, Matthew Wilcox,
Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4, Len Brown,
kilobyte, Rik van Riel, yuval shaia, Stefan Hajnoczi,
Igor Mammedov, lcapitulino, Nitesh Narayan Lal <nila>
In-Reply-To: <1463291806.21158433.1554998258746.JavaMail.zimbra@redhat.com>
On Thu, Apr 11, 2019 at 9:02 AM Pankaj Gupta <pagupta@redhat.com> wrote:
>
>
>
> > >
> > > This patch adds functionality to perform flush from guest
> > > to host over VIRTIO. We are registering a callback based
> > > on 'nd_region' type. virtio_pmem driver requires this special
> > > flush function. For rest of the region types we are registering
> > > existing flush function. Report error returned by host fsync
> > > failure to userspace.
> > >
> > > This also handles asynchronous flush requests from the block layer
> > > by creating a child bio and chaining it with parent bio.
> > >
> > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > > ---bio_chain Dan williams
> > [..]
> > > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > > index b4ef7d9ff22e..fb1041ab32a6 100644
> > > --- a/drivers/nvdimm/region_devs.c
> > > +++ b/drivers/nvdimm/region_devs.c
> > > @@ -295,7 +295,9 @@ static ssize_t deep_flush_store(struct device *dev,
> > > struct device_attribute *att
> > > return rc;
> > > if (!flush)
> > > return -EINVAL;
> > > - nvdimm_flush(nd_region);
> > > + rc = nvdimm_flush(nd_region, NULL, false);
> > > + if (rc)
> > > + return rc;
> > >
> > > return len;
> > > }
> > > @@ -1085,6 +1087,11 @@ static struct nd_region *nd_region_create(struct
> > > nvdimm_bus *nvdimm_bus,
> > > dev->of_node = ndr_desc->of_node;
> > > nd_region->ndr_size = resource_size(ndr_desc->res);
> > > nd_region->ndr_start = ndr_desc->res->start;
> > > + if (ndr_desc->flush)
> > > + nd_region->flush = ndr_desc->flush;
> > > + else
> > > + nd_region->flush = generic_nvdimm_flush;
> > > +
> > > nd_device_register(dev);
> > >
> > > return nd_region;
> > > @@ -1125,11 +1132,36 @@ struct nd_region
> > > *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> > > }
> > > EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
> > >
> > > +int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async)
> > > +{
> >
> > I don't quite see the point of the 'async' argument. All the usages of
> > this routine are either
> >
> > nvdimm_flush(nd_region, bio, true)
> > ...or:
> > nvdimm_flush(nd_region, NULL, false)
>
> Agree.
>
> >
> > ...so why not gate async behavior on the presence of the 'bio' argument?
>
> Sure.
>
> >
> >
> > > + int rc = 0;
> > > +
> > > + /* Create child bio for asynchronous flush and chain with
> > > + * parent bio. Otherwise directly call nd_region flush.
> > > + */
> > > + if (async && bio->bi_iter.bi_sector != -1) {
> > > +
> > > + struct bio *child = bio_alloc(GFP_ATOMIC, 0);
> > > +
> > > + if (!child)
> > > + return -ENOMEM;
> > > + bio_copy_dev(child, bio);
> > > + child->bi_opf = REQ_PREFLUSH;
> > > + child->bi_iter.bi_sector = -1;
> > > + bio_chain(child, bio);
> > > + submit_bio(child);
> >
> > I understand how this works, but it's a bit too "magical" for my
> > taste. I would prefer that all flush implementations take an optional
> > 'bio' argument rather than rely on the make_request implementation to
> > stash the bio away on a driver specific list.
>
> I did this to make use of "bio_chain" for chaining child bio for async flush
> suggested [1]. Are you saying to remove this and just call "flush" based on
> bio argument? Or I implemented the 'bio_chain' request entirely wrong?
No, I think you implemented it correctly. I'm just asking for the
chaining to be performed internal to the ->flush() callback rather
than in the common nvdimm_flush() front-end.
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v5 1/6] libnvdimm: nd_region flush callback support
From: Pankaj Gupta @ 2019-04-11 15:57 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, KVM list, Michael S. Tsirkin, david, Qemu Developers,
virtualization, Andreas Dilger, Ross Zwisler, Andrea Arcangeli,
Dave Jiang, linux-nvdimm, Vishal L Verma, Matthew Wilcox,
Christoph Hellwig, Linux ACPI, jmoyer, linux-ext4, Len Brown,
kilobyte, Rik van Riel, yuval shaia, Stefan Hajnoczi,
Igor Mammedov, lcapitulino, Nitesh Narayan Lal <nila>
In-Reply-To: <CAPcyv4i1FjBzfcWtzeCvj-giHa9xaX0dMQ==q4LZC_L8VEqkHg@mail.gmail.com>
> >
> > This patch adds functionality to perform flush from guest
> > to host over VIRTIO. We are registering a callback based
> > on 'nd_region' type. virtio_pmem driver requires this special
> > flush function. For rest of the region types we are registering
> > existing flush function. Report error returned by host fsync
> > failure to userspace.
> >
> > This also handles asynchronous flush requests from the block layer
> > by creating a child bio and chaining it with parent bio.
> >
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---bio_chain Dan williams
> [..]
> > diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
> > index b4ef7d9ff22e..fb1041ab32a6 100644
> > --- a/drivers/nvdimm/region_devs.c
> > +++ b/drivers/nvdimm/region_devs.c
> > @@ -295,7 +295,9 @@ static ssize_t deep_flush_store(struct device *dev,
> > struct device_attribute *att
> > return rc;
> > if (!flush)
> > return -EINVAL;
> > - nvdimm_flush(nd_region);
> > + rc = nvdimm_flush(nd_region, NULL, false);
> > + if (rc)
> > + return rc;
> >
> > return len;
> > }
> > @@ -1085,6 +1087,11 @@ static struct nd_region *nd_region_create(struct
> > nvdimm_bus *nvdimm_bus,
> > dev->of_node = ndr_desc->of_node;
> > nd_region->ndr_size = resource_size(ndr_desc->res);
> > nd_region->ndr_start = ndr_desc->res->start;
> > + if (ndr_desc->flush)
> > + nd_region->flush = ndr_desc->flush;
> > + else
> > + nd_region->flush = generic_nvdimm_flush;
> > +
> > nd_device_register(dev);
> >
> > return nd_region;
> > @@ -1125,11 +1132,36 @@ struct nd_region
> > *nvdimm_volatile_region_create(struct nvdimm_bus *nvdimm_bus,
> > }
> > EXPORT_SYMBOL_GPL(nvdimm_volatile_region_create);
> >
> > +int nvdimm_flush(struct nd_region *nd_region, struct bio *bio, bool async)
> > +{
>
> I don't quite see the point of the 'async' argument. All the usages of
> this routine are either
>
> nvdimm_flush(nd_region, bio, true)
> ...or:
> nvdimm_flush(nd_region, NULL, false)
Agree.
>
> ...so why not gate async behavior on the presence of the 'bio' argument?
Sure.
>
>
> > + int rc = 0;
> > +
> > + /* Create child bio for asynchronous flush and chain with
> > + * parent bio. Otherwise directly call nd_region flush.
> > + */
> > + if (async && bio->bi_iter.bi_sector != -1) {
> > +
> > + struct bio *child = bio_alloc(GFP_ATOMIC, 0);
> > +
> > + if (!child)
> > + return -ENOMEM;
> > + bio_copy_dev(child, bio);
> > + child->bi_opf = REQ_PREFLUSH;
> > + child->bi_iter.bi_sector = -1;
> > + bio_chain(child, bio);
> > + submit_bio(child);
>
> I understand how this works, but it's a bit too "magical" for my
> taste. I would prefer that all flush implementations take an optional
> 'bio' argument rather than rely on the make_request implementation to
> stash the bio away on a driver specific list.
I did this to make use of "bio_chain" for chaining child bio for async flush
suggested [1]. Are you saying to remove this and just call "flush" based on
bio argument? Or I implemented the 'bio_chain' request entirely wrong?
[1] https://lkml.org/lkml/2018/9/27/1028
>
> > + } else {
> > + if (nd_region->flush(nd_region))
> > + rc = -EIO;
>
> Given the common case wants to be fast and synchronous I think we
> should try to avoid retpoline overhead by default. So something like
> this:
>
> if (nd_region->flush == generic_nvdimm_flush)
> rc = generic_nvdimm_flush(...);
Sure.
Thanks,
Pankaj
>
>
^ 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