Re: [PATCH 10/13] powerpc/rtas: improve function information lookups

[Nathan Lynch <nathanl@linux.ibm.com>]

Andrew Donnellan <ajd@linux.ibm.com> writes:
> On Wed, 2022-11-23 at 13:32 -0600, Nick Child wrote:
>> On 11/22/22 20:51, Andrew Donnellan wrote:
>> > On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
>> > > +enum rtas_function_flags {
>> > > +       RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE = (1 << 0),
>> > > +};
>> > 
>> > This seems to be new, what's the justification?
>> > 
>> 
>> Seems to be a run-time replacement of:
>> #ifdef CONFIG_CPU_BIG_ENDIAN
>>         { "ibm,suspend-me", -1, -1, -1, -1, -1 },
>>         { "ibm,update-nodes", -1, 0, -1, -1, -1, 4096 },
>>         { "ibm,update-properties", -1, 0, -1, -1, -1, 4096 },
>> #endif
>> 
>> It looks to be handled logically:
>> + if (IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) &&
>> +           (func->flags & RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE))
>> +               goto err;
>> 
>> Perhaps, also allow the addition of any future special cases
>> for rtas functions easier to maintain?
>
> Makes sense, though I'm slightly confused about the original rationale
> for the ifdef and why it's not being fixed in userspace.

Nick C's explanation is correct. I will make the commit message more
explicit about the conversion, and document the flag in the code.

The original rationale:

commit de0f7349a0dd072e54b5fc04c305907b22d28a5f
Author: Nathan Lynch <nathanl@linux.ibm.com>
Date:   Mon Dec 7 15:51:33 2020 -0600

    powerpc/rtas: prevent suspend-related sys_rtas use on LE

    While drmgr has had work in some areas to make its RTAS syscall
    interactions endian-neutral, its code for performing partition
    migration via the syscall has never worked on LE. While it is able to
    complete ibm,suspend-me successfully, it crashes when attempting the
    subsequent ibm,update-nodes call.

    drmgr is the only known (or plausible) user of ibm,suspend-me,
    ibm,update-nodes, and ibm,update-properties, so allow them only in
    big-endian configurations.

To summarize: we know these functions have never had working users via
sys_rtas on ppc64le, and we want to keep it that way.

> Slightly clunky name though, something like
> RTAS_FN_FLAG_SYSCALL_BE_ONLY might be less clunky?

RTAS_FN_FLAG_BANNED_FOR_SYSCALL_ON_LE is verbose, but I think it
communicates better that we are consciously imposing a policy in a
specific context.