| Message ID | 20240722055437.3467900-3-quic_ekangupt@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix user PD inimem requirements | expand |
On Mon, Jul 22, 2024 at 11:24:37AM +0530, Ekansh Gupta wrote: > For unsigned PD offloading requirement, additional memory is required > because of additional static heap initialization. Without this > additional memory, PD initialization would fail. Increase the initmem > size by 2MB for unsigned PD initmem buffer allocation. Any additional > memory sent to DSP during PD init is used as the PD heap. > > Fixes: 7f1f481263c3 ("misc: fastrpc: check before loading process to the DSP") > Cc: stable <stable@kernel.org> > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > --- > drivers/misc/fastrpc.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index a3a5b745936e..18668b020a87 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -40,6 +40,7 @@ > #define FASTRPC_CTXID_MASK (0xFF0) > #define INIT_FILELEN_MAX (2 * 1024 * 1024) > #define FASTRPC_INITLEN_MIN (3 * 1024 * 1024) > +#define FASTRPC_STATIC_HEAP_LEN (2 * 1024 * 1024) > #define INIT_FILE_NAMELEN_MAX (128) > #define FASTRPC_DEVICE_NAME "fastrpc" > > @@ -1411,8 +1412,14 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > goto err; > } > > + /* Allocate buffer in kernel for donating to remote process. > + * Unsigned PD requires additional memory because of the What is "PD"? > + * additional static heap initialized within the process. > + */ Why are you using networking comment style for a non-networking file? > memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4), > 1024 * 1024); > + if (unsigned_module) > + memlen += FASTRPC_STATIC_HEAP_LEN; I don't understand, why is "static heap length" being added for something that is "unsigned"? Why isn't this just "SIGNING FREE SPACE" or something like that? thanks, greg "naming is hard" k-h
On 7/22/2024 11:30 AM, Greg KH wrote: > On Mon, Jul 22, 2024 at 11:24:37AM +0530, Ekansh Gupta wrote: >> For unsigned PD offloading requirement, additional memory is required >> because of additional static heap initialization. Without this >> additional memory, PD initialization would fail. Increase the initmem >> size by 2MB for unsigned PD initmem buffer allocation. Any additional >> memory sent to DSP during PD init is used as the PD heap. >> >> Fixes: 7f1f481263c3 ("misc: fastrpc: check before loading process to the DSP") >> Cc: stable <stable@kernel.org> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> >> --- >> drivers/misc/fastrpc.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> index a3a5b745936e..18668b020a87 100644 >> --- a/drivers/misc/fastrpc.c >> +++ b/drivers/misc/fastrpc.c >> @@ -40,6 +40,7 @@ >> #define FASTRPC_CTXID_MASK (0xFF0) >> #define INIT_FILELEN_MAX (2 * 1024 * 1024) >> #define FASTRPC_INITLEN_MIN (3 * 1024 * 1024) >> +#define FASTRPC_STATIC_HEAP_LEN (2 * 1024 * 1024) >> #define INIT_FILE_NAMELEN_MAX (128) >> #define FASTRPC_DEVICE_NAME "fastrpc" >> >> @@ -1411,8 +1412,14 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, >> goto err; >> } >> >> + /* Allocate buffer in kernel for donating to remote process. >> + * Unsigned PD requires additional memory because of the > What is "PD"? DSP PD(protection domain) is execution environment supported by DSP. > >> + * additional static heap initialized within the process. >> + */ > Why are you using networking comment style for a non-networking file? I observed similar style in this driver file. I will update this in proper style in the next patch. > >> memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4), >> 1024 * 1024); >> + if (unsigned_module) >> + memlen += FASTRPC_STATIC_HEAP_LEN; > I don't understand, why is "static heap length" being added for > something that is "unsigned"? Why isn't this just "SIGNING FREE SPACE" > or something like that? The difference between signed PD and unsigned PD is: Signed PD: Available on all DSPs and requires that the shared objects being loaded in the PD are signed with a digital signature. Unsigned PD: Sandboxed low-rights process that allows signature-free shared objects to run on CDSP. For unsigned PD there are some additional statically initialized heap for which additional memory is required. I'll try to come up with a better name. Thanks for the review. --Ekansh > > thanks, > > greg "naming is hard" k-h >
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index a3a5b745936e..18668b020a87 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -40,6 +40,7 @@ #define FASTRPC_CTXID_MASK (0xFF0) #define INIT_FILELEN_MAX (2 * 1024 * 1024) #define FASTRPC_INITLEN_MIN (3 * 1024 * 1024) +#define FASTRPC_STATIC_HEAP_LEN (2 * 1024 * 1024) #define INIT_FILE_NAMELEN_MAX (128) #define FASTRPC_DEVICE_NAME "fastrpc" @@ -1411,8 +1412,14 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, goto err; } + /* Allocate buffer in kernel for donating to remote process. + * Unsigned PD requires additional memory because of the + * additional static heap initialized within the process. + */ memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4), 1024 * 1024); + if (unsigned_module) + memlen += FASTRPC_STATIC_HEAP_LEN; err = fastrpc_buf_alloc(fl, fl->sctx->dev, memlen, &imem); if (err)
For unsigned PD offloading requirement, additional memory is required because of additional static heap initialization. Without this additional memory, PD initialization would fail. Increase the initmem size by 2MB for unsigned PD initmem buffer allocation. Any additional memory sent to DSP during PD init is used as the PD heap. Fixes: 7f1f481263c3 ("misc: fastrpc: check before loading process to the DSP") Cc: stable <stable@kernel.org> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> --- drivers/misc/fastrpc.c | 7 +++++++ 1 file changed, 7 insertions(+)