[Qemu-devel] [PATCH RFC v4 0/5] QEMU ATAPI-SCSI bridge GSoC project

[Qemu-devel] [PATCH RFC v4 0/5] QEMU ATAPI-SCSI bridge GSoC project

[Alexander Bezzubikov]

This is my QEMU Google Summer of Code project.
Here I introduce new device - ATAPI-SCSI bridge.
Its purpose is to unify IDE ATAPI CD-ROM emulation
with SCSI CD-ROM emulation to reduce code duplication and squash bugs.
It's purpose is simple - it just forwards ATAPI commands
to SCSI side for parsing and execution.

Example of using:
qemu-system-x86_64 \
        -drive if=none,file=<filename>,id=cdrom \
        -drive if=none,id=fake \
        -device ide-bridge,id=bridge,drive=fake \
        -device scsi-cd,drive=cdrom,bus=bridge.0 \
        -hda diskimage \
        -m 1024

Alexander Bezzubikov (5):
  ide: ATAPI-SCSI bridge TypeInfo and init function created
  scsi: SCSIDiskReq declaration moved to header
  ide: necessary checks corrected to treat ATAPI-SCSI bridge as CDROM
  ATAPI-SCSI bridge functions created an can be used by bridge
  ide: ATAPI-SCSI bridge transfer is treated as PIO

 hw/ide/Makefile.objs   |   2 +-
 hw/ide/atapi.c         |  20 ++++++++-
 hw/ide/bridge.c        | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ide/bridge.h        |  10 +++++
 hw/ide/core.c          |  28 +++++++-----
 hw/ide/internal.h      |   6 ++-
 hw/ide/qdev.c          |  43 ++++++++++++++++++-
 hw/scsi/scsi-disk.c    |  55 ++++++++++++++++++------
 include/hw/scsi/scsi.h |  16 +++++++
 9 files changed, 267 insertions(+), 27 deletions(-)
 create mode 100644 hw/ide/bridge.c
 create mode 100644 hw/ide/bridge.h

-- 
2.1.4

[Qemu-devel] [PATCH RFC v4 1/5] ide: ATAPI-SCSI bridge TypeInfo and init function created

[Alexander Bezzubikov]

Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
---
 hw/ide/bridge.h   |  9 +++++++++
 hw/ide/internal.h |  4 +++-
 hw/ide/qdev.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 hw/ide/bridge.h

diff --git a/hw/ide/bridge.h b/hw/ide/bridge.h
new file mode 100644
index 0000000..dca5d73
--- /dev/null
+++ b/hw/ide/bridge.h
@@ -0,0 +1,9 @@
+#ifndef HW_IDE_BRIDGE_H
+#define HW_IDE_BRIDGE_H
+
+#include "hw/ide/internal.h"
+
+void ide_bridge_start_transfer(SCSIRequest *req, uint32_t len);
+void ide_bridge_complete(SCSIRequest *req, uint32_t status, size_t resid);
+
+#endif
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 30fdcbc..79c85be 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -12,6 +12,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/block/block.h"
 #include "block/scsi.h"
+#include "hw/scsi/scsi.h"
 
 /* debug IDE devices */
 //#define DEBUG_IDE
@@ -317,7 +318,7 @@ typedef struct IDEDMAOps IDEDMAOps;
 #define SMART_DISABLE         0xd9
 #define SMART_STATUS          0xda
 
-typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
+typedef enum { IDE_HD, IDE_CD, IDE_CFATA, IDE_BRIDGE } IDEDriveKind;
 
 typedef void EndTransferFunc(IDEState *);
 
@@ -492,6 +493,7 @@ struct IDEDevice {
     char *serial;
     char *model;
     uint64_t wwn;
+    SCSIBus scsi_bus;
 };
 
 /* These are used for the error_status field of IDEBus */
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 788b361..3bf3401 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,6 +25,7 @@
 #include "hw/block/block.h"
 #include "sysemu/sysemu.h"
 #include "qapi/visitor.h"
+#include "hw/ide/bridge.h"
 
 /* --------------------------------- */
 
@@ -143,6 +144,17 @@ int ide_get_bios_chs_trans(BusState *bus, int unit)
     return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
 }
 
+/* BusInfo structure for ATAPI-SCSI bridge */
+static const struct SCSIBusInfo atapi_scsi_info = {
+    .tcq = true,
+    .max_target = 0,
+    .max_lun = 0,
+
+    .transfer_data = NULL,
+    .complete = NULL,
+    .cancel = NULL
+};
+
 /* --------------------------------- */
 
 typedef struct IDEDrive {
@@ -185,6 +197,12 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
         return -1;
     }
 
+    if (kind == IDE_BRIDGE) {
+        scsi_bus_new(&dev->scsi_bus, sizeof(dev->scsi_bus), &dev->qdev,
+                     &atapi_scsi_info, NULL);
+        scsi_bus_legacy_handle_cmdline(&dev->scsi_bus, NULL);
+    }
+
     if (!dev->version) {
         dev->version = g_strdup(s->version);
     }
@@ -253,6 +271,11 @@ static int ide_cd_initfn(IDEDevice *dev)
     return ide_dev_initfn(dev, IDE_CD);
 }
 
+static int ide_bridge_initfn(IDEDevice *dev)
+{
+    return ide_dev_initfn(dev, IDE_BRIDGE);
+}
+
 static int ide_drive_initfn(IDEDevice *dev)
 {
     DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
@@ -314,6 +337,23 @@ static const TypeInfo ide_cd_info = {
     .class_init    = ide_cd_class_init,
 };
 
+static void ide_bridge_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
+    k->init = ide_bridge_initfn;
+    dc->fw_name = "drive";
+    dc->desc = "virtual ATAPI-SCSI bridge";
+    dc->props = ide_cd_properties;
+}
+
+static const TypeInfo ide_bridge_info = {
+    .name          = "ide-bridge",
+    .parent        = TYPE_IDE_DEVICE,
+    .instance_size = sizeof(IDEDrive),
+    .class_init    = ide_bridge_class_init,
+};
+
 static Property ide_drive_properties[] = {
     DEFINE_IDE_DEV_PROPERTIES(),
     DEFINE_PROP_END_OF_LIST(),
@@ -360,6 +400,7 @@ static void ide_register_types(void)
     type_register_static(&ide_bus_info);
     type_register_static(&ide_hd_info);
     type_register_static(&ide_cd_info);
+    type_register_static(&ide_bridge_info);
     type_register_static(&ide_drive_info);
     type_register_static(&ide_device_type_info);
 }
-- 
2.1.4

[Qemu-devel] [PATCH RFC v4 2/5] scsi: SCSIDiskReq declaration moved to header

[Alexander Bezzubikov]

Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
---
 hw/scsi/scsi-disk.c    | 12 ------------
 include/hw/scsi/scsi.h | 13 +++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 64f0694..8626eba 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -53,18 +53,6 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 
 typedef struct SCSIDiskState SCSIDiskState;
 
-typedef struct SCSIDiskReq {
-    SCSIRequest req;
-    /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
-    uint64_t sector;
-    uint32_t sector_count;
-    uint32_t buflen;
-    bool started;
-    struct iovec iov;
-    QEMUIOVector qiov;
-    BlockAcctCookie acct;
-} SCSIDiskReq;
-
 #define SCSI_DISK_F_REMOVABLE             0
 #define SCSI_DISK_F_DPOFUA                1
 #define SCSI_DISK_F_NO_REMOVABLE_DEVOPS   2
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index cdaf0f8..e25fd70 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -6,6 +6,7 @@
 #include "hw/block/block.h"
 #include "sysemu/sysemu.h"
 #include "qemu/notify.h"
+#include "block/accounting.h"
 
 #define MAX_SCSI_DEVS	255
 
@@ -120,6 +121,18 @@ extern const VMStateDescription vmstate_scsi_device;
     .offset     = vmstate_offset_value(_state, _field, SCSIDevice),  \
 }
 
+typedef struct SCSIDiskReq {
+    SCSIRequest req;
+    /* Both sector and sector_count are in terms of qemu 512 byte blocks.  */
+    uint64_t sector;
+    uint32_t sector_count;
+    uint32_t buflen;
+    bool started;
+    struct iovec iov;
+    QEMUIOVector qiov;
+    BlockAcctCookie acct;
+} SCSIDiskReq;
+
 /* cdrom.c */
 int cdrom_read_toc(int nb_sectors, uint8_t *buf, int msf, int start_track);
 int cdrom_read_toc_raw(int nb_sectors, uint8_t *buf, int msf, int session_num);
-- 
2.1.4

[Qemu-devel] [PATCH RFC v4 3/5] ide: necessary checks corrected to treat ATAPI-SCSI bridge as CDROM

[Alexander Bezzubikov]

hw/ide/qdev.c: 	corrected to treat bridge as CDROM
hw/ide/core.c: 	same corrections as in qdev.c
hw/ide/atapi.c:	skip some CDROM checks because bridge has only fake drive

Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
---
 hw/ide/atapi.c |  4 +++-
 hw/ide/core.c  | 24 ++++++++++++++----------
 hw/ide/qdev.c  |  2 +-
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 79dd167..f6135e1 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1230,6 +1230,7 @@ void ide_atapi_cmd(IDEState *s)
      * states rely on this behavior.
      */
     if (!(atapi_cmd_table[s->io_buffer[0]].flags & ALLOW_UA) &&
+        s->drive_kind != IDE_BRIDGE &&
         !s->tray_open && blk_is_inserted(s->blk) && s->cdrom_changed) {
 
         if (s->cdrom_changed == 1) {
@@ -1245,7 +1246,8 @@ void ide_atapi_cmd(IDEState *s)
 
     /* Report a Not Ready condition if appropriate for the command */
     if ((atapi_cmd_table[s->io_buffer[0]].flags & CHECK_READY) &&
-        (!media_present(s) || !blk_is_inserted(s->blk)))
+        (s->drive_kind != IDE_BRIDGE &&
+        (!media_present(s) || !blk_is_inserted(s->blk))))
     {
         ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
         return;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 50449ca..96824ab 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -348,7 +348,7 @@ static void ide_set_signature(IDEState *s)
     /* put signature */
     s->nsector = 1;
     s->sector = 1;
-    if (s->drive_kind == IDE_CD) {
+    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
         s->lcyl = 0x14;
         s->hcyl = 0xeb;
     } else if (s->blk) {
@@ -1144,7 +1144,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
 
 static bool cmd_identify(IDEState *s, uint8_t cmd)
 {
-    if (s->blk && s->drive_kind != IDE_CD) {
+    if (s->blk && s->drive_kind != IDE_CD && s->drive_kind != IDE_BRIDGE) {
         if (s->drive_kind != IDE_CFATA) {
             ide_identify(s);
         } else {
@@ -1155,7 +1155,7 @@ static bool cmd_identify(IDEState *s, uint8_t cmd)
         ide_set_irq(s->bus);
         return false;
     } else {
-        if (s->drive_kind == IDE_CD) {
+        if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
             ide_set_signature(s);
         }
         ide_abort_command(s);
@@ -1232,7 +1232,7 @@ static bool cmd_read_pio(IDEState *s, uint8_t cmd)
 {
     bool lba48 = (cmd == WIN_READ_EXT);
 
-    if (s->drive_kind == IDE_CD) {
+    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
         ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
         ide_abort_command(s);
         return true;
@@ -1426,7 +1426,7 @@ static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
 {
     ide_set_signature(s);
 
-    if (s->drive_kind == IDE_CD) {
+    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
         s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
                         * devices to return a clear status register
                         * with READY_STAT *not* set. */
@@ -1731,7 +1731,7 @@ abort_cmd:
 }
 
 #define HD_OK (1u << IDE_HD)
-#define CD_OK (1u << IDE_CD)
+#define CD_OK ((1u << IDE_CD) | (1u << IDE_BRIDGE))
 #define CFA_OK (1u << IDE_CFATA)
 #define HD_CFA_OK (HD_OK | CFA_OK)
 #define ALL_OK (HD_OK | CD_OK | CFA_OK)
@@ -1978,10 +1978,11 @@ void ide_cmd_write(void *opaque, uint32_t addr, uint32_t val)
         /* high to low */
         for(i = 0;i < 2; i++) {
             s = &bus->ifs[i];
-            if (s->drive_kind == IDE_CD)
+            if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
                 s->status = 0x00; /* NOTE: READY is _not_ set */
-            else
+            } else {
                 s->status = READY_STAT | SEEK_STAT;
+            }
             ide_set_signature(s);
         }
     }
@@ -2234,7 +2235,7 @@ static void ide_resize_cb(void *opaque)
         ide_cfata_identify_size(s);
     } else {
         /* IDE_CD uses a different set of callbacks entirely. */
-        assert(s->drive_kind != IDE_CD);
+        assert(s->drive_kind != IDE_CD && s->drive_kind != IDE_BRIDGE);
         ide_identify_size(s);
     }
 }
@@ -2274,7 +2275,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
     s->smart_autosave = 1;
     s->smart_errors = 0;
     s->smart_selftest_count = 0;
-    if (kind == IDE_CD) {
+    if (kind == IDE_CD || kind == IDE_BRIDGE) {
         blk_set_dev_ops(blk, &ide_cd_block_ops, s);
         blk_set_guest_block_size(blk, 2048);
     } else {
@@ -2301,6 +2302,9 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
         case IDE_CD:
             strcpy(s->drive_model_str, "QEMU DVD-ROM");
             break;
+        case IDE_BRIDGE:
+            strcpy(s->drive_model_str, "QEMU VIRTUAL ATAPI-SCSI BRIDGE");
+            break;
         case IDE_CFATA:
             strcpy(s->drive_model_str, "QEMU MICRODRIVE");
             break;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 3bf3401..2ed0c39 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -182,7 +182,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     }
 
     blkconf_serial(&dev->conf, &dev->serial);
-    if (kind != IDE_CD) {
+    if (kind != IDE_CD && kind != IDE_BRIDGE) {
         blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
         if (err) {
             error_report_err(err);
-- 
2.1.4

[Qemu-devel] [PATCH RFC v4 4/5] ATAPI-SCSI bridge functions created an can be used by bridge

[Alexander Bezzubikov]

ide: bridge functions created
ide: Makefile corrected due to bridge creation
scsi: added function to enable bridge send SCSI requests
ide: bridge can now forward requests to SCSI
ide: bridge functions assigned to SCSIBusInfo
Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
---
 hw/ide/Makefile.objs   |   2 +-
 hw/ide/atapi.c         |  16 +++++++
 hw/ide/bridge.c        | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/ide/bridge.h        |   1 +
 hw/ide/internal.h      |   2 +
 hw/ide/qdev.c          |   4 +-
 hw/scsi/scsi-disk.c    |  43 +++++++++++++++++++
 include/hw/scsi/scsi.h |   3 ++
 8 files changed, 182 insertions(+), 3 deletions(-)
 create mode 100644 hw/ide/bridge.c

diff --git a/hw/ide/Makefile.objs b/hw/ide/Makefile.objs
index 729e9bd..f54f275 100644
--- a/hw/ide/Makefile.objs
+++ b/hw/ide/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-$(CONFIG_IDE_CORE) += core.o atapi.o
+common-obj-$(CONFIG_IDE_CORE) += core.o atapi.o bridge.o
 common-obj-$(CONFIG_IDE_QDEV) += qdev.o
 common-obj-$(CONFIG_IDE_PCI) += pci.o
 common-obj-$(CONFIG_IDE_ISA) += isa.o
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f6135e1..1233fbe 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -1253,6 +1253,22 @@ void ide_atapi_cmd(IDEState *s)
         return;
     }
 
+    if (s->drive_kind == IDE_BRIDGE) {
+        IDEDevice *dev = s->bus->master;
+        SCSIDevice *scsi_dev = scsi_device_find(&dev->scsi_bus, 0, 0, 0);
+        s->scsi_req = scsi_new_request_from_bridge(scsi_dev, 0, 0, buf, NULL);
+
+        /* Necessary to prevent ide from reading while data isn't ready */
+        if (buf[0] == READ_10) {
+            s->status |= BUSY_STAT;
+        }
+
+        if (scsi_req_enqueue(s->scsi_req)) {
+            scsi_req_continue(s->scsi_req);
+        }
+        return;
+    }
+
     /* Execute the command */
     if (atapi_cmd_table[s->io_buffer[0]].handler) {
         atapi_cmd_table[s->io_buffer[0]].handler(s, buf);
diff --git a/hw/ide/bridge.c b/hw/ide/bridge.c
new file mode 100644
index 0000000..324e02e
--- /dev/null
+++ b/hw/ide/bridge.c
@@ -0,0 +1,114 @@
+#include "hw/ide/bridge.h"
+
+void ide_bridge_do_transfer(IDEState *s)
+{
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, s->scsi_req);
+
+    if (r->buflen > 0) {
+        int size = r->buflen;
+
+        int byte_count_limit = s->lcyl | (s->hcyl << 8);
+        if (byte_count_limit == 0xffff) {
+            byte_count_limit--;
+        }
+        if (size > byte_count_limit) {
+            /* byte count limit must be even if this case */
+            if (byte_count_limit & 1) {
+                byte_count_limit--;
+            }
+            size = byte_count_limit;
+        }
+        s->lcyl = size;
+        s->hcyl = size >> 8;
+        s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
+
+        int offset = (r->buflen == r->qiov.size) ? 0 : r->qiov.size - r->buflen;
+        r->buflen -= size;
+
+        ide_transfer_start(s, s->io_buffer + offset, size,
+                           ide_bridge_do_transfer);
+        ide_set_irq(s->bus);
+    } else {
+        scsi_req_complete(s->scsi_req, GOOD);
+    }
+}
+
+static void ide_bridge_dma_complete(void *opaque, int ret)
+{
+    IDEState *s = opaque;
+
+    s->io_buffer_size = s->bus->dma->iov.iov_len;
+    s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
+    s->bus->dma->ops->rw_buf(s->bus->dma, 1);
+    scsi_req_complete(s->scsi_req, GOOD);
+
+    s->status = READY_STAT | SEEK_STAT;
+
+    ide_set_irq(s->bus);
+    ide_set_inactive(s, false);
+}
+
+void ide_bridge_start_transfer(SCSIRequest *req, uint32_t len)
+{
+    IDEDevice *dev = IDE_DEVICE(req->bus->qbus.parent);
+    IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
+    IDEState *s = bus->ifs;
+    SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
+
+    int cmd = req->cmd.buf[0];
+    if (cmd == READ_10) {
+        if (s->feature & 1) {
+            s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
+            qemu_iovec_clone(&s->bus->dma->qiov, &r->qiov, NULL);
+            qemu_iovec_to_buf(&r->qiov, 0, s->io_buffer, r->qiov.size);
+        } else {
+            qemu_iovec_to_buf(&r->qiov, 0, s->io_buffer, r->qiov.size);
+        }
+    } else {
+        if (cmd == INQUIRY) {
+            len = 36;
+        }
+        r->iov.iov_len = len;
+        qemu_iovec_concat_iov(&r->qiov, &r->iov, len, 0, len);
+        qemu_iovec_to_buf(&r->qiov, 0, s->io_buffer, r->qiov.size);
+    }
+
+    s->io_buffer_index = 0;
+    s->status = READY_STAT | SEEK_STAT;
+
+    if (cmd != TEST_UNIT_READY && cmd != ALLOW_MEDIUM_REMOVAL) {
+        if (s->feature & 1) {
+            s->io_buffer_index = 0;
+            s->bus->retry_unit = s->unit;
+            s->bus->retry_sector_num = ide_get_sector(s);
+            s->bus->retry_nsector = s->nsector;
+
+            s->bus->dma->iov.iov_base = (void *)(s->io_buffer);
+            s->bus->dma->iov.iov_len = r->qiov.size;
+
+            if (cmd != READ_10) {
+                s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
+            }
+
+            if (s->bus->dma->ops->start_dma) {
+                s->bus->dma->ops->start_dma(s->bus->dma, s,
+                                            ide_bridge_dma_complete);
+            }
+        } else {
+            r->buflen = r->qiov.size;
+            ide_bridge_do_transfer(s);
+        }
+    } else {
+        scsi_req_complete(req, GOOD);
+    }
+}
+
+void ide_bridge_complete(SCSIRequest *req, uint32_t status, size_t resid)
+{
+    IDEDevice *dev = IDE_DEVICE(req->bus->qbus.parent);
+    IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
+    IDEState *s = bus->ifs;
+
+    ide_atapi_cmd_ok(s);
+    ide_set_irq(s->bus);
+}
diff --git a/hw/ide/bridge.h b/hw/ide/bridge.h
index dca5d73..59f2f25 100644
--- a/hw/ide/bridge.h
+++ b/hw/ide/bridge.h
@@ -5,5 +5,6 @@
 
 void ide_bridge_start_transfer(SCSIRequest *req, uint32_t len);
 void ide_bridge_complete(SCSIRequest *req, uint32_t status, size_t resid);
+void ide_bridge_do_transfer(IDEState *s);
 
 #endif
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 79c85be..a79bdaf 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -429,6 +429,8 @@ struct IDEState {
     uint8_t *smart_selftest_data;
     /* AHCI */
     int ncq_queues;
+    /* ATAPI-SCSI bridge */
+    SCSIRequest *scsi_req;
 };
 
 struct IDEDMAOps {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 2ed0c39..e18d222 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -150,8 +150,8 @@ static const struct SCSIBusInfo atapi_scsi_info = {
     .max_target = 0,
     .max_lun = 0,
 
-    .transfer_data = NULL,
-    .complete = NULL,
+    .transfer_data = ide_bridge_start_transfer,
+    .complete = ide_bridge_complete,
     .cancel = NULL
 };
 
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 8626eba..9d5f0a4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2428,6 +2428,49 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun,
     return req;
 }
 
+SCSIRequest *scsi_new_request_from_bridge(SCSIDevice *d, uint32_t tag,
+                                          uint32_t lun, uint8_t *buf,
+                                          void *hba_private)
+{
+    SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
+    SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(d);
+    SCSIRequest *req;
+    const SCSIReqOps *ops;
+    uint8_t command;
+
+    command = buf[0];
+    ops = scsi_disk_reqops_dispatch[command];
+    if (!ops) {
+        ops = &scsi_disk_emulate_reqops;
+    }
+    req = scsi_req_alloc(ops, &s->qdev, tag, lun, hba_private);
+    memcpy(req->cmd.buf, buf, 16);
+
+    SCSICommand cmd = { .len = 0 };
+
+    if (ops != NULL || !sc->parse_cdb) {
+        scsi_req_parse_cdb(d, &cmd, buf);
+    } else {
+        sc->parse_cdb(d, &cmd, buf, hba_private);
+    }
+
+    req->cmd = cmd;
+    req->resid = req->cmd.xfer;
+
+    #ifdef DEBUG_SCSI
+    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
+    {
+        int i;
+        for (i = 1; i < scsi_cdb_length(buf); i++) {
+            printf(" 0x%02x", buf[i]);
+        }
+        printf("\n");
+    }
+    #endif
+
+    return req;
+}
+
 #ifdef __linux__
 static int get_device_type(SCSIDiskState *s)
 {
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index e25fd70..de0546e 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -262,6 +262,9 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
                             uint32_t tag, uint32_t lun, void *hba_private);
 SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
                           uint8_t *buf, void *hba_private);
+SCSIRequest *scsi_new_request_from_bridge(SCSIDevice *d, uint32_t tag,
+                                          uint32_t lun, uint8_t *buf,
+                                          void *hba_private);
 int32_t scsi_req_enqueue(SCSIRequest *req);
 void scsi_req_free(SCSIRequest *req);
 SCSIRequest *scsi_req_ref(SCSIRequest *req);
-- 
2.1.4

[Qemu-devel] [PATCH RFC v4 5/5] ide: ATAPI-SCSI bridge transfer is treated as PIO

[Alexander Bezzubikov]

This patch is necessary because ATAPI-SCSI bridge transfer uses
ide_transfer_start/stop and ide_data_read function check if
PIO transfer is running, so bridge function should be added
to this check

Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
---
 hw/ide/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 96824ab..4988db6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -34,6 +34,7 @@
 #include "sysemu/block-backend.h"
 
 #include <hw/ide/internal.h>
+#include "hw/ide/bridge.h"
 
 /* These values were based on a Seagate ST3500418AS but have been modified
    to make more sense in QEMU */
@@ -2002,7 +2003,8 @@ static bool ide_is_pio_out(IDEState *s)
     } else if (s->end_transfer_func == ide_sector_read ||
                s->end_transfer_func == ide_transfer_stop ||
                s->end_transfer_func == ide_atapi_cmd_reply_end ||
-               s->end_transfer_func == ide_dummy_transfer_stop) {
+               s->end_transfer_func == ide_dummy_transfer_stop ||
+               s->end_transfer_func == ide_bridge_do_transfer) {
         return true;
     }
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH RFC v4 0/5] QEMU ATAPI-SCSI bridge GSoC project

[Hannes Reinecke]

On 08/19/2015 02:49 PM, Alexander Bezzubikov wrote:
> This is my QEMU Google Summer of Code project.
> Here I introduce new device - ATAPI-SCSI bridge.
> Its purpose is to unify IDE ATAPI CD-ROM emulation
> with SCSI CD-ROM emulation to reduce code duplication and squash bugs.
> It's purpose is simple - it just forwards ATAPI commands
> to SCSI side for parsing and execution.
> 
> Example of using:
> qemu-system-x86_64 \
>         -drive if=none,file=<filename>,id=cdrom \
>         -drive if=none,id=fake \
>         -device ide-bridge,id=bridge,drive=fake \
>         -device scsi-cd,drive=cdrom,bus=bridge.0 \
>         -hda diskimage \
>         -m 1024
> 
> Alexander Bezzubikov (5):
>   ide: ATAPI-SCSI bridge TypeInfo and init function created
>   scsi: SCSIDiskReq declaration moved to header
>   ide: necessary checks corrected to treat ATAPI-SCSI bridge as CDROM
>   ATAPI-SCSI bridge functions created an can be used by bridge
>   ide: ATAPI-SCSI bridge transfer is treated as PIO
> 
>  hw/ide/Makefile.objs   |   2 +-
>  hw/ide/atapi.c         |  20 ++++++++-
>  hw/ide/bridge.c        | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/ide/bridge.h        |  10 +++++
>  hw/ide/core.c          |  28 +++++++-----
>  hw/ide/internal.h      |   6 ++-
>  hw/ide/qdev.c          |  43 ++++++++++++++++++-
>  hw/scsi/scsi-disk.c    |  55 ++++++++++++++++++------
>  include/hw/scsi/scsi.h |  16 +++++++
>  9 files changed, 267 insertions(+), 27 deletions(-)
>  create mode 100644 hw/ide/bridge.c
>  create mode 100644 hw/ide/bridge.h
> 
Very nice patchset. Thanks a lot for doing that.

For the entire series:

Acked-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH RFC v4 3/5] ide: necessary checks corrected to treat ATAPI-SCSI bridge as CDROM

[John Snow]

On 08/19/2015 08:49 AM, Alexander Bezzubikov wrote:
> hw/ide/qdev.c: 	corrected to treat bridge as CDROM
> hw/ide/core.c: 	same corrections as in qdev.c
> hw/ide/atapi.c:	skip some CDROM checks because bridge has only fake drive
> 
> Signed-off-by: Alexander Bezzubikov <abezzubikov@ispras.ru>
> ---
>  hw/ide/atapi.c |  4 +++-
>  hw/ide/core.c  | 24 ++++++++++++++----------
>  hw/ide/qdev.c  |  2 +-
>  3 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 79dd167..f6135e1 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -1230,6 +1230,7 @@ void ide_atapi_cmd(IDEState *s)
>       * states rely on this behavior.
>       */
>      if (!(atapi_cmd_table[s->io_buffer[0]].flags & ALLOW_UA) &&
> +        s->drive_kind != IDE_BRIDGE &&
>          !s->tray_open && blk_is_inserted(s->blk) && s->cdrom_changed) {
>  
>          if (s->cdrom_changed == 1) {
> @@ -1245,7 +1246,8 @@ void ide_atapi_cmd(IDEState *s)
>  
>      /* Report a Not Ready condition if appropriate for the command */
>      if ((atapi_cmd_table[s->io_buffer[0]].flags & CHECK_READY) &&
> -        (!media_present(s) || !blk_is_inserted(s->blk)))
> +        (s->drive_kind != IDE_BRIDGE &&
> +        (!media_present(s) || !blk_is_inserted(s->blk))))
>      {
>          ide_atapi_cmd_error(s, NOT_READY, ASC_MEDIUM_NOT_PRESENT);
>          return;
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 50449ca..96824ab 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -348,7 +348,7 @@ static void ide_set_signature(IDEState *s)
>      /* put signature */
>      s->nsector = 1;
>      s->sector = 1;
> -    if (s->drive_kind == IDE_CD) {
> +    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
>          s->lcyl = 0x14;
>          s->hcyl = 0xeb;
>      } else if (s->blk) {
> @@ -1144,7 +1144,7 @@ static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
>  
>  static bool cmd_identify(IDEState *s, uint8_t cmd)
>  {
> -    if (s->blk && s->drive_kind != IDE_CD) {
> +    if (s->blk && s->drive_kind != IDE_CD && s->drive_kind != IDE_BRIDGE) {
>          if (s->drive_kind != IDE_CFATA) {
>              ide_identify(s);
>          } else {
> @@ -1155,7 +1155,7 @@ static bool cmd_identify(IDEState *s, uint8_t cmd)
>          ide_set_irq(s->bus);
>          return false;
>      } else {
> -        if (s->drive_kind == IDE_CD) {
> +        if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
>              ide_set_signature(s);
>          }
>          ide_abort_command(s);
> @@ -1232,7 +1232,7 @@ static bool cmd_read_pio(IDEState *s, uint8_t cmd)
>  {
>      bool lba48 = (cmd == WIN_READ_EXT);
>  
> -    if (s->drive_kind == IDE_CD) {
> +    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
>          ide_set_signature(s); /* odd, but ATA4 8.27.5.2 requires it */
>          ide_abort_command(s);
>          return true;
> @@ -1426,7 +1426,7 @@ static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
>  {
>      ide_set_signature(s);
>  
> -    if (s->drive_kind == IDE_CD) {
> +    if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
>          s->status = 0; /* ATAPI spec (v6) section 9.10 defines packet
>                          * devices to return a clear status register
>                          * with READY_STAT *not* set. */
> @@ -1731,7 +1731,7 @@ abort_cmd:
>  }
>  
>  #define HD_OK (1u << IDE_HD)
> -#define CD_OK (1u << IDE_CD)
> +#define CD_OK ((1u << IDE_CD) | (1u << IDE_BRIDGE))
>  #define CFA_OK (1u << IDE_CFATA)
>  #define HD_CFA_OK (HD_OK | CFA_OK)
>  #define ALL_OK (HD_OK | CD_OK | CFA_OK)
> @@ -1978,10 +1978,11 @@ void ide_cmd_write(void *opaque, uint32_t addr, uint32_t val)
>          /* high to low */
>          for(i = 0;i < 2; i++) {
>              s = &bus->ifs[i];
> -            if (s->drive_kind == IDE_CD)
> +            if (s->drive_kind == IDE_CD || s->drive_kind == IDE_BRIDGE) {
>                  s->status = 0x00; /* NOTE: READY is _not_ set */
> -            else
> +            } else {
>                  s->status = READY_STAT | SEEK_STAT;
> +            }
>              ide_set_signature(s);
>          }
>      }
> @@ -2234,7 +2235,7 @@ static void ide_resize_cb(void *opaque)
>          ide_cfata_identify_size(s);
>      } else {
>          /* IDE_CD uses a different set of callbacks entirely. */
> -        assert(s->drive_kind != IDE_CD);
> +        assert(s->drive_kind != IDE_CD && s->drive_kind != IDE_BRIDGE);
>          ide_identify_size(s);
>      }
>  }
> @@ -2274,7 +2275,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
>      s->smart_autosave = 1;
>      s->smart_errors = 0;
>      s->smart_selftest_count = 0;
> -    if (kind == IDE_CD) {
> +    if (kind == IDE_CD || kind == IDE_BRIDGE) {
>          blk_set_dev_ops(blk, &ide_cd_block_ops, s);
>          blk_set_guest_block_size(blk, 2048);
>      } else {
> @@ -2301,6 +2302,9 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
>          case IDE_CD:
>              strcpy(s->drive_model_str, "QEMU DVD-ROM");
>              break;
> +        case IDE_BRIDGE:
> +            strcpy(s->drive_model_str, "QEMU VIRTUAL ATAPI-SCSI BRIDGE");
> +            break;
>          case IDE_CFATA:
>              strcpy(s->drive_model_str, "QEMU MICRODRIVE");
>              break;
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 3bf3401..2ed0c39 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -182,7 +182,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>      }
>  
>      blkconf_serial(&dev->conf, &dev->serial);
> -    if (kind != IDE_CD) {
> +    if (kind != IDE_CD && kind != IDE_BRIDGE) {
>          blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
>          if (err) {
>              error_report_err(err);
> 

More comments to follow pending a more rigorous look, but there are some
checks for IDE_CD in ahci.c that we should be adjusting to work with the
bridge as well.

Maybe there's a better way to handle the signature generation, relying
on the core layer here that's already managing it, which is something I
might have to do for 2.5, but in the meantime your patches should likely
update ahci.c to understand the scsi bridge as well.

Looks good at a glance otherwise, though I have some questions about the
edits made to atapi.c here that I'll save until after I read the
subsequent patches.

--js