Re: [PATCH v4 06/13] powerpc/rtas: Serialize firmware activation sequences

[Aneesh Kumar K.V (IBM) <aneesh.kumar@kernel.org>]

Nathan Lynch <nathanl@linux.ibm.com> writes:

> "Aneesh Kumar K.V (IBM)" <aneesh.kumar@kernel.org> writes:
>> Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm.com@kernel.org>
>> writes:
>>
>>>
>>> Use the function lock API to prevent interleaving call sequences of
>>> the ibm,activate-firmware RTAS function, which typically requires
>>> multiple calls to complete the update. While the spec does not
>>> specifically prohibit interleaved sequences, there's almost certainly
>>> no advantage to allowing them.
>>>
>>
>> Can we document what is the equivalent thing the userspace does?
>
> I'm not sure what we would document.
>
> As best I can tell, the activate_firmware command in powerpc-utils does
> not make any effort to protect its use of the ibm,activate-firmware RTAS
> function. The command is not intended to be run manually and I guess
> it's relying on the platform's management console to serialize its
> invocations.
>
> drmgr (also from powerpc-utils) has some dead code for LPM that calls
> ibm,activate-firmware; it should probably be removed. The command uses a
> lock file to serialize all of its executions.
>
> Something that could happen with interleaved ibm,activate-firmware
> sequences is something like this:
>
> 1. Process A initiates an ibm,activate-firmware sequence and receives a
>    "retry" status (-2/990x).
> 2. Process B calls ibm,activate-firmware and receives the "done" status
>    (0), concluding the sequence A began.
> 3. Process A, unaware of B, calls ibm,activate-firmware again,
>    inadvertently beginning a new sequence.
>

So this patch won't protect us against a parallel userspace invocation.
We can add static bool call_in_progress to track the ongoing
ibm,activate-firmware call from userspace? My only concern is we are
adding locks to protect against parallel calls in the kernel, but at the
same time, we ignore any userspace call regarding the same. We should at
least document this if this is not important to be fixed.

-aneesh