* virtio remoteproc device
@ 2018-04-20 16:53 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2018-04-20 16:53 UTC (permalink / raw)
To: linux-remoteproc; +Cc: Ohad Ben-Cohen, Bjorn Andersson, virtualization
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
^ permalink raw reply [flat|nested] 14+ messages in thread* virtio remoteproc device @ 2018-04-20 16:53 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-20 16:53 UTC (permalink / raw) To: linux-remoteproc; +Cc: Ohad Ben-Cohen, virtualization, Bjorn Andersson 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device 2018-04-20 16:53 ` Michael S. Tsirkin @ 2018-04-22 4:08 ` Anup Patel -1 siblings, 0 replies; 14+ messages in thread From: Anup Patel @ 2018-04-22 4:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson, virtualization 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device @ 2018-04-22 4:08 ` Anup Patel 0 siblings, 0 replies; 14+ messages in thread From: Anup Patel @ 2018-04-22 4:08 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ohad Ben-Cohen, virtualization, linux-remoteproc, Bjorn Andersson 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: virtio remoteproc device 2018-04-22 4:08 ` Anup Patel @ 2018-04-23 8:55 ` Loic PALLARDY -1 siblings, 0 replies; 14+ messages in thread From: Loic PALLARDY @ 2018-04-23 8:55 UTC (permalink / raw) To: Anup Patel, Michael S. Tsirkin Cc: linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson, virtualization > -----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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: virtio remoteproc device @ 2018-04-23 8:55 ` Loic PALLARDY 0 siblings, 0 replies; 14+ messages in thread From: Loic PALLARDY @ 2018-04-23 8:55 UTC (permalink / raw) To: Anup Patel, Michael S. Tsirkin Cc: Ohad Ben-Cohen, virtualization, linux-remoteproc, Bjorn Andersson > -----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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device 2018-04-23 8:55 ` Loic PALLARDY @ 2018-04-23 11:52 ` Michael S. Tsirkin -1 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 11:52 UTC (permalink / raw) To: Loic PALLARDY Cc: Anup Patel, linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson, virtualization 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device @ 2018-04-23 11:52 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 11:52 UTC (permalink / raw) To: Loic PALLARDY Cc: Ohad Ben-Cohen, Anup Patel, virtualization, linux-remoteproc, Bjorn Andersson 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device 2018-04-23 8:55 ` Loic PALLARDY @ 2018-04-23 19:40 ` Michael S. Tsirkin -1 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 19:40 UTC (permalink / raw) To: Loic PALLARDY Cc: Anup Patel, linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson, virtualization 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device @ 2018-04-23 19:40 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 19:40 UTC (permalink / raw) To: Loic PALLARDY Cc: Ohad Ben-Cohen, Anup Patel, virtualization, linux-remoteproc, Bjorn Andersson 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: virtio remoteproc device 2018-04-23 19:40 ` Michael S. Tsirkin @ 2018-04-23 20:45 ` Loic PALLARDY -1 siblings, 0 replies; 14+ messages in thread From: Loic PALLARDY @ 2018-04-23 20:45 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Anup Patel, linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson <bjorn.andersson@linaro.org>, virtualization@lists.linux-foundation.org > -----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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: virtio remoteproc device @ 2018-04-23 20:45 ` Loic PALLARDY 0 siblings, 0 replies; 14+ messages in thread From: Loic PALLARDY @ 2018-04-23 20:45 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ohad Ben-Cohen, Anup Patel, virtualization, linux-remoteproc, Bjorn Andersson > -----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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device 2018-04-23 20:45 ` Loic PALLARDY @ 2018-04-23 21:02 ` Michael S. Tsirkin -1 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 21:02 UTC (permalink / raw) To: Loic PALLARDY Cc: Anup Patel, linux-remoteproc, Ohad Ben-Cohen, Bjorn Andersson, virtualization 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: virtio remoteproc device @ 2018-04-23 21:02 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2018-04-23 21:02 UTC (permalink / raw) To: Loic PALLARDY Cc: Ohad Ben-Cohen, Anup Patel, virtualization, linux-remoteproc, Bjorn Andersson 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-04-23 21:02 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-20 16:53 virtio remoteproc device Michael S. Tsirkin 2018-04-20 16:53 ` Michael S. Tsirkin 2018-04-22 4:08 ` Anup Patel 2018-04-22 4:08 ` Anup Patel 2018-04-23 8:55 ` Loic PALLARDY 2018-04-23 8:55 ` Loic PALLARDY 2018-04-23 11:52 ` Michael S. Tsirkin 2018-04-23 11:52 ` Michael S. Tsirkin 2018-04-23 19:40 ` Michael S. Tsirkin 2018-04-23 19:40 ` Michael S. Tsirkin 2018-04-23 20:45 ` Loic PALLARDY 2018-04-23 20:45 ` Loic PALLARDY 2018-04-23 21:02 ` Michael S. Tsirkin 2018-04-23 21:02 ` Michael S. Tsirkin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.