Re: [Qemu-devel] Re: [PATCH] lsi53c895a: add support for ABORT messages

[Peter Lieven]

Am 07.10.2010 um 13:27 schrieb Kevin Wolf:

> Am 06.09.2010 16:42, schrieb Bernhard Kohl:
>> If these messages are not handled correctly the guest driver may hang.
>> 
>> Always mandatory:
>> - ABORT
>> - BUS DEVICE RESET
>> 
>> Mandatory if tagged queuing is implemented (which disks usually do):
>> - ABORT TAG
>> - CLEAR QUEUE
>> 
>> Signed-off-by: Bernhard Kohl <address@hidden>
> 
> Nicholas, as you seem to have touched the lsi code recently, care to
> review this one? Assuming that you are reasonably familiar with both the
> hardware and the code, you should be quicker than me with this.

Is there a reason why this patch was never added to the stable qemu-kvm release?
At least int qemu-kvm-0.14.0 I still can't find it.

Peter


> 
> Kevin
> 
>> ---
>> hw/lsi53c895a.c |   57 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 57 insertions(+), 0 deletions(-)
>> 
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index 5eaf69e..40f2d10 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -846,6 +846,18 @@ static void lsi_do_msgout(LSIState *s)
>> {
>>     uint8_t msg;
>>     int len;
>> +    uint32_t current_tag;
>> +    SCSIDevice *current_dev;
>> +    lsi_request *p, *p_next;
>> +    int id;
>> +
>> +    if (s->current) {
>> +        current_tag = s->current->tag;
>> +    } else {
>> +        current_tag = s->select_tag;
>> +    }
>> +    id = (current_tag >> 8) & 0xf;
>> +    current_dev = s->bus.devs[id];
>> 
>>     DPRINTF("MSG out len=%d\n", s->dbc);
>>     while (s->dbc) {
>> @@ -890,6 +902,51 @@ static void lsi_do_msgout(LSIState *s)
>>             BADF("ORDERED queue not implemented\n");
>>             s->select_tag |= lsi_get_msgbyte(s) | LSI_TAG_VALID;
>>             break;
>> +        case 0x0d:
>> +            /* The ABORT TAG message clears the current I/O process only. */
>> +            DPRINTF("MSG: ABORT TAG tag=0x%x\n", current_tag);
>> +            current_dev->info->cancel_io(current_dev, current_tag);
>> +            lsi_disconnect(s);
>> +            break;
>> +        case 0x06:
>> +        case 0x0e:
>> +        case 0x0c:
>> +            /* The ABORT message clears all I/O processes for the selecting
>> +               initiator on the specified logical unit of the target. */
>> +            if (msg == 0x06) {
>> +                DPRINTF("MSG: ABORT tag=0x%x\n", current_tag);
>> +            }
>> +            /* The CLEAR QUEUE message clears all I/O processes for all
>> +               initiators on the specified logical unit of the target. */
>> +            if (msg == 0x0e) {
>> +                DPRINTF("MSG: CLEAR QUEUE tag=0x%x\n", current_tag);
>> +            }
>> +            /* The BUS DEVICE RESET message clears all I/O processes for all
>> +               initiators on all logical units of the target. */
>> +            if (msg == 0x0c) {
>> +                DPRINTF("MSG: BUS DEVICE RESET tag=0x%x\n", current_tag);
>> +            }
>> +
>> +            /* clear the current I/O process */
>> +            current_dev->info->cancel_io(current_dev, current_tag);
>> +
>> +            /* As the current implemented devices scsi_disk and scsi_generic
>> +               only support one LUN, we don't need to keep track of LUNs.
>> +               Clearing I/O processes for other initiators could be possible
>> +               for scsi_generic by sending a SG_SCSI_RESET to the /dev/sgX
>> +               device, but this is currently not implemented (and seems not
>> +               to be really necessary). So let's simply clear all queued
>> +               commands for the current device: */
>> +            id = current_tag & 0x0000ff00;
>> +            QTAILQ_FOREACH_SAFE(p, &s->queue, next, p_next) {
>> +                if ((p->tag & 0x0000ff00) == id) {
>> +                    current_dev->info->cancel_io(current_dev, p->tag);
>> +                    QTAILQ_REMOVE(&s->queue, p, next);
>> +                }
>> +            }
>> +
>> +            lsi_disconnect(s);
>> +            break;
>>         default:
>>             if ((msg & 0x80) == 0) {
>>                 goto bad;
> 
>