On 20/01/2021 12:17, Nathan Lynch wrote:
Alexey Kardashevskiy <a...@ozlabs.ru> writes:
On 16/01/2021 02:56, Nathan Lynch wrote:
Alexey Kardashevskiy <a...@ozlabs.ru> 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.
Yup, "almost".
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.
I am not arguing keeping the numbers but you are replacing one magic
number with another and for neither it is horribly obvious where they
came from. Is 16 the max number of concurrently running sys_rtas system
calls? Does the userspace ensure there is no more than 16? btw where is
that userspace code? I thought
https://github.com/power-ras/ppc64-diag.git but no. Thanks,
--
Alexey
