Re: [Qemu-devel] [PATCH v2] lsi: Reselection needed to remove pending commands from queue

[Paolo Bonzini]

On 23/10/2018 23:36, George Kennedy wrote:
> 
> 
> On 10/23/2018 10:33 AM, Paolo Bonzini wrote:
>> On 22/10/2018 23:28, George Kennedy wrote:
>>> As you suggested I moved the loading of "s->resel_dsp" down to the
>>> "Wait Reselect"
>>> case. The address of the Reselection Scripts, though, is contained in
>>> "s->dsp - 8"
>>> and not in s->dnad.
>> Are you sure?  s->dsp - 8 should be the address of the Wait Reselect
>> instruction itself.  But you're right that s->dnad is the address at
>> which to jump "if the LSI53C895A is selected before being reselected"
>> (as the spec puts it) so the reselection DSP should be just s->dsp.
> 
> See within the 1st 25 lines of lsi_execute_script() where dsp is bumped
> up by 8, "s->dsp += 8", so it needs to be adjusted back to what it was.

The spec says "If the LSI53C895A is reselected, it fetches the next
instruction from the address pointed to by the DMA
SCRIPTS Pointer (DSP) register".  The first instruction of the
reselection scripts is the one after WAIT RESELECT, i.e. s->dsp.


>> Reselection should only happen when the target needs access to the bus,
>> which is when I/O has finished.  There should be no need for such a
>> deadline; reselection should already be happening at the right time when
>> lsi_transfer_data calls lsi_queue_req, which in turn calls lsi_reselect.
> Agree that it should happen as you describe, but under heavy IO (fio),
> it does not.
> 
> When it works as expected the check for "s->waiting == 1" (Wait Reselect
> instruction has been issued) in lsi_transfer_data() is true. Under heavy
> IO, s->waiting is not "1" for an extended period of time

What about "req->hba_private != s->current"?  That should cause a call
to lsi_queue_req, and then you can check s->want_resel in lsi_queue_req.

> I am not strongly attached to my proposed fix. If an alternative fix can
> be suggested, I'd be more than willing to try that.

The problem is that the timeout has no explanation under the SCSI
protocol, so I would like to understand where the logic is wrong in the
Parallel SCSI emulation.

Paolo