SLES10 RC2 install, and other OSes have triggered the following error from the
LSI device:
lsi_scsi: error: Reselect with pending DMA
After some debugging, I noticed that in the lsi code, the phase would change
from PHASE_DO which corresponded to the current Write command we were handling,
to PHASE_DI. After tracking down all of the places in lsi were we phase change
to PHASE_DI, lsi_do_command was triggering this change after calling send
command. This was a most odd code path. Then I saw it:
lsi_do_command()
device->send_command()
scsi_command_complete()
lsi_command_complete()
lsi_resume_script()
lsi_do_command()
This can be detected by tracking s->current_tag when we start lsi_do_command()
and checking against that once we return from send_command(). If the tags are
different, then the previous command has completed and we exit the function.
A similar reentrance bug happens in scsi-disk.c.
scsi_send_command()
scsi_command_complete() // we remove the request and put it on the freelist
lsi_command_complete()
lsi_resume_script()
lsi_do_command()
scsi_send_command() // fetchings recently free'd request
// and updates r->sector_count
// clobbering the value that was being
// used by the previous user of
// this request
As with the lsi device, we can test for this by comparing the tag value from the
start of the function to the request tag (r->tag) and if they differ, then the
current function can exit as that command has already completed.
Fixing these two re-entrance issues allows SLES10 SP2 installer to complete and
guest function fully.
Signed-off-by: Ryan Harper <ryanh@xxxxxxxxxx>
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 912d7d5..34f50df 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -668,6 +668,7 @@ static void lsi_do_command(LSIState *s)
{
uint8_t buf[16];
int n;
+ uint32_t tag = s->current_tag;
DPRINTF("Send command len=%d\n", s->dbc);
if (s->dbc > 16)
@@ -677,6 +678,15 @@ static void lsi_do_command(LSIState *s)
s->command_complete = 0;
n = s->current_dev->send_command(s->current_dev, s->current_tag, buf,
s->current_lun);
+ /* send_command may trigger a completion and call lsi_command_complete()
+ * which can resume SCRIPTS without returning here and actually complete
+ * this current tag. We check the current_tag against a copy when we
+ * started this function and if we don't match, just exit the function. */
+ if (tag != s->current_tag) {
+ DPRINTF("%s: tag=0x%x already completed.\n");
+ return;
+ }
+
if (n > 0) {
lsi_set_phase(s, PHASE_DI);
s->current_dev->read_data(s->current_dev, s->current_tag);
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 744573e..4fa09a2 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -785,6 +785,12 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
}
if (r->sector_count == 0 && r->buf_len == 0) {
scsi_command_complete(r, STATUS_GOOD, SENSE_NO_SENSE);
+ /* scsi_send_command can nest since scsi_command_complete calls
+ * the driver completion function which may proceed to call
+ * send_command a second time. If that happens the current
+ * task has completed and we can just exit send_command() */
+ if (tag != r->tag)
+ return 0;
}
len = r->sector_count * 512 + r->buf_len;
if (is_write) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
