"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. > 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. It's not accurate to say we're ignoring user space calls. Patch 5/13 makes it so that sys_rtas(ibm,activate-firmware) will serialize on the same lock used here.
