ST Restricted
> -----Original Message-----
> From: Mathieu Poirier <mathieu.poirier(a)linaro.org>
> Sent: Saturday, May 20, 2023 1:00 AM
> To: Bill Mills <bill.mills(a)linaro.org>
> Cc: Shah, Tanmay <tanmay.shah(a)amd.com>; ed.mooring(a)gmail.com;
> tammy.leino(a)siemens.com; Arnaud POULIQUEN
> <arnaud.pouliquen(a)st.com>; openamp-rp(a)lists.openampproject.org
> Subject: Re: [RFC]: Variable RpMsg buffer size in Linux Kernel
>
> On Thu, 18 May 2023 at 10:27, Bill Mills <bill.mills(a)linaro.org> wrote:
> >
> >
> >
> > On 5/18/23 12:00 PM, Shah, Tanmay wrote:
> > > Hi all,
> > >
> > > As of now, rpmsg buffer size is decided by RPMSG_BUF_SIZE macro.
> > > This is fixed to 512 bytes.
> > >
> > > If platform needs to send larger than 512 bytes of payload, then the
> > > payload needs to be split between multiple buffers.
> > >
> > > Instead of that, it would be useful if platform can configure
> > > RPMSG_BUF_SIZE somehow.
> > >
> > > There are multiple options to achieve this:
> > >
> > > 1. Move RPMSG_BUF_SIZE to Kconfig -> The approach is not accepted as
> > > single kernel may not work for all platforms.
> > > 2. Keep device-tree property that mentions rpmsg buffer size.
> > > 1. Something in dts: “rpmsg-buf-size = <512>”
> > > 2. This way one kernel can work for all the platforms. If property
> > > is not mentioned, then we fallback to default size of 512 bytes.
> > > 3. Dynamic buffer allocation
> > > 1. This effort was done previously. Please refer to following
> > > discussion from kernel and openamp library:
> > >
> > > i.Openamp library: https://github.com/OpenAMP/open-amp/pull/155
> > > <https://github.com/OpenAMP/open-amp/pull/155>
> > >
> > > 1. Conclusion:
> > >
> > > https://github.com/OpenAMP/open-amp/issues/322#issuecomment-
> 98657382
> > > 8
> > > <https://github.com/OpenAMP/open-amp/issues/322#issuecomment-
> 9865738
> > > 28>
> > >
> > > ii.Kernel:
> > > https://patchwork.kernel.org/project/linux-remoteproc/cover/15489492
> > > 80-31794-1-git-send-email-xiaoxiang(a)xiaomi.com/
> > > <https://patchwork.kernel.org/project/linux-remoteproc/cover/1548949
> > > 280-31794-1-git-send-email-xiaoxiang(a)xiaomi.com/>
> > >
> >
> > Lore link to same:
> > https://lore.kernel.org/lkml/20190605043452.GI22737@tuxbook-pro/T/
> >
> > I saw nothing in this thread that I would call a rejection.
> >
> > It made the size choice based on the virtio config space which could
> > come from firmware resource table now or virtio device config space if
> > using virtio-mmio etc.
> >
> > Bjorn did point out that a system where each side allocated its own
> > buffers of whatever size desired would be more flexible but:
> > * did not object to this step
> > * that would be a pretty big departure from what we have not and we
> > would still need to negotiate its use.
> >
> > The biggest issues I saw with the patch series was the details of how
> > it did the config. That could have been fixed with a bit more effort.
> >
> > I would vote we just resurrect this patch set and clean it up rather
> > than going down the DTS route.
> >
>
> I agree - I did not find anything controversial in that patchset either. We can
> start with that and later supplement with the DTS if needed.
Also in favor of this approach
Regards,
Arnaud
>
> > Bill
> >
> > > 2. As part of this, RPMSG_BUF_SIZE is configurable on library side,
> > > but not on kernel side.
> > >
> > > If it looks reasonable to go with approach 2 without side-effects, I
> > > can develop and send the patch.
> > >
> > > Thanks,
> > >
> > > Tanmay
> > >
> >
> > --
> > Bill Mills
> > Principal Technical Consultant, Linaro
> > +1-240-643-0836
> > TZ: US Eastern
> > Work Schedule: Tues/Wed/Thur
Branch: refs/heads/main
Home: https://github.com/OpenAMP/open-amp
Commit: 1697a15f477578104e81092406fb2318a56e2f72
https://github.com/OpenAMP/open-amp/commit/1697a15f477578104e81092406fb2318…
Author: Ben Levinsky <ben.levinsky(a)amd.com>
Date: 2023-05-22 (Mon, 22 May 2023)
Changed paths:
M apps/machine/zynqmp/platform_info.c
Log Message:
-----------
apps: zynqmp: Add Versal_net IPI values.
Enable Linux demos to run on Versal_net by updating the IPI values that
are specific to this hardware platform.
Signed-off-by: Ben Levinsky <ben.levinsky(a)amd.com>
On 5/18/23 12:00 PM, Shah, Tanmay wrote:
> Hi all,
>
> As of now, rpmsg buffer size is decided by RPMSG_BUF_SIZE macro. This is
> fixed to 512 bytes.
>
> If platform needs to send larger than 512 bytes of payload, then the
> payload needs to be split between multiple buffers.
>
> Instead of that, it would be useful if platform can configure
> RPMSG_BUF_SIZE somehow.
>
> There are multiple options to achieve this:
>
> 1. Move RPMSG_BUF_SIZE to Kconfig -> The approach is not accepted as
> single kernel may not work for all platforms.
> 2. Keep device-tree property that mentions rpmsg buffer size.
> 1. Something in dts: “rpmsg-buf-size = <512>”
> 2. This way one kernel can work for all the platforms. If property
> is not mentioned, then we fallback to default size of 512 bytes.
> 3. Dynamic buffer allocation
> 1. This effort was done previously. Please refer to following
> discussion from kernel and openamp library:
>
> i.Openamp library: https://github.com/OpenAMP/open-amp/pull/155
> <https://github.com/OpenAMP/open-amp/pull/155>
>
> 1. Conclusion:
> https://github.com/OpenAMP/open-amp/issues/322#issuecomment-986573828 <https://github.com/OpenAMP/open-amp/issues/322#issuecomment-986573828>
>
> ii.Kernel:
> https://patchwork.kernel.org/project/linux-remoteproc/cover/1548949280-3179… <https://patchwork.kernel.org/project/linux-remoteproc/cover/1548949280-3179…>
>
Lore link to same:
https://lore.kernel.org/lkml/20190605043452.GI22737@tuxbook-pro/T/
I saw nothing in this thread that I would call a rejection.
It made the size choice based on the virtio config space which could
come from firmware resource table now or virtio device config space if
using virtio-mmio etc.
Bjorn did point out that a system where each side allocated its own
buffers of whatever size desired would be more flexible but:
* did not object to this step
* that would be a pretty big departure from what we have not and we
would still need to negotiate its use.
The biggest issues I saw with the patch series was the details of how it
did the config. That could have been fixed with a bit more effort.
I would vote we just resurrect this patch set and clean it up rather
than going down the DTS route.
Bill
> 2. As part of this, RPMSG_BUF_SIZE is configurable on library side,
> but not on kernel side.
>
> If it looks reasonable to go with approach 2 without side-effects, I can
> develop and send the patch.
>
> Thanks,
>
> Tanmay
>
--
Bill Mills
Principal Technical Consultant, Linaro
+1-240-643-0836
TZ: US Eastern
Work Schedule: Tues/Wed/Thur
