[PATCH v5 0/2] Fix user PD inimem requirements

[PATCH v5 0/2] Fix user PD inimem requirements

[Ekansh Gupta]

This patch series fixes the incorrect initmem size assumptions for
signed and unsigned user PD.
Previous patch: https://lore.kernel.org/all/20240719085708.1764952-1-quic_ekangupt@quicinc.com/

Changes in v2:
  - Modified commit text.
  - Removed size check instead of updating max file size.
Changes in v3:
  - Added bound check again with a higher max size definition.
  - Modified commit text accordingly.
Changes in v4:
  - Defined new initmem specific MACROs.
  - Adding extra memory for unsigned PD.
  - Added comment suggesting the reason for this change.
  - Modified commit text.
Changes in v5:
  - Splitted the change into separate patches.

Ekansh Gupta (2):
  misc: fastrpc: Define a new initmem size for user PD
  misc: fastrpc: Increase unsigned PD initmem size

 drivers/misc/fastrpc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Ekansh Gupta]

For user PD initialization, initmem is allocated and sent to DSP for
initial memory requirements like shell loading. The size of this memory
is decided based on the shell size that is passed by the user space.
With the current implementation, a minimum of 2MB is always allocated
for initmem even if the size passed by user is less than that. For this
a MACRO is being used which is intended for shell size bound check.
This minimum size of 2MB is not recommended as the PD will have very
less memory for heap and will have to request HLOS again for memory.
Define a new macro for initmem minimum length of 3MB.

Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
Cc: stable <stable@kernel.org>
Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
 drivers/misc/fastrpc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index a7a2bcedb37e..a3a5b745936e 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -39,6 +39,7 @@
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
 #define FASTRPC_CTXID_MASK (0xFF0)
 #define INIT_FILELEN_MAX (2 * 1024 * 1024)
+#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
 #define INIT_FILE_NAMELEN_MAX (128)
 #define FASTRPC_DEVICE_NAME	"fastrpc"
 
@@ -1410,7 +1411,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
 			goto err;
 	}
 
-	memlen = ALIGN(max(INIT_FILELEN_MAX, (int)init.filelen * 4),
+	memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4),
 		       1024 * 1024);
 	err = fastrpc_buf_alloc(fl, fl->sctx->dev, memlen,
 				&imem);
-- 
2.34.1

Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Greg KH]

On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
> For user PD initialization, initmem is allocated and sent to DSP for
> initial memory requirements like shell loading. The size of this memory
> is decided based on the shell size that is passed by the user space.
> With the current implementation, a minimum of 2MB is always allocated
> for initmem even if the size passed by user is less than that. For this
> a MACRO is being used which is intended for shell size bound check.
> This minimum size of 2MB is not recommended as the PD will have very
> less memory for heap and will have to request HLOS again for memory.
> Define a new macro for initmem minimum length of 3MB.
> 
> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
> Cc: stable <stable@kernel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
>  drivers/misc/fastrpc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index a7a2bcedb37e..a3a5b745936e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -39,6 +39,7 @@
>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
>  #define FASTRPC_CTXID_MASK (0xFF0)
>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)

Meta-comment, for a future change, why not tabs to line things up?

>  #define INIT_FILE_NAMELEN_MAX (128)
>  #define FASTRPC_DEVICE_NAME	"fastrpc"
>  
> @@ -1410,7 +1411,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  			goto err;
>  	}
>  
> -	memlen = ALIGN(max(INIT_FILELEN_MAX, (int)init.filelen * 4),
> +	memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4),

You are aligning it this way, but still checking the file size for
INIT_FILELEN_MAX, so that's not really going to change anything here.

How was this tested?

thanks,

greg k-h

Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Ekansh Gupta]

On 7/22/2024 11:28 AM, Greg KH wrote:
> On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
>> For user PD initialization, initmem is allocated and sent to DSP for
>> initial memory requirements like shell loading. The size of this memory
>> is decided based on the shell size that is passed by the user space.
>> With the current implementation, a minimum of 2MB is always allocated
>> for initmem even if the size passed by user is less than that. For this
>> a MACRO is being used which is intended for shell size bound check.
>> This minimum size of 2MB is not recommended as the PD will have very
>> less memory for heap and will have to request HLOS again for memory.
>> Define a new macro for initmem minimum length of 3MB.
>>
>> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
>> Cc: stable <stable@kernel.org>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>>  drivers/misc/fastrpc.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index a7a2bcedb37e..a3a5b745936e 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -39,6 +39,7 @@
>>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
>>  #define FASTRPC_CTXID_MASK (0xFF0)
>>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
>> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
> Meta-comment, for a future change, why not tabs to line things up?
Sure, I'll add a comment.
Should I line up all the MACRO definitions? If yes, should I send it as a separate patch?
>
>>  #define INIT_FILE_NAMELEN_MAX (128)
>>  #define FASTRPC_DEVICE_NAME	"fastrpc"
>>  
>> @@ -1410,7 +1411,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>>  			goto err;
>>  	}
>>  
>> -	memlen = ALIGN(max(INIT_FILELEN_MAX, (int)init.filelen * 4),
>> +	memlen = ALIGN(max(FASTRPC_INITLEN_MIN, (int)init.filelen * 4),
> You are aligning it this way, but still checking the file size for
> INIT_FILELEN_MAX, so that's not really going to change anything here.
File size is for the fastrpc shell that is to be loaded. This is the size of buffer allocated by user and passed
to the driver. The driver will map this buffer to SMMU before passing it to DSP. Therefore the bound
check on this size is necessary. As for initmem size for remote process on DSP, this size is also inclusive of
initial requirements of DSP size user process(user PD), hence separating it from file size macro.
> How was this tested?
This is tested with fastrpc use cases available in hexagon SDK:
https://developer.qualcomm.com/software/hexagon-dsp-sdk/sample-apps

--Ekansh
> thanks,
>
> greg k-h

Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Greg KH]

On Mon, Jul 22, 2024 at 11:42:52AM +0530, Ekansh Gupta wrote:
> 
> 
> On 7/22/2024 11:28 AM, Greg KH wrote:
> > On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
> >> For user PD initialization, initmem is allocated and sent to DSP for
> >> initial memory requirements like shell loading. The size of this memory
> >> is decided based on the shell size that is passed by the user space.
> >> With the current implementation, a minimum of 2MB is always allocated
> >> for initmem even if the size passed by user is less than that. For this
> >> a MACRO is being used which is intended for shell size bound check.
> >> This minimum size of 2MB is not recommended as the PD will have very
> >> less memory for heap and will have to request HLOS again for memory.
> >> Define a new macro for initmem minimum length of 3MB.
> >>
> >> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
> >> Cc: stable <stable@kernel.org>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >>  drivers/misc/fastrpc.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> >> index a7a2bcedb37e..a3a5b745936e 100644
> >> --- a/drivers/misc/fastrpc.c
> >> +++ b/drivers/misc/fastrpc.c
> >> @@ -39,6 +39,7 @@
> >>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
> >>  #define FASTRPC_CTXID_MASK (0xFF0)
> >>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
> >> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
> > Meta-comment, for a future change, why not tabs to line things up?
> Sure, I'll add a comment.

I didn't say anything about comments :(

> Should I line up all the MACRO definitions? If yes, should I send it as a separate patch?

As I said, yes, for a future change.

> > How was this tested?
> This is tested with fastrpc use cases available in hexagon SDK:
> https://developer.qualcomm.com/software/hexagon-dsp-sdk/sample-apps

Do you have regression tests that attempt to check the boundry
conditions and alignment here?

thanks,

greg k-h

Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Ekansh Gupta]

On 7/22/2024 1:09 PM, Greg KH wrote:
> On Mon, Jul 22, 2024 at 11:42:52AM +0530, Ekansh Gupta wrote:
>>
>> On 7/22/2024 11:28 AM, Greg KH wrote:
>>> On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
>>>> For user PD initialization, initmem is allocated and sent to DSP for
>>>> initial memory requirements like shell loading. The size of this memory
>>>> is decided based on the shell size that is passed by the user space.
>>>> With the current implementation, a minimum of 2MB is always allocated
>>>> for initmem even if the size passed by user is less than that. For this
>>>> a MACRO is being used which is intended for shell size bound check.
>>>> This minimum size of 2MB is not recommended as the PD will have very
>>>> less memory for heap and will have to request HLOS again for memory.
>>>> Define a new macro for initmem minimum length of 3MB.
>>>>
>>>> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
>>>> Cc: stable <stable@kernel.org>
>>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>> ---
>>>>  drivers/misc/fastrpc.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>>>> index a7a2bcedb37e..a3a5b745936e 100644
>>>> --- a/drivers/misc/fastrpc.c
>>>> +++ b/drivers/misc/fastrpc.c
>>>> @@ -39,6 +39,7 @@
>>>>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
>>>>  #define FASTRPC_CTXID_MASK (0xFF0)
>>>>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
>>>> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
>>> Meta-comment, for a future change, why not tabs to line things up?
>> Sure, I'll add a comment.
> I didn't say anything about comments :(
Oops, sorry.
>
>> Should I line up all the MACRO definitions? If yes, should I send it as a separate patch?
> As I said, yes, for a future change.
Noted, thanks.
>>> How was this tested?
>> This is tested with fastrpc use cases available in hexagon SDK:
>> https://developer.qualcomm.com/software/hexagon-dsp-sdk/sample-apps
> Do you have regression tests that attempt to check the boundry
> conditions and alignment here?
For most of the test cases, I used the fastrpc lib:
https://github.com/quic/fastrpc

This library is taking care of passing proper shell size which is within the boundary for
all the platform that I've tried.
I'll try creating and running some regression tests for this change.

--Ekansh
>
> thanks,
>
> greg k-h

Re: [PATCH v5 1/2] misc: fastrpc: Define a new initmem size for user PD

[Dmitry Baryshkov]

On Mon, Jul 22, 2024 at 01:23:56PM GMT, Ekansh Gupta wrote:
> 
> 
> On 7/22/2024 1:09 PM, Greg KH wrote:
> > On Mon, Jul 22, 2024 at 11:42:52AM +0530, Ekansh Gupta wrote:
> >>
> >> On 7/22/2024 11:28 AM, Greg KH wrote:
> >>> On Mon, Jul 22, 2024 at 11:24:36AM +0530, Ekansh Gupta wrote:
> >>>> For user PD initialization, initmem is allocated and sent to DSP for
> >>>> initial memory requirements like shell loading. The size of this memory
> >>>> is decided based on the shell size that is passed by the user space.
> >>>> With the current implementation, a minimum of 2MB is always allocated
> >>>> for initmem even if the size passed by user is less than that. For this
> >>>> a MACRO is being used which is intended for shell size bound check.
> >>>> This minimum size of 2MB is not recommended as the PD will have very
> >>>> less memory for heap and will have to request HLOS again for memory.
> >>>> Define a new macro for initmem minimum length of 3MB.
> >>>>
> >>>> Fixes: d73f71c7c6ee ("misc: fastrpc: Add support for create remote init process")
> >>>> Cc: stable <stable@kernel.org>
> >>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >>>> ---
> >>>>  drivers/misc/fastrpc.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> >>>> index a7a2bcedb37e..a3a5b745936e 100644
> >>>> --- a/drivers/misc/fastrpc.c
> >>>> +++ b/drivers/misc/fastrpc.c
> >>>> @@ -39,6 +39,7 @@
> >>>>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
> >>>>  #define FASTRPC_CTXID_MASK (0xFF0)
> >>>>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
> >>>> +#define FASTRPC_INITLEN_MIN (3 * 1024 * 1024)
> >>> Meta-comment, for a future change, why not tabs to line things up?
> >> Sure, I'll add a comment.
> > I didn't say anything about comments :(
> Oops, sorry.
> >
> >> Should I line up all the MACRO definitions? If yes, should I send it as a separate patch?
> > As I said, yes, for a future change.
> Noted, thanks.
> >>> How was this tested?
> >> This is tested with fastrpc use cases available in hexagon SDK:
> >> https://developer.qualcomm.com/software/hexagon-dsp-sdk/sample-apps
> > Do you have regression tests that attempt to check the boundry
> > conditions and alignment here?
> For most of the test cases, I used the fastrpc lib:
> https://github.com/quic/fastrpc
> 
> This library is taking care of passing proper shell size which is within the boundary for
> all the platform that I've tried.
> I'll try creating and running some regression tests for this change.

Existing userspace implementation provides an example of good behaviour.
Please consider implenting and publishing 'bad' behaviour testcases
which make sure that the driver doesn't break if it gets passed 'bad'
data.

-- 
With best wishes
Dmitry

[PATCH v5 2/2] misc: fastrpc: Increase unsigned PD initmem size

[Ekansh Gupta]

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
+	 * 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)
-- 
2.34.1

Re: [PATCH v5 2/2] misc: fastrpc: Increase unsigned PD initmem size

[Greg KH]

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

Re: [PATCH v5 2/2] misc: fastrpc: Increase unsigned PD initmem size

[Ekansh Gupta]

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
>