Re: [PATCH 2/2] remoteproc: Add support for host virtio rings (vringh)
Hi Ohad, On Fri, Mar 15, 2013 at 10:29 AM, Erwan Yvin wrote: > From: Erwan Yvin > > Implement the vringh callback functions in order > to manage host virtio rings and handle kicks. > This allows virtio device to request host-virtio-rings. > > Signed-off-by: Erwan Yvin Any chance that you could review this in time for 3.10 merge window? This is the last missing piece for host vring support. The patches would have to be Acked/Reviewed by and should go via Rusty's tree. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] remoteproc: Add support for host virtio rings (vringh)
Hi Ohad, On Fri, Mar 15, 2013 at 10:29 AM, Erwan Yvin erwan.y...@stericsson.com wrote: From: Erwan Yvin erwan.y...@stericsson.com Implement the vringh callback functions in order to manage host virtio rings and handle kicks. This allows virtio device to request host-virtio-rings. Signed-off-by: Erwan Yvin erwan.y...@stericsson.com Any chance that you could review this in time for 3.10 merge window? This is the last missing piece for host vring support. The patches would have to be Acked/Reviewed by and should go via Rusty's tree. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CAIF: fix indentation for function arguments
On Wed, Mar 6, 2013 at 10:48 PM, Silviu-Mihai Popescu wrote: > This lines up function arguments on second and subsequent lines at the > first column after the openning parenthesis of the first line. > > Signed-off-by: Silviu-Mihai Popescu There is quite a bit of churn for stuff not even noticed by checkpatch --strict, but it does improve the style... Acked-by: Sjur Brændeland -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] CAIF: fix indentation for function arguments
On Wed, Mar 6, 2013 at 10:48 PM, Silviu-Mihai Popescu silviupopescu1...@gmail.com wrote: This lines up function arguments on second and subsequent lines at the first column after the openning parenthesis of the first line. Signed-off-by: Silviu-Mihai Popescu silviupopescu1...@gmail.com There is quite a bit of churn for stuff not even noticed by checkpatch --strict, but it does improve the style... Acked-by: Sjur Brændeland sjur.brandel...@stericsson.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
Hi Ohad, On Thu, Feb 21, 2013 at 6:55 PM, Ohad Ben-Cohen wrote: > On Thu, Feb 21, 2013 at 7:28 PM, Sjur Brændeland wrote: >> The motivation for using vringh was to avoid copying buffers >> when sending data from the modem to the host. > > I may be missing something here, but why do you need vringh for that? > > With rpmsg (which uses two regular vrings) both ends send huge amount > of data without copying it - we just send the pointers across. OK, We did carefully consider using the normal vrings, but concluded it was not doable. I'll try to give you some of the background that I can recall from top of my head. (I can dig out more if you're still not convinced :) The modem we're integrating with is a complex beast (multi mode modem LTE, HSPA+, etc). When a packet is received deep down in the radio stack, it allocates packet buffers using the internal slab allocator.The packet may contain anything from control, voice, or IP packets. If the packet contains IP-payload it will travel to a different asymmetric CPU that handles the IPC towards the modem. If the packet is not a IP-packet it will be processed internally and eventually freed. What we have done to integrate virtio is to inject the carveout into the the modem internal slab-allocator. Using the reversed ring allows us to introduce zero-copy for virtio with virtually no impact on the radio-stack, only a small change on the modem slab allocator. It supports dynamic size buffer allocation, and modem internal messaging using data allocated from the shared region. It also allows modem to manage it's own memory, without any dependency on host side allocators. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
Hi Ohad, > I was wondering - can you please explain your motivation for using > vringh in caif ? > > We have internally discussed supporting multiple remote processors > talking to each other using rpmsg, and in that scenario using vringh > can considerably simplifies the solution (no need to decide in advance > which side is the "guest" and which is the "host"). Was this the > general incentive in using vringh in caif too or something else? The motivation for using vringh was to avoid copying buffers when sending data from the modem to the host. By using vringh the modem can transfer datagrams to host only by transfering buffer pointers via the rings. We use a carveout to declare a memory area that is passed to the modem internal memory allocator. This way we get both modem and host to work on the shared memory region. For TX traffic we use normal virtio queues for the same reason. TX buffers can be handled without copying as well. We are seeing a nice performance gain on the modem side after implementing this. Host size zero-copy is more tricky. It's hard to handle ownership of buffers, if the modem crashes. But we might look into this in the future as well. Regards, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/9] remoteproc: Always perserve resource table data
Hi Ido, On Wed, Feb 20, 2013 at 11:46 AM, Ido Yariv wrote: > Hi Sjur, > > On Sun, Feb 10, 2013 at 12:39:12PM +0100, sjur.brandel...@stericsson.com > wrote: >> From: Sjur Brændeland >> >> Copy resource table from first to second firmware loading. >> After firmware is loaded to memory, update the vdevs resource >> pointer to the resource table kept in device memory. >> >> Signed-off-by: Sjur Brændeland >> --- >> drivers/remoteproc/remoteproc_core.c | 61 >> +++-- >> include/linux/remoteproc.h |3 ++ >> 2 files changed, 53 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index 14f40eb..13dc7b4 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -694,17 +694,16 @@ static rproc_handle_resource_t >> rproc_handle_notifyid_rsc[RSC_LAST] = { >> >> /* handle firmware resource entries before booting the remote processor */ >> static int >> -rproc_handle_resource(struct rproc *rproc, struct resource_table *table, >> - int len, >> - rproc_handle_resource_t handlers[RSC_LAST]) >> +rproc_handle_resource(struct rproc *rproc, int len, >> + rproc_handle_resource_t handlers[RSC_LAST]) >> { >> struct device *dev = >dev; >> rproc_handle_resource_t handler; >> int ret = 0, i; >> >> - for (i = 0; i < table->num; i++) { >> - int offset = table->offset[i]; >> - struct fw_rsc_hdr *hdr = (void *)table + offset; >> + for (i = 0; i < rproc->rsc->num; i++) { >> + int offset = rproc->rsc->offset[i]; >> + struct fw_rsc_hdr *hdr = (void *)rproc->rsc + offset; >> int avail = len - offset - sizeof(*hdr); >> void *rsc = (void *)hdr + sizeof(*hdr); >> >> @@ -783,9 +782,13 @@ static int rproc_fw_boot(struct rproc *rproc, const >> struct firmware *fw) >> { >> struct device *dev = >dev; >> const char *name = rproc->firmware; >> - struct resource_table *table; >> + struct rproc_vdev *rvdev; >> + struct resource_table *table, *devmem_rsc, *tmp; >> int ret, tablesz; >> >> + if (!rproc->rsc) >> + return -ENOMEM; >> + >> ret = rproc_fw_sanity_check(rproc, fw); >> if (ret) >> return ret; >> @@ -811,8 +814,17 @@ static int rproc_fw_boot(struct rproc *rproc, const >> struct firmware *fw) >> goto clean_up; >> } >> >> + /* Verify that resource table in loaded fw is unchanged */ >> + if (rproc->rsc_csum != ip_compute_csum(table, tablesz)) { >> + dev_err(dev, "resource checksum failed, fw changed?\n"); >> + ret = -EINVAL; >> + goto clean_up; >> + } >> + >> + > > Remove empty line? Ok. > >> /* handle fw resources which are required to boot rproc */ >> - ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_rsc); >> + ret = rproc_handle_resource(rproc, tablesz, >> + rproc_handle_rsc); > > This can fit in one line now :) Ok, > >> if (ret) { >> dev_err(dev, "Failed to process resources: %d\n", ret); >> goto clean_up; >> @@ -825,6 +837,26 @@ static int rproc_fw_boot(struct rproc *rproc, const >> struct firmware *fw) >> goto clean_up; >> } >> >> + /* Get the resource table address in device memory */ >> + devmem_rsc = rproc_get_rsctab_addr(rproc, fw); >> + >> + /* Copy the updated resource table to device memory */ >> + memcpy(devmem_rsc, rproc->rsc, tablesz); >> + >> + /* Free the copy of the resource table */ >> + tmp = rproc->rsc; >> + rproc->rsc = devmem_rsc; > > This is also set a few lines below, and we can probably do without tmp > altogether. > >> + kfree(tmp); > > If rproc_fw_boot is called more than once, kfree will try to free memory that > was not kmalloced, leading to a panic. > > Instead of freeing the pointer here, it should be kept until rproc is > released, reverting rproc->rsc to it every time shutdown is called. It > can be freed when rproc is finally released. Agree. I will free the copy in rproc_trig
Re: [PATCH 9/9] remoteproc: Always perserve resource table data
Hi Ido, On Wed, Feb 20, 2013 at 11:46 AM, Ido Yariv i...@wizery.com wrote: Hi Sjur, On Sun, Feb 10, 2013 at 12:39:12PM +0100, sjur.brandel...@stericsson.com wrote: From: Sjur Brændeland sjur.brandel...@stericsson.com Copy resource table from first to second firmware loading. After firmware is loaded to memory, update the vdevs resource pointer to the resource table kept in device memory. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 61 +++-- include/linux/remoteproc.h |3 ++ 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 14f40eb..13dc7b4 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -694,17 +694,16 @@ static rproc_handle_resource_t rproc_handle_notifyid_rsc[RSC_LAST] = { /* handle firmware resource entries before booting the remote processor */ static int -rproc_handle_resource(struct rproc *rproc, struct resource_table *table, - int len, - rproc_handle_resource_t handlers[RSC_LAST]) +rproc_handle_resource(struct rproc *rproc, int len, + rproc_handle_resource_t handlers[RSC_LAST]) { struct device *dev = rproc-dev; rproc_handle_resource_t handler; int ret = 0, i; - for (i = 0; i table-num; i++) { - int offset = table-offset[i]; - struct fw_rsc_hdr *hdr = (void *)table + offset; + for (i = 0; i rproc-rsc-num; i++) { + int offset = rproc-rsc-offset[i]; + struct fw_rsc_hdr *hdr = (void *)rproc-rsc + offset; int avail = len - offset - sizeof(*hdr); void *rsc = (void *)hdr + sizeof(*hdr); @@ -783,9 +782,13 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) { struct device *dev = rproc-dev; const char *name = rproc-firmware; - struct resource_table *table; + struct rproc_vdev *rvdev; + struct resource_table *table, *devmem_rsc, *tmp; int ret, tablesz; + if (!rproc-rsc) + return -ENOMEM; + ret = rproc_fw_sanity_check(rproc, fw); if (ret) return ret; @@ -811,8 +814,17 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) goto clean_up; } + /* Verify that resource table in loaded fw is unchanged */ + if (rproc-rsc_csum != ip_compute_csum(table, tablesz)) { + dev_err(dev, resource checksum failed, fw changed?\n); + ret = -EINVAL; + goto clean_up; + } + + Remove empty line? Ok. /* handle fw resources which are required to boot rproc */ - ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_rsc); + ret = rproc_handle_resource(rproc, tablesz, + rproc_handle_rsc); This can fit in one line now :) Ok, if (ret) { dev_err(dev, Failed to process resources: %d\n, ret); goto clean_up; @@ -825,6 +837,26 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) goto clean_up; } + /* Get the resource table address in device memory */ + devmem_rsc = rproc_get_rsctab_addr(rproc, fw); + + /* Copy the updated resource table to device memory */ + memcpy(devmem_rsc, rproc-rsc, tablesz); + + /* Free the copy of the resource table */ + tmp = rproc-rsc; + rproc-rsc = devmem_rsc; This is also set a few lines below, and we can probably do without tmp altogether. + kfree(tmp); If rproc_fw_boot is called more than once, kfree will try to free memory that was not kmalloced, leading to a panic. Instead of freeing the pointer here, it should be kept until rproc is released, reverting rproc-rsc to it every time shutdown is called. It can be freed when rproc is finally released. Agree. I will free the copy in rproc_trigger_recovery and in rproc_del, and set rproc-rsc = rproc-rsc_copy at shutdown. + + /* Update the vdev rsc address */ + list_for_each_entry(rvdev, rproc-rvdevs, node) { + int offset = (void *)rvdev-rsc - (void *)tmp; + rvdev-rsc = (void *)devmem_rsc + offset; + } I think it might be nicer and less error prone to hold the offsets to the resources instead of pointers. This will require modifying remoteproc: Support virtio config space accordingly, but will save the trouble of updating pointers later on (both here, and on shutdown as suggested above). Ok, I've changed this as well. We now work with offsets from the start of the resource table. I also updated the typedef rproc_handle_resource_t to include the resource offset as well. In this way we're consistently handling resource offsets
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
Hi Ohad, I was wondering - can you please explain your motivation for using vringh in caif ? We have internally discussed supporting multiple remote processors talking to each other using rpmsg, and in that scenario using vringh can considerably simplifies the solution (no need to decide in advance which side is the guest and which is the host). Was this the general incentive in using vringh in caif too or something else? The motivation for using vringh was to avoid copying buffers when sending data from the modem to the host. By using vringh the modem can transfer datagrams to host only by transfering buffer pointers via the rings. We use a carveout to declare a memory area that is passed to the modem internal memory allocator. This way we get both modem and host to work on the shared memory region. For TX traffic we use normal virtio queues for the same reason. TX buffers can be handled without copying as well. We are seeing a nice performance gain on the modem side after implementing this. Host size zero-copy is more tricky. It's hard to handle ownership of buffers, if the modem crashes. But we might look into this in the future as well. Regards, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
Hi Ohad, On Thu, Feb 21, 2013 at 6:55 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Thu, Feb 21, 2013 at 7:28 PM, Sjur Brændeland sjurb...@gmail.com wrote: The motivation for using vringh was to avoid copying buffers when sending data from the modem to the host. I may be missing something here, but why do you need vringh for that? With rpmsg (which uses two regular vrings) both ends send huge amount of data without copying it - we just send the pointers across. OK, We did carefully consider using the normal vrings, but concluded it was not doable. I'll try to give you some of the background that I can recall from top of my head. (I can dig out more if you're still not convinced :) The modem we're integrating with is a complex beast (multi mode modem LTE, HSPA+, etc). When a packet is received deep down in the radio stack, it allocates packet buffers using the internal slab allocator.The packet may contain anything from control, voice, or IP packets. If the packet contains IP-payload it will travel to a different asymmetric CPU that handles the IPC towards the modem. If the packet is not a IP-packet it will be processed internally and eventually freed. What we have done to integrate virtio is to inject the carveout into the the modem internal slab-allocator. Using the reversed ring allows us to introduce zero-copy for virtio with virtually no impact on the radio-stack, only a small change on the modem slab allocator. It supports dynamic size buffer allocation, and modem internal messaging using data allocated from the shared region. It also allows modem to manage it's own memory, without any dependency on host side allocators. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
On Wed, Feb 20, 2013 at 5:05 PM, Ohad Ben-Cohen wrote: > Hi Sjur, > > On Tue, Feb 12, 2013 at 1:49 PM, wrote: >> From: Sjur Brændeland >> >> Add functions for creating, deleting and kicking host-side virtio rings. >> >> The host ring is not integrated with virtiqueues and cannot be managed >> through virtio-config. > > Is that an inherent design/issue of vringh or just a description of > the current vringh code ? > >> Remoteproc must export functions for handling the host-side virtio rings. > > Have you considered exporting this via virtio instead ? Rusty should comment on this... I asked Rusty the same question a while a go, see http://lkml.org/lkml/2013/1/11/559 AFAIK, using the vringh API directly is a deliberate design choice. [Sjur:] >> How do you see the in-kernel API for this? I would like to see >> something similar to my previous patches, where we extend >> the virtqueue API. E.g. something like this: >> struct virtqueue *vring_new_virtqueueh(...)... [Rusty:] >I was just going to create _kernel variants of all the _user helpers, >and let you drive it directly like that. > >If we get a second in-kernel user, we create wrappers (I'd prefer not to >overload struct virtqueue though). >> The functions rproc_virtio_get_vringh(), rproc_virtio_del_vringh(), >> rproc_virtio_kick_vringh() are added to remoteproc_virtio.c. > > I wonder if this is the way we want things to work. > > Following this design, virtio drivers that use these rproc_* functions > will be coupled with the remoteproc framework. > > One issue with this is what happens if, e.g., a VIRTIO_ID_CAIF vdev is > added by other than remoteproc (e.g. by virtio_pci or virtio_mmio). > Not sure how probable this really is, and whether there's anything > that prevents this, but things will go awry if this happens. Yes, if you insert a "malicious device" like that you can make it crash, but wouldn't most drivers do if you try to register a malicious device...? If we really want to protect from this, we could perhaps validate the vdev pointer in function rproc_virtio_new_vringh() by looking through the vdevs of the registered rprocs. > But maybe the important aspect to consider is whether we really want > to couple virtio drivers (such as the upcoming caif one) with the > remoteproc framework. I'm not sure this is an issue for the CAIF driver. It would be very nice if someone else could make use of it, but right now cannot see the CAIF driver being used outside the remoteproc framework. This driver is designed specifically to work with the STE-modem using the CAIF protocol over a shared memory interface. > If you'll take a look at the rpmsg virtio driver, there's nothing > there which couples it with remoteproc. It's just a standard virtio > driver, that can be easily used with traditional virtio hosts as well. > > This is possible of course thanks to the abstraction provided by > virtio: remoteproc only implements a set of callbacks which virtio > invokes when needed. Yes, and generalizing the use of virtio devices in remoteproc has been useful. It has enabled me to let remoteproc manage both virtio_serial and virtio_caif devices :-) > > Do we not want to follow a similar design scheme with vringh ? I know some of my colleagues has been working on symmetric vring for rpmsg. Last I heard from them they were going to use the same approach I've done for CAIF, by "reversing" the direction of the rings. AFAIK this means that the current API I have proposed will work for them as well. If some other driver is showing up using the vringh kernel API where the current API don't fit, I guess it's time to create some abstractions and wrappers... But I hope the current approach will do for now? > I have some other questions as well but maybe it's better to discuss > first the bigger picture. OK, but please don't hesitate to address this. I'm still aiming for this to go into 3.9. Regards, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
On Wed, Feb 20, 2013 at 5:05 PM, Ohad Ben-Cohen o...@wizery.com wrote: Hi Sjur, On Tue, Feb 12, 2013 at 1:49 PM, sjur.brandel...@stericsson.com wrote: From: Sjur Brændeland sjur.brandel...@stericsson.com Add functions for creating, deleting and kicking host-side virtio rings. The host ring is not integrated with virtiqueues and cannot be managed through virtio-config. Is that an inherent design/issue of vringh or just a description of the current vringh code ? Remoteproc must export functions for handling the host-side virtio rings. Have you considered exporting this via virtio instead ? Rusty should comment on this... I asked Rusty the same question a while a go, see http://lkml.org/lkml/2013/1/11/559 AFAIK, using the vringh API directly is a deliberate design choice. [Sjur:] How do you see the in-kernel API for this? I would like to see something similar to my previous patches, where we extend the virtqueue API. E.g. something like this: struct virtqueue *vring_new_virtqueueh(...)... [Rusty:] I was just going to create _kernel variants of all the _user helpers, and let you drive it directly like that. If we get a second in-kernel user, we create wrappers (I'd prefer not to overload struct virtqueue though). The functions rproc_virtio_get_vringh(), rproc_virtio_del_vringh(), rproc_virtio_kick_vringh() are added to remoteproc_virtio.c. I wonder if this is the way we want things to work. Following this design, virtio drivers that use these rproc_* functions will be coupled with the remoteproc framework. One issue with this is what happens if, e.g., a VIRTIO_ID_CAIF vdev is added by other than remoteproc (e.g. by virtio_pci or virtio_mmio). Not sure how probable this really is, and whether there's anything that prevents this, but things will go awry if this happens. Yes, if you insert a malicious device like that you can make it crash, but wouldn't most drivers do if you try to register a malicious device...? If we really want to protect from this, we could perhaps validate the vdev pointer in function rproc_virtio_new_vringh() by looking through the vdevs of the registered rprocs. But maybe the important aspect to consider is whether we really want to couple virtio drivers (such as the upcoming caif one) with the remoteproc framework. I'm not sure this is an issue for the CAIF driver. It would be very nice if someone else could make use of it, but right now cannot see the CAIF driver being used outside the remoteproc framework. This driver is designed specifically to work with the STE-modem using the CAIF protocol over a shared memory interface. If you'll take a look at the rpmsg virtio driver, there's nothing there which couples it with remoteproc. It's just a standard virtio driver, that can be easily used with traditional virtio hosts as well. This is possible of course thanks to the abstraction provided by virtio: remoteproc only implements a set of callbacks which virtio invokes when needed. Yes, and generalizing the use of virtio devices in remoteproc has been useful. It has enabled me to let remoteproc manage both virtio_serial and virtio_caif devices :-) Do we not want to follow a similar design scheme with vringh ? I know some of my colleagues has been working on symmetric vring for rpmsg. Last I heard from them they were going to use the same approach I've done for CAIF, by reversing the direction of the rings. AFAIK this means that the current API I have proposed will work for them as well. If some other driver is showing up using the vringh kernel API where the current API don't fit, I guess it's time to create some abstractions and wrappers... But I hope the current approach will do for now? I have some other questions as well but maybe it's better to discuss first the bigger picture. OK, but please don't hesitate to address this. I'm still aiming for this to go into 3.9. Regards, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
> Sjur Brændeland writes: >> How about supporting struct vringh_kiov and struct kvec as well? >> I currently get the following complaints with my V2 patch-set: >> >> drivers/net/caif/caif_virtio.c:486:2: warning: passing argument 1 of >> ‘vringh_iov_init’ from incompatible pointer type [enabled by default] > > vringh_kiov_init()? Did I miss something else? I get a warning for my useof vringh_iov_cleanup() in addition to the one above. How about adding kiov variants of vringh_iov_cleanup() and vringh_iov_reset() as well? Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
Sjur Brændeland sjurb...@gmail.com writes: How about supporting struct vringh_kiov and struct kvec as well? I currently get the following complaints with my V2 patch-set: drivers/net/caif/caif_virtio.c:486:2: warning: passing argument 1 of ‘vringh_iov_init’ from incompatible pointer type [enabled by default] vringh_kiov_init()? Did I miss something else? I get a warning for my useof vringh_iov_cleanup() in addition to the one above. How about adding kiov variants of vringh_iov_cleanup() and vringh_iov_reset() as well? Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
Hi Rusty, On Wed, Feb 13, 2013 at 11:16 AM, Rusty Russell wrote: > Sjur BRENDELAND writes: >>> > +static inline void ctx_prep_iov(struct cfv_napi_context *ctx) >>> > +{ >>> > + if (ctx->riov.allocated) { >>> > + kfree(ctx->riov.iov); >>> > + ctx->riov.iov = NULL; >>> > + ctx->riov.allocated = false; >>> > + } >>> > + ctx->riov.iov = NULL; >>> > + ctx->riov.i = 0; >>> > + ctx->riov.max = 0; >>> > +} >>> >>> Hmm, we should probably make sure you don't have to do this: that if >>> allocated once, you can simply reuse it by setting i = 0. >> >> Yes, I had problems getting the alloc/free of iov right. This is >> perhaps the least intuitively part of the API. I maybe it's just me, but >> I think some more helper functions and support from vringh in this >> area would be great. > > Yes, I've neatened my git tree, an in particular added a commit which > covers this explicitly, and one for the -EPROTO when we get unexpected > r/w bufs. I've appended them below. Great, thanks. This helps a lot. I picked up your patch-sett yesterday so my V2 patch-set is using this already. ... > +static inline void vringh_iov_init(struct vringh_iov *iov, > + struct iovec *iovec, unsigned num) > +{ > + iov->used = iov->i = 0; > + iov->off = 0; > + iov->max_num = num; > + iov->iov = iovec; > +} How about supporting struct vringh_kiov and struct kvec as well? I currently get the following complaints with my V2 patch-set: drivers/net/caif/caif_virtio.c:486:2: warning: passing argument 1 of ‘vringh_iov_init’ from incompatible pointer type [enabled by default] ... Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
Hi Rusty, On Wed, Feb 13, 2013 at 11:16 AM, Rusty Russell ru...@rustcorp.com.au wrote: Sjur BRENDELAND sjur.brandel...@stericsson.com writes: +static inline void ctx_prep_iov(struct cfv_napi_context *ctx) +{ + if (ctx-riov.allocated) { + kfree(ctx-riov.iov); + ctx-riov.iov = NULL; + ctx-riov.allocated = false; + } + ctx-riov.iov = NULL; + ctx-riov.i = 0; + ctx-riov.max = 0; +} Hmm, we should probably make sure you don't have to do this: that if allocated once, you can simply reuse it by setting i = 0. Yes, I had problems getting the alloc/free of iov right. This is perhaps the least intuitively part of the API. I maybe it's just me, but I think some more helper functions and support from vringh in this area would be great. Yes, I've neatened my git tree, an in particular added a commit which covers this explicitly, and one for the -EPROTO when we get unexpected r/w bufs. I've appended them below. Great, thanks. This helps a lot. I picked up your patch-sett yesterday so my V2 patch-set is using this already. ... +static inline void vringh_iov_init(struct vringh_iov *iov, + struct iovec *iovec, unsigned num) +{ + iov-used = iov-i = 0; + iov-off = 0; + iov-max_num = num; + iov-iov = iovec; +} How about supporting struct vringh_kiov and struct kvec as well? I currently get the following complaints with my V2 patch-set: drivers/net/caif/caif_virtio.c:486:2: warning: passing argument 1 of ‘vringh_iov_init’ from incompatible pointer type [enabled by default] ... Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
On Fri, Jan 11, 2013 at 7:37 AM, Rusty Russell wrote: >virtio_host: host-side implementation of virtio rings (untested!) > >Getting use of virtio rings correct is tricky, and a recent patch saw >an implementation of in-kernel rings (as separate from userspace). How do you see the in-kernel API for this? I would like to see something similar to my previous patches, where we extend the virtqueue API. E.g. something like this: struct virtqueue *vring_new_virtqueueh(unsigned int index, unsigned int num, unsigned int vring_align, struct virtio_device *vdev, bool weak_barriers, void *pages, void (*notify)(struct virtqueue *), void (*callback)(struct virtqueue *), const char *name); int virtqueueh_get_iov(struct virtqueue *vqh, struct vringh_iov *riov, struct vringh_iov *wiov, gfp_t gfp); int virtqueueh_add_iov(struct virtqueue *vqh, struct vringh_iov *riov, struct vringh_iov *wiov); I guess implementation of the host-virtqueue should stay in drivers/virtio? Regards, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
On Fri, Jan 11, 2013 at 12:35 AM, Rusty Russell wrote: > Hi Sjur! > > OK, the Internet was no help here, how do you pronounce Sjur? > I'm guessing "shoor" rhyming with tour until I know better. Thank you for asking! This is pretty close yes. I usually tell people to pronounce it like "sure" for simplicity. But Google translate has a perfect Norwegian voice pronouncing my name: http://translate.google.com/#auto/no/sjur (Press the speaker button) While at the subject: Norwegian names can be a laugh: I have people named "Odd" and "Even" in my family, and I once had a friend named "Aashold". ... >> I would prefer not to split into pages at this point, but rather provide an >> iterator or the original list found in the descriptor to the client. ... > In other words, boundaries matter? > > While the sg-in-place hack is close to optimal for TCM_VHOST, neither > net not you can use it directly. I'll switch to an iovec (with a > similar use-caller-supplied-if-it-fits trick); they're smaller anyway. Great, I think iovec will be a good fit for my CAIF driver. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
On Fri, Jan 11, 2013 at 12:35 AM, Rusty Russell ru...@rustcorp.com.au wrote: Hi Sjur! OK, the Internet was no help here, how do you pronounce Sjur? I'm guessing shoor rhyming with tour until I know better. Thank you for asking! This is pretty close yes. I usually tell people to pronounce it like sure for simplicity. But Google translate has a perfect Norwegian voice pronouncing my name: http://translate.google.com/#auto/no/sjur (Press the speaker button) While at the subject: Norwegian names can be a laugh: I have people named Odd and Even in my family, and I once had a friend named Aashold. ... I would prefer not to split into pages at this point, but rather provide an iterator or the original list found in the descriptor to the client. ... In other words, boundaries matter? While the sg-in-place hack is close to optimal for TCM_VHOST, neither net not you can use it directly. I'll switch to an iovec (with a similar use-caller-supplied-if-it-fits trick); they're smaller anyway. Great, I think iovec will be a good fit for my CAIF driver. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
On Fri, Jan 11, 2013 at 7:37 AM, Rusty Russell ru...@rustcorp.com.au wrote: virtio_host: host-side implementation of virtio rings (untested!) Getting use of virtio rings correct is tricky, and a recent patch saw an implementation of in-kernel rings (as separate from userspace). How do you see the in-kernel API for this? I would like to see something similar to my previous patches, where we extend the virtqueue API. E.g. something like this: struct virtqueue *vring_new_virtqueueh(unsigned int index, unsigned int num, unsigned int vring_align, struct virtio_device *vdev, bool weak_barriers, void *pages, void (*notify)(struct virtqueue *), void (*callback)(struct virtqueue *), const char *name); int virtqueueh_get_iov(struct virtqueue *vqh, struct vringh_iov *riov, struct vringh_iov *wiov, gfp_t gfp); int virtqueueh_add_iov(struct virtqueue *vqh, struct vringh_iov *riov, struct vringh_iov *wiov); I guess implementation of the host-virtqueue should stay in drivers/virtio? Regards, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
Hi Rusty, On Thu, Jan 10, 2013 at 11:30 AM, Rusty Russell wrote: ... >I now have some lightly-tested code (via a userspace harness). Great - thank you for looking into this. I will start integrating this with my patches when you send out a proper patch. ... > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 8d5bddb..fd95d3e 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -5,6 +5,12 @@ config VIRTIO > bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST, > CONFIG_RPMSG or CONFIG_S390_GUEST. > > +config VHOST > + tristate Inclusion of drivers/virtio from drivers/Makefile depends on VIRTIO. So I guess VHOST should select VIRTIO to ensure that drivers/virtio/virtio_host.c is part of the build. > + ---help--- > + This option is selected by any driver which needs to access > + the host side of a virtio ring. > + ... > +/* Returns vring->num if empty, -ve on error. */ > +static inline int __vringh_get_head(const struct vringh *vrh, > + int (*getu16)(u16 *val, const u16 *p), > + u16 *last_avail_idx) > +{ > + u16 avail_idx, i, head; > + int err; > + > + err = getu16(_idx, >vring.avail->idx); > + if (err) { > + vringh_bad("Failed to access avail idx at %p", > + >vring.avail->idx); > + return err; > + } > + > + err = getu16(last_avail_idx, _avail_event(>vring)); > + if (err) { > + vringh_bad("Failed to access last avail idx at %p", > + _avail_event(>vring)); > + return err; > + } > + > + if (*last_avail_idx == avail_idx) > + return vrh->vring.num; > + > + /* Only get avail ring entries after they have been exposed by guest. > */ > + smp_rmb(); We are accessing memory shared with a remote device (modem), so we probably need mandatory barriers here, e.g. something like the virtio_rmb defined in virtio_ring.c. > + > + i = *last_avail_idx & (vrh->vring.num - 1); > + > + err = getu16(, >vring.avail->ring[i]); > + if (err) { > + vringh_bad("Failed to read head: idx %d address %p", > + *last_avail_idx, >vring.avail->ring[i]); > + return err; > + } > + > + if (head >= vrh->vring.num) { > + vringh_bad("Guest says index %u > %u is available", > + head, vrh->vring.num); > + return -EINVAL; > + } > + return head; > +} ... > +static inline int > +__vringh_sg(struct vringh_acc *acc, > + struct vringh_sg *vsg, > + unsigned max, > + u16 write_flag, > + gfp_t gfp, > + int (*getdesc)(struct vring_desc *dst, const struct vring_desc > *s)) > +{ > + unsigned count = 0, num_descs = 0; > + struct vringh_sg *orig_vsg = vsg; > + int err; > + > + /* This sends end marker on sg[max-1], so we know when to chain. */ > + if (max) > + sg_init_table(>sg, max); > + > + for (;;) { > + /* Exhausted this descriptor? Read next. */ > + if (acc->desc.len == 0) { > + if (!(acc->desc.flags & VRING_DESC_F_NEXT)) > + break; > + > + if (num_descs++ == acc->max) { > + err = -ELOOP; > + goto fail; > + } > + > + if (acc->desc.next >= acc->max) { > + vringh_bad("Chained index %u > %u", > + acc->desc.next, acc->max); > + err = -EINVAL; > + goto fail; > + } > + > + acc->idx = acc->desc.next; > + err = getdesc(>desc, acc->start + acc->idx); > + if (unlikely(err)) > + goto fail; > + } > + > + if (unlikely(!max)) { > + vringh_bad("Unexpected %s descriptor", > + write_flag ? "writable" : "readable"); > + return -EINVAL; > + } > + > + /* No more readable/writable descriptors? */ > + if ((acc->desc.flags & VRING_DESC_F_WRITE) != write_flag) { > + /* We should not have readable after writable */ > + if (write_flag) { > + vringh_bad("Readable desc %p after writable", > + acc->start + acc->idx); > + err = -EINVAL; > + goto fail; > + } > + break; > +
Re: [RFCv2 00/12] Introduce host-side virtio queue and CAIF Virtio.
Hi Rusty, On Thu, Jan 10, 2013 at 11:30 AM, Rusty Russell ru...@rustcorp.com.au wrote: ... I now have some lightly-tested code (via a userspace harness). Great - thank you for looking into this. I will start integrating this with my patches when you send out a proper patch. ... diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 8d5bddb..fd95d3e 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -5,6 +5,12 @@ config VIRTIO bus, such as CONFIG_VIRTIO_PCI, CONFIG_VIRTIO_MMIO, CONFIG_LGUEST, CONFIG_RPMSG or CONFIG_S390_GUEST. +config VHOST + tristate Inclusion of drivers/virtio from drivers/Makefile depends on VIRTIO. So I guess VHOST should select VIRTIO to ensure that drivers/virtio/virtio_host.c is part of the build. + ---help--- + This option is selected by any driver which needs to access + the host side of a virtio ring. + ... +/* Returns vring-num if empty, -ve on error. */ +static inline int __vringh_get_head(const struct vringh *vrh, + int (*getu16)(u16 *val, const u16 *p), + u16 *last_avail_idx) +{ + u16 avail_idx, i, head; + int err; + + err = getu16(avail_idx, vrh-vring.avail-idx); + if (err) { + vringh_bad(Failed to access avail idx at %p, + vrh-vring.avail-idx); + return err; + } + + err = getu16(last_avail_idx, vring_avail_event(vrh-vring)); + if (err) { + vringh_bad(Failed to access last avail idx at %p, + vring_avail_event(vrh-vring)); + return err; + } + + if (*last_avail_idx == avail_idx) + return vrh-vring.num; + + /* Only get avail ring entries after they have been exposed by guest. */ + smp_rmb(); We are accessing memory shared with a remote device (modem), so we probably need mandatory barriers here, e.g. something like the virtio_rmb defined in virtio_ring.c. + + i = *last_avail_idx (vrh-vring.num - 1); + + err = getu16(head, vrh-vring.avail-ring[i]); + if (err) { + vringh_bad(Failed to read head: idx %d address %p, + *last_avail_idx, vrh-vring.avail-ring[i]); + return err; + } + + if (head = vrh-vring.num) { + vringh_bad(Guest says index %u %u is available, + head, vrh-vring.num); + return -EINVAL; + } + return head; +} ... +static inline int +__vringh_sg(struct vringh_acc *acc, + struct vringh_sg *vsg, + unsigned max, + u16 write_flag, + gfp_t gfp, + int (*getdesc)(struct vring_desc *dst, const struct vring_desc *s)) +{ + unsigned count = 0, num_descs = 0; + struct vringh_sg *orig_vsg = vsg; + int err; + + /* This sends end marker on sg[max-1], so we know when to chain. */ + if (max) + sg_init_table(vsg-sg, max); + + for (;;) { + /* Exhausted this descriptor? Read next. */ + if (acc-desc.len == 0) { + if (!(acc-desc.flags VRING_DESC_F_NEXT)) + break; + + if (num_descs++ == acc-max) { + err = -ELOOP; + goto fail; + } + + if (acc-desc.next = acc-max) { + vringh_bad(Chained index %u %u, + acc-desc.next, acc-max); + err = -EINVAL; + goto fail; + } + + acc-idx = acc-desc.next; + err = getdesc(acc-desc, acc-start + acc-idx); + if (unlikely(err)) + goto fail; + } + + if (unlikely(!max)) { + vringh_bad(Unexpected %s descriptor, + write_flag ? writable : readable); + return -EINVAL; + } + + /* No more readable/writable descriptors? */ + if ((acc-desc.flags VRING_DESC_F_WRITE) != write_flag) { + /* We should not have readable after writable */ + if (write_flag) { + vringh_bad(Readable desc %p after writable, + acc-start + acc-idx); + err = -EINVAL; + goto fail; + } + break; + } + + /* Append the pages into the sg. */ + err = add_to_sg(vsg,
[RFCv2 03/11] remoteproc: Set vring addresses in resource table
Set the vring addresses in the resource table so that the remote device can read the actual addresses used. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c |5 + include/linux/remoteproc.h |2 ++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 7d593a1..151e138 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -238,6 +238,8 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) rvring->dma = dma; rvring->notifyid = notifyid; + rvdev->rsc->vring[i].da = dma; + rvdev->rsc->vring[i].notifyid = notifyid; return 0; } @@ -365,6 +367,9 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, /* remember the device features */ rvdev->dfeatures = rsc->dfeatures; + /* remember the resource entry */ + rvdev->rsc = rsc; + list_add_tail(>node, >rvdevs); /* it is now safe to add the virtio device */ diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index faf3332..cdacd66 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -464,6 +464,7 @@ struct rproc_vring { * @vring: the vrings for this vdev * @dfeatures: virtio device features * @gfeatures: virtio guest features + * @rsc: vdev resource entry */ struct rproc_vdev { struct list_head node; @@ -472,6 +473,7 @@ struct rproc_vdev { struct rproc_vring vring[RVDEV_NUM_VRINGS]; unsigned long dfeatures; unsigned long gfeatures; + struct fw_rsc_vdev *rsc; }; struct rproc *rproc_alloc(struct device *dev, const char *name, -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 02/11] remoteproc: Move check on firmware name to rproc_add
Verify that firmware name is defined in rproc_add. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 13 ++--- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index feb39a7..7d593a1 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -989,13 +989,6 @@ int rproc_boot(struct rproc *rproc) return ret; } - /* loading a firmware is required */ - if (!rproc->firmware) { - dev_err(dev, "%s: no firmware to load\n", __func__); - ret = -EINVAL; - goto unlock_mutex; - } - /* prevent underlying implementation from being removed */ if (!try_module_get(dev->parent->driver->owner)) { dev_err(dev, "%s: can't get owner\n", __func__); @@ -1120,6 +1113,12 @@ int rproc_add(struct rproc *rproc) struct device *dev = >dev; int ret; + /* loading a firmware is required */ + if (!rproc->firmware) { + dev_err(dev, "%s: no firmware to load\n", __func__); + return -EINVAL; + } + ret = device_add(dev); if (ret < 0) return ret; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 00/10] remoteproc: Support bi-directional vdev config space
Changes since v1: - Separate vring allocation and registration - Break up patches in smaller parts to facilitate review. (The patches here are perhaps too fine grained and can be squashed later). This patch-set adds support for shared resource table between Linux kernel and remote devices. Features: - dynamically-allocated address of the vrings can be communicated - vdev statuses can be communicated - virtio config space becomes bi-directional - virtio feature negotiation is two-way NOTE: This change may not be backwards compatible with existing device firmware! The memory allocation order may change. Previously the Virtio Rings were always allocated at start of the share memory area, but with this patch-set the memory allocation are done in the order defined resource table. A number of changes are introduced to achieve this: - The firmware is loaded once. - The allocations of resources is done before registering the virtio devices. - The virtio device uses resource bits, status and config space in memory shared with the device. Review comments and feedback are welcomed! Thanks, Sjur Sjur Brændeland (11): remoteproc: Code cleanup of resource parsing remoteproc: Move check on firmware name to rproc_add remoteproc: Set vring addresses in resource table remoteproc: Add state RPROC_LOADED remoteproc: Load firmware once. remoteproc: Add resource entry number to callbacks remoteproc: Register virtio devices after vring allocation remoteproc: Refactor function rproc_elf_find_rsc_table remoteproc: Add operation to find resource table in memory remoteproc: Find resource table in device memory remoteproc: Support virtio config space. drivers/remoteproc/remoteproc_core.c | 216 --- drivers/remoteproc/remoteproc_debugfs.c|1 + drivers/remoteproc/remoteproc_elf_loader.c | 99 +- drivers/remoteproc/remoteproc_internal.h | 13 ++ drivers/remoteproc/remoteproc_virtio.c | 30 - drivers/remoteproc/ste_modem_rproc.c | 43 -- include/linux/remoteproc.h |9 +- 7 files changed, 238 insertions(+), 173 deletions(-) -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 08/11] remoteproc: Refactor function rproc_elf_find_rsc_table
Refatcor rproc_elf_find_rsc_table and split out the scanning for the section header named resource table. This is done to prepare for loading firmware once. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_elf_loader.c | 83 +--- 1 files changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index e1f89d6..69832d9 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -208,38 +208,19 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw) return ret; } -/** - * rproc_elf_find_rsc_table() - find the resource table - * @rproc: the rproc handle - * @fw: the ELF firmware image - * @tablesz: place holder for providing back the table size - * - * This function finds the resource table inside the remote processor's - * firmware. It is used both upon the registration of @rproc (in order - * to look for and register the supported virito devices), and when the - * @rproc is booted. - * - * Returns the pointer to the resource table if it is found, and write its - * size into @tablesz. If a valid table isn't found, NULL is returned - * (and @tablesz isn't set). - */ -static struct resource_table * -rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, - int *tablesz) +static struct elf32_shdr * +find_rsc_shdr(struct device *dev, struct elf32_hdr *ehdr) { - struct elf32_hdr *ehdr; struct elf32_shdr *shdr; + int i; const char *name_table; - struct device *dev = >dev; struct resource_table *table = NULL; - int i; - const u8 *elf_data = fw->data; + const u8 *elf_data = (void *)ehdr; - ehdr = (struct elf32_hdr *)elf_data; + /* look for the resource table and handle it */ shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff); name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset; - /* look for the resource table and handle it */ for (i = 0; i < ehdr->e_shnum; i++, shdr++) { int size = shdr->sh_size; int offset = shdr->sh_offset; @@ -249,12 +230,6 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, table = (struct resource_table *)(elf_data + offset); - /* make sure we have the entire table */ - if (offset + size > fw->size) { - dev_err(dev, "resource table truncated\n"); - return NULL; - } - /* make sure table has at least the header */ if (sizeof(struct resource_table) > size) { dev_err(dev, "header-less resource table\n"); @@ -280,10 +255,54 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return NULL; } - *tablesz = shdr->sh_size; - break; + return shdr; + } + + return NULL; +} + +/** + * rproc_elf_find_rsc_table() - find the resource table + * @rproc: the rproc handle + * @fw: the ELF firmware image + * @tablesz: place holder for providing back the table size + * + * This function finds the resource table inside the remote processor's + * firmware. It is used both upon the registration of @rproc (in order + * to look for and register the supported virito devices), and when the + * @rproc is booted. + * + * Returns the pointer to the resource table if it is found, and write its + * size into @tablesz. If a valid table isn't found, NULL is returned + * (and @tablesz isn't set). + */ +static struct resource_table * +rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, + int *tablesz) +{ + struct elf32_hdr *ehdr; + struct elf32_shdr *shdr; + + struct device *dev = >dev; + struct resource_table *table = NULL; + + const u8 *elf_data = fw->data; + + ehdr = (struct elf32_hdr *)elf_data; + + shdr = find_rsc_shdr(dev, ehdr); + if (!shdr) + return NULL; + + /* make sure we have the entire table */ + if (shdr->sh_offset + shdr->sh_size > fw->size) { + dev_err(dev, "resource table truncated\n"); + return NULL; } + table = (struct resource_table *)(elf_data + shdr->sh_offset); + *tablesz = shdr->sh_size; + return table; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 10/11] remoteproc: Find resource table in device memory
Call rproc_get_rsctab_addr() to get the address of the resource table in shared memory. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 8a7de5c..f95c08c 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -854,6 +854,13 @@ static void rproc_fw_load(const struct firmware *fw, void *context) goto clean_up; } + /* Find the new location of the resource table in device memory */ + table = rproc_get_rsctab_addr(rproc, fw); + if (!table) { + ret = -EINVAL; + goto clean_up; + } + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_reg); if (ret) { dev_err(dev, "Failed to process resources: %d\n", ret); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 11/11] remoteproc: Support virtio config space.
Support virtio configuration space and device status and feature negotiation with remote device. This virtio device can now access the resource table in shared memory. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 10 -- drivers/remoteproc/remoteproc_virtio.c | 30 +++--- include/linux/remoteproc.h |2 -- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index f95c08c..d59c4cf 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -364,13 +364,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, goto free_rvdev; } - /* remember the device features */ - rvdev->dfeatures = rsc->dfeatures; - /* remember the device resource entry number */ rvdev->rsc_seqno = seqno; - /* remember the resource entry */ - rvdev->rsc = rsc; list_add_tail(>node, >rvdevs); @@ -394,8 +389,11 @@ static int rproc_register_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, dev_dbg(>dev, "register device %s\n", rproc->name); list_for_each_entry_safe(rvdev, rvtmp, >rvdevs, node) - if (seqno == rvdev->rsc_seqno) + if (seqno == rvdev->rsc_seqno) { + /* remember the resource entry */ + rvdev->rsc = rsc; return rproc_add_virtio_dev(rvdev, rsc->id); + } return -ENODEV; } diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..9ebc5d9 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -176,16 +176,21 @@ error: */ static u8 rproc_virtio_get_status(struct virtio_device *vdev) { - return 0; + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + return rvdev->rsc->status; } static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev->rsc->status = status; dev_dbg(>dev, "status: %d\n", status); } static void rproc_virtio_reset(struct virtio_device *vdev) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev->rsc->status = 0; dev_dbg(>dev, "reset !\n"); } @@ -194,7 +199,7 @@ static u32 rproc_virtio_get_features(struct virtio_device *vdev) { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); - return rvdev->dfeatures; + return rvdev->rsc->dfeatures; } static void rproc_virtio_finalize_features(struct virtio_device *vdev) @@ -213,7 +218,23 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * fixed as part of a small resource table overhaul and then an * extension of the virtio resource entries. */ - rvdev->gfeatures = vdev->features[0]; + rvdev->rsc->gfeatures = vdev->features[0]; +} + +void rproc_virtio_get(struct virtio_device *vdev, unsigned offset, + void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = >rsc->vring[rvdev->rsc->num_of_vrings]; + memcpy(buf, cfg + offset, len); +} + +void rproc_virtio_set(struct virtio_device *vdev, unsigned offset, + const void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = >rsc->vring[rvdev->rsc->num_of_vrings]; + memcpy(cfg + offset, buf, len); } static struct virtio_config_ops rproc_virtio_config_ops = { @@ -224,6 +245,9 @@ static struct virtio_config_ops rproc_virtio_config_ops = { .reset = rproc_virtio_reset, .set_status = rproc_virtio_set_status, .get_status = rproc_virtio_get_status, + .get= rproc_virtio_get, + .set= rproc_virtio_set, + }; /* diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 60a1002..de20e39 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -473,8 +473,6 @@ struct rproc_vdev { struct rproc *rproc; struct virtio_device vdev; struct rproc_vring vring[RVDEV_NUM_VRINGS]; - unsigned long dfeatures; - unsigned long gfeatures; u32 rsc_seqno; struct fw_rsc_vdev *rsc; }; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 09/11] remoteproc: Add operation to find resource table in memory
Add function find_rsc_table_va to firmware ops. This function returns the location of the resource table in shared memory after loading. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_elf_loader.c | 16 ++- drivers/remoteproc/remoteproc_internal.h | 13 drivers/remoteproc/ste_modem_rproc.c | 43 +++- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index 69832d9..3f6e315 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -306,9 +306,23 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table; } +struct resource_table *rproc_elf_get_rsctab_addr(struct rproc *rproc, +const struct firmware *fw) +{ + struct elf32_shdr *shdr; + + shdr = find_rsc_shdr(>dev, (struct elf32_hdr *)fw->data); + if (!shdr) + return NULL; + + /* Find resource table in loaded segments */ + return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size); +} + const struct rproc_fw_ops rproc_elf_fw_ops = { .load = rproc_elf_load_segments, .find_rsc_table = rproc_elf_find_rsc_table, .sanity_check = rproc_elf_sanity_check, - .get_boot_addr = rproc_elf_get_boot_addr + .get_boot_addr = rproc_elf_get_boot_addr, + .get_rsctab_addr = rproc_elf_get_rsctab_addr }; diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index 7bb6648..3a5cb7d 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -32,6 +32,7 @@ struct rproc; * expects to find it * @sanity_check: sanity check the fw image * @get_boot_addr: get boot address to entry point specified in firmware + * @get_rsctab_addr: get resouce table address as specified in firmware */ struct rproc_fw_ops { struct resource_table *(*find_rsc_table) (struct rproc *rproc, @@ -40,6 +41,8 @@ struct rproc_fw_ops { int (*load)(struct rproc *rproc, const struct firmware *fw); int (*sanity_check)(struct rproc *rproc, const struct firmware *fw); u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw); + struct resource_table *(*get_rsctab_addr)(struct rproc *rproc, + const struct firmware *fw); }; /* from remoteproc_core.c */ @@ -102,6 +105,16 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc, return NULL; } +static inline +struct resource_table *rproc_get_rsctab_addr(struct rproc *rproc, + const struct firmware *fw) +{ + if (rproc->fw_ops->get_rsctab_addr) + return rproc->fw_ops->get_rsctab_addr(rproc, fw); + + return NULL; +} + extern const struct rproc_fw_ops rproc_elf_fw_ops; #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c index a7743c0..59e99f1 100644 --- a/drivers/remoteproc/ste_modem_rproc.c +++ b/drivers/remoteproc/ste_modem_rproc.c @@ -64,26 +64,18 @@ static int sproc_load_segments(struct rproc *rproc, const struct firmware *fw) } /* Find the entry for resource table in the Table of Content */ -static struct ste_toc_entry *sproc_find_rsc_entry(const struct firmware *fw) +static const struct ste_toc_entry *sproc_find_rsc_entry(const void *data) { int i; - struct ste_toc *toc; - - if (!fw) - return NULL; - - toc = (void *)fw->data; + const struct ste_toc *toc; + toc = data; /* Search the table for the resource table */ for (i = 0; i < SPROC_MAX_TOC_ENTRIES && toc->table[i].start != 0x; i++) { - if (!strncmp(toc->table[i].name, SPROC_RESOURCE_NAME, -sizeof(toc->table[i].name))) { - if (toc->table[i].start > fw->size) - return NULL; +sizeof(toc->table[i].name))) return >table[i]; - } } return NULL; @@ -96,9 +88,12 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, { struct sproc *sproc = rproc->priv; struct resource_table *table; - struct ste_toc_entry *entry; + const struct ste_toc_entry *entry; - entry = sproc_find_rsc_entry(fw); + if (!fw) + return NULL; + + entry = sproc_find_rsc_entry(fw->data); if (!entry) { sproc_err(sproc, "resource table not found in fw\n"); return NULL; @@ -149,10 +144,30 @@ sproc_find_rsc_table(struct
[RFCv2 07/11] remoteproc: Register virtio devices after vring allocation
Postpone the registration of virtio devices until all vritio ring resource has been allocated. This fixes the following bug: The driver's start callback is called before all vring notify ids are allocated and max_notifyid will be increased after starting the remoteproc. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 26 ++ include/linux/remoteproc.h |2 ++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 2c78ea5..8a7de5c 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -367,6 +367,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, /* remember the device features */ rvdev->dfeatures = rsc->dfeatures; + /* remember the device resource entry number */ + rvdev->rsc_seqno = seqno; /* remember the resource entry */ rvdev->rsc = rsc; @@ -384,6 +386,20 @@ free_rvdev: return ret; } +static int rproc_register_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, + int avail, int seqno) +{ + struct rproc_vdev *rvdev, *rvtmp; + + dev_dbg(>dev, "register device %s\n", rproc->name); + + list_for_each_entry_safe(rvdev, rvtmp, >rvdevs, node) + if (seqno == rvdev->rsc_seqno) + return rproc_add_virtio_dev(rvdev, rsc->id); + + return -ENODEV; +} + /** * rproc_handle_trace() - handle a shared trace buffer resource * @rproc: the remote processor @@ -692,6 +708,10 @@ static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; +static rproc_handle_resource_t rproc_handle_reg[RSC_LAST] = { + [RSC_VDEV] = (rproc_handle_resource_t)rproc_register_vdev, +}; + /* handle firmware resource entries */ static int rproc_handle_resource(struct rproc *rproc, struct resource_table *table, @@ -834,6 +854,12 @@ static void rproc_fw_load(const struct firmware *fw, void *context) goto clean_up; } + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_reg); + if (ret) { + dev_err(dev, "Failed to process resources: %d\n", ret); + goto clean_up; + } + rproc->state = RPROC_LOADED; dev_info(dev, "remote processor %s is loaded to memory\n", rproc->name); diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 932edc7..60a1002 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -465,6 +465,7 @@ struct rproc_vring { * @vring: the vrings for this vdev * @dfeatures: virtio device features * @gfeatures: virtio guest features + * @rsc_seqno: sequence number in resource table * @rsc: vdev resource entry */ struct rproc_vdev { @@ -474,6 +475,7 @@ struct rproc_vdev { struct rproc_vring vring[RVDEV_NUM_VRINGS]; unsigned long dfeatures; unsigned long gfeatures; + u32 rsc_seqno; struct fw_rsc_vdev *rsc; }; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 06/11] remoteproc: Add resource entry number to callbacks
Add information on the resource entry sequence number to the callback functions. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 19 +++ 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index ea4b76a..2c78ea5 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -43,9 +43,8 @@ #include "remoteproc_internal.h" -typedef int (*rproc_handle_resources_t)(struct rproc *rproc, - struct resource_table *table, int len); -typedef int (*rproc_handle_resource_t)(struct rproc *rproc, void *, int avail); +typedef int (*rproc_handle_resource_t)(struct rproc *rproc, void *, + int avail, int seqno); /* Unique indices for remoteproc devices */ static DEFINE_IDA(rproc_dev_index); @@ -300,6 +299,7 @@ void rproc_free_vring(struct rproc_vring *rvring) * @rproc: the remote processor * @rsc: the vring resource descriptor * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * This resource entry requests the host to statically register a virtio * device (vdev), and setup everything needed to support it. It contains @@ -323,7 +323,7 @@ void rproc_free_vring(struct rproc_vring *rvring) * Returns 0 on success, or an appropriate error code otherwise */ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, - int avail) + int avail, int seqno) { struct device *dev = >dev; struct rproc_vdev *rvdev; @@ -389,6 +389,7 @@ free_rvdev: * @rproc: the remote processor * @rsc: the trace resource descriptor * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * In case the remote processor dumps trace logs into memory, * export it via debugfs. @@ -401,7 +402,7 @@ free_rvdev: * Returns 0 on success, or an appropriate error code otherwise */ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, - int avail) + int avail, int seqno) { struct rproc_mem_entry *trace; struct device *dev = >dev; @@ -462,6 +463,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, * @rproc: remote processor handle * @rsc: the devmem resource entry * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * Remote processors commonly need to access certain on-chip peripherals. * @@ -483,7 +485,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, * are outside those ranges. */ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc, - int avail) + int avail, int seqno) { struct rproc_mem_entry *mapping; struct device *dev = >dev; @@ -542,6 +544,7 @@ out: * @rproc: rproc handle * @rsc: the resource entry * @avail: size of available data (for image validation) + * @seqno: resource entry sequence number * * This function will handle firmware requests for allocation of physically * contiguous memory regions. @@ -556,7 +559,7 @@ out: * pressure is important; it may have a substantial impact on performance. */ static int rproc_handle_carveout(struct rproc *rproc, - struct fw_rsc_carveout *rsc, int avail) + struct fw_rsc_carveout *rsc, int avail, int seqno) { struct rproc_mem_entry *carveout, *mapping; struct device *dev = >dev; @@ -722,7 +725,7 @@ rproc_handle_resource(struct rproc *rproc, struct resource_table *table, if (!handler) continue; - ret = handler(rproc, rsc, avail); + ret = handler(rproc, rsc, avail, i); if (ret) break; } -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 05/11] remoteproc: Load firmware once.
Load the firmware once and pave the way for two way communication of virtio configuration and status bits. The virtio ring device address is now stored in the resource table. NOTE: This patch requires device firmware change! The device memory layout changes. The virtio rings are no longer located at start of device memory. But the vring address can be read from the resource table. Also Carveout must be located before the virtio rings in the resource table. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 111 -- 1 files changed, 38 insertions(+), 73 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 151e138..ea4b76a 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -686,14 +686,10 @@ static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, - [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ -}; - -static rproc_handle_resource_t rproc_handle_vdev_rsc[RSC_LAST] = { [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; -/* handle firmware resource entries before booting the remote processor */ +/* handle firmware resource entries */ static int rproc_handle_resource(struct rproc *rproc, struct resource_table *table, int len, @@ -778,18 +774,27 @@ static void rproc_resource_cleanup(struct rproc *rproc) } /* - * take a firmware and boot a remote processor with it. + * take a firmware, parse the resouce table and load it + * + * Note: this function is called asynchronously upon registration of the + * remote processor (so we must wait until it completes before we try + * to unregister the device. one other option is just to use kref here, + * that might be cleaner). */ -static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) +static void rproc_fw_load(const struct firmware *fw, void *context) { + struct rproc *rproc = context; struct device *dev = >dev; const char *name = rproc->firmware; struct resource_table *table; int ret, tablesz; + if (!fw) + goto release_fw; + ret = rproc_fw_sanity_check(rproc, fw); if (ret) - return ret; + goto release_fw; dev_info(dev, "Booting fw image %s, size %zd\n", name, fw->size); @@ -800,7 +805,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) ret = rproc_enable_iommu(rproc); if (ret) { dev_err(dev, "can't enable iommu: %d\n", ret); - return ret; + goto release_fw; } rproc->bootaddr = rproc_get_boot_addr(rproc, fw); @@ -826,60 +831,23 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) goto clean_up; } - /* power up the remote processor */ - ret = rproc->ops->start(rproc); - if (ret) { - dev_err(dev, "can't start rproc %s: %d\n", rproc->name, ret); - goto clean_up; - } - - rproc->state = RPROC_RUNNING; - - dev_info(dev, "remote processor %s is now up\n", rproc->name); + rproc->state = RPROC_LOADED; - return 0; + dev_info(dev, "remote processor %s is loaded to memory\n", rproc->name); clean_up: - rproc_resource_cleanup(rproc); - rproc_disable_iommu(rproc); - return ret; -} - -/* - * take a firmware and look for virtio devices to register. - * - * Note: this function is called asynchronously upon registration of the - * remote processor (so we must wait until it completes before we try - * to unregister the device. one other option is just to use kref here, - * that might be cleaner). - */ -static void rproc_fw_config_virtio(const struct firmware *fw, void *context) -{ - struct rproc *rproc = context; - struct resource_table *table; - int ret, tablesz; - - if (rproc_fw_sanity_check(rproc, fw) < 0) - goto out; - - /* look for the resource table */ - table = rproc_find_rsc_table(rproc, fw, ); - if (!table) - goto out; - - /* look for virtio devices and register them */ - ret = rproc_handle_resource(rproc, table, tablesz, - rproc_handle_vdev_rsc); - if (ret) - goto out; - -out: + if (ret) { + rproc->state = RPROC_OFFLINE; + rproc_resource_cleanup(rproc); + rproc_disable_iommu(rproc); + } +release_fw
[RFCv2 01/11] remoteproc: Code cleanup of resource parsing
Combine the almost identical functions rproc_handle_virtio_rsc and rproc_handle_boot_rsc. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 51 -- 1 files changed, 12 insertions(+), 39 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index dd3bfaf..feb39a7 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -677,16 +677,22 @@ free_carv: * A lookup table for resource handlers. The indices are defined in * enum fw_resource_type. */ -static rproc_handle_resource_t rproc_handle_rsc[] = { +static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ }; +static rproc_handle_resource_t rproc_handle_vdev_rsc[RSC_LAST] = { + [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, +}; + /* handle firmware resource entries before booting the remote processor */ static int -rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len) +rproc_handle_resource(struct rproc *rproc, struct resource_table *table, + int len, + rproc_handle_resource_t handlers[RSC_LAST]) { struct device *dev = >dev; rproc_handle_resource_t handler; @@ -711,7 +717,7 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len continue; } - handler = rproc_handle_rsc[hdr->type]; + handler = handlers[hdr->type]; if (!handler) continue; @@ -723,40 +729,6 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len return ret; } -/* handle firmware resource entries while registering the remote processor */ -static int -rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int len) -{ - struct device *dev = >dev; - int ret = 0, i; - - for (i = 0; i < table->num; i++) { - int offset = table->offset[i]; - struct fw_rsc_hdr *hdr = (void *)table + offset; - int avail = len - offset - sizeof(*hdr); - struct fw_rsc_vdev *vrsc; - - /* make sure table isn't truncated */ - if (avail < 0) { - dev_err(dev, "rsc table is truncated\n"); - return -EINVAL; - } - - dev_dbg(dev, "%s: rsc type %d\n", __func__, hdr->type); - - if (hdr->type != RSC_VDEV) - continue; - - vrsc = (struct fw_rsc_vdev *)hdr->data; - - ret = rproc_handle_vdev(rproc, vrsc, avail); - if (ret) - break; - } - - return ret; -} - /** * rproc_resource_cleanup() - clean up and free all acquired resources * @rproc: rproc handle @@ -836,7 +808,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) } /* handle fw resources which are required to boot rproc */ - ret = rproc_handle_boot_rsc(rproc, table, tablesz); + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_rsc); if (ret) { dev_err(dev, "Failed to process resources: %d\n", ret); goto clean_up; @@ -891,7 +863,8 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context) goto out; /* look for virtio devices and register them */ - ret = rproc_handle_virtio_rsc(rproc, table, tablesz); + ret = rproc_handle_resource(rproc, table, tablesz, + rproc_handle_vdev_rsc); if (ret) goto out; -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 04/11] remoteproc: Add state RPROC_LOADED
Add state RPROC_LOADED as firmware loading and startup will be different states. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_debugfs.c |1 + include/linux/remoteproc.h |3 ++- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 157a573..a026359 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -68,6 +68,7 @@ static const char * const rproc_state_string[] = { "suspended", "running", "crashed", + "loaded", "invalid", }; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index cdacd66..932edc7 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -357,7 +357,8 @@ enum rproc_state { RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, - RPROC_LAST = 4, + RPROC_LOADED= 4, + RPROC_LAST = 5, }; /** -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 01/11] remoteproc: Code cleanup of resource parsing
Combine the almost identical functions rproc_handle_virtio_rsc and rproc_handle_boot_rsc. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 51 -- 1 files changed, 12 insertions(+), 39 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index dd3bfaf..feb39a7 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -677,16 +677,22 @@ free_carv: * A lookup table for resource handlers. The indices are defined in * enum fw_resource_type. */ -static rproc_handle_resource_t rproc_handle_rsc[] = { +static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ }; +static rproc_handle_resource_t rproc_handle_vdev_rsc[RSC_LAST] = { + [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, +}; + /* handle firmware resource entries before booting the remote processor */ static int -rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len) +rproc_handle_resource(struct rproc *rproc, struct resource_table *table, + int len, + rproc_handle_resource_t handlers[RSC_LAST]) { struct device *dev = rproc-dev; rproc_handle_resource_t handler; @@ -711,7 +717,7 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len continue; } - handler = rproc_handle_rsc[hdr-type]; + handler = handlers[hdr-type]; if (!handler) continue; @@ -723,40 +729,6 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len return ret; } -/* handle firmware resource entries while registering the remote processor */ -static int -rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int len) -{ - struct device *dev = rproc-dev; - int ret = 0, i; - - for (i = 0; i table-num; i++) { - int offset = table-offset[i]; - struct fw_rsc_hdr *hdr = (void *)table + offset; - int avail = len - offset - sizeof(*hdr); - struct fw_rsc_vdev *vrsc; - - /* make sure table isn't truncated */ - if (avail 0) { - dev_err(dev, rsc table is truncated\n); - return -EINVAL; - } - - dev_dbg(dev, %s: rsc type %d\n, __func__, hdr-type); - - if (hdr-type != RSC_VDEV) - continue; - - vrsc = (struct fw_rsc_vdev *)hdr-data; - - ret = rproc_handle_vdev(rproc, vrsc, avail); - if (ret) - break; - } - - return ret; -} - /** * rproc_resource_cleanup() - clean up and free all acquired resources * @rproc: rproc handle @@ -836,7 +808,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) } /* handle fw resources which are required to boot rproc */ - ret = rproc_handle_boot_rsc(rproc, table, tablesz); + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_rsc); if (ret) { dev_err(dev, Failed to process resources: %d\n, ret); goto clean_up; @@ -891,7 +863,8 @@ static void rproc_fw_config_virtio(const struct firmware *fw, void *context) goto out; /* look for virtio devices and register them */ - ret = rproc_handle_virtio_rsc(rproc, table, tablesz); + ret = rproc_handle_resource(rproc, table, tablesz, + rproc_handle_vdev_rsc); if (ret) goto out; -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 04/11] remoteproc: Add state RPROC_LOADED
Add state RPROC_LOADED as firmware loading and startup will be different states. Signed-off-by: Sjur Brændeland sjur.brandel...@steicsson.com --- drivers/remoteproc/remoteproc_debugfs.c |1 + include/linux/remoteproc.h |3 ++- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 157a573..a026359 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -68,6 +68,7 @@ static const char * const rproc_state_string[] = { suspended, running, crashed, + loaded, invalid, }; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index cdacd66..932edc7 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -357,7 +357,8 @@ enum rproc_state { RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, - RPROC_LAST = 4, + RPROC_LOADED= 4, + RPROC_LAST = 5, }; /** -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 05/11] remoteproc: Load firmware once.
Load the firmware once and pave the way for two way communication of virtio configuration and status bits. The virtio ring device address is now stored in the resource table. NOTE: This patch requires device firmware change! The device memory layout changes. The virtio rings are no longer located at start of device memory. But the vring address can be read from the resource table. Also Carveout must be located before the virtio rings in the resource table. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 111 -- 1 files changed, 38 insertions(+), 73 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 151e138..ea4b76a 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -686,14 +686,10 @@ static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, - [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ -}; - -static rproc_handle_resource_t rproc_handle_vdev_rsc[RSC_LAST] = { [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; -/* handle firmware resource entries before booting the remote processor */ +/* handle firmware resource entries */ static int rproc_handle_resource(struct rproc *rproc, struct resource_table *table, int len, @@ -778,18 +774,27 @@ static void rproc_resource_cleanup(struct rproc *rproc) } /* - * take a firmware and boot a remote processor with it. + * take a firmware, parse the resouce table and load it + * + * Note: this function is called asynchronously upon registration of the + * remote processor (so we must wait until it completes before we try + * to unregister the device. one other option is just to use kref here, + * that might be cleaner). */ -static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) +static void rproc_fw_load(const struct firmware *fw, void *context) { + struct rproc *rproc = context; struct device *dev = rproc-dev; const char *name = rproc-firmware; struct resource_table *table; int ret, tablesz; + if (!fw) + goto release_fw; + ret = rproc_fw_sanity_check(rproc, fw); if (ret) - return ret; + goto release_fw; dev_info(dev, Booting fw image %s, size %zd\n, name, fw-size); @@ -800,7 +805,7 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) ret = rproc_enable_iommu(rproc); if (ret) { dev_err(dev, can't enable iommu: %d\n, ret); - return ret; + goto release_fw; } rproc-bootaddr = rproc_get_boot_addr(rproc, fw); @@ -826,60 +831,23 @@ static int rproc_fw_boot(struct rproc *rproc, const struct firmware *fw) goto clean_up; } - /* power up the remote processor */ - ret = rproc-ops-start(rproc); - if (ret) { - dev_err(dev, can't start rproc %s: %d\n, rproc-name, ret); - goto clean_up; - } - - rproc-state = RPROC_RUNNING; - - dev_info(dev, remote processor %s is now up\n, rproc-name); + rproc-state = RPROC_LOADED; - return 0; + dev_info(dev, remote processor %s is loaded to memory\n, rproc-name); clean_up: - rproc_resource_cleanup(rproc); - rproc_disable_iommu(rproc); - return ret; -} - -/* - * take a firmware and look for virtio devices to register. - * - * Note: this function is called asynchronously upon registration of the - * remote processor (so we must wait until it completes before we try - * to unregister the device. one other option is just to use kref here, - * that might be cleaner). - */ -static void rproc_fw_config_virtio(const struct firmware *fw, void *context) -{ - struct rproc *rproc = context; - struct resource_table *table; - int ret, tablesz; - - if (rproc_fw_sanity_check(rproc, fw) 0) - goto out; - - /* look for the resource table */ - table = rproc_find_rsc_table(rproc, fw, tablesz); - if (!table) - goto out; - - /* look for virtio devices and register them */ - ret = rproc_handle_resource(rproc, table, tablesz, - rproc_handle_vdev_rsc); - if (ret) - goto out; - -out: + if (ret) { + rproc-state = RPROC_OFFLINE; + rproc_resource_cleanup(rproc); + rproc_disable_iommu(rproc); + } +release_fw: release_firmware(fw); /* allow rproc_del() contexts, if any, to proceed
[RFCv2 06/11] remoteproc: Add resource entry number to callbacks
Add information on the resource entry sequence number to the callback functions. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 19 +++ 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index ea4b76a..2c78ea5 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -43,9 +43,8 @@ #include remoteproc_internal.h -typedef int (*rproc_handle_resources_t)(struct rproc *rproc, - struct resource_table *table, int len); -typedef int (*rproc_handle_resource_t)(struct rproc *rproc, void *, int avail); +typedef int (*rproc_handle_resource_t)(struct rproc *rproc, void *, + int avail, int seqno); /* Unique indices for remoteproc devices */ static DEFINE_IDA(rproc_dev_index); @@ -300,6 +299,7 @@ void rproc_free_vring(struct rproc_vring *rvring) * @rproc: the remote processor * @rsc: the vring resource descriptor * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * This resource entry requests the host to statically register a virtio * device (vdev), and setup everything needed to support it. It contains @@ -323,7 +323,7 @@ void rproc_free_vring(struct rproc_vring *rvring) * Returns 0 on success, or an appropriate error code otherwise */ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, - int avail) + int avail, int seqno) { struct device *dev = rproc-dev; struct rproc_vdev *rvdev; @@ -389,6 +389,7 @@ free_rvdev: * @rproc: the remote processor * @rsc: the trace resource descriptor * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * In case the remote processor dumps trace logs into memory, * export it via debugfs. @@ -401,7 +402,7 @@ free_rvdev: * Returns 0 on success, or an appropriate error code otherwise */ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, - int avail) + int avail, int seqno) { struct rproc_mem_entry *trace; struct device *dev = rproc-dev; @@ -462,6 +463,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, * @rproc: remote processor handle * @rsc: the devmem resource entry * @avail: size of available data (for sanity checking the image) + * @seqno: resource entry sequence number * * Remote processors commonly need to access certain on-chip peripherals. * @@ -483,7 +485,7 @@ static int rproc_handle_trace(struct rproc *rproc, struct fw_rsc_trace *rsc, * are outside those ranges. */ static int rproc_handle_devmem(struct rproc *rproc, struct fw_rsc_devmem *rsc, - int avail) + int avail, int seqno) { struct rproc_mem_entry *mapping; struct device *dev = rproc-dev; @@ -542,6 +544,7 @@ out: * @rproc: rproc handle * @rsc: the resource entry * @avail: size of available data (for image validation) + * @seqno: resource entry sequence number * * This function will handle firmware requests for allocation of physically * contiguous memory regions. @@ -556,7 +559,7 @@ out: * pressure is important; it may have a substantial impact on performance. */ static int rproc_handle_carveout(struct rproc *rproc, - struct fw_rsc_carveout *rsc, int avail) + struct fw_rsc_carveout *rsc, int avail, int seqno) { struct rproc_mem_entry *carveout, *mapping; struct device *dev = rproc-dev; @@ -722,7 +725,7 @@ rproc_handle_resource(struct rproc *rproc, struct resource_table *table, if (!handler) continue; - ret = handler(rproc, rsc, avail); + ret = handler(rproc, rsc, avail, i); if (ret) break; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 07/11] remoteproc: Register virtio devices after vring allocation
Postpone the registration of virtio devices until all vritio ring resource has been allocated. This fixes the following bug: The driver's start callback is called before all vring notify ids are allocated and max_notifyid will be increased after starting the remoteproc. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 26 ++ include/linux/remoteproc.h |2 ++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 2c78ea5..8a7de5c 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -367,6 +367,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, /* remember the device features */ rvdev-dfeatures = rsc-dfeatures; + /* remember the device resource entry number */ + rvdev-rsc_seqno = seqno; /* remember the resource entry */ rvdev-rsc = rsc; @@ -384,6 +386,20 @@ free_rvdev: return ret; } +static int rproc_register_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, + int avail, int seqno) +{ + struct rproc_vdev *rvdev, *rvtmp; + + dev_dbg(rproc-dev, register device %s\n, rproc-name); + + list_for_each_entry_safe(rvdev, rvtmp, rproc-rvdevs, node) + if (seqno == rvdev-rsc_seqno) + return rproc_add_virtio_dev(rvdev, rsc-id); + + return -ENODEV; +} + /** * rproc_handle_trace() - handle a shared trace buffer resource * @rproc: the remote processor @@ -692,6 +708,10 @@ static rproc_handle_resource_t rproc_handle_rsc[RSC_LAST] = { [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; +static rproc_handle_resource_t rproc_handle_reg[RSC_LAST] = { + [RSC_VDEV] = (rproc_handle_resource_t)rproc_register_vdev, +}; + /* handle firmware resource entries */ static int rproc_handle_resource(struct rproc *rproc, struct resource_table *table, @@ -834,6 +854,12 @@ static void rproc_fw_load(const struct firmware *fw, void *context) goto clean_up; } + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_reg); + if (ret) { + dev_err(dev, Failed to process resources: %d\n, ret); + goto clean_up; + } + rproc-state = RPROC_LOADED; dev_info(dev, remote processor %s is loaded to memory\n, rproc-name); diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 932edc7..60a1002 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -465,6 +465,7 @@ struct rproc_vring { * @vring: the vrings for this vdev * @dfeatures: virtio device features * @gfeatures: virtio guest features + * @rsc_seqno: sequence number in resource table * @rsc: vdev resource entry */ struct rproc_vdev { @@ -474,6 +475,7 @@ struct rproc_vdev { struct rproc_vring vring[RVDEV_NUM_VRINGS]; unsigned long dfeatures; unsigned long gfeatures; + u32 rsc_seqno; struct fw_rsc_vdev *rsc; }; -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 09/11] remoteproc: Add operation to find resource table in memory
Add function find_rsc_table_va to firmware ops. This function returns the location of the resource table in shared memory after loading. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_elf_loader.c | 16 ++- drivers/remoteproc/remoteproc_internal.h | 13 drivers/remoteproc/ste_modem_rproc.c | 43 +++- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index 69832d9..3f6e315 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -306,9 +306,23 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table; } +struct resource_table *rproc_elf_get_rsctab_addr(struct rproc *rproc, +const struct firmware *fw) +{ + struct elf32_shdr *shdr; + + shdr = find_rsc_shdr(rproc-dev, (struct elf32_hdr *)fw-data); + if (!shdr) + return NULL; + + /* Find resource table in loaded segments */ + return rproc_da_to_va(rproc, shdr-sh_addr, shdr-sh_size); +} + const struct rproc_fw_ops rproc_elf_fw_ops = { .load = rproc_elf_load_segments, .find_rsc_table = rproc_elf_find_rsc_table, .sanity_check = rproc_elf_sanity_check, - .get_boot_addr = rproc_elf_get_boot_addr + .get_boot_addr = rproc_elf_get_boot_addr, + .get_rsctab_addr = rproc_elf_get_rsctab_addr }; diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index 7bb6648..3a5cb7d 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -32,6 +32,7 @@ struct rproc; * expects to find it * @sanity_check: sanity check the fw image * @get_boot_addr: get boot address to entry point specified in firmware + * @get_rsctab_addr: get resouce table address as specified in firmware */ struct rproc_fw_ops { struct resource_table *(*find_rsc_table) (struct rproc *rproc, @@ -40,6 +41,8 @@ struct rproc_fw_ops { int (*load)(struct rproc *rproc, const struct firmware *fw); int (*sanity_check)(struct rproc *rproc, const struct firmware *fw); u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw); + struct resource_table *(*get_rsctab_addr)(struct rproc *rproc, + const struct firmware *fw); }; /* from remoteproc_core.c */ @@ -102,6 +105,16 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc, return NULL; } +static inline +struct resource_table *rproc_get_rsctab_addr(struct rproc *rproc, + const struct firmware *fw) +{ + if (rproc-fw_ops-get_rsctab_addr) + return rproc-fw_ops-get_rsctab_addr(rproc, fw); + + return NULL; +} + extern const struct rproc_fw_ops rproc_elf_fw_ops; #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c index a7743c0..59e99f1 100644 --- a/drivers/remoteproc/ste_modem_rproc.c +++ b/drivers/remoteproc/ste_modem_rproc.c @@ -64,26 +64,18 @@ static int sproc_load_segments(struct rproc *rproc, const struct firmware *fw) } /* Find the entry for resource table in the Table of Content */ -static struct ste_toc_entry *sproc_find_rsc_entry(const struct firmware *fw) +static const struct ste_toc_entry *sproc_find_rsc_entry(const void *data) { int i; - struct ste_toc *toc; - - if (!fw) - return NULL; - - toc = (void *)fw-data; + const struct ste_toc *toc; + toc = data; /* Search the table for the resource table */ for (i = 0; i SPROC_MAX_TOC_ENTRIES toc-table[i].start != 0x; i++) { - if (!strncmp(toc-table[i].name, SPROC_RESOURCE_NAME, -sizeof(toc-table[i].name))) { - if (toc-table[i].start fw-size) - return NULL; +sizeof(toc-table[i].name))) return toc-table[i]; - } } return NULL; @@ -96,9 +88,12 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, { struct sproc *sproc = rproc-priv; struct resource_table *table; - struct ste_toc_entry *entry; + const struct ste_toc_entry *entry; - entry = sproc_find_rsc_entry(fw); + if (!fw) + return NULL; + + entry = sproc_find_rsc_entry(fw-data); if (!entry) { sproc_err(sproc, resource table not found in fw\n); return NULL; @@ -149,10 +144,30 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table
[RFCv2 11/11] remoteproc: Support virtio config space.
Support virtio configuration space and device status and feature negotiation with remote device. This virtio device can now access the resource table in shared memory. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 10 -- drivers/remoteproc/remoteproc_virtio.c | 30 +++--- include/linux/remoteproc.h |2 -- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index f95c08c..d59c4cf 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -364,13 +364,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, goto free_rvdev; } - /* remember the device features */ - rvdev-dfeatures = rsc-dfeatures; - /* remember the device resource entry number */ rvdev-rsc_seqno = seqno; - /* remember the resource entry */ - rvdev-rsc = rsc; list_add_tail(rvdev-node, rproc-rvdevs); @@ -394,8 +389,11 @@ static int rproc_register_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, dev_dbg(rproc-dev, register device %s\n, rproc-name); list_for_each_entry_safe(rvdev, rvtmp, rproc-rvdevs, node) - if (seqno == rvdev-rsc_seqno) + if (seqno == rvdev-rsc_seqno) { + /* remember the resource entry */ + rvdev-rsc = rsc; return rproc_add_virtio_dev(rvdev, rsc-id); + } return -ENODEV; } diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..9ebc5d9 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -176,16 +176,21 @@ error: */ static u8 rproc_virtio_get_status(struct virtio_device *vdev) { - return 0; + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + return rvdev-rsc-status; } static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev-rsc-status = status; dev_dbg(vdev-dev, status: %d\n, status); } static void rproc_virtio_reset(struct virtio_device *vdev) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev-rsc-status = 0; dev_dbg(vdev-dev, reset !\n); } @@ -194,7 +199,7 @@ static u32 rproc_virtio_get_features(struct virtio_device *vdev) { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); - return rvdev-dfeatures; + return rvdev-rsc-dfeatures; } static void rproc_virtio_finalize_features(struct virtio_device *vdev) @@ -213,7 +218,23 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * fixed as part of a small resource table overhaul and then an * extension of the virtio resource entries. */ - rvdev-gfeatures = vdev-features[0]; + rvdev-rsc-gfeatures = vdev-features[0]; +} + +void rproc_virtio_get(struct virtio_device *vdev, unsigned offset, + void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = rvdev-rsc-vring[rvdev-rsc-num_of_vrings]; + memcpy(buf, cfg + offset, len); +} + +void rproc_virtio_set(struct virtio_device *vdev, unsigned offset, + const void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = rvdev-rsc-vring[rvdev-rsc-num_of_vrings]; + memcpy(cfg + offset, buf, len); } static struct virtio_config_ops rproc_virtio_config_ops = { @@ -224,6 +245,9 @@ static struct virtio_config_ops rproc_virtio_config_ops = { .reset = rproc_virtio_reset, .set_status = rproc_virtio_set_status, .get_status = rproc_virtio_get_status, + .get= rproc_virtio_get, + .set= rproc_virtio_set, + }; /* diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 60a1002..de20e39 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -473,8 +473,6 @@ struct rproc_vdev { struct rproc *rproc; struct virtio_device vdev; struct rproc_vring vring[RVDEV_NUM_VRINGS]; - unsigned long dfeatures; - unsigned long gfeatures; u32 rsc_seqno; struct fw_rsc_vdev *rsc; }; -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 10/11] remoteproc: Find resource table in device memory
Call rproc_get_rsctab_addr() to get the address of the resource table in shared memory. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 8a7de5c..f95c08c 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -854,6 +854,13 @@ static void rproc_fw_load(const struct firmware *fw, void *context) goto clean_up; } + /* Find the new location of the resource table in device memory */ + table = rproc_get_rsctab_addr(rproc, fw); + if (!table) { + ret = -EINVAL; + goto clean_up; + } + ret = rproc_handle_resource(rproc, table, tablesz, rproc_handle_reg); if (ret) { dev_err(dev, Failed to process resources: %d\n, ret); -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 08/11] remoteproc: Refactor function rproc_elf_find_rsc_table
Refatcor rproc_elf_find_rsc_table and split out the scanning for the section header named resource table. This is done to prepare for loading firmware once. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_elf_loader.c | 83 +--- 1 files changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index e1f89d6..69832d9 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -208,38 +208,19 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw) return ret; } -/** - * rproc_elf_find_rsc_table() - find the resource table - * @rproc: the rproc handle - * @fw: the ELF firmware image - * @tablesz: place holder for providing back the table size - * - * This function finds the resource table inside the remote processor's - * firmware. It is used both upon the registration of @rproc (in order - * to look for and register the supported virito devices), and when the - * @rproc is booted. - * - * Returns the pointer to the resource table if it is found, and write its - * size into @tablesz. If a valid table isn't found, NULL is returned - * (and @tablesz isn't set). - */ -static struct resource_table * -rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, - int *tablesz) +static struct elf32_shdr * +find_rsc_shdr(struct device *dev, struct elf32_hdr *ehdr) { - struct elf32_hdr *ehdr; struct elf32_shdr *shdr; + int i; const char *name_table; - struct device *dev = rproc-dev; struct resource_table *table = NULL; - int i; - const u8 *elf_data = fw-data; + const u8 *elf_data = (void *)ehdr; - ehdr = (struct elf32_hdr *)elf_data; + /* look for the resource table and handle it */ shdr = (struct elf32_shdr *)(elf_data + ehdr-e_shoff); name_table = elf_data + shdr[ehdr-e_shstrndx].sh_offset; - /* look for the resource table and handle it */ for (i = 0; i ehdr-e_shnum; i++, shdr++) { int size = shdr-sh_size; int offset = shdr-sh_offset; @@ -249,12 +230,6 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, table = (struct resource_table *)(elf_data + offset); - /* make sure we have the entire table */ - if (offset + size fw-size) { - dev_err(dev, resource table truncated\n); - return NULL; - } - /* make sure table has at least the header */ if (sizeof(struct resource_table) size) { dev_err(dev, header-less resource table\n); @@ -280,10 +255,54 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return NULL; } - *tablesz = shdr-sh_size; - break; + return shdr; + } + + return NULL; +} + +/** + * rproc_elf_find_rsc_table() - find the resource table + * @rproc: the rproc handle + * @fw: the ELF firmware image + * @tablesz: place holder for providing back the table size + * + * This function finds the resource table inside the remote processor's + * firmware. It is used both upon the registration of @rproc (in order + * to look for and register the supported virito devices), and when the + * @rproc is booted. + * + * Returns the pointer to the resource table if it is found, and write its + * size into @tablesz. If a valid table isn't found, NULL is returned + * (and @tablesz isn't set). + */ +static struct resource_table * +rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, + int *tablesz) +{ + struct elf32_hdr *ehdr; + struct elf32_shdr *shdr; + + struct device *dev = rproc-dev; + struct resource_table *table = NULL; + + const u8 *elf_data = fw-data; + + ehdr = (struct elf32_hdr *)elf_data; + + shdr = find_rsc_shdr(dev, ehdr); + if (!shdr) + return NULL; + + /* make sure we have the entire table */ + if (shdr-sh_offset + shdr-sh_size fw-size) { + dev_err(dev, resource table truncated\n); + return NULL; } + table = (struct resource_table *)(elf_data + shdr-sh_offset); + *tablesz = shdr-sh_size; + return table; } -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 00/10] remoteproc: Support bi-directional vdev config space
Changes since v1: - Separate vring allocation and registration - Break up patches in smaller parts to facilitate review. (The patches here are perhaps too fine grained and can be squashed later). This patch-set adds support for shared resource table between Linux kernel and remote devices. Features: - dynamically-allocated address of the vrings can be communicated - vdev statuses can be communicated - virtio config space becomes bi-directional - virtio feature negotiation is two-way NOTE: This change may not be backwards compatible with existing device firmware! The memory allocation order may change. Previously the Virtio Rings were always allocated at start of the share memory area, but with this patch-set the memory allocation are done in the order defined resource table. A number of changes are introduced to achieve this: - The firmware is loaded once. - The allocations of resources is done before registering the virtio devices. - The virtio device uses resource bits, status and config space in memory shared with the device. Review comments and feedback are welcomed! Thanks, Sjur Sjur Brændeland (11): remoteproc: Code cleanup of resource parsing remoteproc: Move check on firmware name to rproc_add remoteproc: Set vring addresses in resource table remoteproc: Add state RPROC_LOADED remoteproc: Load firmware once. remoteproc: Add resource entry number to callbacks remoteproc: Register virtio devices after vring allocation remoteproc: Refactor function rproc_elf_find_rsc_table remoteproc: Add operation to find resource table in memory remoteproc: Find resource table in device memory remoteproc: Support virtio config space. drivers/remoteproc/remoteproc_core.c | 216 --- drivers/remoteproc/remoteproc_debugfs.c|1 + drivers/remoteproc/remoteproc_elf_loader.c | 99 +- drivers/remoteproc/remoteproc_internal.h | 13 ++ drivers/remoteproc/remoteproc_virtio.c | 30 - drivers/remoteproc/ste_modem_rproc.c | 43 -- include/linux/remoteproc.h |9 +- 7 files changed, 238 insertions(+), 173 deletions(-) -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 03/11] remoteproc: Set vring addresses in resource table
Set the vring addresses in the resource table so that the remote device can read the actual addresses used. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c |5 + include/linux/remoteproc.h |2 ++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 7d593a1..151e138 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -238,6 +238,8 @@ int rproc_alloc_vring(struct rproc_vdev *rvdev, int i) rvring-dma = dma; rvring-notifyid = notifyid; + rvdev-rsc-vring[i].da = dma; + rvdev-rsc-vring[i].notifyid = notifyid; return 0; } @@ -365,6 +367,9 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, /* remember the device features */ rvdev-dfeatures = rsc-dfeatures; + /* remember the resource entry */ + rvdev-rsc = rsc; + list_add_tail(rvdev-node, rproc-rvdevs); /* it is now safe to add the virtio device */ diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index faf3332..cdacd66 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -464,6 +464,7 @@ struct rproc_vring { * @vring: the vrings for this vdev * @dfeatures: virtio device features * @gfeatures: virtio guest features + * @rsc: vdev resource entry */ struct rproc_vdev { struct list_head node; @@ -472,6 +473,7 @@ struct rproc_vdev { struct rproc_vring vring[RVDEV_NUM_VRINGS]; unsigned long dfeatures; unsigned long gfeatures; + struct fw_rsc_vdev *rsc; }; struct rproc *rproc_alloc(struct device *dev, const char *name, -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFCv2 02/11] remoteproc: Move check on firmware name to rproc_add
Verify that firmware name is defined in rproc_add. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 13 ++--- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index feb39a7..7d593a1 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -989,13 +989,6 @@ int rproc_boot(struct rproc *rproc) return ret; } - /* loading a firmware is required */ - if (!rproc-firmware) { - dev_err(dev, %s: no firmware to load\n, __func__); - ret = -EINVAL; - goto unlock_mutex; - } - /* prevent underlying implementation from being removed */ if (!try_module_get(dev-parent-driver-owner)) { dev_err(dev, %s: can't get owner\n, __func__); @@ -1120,6 +1113,12 @@ int rproc_add(struct rproc *rproc) struct device *dev = rproc-dev; int ret; + /* loading a firmware is required */ + if (!rproc-firmware) { + dev_err(dev, %s: no firmware to load\n, __func__); + return -EINVAL; + } + ret = device_add(dev); if (ret 0) return ret; -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv9 2/2] virtio_console: Add support for remoteproc serial
Add a simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for communicating with a remote processor in an asymmetric multi-processing configuration. This implementation reuses the existing virtio_console implementation, and adds support for DMA allocation of data buffers and disables use of tty console and the virtio control queue. Signed-off-by: Sjur Brændeland Acked-by: Amit Shah --- drivers/char/virtio_console.c | 192 ++- include/uapi/linux/virtio_ids.h |1 + 2 files changed, 170 insertions(+), 23 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 5482246..55a89a4 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -37,8 +37,12 @@ #include #include #include +#include +#include #include "../tty/hvc/hvc_console.h" +#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC) + /* * This is a global struct for storing common data for all the devices * this driver handles. @@ -112,6 +116,15 @@ struct port_buffer { /* offset in the buf from which to consume data */ size_t offset; + /* DMA address of buffer */ + dma_addr_t dma; + + /* Device we got DMA memory from */ + struct device *dev; + + /* List of pending dma buffers to free */ + struct list_head list; + /* If sgpages == 0 then buf is used */ unsigned int sgpages; @@ -331,6 +344,11 @@ static bool is_console_port(struct port *port) return false; } +static bool is_rproc_serial(const struct virtio_device *vdev) +{ + return is_rproc_enabled && vdev->id.device == VIRTIO_ID_RPROC_SERIAL; +} + static inline bool use_multiport(struct ports_device *portdev) { /* @@ -342,11 +360,13 @@ static inline bool use_multiport(struct ports_device *portdev) return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT); } -static void free_buf(struct port_buffer *buf) +static DEFINE_SPINLOCK(dma_bufs_lock); +static LIST_HEAD(pending_free_dma_bufs); + +static void free_buf(struct port_buffer *buf, bool can_sleep) { unsigned int i; - kfree(buf->buf); for (i = 0; i < buf->sgpages; i++) { struct page *page = sg_page(>sg[i]); if (!page) @@ -354,14 +374,57 @@ static void free_buf(struct port_buffer *buf) put_page(page); } + if (!buf->dev) { + kfree(buf->buf); + } else if (is_rproc_enabled) { + unsigned long flags; + + /* dma_free_coherent requires interrupts to be enabled. */ + if (!can_sleep) { + /* queue up dma-buffers to be freed later */ + spin_lock_irqsave(_bufs_lock, flags); + list_add_tail(>list, _free_dma_bufs); + spin_unlock_irqrestore(_bufs_lock, flags); + return; + } + dma_free_coherent(buf->dev, buf->size, buf->buf, buf->dma); + + /* Release device refcnt and allow it to be freed */ + put_device(buf->dev); + } + kfree(buf); } +static void reclaim_dma_bufs(void) +{ + unsigned long flags; + struct port_buffer *buf, *tmp; + LIST_HEAD(tmp_list); + + if (list_empty(_free_dma_bufs)) + return; + + /* Create a copy of the pending_free_dma_bufs while holding the lock */ + spin_lock_irqsave(_bufs_lock, flags); + list_cut_position(_list, _free_dma_bufs, + pending_free_dma_bufs.prev); + spin_unlock_irqrestore(_bufs_lock, flags); + + /* Release the dma buffers, without irqs enabled */ + list_for_each_entry_safe(buf, tmp, _list, list) { + list_del(>list); + free_buf(buf, true); + } +} + static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, int pages) { struct port_buffer *buf; + reclaim_dma_bufs(); + /* * Allocate buffer and the sg list. The sg list array is allocated * directly after the port_buffer struct. @@ -373,11 +436,34 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, buf->sgpages = pages; if (pages > 0) { + buf->dev = NULL; buf->buf = NULL; return buf; } - buf->buf = kmalloc(buf_size, GFP_KERNEL); + if (is_rproc_serial(vq->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. +* The code he
[PATCHv9 0/2] virtio_console: Add rproc_serial driver
This patch-set introduces a new virtio type "rproc_serial" for communicating with remote processors over shared memory. The driver depends on the the remoteproc framework. As preparation for introducing "rproc_serial" I've done a refactoring of the transmit buffer handling. NOTE: These two patches are identical to first two patches of the V8 patch-set, but are rebased to virtio-next. Regards, Sjur Sjur Brændeland (2): virtio_console: Merge struct buffer_token into struct port_buffer virtio_console: Add support for remoteproc serial drivers/char/virtio_console.c | 317 +++ include/uapi/linux/virtio_ids.h |1 + 2 files changed, 221 insertions(+), 97 deletions(-) -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv9 1/2] virtio_console: Merge struct buffer_token into struct port_buffer
Refactoring the splice functionality by unifying the approach for sending scatter-lists and regular buffers. This simplifies buffer handling and reduces code size. Splice will now allocate a port_buffer and send_buf() and free_buf() can always be used for any buffer. Signed-off-by: Sjur Brændeland Acked-by: Amit Shah --- drivers/char/virtio_console.c | 129 + 1 files changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index db244b5..5482246 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -111,6 +111,12 @@ struct port_buffer { size_t len; /* offset in the buf from which to consume data */ size_t offset; + + /* If sgpages == 0 then buf is used */ + unsigned int sgpages; + + /* sg is used if spages > 0. sg must be the last in is struct */ + struct scatterlist sg[0]; }; /* @@ -338,17 +344,39 @@ static inline bool use_multiport(struct ports_device *portdev) static void free_buf(struct port_buffer *buf) { + unsigned int i; + kfree(buf->buf); + for (i = 0; i < buf->sgpages; i++) { + struct page *page = sg_page(>sg[i]); + if (!page) + break; + put_page(page); + } + kfree(buf); } -static struct port_buffer *alloc_buf(size_t buf_size) +static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, +int pages) { struct port_buffer *buf; - buf = kmalloc(sizeof(*buf), GFP_KERNEL); + /* +* Allocate buffer and the sg list. The sg list array is allocated +* directly after the port_buffer struct. +*/ + buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages, + GFP_KERNEL); if (!buf) goto fail; + + buf->sgpages = pages; + if (pages > 0) { + buf->buf = NULL; + return buf; + } + buf->buf = kmalloc(buf_size, GFP_KERNEL); if (!buf->buf) goto free_buf; @@ -478,52 +506,26 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, return 0; } -struct buffer_token { - union { - void *buf; - struct scatterlist *sg; - } u; - /* If sgpages == 0 then buf is used, else sg is used */ - unsigned int sgpages; -}; - -static void reclaim_sg_pages(struct scatterlist *sg, unsigned int nrpages) -{ - int i; - struct page *page; - - for (i = 0; i < nrpages; i++) { - page = sg_page([i]); - if (!page) - break; - put_page(page); - } - kfree(sg); -} /* Callers must take the port->outvq_lock */ static void reclaim_consumed_buffers(struct port *port) { - struct buffer_token *tok; + struct port_buffer *buf; unsigned int len; if (!port->portdev) { /* Device has been unplugged. vqs are already gone. */ return; } - while ((tok = virtqueue_get_buf(port->out_vq, ))) { - if (tok->sgpages) - reclaim_sg_pages(tok->u.sg, tok->sgpages); - else - kfree(tok->u.buf); - kfree(tok); + while ((buf = virtqueue_get_buf(port->out_vq, ))) { + free_buf(buf); port->outvq_full = false; } } static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, int nents, size_t in_count, - struct buffer_token *tok, bool nonblock) + void *data, bool nonblock) { struct virtqueue *out_vq; int err; @@ -536,7 +538,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, reclaim_consumed_buffers(port); - err = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC); + err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC); /* Tell Host to go! */ virtqueue_kick(out_vq); @@ -574,37 +576,6 @@ done: return in_count; } -static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, - bool nonblock) -{ - struct scatterlist sg[1]; - struct buffer_token *tok; - - tok = kmalloc(sizeof(*tok), GFP_ATOMIC); - if (!tok) - return -ENOMEM; - tok->sgpages = 0; - tok->u.buf = in_buf; - - sg_init_one(sg, in_buf, in_count); - - return __send_to_port(port, sg, 1, in_count, tok, nonblock); -} - -static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents, - size_t in_count, bool nonblock) -{ - struct b
[PATCHv9 0/2] virtio_console: Add rproc_serial driver
This patch-set introduces a new virtio type rproc_serial for communicating with remote processors over shared memory. The driver depends on the the remoteproc framework. As preparation for introducing rproc_serial I've done a refactoring of the transmit buffer handling. NOTE: These two patches are identical to first two patches of the V8 patch-set, but are rebased to virtio-next. Regards, Sjur Sjur Brændeland (2): virtio_console: Merge struct buffer_token into struct port_buffer virtio_console: Add support for remoteproc serial drivers/char/virtio_console.c | 317 +++ include/uapi/linux/virtio_ids.h |1 + 2 files changed, 221 insertions(+), 97 deletions(-) -- 1.7.5.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv9 1/2] virtio_console: Merge struct buffer_token into struct port_buffer
Refactoring the splice functionality by unifying the approach for sending scatter-lists and regular buffers. This simplifies buffer handling and reduces code size. Splice will now allocate a port_buffer and send_buf() and free_buf() can always be used for any buffer. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com Acked-by: Amit Shah amit.s...@redhat.com --- drivers/char/virtio_console.c | 129 + 1 files changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index db244b5..5482246 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -111,6 +111,12 @@ struct port_buffer { size_t len; /* offset in the buf from which to consume data */ size_t offset; + + /* If sgpages == 0 then buf is used */ + unsigned int sgpages; + + /* sg is used if spages 0. sg must be the last in is struct */ + struct scatterlist sg[0]; }; /* @@ -338,17 +344,39 @@ static inline bool use_multiport(struct ports_device *portdev) static void free_buf(struct port_buffer *buf) { + unsigned int i; + kfree(buf-buf); + for (i = 0; i buf-sgpages; i++) { + struct page *page = sg_page(buf-sg[i]); + if (!page) + break; + put_page(page); + } + kfree(buf); } -static struct port_buffer *alloc_buf(size_t buf_size) +static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, +int pages) { struct port_buffer *buf; - buf = kmalloc(sizeof(*buf), GFP_KERNEL); + /* +* Allocate buffer and the sg list. The sg list array is allocated +* directly after the port_buffer struct. +*/ + buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages, + GFP_KERNEL); if (!buf) goto fail; + + buf-sgpages = pages; + if (pages 0) { + buf-buf = NULL; + return buf; + } + buf-buf = kmalloc(buf_size, GFP_KERNEL); if (!buf-buf) goto free_buf; @@ -478,52 +506,26 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, return 0; } -struct buffer_token { - union { - void *buf; - struct scatterlist *sg; - } u; - /* If sgpages == 0 then buf is used, else sg is used */ - unsigned int sgpages; -}; - -static void reclaim_sg_pages(struct scatterlist *sg, unsigned int nrpages) -{ - int i; - struct page *page; - - for (i = 0; i nrpages; i++) { - page = sg_page(sg[i]); - if (!page) - break; - put_page(page); - } - kfree(sg); -} /* Callers must take the port-outvq_lock */ static void reclaim_consumed_buffers(struct port *port) { - struct buffer_token *tok; + struct port_buffer *buf; unsigned int len; if (!port-portdev) { /* Device has been unplugged. vqs are already gone. */ return; } - while ((tok = virtqueue_get_buf(port-out_vq, len))) { - if (tok-sgpages) - reclaim_sg_pages(tok-u.sg, tok-sgpages); - else - kfree(tok-u.buf); - kfree(tok); + while ((buf = virtqueue_get_buf(port-out_vq, len))) { + free_buf(buf); port-outvq_full = false; } } static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, int nents, size_t in_count, - struct buffer_token *tok, bool nonblock) + void *data, bool nonblock) { struct virtqueue *out_vq; int err; @@ -536,7 +538,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, reclaim_consumed_buffers(port); - err = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC); + err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC); /* Tell Host to go! */ virtqueue_kick(out_vq); @@ -574,37 +576,6 @@ done: return in_count; } -static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, - bool nonblock) -{ - struct scatterlist sg[1]; - struct buffer_token *tok; - - tok = kmalloc(sizeof(*tok), GFP_ATOMIC); - if (!tok) - return -ENOMEM; - tok-sgpages = 0; - tok-u.buf = in_buf; - - sg_init_one(sg, in_buf, in_count); - - return __send_to_port(port, sg, 1, in_count, tok, nonblock); -} - -static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents, - size_t in_count, bool nonblock) -{ - struct buffer_token *tok
[PATCHv9 2/2] virtio_console: Add support for remoteproc serial
Add a simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for communicating with a remote processor in an asymmetric multi-processing configuration. This implementation reuses the existing virtio_console implementation, and adds support for DMA allocation of data buffers and disables use of tty console and the virtio control queue. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com Acked-by: Amit Shah amit.s...@redhat.com --- drivers/char/virtio_console.c | 192 ++- include/uapi/linux/virtio_ids.h |1 + 2 files changed, 170 insertions(+), 23 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 5482246..55a89a4 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -37,8 +37,12 @@ #include linux/wait.h #include linux/workqueue.h #include linux/module.h +#include linux/dma-mapping.h +#include linux/kconfig.h #include ../tty/hvc/hvc_console.h +#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC) + /* * This is a global struct for storing common data for all the devices * this driver handles. @@ -112,6 +116,15 @@ struct port_buffer { /* offset in the buf from which to consume data */ size_t offset; + /* DMA address of buffer */ + dma_addr_t dma; + + /* Device we got DMA memory from */ + struct device *dev; + + /* List of pending dma buffers to free */ + struct list_head list; + /* If sgpages == 0 then buf is used */ unsigned int sgpages; @@ -331,6 +344,11 @@ static bool is_console_port(struct port *port) return false; } +static bool is_rproc_serial(const struct virtio_device *vdev) +{ + return is_rproc_enabled vdev-id.device == VIRTIO_ID_RPROC_SERIAL; +} + static inline bool use_multiport(struct ports_device *portdev) { /* @@ -342,11 +360,13 @@ static inline bool use_multiport(struct ports_device *portdev) return portdev-vdev-features[0] (1 VIRTIO_CONSOLE_F_MULTIPORT); } -static void free_buf(struct port_buffer *buf) +static DEFINE_SPINLOCK(dma_bufs_lock); +static LIST_HEAD(pending_free_dma_bufs); + +static void free_buf(struct port_buffer *buf, bool can_sleep) { unsigned int i; - kfree(buf-buf); for (i = 0; i buf-sgpages; i++) { struct page *page = sg_page(buf-sg[i]); if (!page) @@ -354,14 +374,57 @@ static void free_buf(struct port_buffer *buf) put_page(page); } + if (!buf-dev) { + kfree(buf-buf); + } else if (is_rproc_enabled) { + unsigned long flags; + + /* dma_free_coherent requires interrupts to be enabled. */ + if (!can_sleep) { + /* queue up dma-buffers to be freed later */ + spin_lock_irqsave(dma_bufs_lock, flags); + list_add_tail(buf-list, pending_free_dma_bufs); + spin_unlock_irqrestore(dma_bufs_lock, flags); + return; + } + dma_free_coherent(buf-dev, buf-size, buf-buf, buf-dma); + + /* Release device refcnt and allow it to be freed */ + put_device(buf-dev); + } + kfree(buf); } +static void reclaim_dma_bufs(void) +{ + unsigned long flags; + struct port_buffer *buf, *tmp; + LIST_HEAD(tmp_list); + + if (list_empty(pending_free_dma_bufs)) + return; + + /* Create a copy of the pending_free_dma_bufs while holding the lock */ + spin_lock_irqsave(dma_bufs_lock, flags); + list_cut_position(tmp_list, pending_free_dma_bufs, + pending_free_dma_bufs.prev); + spin_unlock_irqrestore(dma_bufs_lock, flags); + + /* Release the dma buffers, without irqs enabled */ + list_for_each_entry_safe(buf, tmp, tmp_list, list) { + list_del(buf-list); + free_buf(buf, true); + } +} + static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, int pages) { struct port_buffer *buf; + reclaim_dma_bufs(); + /* * Allocate buffer and the sg list. The sg list array is allocated * directly after the port_buffer struct. @@ -373,11 +436,34 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, buf-sgpages = pages; if (pages 0) { + buf-dev = NULL; buf-buf = NULL; return buf; } - buf-buf = kmalloc(buf_size, GFP_KERNEL); + if (is_rproc_serial(vq-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
[PATCH] Introduce rproc serial device specification.
Signed-off-by: Sjur Brændeland --- >> Please pick the virtio spec from >> >> https://github.com/rustyrussell/virtio-spec >> >> and update the spec with info for remote-proc. > >Hi Sjur, > >I didn't hear back from you on this.. > Hi Amit Sorry for the delay. I've had this ready for a while waiting for go-ahead from our legal dept, and then it got stuck in my backlog. Anyway, here is a stab at describing the rproc serial device. Feedback is welcomed. Thanks, Sjur virtio-spec.lyx | 58 +++ 1 file changed, 58 insertions(+) diff --git a/virtio-spec.lyx b/virtio-spec.lyx index 83f2771..bbb5c75 100644 --- a/virtio-spec.lyx +++ b/virtio-spec.lyx @@ -9079,6 +9079,64 @@ For MMC devices (inquiry type 5) there would be some overlap between this \end_deeper \begin_layout Chapter* +Appendix J: Rproc Serial Device +\end_layout + +\begin_layout Standard +The Remoteproc Serial Device device is used when communicating with a remote + processor. + The remote processor may be a modem communicating over a shared memory + interface, or a CPU in an asymmetric multi-processing (AMP) configuration. + The Remoteproc Serial Device provides a simple device for data IO. + A device has one port and a single pair of virtqueues for input and output. + Empty buffers are placed in one virtqueue for receiving packets, and outgoing + packets are enqueued into another for transmission. +\end_layout + +\begin_layout Section* +Configuration +\end_layout + +\begin_layout Description +Subsystem +\begin_inset space ~ +\end_inset + +Device +\begin_inset space ~ +\end_inset + +ID 11 +\end_layout + +\begin_deeper +\begin_layout Description +Virtqueues 0:receiveq(port0). + 1:transmitq(port0), +\end_layout + +\begin_layout Section* +Device Initialization +\end_layout + +\begin_layout Enumerate +The receiveq for each port is populated with one or more receive buffers. +\end_layout + +\begin_layout Section* +Device Operation +\end_layout + +\begin_layout Enumerate +For output, a buffer containing the characters is placed in the port's transmitq. +\end_layout + +\begin_layout Enumerate +Inbound messages are placed in the receiveq and signalled by an interrupt. +\end_layout + +\end_deeper +\begin_layout Chapter* Appendix X: virtio-mmio \end_layout -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Introduce rproc serial device specification.
Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- Please pick the virtio spec from https://github.com/rustyrussell/virtio-spec and update the spec with info for remote-proc. Hi Sjur, I didn't hear back from you on this.. Hi Amit Sorry for the delay. I've had this ready for a while waiting for go-ahead from our legal dept, and then it got stuck in my backlog. Anyway, here is a stab at describing the rproc serial device. Feedback is welcomed. Thanks, Sjur virtio-spec.lyx | 58 +++ 1 file changed, 58 insertions(+) diff --git a/virtio-spec.lyx b/virtio-spec.lyx index 83f2771..bbb5c75 100644 --- a/virtio-spec.lyx +++ b/virtio-spec.lyx @@ -9079,6 +9079,64 @@ For MMC devices (inquiry type 5) there would be some overlap between this \end_deeper \begin_layout Chapter* +Appendix J: Rproc Serial Device +\end_layout + +\begin_layout Standard +The Remoteproc Serial Device device is used when communicating with a remote + processor. + The remote processor may be a modem communicating over a shared memory + interface, or a CPU in an asymmetric multi-processing (AMP) configuration. + The Remoteproc Serial Device provides a simple device for data IO. + A device has one port and a single pair of virtqueues for input and output. + Empty buffers are placed in one virtqueue for receiving packets, and outgoing + packets are enqueued into another for transmission. +\end_layout + +\begin_layout Section* +Configuration +\end_layout + +\begin_layout Description +Subsystem +\begin_inset space ~ +\end_inset + +Device +\begin_inset space ~ +\end_inset + +ID 11 +\end_layout + +\begin_deeper +\begin_layout Description +Virtqueues 0:receiveq(port0). + 1:transmitq(port0), +\end_layout + +\begin_layout Section* +Device Initialization +\end_layout + +\begin_layout Enumerate +The receiveq for each port is populated with one or more receive buffers. +\end_layout + +\begin_layout Section* +Device Operation +\end_layout + +\begin_layout Enumerate +For output, a buffer containing the characters is placed in the port's transmitq. +\end_layout + +\begin_layout Enumerate +Inbound messages are placed in the receiveq and signalled by an interrupt. +\end_layout + +\end_deeper +\begin_layout Chapter* Appendix X: virtio-mmio \end_layout -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)
Hi Ohad, On Sat, Aug 11, 2012 at 2:34 PM, Ohad Ben-Cohen wrote: > On Fri, Aug 10, 2012 at 6:30 PM, Ohad Ben-Cohen wrote: >> This will solve all sort of open issues we have (or going to have soon): >> ... > 1. dynamically-allocated address of the vrings can be communicated > 2. vdev statuses can be communicated > 3. virtio config space will finally become bi-directional as it should I just posted a RFC patch-set on this to linux-kernel@vger.kernel.org. Review comments is welcomed :-) > and 5. let the remote processor know about the notifyids that we've > dynamically allocated. I have run into one issue with the dynamically allocated notifyids. When allocating multiple virtio devices, only ids for the first virtio device is allocated when the remote device is started. This is fails for me, because I need to know all of the kick-ids before starting the remote device. Do you have any preference on how to solve this, Ohad? One idea could be to call rproc->start from a work-queue and wait for completion of firmware loading before actually starting. In this way we will not trigger start-up of the remote device before all virtio devices and their kick-ids are allocated. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 1/5] remoteproc: Refactor function rproc_elf_find_rsc_table
Refatcor rproc_elf_find_rsc_table and split out the scanning for the section header named resource table. This is done to prepare for loading firmware once when rproc is registered. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_elf_loader.c | 83 +--- 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index e1f89d6..69832d9 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -208,38 +208,19 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw) return ret; } -/** - * rproc_elf_find_rsc_table() - find the resource table - * @rproc: the rproc handle - * @fw: the ELF firmware image - * @tablesz: place holder for providing back the table size - * - * This function finds the resource table inside the remote processor's - * firmware. It is used both upon the registration of @rproc (in order - * to look for and register the supported virito devices), and when the - * @rproc is booted. - * - * Returns the pointer to the resource table if it is found, and write its - * size into @tablesz. If a valid table isn't found, NULL is returned - * (and @tablesz isn't set). - */ -static struct resource_table * -rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, - int *tablesz) +static struct elf32_shdr * +find_rsc_shdr(struct device *dev, struct elf32_hdr *ehdr) { - struct elf32_hdr *ehdr; struct elf32_shdr *shdr; + int i; const char *name_table; - struct device *dev = >dev; struct resource_table *table = NULL; - int i; - const u8 *elf_data = fw->data; + const u8 *elf_data = (void *)ehdr; - ehdr = (struct elf32_hdr *)elf_data; + /* look for the resource table and handle it */ shdr = (struct elf32_shdr *)(elf_data + ehdr->e_shoff); name_table = elf_data + shdr[ehdr->e_shstrndx].sh_offset; - /* look for the resource table and handle it */ for (i = 0; i < ehdr->e_shnum; i++, shdr++) { int size = shdr->sh_size; int offset = shdr->sh_offset; @@ -249,12 +230,6 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, table = (struct resource_table *)(elf_data + offset); - /* make sure we have the entire table */ - if (offset + size > fw->size) { - dev_err(dev, "resource table truncated\n"); - return NULL; - } - /* make sure table has at least the header */ if (sizeof(struct resource_table) > size) { dev_err(dev, "header-less resource table\n"); @@ -280,10 +255,54 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return NULL; } - *tablesz = shdr->sh_size; - break; + return shdr; + } + + return NULL; +} + +/** + * rproc_elf_find_rsc_table() - find the resource table + * @rproc: the rproc handle + * @fw: the ELF firmware image + * @tablesz: place holder for providing back the table size + * + * This function finds the resource table inside the remote processor's + * firmware. It is used both upon the registration of @rproc (in order + * to look for and register the supported virito devices), and when the + * @rproc is booted. + * + * Returns the pointer to the resource table if it is found, and write its + * size into @tablesz. If a valid table isn't found, NULL is returned + * (and @tablesz isn't set). + */ +static struct resource_table * +rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, + int *tablesz) +{ + struct elf32_hdr *ehdr; + struct elf32_shdr *shdr; + + struct device *dev = >dev; + struct resource_table *table = NULL; + + const u8 *elf_data = fw->data; + + ehdr = (struct elf32_hdr *)elf_data; + + shdr = find_rsc_shdr(dev, ehdr); + if (!shdr) + return NULL; + + /* make sure we have the entire table */ + if (shdr->sh_offset + shdr->sh_size > fw->size) { + dev_err(dev, "resource table truncated\n"); + return NULL; } + table = (struct resource_table *)(elf_data + shdr->sh_offset); + *tablesz = shdr->sh_size; + return table; } -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 4/5] remoteproc: Load firmware to device memory once.
Note: This patch changes device memory layout!! Virtio Rings are no longer located at start of device memory. But Ring Address in resource table is now supported. Load firmware into device memory before virtio devices are added, and use resource table located in device memory when crating virtio devices. This enables dynamically-allocated address and kick-ids of vrings to be communicated to the remote device. This also pave the way for implementing bi-directional virtio config-space and virtio feature negotiation. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c | 222 ++ 1 file changed, 91 insertions(+), 131 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index dd3bfaf..e407197 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -681,16 +681,36 @@ static rproc_handle_resource_t rproc_handle_rsc[] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, - [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ + [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; -/* handle firmware resource entries before booting the remote processor */ +static bool +is_mem_resource(int rsc_type) +{ + /* Handle memory configurations only */ + switch (rsc_type) { + case RSC_CARVEOUT: + case RSC_DEVMEM: + return true; + default: + return false; + } +} + +static inline bool +isnt_mem_resource(int rsc_type) +{ + return !is_mem_resource(rsc_type); +} + +/* handle firmware resource entries */ static int -rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len) +rproc_handle_resouce(struct rproc *rproc, struct resource_table *table, int len, + bool (*rsc_filter)(int type)) { struct device *dev = >dev; - rproc_handle_resource_t handler; int ret = 0, i; + rproc_handle_resource_t handler; for (i = 0; i < table->num; i++) { int offset = table->offset[i]; @@ -698,58 +718,27 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len int avail = len - offset - sizeof(*hdr); void *rsc = (void *)hdr + sizeof(*hdr); - /* make sure table isn't truncated */ - if (avail < 0) { - dev_err(dev, "rsc table is truncated\n"); - return -EINVAL; - } - - dev_dbg(dev, "rsc: type %d\n", hdr->type); - if (hdr->type >= RSC_LAST) { dev_warn(dev, "unsupported resource %d\n", hdr->type); continue; } - handler = rproc_handle_rsc[hdr->type]; - if (!handler) - continue; - - ret = handler(rproc, rsc, avail); - if (ret) - break; - } - - return ret; -} - -/* handle firmware resource entries while registering the remote processor */ -static int -rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int len) -{ - struct device *dev = >dev; - int ret = 0, i; - - for (i = 0; i < table->num; i++) { - int offset = table->offset[i]; - struct fw_rsc_hdr *hdr = (void *)table + offset; - int avail = len - offset - sizeof(*hdr); - struct fw_rsc_vdev *vrsc; - /* make sure table isn't truncated */ if (avail < 0) { dev_err(dev, "rsc table is truncated\n"); return -EINVAL; } + if (!rsc_filter(hdr->type)) + continue; + dev_dbg(dev, "%s: rsc type %d\n", __func__, hdr->type); - if (hdr->type != RSC_VDEV) + handler = rproc_handle_rsc[hdr->type]; + if (!handler) continue; - vrsc = (struct fw_rsc_vdev *)hdr->data; - - ret = rproc_handle_vdev(rproc, vrsc, avail); + ret = handler(rproc, rsc, avail); if (ret) break; } @@ -801,20 +790,40 @@ static void rproc_resource_cleanup(struct rproc *rproc) } /* - * take a firmware and boot a remote processor with it. + * take a firmware parse the resouce table and load it + * + * Note: this function is called asynchronously upon registration of the + * remote processor (so we must wait until it completes before we try + * to unregister the device. one
[RFC 3/5] remoteproc: Add state RPROC_LOADED
Add state RPROC_LOADED. Prepare for the state when the firmware is loaded, but remote device is not started. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_debugfs.c |1 + include/linux/remoteproc.h |3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 157a573..a026359 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -68,6 +68,7 @@ static const char * const rproc_state_string[] = { "suspended", "running", "crashed", + "loaded", "invalid", }; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index faf3332..17b8120 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -357,7 +357,8 @@ enum rproc_state { RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, - RPROC_LAST = 4, + RPROC_LOADED= 4, + RPROC_LAST = 5, }; /** -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 5/5] remoteproc: Support virtio config space.
Add support for bi-directional virtio configuration space, visibility of device status to remote processor and device feature-bit negotiation with remote device. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_core.c |4 ++-- drivers/remoteproc/remoteproc_virtio.c | 30 +++--- include/linux/remoteproc.h |3 +-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index e407197..5f99587 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -362,8 +362,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, goto free_rvdev; } - /* remember the device features */ - rvdev->dfeatures = rsc->dfeatures; + /* remember the vring resource address */ + rvdev->rsc = rsc; list_add_tail(>node, >rvdevs); diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..9ebc5d9 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -176,16 +176,21 @@ error: */ static u8 rproc_virtio_get_status(struct virtio_device *vdev) { - return 0; + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + return rvdev->rsc->status; } static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev->rsc->status = status; dev_dbg(>dev, "status: %d\n", status); } static void rproc_virtio_reset(struct virtio_device *vdev) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev->rsc->status = 0; dev_dbg(>dev, "reset !\n"); } @@ -194,7 +199,7 @@ static u32 rproc_virtio_get_features(struct virtio_device *vdev) { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); - return rvdev->dfeatures; + return rvdev->rsc->dfeatures; } static void rproc_virtio_finalize_features(struct virtio_device *vdev) @@ -213,7 +218,23 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * fixed as part of a small resource table overhaul and then an * extension of the virtio resource entries. */ - rvdev->gfeatures = vdev->features[0]; + rvdev->rsc->gfeatures = vdev->features[0]; +} + +void rproc_virtio_get(struct virtio_device *vdev, unsigned offset, + void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = >rsc->vring[rvdev->rsc->num_of_vrings]; + memcpy(buf, cfg + offset, len); +} + +void rproc_virtio_set(struct virtio_device *vdev, unsigned offset, + const void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = >rsc->vring[rvdev->rsc->num_of_vrings]; + memcpy(cfg + offset, buf, len); } static struct virtio_config_ops rproc_virtio_config_ops = { @@ -224,6 +245,9 @@ static struct virtio_config_ops rproc_virtio_config_ops = { .reset = rproc_virtio_reset, .set_status = rproc_virtio_set_status, .get_status = rproc_virtio_get_status, + .get= rproc_virtio_get, + .set= rproc_virtio_set, + }; /* diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 17b8120..39ce14e 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -471,8 +471,7 @@ struct rproc_vdev { struct rproc *rproc; struct virtio_device vdev; struct rproc_vring vring[RVDEV_NUM_VRINGS]; - unsigned long dfeatures; - unsigned long gfeatures; + struct fw_rsc_vdev *rsc; }; struct rproc *rproc_alloc(struct device *dev, const char *name, -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 2/5] remoteproc: Add operation to find resource table in memory
Add function find_rsc_table_va to firmware ops. This function returns the location of the resource table in device memory after the firmware is loaded. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_elf_loader.c | 16 ++- drivers/remoteproc/remoteproc_internal.h | 13 + drivers/remoteproc/ste_modem_rproc.c | 43 +++- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index 69832d9..3f6e315 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -306,9 +306,23 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table; } +struct resource_table *rproc_elf_get_rsctab_addr(struct rproc *rproc, +const struct firmware *fw) +{ + struct elf32_shdr *shdr; + + shdr = find_rsc_shdr(>dev, (struct elf32_hdr *)fw->data); + if (!shdr) + return NULL; + + /* Find resource table in loaded segments */ + return rproc_da_to_va(rproc, shdr->sh_addr, shdr->sh_size); +} + const struct rproc_fw_ops rproc_elf_fw_ops = { .load = rproc_elf_load_segments, .find_rsc_table = rproc_elf_find_rsc_table, .sanity_check = rproc_elf_sanity_check, - .get_boot_addr = rproc_elf_get_boot_addr + .get_boot_addr = rproc_elf_get_boot_addr, + .get_rsctab_addr = rproc_elf_get_rsctab_addr }; diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index 7bb6648..3a5cb7d 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -32,6 +32,7 @@ struct rproc; * expects to find it * @sanity_check: sanity check the fw image * @get_boot_addr: get boot address to entry point specified in firmware + * @get_rsctab_addr: get resouce table address as specified in firmware */ struct rproc_fw_ops { struct resource_table *(*find_rsc_table) (struct rproc *rproc, @@ -40,6 +41,8 @@ struct rproc_fw_ops { int (*load)(struct rproc *rproc, const struct firmware *fw); int (*sanity_check)(struct rproc *rproc, const struct firmware *fw); u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw); + struct resource_table *(*get_rsctab_addr)(struct rproc *rproc, + const struct firmware *fw); }; /* from remoteproc_core.c */ @@ -102,6 +105,16 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc, return NULL; } +static inline +struct resource_table *rproc_get_rsctab_addr(struct rproc *rproc, + const struct firmware *fw) +{ + if (rproc->fw_ops->get_rsctab_addr) + return rproc->fw_ops->get_rsctab_addr(rproc, fw); + + return NULL; +} + extern const struct rproc_fw_ops rproc_elf_fw_ops; #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c index a7743c0..59e99f1 100644 --- a/drivers/remoteproc/ste_modem_rproc.c +++ b/drivers/remoteproc/ste_modem_rproc.c @@ -64,26 +64,18 @@ static int sproc_load_segments(struct rproc *rproc, const struct firmware *fw) } /* Find the entry for resource table in the Table of Content */ -static struct ste_toc_entry *sproc_find_rsc_entry(const struct firmware *fw) +static const struct ste_toc_entry *sproc_find_rsc_entry(const void *data) { int i; - struct ste_toc *toc; - - if (!fw) - return NULL; - - toc = (void *)fw->data; + const struct ste_toc *toc; + toc = data; /* Search the table for the resource table */ for (i = 0; i < SPROC_MAX_TOC_ENTRIES && toc->table[i].start != 0x; i++) { - if (!strncmp(toc->table[i].name, SPROC_RESOURCE_NAME, -sizeof(toc->table[i].name))) { - if (toc->table[i].start > fw->size) - return NULL; +sizeof(toc->table[i].name))) return >table[i]; - } } return NULL; @@ -96,9 +88,12 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, { struct sproc *sproc = rproc->priv; struct resource_table *table; - struct ste_toc_entry *entry; + const struct ste_toc_entry *entry; - entry = sproc_find_rsc_entry(fw); + if (!fw) + return NULL; + + entry = sproc_find_rsc_entry(fw->data); if (!entry) { sproc_err(sproc, "resource table not found in fw\n"); return NULL; @@ -149,10 +144,30 @@ sproc_find_r
[RFC 0/5] remoteproc: Support bi-directional vdev config space.
From: Sjur Brændeland This patch-set adds support for shared resource table between Linux kernel and remote devices. Features: 1. dynamically-allocated address of the vrings can be communicated 2. vdev statuses can be communicated 3. virtio config space becomes bi-directional 4. virtio feature negotiation is two-way This is done by loading firmware once. First pass parsing of the resource table will allocate carveouts. Then the firmware is loaded into device memory. After this the address of the resource table in shared memory is located. This address is used in the second pass of parsing of the resource table. This time the virtio devices are created. When the virtio device are created the virto feature bits, status, and configuration-space is shared with the remote device. NOTE: This change will probably break existing device firmware! The device memory allocation will change. Virtio Rings are no longer located at start of device memory. But Ring Address in resource table is supported. Thanks, Sjur Sjur Brændeland (5): remoteproc: Refactor function rproc_elf_find_rsc_table remoteproc: Add operation to find resource table in memory remoteproc: Add state RPROC_LOADED remoteproc: Load firmware to device memory once. remoteproc: Support virtio config space. drivers/remoteproc/remoteproc_core.c | 226 drivers/remoteproc/remoteproc_debugfs.c|1 + drivers/remoteproc/remoteproc_elf_loader.c | 99 drivers/remoteproc/remoteproc_internal.h | 13 ++ drivers/remoteproc/remoteproc_virtio.c | 30 +++- drivers/remoteproc/ste_modem_rproc.c | 43 -- include/linux/remoteproc.h |6 +- 7 files changed, 232 insertions(+), 186 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 0/5] remoteproc: Support bi-directional vdev config space.
From: Sjur Brændeland s...@brendeland.net This patch-set adds support for shared resource table between Linux kernel and remote devices. Features: 1. dynamically-allocated address of the vrings can be communicated 2. vdev statuses can be communicated 3. virtio config space becomes bi-directional 4. virtio feature negotiation is two-way This is done by loading firmware once. First pass parsing of the resource table will allocate carveouts. Then the firmware is loaded into device memory. After this the address of the resource table in shared memory is located. This address is used in the second pass of parsing of the resource table. This time the virtio devices are created. When the virtio device are created the virto feature bits, status, and configuration-space is shared with the remote device. NOTE: This change will probably break existing device firmware! The device memory allocation will change. Virtio Rings are no longer located at start of device memory. But Ring Address in resource table is supported. Thanks, Sjur Sjur Brændeland (5): remoteproc: Refactor function rproc_elf_find_rsc_table remoteproc: Add operation to find resource table in memory remoteproc: Add state RPROC_LOADED remoteproc: Load firmware to device memory once. remoteproc: Support virtio config space. drivers/remoteproc/remoteproc_core.c | 226 drivers/remoteproc/remoteproc_debugfs.c|1 + drivers/remoteproc/remoteproc_elf_loader.c | 99 drivers/remoteproc/remoteproc_internal.h | 13 ++ drivers/remoteproc/remoteproc_virtio.c | 30 +++- drivers/remoteproc/ste_modem_rproc.c | 43 -- include/linux/remoteproc.h |6 +- 7 files changed, 232 insertions(+), 186 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 2/5] remoteproc: Add operation to find resource table in memory
Add function find_rsc_table_va to firmware ops. This function returns the location of the resource table in device memory after the firmware is loaded. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_elf_loader.c | 16 ++- drivers/remoteproc/remoteproc_internal.h | 13 + drivers/remoteproc/ste_modem_rproc.c | 43 +++- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index 69832d9..3f6e315 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -306,9 +306,23 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return table; } +struct resource_table *rproc_elf_get_rsctab_addr(struct rproc *rproc, +const struct firmware *fw) +{ + struct elf32_shdr *shdr; + + shdr = find_rsc_shdr(rproc-dev, (struct elf32_hdr *)fw-data); + if (!shdr) + return NULL; + + /* Find resource table in loaded segments */ + return rproc_da_to_va(rproc, shdr-sh_addr, shdr-sh_size); +} + const struct rproc_fw_ops rproc_elf_fw_ops = { .load = rproc_elf_load_segments, .find_rsc_table = rproc_elf_find_rsc_table, .sanity_check = rproc_elf_sanity_check, - .get_boot_addr = rproc_elf_get_boot_addr + .get_boot_addr = rproc_elf_get_boot_addr, + .get_rsctab_addr = rproc_elf_get_rsctab_addr }; diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h index 7bb6648..3a5cb7d 100644 --- a/drivers/remoteproc/remoteproc_internal.h +++ b/drivers/remoteproc/remoteproc_internal.h @@ -32,6 +32,7 @@ struct rproc; * expects to find it * @sanity_check: sanity check the fw image * @get_boot_addr: get boot address to entry point specified in firmware + * @get_rsctab_addr: get resouce table address as specified in firmware */ struct rproc_fw_ops { struct resource_table *(*find_rsc_table) (struct rproc *rproc, @@ -40,6 +41,8 @@ struct rproc_fw_ops { int (*load)(struct rproc *rproc, const struct firmware *fw); int (*sanity_check)(struct rproc *rproc, const struct firmware *fw); u32 (*get_boot_addr)(struct rproc *rproc, const struct firmware *fw); + struct resource_table *(*get_rsctab_addr)(struct rproc *rproc, + const struct firmware *fw); }; /* from remoteproc_core.c */ @@ -102,6 +105,16 @@ struct resource_table *rproc_find_rsc_table(struct rproc *rproc, return NULL; } +static inline +struct resource_table *rproc_get_rsctab_addr(struct rproc *rproc, + const struct firmware *fw) +{ + if (rproc-fw_ops-get_rsctab_addr) + return rproc-fw_ops-get_rsctab_addr(rproc, fw); + + return NULL; +} + extern const struct rproc_fw_ops rproc_elf_fw_ops; #endif /* REMOTEPROC_INTERNAL_H */ diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c index a7743c0..59e99f1 100644 --- a/drivers/remoteproc/ste_modem_rproc.c +++ b/drivers/remoteproc/ste_modem_rproc.c @@ -64,26 +64,18 @@ static int sproc_load_segments(struct rproc *rproc, const struct firmware *fw) } /* Find the entry for resource table in the Table of Content */ -static struct ste_toc_entry *sproc_find_rsc_entry(const struct firmware *fw) +static const struct ste_toc_entry *sproc_find_rsc_entry(const void *data) { int i; - struct ste_toc *toc; - - if (!fw) - return NULL; - - toc = (void *)fw-data; + const struct ste_toc *toc; + toc = data; /* Search the table for the resource table */ for (i = 0; i SPROC_MAX_TOC_ENTRIES toc-table[i].start != 0x; i++) { - if (!strncmp(toc-table[i].name, SPROC_RESOURCE_NAME, -sizeof(toc-table[i].name))) { - if (toc-table[i].start fw-size) - return NULL; +sizeof(toc-table[i].name))) return toc-table[i]; - } } return NULL; @@ -96,9 +88,12 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw, { struct sproc *sproc = rproc-priv; struct resource_table *table; - struct ste_toc_entry *entry; + const struct ste_toc_entry *entry; - entry = sproc_find_rsc_entry(fw); + if (!fw) + return NULL; + + entry = sproc_find_rsc_entry(fw-data); if (!entry) { sproc_err(sproc, resource table not found in fw\n); return NULL; @@ -149,10 +144,30 @@ sproc_find_rsc_table(struct rproc *rproc, const struct firmware *fw
[RFC 5/5] remoteproc: Support virtio config space.
Add support for bi-directional virtio configuration space, visibility of device status to remote processor and device feature-bit negotiation with remote device. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c |4 ++-- drivers/remoteproc/remoteproc_virtio.c | 30 +++--- include/linux/remoteproc.h |3 +-- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index e407197..5f99587 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -362,8 +362,8 @@ static int rproc_handle_vdev(struct rproc *rproc, struct fw_rsc_vdev *rsc, goto free_rvdev; } - /* remember the device features */ - rvdev-dfeatures = rsc-dfeatures; + /* remember the vring resource address */ + rvdev-rsc = rsc; list_add_tail(rvdev-node, rproc-rvdevs); diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..9ebc5d9 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -176,16 +176,21 @@ error: */ static u8 rproc_virtio_get_status(struct virtio_device *vdev) { - return 0; + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + return rvdev-rsc-status; } static void rproc_virtio_set_status(struct virtio_device *vdev, u8 status) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev-rsc-status = status; dev_dbg(vdev-dev, status: %d\n, status); } static void rproc_virtio_reset(struct virtio_device *vdev) { + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + rvdev-rsc-status = 0; dev_dbg(vdev-dev, reset !\n); } @@ -194,7 +199,7 @@ static u32 rproc_virtio_get_features(struct virtio_device *vdev) { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); - return rvdev-dfeatures; + return rvdev-rsc-dfeatures; } static void rproc_virtio_finalize_features(struct virtio_device *vdev) @@ -213,7 +218,23 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * fixed as part of a small resource table overhaul and then an * extension of the virtio resource entries. */ - rvdev-gfeatures = vdev-features[0]; + rvdev-rsc-gfeatures = vdev-features[0]; +} + +void rproc_virtio_get(struct virtio_device *vdev, unsigned offset, + void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = rvdev-rsc-vring[rvdev-rsc-num_of_vrings]; + memcpy(buf, cfg + offset, len); +} + +void rproc_virtio_set(struct virtio_device *vdev, unsigned offset, + const void *buf, unsigned len) +{ + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); + void *cfg = rvdev-rsc-vring[rvdev-rsc-num_of_vrings]; + memcpy(cfg + offset, buf, len); } static struct virtio_config_ops rproc_virtio_config_ops = { @@ -224,6 +245,9 @@ static struct virtio_config_ops rproc_virtio_config_ops = { .reset = rproc_virtio_reset, .set_status = rproc_virtio_set_status, .get_status = rproc_virtio_get_status, + .get= rproc_virtio_get, + .set= rproc_virtio_set, + }; /* diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 17b8120..39ce14e 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -471,8 +471,7 @@ struct rproc_vdev { struct rproc *rproc; struct virtio_device vdev; struct rproc_vring vring[RVDEV_NUM_VRINGS]; - unsigned long dfeatures; - unsigned long gfeatures; + struct fw_rsc_vdev *rsc; }; struct rproc *rproc_alloc(struct device *dev, const char *name, -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 3/5] remoteproc: Add state RPROC_LOADED
Add state RPROC_LOADED. Prepare for the state when the firmware is loaded, but remote device is not started. Signed-off-by: Sjur Brændeland sjur.brandel...@steicsson.com --- drivers/remoteproc/remoteproc_debugfs.c |1 + include/linux/remoteproc.h |3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 157a573..a026359 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -68,6 +68,7 @@ static const char * const rproc_state_string[] = { suspended, running, crashed, + loaded, invalid, }; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index faf3332..17b8120 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -357,7 +357,8 @@ enum rproc_state { RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, - RPROC_LAST = 4, + RPROC_LOADED= 4, + RPROC_LAST = 5, }; /** -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC 4/5] remoteproc: Load firmware to device memory once.
Note: This patch changes device memory layout!! Virtio Rings are no longer located at start of device memory. But Ring Address in resource table is now supported. Load firmware into device memory before virtio devices are added, and use resource table located in device memory when crating virtio devices. This enables dynamically-allocated address and kick-ids of vrings to be communicated to the remote device. This also pave the way for implementing bi-directional virtio config-space and virtio feature negotiation. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_core.c | 222 ++ 1 file changed, 91 insertions(+), 131 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index dd3bfaf..e407197 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -681,16 +681,36 @@ static rproc_handle_resource_t rproc_handle_rsc[] = { [RSC_CARVEOUT] = (rproc_handle_resource_t)rproc_handle_carveout, [RSC_DEVMEM] = (rproc_handle_resource_t)rproc_handle_devmem, [RSC_TRACE] = (rproc_handle_resource_t)rproc_handle_trace, - [RSC_VDEV] = NULL, /* VDEVs were handled upon registrarion */ + [RSC_VDEV] = (rproc_handle_resource_t)rproc_handle_vdev, }; -/* handle firmware resource entries before booting the remote processor */ +static bool +is_mem_resource(int rsc_type) +{ + /* Handle memory configurations only */ + switch (rsc_type) { + case RSC_CARVEOUT: + case RSC_DEVMEM: + return true; + default: + return false; + } +} + +static inline bool +isnt_mem_resource(int rsc_type) +{ + return !is_mem_resource(rsc_type); +} + +/* handle firmware resource entries */ static int -rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len) +rproc_handle_resouce(struct rproc *rproc, struct resource_table *table, int len, + bool (*rsc_filter)(int type)) { struct device *dev = rproc-dev; - rproc_handle_resource_t handler; int ret = 0, i; + rproc_handle_resource_t handler; for (i = 0; i table-num; i++) { int offset = table-offset[i]; @@ -698,58 +718,27 @@ rproc_handle_boot_rsc(struct rproc *rproc, struct resource_table *table, int len int avail = len - offset - sizeof(*hdr); void *rsc = (void *)hdr + sizeof(*hdr); - /* make sure table isn't truncated */ - if (avail 0) { - dev_err(dev, rsc table is truncated\n); - return -EINVAL; - } - - dev_dbg(dev, rsc: type %d\n, hdr-type); - if (hdr-type = RSC_LAST) { dev_warn(dev, unsupported resource %d\n, hdr-type); continue; } - handler = rproc_handle_rsc[hdr-type]; - if (!handler) - continue; - - ret = handler(rproc, rsc, avail); - if (ret) - break; - } - - return ret; -} - -/* handle firmware resource entries while registering the remote processor */ -static int -rproc_handle_virtio_rsc(struct rproc *rproc, struct resource_table *table, int len) -{ - struct device *dev = rproc-dev; - int ret = 0, i; - - for (i = 0; i table-num; i++) { - int offset = table-offset[i]; - struct fw_rsc_hdr *hdr = (void *)table + offset; - int avail = len - offset - sizeof(*hdr); - struct fw_rsc_vdev *vrsc; - /* make sure table isn't truncated */ if (avail 0) { dev_err(dev, rsc table is truncated\n); return -EINVAL; } + if (!rsc_filter(hdr-type)) + continue; + dev_dbg(dev, %s: rsc type %d\n, __func__, hdr-type); - if (hdr-type != RSC_VDEV) + handler = rproc_handle_rsc[hdr-type]; + if (!handler) continue; - vrsc = (struct fw_rsc_vdev *)hdr-data; - - ret = rproc_handle_vdev(rproc, vrsc, avail); + ret = handler(rproc, rsc, avail); if (ret) break; } @@ -801,20 +790,40 @@ static void rproc_resource_cleanup(struct rproc *rproc) } /* - * take a firmware and boot a remote processor with it. + * take a firmware parse the resouce table and load it + * + * Note: this function is called asynchronously upon registration of the + * remote processor (so we must wait until it completes before we try + * to unregister the device. one other option is just to use kref here, + * that might be cleaner). */ -static int rproc_fw_boot
[RFC 1/5] remoteproc: Refactor function rproc_elf_find_rsc_table
Refatcor rproc_elf_find_rsc_table and split out the scanning for the section header named resource table. This is done to prepare for loading firmware once when rproc is registered. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_elf_loader.c | 83 +--- 1 file changed, 51 insertions(+), 32 deletions(-) diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c index e1f89d6..69832d9 100644 --- a/drivers/remoteproc/remoteproc_elf_loader.c +++ b/drivers/remoteproc/remoteproc_elf_loader.c @@ -208,38 +208,19 @@ rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw) return ret; } -/** - * rproc_elf_find_rsc_table() - find the resource table - * @rproc: the rproc handle - * @fw: the ELF firmware image - * @tablesz: place holder for providing back the table size - * - * This function finds the resource table inside the remote processor's - * firmware. It is used both upon the registration of @rproc (in order - * to look for and register the supported virito devices), and when the - * @rproc is booted. - * - * Returns the pointer to the resource table if it is found, and write its - * size into @tablesz. If a valid table isn't found, NULL is returned - * (and @tablesz isn't set). - */ -static struct resource_table * -rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, - int *tablesz) +static struct elf32_shdr * +find_rsc_shdr(struct device *dev, struct elf32_hdr *ehdr) { - struct elf32_hdr *ehdr; struct elf32_shdr *shdr; + int i; const char *name_table; - struct device *dev = rproc-dev; struct resource_table *table = NULL; - int i; - const u8 *elf_data = fw-data; + const u8 *elf_data = (void *)ehdr; - ehdr = (struct elf32_hdr *)elf_data; + /* look for the resource table and handle it */ shdr = (struct elf32_shdr *)(elf_data + ehdr-e_shoff); name_table = elf_data + shdr[ehdr-e_shstrndx].sh_offset; - /* look for the resource table and handle it */ for (i = 0; i ehdr-e_shnum; i++, shdr++) { int size = shdr-sh_size; int offset = shdr-sh_offset; @@ -249,12 +230,6 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, table = (struct resource_table *)(elf_data + offset); - /* make sure we have the entire table */ - if (offset + size fw-size) { - dev_err(dev, resource table truncated\n); - return NULL; - } - /* make sure table has at least the header */ if (sizeof(struct resource_table) size) { dev_err(dev, header-less resource table\n); @@ -280,10 +255,54 @@ rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, return NULL; } - *tablesz = shdr-sh_size; - break; + return shdr; + } + + return NULL; +} + +/** + * rproc_elf_find_rsc_table() - find the resource table + * @rproc: the rproc handle + * @fw: the ELF firmware image + * @tablesz: place holder for providing back the table size + * + * This function finds the resource table inside the remote processor's + * firmware. It is used both upon the registration of @rproc (in order + * to look for and register the supported virito devices), and when the + * @rproc is booted. + * + * Returns the pointer to the resource table if it is found, and write its + * size into @tablesz. If a valid table isn't found, NULL is returned + * (and @tablesz isn't set). + */ +static struct resource_table * +rproc_elf_find_rsc_table(struct rproc *rproc, const struct firmware *fw, + int *tablesz) +{ + struct elf32_hdr *ehdr; + struct elf32_shdr *shdr; + + struct device *dev = rproc-dev; + struct resource_table *table = NULL; + + const u8 *elf_data = fw-data; + + ehdr = (struct elf32_hdr *)elf_data; + + shdr = find_rsc_shdr(dev, ehdr); + if (!shdr) + return NULL; + + /* make sure we have the entire table */ + if (shdr-sh_offset + shdr-sh_size fw-size) { + dev_err(dev, resource table truncated\n); + return NULL; } + table = (struct resource_table *)(elf_data + shdr-sh_offset); + *tablesz = shdr-sh_size; + return table; } -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)
Hi Ohad, On Sat, Aug 11, 2012 at 2:34 PM, Ohad Ben-Cohen o...@wizery.com wrote: On Fri, Aug 10, 2012 at 6:30 PM, Ohad Ben-Cohen o...@wizery.com wrote: This will solve all sort of open issues we have (or going to have soon): ... 1. dynamically-allocated address of the vrings can be communicated 2. vdev statuses can be communicated 3. virtio config space will finally become bi-directional as it should I just posted a RFC patch-set on this to linux-kernel@vger.kernel.org. Review comments is welcomed :-) and 5. let the remote processor know about the notifyids that we've dynamically allocated. I have run into one issue with the dynamically allocated notifyids. When allocating multiple virtio devices, only ids for the first virtio device is allocated when the remote device is started. This is fails for me, because I need to know all of the kick-ids before starting the remote device. Do you have any preference on how to solve this, Ohad? One idea could be to call rproc-start from a work-queue and wait for completion of firmware loading before actually starting. In this way we will not trigger start-up of the remote device before all virtio devices and their kick-ids are allocated. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio: Don't access index after unregister.
On Thu, Nov 8, 2012 at 11:43 AM, Cornelia Huck wrote: > Virtio wants to release used indices after the corresponding > virtio device has been unregistered. However, virtio does not > hold an extra reference, giving up its last reference with > device_unregister(), making accessing dev->index afterwards > invalid. > > I actually saw problems when testing my (not-yet-merged) > virtio-ccw code: > > - device_add virtio-net,id=xxx > -> creates device virtio with n>0 > > - device_del xxx > -> deletes virtio, but calls ida_simple_remove with an >index of 0 > > - device_add virtio-net,id=xxx > -> tries to add virtio0, which is still in use... > > So let's save the index we want to release before calling > device_unregister(). > > Signed-off-by: Cornelia Huck > --- > drivers/virtio/virtio.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > index 1e8659c..809b0de 100644 > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -225,8 +225,10 @@ EXPORT_SYMBOL_GPL(register_virtio_device); > > void unregister_virtio_device(struct virtio_device *dev) > { > + int index = dev->index; /* save for after device release */ > + > device_unregister(>dev); > - ida_simple_remove(_index_ida, dev->index); > + ida_simple_remove(_index_ida, index); > } > EXPORT_SYMBOL_GPL(unregister_virtio_device); Acked-by: Sjur Brændeland Great minds think alike! I discovered issues with this implementation a while back and Michael suggested an identical patch: https://lkml.org/lkml/2012/9/4/173 https://lkml.org/lkml/2012/9/7/105 The issue I ran into was that when virtio devices are created by remoteproc the device memory might be freed when calling device_unregister(), and the value of dev->index is then undefined. So this bug bites when unregistering a Virtio devices from remoteproc with CONFIG_DEBUG_SLAB enabled. However this bug is not triggered by virtio_pci as it implements a non-standard device release-function that does not free the device memory. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH resend] virtio_console: Free buffers from out-queue upon close
>> > Note: This patch is compile tested only. I have done the removal >> > of buffers from out-queue in handle_control_message() >> > when host has acked the close request. This seems less >> > racy than doing it in the release function. >> >> This confuses me... why are we doing this in case >> VIRTIO_CONSOLE_PORT_OPEN:? >> >> We can't pull unconsumed buffers out of the ring when the other side may >> still access it, and this seems to be doing that. > > Yes -- and it's my fault; I asked Sjur to do that in the close fops > function. Thanks Amit :-), but this was really my bad. > We should only do this in the port remove case (unplug or device > remove) -- so the original patch, with just the WARN_ON removed is the > right way. > > I'll send the revised 3/3 patch for you. Thank you. Regards, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH resend] virtio_console: Free buffers from out-queue upon close
Note: This patch is compile tested only. I have done the removal of buffers from out-queue in handle_control_message() when host has acked the close request. This seems less racy than doing it in the release function. This confuses me... why are we doing this in case VIRTIO_CONSOLE_PORT_OPEN:? We can't pull unconsumed buffers out of the ring when the other side may still access it, and this seems to be doing that. Yes -- and it's my fault; I asked Sjur to do that in the close fops function. Thanks Amit :-), but this was really my bad. We should only do this in the port remove case (unplug or device remove) -- so the original patch, with just the WARN_ON removed is the right way. I'll send the revised 3/3 patch for you. Thank you. Regards, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] virtio: Don't access index after unregister.
On Thu, Nov 8, 2012 at 11:43 AM, Cornelia Huck cornelia.h...@de.ibm.com wrote: Virtio wants to release used indices after the corresponding virtio device has been unregistered. However, virtio does not hold an extra reference, giving up its last reference with device_unregister(), making accessing dev-index afterwards invalid. I actually saw problems when testing my (not-yet-merged) virtio-ccw code: - device_add virtio-net,id=xxx - creates device virtion with n0 - device_del xxx - deletes virtion, but calls ida_simple_remove with an index of 0 - device_add virtio-net,id=xxx - tries to add virtio0, which is still in use... So let's save the index we want to release before calling device_unregister(). Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- drivers/virtio/virtio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 1e8659c..809b0de 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -225,8 +225,10 @@ EXPORT_SYMBOL_GPL(register_virtio_device); void unregister_virtio_device(struct virtio_device *dev) { + int index = dev-index; /* save for after device release */ + device_unregister(dev-dev); - ida_simple_remove(virtio_index_ida, dev-index); + ida_simple_remove(virtio_index_ida, index); } EXPORT_SYMBOL_GPL(unregister_virtio_device); Acked-by: Sjur Brændeland sjur.brandel...@stericsson.com Great minds think alike! I discovered issues with this implementation a while back and Michael suggested an identical patch: https://lkml.org/lkml/2012/9/4/173 https://lkml.org/lkml/2012/9/7/105 The issue I ran into was that when virtio devices are created by remoteproc the device memory might be freed when calling device_unregister(), and the value of dev-index is then undefined. So this bug bites when unregistering a Virtio devices from remoteproc with CONFIG_DEBUG_SLAB enabled. However this bug is not triggered by virtio_pci as it implements a non-standard device release-function that does not free the device memory. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC virtio-next 0/4] Introduce CAIF Virtio and reversed Vrings
Hi Rusty, > So, this adds another host-side virtqueue implementation. > > Can we combine them together conveniently? You pulled out more stuff > into vring.h which is a start, but it's a bit overloaded. > Perhaps we should separate the common fields into struct vring, and use > it to build: > > struct vring_guest { > struct vring vr; > u16 last_used_idx; > }; > > struct vring_host { > struct vring vr; > u16 last_avail_idx; > }; > I haven't looked closely at vhost to see what it wants, but I would > think we could share more code. I have played around with the code in vhost.c to explore your idea. The main issue I run into is that vhost.c is accessing user data while my new code does not. So I end up with some quirky code testing if the ring lives in user memory or not. Another issue is sparse warnings when accessing user memory. With your suggested changes I end up sharing about 100 lines of code. So in sum, I feel this add more complexity than what we gain by sharing. Below is an initial draft of the re-usable code. I added "is_uaccess" to struct virtio_ring in order to know if the ring lives in user memory. Let me know what you think. [snip] int virtqueue_add_used(struct vring_host *vr, unsigned int head, int len, struct vring_used_elem **used) { /* The virtqueue contains a ring of used buffers. Get a pointer to the * next entry in that used ring. */ *used = >vring.used->ring[vr->last_used_idx % vr->vring.num]; if (vr->is_uaccess) { if(unlikely(__put_user(head, &(*used)->id))) { pr_debug("Failed to write used id"); return -EFAULT; } if (unlikely(__put_user(len, &(*used)->len))) { pr_debug("Failed to write used len"); return -EFAULT; } smp_wmb(); if (__put_user(vr->last_used_idx + 1, >vring.used->idx)) { pr_debug("Failed to increment used idx"); return -EFAULT; } } else { (*used)->id = head; (*used)->len = len; smp_wmb(); vr->vring.used->idx = vr->last_used_idx + 1; } vr->last_used_idx++; return 0; } /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ unsigned virtqueue_next_desc(struct vring_desc *desc) { unsigned int next; /* If this descriptor says it doesn't chain, we're done. */ if (!(desc->flags & VRING_DESC_F_NEXT)) return -1U; /* Check they're not leading us off end of descriptors. */ next = desc->next; /* Make sure compiler knows to grab that: we don't want it changing! */ /* We will use the result as an index in an array, so most * architectures only need a compiler barrier here. */ read_barrier_depends(); return next; } static int virtqueue_next_avail_desc(struct vring_host *vr) { int head; u16 last_avail_idx; /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vr->last_avail_idx; if (vr->is_uaccess) { if (__get_user(vr->avail_idx, >vring.avail->idx)) { pr_debug("Failed to access avail idx at %p\n", >vring.avail->idx); return -EFAULT; } } else vr->avail_idx = vr->vring.avail->idx; if (unlikely((u16)(vr->avail_idx - last_avail_idx) > vr->vring.num)) { pr_debug("Guest moved used index from %u to %u", last_avail_idx, vr->avail_idx); return -EFAULT; } /* If there's nothing new since last we looked, return invalid. */ if (vr->avail_idx == last_avail_idx) return vr->vring.num; /* Only get avail ring entries after they have been exposed by guest. */ smp_rmb(); /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ if (vr->is_uaccess) { if (unlikely(__get_user(head, >vring.avail->ring[last_avail_idx % vr->vring.num]))) { pr_debug("Failed to read head: idx %d address %p\n", last_avail_idx, >vring.avail->ring[last_avail_idx % vr->vring.num]); return -EFAULT; } } else
Re: [RFC virtio-next 0/4] Introduce CAIF Virtio and reversed Vrings
Hi Rusty, So, this adds another host-side virtqueue implementation. Can we combine them together conveniently? You pulled out more stuff into vring.h which is a start, but it's a bit overloaded. Perhaps we should separate the common fields into struct vring, and use it to build: struct vring_guest { struct vring vr; u16 last_used_idx; }; struct vring_host { struct vring vr; u16 last_avail_idx; }; I haven't looked closely at vhost to see what it wants, but I would think we could share more code. I have played around with the code in vhost.c to explore your idea. The main issue I run into is that vhost.c is accessing user data while my new code does not. So I end up with some quirky code testing if the ring lives in user memory or not. Another issue is sparse warnings when accessing user memory. With your suggested changes I end up sharing about 100 lines of code. So in sum, I feel this add more complexity than what we gain by sharing. Below is an initial draft of the re-usable code. I added is_uaccess to struct virtio_ring in order to know if the ring lives in user memory. Let me know what you think. [snip] int virtqueue_add_used(struct vring_host *vr, unsigned int head, int len, struct vring_used_elem **used) { /* The virtqueue contains a ring of used buffers. Get a pointer to the * next entry in that used ring. */ *used = vr-vring.used-ring[vr-last_used_idx % vr-vring.num]; if (vr-is_uaccess) { if(unlikely(__put_user(head, (*used)-id))) { pr_debug(Failed to write used id); return -EFAULT; } if (unlikely(__put_user(len, (*used)-len))) { pr_debug(Failed to write used len); return -EFAULT; } smp_wmb(); if (__put_user(vr-last_used_idx + 1, vr-vring.used-idx)) { pr_debug(Failed to increment used idx); return -EFAULT; } } else { (*used)-id = head; (*used)-len = len; smp_wmb(); vr-vring.used-idx = vr-last_used_idx + 1; } vr-last_used_idx++; return 0; } /* Each buffer in the virtqueues is actually a chain of descriptors. This * function returns the next descriptor in the chain, * or -1U if we're at the end. */ unsigned virtqueue_next_desc(struct vring_desc *desc) { unsigned int next; /* If this descriptor says it doesn't chain, we're done. */ if (!(desc-flags VRING_DESC_F_NEXT)) return -1U; /* Check they're not leading us off end of descriptors. */ next = desc-next; /* Make sure compiler knows to grab that: we don't want it changing! */ /* We will use the result as an index in an array, so most * architectures only need a compiler barrier here. */ read_barrier_depends(); return next; } static int virtqueue_next_avail_desc(struct vring_host *vr) { int head; u16 last_avail_idx; /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vr-last_avail_idx; if (vr-is_uaccess) { if (__get_user(vr-avail_idx, vr-vring.avail-idx)) { pr_debug(Failed to access avail idx at %p\n, vr-vring.avail-idx); return -EFAULT; } } else vr-avail_idx = vr-vring.avail-idx; if (unlikely((u16)(vr-avail_idx - last_avail_idx) vr-vring.num)) { pr_debug(Guest moved used index from %u to %u, last_avail_idx, vr-avail_idx); return -EFAULT; } /* If there's nothing new since last we looked, return invalid. */ if (vr-avail_idx == last_avail_idx) return vr-vring.num; /* Only get avail ring entries after they have been exposed by guest. */ smp_rmb(); /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ if (vr-is_uaccess) { if (unlikely(__get_user(head, vr-vring.avail-ring[last_avail_idx % vr-vring.num]))) { pr_debug(Failed to read head: idx %d address %p\n, last_avail_idx, vr-vring.avail-ring[last_avail_idx % vr-vring.num]); return -EFAULT; } } else head = vr-vring.avail-ring[last_avail_idx %
[RFC virtio-next 1/4] virtio: Move definitions to header file vring.h
From: Sjur Brændeland Move the vring_virtqueue structure, memory barrier and debug macros out from virtio_ring.c to the new header file vring.h. This is done in order to allow other kernel modules to access the virtio internal data-structures. Signed-off-by: Sjur Brændeland --- Tis patch triggers a couple of checkpatch warnings, but I've chosen to do a clean copy and not do any corrections. drivers/virtio/virtio_ring.c | 96 + drivers/virtio/vring.h | 121 ++ 2 files changed, 122 insertions(+), 95 deletions(-) create mode 100644 drivers/virtio/vring.h diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index ffd7e7d..9027af6 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -23,101 +23,7 @@ #include #include #include - -/* virtio guest is communicating with a virtual "device" that actually runs on - * a host processor. Memory barriers are used to control SMP effects. */ -#ifdef CONFIG_SMP -/* Where possible, use SMP barriers which are more lightweight than mandatory - * barriers, because mandatory barriers control MMIO effects on accesses - * through relaxed memory I/O windows (which virtio-pci does not use). */ -#define virtio_mb(vq) \ - do { if ((vq)->weak_barriers) smp_mb(); else mb(); } while(0) -#define virtio_rmb(vq) \ - do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0) -#define virtio_wmb(vq) \ - do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0) -#else -/* We must force memory ordering even if guest is UP since host could be - * running on another CPU, but SMP barriers are defined to barrier() in that - * configuration. So fall back to mandatory barriers instead. */ -#define virtio_mb(vq) mb() -#define virtio_rmb(vq) rmb() -#define virtio_wmb(vq) wmb() -#endif - -#ifdef DEBUG -/* For development, we want to crash whenever the ring is screwed. */ -#define BAD_RING(_vq, fmt, args...)\ - do {\ - dev_err(&(_vq)->vq.vdev->dev, \ - "%s:"fmt, (_vq)->vq.name, ##args); \ - BUG(); \ - } while (0) -/* Caller is supposed to guarantee no reentry. */ -#define START_USE(_vq) \ - do {\ - if ((_vq)->in_use) \ - panic("%s:in_use = %i\n", \ - (_vq)->vq.name, (_vq)->in_use); \ - (_vq)->in_use = __LINE__; \ - } while (0) -#define END_USE(_vq) \ - do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; } while(0) -#else -#define BAD_RING(_vq, fmt, args...)\ - do {\ - dev_err(&_vq->vq.vdev->dev, \ - "%s:"fmt, (_vq)->vq.name, ##args); \ - (_vq)->broken = true; \ - } while (0) -#define START_USE(vq) -#define END_USE(vq) -#endif - -struct vring_virtqueue -{ - struct virtqueue vq; - - /* Actual memory layout for this queue */ - struct vring vring; - - /* Can we use weak barriers? */ - bool weak_barriers; - - /* Other side has made a mess, don't try any more. */ - bool broken; - - /* Host supports indirect buffers */ - bool indirect; - - /* Host publishes avail event idx */ - bool event; - - /* Head of free buffer list. */ - unsigned int free_head; - /* Number we've added since last sync. */ - unsigned int num_added; - - /* Last used index we've seen. */ - u16 last_used_idx; - - /* How to notify other side. FIXME: commonalize hcalls! */ - void (*notify)(struct virtqueue *vq); - -#ifdef DEBUG - /* They're supposed to lock for us. */ - unsigned int in_use; - - /* Figure out if their kicks are too delayed. */ - bool last_add_time_valid; - ktime_t last_add_time; -#endif - - /* Tokens for callbacks. */ - void *data[]; -}; - -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) +#include "vring.h" /* Set up an indirect table of descriptors and add it to the queue. */ static int vring_add_indirect(struct vring_virtqueue *vq, diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h new file mode 100644 index 000..b997fc3 --- /dev/null +++ b/drivers/virtio/vring.h @@ -0,0 +1,121 @@ +/* Virtio ring implementation. + * + * Copyright 2007 Rusty Russell IBM Corporation + * + * This program is free software; you can redistribut
[RFC virtio-next 2/4] include/vring.h: Add support for reversed vritio rings.
From: Sjur Brændeland Add last avilable index to the vring_virtqueue structure, this is done to prepare for implementation of the reversed vring. Signed-off-by: Sjur Brændeland --- drivers/virtio/vring.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h index b997fc3..3b53961 100644 --- a/drivers/virtio/vring.h +++ b/drivers/virtio/vring.h @@ -51,6 +51,9 @@ struct vring_virtqueue /* Last used index we've seen. */ u16 last_used_idx; + /* Last avail index seen. NOTE: Only used for reversed rings.*/ + u16 last_avail_idx; + /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 4/4] caif_virtio: Add CAIF over virtio
From: Sjur Brændeland Add the CAIF Virtio Link layer, used for communicating with a modem over shared memory. Virtio is used as the transport mechanism. In the TX direction the virtio rings are used in the normal fashion, sending data in the available ring. But in the rx direction the the we have flipped the direction of the virtio ring, and implemented the virtio access-function similar to what is found in vhost.c. CAIF also uses the virtio configuration space for getting configuration parameters such as headroom, tailroom etc. Signed-off-by: Vikram ARV Signed-off-by: Sjur Brændeland --- drivers/net/caif/Kconfig|9 + drivers/net/caif/Makefile |3 + drivers/net/caif/caif_virtio.c | 627 +++ include/uapi/linux/virtio_ids.h |1 + 4 files changed, 640 insertions(+) create mode 100644 drivers/net/caif/caif_virtio.c diff --git a/drivers/net/caif/Kconfig b/drivers/net/caif/Kconfig index abf4d7a..a01f617 100644 --- a/drivers/net/caif/Kconfig +++ b/drivers/net/caif/Kconfig @@ -47,3 +47,12 @@ config CAIF_HSI The caif low level driver for CAIF over HSI. Be aware that if you enable this then you also need to enable a low-level HSI driver. + +config CAIF_VIRTIO + tristate "CAIF virtio transport driver" + default n + depends on CAIF + depends on REMOTEPROC + select VIRTIO + ---help--- + The caif driver for CAIF over Virtio. diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile index 91dff86..d9ee26a 100644 --- a/drivers/net/caif/Makefile +++ b/drivers/net/caif/Makefile @@ -13,3 +13,6 @@ obj-$(CONFIG_CAIF_SHM) += caif_shm.o # HSI interface obj-$(CONFIG_CAIF_HSI) += caif_hsi.o + +# Virtio interface +obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c new file mode 100644 index 000..e50940f --- /dev/null +++ b/drivers/net/caif/caif_virtio.c @@ -0,0 +1,627 @@ +/* + * Copyright (C) ST-Ericsson AB 2012 + * Contact: Sjur Brendeland / sjur.brandel...@stericsson.com + * Authors: Vicram Arv / vikram@stericsson.com, + * Dmitry Tarnyagin / dmitry.tarnya...@stericsson.com + * Sjur Brendeland / sjur.brandel...@stericsson.com + * License terms: GNU General Public License (GPL) version 2 + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ":" fmt +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "../drivers/virtio/vring.h" + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Vicram Arv "); +MODULE_DESCRIPTION("Virtio CAIF Driver"); + +/* + * struct cfv_info - Caif Virtio control structure + * @cfdev: caif common header + * @vdev: Associated virtio device + * @vq_rx: rx/downlink virtqueue + * @vq_tx: tx/uplink virtqueue + * @ndev: associated netdevice + * @queued_tx: number of buffers queued in the tx virtqueue + * @watermark_tx: indicates number of buffers the tx queue + * should shrink to to unblock datapath + * @tx_lock: protects vq_tx to allow concurrent senders + * @tx_hr: transmit headroom + * @rx_hr: receive headroom + * @tx_tr: transmit tailroom + * @rx_tr: receive tailroom + * @mtu: transmit max size + * @mru: receive max size + */ +struct cfv_info { + struct caif_dev_common cfdev; + struct virtio_device *vdev; + struct virtqueue *vq_rx; + struct virtqueue *vq_tx; + struct net_device *ndev; + unsigned int queued_tx; + unsigned int watermark_tx; + /* Protect access to vq_tx */ + spinlock_t tx_lock; + /* Copied from Virtio config space */ + u16 tx_hr; + u16 rx_hr; + u16 tx_tr; + u16 rx_tr; + u32 mtu; + u32 mru; +}; + +/* + * struct token_info - maintains Transmit buffer data handle + * @size: size of transmit buffer + * @dma_handle: handle to allocated dma device memory area + * @vaddr: virtual address mapping to allocated memory area + */ +struct token_info { + size_t size; + u8 *vaddr; + dma_addr_t dma_handle; +}; + +/* Default if virtio config space is unavailable */ +#define CFV_DEF_MTU_SIZE 4096 +#define CFV_DEF_HEADROOM 16 +#define CFV_DEF_TAILROOM 16 + +/* Require IP header to be 4-byte aligned. */ +#define IP_HDR_ALIGN 4 + +/* + * virtqueue_next_avail_desc - get the next available descriptor + * @_vq: the struct virtqueue we're talking about + * @head: index of the descriptor in the ring + * + * Look for the next available descriptor in the available ring. + * Return NULL if nothing new in the available. + */ +static struct vring_desc *virtqueue_next_avail_desc(struct virtqueue *_vq, + int *head) +{ + struct vring_virtqueue *vq = to_vvq(_vq); + u16 avail_idx, hd, last_avail_idx = vq->last_avail_idx; + + START_US
[RFC virtio-next 3/4] virtio_ring: Call callback function even when used ring is empty
From: Sjur Brændeland Enable option to force call of callback function even if used ring is empty. This is needed for reversed vring. Add a helper function __vring_interrupt and add extra boolean argument for forcing callback when interrupt is called. The original vring_interrupt semantic and signature is perserved. Signed-off-by: Sjur Brændeland --- drivers/remoteproc/remoteproc_virtio.c |2 +- drivers/virtio/virtio_ring.c |6 +++--- include/linux/virtio_ring.h|8 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..ddde863 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -63,7 +63,7 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid) if (!rvring || !rvring->vq) return IRQ_NONE; - return vring_interrupt(0, rvring->vq); + return __vring_interrupt(0, rvring->vq, true); } EXPORT_SYMBOL(rproc_vq_interrupt); diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 9027af6..af85034 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -504,11 +504,11 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq) } EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf); -irqreturn_t vring_interrupt(int irq, void *_vq) +irqreturn_t __vring_interrupt(int irq, void *_vq, bool force) { struct vring_virtqueue *vq = to_vvq(_vq); - if (!more_used(vq)) { + if (!force && !more_used(vq)) { pr_debug("virtqueue interrupt with no work for %p\n", vq); return IRQ_NONE; } @@ -522,7 +522,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq) return IRQ_HANDLED; } -EXPORT_SYMBOL_GPL(vring_interrupt); +EXPORT_SYMBOL_GPL(__vring_interrupt); struct virtqueue *vring_new_virtqueue(unsigned int index, unsigned int num, diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 63c6ea1..ccb7915 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -20,5 +20,11 @@ void vring_del_virtqueue(struct virtqueue *vq); /* Filter out transport-specific feature bits. */ void vring_transport_features(struct virtio_device *vdev); -irqreturn_t vring_interrupt(int irq, void *_vq); +irqreturn_t __vring_interrupt(int irq, void *_vq, bool force); + +static inline irqreturn_t vring_interrupt(int irq, void *_vq) +{ + return __vring_interrupt(irq, _vq, false); +} + #endif /* _LINUX_VIRTIO_RING_H */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 0/4] Introduce CAIF Virtio and reversed Vrings
This patch-set introduces the CAIF Virtio Link layer. The purpose is to communicate with a remote processor (a modem) over shared memory. Virtio is used as the transport mechanism, and the Remoteproc framework provides configuration and management of the Virtio rings and devices. The modem and Linux host may be on the same SoC, or connected over a shared memory interface such as LLI. Zero-Copy data transport on the modem is primary goal for CAIF Virtio. In order to achieve Zero-Copy the direction of the Virtio rings are flipped in the RX direction. So we have implemented the Virtio access-function similar to what is found in vhost.c. The connected LTE-modem is a multi-processor system with an advanced memory allocator on board. In order to provide zero-copy from the modem to the connected Linux host, the direction of the Virtio rings are reversed. This allows the modem to allocate data-buffers in RX direction and pass them to the Linux host, and recycled buffers to be sent back to the modem. The option of providing pre-allocated buffers in RX direction has been considered, but rejected. The allocation of data buffers happens deep down in the network signaling stack on the LTE modem before it is known what type of data is received. It may be data that is handled within the modem and never sent to the Linux host, or IP traffic going to the host. Pre-allocated Virtio buffers does not fit for this usage. Another issue is that the network traffic pattern may vary, resulting in variation of number and size of buffers allocated from the memory allocator. Dynamic allocation is needed in order to utilize memory properly. Due to this, we decided we had to implement "reversed" vrings. Reversed vrings allows us to minimize the impact on the current memory allocator and buffer handling on the modem. In order to implement reversed rings we have added functions for reading descriptors from the available-ring and adding descriptors to the used-ring. The internal data-structures in virtio_ring.c are moved into a new header file so the data-structures can be accessed by caif_virtio. The data buffers in TX direction are allocated using dma_alloc_coherent(). This allows memory to be allocated from the memory region shared between the Host and modem. In TX direction single linearized TX buffers are added to the vring. In RX direction linearized frames are also used, but multiple descriptors may be linked. This is done to allow maximum efficiency for the LTE modem. This patch set is not yet fully tested and does not handle all negative scenarios correctly. So at this stage we're primarily looking for review comments related to the structure of the Virtio code. There are several options on how to structure this, and feedback is welcomed. Thanks, Sjur Sjur Brændeland (4): virtio: Move definitions to header file vring.h include/vring.h: Add support for reversed vritio rings. virtio_ring: Call callback function even when used ring is empty caif_virtio: Add CAIF over virtio drivers/net/caif/Kconfig |9 + drivers/net/caif/Makefile |3 + drivers/net/caif/caif_virtio.c | 627 drivers/remoteproc/remoteproc_virtio.c |2 +- drivers/virtio/virtio_ring.c | 102 +- drivers/virtio/vring.h | 124 +++ include/linux/virtio_ring.h|8 +- include/uapi/linux/virtio_ids.h|1 + 8 files changed, 776 insertions(+), 100 deletions(-) create mode 100644 drivers/net/caif/caif_virtio.c create mode 100644 drivers/virtio/vring.h -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 0/4] Introduce CAIF Virtio and reversed Vrings
This patch-set introduces the CAIF Virtio Link layer. The purpose is to communicate with a remote processor (a modem) over shared memory. Virtio is used as the transport mechanism, and the Remoteproc framework provides configuration and management of the Virtio rings and devices. The modem and Linux host may be on the same SoC, or connected over a shared memory interface such as LLI. Zero-Copy data transport on the modem is primary goal for CAIF Virtio. In order to achieve Zero-Copy the direction of the Virtio rings are flipped in the RX direction. So we have implemented the Virtio access-function similar to what is found in vhost.c. The connected LTE-modem is a multi-processor system with an advanced memory allocator on board. In order to provide zero-copy from the modem to the connected Linux host, the direction of the Virtio rings are reversed. This allows the modem to allocate data-buffers in RX direction and pass them to the Linux host, and recycled buffers to be sent back to the modem. The option of providing pre-allocated buffers in RX direction has been considered, but rejected. The allocation of data buffers happens deep down in the network signaling stack on the LTE modem before it is known what type of data is received. It may be data that is handled within the modem and never sent to the Linux host, or IP traffic going to the host. Pre-allocated Virtio buffers does not fit for this usage. Another issue is that the network traffic pattern may vary, resulting in variation of number and size of buffers allocated from the memory allocator. Dynamic allocation is needed in order to utilize memory properly. Due to this, we decided we had to implement reversed vrings. Reversed vrings allows us to minimize the impact on the current memory allocator and buffer handling on the modem. In order to implement reversed rings we have added functions for reading descriptors from the available-ring and adding descriptors to the used-ring. The internal data-structures in virtio_ring.c are moved into a new header file so the data-structures can be accessed by caif_virtio. The data buffers in TX direction are allocated using dma_alloc_coherent(). This allows memory to be allocated from the memory region shared between the Host and modem. In TX direction single linearized TX buffers are added to the vring. In RX direction linearized frames are also used, but multiple descriptors may be linked. This is done to allow maximum efficiency for the LTE modem. This patch set is not yet fully tested and does not handle all negative scenarios correctly. So at this stage we're primarily looking for review comments related to the structure of the Virtio code. There are several options on how to structure this, and feedback is welcomed. Thanks, Sjur Sjur Brændeland (4): virtio: Move definitions to header file vring.h include/vring.h: Add support for reversed vritio rings. virtio_ring: Call callback function even when used ring is empty caif_virtio: Add CAIF over virtio drivers/net/caif/Kconfig |9 + drivers/net/caif/Makefile |3 + drivers/net/caif/caif_virtio.c | 627 drivers/remoteproc/remoteproc_virtio.c |2 +- drivers/virtio/virtio_ring.c | 102 +- drivers/virtio/vring.h | 124 +++ include/linux/virtio_ring.h|8 +- include/uapi/linux/virtio_ids.h|1 + 8 files changed, 776 insertions(+), 100 deletions(-) create mode 100644 drivers/net/caif/caif_virtio.c create mode 100644 drivers/virtio/vring.h -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 3/4] virtio_ring: Call callback function even when used ring is empty
From: Sjur Brændeland sjur.brandel...@stericsson.com Enable option to force call of callback function even if used ring is empty. This is needed for reversed vring. Add a helper function __vring_interrupt and add extra boolean argument for forcing callback when interrupt is called. The original vring_interrupt semantic and signature is perserved. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/remoteproc/remoteproc_virtio.c |2 +- drivers/virtio/virtio_ring.c |6 +++--- include/linux/virtio_ring.h|8 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index e7a4780..ddde863 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -63,7 +63,7 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid) if (!rvring || !rvring-vq) return IRQ_NONE; - return vring_interrupt(0, rvring-vq); + return __vring_interrupt(0, rvring-vq, true); } EXPORT_SYMBOL(rproc_vq_interrupt); diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 9027af6..af85034 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -504,11 +504,11 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq) } EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf); -irqreturn_t vring_interrupt(int irq, void *_vq) +irqreturn_t __vring_interrupt(int irq, void *_vq, bool force) { struct vring_virtqueue *vq = to_vvq(_vq); - if (!more_used(vq)) { + if (!force !more_used(vq)) { pr_debug(virtqueue interrupt with no work for %p\n, vq); return IRQ_NONE; } @@ -522,7 +522,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq) return IRQ_HANDLED; } -EXPORT_SYMBOL_GPL(vring_interrupt); +EXPORT_SYMBOL_GPL(__vring_interrupt); struct virtqueue *vring_new_virtqueue(unsigned int index, unsigned int num, diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h index 63c6ea1..ccb7915 100644 --- a/include/linux/virtio_ring.h +++ b/include/linux/virtio_ring.h @@ -20,5 +20,11 @@ void vring_del_virtqueue(struct virtqueue *vq); /* Filter out transport-specific feature bits. */ void vring_transport_features(struct virtio_device *vdev); -irqreturn_t vring_interrupt(int irq, void *_vq); +irqreturn_t __vring_interrupt(int irq, void *_vq, bool force); + +static inline irqreturn_t vring_interrupt(int irq, void *_vq) +{ + return __vring_interrupt(irq, _vq, false); +} + #endif /* _LINUX_VIRTIO_RING_H */ -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 4/4] caif_virtio: Add CAIF over virtio
From: Sjur Brændeland sjur.brandel...@stericsson.com Add the CAIF Virtio Link layer, used for communicating with a modem over shared memory. Virtio is used as the transport mechanism. In the TX direction the virtio rings are used in the normal fashion, sending data in the available ring. But in the rx direction the the we have flipped the direction of the virtio ring, and implemented the virtio access-function similar to what is found in vhost.c. CAIF also uses the virtio configuration space for getting configuration parameters such as headroom, tailroom etc. Signed-off-by: Vikram ARV vikram@stericsson.com Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/net/caif/Kconfig|9 + drivers/net/caif/Makefile |3 + drivers/net/caif/caif_virtio.c | 627 +++ include/uapi/linux/virtio_ids.h |1 + 4 files changed, 640 insertions(+) create mode 100644 drivers/net/caif/caif_virtio.c diff --git a/drivers/net/caif/Kconfig b/drivers/net/caif/Kconfig index abf4d7a..a01f617 100644 --- a/drivers/net/caif/Kconfig +++ b/drivers/net/caif/Kconfig @@ -47,3 +47,12 @@ config CAIF_HSI The caif low level driver for CAIF over HSI. Be aware that if you enable this then you also need to enable a low-level HSI driver. + +config CAIF_VIRTIO + tristate CAIF virtio transport driver + default n + depends on CAIF + depends on REMOTEPROC + select VIRTIO + ---help--- + The caif driver for CAIF over Virtio. diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile index 91dff86..d9ee26a 100644 --- a/drivers/net/caif/Makefile +++ b/drivers/net/caif/Makefile @@ -13,3 +13,6 @@ obj-$(CONFIG_CAIF_SHM) += caif_shm.o # HSI interface obj-$(CONFIG_CAIF_HSI) += caif_hsi.o + +# Virtio interface +obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c new file mode 100644 index 000..e50940f --- /dev/null +++ b/drivers/net/caif/caif_virtio.c @@ -0,0 +1,627 @@ +/* + * Copyright (C) ST-Ericsson AB 2012 + * Contact: Sjur Brendeland / sjur.brandel...@stericsson.com + * Authors: Vicram Arv / vikram@stericsson.com, + * Dmitry Tarnyagin / dmitry.tarnya...@stericsson.com + * Sjur Brendeland / sjur.brandel...@stericsson.com + * License terms: GNU General Public License (GPL) version 2 + */ + +#define pr_fmt(fmt) KBUILD_MODNAME : fmt +#include linux/module.h +#include linux/virtio.h +#include linux/virtio_ids.h +#include linux/virtio_config.h +#include linux/dma-mapping.h +#include linux/netdevice.h +#include linux/if_arp.h +#include linux/spinlock.h +#include net/caif/caif_dev.h +#include linux/virtio_caif.h +#include ../drivers/virtio/vring.h + +MODULE_LICENSE(GPL v2); +MODULE_AUTHOR(Vicram Arv vikram@stericsson.com); +MODULE_DESCRIPTION(Virtio CAIF Driver); + +/* + * struct cfv_info - Caif Virtio control structure + * @cfdev: caif common header + * @vdev: Associated virtio device + * @vq_rx: rx/downlink virtqueue + * @vq_tx: tx/uplink virtqueue + * @ndev: associated netdevice + * @queued_tx: number of buffers queued in the tx virtqueue + * @watermark_tx: indicates number of buffers the tx queue + * should shrink to to unblock datapath + * @tx_lock: protects vq_tx to allow concurrent senders + * @tx_hr: transmit headroom + * @rx_hr: receive headroom + * @tx_tr: transmit tailroom + * @rx_tr: receive tailroom + * @mtu: transmit max size + * @mru: receive max size + */ +struct cfv_info { + struct caif_dev_common cfdev; + struct virtio_device *vdev; + struct virtqueue *vq_rx; + struct virtqueue *vq_tx; + struct net_device *ndev; + unsigned int queued_tx; + unsigned int watermark_tx; + /* Protect access to vq_tx */ + spinlock_t tx_lock; + /* Copied from Virtio config space */ + u16 tx_hr; + u16 rx_hr; + u16 tx_tr; + u16 rx_tr; + u32 mtu; + u32 mru; +}; + +/* + * struct token_info - maintains Transmit buffer data handle + * @size: size of transmit buffer + * @dma_handle: handle to allocated dma device memory area + * @vaddr: virtual address mapping to allocated memory area + */ +struct token_info { + size_t size; + u8 *vaddr; + dma_addr_t dma_handle; +}; + +/* Default if virtio config space is unavailable */ +#define CFV_DEF_MTU_SIZE 4096 +#define CFV_DEF_HEADROOM 16 +#define CFV_DEF_TAILROOM 16 + +/* Require IP header to be 4-byte aligned. */ +#define IP_HDR_ALIGN 4 + +/* + * virtqueue_next_avail_desc - get the next available descriptor + * @_vq: the struct virtqueue we're talking about + * @head: index of the descriptor in the ring + * + * Look for the next available descriptor in the available ring. + * Return NULL if nothing new in the available. + */ +static struct vring_desc *virtqueue_next_avail_desc(struct
[RFC virtio-next 2/4] include/vring.h: Add support for reversed vritio rings.
From: Sjur Brændeland sjur.brandel...@stericsson.com Add last avilable index to the vring_virtqueue structure, this is done to prepare for implementation of the reversed vring. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/virtio/vring.h |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h index b997fc3..3b53961 100644 --- a/drivers/virtio/vring.h +++ b/drivers/virtio/vring.h @@ -51,6 +51,9 @@ struct vring_virtqueue /* Last used index we've seen. */ u16 last_used_idx; + /* Last avail index seen. NOTE: Only used for reversed rings.*/ + u16 last_avail_idx; + /* How to notify other side. FIXME: commonalize hcalls! */ void (*notify)(struct virtqueue *vq); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC virtio-next 1/4] virtio: Move definitions to header file vring.h
From: Sjur Brændeland sjur.brandel...@stericsson.com Move the vring_virtqueue structure, memory barrier and debug macros out from virtio_ring.c to the new header file vring.h. This is done in order to allow other kernel modules to access the virtio internal data-structures. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- Tis patch triggers a couple of checkpatch warnings, but I've chosen to do a clean copy and not do any corrections. drivers/virtio/virtio_ring.c | 96 + drivers/virtio/vring.h | 121 ++ 2 files changed, 122 insertions(+), 95 deletions(-) create mode 100644 drivers/virtio/vring.h diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index ffd7e7d..9027af6 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -23,101 +23,7 @@ #include linux/slab.h #include linux/module.h #include linux/hrtimer.h - -/* virtio guest is communicating with a virtual device that actually runs on - * a host processor. Memory barriers are used to control SMP effects. */ -#ifdef CONFIG_SMP -/* Where possible, use SMP barriers which are more lightweight than mandatory - * barriers, because mandatory barriers control MMIO effects on accesses - * through relaxed memory I/O windows (which virtio-pci does not use). */ -#define virtio_mb(vq) \ - do { if ((vq)-weak_barriers) smp_mb(); else mb(); } while(0) -#define virtio_rmb(vq) \ - do { if ((vq)-weak_barriers) smp_rmb(); else rmb(); } while(0) -#define virtio_wmb(vq) \ - do { if ((vq)-weak_barriers) smp_wmb(); else wmb(); } while(0) -#else -/* We must force memory ordering even if guest is UP since host could be - * running on another CPU, but SMP barriers are defined to barrier() in that - * configuration. So fall back to mandatory barriers instead. */ -#define virtio_mb(vq) mb() -#define virtio_rmb(vq) rmb() -#define virtio_wmb(vq) wmb() -#endif - -#ifdef DEBUG -/* For development, we want to crash whenever the ring is screwed. */ -#define BAD_RING(_vq, fmt, args...)\ - do {\ - dev_err((_vq)-vq.vdev-dev, \ - %s:fmt, (_vq)-vq.name, ##args); \ - BUG(); \ - } while (0) -/* Caller is supposed to guarantee no reentry. */ -#define START_USE(_vq) \ - do {\ - if ((_vq)-in_use) \ - panic(%s:in_use = %i\n, \ - (_vq)-vq.name, (_vq)-in_use); \ - (_vq)-in_use = __LINE__; \ - } while (0) -#define END_USE(_vq) \ - do { BUG_ON(!(_vq)-in_use); (_vq)-in_use = 0; } while(0) -#else -#define BAD_RING(_vq, fmt, args...)\ - do {\ - dev_err(_vq-vq.vdev-dev, \ - %s:fmt, (_vq)-vq.name, ##args); \ - (_vq)-broken = true; \ - } while (0) -#define START_USE(vq) -#define END_USE(vq) -#endif - -struct vring_virtqueue -{ - struct virtqueue vq; - - /* Actual memory layout for this queue */ - struct vring vring; - - /* Can we use weak barriers? */ - bool weak_barriers; - - /* Other side has made a mess, don't try any more. */ - bool broken; - - /* Host supports indirect buffers */ - bool indirect; - - /* Host publishes avail event idx */ - bool event; - - /* Head of free buffer list. */ - unsigned int free_head; - /* Number we've added since last sync. */ - unsigned int num_added; - - /* Last used index we've seen. */ - u16 last_used_idx; - - /* How to notify other side. FIXME: commonalize hcalls! */ - void (*notify)(struct virtqueue *vq); - -#ifdef DEBUG - /* They're supposed to lock for us. */ - unsigned int in_use; - - /* Figure out if their kicks are too delayed. */ - bool last_add_time_valid; - ktime_t last_add_time; -#endif - - /* Tokens for callbacks. */ - void *data[]; -}; - -#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) +#include vring.h /* Set up an indirect table of descriptors and add it to the queue. */ static int vring_add_indirect(struct vring_virtqueue *vq, diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h new file mode 100644 index 000..b997fc3 --- /dev/null +++ b/drivers/virtio/vring.h @@ -0,0 +1,121 @@ +/* Virtio ring implementation. + * + * Copyright 2007 Rusty Russell IBM Corporation + * + * This program is free software; you can redistribute it and/or modify + * it under
[PATCH v8 1/3] virtio_console: Merge struct buffer_token into struct port_buffer
From: Sjur Brændeland Refactoring the splice functionality by unifying the approach for sending scatter-lists and regular buffers. This simplifies buffer handling and reduces code size. Splice will now allocate a port_buffer and send_buf() and free_buf() can always be used for any buffer. Signed-off-by: Sjur Brændeland Acked-by: Amit Shah --- drivers/char/virtio_console.c | 129 + 1 file changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 4ad8aca..b84390f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -111,6 +111,12 @@ struct port_buffer { size_t len; /* offset in the buf from which to consume data */ size_t offset; + + /* If sgpages == 0 then buf is used */ + unsigned int sgpages; + + /* sg is used if spages > 0. sg must be the last in is struct */ + struct scatterlist sg[0]; }; /* @@ -338,17 +344,39 @@ static inline bool use_multiport(struct ports_device *portdev) static void free_buf(struct port_buffer *buf) { + unsigned int i; + kfree(buf->buf); + for (i = 0; i < buf->sgpages; i++) { + struct page *page = sg_page(>sg[i]); + if (!page) + break; + put_page(page); + } + kfree(buf); } -static struct port_buffer *alloc_buf(size_t buf_size) +static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, +int pages) { struct port_buffer *buf; - buf = kmalloc(sizeof(*buf), GFP_KERNEL); + /* +* Allocate buffer and the sg list. The sg list array is allocated +* directly after the port_buffer struct. +*/ + buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages, + GFP_KERNEL); if (!buf) goto fail; + + buf->sgpages = pages; + if (pages > 0) { + buf->buf = NULL; + return buf; + } + buf->buf = kmalloc(buf_size, GFP_KERNEL); if (!buf->buf) goto free_buf; @@ -476,52 +504,26 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, return 0; } -struct buffer_token { - union { - void *buf; - struct scatterlist *sg; - } u; - /* If sgpages == 0 then buf is used, else sg is used */ - unsigned int sgpages; -}; - -static void reclaim_sg_pages(struct scatterlist *sg, unsigned int nrpages) -{ - int i; - struct page *page; - - for (i = 0; i < nrpages; i++) { - page = sg_page([i]); - if (!page) - break; - put_page(page); - } - kfree(sg); -} /* Callers must take the port->outvq_lock */ static void reclaim_consumed_buffers(struct port *port) { - struct buffer_token *tok; + struct port_buffer *buf; unsigned int len; if (!port->portdev) { /* Device has been unplugged. vqs are already gone. */ return; } - while ((tok = virtqueue_get_buf(port->out_vq, ))) { - if (tok->sgpages) - reclaim_sg_pages(tok->u.sg, tok->sgpages); - else - kfree(tok->u.buf); - kfree(tok); + while ((buf = virtqueue_get_buf(port->out_vq, ))) { + free_buf(buf); port->outvq_full = false; } } static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, int nents, size_t in_count, - struct buffer_token *tok, bool nonblock) + void *data, bool nonblock) { struct virtqueue *out_vq; int err; @@ -534,7 +536,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, reclaim_consumed_buffers(port); - err = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC); + err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC); /* Tell Host to go! */ virtqueue_kick(out_vq); @@ -572,37 +574,6 @@ done: return in_count; } -static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, - bool nonblock) -{ - struct scatterlist sg[1]; - struct buffer_token *tok; - - tok = kmalloc(sizeof(*tok), GFP_ATOMIC); - if (!tok) - return -ENOMEM; - tok->sgpages = 0; - tok->u.buf = in_buf; - - sg_init_one(sg, in_buf, in_count); - - return __send_to_port(port, sg, 1, in_count, tok, nonblock); -} - -static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents, - size_t in_count, bool nonb
[PATCH v8 2/3] virtio_console: Add support for remoteproc serial
From: Sjur Brændeland Add a simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for communicating with a remote processor in an asymmetric multi-processing configuration. This implementation reuses the existing virtio_console implementation, and adds support for DMA allocation of data buffers and disables use of tty console and the virtio control queue. Signed-off-by: Sjur Brændeland Acked-by: Amit Shah --- drivers/char/virtio_console.c | 190 ++- include/uapi/linux/virtio_ids.h |1 + 2 files changed, 169 insertions(+), 22 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index b84390f..9ebadcb 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -37,8 +37,12 @@ #include #include #include +#include +#include #include "../tty/hvc/hvc_console.h" +#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC) + /* * This is a global struct for storing common data for all the devices * this driver handles. @@ -112,6 +116,15 @@ struct port_buffer { /* offset in the buf from which to consume data */ size_t offset; + /* DMA address of buffer */ + dma_addr_t dma; + + /* Device we got DMA memory from */ + struct device *dev; + + /* List of pending dma buffers to free */ + struct list_head list; + /* If sgpages == 0 then buf is used */ unsigned int sgpages; @@ -331,6 +344,11 @@ static bool is_console_port(struct port *port) return false; } +static bool is_rproc_serial(const struct virtio_device *vdev) +{ + return is_rproc_enabled && vdev->id.device == VIRTIO_ID_RPROC_SERIAL; +} + static inline bool use_multiport(struct ports_device *portdev) { /* @@ -342,11 +360,13 @@ static inline bool use_multiport(struct ports_device *portdev) return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT); } -static void free_buf(struct port_buffer *buf) +static DEFINE_SPINLOCK(dma_bufs_lock); +static LIST_HEAD(pending_free_dma_bufs); + +static void free_buf(struct port_buffer *buf, bool can_sleep) { unsigned int i; - kfree(buf->buf); for (i = 0; i < buf->sgpages; i++) { struct page *page = sg_page(>sg[i]); if (!page) @@ -354,14 +374,57 @@ static void free_buf(struct port_buffer *buf) put_page(page); } + if (!buf->dev) { + kfree(buf->buf); + } else if (is_rproc_enabled) { + unsigned long flags; + + /* dma_free_coherent requires interrupts to be enabled. */ + if (!can_sleep) { + /* queue up dma-buffers to be freed later */ + spin_lock_irqsave(_bufs_lock, flags); + list_add_tail(>list, _free_dma_bufs); + spin_unlock_irqrestore(_bufs_lock, flags); + return; + } + dma_free_coherent(buf->dev, buf->size, buf->buf, buf->dma); + + /* Release device refcnt and allow it to be freed */ + put_device(buf->dev); + } + kfree(buf); } +static void reclaim_dma_bufs(void) +{ + unsigned long flags; + struct port_buffer *buf, *tmp; + LIST_HEAD(tmp_list); + + if (list_empty(_free_dma_bufs)) + return; + + /* Create a copy of the pending_free_dma_bufs while holding the lock */ + spin_lock_irqsave(_bufs_lock, flags); + list_cut_position(_list, _free_dma_bufs, + pending_free_dma_bufs.prev); + spin_unlock_irqrestore(_bufs_lock, flags); + + /* Release the dma buffers, without irqs enabled */ + list_for_each_entry_safe(buf, tmp, _list, list) { + list_del(>list); + free_buf(buf, true); + } +} + static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, int pages) { struct port_buffer *buf; + reclaim_dma_bufs(); + /* * Allocate buffer and the sg list. The sg list array is allocated * directly after the port_buffer struct. @@ -373,11 +436,34 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, buf->sgpages = pages; if (pages > 0) { + buf->dev = NULL; buf->buf = NULL; return buf; } - buf->buf = kmalloc(buf_size, GFP_KERNEL); + if (is_rproc_serial(vq->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. +* T
[PATCH v8 3/3] virtio_console: Remove buffers from out_vq at port removal
From: Sjur Brændeland Remove buffers from the out-queue when a rproc_serial device is removed. Rproc_serial communicates with remote processors that may crash and leave buffers in the out-queue. But the virtio_console device is not supposed to leave buffers in the out-queue when a port is removed, so in this case throw a warning. Signed-off-by: Sjur Brændeland --- drivers/char/virtio_console.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 9ebadcb..3fa036a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1521,6 +1521,16 @@ static void remove_port_data(struct port *port) /* Remove buffers we queued up for the Host to send us data in. */ while ((buf = virtqueue_detach_unused_buf(port->in_vq))) free_buf(buf, true); + + /* +* Check the out-queue for buffers. For VIRTIO_CONSOLE it is a +* bug if this happens. But for RPROC_SERIAL the remote processor +* may have crashed, leaving buffers hanging in the out-queue. +*/ + while ((buf = virtqueue_detach_unused_buf(port->out_vq))) { + WARN_ON_ONCE(!is_rproc_serial(port->portdev->vdev)); + free_buf(buf, true); + } } /* -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv8 0/3]virtio_console: Add rproc_serial driver
From: Sjur Brændeland This patch-set introduces a new virtio type "rproc_serial" for communicating with remote processors over shared memory. The driver depends on the the remoteproc framework. As preparation for introducing "rproc_serial" I've done a refactoring of the transmit buffer handling. Changes since v7: - Rebased to virtio-next branch. - Removed extra added lines. - Removed superfluous checks before calling reclaim_dma_bufs(). - Rusty raised some question regarding garbage collection of the out-vq, so I moved this into a separate patch. Thanks, Sjur Sjur Brændeland (3): virtio_console: Merge struct buffer_token into struct port_buffer virtio_console: Add support for remoteproc serial virtio_console: Remove buffers from out_vq at port removal drivers/char/virtio_console.c | 325 +++ include/uapi/linux/virtio_ids.h |1 + 2 files changed, 230 insertions(+), 96 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCHv8 0/3]virtio_console: Add rproc_serial driver
From: Sjur Brændeland sjur.brandel...@stericsson.com This patch-set introduces a new virtio type rproc_serial for communicating with remote processors over shared memory. The driver depends on the the remoteproc framework. As preparation for introducing rproc_serial I've done a refactoring of the transmit buffer handling. Changes since v7: - Rebased to virtio-next branch. - Removed extra added lines. - Removed superfluous checks before calling reclaim_dma_bufs(). - Rusty raised some question regarding garbage collection of the out-vq, so I moved this into a separate patch. Thanks, Sjur Sjur Brændeland (3): virtio_console: Merge struct buffer_token into struct port_buffer virtio_console: Add support for remoteproc serial virtio_console: Remove buffers from out_vq at port removal drivers/char/virtio_console.c | 325 +++ include/uapi/linux/virtio_ids.h |1 + 2 files changed, 230 insertions(+), 96 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 3/3] virtio_console: Remove buffers from out_vq at port removal
From: Sjur Brændeland sjur.brandel...@stericsson.com Remove buffers from the out-queue when a rproc_serial device is removed. Rproc_serial communicates with remote processors that may crash and leave buffers in the out-queue. But the virtio_console device is not supposed to leave buffers in the out-queue when a port is removed, so in this case throw a warning. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com --- drivers/char/virtio_console.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 9ebadcb..3fa036a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1521,6 +1521,16 @@ static void remove_port_data(struct port *port) /* Remove buffers we queued up for the Host to send us data in. */ while ((buf = virtqueue_detach_unused_buf(port-in_vq))) free_buf(buf, true); + + /* +* Check the out-queue for buffers. For VIRTIO_CONSOLE it is a +* bug if this happens. But for RPROC_SERIAL the remote processor +* may have crashed, leaving buffers hanging in the out-queue. +*/ + while ((buf = virtqueue_detach_unused_buf(port-out_vq))) { + WARN_ON_ONCE(!is_rproc_serial(port-portdev-vdev)); + free_buf(buf, true); + } } /* -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 2/3] virtio_console: Add support for remoteproc serial
From: Sjur Brændeland sjur.brandel...@stericsson.com Add a simple serial connection driver called VIRTIO_ID_RPROC_SERIAL (11) for communicating with a remote processor in an asymmetric multi-processing configuration. This implementation reuses the existing virtio_console implementation, and adds support for DMA allocation of data buffers and disables use of tty console and the virtio control queue. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com Acked-by: Amit Shah amit.s...@redhat.com --- drivers/char/virtio_console.c | 190 ++- include/uapi/linux/virtio_ids.h |1 + 2 files changed, 169 insertions(+), 22 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index b84390f..9ebadcb 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -37,8 +37,12 @@ #include linux/wait.h #include linux/workqueue.h #include linux/module.h +#include linux/dma-mapping.h +#include linux/kconfig.h #include ../tty/hvc/hvc_console.h +#define is_rproc_enabled IS_ENABLED(CONFIG_REMOTEPROC) + /* * This is a global struct for storing common data for all the devices * this driver handles. @@ -112,6 +116,15 @@ struct port_buffer { /* offset in the buf from which to consume data */ size_t offset; + /* DMA address of buffer */ + dma_addr_t dma; + + /* Device we got DMA memory from */ + struct device *dev; + + /* List of pending dma buffers to free */ + struct list_head list; + /* If sgpages == 0 then buf is used */ unsigned int sgpages; @@ -331,6 +344,11 @@ static bool is_console_port(struct port *port) return false; } +static bool is_rproc_serial(const struct virtio_device *vdev) +{ + return is_rproc_enabled vdev-id.device == VIRTIO_ID_RPROC_SERIAL; +} + static inline bool use_multiport(struct ports_device *portdev) { /* @@ -342,11 +360,13 @@ static inline bool use_multiport(struct ports_device *portdev) return portdev-vdev-features[0] (1 VIRTIO_CONSOLE_F_MULTIPORT); } -static void free_buf(struct port_buffer *buf) +static DEFINE_SPINLOCK(dma_bufs_lock); +static LIST_HEAD(pending_free_dma_bufs); + +static void free_buf(struct port_buffer *buf, bool can_sleep) { unsigned int i; - kfree(buf-buf); for (i = 0; i buf-sgpages; i++) { struct page *page = sg_page(buf-sg[i]); if (!page) @@ -354,14 +374,57 @@ static void free_buf(struct port_buffer *buf) put_page(page); } + if (!buf-dev) { + kfree(buf-buf); + } else if (is_rproc_enabled) { + unsigned long flags; + + /* dma_free_coherent requires interrupts to be enabled. */ + if (!can_sleep) { + /* queue up dma-buffers to be freed later */ + spin_lock_irqsave(dma_bufs_lock, flags); + list_add_tail(buf-list, pending_free_dma_bufs); + spin_unlock_irqrestore(dma_bufs_lock, flags); + return; + } + dma_free_coherent(buf-dev, buf-size, buf-buf, buf-dma); + + /* Release device refcnt and allow it to be freed */ + put_device(buf-dev); + } + kfree(buf); } +static void reclaim_dma_bufs(void) +{ + unsigned long flags; + struct port_buffer *buf, *tmp; + LIST_HEAD(tmp_list); + + if (list_empty(pending_free_dma_bufs)) + return; + + /* Create a copy of the pending_free_dma_bufs while holding the lock */ + spin_lock_irqsave(dma_bufs_lock, flags); + list_cut_position(tmp_list, pending_free_dma_bufs, + pending_free_dma_bufs.prev); + spin_unlock_irqrestore(dma_bufs_lock, flags); + + /* Release the dma buffers, without irqs enabled */ + list_for_each_entry_safe(buf, tmp, tmp_list, list) { + list_del(buf-list); + free_buf(buf, true); + } +} + static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, int pages) { struct port_buffer *buf; + reclaim_dma_bufs(); + /* * Allocate buffer and the sg list. The sg list array is allocated * directly after the port_buffer struct. @@ -373,11 +436,34 @@ static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, buf-sgpages = pages; if (pages 0) { + buf-dev = NULL; buf-buf = NULL; return buf; } - buf-buf = kmalloc(buf_size, GFP_KERNEL); + if (is_rproc_serial(vq-vdev)) { + /* +* Allocate DMA memory from ancestor. When a virtio +* device is created by remoteproc, the DMA memory is +* associated with the grandparent
[PATCH v8 1/3] virtio_console: Merge struct buffer_token into struct port_buffer
From: Sjur Brændeland sjur.brandel...@stericsson.com Refactoring the splice functionality by unifying the approach for sending scatter-lists and regular buffers. This simplifies buffer handling and reduces code size. Splice will now allocate a port_buffer and send_buf() and free_buf() can always be used for any buffer. Signed-off-by: Sjur Brændeland sjur.brandel...@stericsson.com Acked-by: Amit Shah amit.s...@redhat.com --- drivers/char/virtio_console.c | 129 + 1 file changed, 53 insertions(+), 76 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 4ad8aca..b84390f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -111,6 +111,12 @@ struct port_buffer { size_t len; /* offset in the buf from which to consume data */ size_t offset; + + /* If sgpages == 0 then buf is used */ + unsigned int sgpages; + + /* sg is used if spages 0. sg must be the last in is struct */ + struct scatterlist sg[0]; }; /* @@ -338,17 +344,39 @@ static inline bool use_multiport(struct ports_device *portdev) static void free_buf(struct port_buffer *buf) { + unsigned int i; + kfree(buf-buf); + for (i = 0; i buf-sgpages; i++) { + struct page *page = sg_page(buf-sg[i]); + if (!page) + break; + put_page(page); + } + kfree(buf); } -static struct port_buffer *alloc_buf(size_t buf_size) +static struct port_buffer *alloc_buf(struct virtqueue *vq, size_t buf_size, +int pages) { struct port_buffer *buf; - buf = kmalloc(sizeof(*buf), GFP_KERNEL); + /* +* Allocate buffer and the sg list. The sg list array is allocated +* directly after the port_buffer struct. +*/ + buf = kmalloc(sizeof(*buf) + sizeof(struct scatterlist) * pages, + GFP_KERNEL); if (!buf) goto fail; + + buf-sgpages = pages; + if (pages 0) { + buf-buf = NULL; + return buf; + } + buf-buf = kmalloc(buf_size, GFP_KERNEL); if (!buf-buf) goto free_buf; @@ -476,52 +504,26 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, return 0; } -struct buffer_token { - union { - void *buf; - struct scatterlist *sg; - } u; - /* If sgpages == 0 then buf is used, else sg is used */ - unsigned int sgpages; -}; - -static void reclaim_sg_pages(struct scatterlist *sg, unsigned int nrpages) -{ - int i; - struct page *page; - - for (i = 0; i nrpages; i++) { - page = sg_page(sg[i]); - if (!page) - break; - put_page(page); - } - kfree(sg); -} /* Callers must take the port-outvq_lock */ static void reclaim_consumed_buffers(struct port *port) { - struct buffer_token *tok; + struct port_buffer *buf; unsigned int len; if (!port-portdev) { /* Device has been unplugged. vqs are already gone. */ return; } - while ((tok = virtqueue_get_buf(port-out_vq, len))) { - if (tok-sgpages) - reclaim_sg_pages(tok-u.sg, tok-sgpages); - else - kfree(tok-u.buf); - kfree(tok); + while ((buf = virtqueue_get_buf(port-out_vq, len))) { + free_buf(buf); port-outvq_full = false; } } static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, int nents, size_t in_count, - struct buffer_token *tok, bool nonblock) + void *data, bool nonblock) { struct virtqueue *out_vq; int err; @@ -534,7 +536,7 @@ static ssize_t __send_to_port(struct port *port, struct scatterlist *sg, reclaim_consumed_buffers(port); - err = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC); + err = virtqueue_add_buf(out_vq, sg, nents, 0, data, GFP_ATOMIC); /* Tell Host to go! */ virtqueue_kick(out_vq); @@ -572,37 +574,6 @@ done: return in_count; } -static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, - bool nonblock) -{ - struct scatterlist sg[1]; - struct buffer_token *tok; - - tok = kmalloc(sizeof(*tok), GFP_ATOMIC); - if (!tok) - return -ENOMEM; - tok-sgpages = 0; - tok-u.buf = in_buf; - - sg_init_one(sg, in_buf, in_count); - - return __send_to_port(port, sg, 1, in_count, tok, nonblock); -} - -static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents, - size_t in_count, bool
Re: [PATCHv7 4/4] virtio_console: Add support for remoteproc serial
Hi Rusty, > The free-outside-interrupt issue is usually dealt with by offloading to > a wq, but your variant works (and isn't too ugly). Ok, thanks. >> +static void reclaim_dma_bufs(void) >> +{ >> + unsigned long flags; >> + struct port_buffer *buf, *tmp; >> + LIST_HEAD(tmp_list); >> + >> + if (list_empty(_free_dma_bufs)) >> + return; ... > Looks like this should be an easy noop even if !is_rproc_serial. Ok, I'll drop the superfluous check before calling reclaim_dma_bufs(). >> @@ -1415,7 +1524,16 @@ static void remove_port_data(struct port *port) >> >> /* Remove buffers we queued up for the Host to send us data in. */ >> while ((buf = virtqueue_detach_unused_buf(port->in_vq))) >> - free_buf(buf); >> + free_buf(buf, true); >> + >> + /* >> + * Remove buffers from out queue for rproc-serial. We cannot afford >> + * to leak any DMA mem, so reclaim this memory even if this might be >> + * racy for the remote processor. >> + */ >> + if (is_rproc_serial(port->portdev->vdev)) >> + while ((buf = virtqueue_detach_unused_buf(port->out_vq))) >> + free_buf(buf, true); >> } > > This seems wrong; either this is needed even if !is_rproc_serial(), or > it's not necessary as the out_vq is empty. > > Every path I can see has the device reset (in which case the queues > should not be active), or we got a VIRTIO_CONSOLE_PORT_REMOVE event (in > which case, the same). > > I think we can have non-blocking writes which could leave buffers in > out_vq: Amit? Hm, the remote device could potentially freeze whenever. So I think we should handle the situation where buffer are stuck in the out-queue for the rproc_serial device. But I'll move this piece of code into a new follow-up patch so we can handle this issue separately. Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv7 4/4] virtio_console: Add support for remoteproc serial
Hi Rusty, The free-outside-interrupt issue is usually dealt with by offloading to a wq, but your variant works (and isn't too ugly). Ok, thanks. +static void reclaim_dma_bufs(void) +{ + unsigned long flags; + struct port_buffer *buf, *tmp; + LIST_HEAD(tmp_list); + + if (list_empty(pending_free_dma_bufs)) + return; ... Looks like this should be an easy noop even if !is_rproc_serial. Ok, I'll drop the superfluous check before calling reclaim_dma_bufs(). @@ -1415,7 +1524,16 @@ static void remove_port_data(struct port *port) /* Remove buffers we queued up for the Host to send us data in. */ while ((buf = virtqueue_detach_unused_buf(port-in_vq))) - free_buf(buf); + free_buf(buf, true); + + /* + * Remove buffers from out queue for rproc-serial. We cannot afford + * to leak any DMA mem, so reclaim this memory even if this might be + * racy for the remote processor. + */ + if (is_rproc_serial(port-portdev-vdev)) + while ((buf = virtqueue_detach_unused_buf(port-out_vq))) + free_buf(buf, true); } This seems wrong; either this is needed even if !is_rproc_serial(), or it's not necessary as the out_vq is empty. Every path I can see has the device reset (in which case the queues should not be active), or we got a VIRTIO_CONSOLE_PORT_REMOVE event (in which case, the same). I think we can have non-blocking writes which could leave buffers in out_vq: Amit? Hm, the remote device could potentially freeze whenever. So I think we should handle the situation where buffer are stuck in the out-queue for the rproc_serial device. But I'll move this piece of code into a new follow-up patch so we can handle this issue separately. Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
remoteproc open issues (was [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)
Hi, On Fri, Aug 10, 2012 at 5:30 PM, Ohad Ben-Cohen wrote: > The general direction I have in mind is to put the resource table in > its final location while we do the first pass of fw parsing. > > This will solve all sort of open issues we have (or going to have soon): > > 1. dynamically-allocated address of the vrings can be communicated > 2. vdev statuses can be communicated > 3. virtio config space will finally become bi-directional as it should > 4. dynamically probed rproc-to-rproc IPC could then take place > > It's the real deal :) > > The only problem with this approach is that the resource table isn't > reloaded throughout cycles of power up/down, and that is insecure. > We'll have to manually reload it somewhere after the rproc is powered > down (or before it is powered up again). > > This change will break existing firmwares, but it looks required and > inevitable. Has anyone started looking into any of the open issues mentioned above? Thanks, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
remoteproc open issues (was [RFC 1/4] remoteproc: Bugfix assign device address to carveout (noiommu)
Hi, On Fri, Aug 10, 2012 at 5:30 PM, Ohad Ben-Cohen o...@wizery.com wrote: The general direction I have in mind is to put the resource table in its final location while we do the first pass of fw parsing. This will solve all sort of open issues we have (or going to have soon): 1. dynamically-allocated address of the vrings can be communicated 2. vdev statuses can be communicated 3. virtio config space will finally become bi-directional as it should 4. dynamically probed rproc-to-rproc IPC could then take place It's the real deal :) The only problem with this approach is that the resource table isn't reloaded throughout cycles of power up/down, and that is insecure. We'll have to manually reload it somewhere after the rproc is powered down (or before it is powered up again). This change will break existing firmwares, but it looks required and inevitable. Has anyone started looking into any of the open issues mentioned above? Thanks, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4] remoteproc: Add STE modem driver for remoteproc
Hi Ohad, >>> It might be safer though to invoke ->setup() in probe/remove, instead >>> of in start/stop (just like you essentially did before). >>> >>> This way we don't assume that stop is always called before remove (as >>> assumption that might be implicitly valid today, but may break in the >>> future). >>> >>> If you want, I can just change that before applying. >> >> Sounds good, please just go ahead and change this before applying. >> Thank you. > > I'm going to squash the below into the patch - please ack before I > apply and push - thanks! Looks good, thanks. Acked-by: Sjur Brændeland > > diff --git a/drivers/remoteproc/ste_modem_rproc.c > b/drivers/remoteproc/ste_modem_rproc.c > index fcc8677..a7743c0 100644 > --- a/drivers/remoteproc/ste_modem_rproc.c > +++ b/drivers/remoteproc/ste_modem_rproc.c > @@ -190,9 +190,6 @@ static int sproc_start(struct rproc *rproc) > > sproc_dbg(sproc, "start ste-modem\n"); > > - /* Provide callback functions to modem device */ > - sproc->mdev->ops.setup(sproc->mdev, _dev_cb); > - > /* Sanity test the max_notifyid */ > if (rproc->max_notifyid > SPROC_MAX_NOTIFY_ID) { > sproc_err(sproc, "Notification IDs too high:%d\n", > @@ -220,9 +217,6 @@ static int sproc_stop(struct rproc *rproc) > struct sproc *sproc = rproc->priv; > sproc_dbg(sproc, "stop ste-modem\n"); > > - /* Reset device callback functions */ > - sproc->mdev->ops.setup(sproc->mdev, NULL); > - > return sproc->mdev->ops.power(sproc->mdev, false); > } > > @@ -241,6 +235,9 @@ static int sproc_drv_remove(struct platform_device *pdev) > > sproc_dbg(sproc, "remove ste-modem\n"); > > + /* Reset device callback functions */ > + sproc->mdev->ops.setup(sproc->mdev, NULL); > + > /* Unregister as remoteproc device */ > rproc_del(sproc->rproc); > rproc_put(sproc->rproc); > @@ -260,6 +257,13 @@ static int sproc_probe(struct platform_device *pdev) > int err; > > dev_dbg(>pdev.dev, "probe ste-modem\n"); > + > + if (!mdev->ops.setup || !mdev->ops.kick || !mdev->ops.kick_subscribe > || > + !mdev->ops.power) { > + dev_err(>pdev.dev, "invalid mdev ops\n"); > + return -EINVAL; > + } > + > rproc = rproc_alloc(>pdev.dev, mdev->pdev.name, _ops, > SPROC_MODEM_FIRMWARE, sizeof(*sproc)); > if (!rproc) > @@ -270,6 +274,9 @@ static int sproc_probe(struct platform_device *pdev) > sproc->rproc = rproc; > mdev->drv_data = sproc; > > + /* Provide callback functions to modem device */ > + sproc->mdev->ops.setup(sproc->mdev, _dev_cb); > + > /* Set the STE-modem specific firmware handler */ > rproc->fw_ops = _fw_ops; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4] remoteproc: Add STE modem driver for remoteproc
Hi Ohad, > Thanks, it looks good. > > It might be safer though to invoke ->setup() in probe/remove, instead > of in start/stop (just like you essentially did before). > > This way we don't assume that stop is always called before remove (as > assumption that might be implicitly valid today, but may break in the > future). > > If you want, I can just change that before applying. Sounds good, please just go ahead and change this before applying. Thank you. Regards, Sjur -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4] remoteproc: Add STE modem driver for remoteproc
Hi Ohad, It might be safer though to invoke -setup() in probe/remove, instead of in start/stop (just like you essentially did before). This way we don't assume that stop is always called before remove (as assumption that might be implicitly valid today, but may break in the future). If you want, I can just change that before applying. Sounds good, please just go ahead and change this before applying. Thank you. I'm going to squash the below into the patch - please ack before I apply and push - thanks! Looks good, thanks. Acked-by: Sjur Brændeland sjur.brandel...@stericsson.om diff --git a/drivers/remoteproc/ste_modem_rproc.c b/drivers/remoteproc/ste_modem_rproc.c index fcc8677..a7743c0 100644 --- a/drivers/remoteproc/ste_modem_rproc.c +++ b/drivers/remoteproc/ste_modem_rproc.c @@ -190,9 +190,6 @@ static int sproc_start(struct rproc *rproc) sproc_dbg(sproc, start ste-modem\n); - /* Provide callback functions to modem device */ - sproc-mdev-ops.setup(sproc-mdev, sproc_dev_cb); - /* Sanity test the max_notifyid */ if (rproc-max_notifyid SPROC_MAX_NOTIFY_ID) { sproc_err(sproc, Notification IDs too high:%d\n, @@ -220,9 +217,6 @@ static int sproc_stop(struct rproc *rproc) struct sproc *sproc = rproc-priv; sproc_dbg(sproc, stop ste-modem\n); - /* Reset device callback functions */ - sproc-mdev-ops.setup(sproc-mdev, NULL); - return sproc-mdev-ops.power(sproc-mdev, false); } @@ -241,6 +235,9 @@ static int sproc_drv_remove(struct platform_device *pdev) sproc_dbg(sproc, remove ste-modem\n); + /* Reset device callback functions */ + sproc-mdev-ops.setup(sproc-mdev, NULL); + /* Unregister as remoteproc device */ rproc_del(sproc-rproc); rproc_put(sproc-rproc); @@ -260,6 +257,13 @@ static int sproc_probe(struct platform_device *pdev) int err; dev_dbg(mdev-pdev.dev, probe ste-modem\n); + + if (!mdev-ops.setup || !mdev-ops.kick || !mdev-ops.kick_subscribe || + !mdev-ops.power) { + dev_err(mdev-pdev.dev, invalid mdev ops\n); + return -EINVAL; + } + rproc = rproc_alloc(mdev-pdev.dev, mdev-pdev.name, sproc_ops, SPROC_MODEM_FIRMWARE, sizeof(*sproc)); if (!rproc) @@ -270,6 +274,9 @@ static int sproc_probe(struct platform_device *pdev) sproc-rproc = rproc; mdev-drv_data = sproc; + /* Provide callback functions to modem device */ + sproc-mdev-ops.setup(sproc-mdev, sproc_dev_cb); + /* Set the STE-modem specific firmware handler */ rproc-fw_ops = sproc_fw_ops; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv4] remoteproc: Add STE modem driver for remoteproc
Hi Ohad, Thanks, it looks good. It might be safer though to invoke -setup() in probe/remove, instead of in start/stop (just like you essentially did before). This way we don't assume that stop is always called before remove (as assumption that might be implicitly valid today, but may break in the future). If you want, I can just change that before applying. Sounds good, please just go ahead and change this before applying. Thank you. Regards, Sjur -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/