On Tue, Jan 25, 2022 at 11:58 AM <Ajish.Koshy@xxxxxxxxxxxxx> wrote: > > > > > > > For SATA devices, correct the double > > > completion issue. > > > > > > Current code handles completions for sata devices in > > > mpi_sata_completion() and mpi_sata_event(). > > > > > > But at the time when any sata event happens, for almost all the event > > > types, the command is still in the target. It is wrong to complete the > > > task in sata_event(). > > > > Is it also an issue for SSP? what is the side effect, the current code will lead to > > (w/o this patch)? > > why SATA is different? > > > > Thanks! > > Thanks Jinpu. > > Yes, same issue with the SSP event path too. We need > similar changes there also. > If that's the case, probably it should fix in same patch. > My understanding here on side effects without this patch- > Here is the expectation of the firmware. > The controller sends a sata_event notification to indicate > that the I/O is not finished (may be pending in the controller > or in the remote target) and that the host must take > additional action. The host action required depends on the > event received. > > There are cases, where commands need special handling mentioned > as per the spec. > > But for most of the event types, the host must reset the SATA > drive and abort the pending I/O in the controller by sending abort > command > > Till here inside the firmware all the resource are intact, tag, > memory, etc. > > Now in the host side, once we receive the event, host will free > its own resources, tag, dma memory, etc. by calling its own > completion. > > So at this point firmware has not freed its own the resources and > host freed its resources corresponding to the given command/tag. > > For example, in the case of IO_XFER_ERROR_NAK_RECEIVED > sata event. > Here firmware will keep retrying the command. There are > chances the corresponding command may get successful and > firmware may give response through sata completion. > > Since we received the sata_event, we will complete the command. > Host will free its own resouces, tags, dma memory, etc. At this > point when we received this event, command was still pending in > the target. > > In this case since we freed the DMA memory, while firmware > completes the command, it will generate > IO_XFER_READ_COMPL_ERR event (pcie error based) for the same tag/ > command since we already freed the DMA memory/resources. My impression was the FW should either generate SATA_EVENT or SATA_COPMLETION, but not both, was this behavior changed? Thanks! > > That's one side effect right now. > > Another instance I can see is during I/O, host will free the tag > once we receive an event, same tag/command is still pending in the > firmware and post this event > host re-allocates this tag to some other new request. This may lead > to some un-expected situation. > > That is what I can visualize right now as side effects without this > patch. > > Thanks, > Ajish >
