Nathan Lynch <nath...@linux.ibm.com> writes: > "Aneesh Kumar K.V (IBM)" <aneesh.ku...@kernel.org> writes: > >> Nathan Lynch <nath...@linux.ibm.com> writes: >> >>> "Aneesh Kumar K.V (IBM)" <aneesh.ku...@kernel.org> writes: >>>> Nathan Lynch via B4 Relay <devnull+nathanl.linux.ibm....@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. > > It does protect in-kernel sequences from disruption by sys_rtas-based > sequences. Patch 5/13 "Facilitate high-level call sequences" makes it so > sys_rtas-based invocations of ibm,activate-firmware acquire > rtas_ibm_activate_firmware_lock. > >> We can add static bool call_in_progress to track the ongoing >> ibm,activate-firmware call from userspace? > > We can't reliably maintain any such state in the kernel. A user of > sys_rtas could exit with a sequence in progress, or it could simply > decline to complete a sequence it has initiated for any reason. This is > one of the fundamental problems with directly exposing more complex RTAS > functions to user space.
That said, I should resurrect "powerpc/rtas: consume retry statuses in sys_rtas()": https://lore.kernel.org/linuxppc-dev/20230220-rtas-queue-for-6-4-v1-8-010e4416f...@linux.ibm.com/ That ought to have the effect of perfectly serializing all ibm,activate-firmware sequences regardless of how they're initiated. But I'd like to leave that until later instead of adding to this series.
