• Even Xu's avatar
    hid: intel-ish-hid: ishtp: Fix ishtp client sending disordered message · e1fa0767
    Even Xu authored
    There is a timing issue captured during ishtp client sending stress tests.
    It was observed during stress tests that ISH firmware is getting out of
    ordered messages. This is a rare scenario as the current set of ISH client
    drivers don't send much data to firmware. But this may not be the case
    going forward.
    
    When message size is bigger than IPC MTU, ishtp splits the message into
    fragments and uses serialized async method to send message fragments.
    The call stack:
    ishtp_cl_send_msg_ipc->ipc_tx_callback(first fregment)->
    ishtp_send_msg(with callback)->write_ipc_to_queue->
    write_ipc_from_queue->callback->ipc_tx_callback(next fregment)......
    
    When an ipc write complete interrupt is received, driver also calls
    write_ipc_from_queue->ipc_tx_callback in ISR to start sending of next fragment.
    
    Through ipc_tx_callback uses spin_lock to protect message splitting, as the
    serialized sending method will call back to ipc_tx_callback again, so it doesn't
    put sending under spin_lock, it causes driver cannot guarantee all fragments
    be sent in order.
    
    Considering this scenario:
    ipc_tx_callback just finished a fragment splitting, and not call ishtp_send_msg
    yet, there is a write complete interrupt happens, then ISR->write_ipc_from_queue
    ->ipc_tx_callback->ishtp_send_msg->write_ipc_to_queue......
    
    Because ISR has higher exec priority than normal thread, this causes the new
    fragment be sent out before previous fragment. This disordered message causes
    invalid message to firmware.
    
    The solution is, to send fragments synchronously:
    Use ishtp_write_message writing fragments into tx queue directly one by one,
    instead of ishtp_send_msg only writing one fragment with completion callback.
    As no completion callback be used, so change ipc_tx_callback to ipc_tx_send.
    Signed-off-by: default avatarEven Xu <even.xu@intel.com>
    Acked-by: default avatarSrinivas Pandruvada <srinivas.pandruvada@intel.com>
    Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
    e1fa0767
client.c 28.7 KB