Subject: Re: [PATCH 5/6] powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE

From: Nathan Lynch <[email-masked]>
Date: Tue, 19 Jan 2021 19:17:13 -0600

Alexey Kardashevskiy <[email-masked]> writes:
> On 16/01/2021 02:56, Nathan Lynch wrote:
>> Alexey Kardashevskiy <[email-masked]> writes:
>>> On 15/01/2021 09:00, Nathan Lynch wrote:
>>>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>>>> index 332e1000ca0f..1aa7ab1cbc84 100644
>>>> --- a/arch/powerpc/include/asm/rtas.h
>>>> +++ b/arch/powerpc/include/asm/rtas.h
>>>> @@ -19,8 +19,11 @@
>>>>    #define RTAS_UNKNOWN_SERVICE (-1)
>>>>    #define RTAS_INSTANTIATE_MAX (1ULL<<30) /* Don't instantiate rtas at/above this value */
>>>>    
>>>> -/* Buffer size for ppc_rtas system call. */
>>>> -#define RTAS_RMOBUF_MAX (64 * 1024)
>>>> +/* Work areas shared with RTAS must be 4K, naturally aligned. */
>>>
>>> Why exactly 4K and not (for example) PAGE_SIZE?
>> 
>> 4K is a platform requirement and isn't related to Linux's configured
>> page size. See the PAPR specification for RTAS functions such as
>> ibm,configure-connector, ibm,update-nodes, ibm,update-properties.
>
> Good, since we are documenting things here - add to the comment ("per 
> PAPR")?

But almost every constant in this header relates to a specification or
requirement in PAPR.

>> There are other calls with work area parameters where alignment isn't
>> specified (e.g. ibm,get-system-parameter) but 4KB alignment is a safe
>> choice for those.
>> 
>>>> +#define RTAS_WORK_AREA_SIZE   4096
>>>> +
>>>> +/* Work areas allocated for user space access. */
>>>> +#define RTAS_USER_REGION_SIZE (RTAS_WORK_AREA_SIZE * 16)
>>>
>>> This is still 64K but no clarity why. There is 16 of something, what
>>> is it?
>> 
>> There are 16 4KB work areas in the region. I can name it
>> RTAS_NR_USER_WORK_AREAS or similar.
>
>
> Why 16? PAPR (then add "per PAPR") or we just like 16 ("should be
> enough")?

PAPR doesn't know anything about the user region; it's a Linux
construct. It's been 64KB since pre-git days and I'm not sure what the
original reason is. At this point, maintaining a kernel-user ABI seems
like enough justification for the value.