[PATCH v2 00/11] scsi: add quirks and features to support m68k Macs

[PATCH v2 00/11] scsi: add quirks and features to support m68k Macs

[Mark Cave-Ayland]

Here are the next set of patches from my ongoing work to allow the q800
machine to boot MacOS related to SCSI devices.

The first patch implements a dummy FORMAT UNIT command which is used by
the Apple HD SC Setup program when preparing an empty disk to install
MacOS.

Patch 2 adds a new quirks bitmap to SCSIDiskState to allow buggy and/or
legacy features to enabled on an individual device basis. Once the quirks
bitmap has been added, patch 3 uses the quirks feature to implement an
Apple-specific mode page which is required to allow the disk to be recognised
and used by Apple HD SC Setup.

Patch 4 adds compat_props to the q800 machine which enable the new
MODE_PAGE_APPLE_VENDOR quirk for all scsi-hd devices attached to the machine.

Patch 5 adds a new quirk to force SCSI CDROMs to always return the block
descriptor for a MODE SENSE command which is expected by A/UX, whilst patch 6
enables the quirk for all scsi-cd devices on the q800 machine.

Patch 7 adds support for truncated MODE SELECT requests which are sent by
A/UX (and also MacOS in some circumstances) when enumerating a SCSI CDROM device
which are shown to be accepted on real hardware as documented in [1].

Patch 8 allows the MODE_PAGE_R_W_ERROR AWRE bit to be changeable since the A/UX
MODE SELECT request sets this bit to 0 rather than the QEMU default which is 1.

Patch 9 adds support for setting the CDROM block size via a MODE SELECT request
which is supported by older CDROMs to allow the block size to be changed from
the default of 2048 bytes to 512 bytes for compatibility purposes. This is used
by A/UX which otherwise fails with SCSI errors if the block size is not set to
512 bytes when accessing CDROMs.

Finally patches 10 and 11 augment the compat_props to set the default vendor,
product and version information for all scsi-hd and scsi-cd devices attached
to the q800 machine, taken from real drives. This is because MacOS will only
allow a known set of SCSI devices to be recognised during the installation
process.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

[1] https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444


v2:
- Change patchset title from "scsi: add support for FORMAT UNIT command and quirks"
  to "scsi: add quirks and features to support m68k Macs"
- Fix missing shift in patch 2 as pointed out by Fam
- Rename MODE_PAGE_APPLE to MODE_PAGE_APPLE_VENDOR
- Add SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk
- Add support for truncated MODE SELECT requests
- Allow MODE_PAGE_R_W_ERROR AWRE bit to be changeable for CDROM devices
- Allow the MODE SELECT block descriptor to set the CDROM block size


Mark Cave-Ayland (11):
  scsi-disk: add FORMAT UNIT command
  scsi-disk: add new quirks bitmap to SCSIDiskState
  scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh
  q800: implement compat_props to enable quirk_mode_page_apple_vendor
    for scsi-hd devices
  scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk for
    Macintosh
  q800: implement compat_props to enable quirk_mode_sense_rom_force_dbd
    for scsi-cd devices
  scsi-disk: allow truncated MODE SELECT requests
  scsi-disk: allow the MODE_PAGE_R_W_ERROR AWRE bit to be changeable for
    CDROM drives
  scsi-disk: allow MODE SELECT block descriptor to set the ROM device
    block size
  q800: add default vendor and product information for scsi-hd devices
  q800: add default vendor and product information for scsi-cd devices

 hw/m68k/q800.c           | 13 ++++++++++
 hw/scsi/scsi-disk.c      | 53 +++++++++++++++++++++++++++++++++++-----
 hw/scsi/trace-events     |  3 +++
 include/hw/scsi/scsi.h   |  4 +++
 include/scsi/constants.h |  1 +
 5 files changed, 68 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH v2 01/11] scsi-disk: add FORMAT UNIT command

[Mark Cave-Ayland]

When initialising a drive ready to install MacOS, Apple HD SC Setup first attempts
to format the drive. Add a simple FORMAT UNIT command which simply returns success
to allow the format to succeed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c  | 4 ++++
 hw/scsi/trace-events | 1 +
 2 files changed, 5 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 072686ed58..090679f3b5 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2127,6 +2127,9 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
         trace_scsi_disk_emulate_command_WRITE_SAME(
                 req->cmd.buf[0] == WRITE_SAME_10 ? 10 : 16, r->req.cmd.xfer);
         break;
+    case FORMAT_UNIT:
+        trace_scsi_disk_emulate_command_FORMAT_UNIT(r->req.cmd.xfer);
+        break;
     default:
         trace_scsi_disk_emulate_command_UNKNOWN(buf[0],
                                                 scsi_command_name(buf[0]));
@@ -2533,6 +2536,7 @@ static const SCSIReqOps *const scsi_disk_reqops_dispatch[256] = {
     [VERIFY_10]                       = &scsi_disk_emulate_reqops,
     [VERIFY_12]                       = &scsi_disk_emulate_reqops,
     [VERIFY_16]                       = &scsi_disk_emulate_reqops,
+    [FORMAT_UNIT]                     = &scsi_disk_emulate_reqops,
 
     [READ_6]                          = &scsi_disk_dma_reqops,
     [READ_10]                         = &scsi_disk_dma_reqops,
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 20fb0dc162..e91b55a961 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -334,6 +334,7 @@ scsi_disk_emulate_command_UNMAP(size_t xfer) "Unmap (len %zd)"
 scsi_disk_emulate_command_VERIFY(int bytchk) "Verify (bytchk %d)"
 scsi_disk_emulate_command_WRITE_SAME(int cmd, size_t xfer) "WRITE SAME %d (len %zd)"
 scsi_disk_emulate_command_UNKNOWN(int cmd, const char *name) "Unknown SCSI command (0x%2.2x=%s)"
+scsi_disk_emulate_command_FORMAT_UNIT(size_t xfer) "Format Unit (len %zd)"
 scsi_disk_dma_command_READ(uint64_t lba, uint32_t len) "Read (sector %" PRId64 ", count %u)"
 scsi_disk_dma_command_WRITE(const char *cmd, uint64_t lba, int len) "Write %s(sector %" PRId64 ", count %u)"
 scsi_disk_new_request(uint32_t lun, uint32_t tag, const char *line) "Command: lun=%d tag=0x%x data=%s"
-- 
2.20.1

[PATCH v2 02/11] scsi-disk: add new quirks bitmap to SCSIDiskState

[Mark Cave-Ayland]

Since the MacOS SCSI implementation is quite old (and Apple added some firmware
customisations to their drives for m68k Macs) there is need to add a mechanism
to correctly handle Apple-specific quirks.

Add a new quirks bitmap to SCSIDiskState that can be used to enable these
features as required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 090679f3b5..d89cdd4e4a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -94,6 +94,7 @@ struct SCSIDiskState {
     uint16_t port_index;
     uint64_t max_unmap_size;
     uint64_t max_io_size;
+    uint32_t quirks;
     QEMUBH *bh;
     char *version;
     char *serial;
-- 
2.20.1

[PATCH v2 03/11] scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh

[Mark Cave-Ayland]

One of the mechanisms MacOS uses to identify drives compatible with MacOS is to
send a custom MODE SELECT command for page 0x30 to the drive. The response to
this is a hard-coded manufacturer string which must match in order for the
drive to be usable within MacOS.

Add an implementation of the MODE SELECT page 0x30 response guarded by a newly
defined SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk bit so that drives attached
to non-Apple machines function exactly as before.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c      | 17 +++++++++++++++++
 include/hw/scsi/scsi.h   |  3 +++
 include/scsi/constants.h |  1 +
 3 files changed, 21 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d89cdd4e4a..5de4506b97 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1085,6 +1085,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         [MODE_PAGE_R_W_ERROR]              = (1 << TYPE_DISK) | (1 << TYPE_ROM),
         [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
         [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
+        [MODE_PAGE_APPLE_VENDOR]           = (1 << TYPE_ROM),
     };
 
     uint8_t *p = *p_outbuf + 2;
@@ -1229,6 +1230,20 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[19] = (16 * 176) & 0xff;
         break;
 
+     case MODE_PAGE_APPLE_VENDOR:
+        if (s->quirks & (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR)) {
+            length = 0x24;
+            if (page_control == 1) { /* Changeable Values */
+                break;
+            }
+
+            memset(p, 0, length);
+            strcpy((char *)p + 8, "APPLE COMPUTER, INC   ");
+            break;
+        } else {
+            return -1;
+        }
+
     default:
         return -1;
     }
@@ -3042,6 +3057,8 @@ static Property scsi_hd_properties[] = {
     DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
     DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version,
                       5),
+    DEFINE_PROP_BIT("quirk_mode_page_apple_vendor", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
     DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1ffb367f94..975d462347 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -226,4 +226,7 @@ SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun);
 /* scsi-generic.c. */
 extern const SCSIReqOps scsi_generic_req_ops;
 
+/* scsi-disk.c */
+#define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR     0
+
 #endif
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 2a32c08b5e..891aa0f45c 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -234,6 +234,7 @@
 #define MODE_PAGE_FAULT_FAIL                  0x1c
 #define MODE_PAGE_TO_PROTECT                  0x1d
 #define MODE_PAGE_CAPABILITIES                0x2a
+#define MODE_PAGE_APPLE_VENDOR                0x30
 #define MODE_PAGE_ALLS                        0x3f
 /* Not in Mt. Fuji, but in ATAPI 2.6 -- deprecated now in favor
  * of MODE_PAGE_SENSE_POWER */
-- 
2.20.1

[PATCH v2 04/11] q800: implement compat_props to enable quirk_mode_page_apple_vendor for scsi-hd devices

[Mark Cave-Ayland]

By default quirk_mode_page_apple_vendor should be enabled for all scsi-hd devices
connected to the q800 machine to enable MacOS to detect and use them.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 099a758c6f..42bf7bb4f0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -686,6 +686,11 @@ static void q800_init(MachineState *machine)
     }
 }
 
+static GlobalProperty hw_compat_q800[] = {
+    { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+};
+static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
+
 static void q800_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -695,6 +700,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 1;
     mc->block_default_type = IF_SCSI;
     mc->default_ram_id = "m68k_mac.ram";
+    compat_props_add(mc->compat_props, hw_compat_q800, hw_compat_q800_len);
 }
 
 static const TypeInfo q800_machine_typeinfo = {
-- 
2.20.1

[PATCH v2 05/11] scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk for Macintosh

[Mark Cave-Ayland]

During SCSI bus enumeration A/UX sends a MODE SENSE command to the CDROM and
expects the response to include a block descriptor. As per the latest SCSI
documentation, QEMU currently force-disables the block descriptor for CDROM
devices but the A/UX driver expects the block descriptor to always be
returned.

If the block descriptor is not returned in the response then A/UX becomes
confused, since the block descriptor returned in the MODE SENSE response is
used to generate a subsequent MODE SELECT command which is then invalid.

Add a new SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD to allow this behaviour
to be enabled as required.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c    | 18 +++++++++++++-----
 include/hw/scsi/scsi.h |  1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 5de4506b97..71fdf132c1 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1279,10 +1279,17 @@ static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r, uint8_t *outbuf)
             dev_specific_param |= 0x80; /* Readonly.  */
         }
     } else {
-        /* MMC prescribes that CD/DVD drives have no block descriptors,
-         * and defines no device-specific parameter.  */
-        dev_specific_param = 0x00;
-        dbd = true;
+        if (s->quirks & (1 << SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD)) {
+            dev_specific_param = 0x00;
+            dbd = false;
+        } else {
+            /*
+             * MMC prescribes that CD/DVD drives have no block descriptors,
+             * and defines no device-specific parameter.
+             */
+            dev_specific_param = 0x00;
+            dbd = true;
+        }
     }
 
     if (r->req.cmd.buf[0] == MODE_SENSE) {
@@ -1578,7 +1585,6 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
     /* Ensure no change is made if there is an error!  */
     for (pass = 0; pass < 2; pass++) {
         if (mode_select_pages(r, p, len, pass == 1) < 0) {
-            assert(pass == 0);
             return;
         }
     }
@@ -3107,6 +3113,8 @@ static Property scsi_cd_properties[] = {
                        DEFAULT_MAX_IO_SIZE),
     DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version,
                       5),
+    DEFINE_PROP_BIT("quirk_mode_sense_rom_force_dbd", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 975d462347..a9e657e03c 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -228,5 +228,6 @@ extern const SCSIReqOps scsi_generic_req_ops;
 
 /* scsi-disk.c */
 #define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR     0
+#define SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD   1
 
 #endif
-- 
2.20.1

[PATCH v2 06/11] q800: implement compat_props to enable quirk_mode_sense_rom_force_dbd for scsi-cd devices

[Mark Cave-Ayland]

By default quirk_mode_sense_rom_force_dbd should be enabled for all scsi-cd devices
connected to the q800 machine to correctly report the CDROM block descriptor back
to A/UX.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 42bf7bb4f0..f27ed01785 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -688,6 +688,7 @@ static void q800_init(MachineState *machine)
 
 static GlobalProperty hw_compat_q800[] = {
     { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+    { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.20.1

[PATCH v2 07/11] scsi-disk: allow truncated MODE SELECT requests

[Mark Cave-Ayland]

When A/UX configures the CDROM device it sends a truncated MODE SELECT request
for page 1 (MODE_PAGE_R_W_ERROR) which is only 6 bytes in length rather than
10. This seems to be due to bug in Apple's code which calculates the CDB message
length incorrectly.

According to [1] this truncated request is accepted on real hardware whereas in
QEMU it generates an INVALID_PARAM_LEN sense code which causes A/UX to get stuck
in a loop retrying the command in an attempt to succeed.

Alter the mode page request length check so that truncated requests are allowed
as per real hardware, adding a trace event to enable the condition to be detected.

[1] https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c  | 2 +-
 hw/scsi/trace-events | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 71fdf132c1..c657e4f5da 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1525,7 +1525,7 @@ static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
             goto invalid_param;
         }
         if (page_len > len) {
-            goto invalid_param_len;
+            trace_scsi_disk_mode_select_page_truncated(page, page_len, len);
         }
 
         if (!change) {
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index e91b55a961..25eae9f307 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -339,6 +339,7 @@ scsi_disk_dma_command_READ(uint64_t lba, uint32_t len) "Read (sector %" PRId64 "
 scsi_disk_dma_command_WRITE(const char *cmd, uint64_t lba, int len) "Write %s(sector %" PRId64 ", count %u)"
 scsi_disk_new_request(uint32_t lun, uint32_t tag, const char *line) "Command: lun=%d tag=0x%x data=%s"
 scsi_disk_aio_sgio_command(uint32_t tag, uint8_t cmd, uint64_t lba, int len, uint32_t timeout) "disk aio sgio: tag=0x%x cmd=0x%x (sector %" PRId64 ", count %d) timeout=%u"
+scsi_disk_mode_select_page_truncated(int page, int len, int page_len) "page %d expected length %d but received length %d"
 
 # scsi-generic.c
 scsi_generic_command_complete_noio(void *req, uint32_t tag, int statuc) "Command complete %p tag=0x%x status=%d"
-- 
2.20.1

[PATCH v2 08/11] scsi-disk: allow the MODE_PAGE_R_W_ERROR AWRE bit to be changeable for CDROM drives

[Mark Cave-Ayland]

A/UX sends a MODE_PAGE_R_W_ERROR command with the AWRE bit set to 0 when enumerating
CDROM drives. Since the bit is currently hardcoded to 1 then indicate that the AWRE
bit can be changed (even though we don't care about the value) so that
the MODE_PAGE_R_W_ERROR page can be set successfully.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index c657e4f5da..6991493cf4 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1187,6 +1187,10 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
     case MODE_PAGE_R_W_ERROR:
         length = 10;
         if (page_control == 1) { /* Changeable Values */
+            if (s->qdev.type == TYPE_ROM) {
+                /* Automatic Write Reallocation Enabled */
+                p[0] = 0x80;
+            }
             break;
         }
         p[0] = 0x80; /* Automatic Write Reallocation Enabled */
-- 
2.20.1

[PATCH v2 09/11] scsi-disk: allow MODE SELECT block descriptor to set the ROM device block size

[Mark Cave-Ayland]

Whilst CDROM drives usually have a 2048 byte sector size, older drives have the
ability to switch between 2048 byte and 512 byte sector sizes by specifying a
block descriptor in the MODE SELECT command.

If a MODE SELECT block descriptor is provided, update the scsi-cd device block
size with the provided value accordingly.

This allows CDROMs to be used with A/UX whose driver only works with a 512 byte
sector size.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/scsi/scsi-disk.c  | 7 +++++++
 hw/scsi/trace-events | 1 +
 2 files changed, 8 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 6991493cf4..41ebbe3045 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1583,6 +1583,13 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
         goto invalid_param;
     }
 
+    /* Allow changing the block size of ROM devices */
+    if (s->qdev.type == TYPE_ROM && bd_len &&
+        p[6] != (s->qdev.blocksize >> 8)) {
+            s->qdev.blocksize = p[6] << 8;
+            trace_scsi_disk_mode_select_rom_set_blocksize(s->qdev.blocksize);
+    }
+
     len -= bd_len;
     p += bd_len;
 
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 25eae9f307..1a021ddae9 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -340,6 +340,7 @@ scsi_disk_dma_command_WRITE(const char *cmd, uint64_t lba, int len) "Write %s(se
 scsi_disk_new_request(uint32_t lun, uint32_t tag, const char *line) "Command: lun=%d tag=0x%x data=%s"
 scsi_disk_aio_sgio_command(uint32_t tag, uint8_t cmd, uint64_t lba, int len, uint32_t timeout) "disk aio sgio: tag=0x%x cmd=0x%x (sector %" PRId64 ", count %d) timeout=%u"
 scsi_disk_mode_select_page_truncated(int page, int len, int page_len) "page %d expected length %d but received length %d"
+scsi_disk_mode_select_rom_set_blocksize(int blocksize) "set ROM block size to %d"
 
 # scsi-generic.c
 scsi_generic_command_complete_noio(void *req, uint32_t tag, int statuc) "Command complete %p tag=0x%x status=%d"
-- 
2.20.1

[PATCH v2 10/11] q800: add default vendor and product information for scsi-hd devices

[Mark Cave-Ayland]

The Apple HD SC Setup program uses a SCSI INQUIRY command to check that any SCSI
hard disks detected match a whitelist of vendors and products before allowing
the "Initialise" button to prepare an empty disk.

Add known-good default vendor and product information using the existing
compat_prop mechanism so the user doesn't have to use long command lines to set
the qdev properties manually.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index f27ed01785..abb549f8d8 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -688,6 +688,9 @@ static void q800_init(MachineState *machine)
 
 static GlobalProperty hw_compat_q800[] = {
     { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+    { "scsi-hd", "vendor", " SEAGATE" },
+    { "scsi-hd", "product", "          ST225N" },
+    { "scsi-hd", "ver", "1.0 " },
     { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
-- 
2.20.1

[PATCH v2 11/11] q800: add default vendor and product information for scsi-cd devices

[Mark Cave-Ayland]

The MacOS CDROM driver uses a SCSI INQUIRY command to check that any SCSI CDROMs
detected match a whitelist of vendors and products before adding them to the
list of available devices.

Add known-good default vendor and product information using the existing
compat_prop mechanism so the user doesn't have to use long command lines to set
the qdev properties manually.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index abb549f8d8..8b34776c8e 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -692,6 +692,9 @@ static GlobalProperty hw_compat_q800[] = {
     { "scsi-hd", "product", "          ST225N" },
     { "scsi-hd", "ver", "1.0 " },
     { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
+    { "scsi-cd", "vendor", "MATSHITA" },
+    { "scsi-cd", "product", "CD-ROM CR-8005" },
+    { "scsi-cd", "ver", "1.0k" },
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.20.1

Re: [PATCH v2 00/11] scsi: add quirks and features to support m68k Macs

[Mark Cave-Ayland]

On 24/04/2022 17:49, Mark Cave-Ayland wrote:

> Here are the next set of patches from my ongoing work to allow the q800
> machine to boot MacOS related to SCSI devices.
> 
> The first patch implements a dummy FORMAT UNIT command which is used by
> the Apple HD SC Setup program when preparing an empty disk to install
> MacOS.
> 
> Patch 2 adds a new quirks bitmap to SCSIDiskState to allow buggy and/or
> legacy features to enabled on an individual device basis. Once the quirks
> bitmap has been added, patch 3 uses the quirks feature to implement an
> Apple-specific mode page which is required to allow the disk to be recognised
> and used by Apple HD SC Setup.
> 
> Patch 4 adds compat_props to the q800 machine which enable the new
> MODE_PAGE_APPLE_VENDOR quirk for all scsi-hd devices attached to the machine.
> 
> Patch 5 adds a new quirk to force SCSI CDROMs to always return the block
> descriptor for a MODE SENSE command which is expected by A/UX, whilst patch 6
> enables the quirk for all scsi-cd devices on the q800 machine.
> 
> Patch 7 adds support for truncated MODE SELECT requests which are sent by
> A/UX (and also MacOS in some circumstances) when enumerating a SCSI CDROM device
> which are shown to be accepted on real hardware as documented in [1].
> 
> Patch 8 allows the MODE_PAGE_R_W_ERROR AWRE bit to be changeable since the A/UX
> MODE SELECT request sets this bit to 0 rather than the QEMU default which is 1.
> 
> Patch 9 adds support for setting the CDROM block size via a MODE SELECT request
> which is supported by older CDROMs to allow the block size to be changed from
> the default of 2048 bytes to 512 bytes for compatibility purposes. This is used
> by A/UX which otherwise fails with SCSI errors if the block size is not set to
> 512 bytes when accessing CDROMs.
> 
> Finally patches 10 and 11 augment the compat_props to set the default vendor,
> product and version information for all scsi-hd and scsi-cd devices attached
> to the q800 machine, taken from real drives. This is because MacOS will only
> allow a known set of SCSI devices to be recognised during the installation
> process.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> [1] https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444
> 
> 
> v2:
> - Change patchset title from "scsi: add support for FORMAT UNIT command and quirks"
>    to "scsi: add quirks and features to support m68k Macs"
> - Fix missing shift in patch 2 as pointed out by Fam
> - Rename MODE_PAGE_APPLE to MODE_PAGE_APPLE_VENDOR
> - Add SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk
> - Add support for truncated MODE SELECT requests
> - Allow MODE_PAGE_R_W_ERROR AWRE bit to be changeable for CDROM devices
> - Allow the MODE SELECT block descriptor to set the CDROM block size
> 
> 
> Mark Cave-Ayland (11):
>    scsi-disk: add FORMAT UNIT command
>    scsi-disk: add new quirks bitmap to SCSIDiskState
>    scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh
>    q800: implement compat_props to enable quirk_mode_page_apple_vendor
>      for scsi-hd devices
>    scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk for
>      Macintosh
>    q800: implement compat_props to enable quirk_mode_sense_rom_force_dbd
>      for scsi-cd devices
>    scsi-disk: allow truncated MODE SELECT requests
>    scsi-disk: allow the MODE_PAGE_R_W_ERROR AWRE bit to be changeable for
>      CDROM drives
>    scsi-disk: allow MODE SELECT block descriptor to set the ROM device
>      block size
>    q800: add default vendor and product information for scsi-hd devices
>    q800: add default vendor and product information for scsi-cd devices
> 
>   hw/m68k/q800.c           | 13 ++++++++++
>   hw/scsi/scsi-disk.c      | 53 +++++++++++++++++++++++++++++++++++-----
>   hw/scsi/trace-events     |  3 +++
>   include/hw/scsi/scsi.h   |  4 +++
>   include/scsi/constants.h |  1 +
>   5 files changed, 68 insertions(+), 6 deletions(-)

Ping? Anyone have any further thoughts on this?


ATB,

Mark.

Re: [PATCH v2 01/11] scsi-disk: add FORMAT UNIT command

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> When initialising a drive ready to install MacOS, Apple HD SC Setup first attempts
> to format the drive. Add a simple FORMAT UNIT command which simply returns success
> to allow the format to succeed.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/scsi/scsi-disk.c  | 4 ++++
>   hw/scsi/trace-events | 1 +
>   2 files changed, 5 insertions(+)
> 

Reviewed-by: Laurent Vivier <laurent@vivier.Eu>

Re: [PATCH v2 07/11] scsi-disk: allow truncated MODE SELECT requests

[Paolo Bonzini]

On 4/24/22 18:49, Mark Cave-Ayland wrote:
> According to [1] this truncated request is accepted on real hardware whereas in
> QEMU it generates an INVALID_PARAM_LEN sense code which causes A/UX to get stuck
> in a loop retrying the command in an attempt to succeed.

That's for MODE SENSE, not MODE SELECT.

Truncated MODE SELECT is a bit more iffy, so I'd rather have a quirk for 
this as well.

Paolo

> Alter the mode page request length check so that truncated requests are allowed
> as per real hardware, adding a trace event to enable the condition to be detected.
> 
> [1]https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444
> 
> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
> ---
>   hw/scsi/scsi-disk.c  | 2 +-
>   hw/scsi/trace-events | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 71fdf132c1..c657e4f5da 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1525,7 +1525,7 @@ static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int len, bool change)
>               goto invalid_param;
>           }
>           if (page_len > len) {
> -            goto invalid_param_len;
> +            trace_scsi_disk_mode_select_page_truncated(page, page_len, len);
>           }

Re: [PATCH v2 09/11] scsi-disk: allow MODE SELECT block descriptor to set the ROM device block size

[Paolo Bonzini]

On 4/24/22 18:49, Mark Cave-Ayland wrote:
> Whilst CDROM drives usually have a 2048 byte sector size, older drives have the
> ability to switch between 2048 byte and 512 byte sector sizes by specifying a
> block descriptor in the MODE SELECT command.
> 
> If a MODE SELECT block descriptor is provided, update the scsi-cd device block
> size with the provided value accordingly.
> 
> This allows CDROMs to be used with A/UX whose driver only works with a 512 byte
> sector size.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Why do this only for MMC?

Paolo

> ---
>   hw/scsi/scsi-disk.c  | 7 +++++++
>   hw/scsi/trace-events | 1 +
>   2 files changed, 8 insertions(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 6991493cf4..41ebbe3045 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1583,6 +1583,13 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
>           goto invalid_param;
>       }
>   
> +    /* Allow changing the block size of ROM devices */
> +    if (s->qdev.type == TYPE_ROM && bd_len &&
> +        p[6] != (s->qdev.blocksize >> 8)) {
> +            s->qdev.blocksize = p[6] << 8;
> +            trace_scsi_disk_mode_select_rom_set_blocksize(s->qdev.blocksize);
> +    }
> +
>       len -= bd_len;
>       p += bd_len;
>   
> diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
> index 25eae9f307..1a021ddae9 100644
> --- a/hw/scsi/trace-events
> +++ b/hw/scsi/trace-events
> @@ -340,6 +340,7 @@ scsi_disk_dma_command_WRITE(const char *cmd, uint64_t lba, int len) "Write %s(se
>   scsi_disk_new_request(uint32_t lun, uint32_t tag, const char *line) "Command: lun=%d tag=0x%x data=%s"
>   scsi_disk_aio_sgio_command(uint32_t tag, uint8_t cmd, uint64_t lba, int len, uint32_t timeout) "disk aio sgio: tag=0x%x cmd=0x%x (sector %" PRId64 ", count %d) timeout=%u"
>   scsi_disk_mode_select_page_truncated(int page, int len, int page_len) "page %d expected length %d but received length %d"
> +scsi_disk_mode_select_rom_set_blocksize(int blocksize) "set ROM block size to %d"
>   
>   # scsi-generic.c
>   scsi_generic_command_complete_noio(void *req, uint32_t tag, int statuc) "Command complete %p tag=0x%x status=%d"

Re: [PATCH v2 00/11] scsi: add quirks and features to support m68k Macs

[Paolo Bonzini]

On 5/18/22 16:16, Mark Cave-Ayland wrote:
> Ping? Anyone have any further thoughts on this?

Just a couple question, thanks for the reminder!

Paolo

Re: [PATCH v2 11/11] q800: add default vendor and product information for scsi-cd devices

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> The MacOS CDROM driver uses a SCSI INQUIRY command to check that any SCSI CDROMs
> detected match a whitelist of vendors and products before adding them to the
> list of available devices.
> 
> Add known-good default vendor and product information using the existing
> compat_prop mechanism so the user doesn't have to use long command lines to set
> the qdev properties manually.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index abb549f8d8..8b34776c8e 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -692,6 +692,9 @@ static GlobalProperty hw_compat_q800[] = {
>       { "scsi-hd", "product", "          ST225N" },
>       { "scsi-hd", "ver", "1.0 " },
>       { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
> +    { "scsi-cd", "vendor", "MATSHITA" },
> +    { "scsi-cd", "product", "CD-ROM CR-8005" },
> +    { "scsi-cd", "ver", "1.0k" },
>   };
>   static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>   

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 10/11] q800: add default vendor and product information for scsi-hd devices

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> The Apple HD SC Setup program uses a SCSI INQUIRY command to check that any SCSI
> hard disks detected match a whitelist of vendors and products before allowing
> the "Initialise" button to prepare an empty disk.
> 
> Add known-good default vendor and product information using the existing
> compat_prop mechanism so the user doesn't have to use long command lines to set
> the qdev properties manually.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index f27ed01785..abb549f8d8 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -688,6 +688,9 @@ static void q800_init(MachineState *machine)
>   
>   static GlobalProperty hw_compat_q800[] = {
>       { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
> +    { "scsi-hd", "vendor", " SEAGATE" },
> +    { "scsi-hd", "product", "          ST225N" },
> +    { "scsi-hd", "ver", "1.0 " },
>       { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
>   };
>   static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 06/11] q800: implement compat_props to enable quirk_mode_sense_rom_force_dbd for scsi-cd devices

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> By default quirk_mode_sense_rom_force_dbd should be enabled for all scsi-cd devices
> connected to the q800 machine to correctly report the CDROM block descriptor back
> to A/UX.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 42bf7bb4f0..f27ed01785 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -688,6 +688,7 @@ static void q800_init(MachineState *machine)
>   
>   static GlobalProperty hw_compat_q800[] = {
>       { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
> +    { "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
>   };
>   static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>   

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 04/11] q800: implement compat_props to enable quirk_mode_page_apple_vendor for scsi-hd devices

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> By default quirk_mode_page_apple_vendor should be enabled for all scsi-hd devices
> connected to the q800 machine to enable MacOS to detect and use them.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 099a758c6f..42bf7bb4f0 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -686,6 +686,11 @@ static void q800_init(MachineState *machine)
>       }
>   }
>   
> +static GlobalProperty hw_compat_q800[] = {
> +    { "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
> +};
> +static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
> +
>   static void q800_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -695,6 +700,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>       mc->max_cpus = 1;
>       mc->block_default_type = IF_SCSI;
>       mc->default_ram_id = "m68k_mac.ram";
> +    compat_props_add(mc->compat_props, hw_compat_q800, hw_compat_q800_len);
>   }
>   
>   static const TypeInfo q800_machine_typeinfo = {

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 02/11] scsi-disk: add new quirks bitmap to SCSIDiskState

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> Since the MacOS SCSI implementation is quite old (and Apple added some firmware
> customisations to their drives for m68k Macs) there is need to add a mechanism
> to correctly handle Apple-specific quirks.
> 
> Add a new quirks bitmap to SCSIDiskState that can be used to enable these
> features as required.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/scsi/scsi-disk.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 090679f3b5..d89cdd4e4a 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -94,6 +94,7 @@ struct SCSIDiskState {
>       uint16_t port_index;
>       uint64_t max_unmap_size;
>       uint64_t max_io_size;
> +    uint32_t quirks;
>       QEMUBH *bh;
>       char *version;
>       char *serial;

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 03/11] scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh

[Laurent Vivier]

Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
> One of the mechanisms MacOS uses to identify drives compatible with MacOS is to
> send a custom MODE SELECT command for page 0x30 to the drive. The response to
> this is a hard-coded manufacturer string which must match in order for the
> drive to be usable within MacOS.
> 
> Add an implementation of the MODE SELECT page 0x30 response guarded by a newly
> defined SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk bit so that drives attached
> to non-Apple machines function exactly as before.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/scsi/scsi-disk.c      | 17 +++++++++++++++++
>   include/hw/scsi/scsi.h   |  3 +++
>   include/scsi/constants.h |  1 +
>   3 files changed, 21 insertions(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index d89cdd4e4a..5de4506b97 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1085,6 +1085,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>           [MODE_PAGE_R_W_ERROR]              = (1 << TYPE_DISK) | (1 << TYPE_ROM),
>           [MODE_PAGE_AUDIO_CTL]              = (1 << TYPE_ROM),
>           [MODE_PAGE_CAPABILITIES]           = (1 << TYPE_ROM),
> +        [MODE_PAGE_APPLE_VENDOR]           = (1 << TYPE_ROM),
>       };
>   
>       uint8_t *p = *p_outbuf + 2;
> @@ -1229,6 +1230,20 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>           p[19] = (16 * 176) & 0xff;
>           break;
>   
> +     case MODE_PAGE_APPLE_VENDOR:
> +        if (s->quirks & (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR)) {
> +            length = 0x24;
> +            if (page_control == 1) { /* Changeable Values */
> +                break;
> +            }
> +
> +            memset(p, 0, length);
> +            strcpy((char *)p + 8, "APPLE COMPUTER, INC   ");
> +            break;
> +        } else {
> +            return -1;
> +        }
> +
>       default:
>           return -1;
>       }
> @@ -3042,6 +3057,8 @@ static Property scsi_hd_properties[] = {
>       DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
>       DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version,
>                         5),
> +    DEFINE_PROP_BIT("quirk_mode_page_apple_vendor", SCSIDiskState, quirks,
> +                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
>       DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
>       DEFINE_PROP_END_OF_LIST(),
>   };
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 1ffb367f94..975d462347 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -226,4 +226,7 @@ SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun);
>   /* scsi-generic.c. */
>   extern const SCSIReqOps scsi_generic_req_ops;
>   
> +/* scsi-disk.c */
> +#define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR     0
> +
>   #endif
> diff --git a/include/scsi/constants.h b/include/scsi/constants.h
> index 2a32c08b5e..891aa0f45c 100644
> --- a/include/scsi/constants.h
> +++ b/include/scsi/constants.h
> @@ -234,6 +234,7 @@
>   #define MODE_PAGE_FAULT_FAIL                  0x1c
>   #define MODE_PAGE_TO_PROTECT                  0x1d
>   #define MODE_PAGE_CAPABILITIES                0x2a
> +#define MODE_PAGE_APPLE_VENDOR                0x30
>   #define MODE_PAGE_ALLS                        0x3f
>   /* Not in Mt. Fuji, but in ATAPI 2.6 -- deprecated now in favor
>    * of MODE_PAGE_SENSE_POWER */

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH v2 07/11] scsi-disk: allow truncated MODE SELECT requests

[Mark Cave-Ayland]

On 26/05/2022 13:06, Paolo Bonzini wrote:

> On 4/24/22 18:49, Mark Cave-Ayland wrote:
>> According to [1] this truncated request is accepted on real hardware whereas in
>> QEMU it generates an INVALID_PARAM_LEN sense code which causes A/UX to get stuck
>> in a loop retrying the command in an attempt to succeed.
> 
> That's for MODE SENSE, not MODE SELECT.
> 
> Truncated MODE SELECT is a bit more iffy, so I'd rather have a quirk for this as well.

Okay let me double-check this again to confirm whether the issue is with MODE SENSE, 
MODE SELECT or both. Adding a quirk to control the behaviour is fairly easy to do.

> Paolo
> 
>> Alter the mode page request length check so that truncated requests are allowed
>> as per real hardware, adding a trace event to enable the condition to be detected.
>>
>> [1]https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444 
>>
>>
>> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/scsi/scsi-disk.c  | 2 +-
>>   hw/scsi/trace-events | 1 +
>>   2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 71fdf132c1..c657e4f5da 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -1525,7 +1525,7 @@ static int mode_select_pages(SCSIDiskReq *r, uint8_t *p, int 
>> len, bool change)
>>               goto invalid_param;
>>           }
>>           if (page_len > len) {
>> -            goto invalid_param_len;
>> +            trace_scsi_disk_mode_select_page_truncated(page, page_len, len);
>>           }
> 

ATB,

Mark.

Re: [PATCH v2 09/11] scsi-disk: allow MODE SELECT block descriptor to set the ROM device block size

[Mark Cave-Ayland]

On 26/05/2022 13:08, Paolo Bonzini wrote:

> On 4/24/22 18:49, Mark Cave-Ayland wrote:
>> Whilst CDROM drives usually have a 2048 byte sector size, older drives have the
>> ability to switch between 2048 byte and 512 byte sector sizes by specifying a
>> block descriptor in the MODE SELECT command.
>>
>> If a MODE SELECT block descriptor is provided, update the scsi-cd device block
>> size with the provided value accordingly.
>>
>> This allows CDROMs to be used with A/UX whose driver only works with a 512 byte
>> sector size.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> Why do this only for MMC?

No good reason other than I only have an guest OS that does this for CDROMs :)  I can 
easily drop the TYPE_ROM check if you don't think it will cause any unexpected issues 
with other SCSI devices?

> Paolo
> 
>> ---
>>   hw/scsi/scsi-disk.c  | 7 +++++++
>>   hw/scsi/trace-events | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index 6991493cf4..41ebbe3045 100644
>> --- a/hw/scsi/scsi-disk.c
>> +++ b/hw/scsi/scsi-disk.c
>> @@ -1583,6 +1583,13 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, 
>> uint8_t *inbuf)
>>           goto invalid_param;
>>       }
>> +    /* Allow changing the block size of ROM devices */
>> +    if (s->qdev.type == TYPE_ROM && bd_len &&
>> +        p[6] != (s->qdev.blocksize >> 8)) {
>> +            s->qdev.blocksize = p[6] << 8;
>> +            trace_scsi_disk_mode_select_rom_set_blocksize(s->qdev.blocksize);
>> +    }
>> +
>>       len -= bd_len;
>>       p += bd_len;
>> diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
>> index 25eae9f307..1a021ddae9 100644
>> --- a/hw/scsi/trace-events
>> +++ b/hw/scsi/trace-events
>> @@ -340,6 +340,7 @@ scsi_disk_dma_command_WRITE(const char *cmd, uint64_t lba, int 
>> len) "Write %s(se
>>   scsi_disk_new_request(uint32_t lun, uint32_t tag, const char *line) "Command: 
>> lun=%d tag=0x%x data=%s"
>>   scsi_disk_aio_sgio_command(uint32_t tag, uint8_t cmd, uint64_t lba, int len, 
>> uint32_t timeout) "disk aio sgio: tag=0x%x cmd=0x%x (sector %" PRId64 ", count %d) 
>> timeout=%u"
>>   scsi_disk_mode_select_page_truncated(int page, int len, int page_len) "page %d 
>> expected length %d but received length %d"
>> +scsi_disk_mode_select_rom_set_blocksize(int blocksize) "set ROM block size to %d"
>>   # scsi-generic.c
>>   scsi_generic_command_complete_noio(void *req, uint32_t tag, int statuc) "Command 
>> complete %p tag=0x%x status=%d"


ATB,

Mark.