Some SCSI drivers like virtio have an internal mapping for the
host_status. This patch moves the host_status translation into
the SCSI drivers to allow those drivers to set up the correct
values.
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
hw/scsi/esp.c | 10 ++++++++++
hw/scsi/lsi53c895a.c | 11 +++++++++++
hw/scsi/megasas.c | 9 +++++++++
hw/scsi/mptsas.c | 9 +++++++++
hw/scsi/scsi-disk.c | 10 ++++------
hw/scsi/scsi-generic.c | 8 +++-----
hw/scsi/spapr_vscsi.c | 12 +++++++++++-
hw/scsi/virtio-scsi.c | 41 +++++++++++++++++++++++++++++++++++++++--
hw/scsi/vmw_pvscsi.c | 25 +++++++++++++++++++++++++
include/hw/scsi/scsi.h | 3 ++-
10 files changed, 123 insertions(+), 15 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 93d9c9c7b9..fc88cfac23 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -28,6 +28,8 @@
#include "migration/vmstate.h"
#include "hw/irq.h"
#include "hw/scsi/esp.h"
+#include "scsi/utils.h"
+#include "scsi/constants.h"
#include "trace.h"
#include "qemu/log.h"
#include "qemu/module.h"
@@ -489,6 +491,14 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
{
ESPState *s = req->hba_private;
+ if (req->host_status != SCSI_HOST_OK) {
+ SCSISense sense;
+
+ req->status = scsi_sense_from_host_status(req->host_status, &sense);
+ if (req->status == CHECK_CONDITION) {
+ scsi_req_build_sense(req, sense);
+ }
+ }
if (s->rregs[ESP_RSTAT] & STAT_INT) {
/* Defer handling command complete until the previous
* interrupt has been handled.
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index a4e58580e4..b6aa98c95a 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -18,6 +18,8 @@
#include "hw/irq.h"
#include "hw/pci/pci.h"
#include "hw/scsi/scsi.h"
+#include "scsi/utils.h"
+#include "scsi/constants.h"
#include "migration/vmstate.h"
#include "sysemu/dma.h"
#include "qemu/log.h"
@@ -792,6 +794,15 @@ static void lsi_command_complete(SCSIRequest *req, size_t resid)
LSIState *s = LSI53C895A(req->bus->qbus.parent);
int out;
+ if (req->host_status != SCSI_HOST_OK) {
+ SCSISense sense;
+
+ req->status = scsi_sense_from_host_status(req->host_status, &sense);
+ if (req->status == CHECK_CONDITION) {
+ scsi_req_build_sense(req, sense);
+ }
+ }
+
out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
trace_lsi_command_complete(req->status);
s->status = req->status;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 35867dbd40..1f7d806ffa 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1857,6 +1857,15 @@ static void megasas_command_complete(SCSIRequest *req, size_t resid)
MegasasCmd *cmd = req->hba_private;
uint8_t cmd_status = MFI_STAT_OK;
+ if (req->host_status != SCSI_HOST_OK) {
+ SCSISense sense;
+
+ req->status = scsi_sense_from_host_status(req->host_status, &sense);
+ if (req->status == CHECK_CONDITION) {
+ scsi_req_build_sense(req, sense);
+ }
+ }
+
trace_megasas_command_complete(cmd->index, req->status, resid);
if (req->io_canceled) {
diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index d4fbfb2da7..be3875ce94 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1143,6 +1143,15 @@ static void mptsas_command_complete(SCSIRequest *sreq,
hwaddr sense_buffer_addr = req->dev->sense_buffer_high_addr |
req->scsi_io.SenseBufferLowAddr;
+ if (sreq->host_status == SCSI_HOST_OK) {
+ SCSISense sense;
+
+ sreq->status = scsi_sense_from_host_status(sreq->host_status, &sense);
+ if (sreq->status == CHECK_CONDITION) {
+ scsi_req_build_sense(sreq, sense);
+ }
+ }
+
trace_mptsas_command_complete(s, req->scsi_io.MsgContext,
sreq->status, resid);
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 6eb0aa3d27..c0cb63707d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1840,7 +1840,7 @@ static void scsi_disk_emulate_write_data(SCSIRequest *req)
case VERIFY_10:
case VERIFY_12:
case VERIFY_16:
- if (r->req.status == -1) {
+ if (r->req.status == GOOD) {
scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
}
break;
@@ -2122,7 +2122,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
}
illegal_request:
- if (r->req.status == -1) {
+ if (r->req.status == GOOD) {
scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
}
return 0;
@@ -2697,10 +2697,8 @@ static void scsi_block_sgio_complete(void *opaque, int ret)
scsi_req_build_sense(&r->req, sense);
} else if (status == GOOD &&
io_hdr.host_status != SCSI_HOST_OK) {
- status = scsi_sense_from_host_status(io_hdr.host_status, &sense);
- if (status == CHECK_CONDITION) {
- scsi_req_build_sense(&r->req, sense);
- }
+ status = INTERMEDIATE_GOOD;
+ r->req.host_status = io_hdr.host_status;
} else if (io_hdr.status == CHECK_CONDITION ||
io_hdr.driver_status & SG_ERR_DRIVER_SENSE) {
status = CHECK_CONDITION;
diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index a2b85678b5..eac108fc6e 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -86,11 +86,9 @@ static void scsi_command_complete_noio(SCSIGenericReq *r, int ret)
if (status == CHECK_CONDITION) {
scsi_req_build_sense(&r->req, sense);
} else if (status == GOOD &&
- io_hdr.host_status != SCSI_HOST_OK) {
- status = scsi_sense_from_host_status(io_hdr.host_status, &sense);
- if (status == CHECK_CONDITION) {
- scsi_req_build_sense(&r->req, sense);
- }
+ io_hdr.host_status != SCSI_HOST_OK) {
+ status = INTERMEDIATE_GOOD;
+ r->req.host_status = io_hdr.host_status;
} else if (io_hdr.status == CHECK_CONDITION ||
io_hdr.driver_status & SG_ERR_DRIVER_SENSE) {
status = CHECK_CONDITION;
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index d653b5a6ad..9327a493c3 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -556,6 +556,16 @@ static void vscsi_command_complete(SCSIRequest *sreq, size_t resid)
VSCSIState *s = VIO_SPAPR_VSCSI_DEVICE(sreq->bus->qbus.parent);
vscsi_req *req = sreq->hba_private;
int32_t res_in = 0, res_out = 0;
+ uint8_t status = sreq->status;
+
+ if (sreq->host_status != SCSI_HOST_OK) {
+ SCSISense sense;
+
+ sreq->status = scsi_sense_from_host_status(sreq->host_status, &sense);
+ if (sreq->status == CHECK_CONDITION) {
+ scsi_req_build_sense(sreq, sense);
+ }
+ }
trace_spapr_vscsi_command_complete(sreq->tag, sreq->status, req);
if (req == NULL) {
@@ -575,7 +585,7 @@ static void vscsi_command_complete(SCSIRequest *sreq, size_t resid)
}
trace_spapr_vscsi_command_complete_status(sreq->status);
- if (sreq->status == 0) {
+ if (sreq->status == GOOD) {
/* We handle overflows, not underflows for normal commands,
* but hopefully nobody cares
*/
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 64cd852d82..62c56713d8 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -504,8 +504,45 @@ static void virtio_scsi_command_complete(SCSIRequest *r, size_t resid)
return;
}
- req->resp.cmd.response = VIRTIO_SCSI_S_OK;
- req->resp.cmd.status = r->status;
+ if (r->host_status != SCSI_HOST_OK) {
+ req->resp.cmd.status = GOOD;
+ switch (r->host_status) {
+ case SCSI_HOST_NO_LUN:
+ req->resp.cmd.response = VIRTIO_SCSI_S_INCORRECT_LUN;
+ break;
+ case SCSI_HOST_BUSY:
+ req->resp.cmd.response = VIRTIO_SCSI_S_BUSY;
+ break;
+ case SCSI_HOST_TIME_OUT:
+ case SCSI_HOST_ABORTED:
+ req->resp.cmd.response = VIRTIO_SCSI_S_ABORTED;
+ break;
+ case SCSI_HOST_BAD_RESPONSE:
+ req->resp.cmd.response = VIRTIO_SCSI_S_BAD_TARGET;
+ break;
+ case SCSI_HOST_RESET:
+ req->resp.cmd.response = VIRTIO_SCSI_S_RESET;
+ break;
+ case SCSI_HOST_TRANSPORT_DISRUPTED:
+ req->resp.cmd.response = VIRTIO_SCSI_S_TRANSPORT_FAILURE;
+ break;
+ case SCSI_HOST_TARGET_FAILURE:
+ req->resp.cmd.response = VIRTIO_SCSI_S_TARGET_FAILURE;
+ break;
+ case SCSI_HOST_RESERVATION_ERROR:
+ req->resp.cmd.response = VIRTIO_SCSI_S_NEXUS_FAILURE;
+ break;
+ case SCSI_HOST_ALLOCATION_FAILURE:
+ case SCSI_HOST_MEDIUM_ERROR:
+ case SCSI_HOST_ERROR:
+ default:
+ req->resp.cmd.response = VIRTIO_SCSI_S_FAILURE;
+ break;
+ }
+ } else {
+ req->resp.cmd.response = VIRTIO_SCSI_S_OK;
+ req->resp.cmd.status = r->status;
+ }
if (req->resp.cmd.status == GOOD) {
req->resp.cmd.resid = virtio_tswap32(vdev, resid);
} else {
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 0da378ed50..2583105dfd 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -522,6 +522,31 @@ pvscsi_command_complete(SCSIRequest *req, size_t resid)
}
s = pvscsi_req->dev;
+ if (req->host_status != SCSI_HOST_OK) {
+ switch (req->host_status) {
+ case SCSI_HOST_NO_LUN:
+ pvscsi_req->cmp.hostStatus = BTSTAT_LUNMISMATCH;
+ break;
+ case SCSI_HOST_BUSY:
+ pvscsi_req->cmp.hostStatus = BTSTAT_ABORTQUEUE;
+ break;
+ case SCSI_HOST_TIME_OUT:
+ case SCSI_HOST_ABORTED:
+ pvscsi_req->cmp.hostStatus = BTSTAT_SENTRST;
+ break;
+ case SCSI_HOST_BAD_RESPONSE:
+ pvscsi_req->cmp.hostStatus = BTSTAT_SELTIMEO;
+ break;
+ case SCSI_HOST_RESET:
+ pvscsi_req->cmp.hostStatus = BTSTAT_BUSRESET;
+ break;
+ default:
+ pvscsi_req->cmp.hostStatus = BTSTAT_HASOFTWARE;
+ break;
+ }
+ req->status = GOOD;
+ }
+
if (resid) {
/* Short transfer. */
trace_pvscsi_command_complete_data_run();
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 23a9b23e50..a097fecb2a 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -27,7 +27,8 @@ struct SCSIRequest {
uint32_t refcount;
uint32_t tag;
uint32_t lun;
- uint32_t status;
+ uint8_t status;
+ uint8_t host_status;
void *hba_private;
size_t resid;
SCSICommand cmd;
--
2.16.4
On 16/11/20 19:40, Hannes Reinecke wrote:
>
> + if (sreq->host_status == SCSI_HOST_OK) {
> + SCSISense sense;
> +
> + sreq->status = scsi_sense_from_host_status(sreq->host_status, &sense);
> + if (sreq->status == CHECK_CONDITION) {
> + scsi_req_build_sense(sreq, sense);
> + }
> + }
Should be != of course.
Paolo
On 11/16/20 7:58 PM, Paolo Bonzini wrote:
> On 16/11/20 19:40, Hannes Reinecke wrote:
>> + if (sreq->host_status == SCSI_HOST_OK) {
>> + SCSISense sense;
>> +
>> + sreq->status = scsi_sense_from_host_status(sreq->host_status,
>> &sense);
>> + if (sreq->status == CHECK_CONDITION) {
>> + scsi_req_build_sense(sreq, sense);
>> + }
>> + }
>
> Should be != of course.
>
No.
scsi_req_build_sense() transfers the sense code from the second argument
into a proper SCSI sense. Which is only set if the status is
CHECK_CONDITION...
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 16/11/20 20:05, Hannes Reinecke wrote:
>>> + if (sreq->host_status == SCSI_HOST_OK) {
>>> + SCSISense sense;
>>> +
>>> + sreq->status =
>>> scsi_sense_from_host_status(sreq->host_status, &sense);
>>> + if (sreq->status == CHECK_CONDITION) {
>>> + scsi_req_build_sense(sreq, sense);
>>> + }
>>> + }
>>
>> Should be != of course.
>>
> No.
> scsi_req_build_sense() transfers the sense code from the second argument
> into a proper SCSI sense. Which is only set if the status is
> CHECK_CONDITION...
I mean sreq->host_status != SCSI_HOST_OK. I might be wrong, but every
other HBA is using that...
Paolo
On 11/16/20 11:00 PM, Paolo Bonzini wrote:
> On 16/11/20 20:05, Hannes Reinecke wrote:
>>>> + if (sreq->host_status == SCSI_HOST_OK) {
>>>> + SCSISense sense;
>>>> +
>>>> + sreq->status =
>>>> scsi_sense_from_host_status(sreq->host_status, &sense);
>>>> + if (sreq->status == CHECK_CONDITION) {
>>>> + scsi_req_build_sense(sreq, sense);
>>>> + }
>>>> + }
>>>
>>> Should be != of course.
>>>
>> No.
>> scsi_req_build_sense() transfers the sense code from the second argument
>> into a proper SCSI sense. Which is only set if the status is
>> CHECK_CONDITION...
>
> I mean sreq->host_status != SCSI_HOST_OK. I might be wrong, but every
> other HBA is using that...
>
Bah. Yes, of course, you are right.
Shall I resubmit? Or how is the process nowadays?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 17/11/20 07:55, Hannes Reinecke wrote:
> On 11/16/20 11:00 PM, Paolo Bonzini wrote:
>> On 16/11/20 20:05, Hannes Reinecke wrote:
>>>>> + if (sreq->host_status == SCSI_HOST_OK) {
>>>>> + SCSISense sense;
>>>>> +
>>>>> + sreq->status =
>>>>> scsi_sense_from_host_status(sreq->host_status, &sense);
>>>>> + if (sreq->status == CHECK_CONDITION) {
>>>>> + scsi_req_build_sense(sreq, sense);
>>>>> + }
>>>>> + }
>>>>
>>>> Should be != of course.
>>>>
>>> No.
>>> scsi_req_build_sense() transfers the sense code from the second argument
>>> into a proper SCSI sense. Which is only set if the status is
>>> CHECK_CONDITION...
>>
>> I mean sreq->host_status != SCSI_HOST_OK. I might be wrong, but every
>> other HBA is using that...
>>
> Bah. Yes, of course, you are right.
>
> Shall I resubmit? Or how is the process nowadays?
Depends on how busy and grumpy I am. :) Since we're right in the middle
of the freeze, let me send a RFC patch for Linux to clean up DID_* a
little bit.
Paolo
On 11/17/20 8:38 AM, Paolo Bonzini wrote:
> On 17/11/20 07:55, Hannes Reinecke wrote:
>> On 11/16/20 11:00 PM, Paolo Bonzini wrote:
>>> On 16/11/20 20:05, Hannes Reinecke wrote:
>>>>>> + if (sreq->host_status == SCSI_HOST_OK) {
>>>>>> + SCSISense sense;
>>>>>> +
>>>>>> + sreq->status =
>>>>>> scsi_sense_from_host_status(sreq->host_status, &sense);
>>>>>> + if (sreq->status == CHECK_CONDITION) {
>>>>>> + scsi_req_build_sense(sreq, sense);
>>>>>> + }
>>>>>> + }
>>>>>
>>>>> Should be != of course.
>>>>>
>>>> No.
>>>> scsi_req_build_sense() transfers the sense code from the second
>>>> argument
>>>> into a proper SCSI sense. Which is only set if the status is
>>>> CHECK_CONDITION...
>>>
>>> I mean sreq->host_status != SCSI_HOST_OK. I might be wrong, but
>>> every other HBA is using that...
>>>
>> Bah. Yes, of course, you are right.
>>
>> Shall I resubmit? Or how is the process nowadays?
>
> Depends on how busy and grumpy I am. :) Since we're right in the middle
> of the freeze, let me send a RFC patch for Linux to clean up DID_* a
> little bit.
>
What's your intention there? I do have (of course) a larger patchset for
revisiting the SCSI status codes, so I could resubmit those portions
relating to DID_ codes ...
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 17/11/20 09:50, Hannes Reinecke wrote: >> Since we're right in the middle of the freeze, let me send a RFC patch >> for Linux to clean up DID_* a little bit. >> > What's your intention there? I do have (of course) a larger patchset for > revisiting the SCSI status codes, so I could resubmit those portions > relating to DID_ codes ... Not much. First, renaming DID_NEXUS_FAILURE and BLK_STS_NEXUS to DID_RESERVATION_CONFLICT and BLK_STS_RESERVATION respectively. Second, adding a __ in front of those DID_* constants that are ultimately changed back to DID_OK. Thanks, Paolo
© 2016 - 2026 Red Hat, Inc.