Re: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO device
From: Arnaud POULIQUEN
Date: Fri Sep 23 2022 - 06:09:35 EST
Hi Peng,
On 9/21/22 16:07, Arnaud POULIQUEN wrote:
> Hi Peng,
>
> On 9/21/22 10:54, Peng Fan wrote:
>> Hi Arnaud,
>>
>>> Subject: [PATCH v8 0/4] remoteproc: restructure the remoteproc VirtIO
>>> device
>>
>> Sorry to get in at this late time, just try to catch up.
>> Not reviewing comments, just have a question,
>> Does remote core firmware requires changes to use this new feature?
>
> For this series, it is not.
> For the whole work, it should not, but it will probably depend on the
> evolutions related to the reviews and requirements that will come.
>
>> Does your 4 branches listed below still work with linux-6.x?
>
> I have to rebase them. Today my github branches are based on v5.18.rc1
> I plan to do this end of this week or next week.
>
>> Could the multiple vdev still share same mbox channel?
>
> Yes I'm trying to keep the legacy support of the mailbox in the
> remoteproc platform driver.
> If no mailbox is declared in the virtio subnode it calls the rproc->ops->kick
>
>>
>> I not own i.MX remote core firmware development, so if no need
>> firmware change, I would like give a try and see how it works.
>
> Great! That would be nice to have your feedback.
> Mailbox management is one point, I'm also ineterested in having feedback on
> the memory regions management
> I will ping you when my work will be rebased on 6.0
My github branches have been rebased on top of thre rproc_next(1d7b61c06dc3)
As a first step you should be able to rebase on my step4-virtio-mailboxes[1]
without any update of your driver. If I did my dev well, I kept the
compatibility with the legacy.
[1] https://github.com/arnopo/linux/commits/step4-virtio-mailboxes
Regards,
Arnaud
>
> Thanks,
> Arnaud
>
>>
>> Thanks,
>> Peng.
>>
>>>
>>> 1) Update from V7 [1]:
>>>
>>> - rebase on rproc-next branch [2], commit 729c16326b7f ("remoteproc:
>>> imx_dsp_rproc: fix argument 2 of rproc_mem_entry_init")
>>> The updates take into account the integration of the
>>> commit 1404acbb7f68 ("remoteproc: Fix dma_mem leak after
>>> rproc_shutdown")
>>> - add Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx> according
>>> to reviews on V7
>>>
>>>
>>> [1]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2022%2F7%2F13%2F663&data=05%7C01%7Cpeng.fan%
>>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=77pEuwAI7Lh61hx1%2B
>>> Hs79Cu0G5KOa6mzQ0PnTC5r8Xk%3D&reserved=0
>>> [2]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ke
>>> rnel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fremoteproc%2Flinux.g
>>> it%2Flog%2F%3Fh%3Dfor-
>>> next&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e7
>>> 439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>>> C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>>> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7
>>> C%7C&sdata=iWUSzKkN9BpHwqbO62awIcyVf9PXcftcdt2kytWVR78%3D
>>> &reserved=0
>>>
>>> 2) Patchset description:
>>>
>>> This series is a part of the work initiated a long time ago in the series
>>> "remoteproc: Decorelate virtio from core"[3]
>>>
>>> Objective of the work:
>>> - Update the remoteproc VirtIO device creation (use platform device)
>>> - Allow to declare remoteproc VirtIO device in DT
>>> - declare resources associated to a remote proc VirtIO
>>> - declare a list of VirtIO supported by the platform.
>>> - Prepare the enhancement to more VirtIO devices (e.g I2C, audio, video, ...).
>>> For instance be able to declare a I2C device in a virtio-i2C node.
>>> - Keep the legacy working!
>>> - Try to improve the picture about concerns reported by Christoph Hellwing
>>> [4][5]
>>>
>>> [3]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2020%2F4%2F16%2F1817&data=05%7C01%7Cpeng.fan
>>> %40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4
>>> c6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7
>>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWw
>>> iLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oPWSfUweLdhUFK5X9
>>> 2YcGHem8s%2Bfelcr%2FHx9JAlKG%2BI%3D&reserved=0
>>> [4]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.
>>> org%2Flkml%2F2021%2F6%2F23%2F607&data=05%7C01%7Cpeng.fan%
>>> 40nxp.com%7Ce0e5200d739a48e7439508da87599d14%7C686ea1d3bc2b4c
>>> 6fa92cd99c5c301635%7C0%7C0%7C637971116202643149%7CUnknown%7C
>>> TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
>>> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HPpnlaykes8R1Kz1dEN
>>> nirEHkDNr7JvRs%2FcsaDPuLdI%3D&reserved=0
>>> [5]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>> hwork.kernel.org%2Fproject%2Flinux-
>>> remoteproc%2Fpatch%2FAOKowLclCbOCKxyiJ71WeNyuAAj2q8EUtxrXbyky5E
>>> %40cp7-web-
>>> 042.plabs.ch%2F&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e520
>>> 0d739a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%
>>> 7C0%7C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIj
>>> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>> 3000%7C%7C%7C&sdata=GtNruefDreOoogL%2BlntAC7GBfk6E1Goq4j%
>>> 2BYXt36RdI%3D&reserved=0
>>>
>>> In term of device tree this would result in such hierarchy (stm32mp1
>>> example with 2 virtio RPMSG):
>>>
>>> m4_rproc: m4@10000000 {
>>> compatible = "st,stm32mp1-m4";
>>> reg = <0x10000000 0x40000>,
>>> <0x30000000 0x40000>,
>>> <0x38000000 0x10000>;
>>> memory-region = <&retram>, <&mcuram>,<&mcuram2>;
>>> mboxes = <&ipcc 2>, <&ipcc 3>;
>>> mbox-names = "shutdown", "detach";
>>> status = "okay";
>>>
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> vdev@0 {
>>> compatible = "rproc-virtio";
>>> reg = <0>;
>>> virtio,id = <7>; /* RPMSG */
>>> memory-region = <&vdev0vring0>, <&vdev0vring1>,
>>> <&vdev0buffer>;
>>> mboxes = <&ipcc 0>, <&ipcc 1>;
>>> mbox-names = "vq0", "vq1";
>>> status = "okay";
>>> };
>>>
>>> vdev@1 {
>>> compatible = "rproc-virtio";
>>> reg = <1>;
>>> virtio,id = <7>; /*RPMSG */
>>> memory-region = <&vdev1vring0>, <&vdev1vring1>,
>>> <&vdev1buffer>;
>>> mboxes = <&ipcc 4>, <&ipcc 5>;
>>> mbox-names = "vq0", "vq1";
>>> status = "okay";
>>> };
>>> };
>>>
>>> I have divided the work in 4 steps to simplify the review, This series
>>> implements only the step 1:
>>> step 1: Redefine the remoteproc VirtIO device as a platform device
>>> - migrate rvdev management in remoteproc virtio.c,
>>> - create a remotproc virtio config ( can be disabled for platform that not
>>> use VirtIO IPC.
>>> step 2: Add possibility to declare and probe a VirtIO sub node
>>> - VirtIO bindings declaration,
>>> - multi DT VirtIO devices support,
>>> - introduction of a remote proc virtio bind device mechanism , =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep2-virtio-in-
>>> DT&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739a48e74
>>> 39508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
>>> 637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
>>> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
>>> %7C&sdata=XtF%2FQnml3QXFL7rgqST1Z2FotUzoj%2FD57WfiuAVMnr8
>>> %3D&reserved=0
>>> step 3: Add memory declaration in VirtIO subnode =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep3-virtio-
>>> memories&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&sdata=6gq28c6a1TJ%2FdkvokcEjgy6FKQcKTXSz%2BNAbJPo
>>> mjac%3D&reserved=0
>>> step 4: Add mailbox declaration in VirtIO subnode =>
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>> b.com%2Farnopo%2Flinux%2Fcommits%2Fstep4-virtio-
>>> mailboxes&data=05%7C01%7Cpeng.fan%40nxp.com%7Ce0e5200d739
>>> a48e7439508da87599d14%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
>>> C0%7C637971116202643149%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
>>> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%
>>> 7C%7C%7C&sdata=wfy2euuOPoMmBMIH3BOsGcsEYGSTWsDaRr7ENN
>>> QCK70%3D&reserved=0
>>>
>>> Arnaud Pouliquen (4):
>>> remoteproc: core: Introduce rproc_rvdev_add_device function
>>> remoteproc: core: Introduce rproc_add_rvdev function
>>> remoteproc: Move rproc_vdev management to remoteproc_virtio.c
>>> remoteproc: virtio: Create platform device for the remoteproc_virtio
>>>
>>> drivers/remoteproc/remoteproc_core.c | 154 +++---------------
>>> drivers/remoteproc/remoteproc_internal.h | 23 ++-
>>> drivers/remoteproc/remoteproc_virtio.c | 189 ++++++++++++++++++++---
>>> include/linux/remoteproc.h | 6 +-
>>> 4 files changed, 210 insertions(+), 162 deletions(-)
>>>
>>> --
>>> 2.24.3
>>