virtio remoteproc device

virtio remoteproc device

[Michael S. Tsirkin]

Hello!
I note the following in the serial console:

      if (is_rproc_serial(vdev)) {
                /*
                 * Allocate DMA memory from ancestor. When a virtio
                 * device is created by remoteproc, the DMA memory is
                 * associated with the grandparent device:
                 * vdev => rproc => platform-dev.
                 */
                if (!vdev->dev.parent || !vdev->dev.parent->parent)
                        goto free_buf;
                buf->dev = vdev->dev.parent->parent;

                /* Increase device refcnt to avoid freeing it */
                get_device(buf->dev);
                buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
                                              GFP_KERNEL);
        }

Added here:
	commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
	Author: Sjur Br�ndeland <sjur.brandeland@stericsson.com>
	Date:   Fri Dec 14 14:40:51 2012 +1030

    virtio_console: Add support for remoteproc serial


I am not familiar with rproc so I have a question:
why is it required to use coherent memory here,
and why through a grandparent device?

Would it work to instead change vring_use_dma_api
to whitelist rproc (like we do for xen)?

I can sent a patch for your testing.
Thanks!

-- 
MST

virtio remoteproc device

[Michael S. Tsirkin]

Hello!
I note the following in the serial console:

      if (is_rproc_serial(vdev)) {
                /*
                 * Allocate DMA memory from ancestor. When a virtio
                 * device is created by remoteproc, the DMA memory is
                 * associated with the grandparent device:
                 * vdev => rproc => platform-dev.
                 */
                if (!vdev->dev.parent || !vdev->dev.parent->parent)
                        goto free_buf;
                buf->dev = vdev->dev.parent->parent;

                /* Increase device refcnt to avoid freeing it */
                get_device(buf->dev);
                buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
                                              GFP_KERNEL);
        }

Added here:
	commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
	Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
	Date:   Fri Dec 14 14:40:51 2012 +1030

    virtio_console: Add support for remoteproc serial


I am not familiar with rproc so I have a question:
why is it required to use coherent memory here,
and why through a grandparent device?

Would it work to instead change vring_use_dma_api
to whitelist rproc (like we do for xen)?

I can sent a patch for your testing.
Thanks!

-- 
MST

Re: virtio remoteproc device

[Anup Patel]

On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Hello!
> I note the following in the serial console:
>
>       if (is_rproc_serial(vdev)) {
>                 /*
>                  * Allocate DMA memory from ancestor. When a virtio
>                  * device is created by remoteproc, the DMA memory is
>                  * associated with the grandparent device:
>                  * vdev => rproc => platform-dev.
>                  */
>                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
>                         goto free_buf;
>                 buf->dev = vdev->dev.parent->parent;
>
>                 /* Increase device refcnt to avoid freeing it */
>                 get_device(buf->dev);
>                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
>                                               GFP_KERNEL);
>         }
>
> Added here:
>         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
>         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
>         Date:   Fri Dec 14 14:40:51 2012 +1030
>
>     virtio_console: Add support for remoteproc serial
>
>
> I am not familiar with rproc so I have a question:
> why is it required to use coherent memory here,
> and why through a grandparent device?

I faced similar issue when I tried VirtIO RPMSG bus over
VirtIO MMIO transport.

Here's my fix for VirtIO RPMSG bus driver:
https://patchwork.kernel.org/patch/10155145/

Regards,
Anup

Re: virtio remoteproc device

[Anup Patel]

On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Hello!
> I note the following in the serial console:
>
>       if (is_rproc_serial(vdev)) {
>                 /*
>                  * Allocate DMA memory from ancestor. When a virtio
>                  * device is created by remoteproc, the DMA memory is
>                  * associated with the grandparent device:
>                  * vdev => rproc => platform-dev.
>                  */
>                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
>                         goto free_buf;
>                 buf->dev = vdev->dev.parent->parent;
>
>                 /* Increase device refcnt to avoid freeing it */
>                 get_device(buf->dev);
>                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
>                                               GFP_KERNEL);
>         }
>
> Added here:
>         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
>         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
>         Date:   Fri Dec 14 14:40:51 2012 +1030
>
>     virtio_console: Add support for remoteproc serial
>
>
> I am not familiar with rproc so I have a question:
> why is it required to use coherent memory here,
> and why through a grandparent device?

I faced similar issue when I tried VirtIO RPMSG bus over
VirtIO MMIO transport.

Here's my fix for VirtIO RPMSG bus driver:
https://patchwork.kernel.org/patch/10155145/

Regards,
Anup
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: virtio remoteproc device

[Loic PALLARDY]

> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> owner@vger.kernel.org] On Behalf Of Anup Patel
> Sent: Sunday, April 22, 2018 6:08 AM
> To: Michael S. Tsirkin <mst@redhat.com>
> Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> virtualization@lists.linux-foundation.org
> Subject: Re: virtio remoteproc device
> 
> On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> wrote:
> > Hello!
> > I note the following in the serial console:
> >
> >       if (is_rproc_serial(vdev)) {
> >                 /*
> >                  * Allocate DMA memory from ancestor. When a virtio
> >                  * device is created by remoteproc, the DMA memory is
> >                  * associated with the grandparent device:
> >                  * vdev => rproc => platform-dev.
> >                  */
> >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> >                         goto free_buf;
> >                 buf->dev = vdev->dev.parent->parent;
> >
> >                 /* Increase device refcnt to avoid freeing it */
> >                 get_device(buf->dev);
> >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> >                                               GFP_KERNEL);
> >         }
> >
> > Added here:
> >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> >         Date:   Fri Dec 14 14:40:51 2012 +1030
> >
> >     virtio_console: Add support for remoteproc serial
> >
> >
> > I am not familiar with rproc so I have a question:
> > why is it required to use coherent memory here,
> > and why through a grandparent device?
> 
> I faced similar issue when I tried VirtIO RPMSG bus over
> VirtIO MMIO transport.
> 
> Here's my fix for VirtIO RPMSG bus driver:
> https://patchwork.kernel.org/patch/10155145/

Hi Anup and Michael,

I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.

Regards,
Loic

[1] https://lkml.org/lkml/2018/3/1/644

> 
> Regards,
> Anup
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: virtio remoteproc device

[Loic PALLARDY]

> -----Original Message-----
> From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> owner@vger.kernel.org] On Behalf Of Anup Patel
> Sent: Sunday, April 22, 2018 6:08 AM
> To: Michael S. Tsirkin <mst@redhat.com>
> Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> virtualization@lists.linux-foundation.org
> Subject: Re: virtio remoteproc device
> 
> On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> wrote:
> > Hello!
> > I note the following in the serial console:
> >
> >       if (is_rproc_serial(vdev)) {
> >                 /*
> >                  * Allocate DMA memory from ancestor. When a virtio
> >                  * device is created by remoteproc, the DMA memory is
> >                  * associated with the grandparent device:
> >                  * vdev => rproc => platform-dev.
> >                  */
> >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> >                         goto free_buf;
> >                 buf->dev = vdev->dev.parent->parent;
> >
> >                 /* Increase device refcnt to avoid freeing it */
> >                 get_device(buf->dev);
> >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> >                                               GFP_KERNEL);
> >         }
> >
> > Added here:
> >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> >         Date:   Fri Dec 14 14:40:51 2012 +1030
> >
> >     virtio_console: Add support for remoteproc serial
> >
> >
> > I am not familiar with rproc so I have a question:
> > why is it required to use coherent memory here,
> > and why through a grandparent device?
> 
> I faced similar issue when I tried VirtIO RPMSG bus over
> VirtIO MMIO transport.
> 
> Here's my fix for VirtIO RPMSG bus driver:
> https://patchwork.kernel.org/patch/10155145/

Hi Anup and Michael,

I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.

Regards,
Loic

[1] https://lkml.org/lkml/2018/3/1/644

> 
> Regards,
> Anup
> --
> To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> > owner@vger.kernel.org] On Behalf Of Anup Patel
> > Sent: Sunday, April 22, 2018 6:08 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> > > Hello!
> > > I note the following in the serial console:
> > >
> > >       if (is_rproc_serial(vdev)) {
> > >                 /*
> > >                  * Allocate DMA memory from ancestor. When a virtio
> > >                  * device is created by remoteproc, the DMA memory is
> > >                  * associated with the grandparent device:
> > >                  * vdev => rproc => platform-dev.
> > >                  */
> > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > >                         goto free_buf;
> > >                 buf->dev = vdev->dev.parent->parent;
> > >
> > >                 /* Increase device refcnt to avoid freeing it */
> > >                 get_device(buf->dev);
> > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> > >                                               GFP_KERNEL);
> > >         }
> > >
> > > Added here:
> > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > >         Author: Sjur Br�ndeland <sjur.brandeland@stericsson.com>
> > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > >
> > >     virtio_console: Add support for remoteproc serial
> > >
> > >
> > > I am not familiar with rproc so I have a question:
> > > why is it required to use coherent memory here,
> > > and why through a grandparent device?
> > 
> > I faced similar issue when I tried VirtIO RPMSG bus over
> > VirtIO MMIO transport.
> > 
> > Here's my fix for VirtIO RPMSG bus driver:
> > https://patchwork.kernel.org/patch/10155145/
> 
> Hi Anup and Michael,
> 
> I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
> It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
> Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.
> 
> Regards,
> Loic
> 
> [1] https://lkml.org/lkml/2018/3/1/644

Thanks!
Based on that patch, can remoteproc force VIRTIO_F_IOMMU_PLATFORM
in features and then drop special-casing of dma completely?

> > 
> > Regards,
> > Anup
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> > owner@vger.kernel.org] On Behalf Of Anup Patel
> > Sent: Sunday, April 22, 2018 6:08 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> > > Hello!
> > > I note the following in the serial console:
> > >
> > >       if (is_rproc_serial(vdev)) {
> > >                 /*
> > >                  * Allocate DMA memory from ancestor. When a virtio
> > >                  * device is created by remoteproc, the DMA memory is
> > >                  * associated with the grandparent device:
> > >                  * vdev => rproc => platform-dev.
> > >                  */
> > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > >                         goto free_buf;
> > >                 buf->dev = vdev->dev.parent->parent;
> > >
> > >                 /* Increase device refcnt to avoid freeing it */
> > >                 get_device(buf->dev);
> > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> > >                                               GFP_KERNEL);
> > >         }
> > >
> > > Added here:
> > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > >
> > >     virtio_console: Add support for remoteproc serial
> > >
> > >
> > > I am not familiar with rproc so I have a question:
> > > why is it required to use coherent memory here,
> > > and why through a grandparent device?
> > 
> > I faced similar issue when I tried VirtIO RPMSG bus over
> > VirtIO MMIO transport.
> > 
> > Here's my fix for VirtIO RPMSG bus driver:
> > https://patchwork.kernel.org/patch/10155145/
> 
> Hi Anup and Michael,
> 
> I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
> It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
> Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.
> 
> Regards,
> Loic
> 
> [1] https://lkml.org/lkml/2018/3/1/644

Thanks!
Based on that patch, can remoteproc force VIRTIO_F_IOMMU_PLATFORM
in features and then drop special-casing of dma completely?

> > 
> > Regards,
> > Anup
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> > owner@vger.kernel.org] On Behalf Of Anup Patel
> > Sent: Sunday, April 22, 2018 6:08 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> > > Hello!
> > > I note the following in the serial console:
> > >
> > >       if (is_rproc_serial(vdev)) {
> > >                 /*
> > >                  * Allocate DMA memory from ancestor. When a virtio
> > >                  * device is created by remoteproc, the DMA memory is
> > >                  * associated with the grandparent device:
> > >                  * vdev => rproc => platform-dev.
> > >                  */
> > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > >                         goto free_buf;
> > >                 buf->dev = vdev->dev.parent->parent;
> > >
> > >                 /* Increase device refcnt to avoid freeing it */
> > >                 get_device(buf->dev);
> > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> > >                                               GFP_KERNEL);
> > >         }
> > >
> > > Added here:
> > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > >         Author: Sjur Br�ndeland <sjur.brandeland@stericsson.com>
> > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > >
> > >     virtio_console: Add support for remoteproc serial
> > >
> > >
> > > I am not familiar with rproc so I have a question:
> > > why is it required to use coherent memory here,
> > > and why through a grandparent device?
> > 
> > I faced similar issue when I tried VirtIO RPMSG bus over
> > VirtIO MMIO transport.
> > 
> > Here's my fix for VirtIO RPMSG bus driver:
> > https://patchwork.kernel.org/patch/10155145/
> 
> Hi Anup and Michael,
> 
> I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
> It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
> Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.
> 
> Regards,
> Loic
> 
> [1] https://lkml.org/lkml/2018/3/1/644
> 
> > 
> > Regards,
> > Anup
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

So on top of your patches, can we just force use of DMA API
and drop special casting?
E.g. maybe something like the below - completely untested - patch?
Does it work for you?


diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 2108551..6c6767c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -40,7 +40,8 @@
 #include <linux/dma-mapping.h>
 #include "../tty/hvc/hvc_console.h"
 
-#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
+//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
+#define is_rproc_enabled 0
 
 /*
  * This is a global struct for storing common data for all the devices
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index b0633fd..4a13ca2 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct virtio_device *vdev)
 
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 
-	return rsc->dfeatures;
+	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
 }
 
 static int rproc_virtio_finalize_features(struct virtio_device *vdev)
@@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 	vring_transport_features(vdev);
 
 	/* Make sure we don't have any features > 32 bits! */
-	BUG_ON((u32)vdev->features != vdev->features);
+	//BUG_ON((u32)vdev->features != vdev->features);
 
 	/*
 	 * Remember the finalized features of our vdev, and provide it

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-remoteproc-
> > owner@vger.kernel.org] On Behalf Of Anup Patel
> > Sent: Sunday, April 22, 2018 6:08 AM
> > To: Michael S. Tsirkin <mst@redhat.com>
> > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> > > Hello!
> > > I note the following in the serial console:
> > >
> > >       if (is_rproc_serial(vdev)) {
> > >                 /*
> > >                  * Allocate DMA memory from ancestor. When a virtio
> > >                  * device is created by remoteproc, the DMA memory is
> > >                  * associated with the grandparent device:
> > >                  * vdev => rproc => platform-dev.
> > >                  */
> > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > >                         goto free_buf;
> > >                 buf->dev = vdev->dev.parent->parent;
> > >
> > >                 /* Increase device refcnt to avoid freeing it */
> > >                 get_device(buf->dev);
> > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf->dma,
> > >                                               GFP_KERNEL);
> > >         }
> > >
> > > Added here:
> > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > >
> > >     virtio_console: Add support for remoteproc serial
> > >
> > >
> > > I am not familiar with rproc so I have a question:
> > > why is it required to use coherent memory here,
> > > and why through a grandparent device?
> > 
> > I faced similar issue when I tried VirtIO RPMSG bus over
> > VirtIO MMIO transport.
> > 
> > Here's my fix for VirtIO RPMSG bus driver:
> > https://patchwork.kernel.org/patch/10155145/
> 
> Hi Anup and Michael,
> 
> I pushed a series to modify virtio device allocation in remoteproc. Please see [1].
> It allows to remove allocation based on "grand-parent" device in the case of virtio device allocated by remoteproc.
> Virto_console patch missing, only virtio_rpmsg modification proposed. I can add it in next version.
> 
> Regards,
> Loic
> 
> [1] https://lkml.org/lkml/2018/3/1/644
> 
> > 
> > Regards,
> > Anup
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

So on top of your patches, can we just force use of DMA API
and drop special casting?
E.g. maybe something like the below - completely untested - patch?
Does it work for you?


diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 2108551..6c6767c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -40,7 +40,8 @@
 #include <linux/dma-mapping.h>
 #include "../tty/hvc/hvc_console.h"
 
-#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
+//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
+#define is_rproc_enabled 0
 
 /*
  * This is a global struct for storing common data for all the devices
diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index b0633fd..4a13ca2 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct virtio_device *vdev)
 
 	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
 
-	return rsc->dfeatures;
+	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
 }
 
 static int rproc_virtio_finalize_features(struct virtio_device *vdev)
@@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct virtio_device *vdev)
 	vring_transport_features(vdev);
 
 	/* Make sure we don't have any features > 32 bits! */
-	BUG_ON((u32)vdev->features != vdev->features);
+	//BUG_ON((u32)vdev->features != vdev->features);
 
 	/*
 	 * Remember the finalized features of our vdev, and provide it

RE: virtio remoteproc device

[Loic PALLARDY]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, April 23, 2018 9:41 PM
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: Anup Patel <anup@brainfault.org>; linux-remoteproc@vger.kernel.org;
> Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> <bjorn.andersson@linaro.org>; virtualization@lists.linux-foundation.org
> Subject: Re: virtio remoteproc device
> 
> On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-
> remoteproc-
> > > owner@vger.kernel.org] On Behalf Of Anup Patel
> > > Sent: Sunday, April 22, 2018 6:08 AM
> > > To: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > > virtualization@lists.linux-foundation.org
> > > Subject: Re: virtio remoteproc device
> > >
> > > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > > wrote:
> > > > Hello!
> > > > I note the following in the serial console:
> > > >
> > > >       if (is_rproc_serial(vdev)) {
> > > >                 /*
> > > >                  * Allocate DMA memory from ancestor. When a virtio
> > > >                  * device is created by remoteproc, the DMA memory is
> > > >                  * associated with the grandparent device:
> > > >                  * vdev => rproc => platform-dev.
> > > >                  */
> > > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > > >                         goto free_buf;
> > > >                 buf->dev = vdev->dev.parent->parent;
> > > >
> > > >                 /* Increase device refcnt to avoid freeing it */
> > > >                 get_device(buf->dev);
> > > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf-
> >dma,
> > > >                                               GFP_KERNEL);
> > > >         }
> > > >
> > > > Added here:
> > > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > > >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > > >
> > > >     virtio_console: Add support for remoteproc serial
> > > >
> > > >
> > > > I am not familiar with rproc so I have a question:
> > > > why is it required to use coherent memory here,
> > > > and why through a grandparent device?
> > >
> > > I faced similar issue when I tried VirtIO RPMSG bus over
> > > VirtIO MMIO transport.
> > >
> > > Here's my fix for VirtIO RPMSG bus driver:
> > > https://patchwork.kernel.org/patch/10155145/
> >
> > Hi Anup and Michael,
> >
> > I pushed a series to modify virtio device allocation in remoteproc. Please
> see [1].
> > It allows to remove allocation based on "grand-parent" device in the case
> of virtio device allocated by remoteproc.
> > Virto_console patch missing, only virtio_rpmsg modification proposed. I can
> add it in next version.
> >
> > Regards,
> > Loic
> >
> > [1] https://lkml.org/lkml/2018/3/1/644
> >
> > >
> > > Regards,
> > > Anup
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> remoteproc" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> So on top of your patches, can we just force use of DMA API
> and drop special casting?
> E.g. maybe something like the below - completely untested - patch?
> Does it work for you?
> 

Just have a look to VIRTIO_F_IOMMU_PLATFORM usage.
If we activate it, this will change virtio_ring behavior, allowing to rely on dma api, but need to have deeper look to virtio_console impacts.

For sure, if you remove/disable rproc specific code in virtio_console, alloc_buf will rely on kmalloc instead of dma_alloc_coherent. As some coprocessors doesn't have access to complete memory map and in some case no access to DDR memory, buffer copy will have to be done in dedicated vdev memory before adding it to vring.
Specific rproc case was added to avoid copy and directly using shared memory for buffer allocation.

Regards,
Loic
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 2108551..6c6767c 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -40,7 +40,8 @@
>  #include <linux/dma-mapping.h>
>  #include "../tty/hvc/hvc_console.h"
> 
> -#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> +//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> +#define is_rproc_enabled 0
> 
>  /*
>   * This is a global struct for storing common data for all the devices
> diff --git a/drivers/remoteproc/remoteproc_virtio.c
> b/drivers/remoteproc/remoteproc_virtio.c
> index b0633fd..4a13ca2 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct
> virtio_device *vdev)
> 
>  	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
> 
> -	return rsc->dfeatures;
> +	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
>  }
> 
>  static int rproc_virtio_finalize_features(struct virtio_device *vdev)
> @@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct
> virtio_device *vdev)
>  	vring_transport_features(vdev);
> 
>  	/* Make sure we don't have any features > 32 bits! */
> -	BUG_ON((u32)vdev->features != vdev->features);
> +	//BUG_ON((u32)vdev->features != vdev->features);
> 
>  	/*
>  	 * Remember the finalized features of our vdev, and provide it

RE: virtio remoteproc device

[Loic PALLARDY]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, April 23, 2018 9:41 PM
> To: Loic PALLARDY <loic.pallardy@st.com>
> Cc: Anup Patel <anup@brainfault.org>; linux-remoteproc@vger.kernel.org;
> Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> <bjorn.andersson@linaro.org>; virtualization@lists.linux-foundation.org
> Subject: Re: virtio remoteproc device
> 
> On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> >
> >
> > > -----Original Message-----
> > > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-
> remoteproc-
> > > owner@vger.kernel.org] On Behalf Of Anup Patel
> > > Sent: Sunday, April 22, 2018 6:08 AM
> > > To: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > > virtualization@lists.linux-foundation.org
> > > Subject: Re: virtio remoteproc device
> > >
> > > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > > wrote:
> > > > Hello!
> > > > I note the following in the serial console:
> > > >
> > > >       if (is_rproc_serial(vdev)) {
> > > >                 /*
> > > >                  * Allocate DMA memory from ancestor. When a virtio
> > > >                  * device is created by remoteproc, the DMA memory is
> > > >                  * associated with the grandparent device:
> > > >                  * vdev => rproc => platform-dev.
> > > >                  */
> > > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > > >                         goto free_buf;
> > > >                 buf->dev = vdev->dev.parent->parent;
> > > >
> > > >                 /* Increase device refcnt to avoid freeing it */
> > > >                 get_device(buf->dev);
> > > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf-
> >dma,
> > > >                                               GFP_KERNEL);
> > > >         }
> > > >
> > > > Added here:
> > > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > > >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > > >
> > > >     virtio_console: Add support for remoteproc serial
> > > >
> > > >
> > > > I am not familiar with rproc so I have a question:
> > > > why is it required to use coherent memory here,
> > > > and why through a grandparent device?
> > >
> > > I faced similar issue when I tried VirtIO RPMSG bus over
> > > VirtIO MMIO transport.
> > >
> > > Here's my fix for VirtIO RPMSG bus driver:
> > > https://patchwork.kernel.org/patch/10155145/
> >
> > Hi Anup and Michael,
> >
> > I pushed a series to modify virtio device allocation in remoteproc. Please
> see [1].
> > It allows to remove allocation based on "grand-parent" device in the case
> of virtio device allocated by remoteproc.
> > Virto_console patch missing, only virtio_rpmsg modification proposed. I can
> add it in next version.
> >
> > Regards,
> > Loic
> >
> > [1] https://lkml.org/lkml/2018/3/1/644
> >
> > >
> > > Regards,
> > > Anup
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> remoteproc" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> So on top of your patches, can we just force use of DMA API
> and drop special casting?
> E.g. maybe something like the below - completely untested - patch?
> Does it work for you?
> 

Just have a look to VIRTIO_F_IOMMU_PLATFORM usage.
If we activate it, this will change virtio_ring behavior, allowing to rely on dma api, but need to have deeper look to virtio_console impacts.

For sure, if you remove/disable rproc specific code in virtio_console, alloc_buf will rely on kmalloc instead of dma_alloc_coherent. As some coprocessors doesn't have access to complete memory map and in some case no access to DDR memory, buffer copy will have to be done in dedicated vdev memory before adding it to vring.
Specific rproc case was added to avoid copy and directly using shared memory for buffer allocation.

Regards,
Loic
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 2108551..6c6767c 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -40,7 +40,8 @@
>  #include <linux/dma-mapping.h>
>  #include "../tty/hvc/hvc_console.h"
> 
> -#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> +//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> +#define is_rproc_enabled 0
> 
>  /*
>   * This is a global struct for storing common data for all the devices
> diff --git a/drivers/remoteproc/remoteproc_virtio.c
> b/drivers/remoteproc/remoteproc_virtio.c
> index b0633fd..4a13ca2 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct
> virtio_device *vdev)
> 
>  	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
> 
> -	return rsc->dfeatures;
> +	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
>  }
> 
>  static int rproc_virtio_finalize_features(struct virtio_device *vdev)
> @@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct
> virtio_device *vdev)
>  	vring_transport_features(vdev);
> 
>  	/* Make sure we don't have any features > 32 bits! */
> -	BUG_ON((u32)vdev->features != vdev->features);
> +	//BUG_ON((u32)vdev->features != vdev->features);
> 
>  	/*
>  	 * Remember the finalized features of our vdev, and provide it

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:45:57PM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, April 23, 2018 9:41 PM
> > To: Loic PALLARDY <loic.pallardy@st.com>
> > Cc: Anup Patel <anup@brainfault.org>; linux-remoteproc@vger.kernel.org;
> > Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> > <bjorn.andersson@linaro.org>; virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-
> > remoteproc-
> > > > owner@vger.kernel.org] On Behalf Of Anup Patel
> > > > Sent: Sunday, April 22, 2018 6:08 AM
> > > > To: Michael S. Tsirkin <mst@redhat.com>
> > > > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > > > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > > > virtualization@lists.linux-foundation.org
> > > > Subject: Re: virtio remoteproc device
> > > >
> > > > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > > > wrote:
> > > > > Hello!
> > > > > I note the following in the serial console:
> > > > >
> > > > >       if (is_rproc_serial(vdev)) {
> > > > >                 /*
> > > > >                  * Allocate DMA memory from ancestor. When a virtio
> > > > >                  * device is created by remoteproc, the DMA memory is
> > > > >                  * associated with the grandparent device:
> > > > >                  * vdev => rproc => platform-dev.
> > > > >                  */
> > > > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > > > >                         goto free_buf;
> > > > >                 buf->dev = vdev->dev.parent->parent;
> > > > >
> > > > >                 /* Increase device refcnt to avoid freeing it */
> > > > >                 get_device(buf->dev);
> > > > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf-
> > >dma,
> > > > >                                               GFP_KERNEL);
> > > > >         }
> > > > >
> > > > > Added here:
> > > > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > > > >         Author: Sjur Br�ndeland <sjur.brandeland@stericsson.com>
> > > > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > > > >
> > > > >     virtio_console: Add support for remoteproc serial
> > > > >
> > > > >
> > > > > I am not familiar with rproc so I have a question:
> > > > > why is it required to use coherent memory here,
> > > > > and why through a grandparent device?
> > > >
> > > > I faced similar issue when I tried VirtIO RPMSG bus over
> > > > VirtIO MMIO transport.
> > > >
> > > > Here's my fix for VirtIO RPMSG bus driver:
> > > > https://patchwork.kernel.org/patch/10155145/
> > >
> > > Hi Anup and Michael,
> > >
> > > I pushed a series to modify virtio device allocation in remoteproc. Please
> > see [1].
> > > It allows to remove allocation based on "grand-parent" device in the case
> > of virtio device allocated by remoteproc.
> > > Virto_console patch missing, only virtio_rpmsg modification proposed. I can
> > add it in next version.
> > >
> > > Regards,
> > > Loic
> > >
> > > [1] https://lkml.org/lkml/2018/3/1/644
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-
> > remoteproc" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > So on top of your patches, can we just force use of DMA API
> > and drop special casting?
> > E.g. maybe something like the below - completely untested - patch?
> > Does it work for you?
> > 
> 
> Just have a look to VIRTIO_F_IOMMU_PLATFORM usage.
> If we activate it, this will change virtio_ring behavior, allowing to rely on dma api, but need to have deeper look to virtio_console impacts.

Could you test the patch though?

> For sure, if you remove/disable rproc specific code in virtio_console, alloc_buf will rely on kmalloc instead of dma_alloc_coherent. As some coprocessors doesn't have access to complete memory map and in some case no access to DDR memory, buffer copy will have to be done in dedicated vdev memory before adding it to vring.

I'm guessing that for console an extra copy is not a big deal. How much
data is pushed there after all, right?

> Specific rproc case was added to avoid copy and directly using shared memory for buffer allocation.
> 
> Regards,
> Loic

I was under the impression that it was more to just make it work
somehow. But I might be wrong.

> > 
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 2108551..6c6767c 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -40,7 +40,8 @@
> >  #include <linux/dma-mapping.h>
> >  #include "../tty/hvc/hvc_console.h"
> > 
> > -#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> > +//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> > +#define is_rproc_enabled 0
> > 
> >  /*
> >   * This is a global struct for storing common data for all the devices
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index b0633fd..4a13ca2 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct
> > virtio_device *vdev)
> > 
> >  	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
> > 
> > -	return rsc->dfeatures;
> > +	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
> >  }
> > 
> >  static int rproc_virtio_finalize_features(struct virtio_device *vdev)
> > @@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct
> > virtio_device *vdev)
> >  	vring_transport_features(vdev);
> > 
> >  	/* Make sure we don't have any features > 32 bits! */
> > -	BUG_ON((u32)vdev->features != vdev->features);
> > +	//BUG_ON((u32)vdev->features != vdev->features);
> > 
> >  	/*
> >  	 * Remember the finalized features of our vdev, and provide it

Re: virtio remoteproc device

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 08:45:57PM +0000, Loic PALLARDY wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Monday, April 23, 2018 9:41 PM
> > To: Loic PALLARDY <loic.pallardy@st.com>
> > Cc: Anup Patel <anup@brainfault.org>; linux-remoteproc@vger.kernel.org;
> > Ohad Ben-Cohen <ohad@wizery.com>; Bjorn Andersson
> > <bjorn.andersson@linaro.org>; virtualization@lists.linux-foundation.org
> > Subject: Re: virtio remoteproc device
> > 
> > On Mon, Apr 23, 2018 at 08:55:57AM +0000, Loic PALLARDY wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: linux-remoteproc-owner@vger.kernel.org [mailto:linux-
> > remoteproc-
> > > > owner@vger.kernel.org] On Behalf Of Anup Patel
> > > > Sent: Sunday, April 22, 2018 6:08 AM
> > > > To: Michael S. Tsirkin <mst@redhat.com>
> > > > Cc: linux-remoteproc@vger.kernel.org; Ohad Ben-Cohen
> > > > <ohad@wizery.com>; Bjorn Andersson <bjorn.andersson@linaro.org>;
> > > > virtualization@lists.linux-foundation.org
> > > > Subject: Re: virtio remoteproc device
> > > >
> > > > On Fri, Apr 20, 2018 at 10:23 PM, Michael S. Tsirkin <mst@redhat.com>
> > > > wrote:
> > > > > Hello!
> > > > > I note the following in the serial console:
> > > > >
> > > > >       if (is_rproc_serial(vdev)) {
> > > > >                 /*
> > > > >                  * Allocate DMA memory from ancestor. When a virtio
> > > > >                  * device is created by remoteproc, the DMA memory is
> > > > >                  * associated with the grandparent device:
> > > > >                  * vdev => rproc => platform-dev.
> > > > >                  */
> > > > >                 if (!vdev->dev.parent || !vdev->dev.parent->parent)
> > > > >                         goto free_buf;
> > > > >                 buf->dev = vdev->dev.parent->parent;
> > > > >
> > > > >                 /* Increase device refcnt to avoid freeing it */
> > > > >                 get_device(buf->dev);
> > > > >                 buf->buf = dma_alloc_coherent(buf->dev, buf_size, &buf-
> > >dma,
> > > > >                                               GFP_KERNEL);
> > > > >         }
> > > > >
> > > > > Added here:
> > > > >         commit 1b6370463e88b0c1c317de16d7b962acc1dab4f2
> > > > >         Author: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > > > >         Date:   Fri Dec 14 14:40:51 2012 +1030
> > > > >
> > > > >     virtio_console: Add support for remoteproc serial
> > > > >
> > > > >
> > > > > I am not familiar with rproc so I have a question:
> > > > > why is it required to use coherent memory here,
> > > > > and why through a grandparent device?
> > > >
> > > > I faced similar issue when I tried VirtIO RPMSG bus over
> > > > VirtIO MMIO transport.
> > > >
> > > > Here's my fix for VirtIO RPMSG bus driver:
> > > > https://patchwork.kernel.org/patch/10155145/
> > >
> > > Hi Anup and Michael,
> > >
> > > I pushed a series to modify virtio device allocation in remoteproc. Please
> > see [1].
> > > It allows to remove allocation based on "grand-parent" device in the case
> > of virtio device allocated by remoteproc.
> > > Virto_console patch missing, only virtio_rpmsg modification proposed. I can
> > add it in next version.
> > >
> > > Regards,
> > > Loic
> > >
> > > [1] https://lkml.org/lkml/2018/3/1/644
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-
> > remoteproc" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > So on top of your patches, can we just force use of DMA API
> > and drop special casting?
> > E.g. maybe something like the below - completely untested - patch?
> > Does it work for you?
> > 
> 
> Just have a look to VIRTIO_F_IOMMU_PLATFORM usage.
> If we activate it, this will change virtio_ring behavior, allowing to rely on dma api, but need to have deeper look to virtio_console impacts.

Could you test the patch though?

> For sure, if you remove/disable rproc specific code in virtio_console, alloc_buf will rely on kmalloc instead of dma_alloc_coherent. As some coprocessors doesn't have access to complete memory map and in some case no access to DDR memory, buffer copy will have to be done in dedicated vdev memory before adding it to vring.

I'm guessing that for console an extra copy is not a big deal. How much
data is pushed there after all, right?

> Specific rproc case was added to avoid copy and directly using shared memory for buffer allocation.
> 
> Regards,
> Loic

I was under the impression that it was more to just make it work
somehow. But I might be wrong.

> > 
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 2108551..6c6767c 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -40,7 +40,8 @@
> >  #include <linux/dma-mapping.h>
> >  #include "../tty/hvc/hvc_console.h"
> > 
> > -#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> > +//#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC)
> > +#define is_rproc_enabled 0
> > 
> >  /*
> >   * This is a global struct for storing common data for all the devices
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index b0633fd..4a13ca2 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -199,7 +199,7 @@ static u64 rproc_virtio_get_features(struct
> > virtio_device *vdev)
> > 
> >  	rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
> > 
> > -	return rsc->dfeatures;
> > +	return rsc->dfeatures | (1ULL << VIRTIO_F_IOMMU_PLATFORM);
> >  }
> > 
> >  static int rproc_virtio_finalize_features(struct virtio_device *vdev)
> > @@ -213,7 +213,7 @@ static int rproc_virtio_finalize_features(struct
> > virtio_device *vdev)
> >  	vring_transport_features(vdev);
> > 
> >  	/* Make sure we don't have any features > 32 bits! */
> > -	BUG_ON((u32)vdev->features != vdev->features);
> > +	//BUG_ON((u32)vdev->features != vdev->features);
> > 
> >  	/*
> >  	 * Remember the finalized features of our vdev, and provide it