Skip to contentSkip to conversation

Ongoing remoteproc discussions

DISCUSSION Bjorn Andersson 4 participants 10 messages Jul 18, 2016 – Aug 16, 2016

lore
Share & tools
lore
#0 lore raw

During discussions with various people interested in moving their remoteproc-related out-of-tree patches towards mainline I have come across a set of topics common among various teams. The purpose of this email is to share some insight into these discussions and the current ideas for moving forward.

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

== Amended resources:
We need a way for the driver to amend resource definitions of the firmware with e.g. physical addresses and size constraints from devicetree, so I suggest that:

* We split the resource table parsing and allocation of resources in two steps; where the parse step updates the lists of resources and then at the time of boot we run through these and allocate the actual resources.

* We expose an API to the drivers so that they can register resources, as if they came from the table parser. We match each resource against previously registered resources based on name and merge or reject the updates. E.g. we merge a reference to the resource table offset and we reject incompatible size changes.

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

== Resource-less firmware with installed resource table:
The now available list of resources that have been registered with the core can serve as input for a function that generates a resource table for the remote. This gives us a mechanism for providing a remoteproc with information about resources in cases where the firmware lacks a resource table.

* We replace the rproc_find_loaded_rsc_table() with an new fw_op that is tasked with installing the resource table and update rproc->table_ptr. This op is made optional, for the remoteprocs with no installed resource table.

* We provide a helper function in the core that can be used in the fw_op, that builds a resource table in memory and copy this into the appropriate address of the remote, and update rproc->table_ptr to this.

The install function will be tasked with making sure table_ptr is valid and we make sure that error paths out of there and upon shutdown the core will make table_ptr reference the core's version - which might be NULL if no resource table exist.

== Supporting rpmsg alternatives:
For systems with other communication mechanisms than rpmsg we still want to be able to instantiate and tear down devices representing these communication channels, in a way that follows the life cycle of the remoteproc. To do this I suggest that:

* Like other resources we split virtio device handling in the remoteproc core into adding rproc_vdevs to rproc->rvdevs and actually calling rproc_add_virtio_dev().

* We generalise the rproc->rvdev somewhat, to be a list of "subdevices" that are registered with the remoteproc; each providing callbacks for registering and unregistering themselves as we bring the remoteproc up and down.

I made a quick hack of this with the Qualcomm SMD code and it turned out pretty good, but I believe it's too ugly to post until we get the cleanups from above tasks sorted out.

== Ramdump:
Being able to acquire core dumps from a miss-behaving remoteproc is an important feature in product development. I believe this snapshot should be taken between the shutdown of the remote and freeing of resources. As such I think it would be appropriate to:

* Split the inner working of rproc_shutdown() into the two steps of shutting down the remote and cleaning up its resources. Giving us a window of opportunity for snapshotting any resources.

In the generic case we will have to repopulate the resources with data from the firmware file (in case of corruptions), but we're expecting to load the same firmware again and as such I see no meaning in releasing and reacquiring carveouts etc. As such if we split the inner working of rproc_boot() into resource allocation, virtio device allocation and booting we can make rproc_trigger_recovery() do:

* Shutdown the remoteproc
* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

(The order of the top two is opposite of todays implementation, but probably more appropriate for the case of getting an accurate snapshot of the device).

We need to discuss the requirements for the format of what comes out of this. I've seen raw memory dumps of a fixed memory segments and I've seen ELF generators with segments matching those of the loaded ELF in the past.

== Firmware from late mounted file systems (needs discussion):
Typical for most remoteproc systems seems to be that the firmware files tend to be way to large for being stored in a initramfs. The two available mechanisms for dealing with this is to build remoteproc drivers as modules and relying on CONFIG_FW_LOADER_USER_HELPER_FALLBACK; the latter is being frowned upon by everyone but works fairly well for our purpose.

Forcing systems to compile remoteproc drivers as modules to achieve the delayed firmware load is not acceptable in my view.

So we need to come up with some mechanism for triggering auto-booting when firmware files are stored in a lately mounted file system.

Interwoven in these discussions are of course topics related to rpmsg and I will try to get back to these as things materialize.

Regards,
Bjorn

#1 lore raw

Hi Bjorn,

Quoted 21 lines · Bjorn Andersson

During discussions with various people interested in moving their remoteproc-related out-of-tree patches towards mainline I have come across a set of topics common among various teams. The purpose of this email is to share some insight into these discussions and the current ideas for moving forward.

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

Quoted 4 lines

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

What happens in the built-in case where firnmware isn't available until rootfs is mounted? In this case async fw load will have failed.

Quoted 124 lines

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

== Amended resources:
We need a way for the driver to amend resource definitions of the firmware with e.g. physical addresses and size constraints from devicetree, so I suggest that:

* We split the resource table parsing and allocation of resources in two steps; where the parse step updates the lists of resources and then at the time of boot we run through these and allocate the actual resources.

* We expose an API to the drivers so that they can register resources, as if they came from the table parser. We match each resource against previously registered resources based on name and merge or reject the updates. E.g. we merge a reference to the resource table offset and we reject incompatible size changes.

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

== Resource-less firmware with installed resource table:
The now available list of resources that have been registered with the core can serve as input for a function that generates a resource table for the remote. This gives us a mechanism for providing a remoteproc with information about resources in cases where the firmware lacks a resource table.

* We replace the rproc_find_loaded_rsc_table() with an new fw_op that is tasked with installing the resource table and update rproc->table_ptr. This op is made optional, for the remoteprocs with no installed resource table.

* We provide a helper function in the core that can be used in the fw_op, that builds a resource table in memory and copy this into the appropriate address of the remote, and update rproc->table_ptr to this.

The install function will be tasked with making sure table_ptr is valid and we make sure that error paths out of there and upon shutdown the core will make table_ptr reference the core's version - which might be NULL if no resource table exist.

== Supporting rpmsg alternatives:
For systems with other communication mechanisms than rpmsg we still want to be able to instantiate and tear down devices representing these communication channels, in a way that follows the life cycle of the remoteproc. To do this I suggest that:

* Like other resources we split virtio device handling in the remoteproc core into adding rproc_vdevs to rproc->rvdevs and actually calling rproc_add_virtio_dev().

* We generalise the rproc->rvdev somewhat, to be a list of "subdevices" that are registered with the remoteproc; each providing callbacks for registering and unregistering themselves as we bring the remoteproc up and down.

I made a quick hack of this with the Qualcomm SMD code and it turned out pretty good, but I believe it's too ugly to post until we get the cleanups from above tasks sorted out.

== Ramdump:
Being able to acquire core dumps from a miss-behaving remoteproc is an important feature in product development. I believe this snapshot should be taken between the shutdown of the remote and freeing of resources. As such I think it would be appropriate to:

* Split the inner working of rproc_shutdown() into the two steps of shutting down the remote and cleaning up its resources. Giving us a window of opportunity for snapshotting any resources.

In the generic case we will have to repopulate the resources with data from the firmware file (in case of corruptions), but we're expecting to load the same firmware again and as such I see no meaning in releasing and reacquiring carveouts etc. As such if we split the inner working of rproc_boot() into resource allocation, virtio device allocation and booting we can make rproc_trigger_recovery() do:

* Shutdown the remoteproc
* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

(The order of the top two is opposite of todays implementation, but probably more appropriate for the case of getting an accurate snapshot of the device).

We need to discuss the requirements for the format of what comes out of this. I've seen raw memory dumps of a fixed memory segments and I've seen ELF generators with segments matching those of the loaded ELF in the past.

Quoted 3 lines

== Firmware from late mounted file systems (needs discussion):
Typical for most remoteproc systems seems to be that the firmware files tend to be way to large for being stored in a initramfs.

In the case of fdma the firmware is actually very small. I think for my usecase I would be best off putting the fdma firmware in the initramfs and then all the current issues with async fw, deferred probe and late rootfs will go away.

If some better solution in remoteproc comes along in the future fdma can migrate over to using it.

Quoted 5 lines

The two
available mechanisms for dealing with this is to build remoteproc drivers as modules and relying on CONFIG_FW_LOADER_USER_HELPER_FALLBACK; the latter is being frowned upon by everyone but works fairly well for our purpose.

I agree has worked well for me in the past, but is frowned upon.

Quoted 3 lines

Forcing systems to compile remoteproc drivers as modules to achieve the delayed firmware load is not acceptable in my view.

Agree again.

Quoted 3 lines

So we need to come up with some mechanism for triggering auto-booting when firmware files are stored in a lately mounted file system.

One approach I've looked at is using -EPROBE_DEFER in the client driver (in my case dmaengine) if rproc_boot() can't find the firmware.

This does require a couple of changes to get working with mainline though: -

1) rproc async fw mechanism in rproc_add_virtio_devices() will have failed, so resource table parsing needs to be re-tried in rproc_boot(). See here

    http://thread.gmane.org/gmane.linux.ports.arm.kernel/512079

2) It requires a change in drivers-core to re-run deferred probes after rootfs has been mounted. See here https://lkml.org/lkml/2016/7/12/503.

With those two changes the deferred approach works well. It also has the benefit of not allowing devices to be registered which can never work due to missing firmware. In my case ALSA ASoC will be deffered until dma which it relies upon to work has its firmware in place.

One other quick question, you haven't mentioned subdev carveout here? Are you ok with me re-submitting fdma series as it is currently, and we can migrate over to using subdev careout if / when it gets merged in the future?

regards,

Peter.

#2 lore raw
Quoted 25 lines · Peter Griffin

Hi Bjorn,

Quoted 21 lines · Bjorn Andersson

During discussions with various people interested in moving their remoteproc-related out-of-tree patches towards mainline I have come across a set of topics common among various teams. The purpose of this email is to share some insight into these discussions and the current ideas for moving forward.

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

Quoted 8 lines
Quoted 4 lines

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

What happens in the built-in case where firnmware isn't available until rootfs is mounted? In this case async fw load will have failed.

As the only purpose of the async callback becomes triggering the boot I believe the result will be that the processor didn't boot. Beyond that I think we're into the question below of late firmware, something we need to discuss more.

With the current implementation one difference that we would have is that we must dereference the rproc according to this in recover and rproc_del(). But I think those needs some TLC anyways, to be more robust.

Quoted 9 lines
Quoted 9 lines

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

[..]

Quoted 11 lines
Quoted 3 lines

== Firmware from late mounted file systems (needs discussion):
Typical for most remoteproc systems seems to be that the firmware files tend to be way to large for being stored in a initramfs.

In the case of fdma the firmware is actually very small. I think for my usecase I would be best off putting the fdma firmware in the initramfs and then all the current issues with async fw, deferred probe and late rootfs will go away.

If some better solution in remoteproc comes along in the future fdma can migrate over to using it.

There are cases where this can be done, but it's something we must deal with for most of them.

Quoted 26 lines
Quoted 5 lines

The two
available mechanisms for dealing with this is to build remoteproc drivers as modules and relying on CONFIG_FW_LOADER_USER_HELPER_FALLBACK; the latter is being frowned upon by everyone but works fairly well for our purpose.

I agree has worked well for me in the past, but is frowned upon.

Quoted 3 lines

Forcing systems to compile remoteproc drivers as modules to achieve the delayed firmware load is not acceptable in my view.

Agree again.

Quoted 3 lines

So we need to come up with some mechanism for triggering auto-booting when firmware files are stored in a lately mounted file system.

One approach I've looked at is using -EPROBE_DEFER in the client driver (in my case dmaengine) if rproc_boot() can't find the firmware.

This does require a couple of changes to get working with mainline though: -

1) rproc async fw mechanism in rproc_add_virtio_devices() will have failed, so resource table parsing needs to be re-tried in rproc_boot(). See here

    http://thread.gmane.org/gmane.linux.ports.arm.kernel/512079
Quoted 8 lines

2) It requires a change in drivers-core to re-run deferred probes after rootfs has been mounted. See here https://lkml.org/lkml/2016/7/12/503.

With those two changes the deferred approach works well. It also has the benefit of not allowing devices to be registered which can never work due to missing firmware. In my case ALSA ASoC will be deffered until dma which it relies upon to work has its firmware in place.

I'm not much in favor of using EPROBE_DEFER as an indicator of the firmware might be available and I'm not sure we can make it work in a sane way for the always-on/auto-boot cases.

Quoted 4 lines

One other quick question, you haven't mentioned subdev carveout here? Are you ok with me re-submitting fdma series as it is currently, and we can migrate over to using subdev careout if / when it gets merged in the future?

I don't remember the details of your resources, but I would prefer if we can get the driver merged separate of any remoteproc improvements.

Thanks for your feedback!

Regards,
Bjorn

#3 lore raw
In reply to Bjorn Andersson

Hi Bjorn,

Please find below some comments/questions.

Regards,
Loic

Quoted 40 lines

-----Original Message-----
From: Bjorn Andersson [mailto:[email protected]]
Sent: Tuesday, July 19, 2016 1:10 AM
To: [email protected]
Cc: [email protected]; Lee Jones <[email protected]>; Sarangdhar Joshi <[email protected]>; Loic PALLARDY <[email protected]>; Eric FINCO <[email protected]>; Russell Wayman <[email protected]>; Matthew Locke <[email protected]>; Kumar Gala <[email protected]>; Bill Fletcher <[email protected]>; Puja Gupta <[email protected]>; Ohad Ben-Cohen <[email protected]>; Suman Anna <[email protected]>
Subject: Ongoing remoteproc discussions

During discussions with various people interested in moving their remoteproc-related out-of-tree patches towards mainline I have come across a set of topics common among various teams. The purpose of this email is to share some insight into these discussions and the current ideas for moving forward.

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

Quoted 6 lines

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

[LPA] As already mentioned in patch review, I would prefer auto-boot name rather than always-on for this feature.
What about coprocessor already loaded and started at boot stage? It may be the case of coprocessor used by bootloader and can't reset without breaking use case or coprocessor with security constraints. In that case, remoteproc should allocate rproc resource at linux level and sync on current rproc state.

Quoted 28 lines

== Amended resources:
We need a way for the driver to amend resource definitions of the firmware with e.g. physical addresses and size constraints from devicetree, so I suggest that:

* We split the resource table parsing and allocation of resources in two steps; where the parse step updates the lists of resources and then at the time of boot we run through these and allocate the actual resources.

* We expose an API to the drivers so that they can register resources, as if they came from the table parser. We match each resource against previously registered resources based on name and merge or reject the updates. E.g. we merge a reference to the resource table offset and we reject incompatible size changes.

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

Quoted 8 lines

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

[LPA] Do you have a list of resources you would like to support here?
In ST we plan to have DT for rproc resource description (PIO, peripheral bus...). Today coprocessor resources are managed dynamically using resource manager developed by TI on omap. But this solution is consuming time and code size.
We would like to implement rproc resource allocation at rproc_boot time, parsing associated DT section and getting the different requested resources... Is it aligned with your view?

Quoted 55 lines

== Resource-less firmware with installed resource table:
The now available list of resources that have been registered with the core can serve as input for a function that generates a resource table for the remote. This gives us a mechanism for providing a remoteproc with information about resources in cases where the firmware lacks a resource table.

* We replace the rproc_find_loaded_rsc_table() with an new fw_op that is tasked with installing the resource table and update rproc->table_ptr. This op is made optional, for the remoteprocs with no installed resource table.

* We provide a helper function in the core that can be used in the fw_op, that builds a resource table in memory and copy this into the appropriate address of the remote, and update rproc->table_ptr to this.

The install function will be tasked with making sure table_ptr is valid and we make sure that error paths out of there and upon shutdown the core will make table_ptr reference the core's version - which might be NULL if no resource table exist.

== Supporting rpmsg alternatives:
For systems with other communication mechanisms than rpmsg we still want to be able to instantiate and tear down devices representing these communication channels, in a way that follows the life cycle of the remoteproc. To do this I suggest that:

* Like other resources we split virtio device handling in the remoteproc core into adding rproc_vdevs to rproc->rvdevs and actually calling rproc_add_virtio_dev().

* We generalise the rproc->rvdev somewhat, to be a list of "subdevices" that are registered with the remoteproc; each providing callbacks for registering and unregistering themselves as we bring the remoteproc up and down.

I made a quick hack of this with the Qualcomm SMD code and it turned out pretty good, but I believe it's too ugly to post until we get the cleanups from above tasks sorted out.

== Ramdump:
Being able to acquire core dumps from a miss-behaving remoteproc is an important feature in product development. I believe this snapshot should be taken between the shutdown of the remote and freeing of resources. As such I think it would be appropriate to:

* Split the inner working of rproc_shutdown() into the two steps of shutting down the remote and cleaning up its resources. Giving us a window of opportunity for snapshotting any resources.

Quoted 8 lines

In the generic case we will have to repopulate the resources with data from the firmware file (in case of corruptions), but we're expecting to load the same firmware again and as such I see no meaning in releasing and reacquiring carveouts etc. As such if we split the inner working of rproc_boot() into resource allocation, virtio device allocation and booting we can make rproc_trigger_recovery() do:

* Shutdown the remoteproc

[LPA] just a technical remark, shutting down the remoteproc may switch off some power domain and disable access to remoteproc memories or registers. An intermediate state is need to allow snapshot generation.

Quoted 5 lines

* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

Quoted 8 lines

(The order of the top two is opposite of todays implementation, but probably more appropriate for the case of getting an accurate snapshot of the device).

We need to discuss the requirements for the format of what comes out of this. I've seen raw memory dumps of a fixed memory segments and I've seen ELF generators with segments matching those of the loaded ELF in the past.

[LPA] +1 for ELF format

Quoted 23 lines

== Firmware from late mounted file systems (needs discussion):
Typical for most remoteproc systems seems to be that the firmware files tend to be way to large for being stored in a initramfs. The two available mechanisms for dealing with this is to build remoteproc drivers as modules and relying on CONFIG_FW_LOADER_USER_HELPER_FALLBACK;
the latter is being frowned upon by everyone but works fairly well for our purpose.

Forcing systems to compile remoteproc drivers as modules to achieve the delayed firmware load is not acceptable in my view.

So we need to come up with some mechanism for triggering auto-booting when firmware files are stored in a lately mounted file system.

Interwoven in these discussions are of course topics related to rpmsg and I will try to get back to these as things materialize.

Regards,
Bjorn

#4 lore raw
Quoted 1 line · Loic PALLARDY
Quoted 1 line

== Auto-boot or always-on:

[..]

Quoted 3 lines
Quoted 1 line

[LPA] As already mentioned in patch review, I would prefer auto-boot name rather than always-on for this feature.

Agreed.

Quoted 3 lines

What about coprocessor already loaded and started at boot stage? It may be the case of coprocessor used by bootloader and can't reset without breaking use case or coprocessor with security constraints.

For the cases I've dealt with we just didn't represent the remote processor in the kernel, we just reserved the carveouts and communicated with it.

Quoted 2 lines

In that case, remoteproc should allocate rproc resource at linux level and sync on current rproc state.

Sure.

Quoted 1 line
Quoted 1 line

[..]

Quoted 15 lines
Quoted 15 lines

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

Quoted 4 lines
Quoted 3 lines

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

[LPA] Do you have a list of resources you would like to support here?

With resources here I meant the existing remoteproc resources, i.e. carveouts, devmem, trace and vdev/vrings.

Quoted 7 lines

In ST we plan to have DT for rproc resource description (PIO, peripheral bus...). Today coprocessor resources are managed dynamically using resource manager developed by TI on omap. But this solution is consuming time and code size.
We would like to implement rproc resource allocation at rproc_boot time, parsing associated DT section and getting the different requested resources...

Are you talking about the resmgr found in downstream TI trees? What kinds of resources and how would this look like?

Quoted 2 lines

Is it aligned with your view?

I'm generally considering these resources (e.g. regulators exposed by resmgr) not being part of the life cycle management of the remote processor, but rather related to the application running on the remote processor; as such I don't think they should reside in the remoteproc core.

That said, for resmgr to move upstream I think it needs to be generalized.

Quoted 2 lines
Quoted 2 lines

[..]

Quoted 1 line
Quoted 1 line

== Ramdump:

[..]

Quoted 7 lines
Quoted 2 lines

* Shutdown the remoteproc

[LPA] just a technical remark, shutting down the remoteproc may switch off some power domain and disable access to remoteproc memories or registers.
An intermediate state is need to allow snapshot generation.

Ahh, you're right. Then we need to complicate this dance a little bit, and probably make it possible for drivers to take part in it.

Quoted 6 lines
Quoted 6 lines

* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

Thanks for your reply Loic.

Regards,
Bjorn

#5 lore raw
Quoted 10 lines · Bjorn Andersson
Quoted 1 line · Loic PALLARDY
Quoted 1 line

== Auto-boot or always-on:

[..]

Quoted 3 lines
Quoted 1 line

[LPA] As already mentioned in patch review, I would prefer auto-boot name rather than always-on for this feature.

Agreed.

Quoted 7 lines
Quoted 3 lines

What about coprocessor already loaded and started at boot stage? It may be the case of coprocessor used by bootloader and can't reset without breaking use case or coprocessor with security constraints.

For the cases I've dealt with we just didn't represent the remote processor in the kernel, we just reserved the carveouts and communicated with it.

Yeah, we have a similar usecase as well, and we do want the remoteproc to behave as in the normal case once the kernel has booted up and the corresponding driver has been probed. We have had to do some magic (not zeroing memory) for presenting the remoteproc still to Linux-side applications and client drivers.

This indeed brings me one of the list of enhancements I have in mind - to add an ops for individual driver control for allocating memory on carveouts, vrings etc with a fall-back to the dma_alloc API in the remoteproc core.

Bjorn, I take it that you are not using rpmsg here if that lifecycle is managed separately from remoteproc.

Quoted 23 lines
Quoted 3 lines

In that case, remoteproc should allocate rproc resource at linux level and sync on current rproc state.

Sure.

Quoted 1 line
Quoted 1 line

[..]

Quoted 15 lines
Quoted 15 lines

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

Quoted 15 lines
Quoted 4 lines
Quoted 3 lines

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

[LPA] Do you have a list of resources you would like to support here?

With resources here I meant the existing remoteproc resources, i.e. carveouts, devmem, trace and vdev/vrings.

Quoted 7 lines

In ST we plan to have DT for rproc resource description (PIO, peripheral bus...). Today coprocessor resources are managed dynamically using resource manager developed by TI on omap. But this solution is consuming time and code size.
We would like to implement rproc resource allocation at rproc_boot time, parsing associated DT section and getting the different requested resources...

Yeah, this becomes somewhat complicated when we are talking about peripherals, because it depends on they get used. I see the following usage patterns:
1. do not instantiate the devices on Linux, and leave them to be managed completely by s/w running on remoteproc.
2. resources that can be managed alongside the remoteproc state (request them up before rproc_boot, and release them after rproc_shutdown). This can always be done within the respective remoteproc driver as the peripherals used are specific to each platform.
3. resources that only need to be managed at runtime, especially if the PM around them in controlled on the Linux-side.

Quoted 4 lines

Are you talking about the resmgr found in downstream TI trees? What kinds of resources and how would this look like?

Quoted 8 lines
Quoted 2 lines

Is it aligned with your view?

I'm generally considering these resources (e.g. regulators exposed by resmgr) not being part of the life cycle management of the remote processor, but rather related to the application running on the remote processor; as such I don't think they should reside in the remoteproc core.

Agreed, we did use resmgr specifically for #3. It also allowed us to recover these resources in case of a remoteproc crash while holding them.

Quoted 3 lines

That said, for resmgr to move upstream I think it needs to be generalized.

Indeed, the TI resmgr was written before DT, and it would need rework if we were to go down that path.

That said, if the management is moving towards the System Control Processor like frameworks, this won't be needed.

regards
Suman

Quoted 29 lines
Quoted 3 lines
Quoted 3 lines

[..]

Quoted 1 line
Quoted 1 line

== Ramdump:

[..]

Quoted 7 lines
Quoted 2 lines

* Shutdown the remoteproc

[LPA] just a technical remark, shutting down the remoteproc may switch off some power domain and disable access to remoteproc memories or registers.
An intermediate state is need to allow snapshot generation.

Ahh, you're right. Then we need to complicate this dance a little bit, and probably make it possible for drivers to take part in it.

Quoted 6 lines
Quoted 6 lines

* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

Thanks for your reply Loic.

Regards,
Bjorn

#6 lore raw
Quoted 25 lines · Suman Anna
Quoted 10 lines · Bjorn Andersson
Quoted 1 line · Loic PALLARDY
Quoted 1 line

== Auto-boot or always-on:

[..]

Quoted 3 lines
Quoted 1 line

[LPA] As already mentioned in patch review, I would prefer auto-boot name rather than always-on for this feature.

Agreed.

Quoted 7 lines
Quoted 3 lines

What about coprocessor already loaded and started at boot stage? It may be the case of coprocessor used by bootloader and can't reset without breaking use case or coprocessor with security constraints.

For the cases I've dealt with we just didn't represent the remote processor in the kernel, we just reserved the carveouts and communicated with it.

Yeah, we have a similar usecase as well, and we do want the remoteproc to behave as in the normal case once the kernel has booted up and the corresponding driver has been probed. We have had to do some magic (not zeroing memory) for presenting the remoteproc still to Linux-side applications and client drivers.

Quoted 5 lines

This indeed brings me one of the list of enhancements I have in mind - to add an ops for individual driver control for allocating memory on carveouts, vrings etc with a fall-back to the dma_alloc API in the remoteproc core.

For this case you would provide the carveout resource a fixed location, Loic is currently looking at one of my suggestions of using memremap() instead of dma_alloc_coherent() for the resources that has a specified "pa".

FYI, the alternative suggestion for handling regions with fixed location is to create "surrogate" devices, that gets assigned the memory range and use this for dma_alloc_coherent() - which would not solve this problem.

Quoted 3 lines

Bjorn, I take it that you are not using rpmsg here if that lifecycle is managed separately from remoteproc.

On this platform it's the Qualcomm equivalent "SMD", and we do. During boot we detect the presence of the vdev-equivalent and the link to this processor comes up and stays up until you power off the board.

Quoted 39 lines
Quoted 23 lines
Quoted 3 lines

In that case, remoteproc should allocate rproc resource at linux level and sync on current rproc state.

Sure.

Quoted 1 line
Quoted 1 line

[..]

Quoted 15 lines
Quoted 15 lines

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

Quoted 15 lines
Quoted 4 lines
Quoted 3 lines

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

[LPA] Do you have a list of resources you would like to support here?

With resources here I meant the existing remoteproc resources, i.e. carveouts, devmem, trace and vdev/vrings.

Quoted 7 lines

In ST we plan to have DT for rproc resource description (PIO, peripheral bus...). Today coprocessor resources are managed dynamically using resource manager developed by TI on omap. But this solution is consuming time and code size.
We would like to implement rproc resource allocation at rproc_boot time, parsing associated DT section and getting the different requested resources...

Quoted 5 lines

Yeah, this becomes somewhat complicated when we are talking about peripherals, because it depends on they get used. I see the following usage patterns:
1. do not instantiate the devices on Linux, and leave them to be managed completely by s/w running on remoteproc.

This is the easy case, if you ship a product you know which resources belong to the remote and can make sure that they are not referenced by the Linux system.

Quoted 6 lines

2. resources that can be managed alongside the remoteproc state (request them up before rproc_boot, and release them after rproc_shutdown). This can always be done within the respective remoteproc driver as the peripherals used are specific to each platform.
3. resources that only need to be managed at runtime, especially if the PM around them in controlled on the Linux-side.

If its resources that are related to the life cycle of the remoteproc I think they belong in the remoteproc driver itself, if it's dynamic, application level resources I think they should be handled by some sort of (e.g. rpmsg) client driver.

Quoted 18 lines
Quoted 5 lines

Are you talking about the resmgr found in downstream TI trees? What kinds of resources and how would this look like?

Quoted 8 lines
Quoted 2 lines

Is it aligned with your view?

I'm generally considering these resources (e.g. regulators exposed by resmgr) not being part of the life cycle management of the remote processor, but rather related to the application running on the remote processor; as such I don't think they should reside in the remoteproc core.

Agreed, we did use resmgr specifically for #3. It also allowed us to recover these resources in case of a remoteproc crash while holding them.

Quoted 1 line
Quoted 9 lines
Quoted 2 lines

That said, for resmgr to move upstream I think it needs to be generalized.

Indeed, the TI resmgr was written before DT, and it would need rework if we were to go down that path.

That said, if the management is moving towards the System Control Processor like frameworks, this won't be needed.

I'm looking forward to learn the details of these requirements, so that we can figure out how to best support them upstream.

Regards,
Bjorn

#7 lore raw
In reply to Bjorn Andersson

Hi Bjorn,

Quoted 5 lines · Bjorn Andersson

During discussions with various people interested in moving their remoteproc-related out-of-tree patches towards mainline I have come across a set of topics common among various teams. The purpose of this email is to share some insight into these discussions and the current ideas for moving forward.

Thanks for putting this together - a very good summary. I have a few comments/questions below.

Quoted 6 lines

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

I am trying to understand the usecase where one doesn't want to auto-boot with rpmsg entries, did you come across such an usecase?

Quoted 4 lines

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

Can this be made into a flags field rather than a boolean, especially to dictate the behavior between firmwares with vdevs and without vdevs. I do have some usecases that can have vdevs (so auto-boot) or no vdevs (with dummy resource tables) based on firmware being loaded (rproc_boot will be called by a client driver in that case).

Your current patch-set does break the wkup_m3_rproc driver as it now auto-boots.

Quoted 12 lines

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

Quoted 5 lines

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

I take it that you are talking about adding a new API down the road which should allow client drivers to set firmwares.

+1 for that.

Quoted 12 lines

== Amended resources:
We need a way for the driver to amend resource definitions of the firmware with e.g. physical addresses and size constraints from devicetree, so I suggest that:

* We split the resource table parsing and allocation of resources in two steps; where the parse step updates the lists of resources and then at the time of boot we run through these and allocate the actual resources.

Quoted 5 lines

* We expose an API to the drivers so that they can register resources, as if they came from the table parser. We match each resource against previously registered resources based on name and merge or reject the updates. E.g. we merge a reference to the resource table offset and we reject incompatible size changes.

So, all this to be invoked by a platform driver before rproc_add() even before requesting and processing a firmware? Is this also only limited for remoteproc platform drivers, or gonna be extended to client drivers (in the case where they are setting the firmwares).

One question w.r.t Lee's series is also adding new resources - how does that work with loaded resource tables given that you are dynamically increasing the table size, and the firmware image is already pre-linked. Or is the assumption here that there will not be a loaded resource table in such a scenario, we will have a loaded resource table for firmwares with vdevs for sure.

Quoted 31 lines

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

== Resource-less firmware with installed resource table:
The now available list of resources that have been registered with the core can serve as input for a function that generates a resource table for the remote. This gives us a mechanism for providing a remoteproc with information about resources in cases where the firmware lacks a resource table.

Quoted 4 lines

* We replace the rproc_find_loaded_rsc_table() with an new fw_op that is tasked with installing the resource table and update rproc->table_ptr. This op is made optional, for the remoteprocs with no installed resource table.

So, kinda similar to above question, how do you find a suitable location for this in the case of installed resource table?

Quoted 18 lines

* We provide a helper function in the core that can be used in the fw_op, that builds a resource table in memory and copy this into the appropriate address of the remote, and update rproc->table_ptr to this.

The install function will be tasked with making sure table_ptr is valid and we make sure that error paths out of there and upon shutdown the core will make table_ptr reference the core's version - which might be NULL if no resource table exist.

== Supporting rpmsg alternatives:
For systems with other communication mechanisms than rpmsg we still want to be able to instantiate and tear down devices representing these communication channels, in a way that follows the life cycle of the remoteproc. To do this I suggest that:

Quoted 8 lines

* Like other resources we split virtio device handling in the remoteproc core into adding rproc_vdevs to rproc->rvdevs and actually calling rproc_add_virtio_dev().

* We generalise the rproc->rvdev somewhat, to be a list of "subdevices" that are registered with the remoteproc; each providing callbacks for registering and unregistering themselves as we bring the remoteproc up and down.

+1, this is a good direction.

regards
Suman

Quoted 61 lines

I made a quick hack of this with the Qualcomm SMD code and it turned out pretty good, but I believe it's too ugly to post until we get the cleanups from above tasks sorted out.

== Ramdump:
Being able to acquire core dumps from a miss-behaving remoteproc is an important feature in product development. I believe this snapshot should be taken between the shutdown of the remote and freeing of resources. As such I think it would be appropriate to:

* Split the inner working of rproc_shutdown() into the two steps of shutting down the remote and cleaning up its resources. Giving us a window of opportunity for snapshotting any resources.

In the generic case we will have to repopulate the resources with data from the firmware file (in case of corruptions), but we're expecting to load the same firmware again and as such I see no meaning in releasing and reacquiring carveouts etc. As such if we split the inner working of rproc_boot() into resource allocation, virtio device allocation and booting we can make rproc_trigger_recovery() do:

* Shutdown the remoteproc
* Shutdown virtio devices
* Take snapshot
* Register virtio devices
* Start the remoteproc

(The order of the top two is opposite of todays implementation, but probably more appropriate for the case of getting an accurate snapshot of the device).

We need to discuss the requirements for the format of what comes out of this. I've seen raw memory dumps of a fixed memory segments and I've seen ELF generators with segments matching those of the loaded ELF in the past.

== Firmware from late mounted file systems (needs discussion):
Typical for most remoteproc systems seems to be that the firmware files tend to be way to large for being stored in a initramfs. The two available mechanisms for dealing with this is to build remoteproc drivers as modules and relying on CONFIG_FW_LOADER_USER_HELPER_FALLBACK; the latter is being frowned upon by everyone but works fairly well for our purpose.

Forcing systems to compile remoteproc drivers as modules to achieve the delayed firmware load is not acceptable in my view.

So we need to come up with some mechanism for triggering auto-booting when firmware files are stored in a lately mounted file system.

Interwoven in these discussions are of course topics related to rpmsg and I will try to get back to these as things materialize.

Regards,
Bjorn

#8 lore raw
Quoted 2 lines · Suman Anna

Hi Bjorn,

Hi,

Quoted 1 line · Bjorn Andersson

[..]

Quoted 9 lines
Quoted 5 lines

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest that:

I am trying to understand the usecase where one doesn't want to auto-boot with rpmsg entries, did you come across such an usecase?

This is a request that comes from Loic (ST). I'm unaware of the details, but I can think of scenarios where rpmsg channels serves as auxiliary functionality to the main purpose of a co-processor (e.g. debug functionality).

Quoted 11 lines
Quoted 4 lines

* We introduce a property in the rproc struct where drivers can toggle if they want always-on or not. We default this to true, as an estimate of backwards compatibility.

Can this be made into a flags field rather than a boolean, especially to dictate the behavior between firmwares with vdevs and without vdevs. I do have some usecases that can have vdevs (so auto-boot) or no vdevs (with dummy resource tables) based on firmware being loaded (rproc_boot will be called by a client driver in that case).

I would prefer that we don't have to scan the resource table and make "advanced" decisions based on its content - and as you say it has to be at least tristate to work based on above.

I'm hoping that we can approximate this with a boolean for now and then I would like to reverse the resource table parsing mechanism, so that instead of calling "give me a resource table" we would have a fw_ops that we call to "parse" the firmware and with below APIs we can build up the resource table from that.

This parse could then work on other structures than the resource table and it could be made processor specific to set auto-boot based on properties of the firmware.

Quoted 3 lines

Your current patch-set does break the wkup_m3_rproc driver as it now auto-boots.

Ahh right, thanks for pointing this out.

I'll look through the tree to figure out which drivers seems to have clients calling rproc_boot() and make them auto_boot = false;

Quoted 18 lines
Quoted 12 lines

* We move the allocation of vrings to be done at the time of the other allocations and follow that with the registration of virtio devices, before actually booting the rproc. And we tear down the virtio devices as part of shutdown.

* We remove the rproc_boot()/shutdown() calls from the remoteproc_virtio.c and based on rproc->always_on we call these in the async firmware loader callback, in rproc_trigger_recovery() and rproc_del().

Quoted 5 lines

A side effect of this is that the async firmware loading only purpose is to trigger the boot as the firmware becomes available (something we need to tweak further). I therefor see no reason to mandate the firmware is unchanged between boots, which simplifies the posed rproc_set_firmware() function - as long as it's done on a remoteproc that's not always_on.

Quoted 4 lines

I take it that you are talking about adding a new API down the road which should allow client drivers to set firmwares.

+1 for that.

A while back Lee posted a patch for this. With the auto-boot rework the resource table resources gets a much cleaner life cycle and that patch can be simplified.

So I'm hoping we can introduce that shortly.

What I was referring to is the fact that you today can't modify your resource table calling rproc_add(), so you can replace your firmware, but only as long as it's sharing the same resource table - unless you start the old one, replace the files and then trigger a crash...

Quoted 19 lines
Quoted 13 lines

== Amended resources:
We need a way for the driver to amend resource definitions of the firmware with e.g. physical addresses and size constraints from devicetree, so I suggest that:

* We split the resource table parsing and allocation of resources in two steps; where the parse step updates the lists of resources and then at the time of boot we run through these and allocate the actual resources.

Quoted 5 lines

* We expose an API to the drivers so that they can register resources, as if they came from the table parser. We match each resource against previously registered resources based on name and merge or reject the updates. E.g. we merge a reference to the resource table offset and we reject incompatible size changes.

Quoted 5 lines

So, all this to be invoked by a platform driver before rproc_add() even before requesting and processing a firmware? Is this also only limited for remoteproc platform drivers, or gonna be extended to client drivers (in the case where they are setting the firmwares).

It provides the mechanism for remoteproc drivers to provide the core with resources specified in e.g. devicetree. Allowing e.g. me to reuse the carveout handling on Qualcomm platforms, where we don't have a resource table.

Quoted 7 lines

One question w.r.t Lee's series is also adding new resources - how does that work with loaded resource tables given that you are dynamically increasing the table size, and the firmware image is already pre-linked. Or is the assumption here that there will not be a loaded resource table in such a scenario, we will have a loaded resource table for firmwares with vdevs for sure.

That's a very valid concern, I would hope that we could look at the size of the loaded-resource segment in the ".resource_table" segment. Perhaps as we validate the resource table?

Can you please bring this issue up on the affected patch?

Quoted 31 lines
Quoted 31 lines

== Resource-less firmware:
To be able to use remoteproc with firmware either without a resource table or resource data in other forms we today provide a resource table stub in each driver, instead we could refactor the resource table parsing code.

* We replace the find_rsc_table operation in rproc_fw_ops with a parse operation, that uses the newly created API (above) to register the resources with the core; largely decoupling the resource table format from the remoteproc core.

* We make the parse() function in rproc_fw_ops optional, allowing remoteproc drivers to specify that there's no resource parsing to be done (they can still provide resources programmatically between rproc_alloc() and rproc_add()).

This setup allows custom resource building functions to be implemented, one such example is the Qualcomm firmware files where most resource data is a combination of static information (DT) and data from the ELF header.

== Resource-less firmware with installed resource table:
The now available list of resources that have been registered with the core can serve as input for a function that generates a resource table for the remote. This gives us a mechanism for providing a remoteproc with information about resources in cases where the firmware lacks a resource table.

Quoted 8 lines
Quoted 4 lines

* We replace the rproc_find_loaded_rsc_table() with an new fw_op that is tasked with installing the resource table and update rproc->table_ptr. This op is made optional, for the remoteprocs with no installed resource table.

So, kinda similar to above question, how do you find a suitable location for this in the case of installed resource table?

Dito.

But here it would be more natural to return -ENOSPC or similar if there is a lack of space.

Quoted 22 lines
Quoted 22 lines

* We provide a helper function in the core that can be used in the fw_op, that builds a resource table in memory and copy this into the appropriate address of the remote, and update rproc->table_ptr to this.

The install function will be tasked with making sure table_ptr is valid and we make sure that error paths out of there and upon shutdown the core will make table_ptr reference the core's version - which might be NULL if no resource table exist.

== Supporting rpmsg alternatives:
For systems with other communication mechanisms than rpmsg we still want to be able to instantiate and tear down devices representing these communication channels, in a way that follows the life cycle of the remoteproc. To do this I suggest that:

* Like other resources we split virtio device handling in the remoteproc core into adding rproc_vdevs to rproc->rvdevs and actually calling rproc_add_virtio_dev().

Quoted 7 lines
Quoted 4 lines

* We generalise the rproc->rvdev somewhat, to be a list of "subdevices" that are registered with the remoteproc; each providing callbacks for registering and unregistering themselves as we bring the remoteproc up and down.

+1, this is a good direction.

Thanks for your input!

Regards,
Bjorn

#9 lore raw
Quoted 31 lines

-----Original Message-----
From: Bjorn Andersson [mailto:[email protected]]
Sent: Thursday, August 11, 2016 8:13 PM
To: Suman Anna <[email protected]>
Cc: [email protected]; [email protected]; Lee Jones <[email protected]>; Sarangdhar Joshi <[email protected]>; Loic PALLARDY <[email protected]>; Eric FINCO <[email protected]>; Russell Wayman <[email protected]>; Matthew Locke <[email protected]>; Kumar Gala <[email protected]>; Bill Fletcher <[email protected]>; Puja Gupta <[email protected]>; Ohad Ben-Cohen <[email protected]>
Subject: Re: Ongoing remoteproc discussions

Quoted 2 lines · Suman Anna

Hi Bjorn,

Hi,

Quoted 1 line · Bjorn Andersson

[..]

Quoted 5 lines
Quoted 5 lines

== Auto-boot or always-on:
There are cases where we want to achieve the current auto-boot mechanism without rpmsg and there are cases where we don't want to auto-boot a remoteproc just because its resource table contains rpmsg entries. So we need to decouple this logic from the vdev. I suggest

that:

Quoted 1 line
Quoted 8 lines
Quoted 3 lines

I am trying to understand the usecase where one doesn't want to auto-boot with rpmsg entries, did you come across such an usecase?

This is a request that comes from Loic (ST). I'm unaware of the details, but I can think of scenarios where rpmsg channels serves as auxiliary functionality to the main purpose of a co-processor (e.g. debug functionality).

Hi Bjorn, Suman,

In ST, in general we have a driver on the top of remoteproc to control rproc activities (set fw name, boot, shutdown...).
The request was to have the same behavior between rproc with and without rpmsg.
With rpmsg, a rproc is automatically boot and so if on the top driver wants to apply same boot sequence (rproc_set_fw_name, rproc_boot, rproc_shutdown), rproc_shutdown will never happened as rproc_boot called twice (one by on the top driver, one by "rpmsg").

So the proposal was to have autoboot capability linked to the rproc itself, not to the communication link.

Regards,
Loic
[...]

Quoted 5 lines

Thanks for your input!

Regards,
Bjorn