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

2015-08-24 Thread 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 abezzubi...@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:
 +

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

2015-08-19 Thread 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 abezzubi...@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