Hi Phil, > You didn't Cc'ed the maintainers of the SCSI subsystem (see > https://wiki.qemu.org/Contribute/SubmitAPatch#CC_the_relevant_maintainer > ) so I'm doing it for you: Thank you!
> It seems you didn't send your patch with the proper tool, see > https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch > > Please also mention the reporter: > > Reported-by: Cheolwoo Myung <cwmy...@snu.ac.kr> > > Also your git-config is missing your name. Fixable using: > > $ git config user.name "Liu Cyrus" > > for your QEMU repository, or globally: > > $ git config --global user.name "Liu Cyrus" Thank you again. I'm sorry that I do miss several basic settings. I will do better next time. > Instead of duplicating multiple times the same complex code, you could > add a helper once and call it. This is really a good idea. > However back to the bug you are trying to fix, I wonder if your check > is correct regarding the hardware we are modeling. Have you looked > at the specifications? See https://docs.broadcom.com/doc/12352475 > Chapter 5.3 Block Move Instruction (from SCSI SCRIPTS Instruction Set), > "DMA Command" register. To be honest, I didn't check the specification at that time. I formed this patch following the idea that we could discard an invalid MMIO operation to avoid crashing. Does it seem that this idea is not enough to form a good patch? What are the best practices to fix an assertion failure in QEMU? > Why are we reaching these places with s->current == NULL in the first > place? We are probably missing something earlier. I checked the specification for several hours today, but it is too difficult for me. I need more time to understand it and form a better patch. When reproducing the crash, I found that I didn't prepare any script to be executed by lsi_execute_script. However, `insn = read_dword(s, s->dsp)` did get an instruction at `s->dsp`. Maybe I should check this more. Best, Qiang