[PATCH v3 00/14] scsi: add quirks and features to support m68k Macs

[PATCH v3 00/14] 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.

Patch 1 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 2 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 3 adds compat_props to the q800 machine which enable the new
MODE_PAGE_APPLE_VENDOR quirk for all scsi-cd devices attached to the machine.

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

Patches 6 and 7 implement a new MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk to
allow PF=0 MODE SELECT requests which are used by both MacOS and A/UX, along
with a MODE_PAGE_VENDOR_SPECIFIC (0x0) mode page compatible with MacOS. Once
again this quirk is only enabled for SCSI devices on the q800 machine.

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

Patches 9 and 10 add support for allowing truncated MODE SELECT requests which are
sent by A/UX when enumerating a SCSI CDROM device. Allowing these broken requests
is protected by a new MODE_PAGE_TRUNCATED quirk which is only enabled for SCSI
CDROM devices attached to the q800 machine.

Patch 11 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 12 adds support for setting the SCSI block size via a MODE SELECT request
which is most commonly used 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 13 and 14 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 detected and initialised during the
installation process.

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

v3:
[Note from v2: this series has changed in structure and functionality based upon
bug reports from Howard off-list regarding detection/changing of CDROM media in
both A/UX and MacOS]

- Rearrange order to aid bisecting differences between CDROM and DISK quirks
- Add R-B tags from Laurent and Phil
- Replace %zd with %zu in trace-events in patch 8
- Add a new SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk to handle truncated MODE SELECT
  requests
- Rename SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk to
  SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD since due to additional changes in this series
  the DBD bit can be honoured rather than forced off
- Add support for PF=0 MODE SELECT commands a and new MODE_PAGE_VENDOR_SPECIFIC (0x0) page
  with a suitable implementation for MacOS protected by a new
  SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk (this fixes detection of CDROM
  media in some cases)
- Allow the SCSI block size to be set for both CDROMs and DISKs as requested by Paolo

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 (14):
  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-cd devices
  scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD quirk for
    Macintosh
  q800: implement compat_props to enable quirk_mode_sense_rom_use_dbd
    for scsi-cd devices
  scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk
    for Macintosh
  q800: implement compat_props to enable
    quirk_mode_page_vendor_specific_apple for scsi devices
  scsi-disk: add FORMAT UNIT command
  scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh
  q800: implement compat_props to enable quirk_mode_page_truncated for
    scsi-cd devices
  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 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           | 16 +++++++
 hw/scsi/scsi-disk.c      | 96 +++++++++++++++++++++++++++++++++++++---
 hw/scsi/trace-events     |  3 ++
 include/hw/scsi/scsi.h   |  6 +++
 include/scsi/constants.h |  2 +
 5 files changed, 116 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH v3 01/14] 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 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 072686ed58..8c28dd8566 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.30.2

[PATCH v3 02/14] scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh

[Mark Cave-Ayland]

One of the mechanisms MacOS uses to identify CDROM 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 CDROM 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 CDROM drives
attached to non-Apple machines function exactly as before.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 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 8c28dd8566..64f9418dc9 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 = 0x1e;
+            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;
     }
@@ -3086,6 +3101,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_page_apple_vendor", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1ffb367f94..e090ea1b40 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.30.2

[PATCH v3 03/14] q800: implement compat_props to enable quirk_mode_page_apple_vendor for scsi-cd devices

[Mark Cave-Ayland]

By default quirk_mode_page_apple_vendor should be enabled for all scsi-cd 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..6fabd35529 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-cd", "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.30.2

[PATCH v3 04/14] scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD quirk for Macintosh

[Mark Cave-Ayland]

During SCSI bus enumeration A/UX sends a MODE SENSE command to the CDROM with
the DBD bit unset 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 requested block
descriptor to 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_USE_DBD quirk to allow this behaviour
to be enabled as required. Note that an additional workaround is required for
the previous SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk which must never
return a block descriptor even though the DBD bit is left unset.

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

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 64f9418dc9..41bcf07272 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1279,10 +1279,27 @@ 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_USE_DBD)) {
+            /* Use DBD from the request... */
+            dev_specific_param = 0x00;
+
+            /*
+             * ... unless we receive a request for MODE_PAGE_APPLE_VENDOR
+             * which should never return a block descriptor even though DBD is
+             * not set, otherwise CDROM detection fails in MacOS
+             */
+            if (s->quirks & (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR) &&
+                page == MODE_PAGE_APPLE_VENDOR) {
+                dbd = true;
+            }
+        } 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) {
@@ -3103,6 +3120,8 @@ static Property scsi_cd_properties[] = {
                       5),
     DEFINE_PROP_BIT("quirk_mode_page_apple_vendor", SCSIDiskState, quirks,
                     SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
+    DEFINE_PROP_BIT("quirk_mode_sense_rom_use_dbd", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index e090ea1b40..845d05722b 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_USE_DBD             1
 
 #endif
-- 
2.30.2

[PATCH v3 05/14] q800: implement compat_props to enable quirk_mode_sense_rom_use_dbd for scsi-cd devices

[Mark Cave-Ayland]

By default quirk_mode_sense_rom_use_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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6fabd35529..4745f72c92 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-cd", "quirk_mode_page_apple_vendor", "on"},
+    { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.30.2

[PATCH v3 06/14] scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk for Macintosh

[Mark Cave-Ayland]

Both MacOS and A/UX make use of vendor-specific MODE SELECT commands with PF=0
to identify SCSI devices:

- MacOS sends a MODE SELECT command with PF=0 for the MODE_PAGE_VENDOR_SPECIFIC
  (0x0) mode page containing 2 bytes before initialising a disk

- A/UX (installed on disk) sends a MODE SELECT command with PF=0 during SCSI
  bus enumeration, and gets stuck in an infinite loop if it fails

Add a new SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk to allow both
PF=0 MODE SELECT commands and implement a MODE_PAGE_VENDOR_SPECIFIC (0x0)
mode page which is compatible with MacOS.

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

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 41bcf07272..ce915f326e 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1079,6 +1079,7 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
                            int page_control)
 {
     static const int mode_sense_valid[0x3f] = {
+        [MODE_PAGE_VENDOR_SPECIFIC]        = (1 << TYPE_DISK) | (1 << TYPE_ROM),
         [MODE_PAGE_HD_GEOMETRY]            = (1 << TYPE_DISK),
         [MODE_PAGE_FLEXIBLE_DISK_GEOMETRY] = (1 << TYPE_DISK),
         [MODE_PAGE_CACHING]                = (1 << TYPE_DISK) | (1 << TYPE_ROM),
@@ -1244,6 +1245,22 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
             return -1;
         }
 
+    case MODE_PAGE_VENDOR_SPECIFIC:
+        if (s->qdev.type == TYPE_DISK && (s->quirks &
+            (1 << SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE))) {
+            length = 0x2;
+            if (page_control == 1) { /* Changeable Values */
+                p[0] = 0xff;
+                p[1] = 0xff;
+                break;
+            }
+            p[0] = 0;
+            p[1] = 0;
+            break;
+        } else {
+            return -1;
+        }
+
     default:
         return -1;
     }
@@ -1570,9 +1587,12 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
     int bd_len;
     int pass;
 
-    /* We only support PF=1, SP=0.  */
     if ((r->req.cmd.buf[1] & 0x11) != 0x10) {
-        goto invalid_field;
+        if (!(s->quirks &
+            (1 << SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE))) {
+            /* We only support PF=1, SP=0.  */
+            goto invalid_field;
+        }
     }
 
     if (len < hdr_len) {
@@ -3070,6 +3090,9 @@ 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_vendor_specific_apple", SCSIDiskState,
+                    quirks, SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE,
+                    0),
     DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
     DEFINE_PROP_END_OF_LIST(),
 };
@@ -3122,6 +3145,9 @@ static Property scsi_cd_properties[] = {
                     SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
     DEFINE_PROP_BIT("quirk_mode_sense_rom_use_dbd", SCSIDiskState, quirks,
                     SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD, 0),
+    DEFINE_PROP_BIT("quirk_mode_page_vendor_specific_apple", SCSIDiskState,
+                    quirks, SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE,
+                    0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 845d05722b..011cb84753 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -229,5 +229,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_USE_DBD             1
+#define SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE    2
 
 #endif
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 891aa0f45c..6a8bad556a 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -225,6 +225,7 @@
 #define TYPE_NO_LUN         0x7f
 
 /* Mode page codes for mode sense/set */
+#define MODE_PAGE_VENDOR_SPECIFIC             0x00
 #define MODE_PAGE_R_W_ERROR                   0x01
 #define MODE_PAGE_HD_GEOMETRY                 0x04
 #define MODE_PAGE_FLEXIBLE_DISK_GEOMETRY      0x05
-- 
2.30.2

[PATCH v3 07/14] q800: implement compat_props to enable quirk_mode_page_vendor_specific_apple for scsi devices

[Mark Cave-Ayland]

By default quirk_mode_page_vendor_specific_apple should be enabled for both scsi-hd
and scsi-cd devices to allow MacOS to format SCSI disk devices, and A/UX to
enumerate SCSI CDROM devices succesfully without getting stuck in a loop.

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

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 4745f72c92..b774a5b20a 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -687,8 +687,10 @@ static void q800_init(MachineState *machine)
 }
 
 static GlobalProperty hw_compat_q800[] = {
+    { "scsi-hd", "quirk_mode_page_vendor_specific_apple", "on"},
     { "scsi-cd", "quirk_mode_page_apple_vendor", "on"},
     { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
+    { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.30.2

[PATCH v3 08/14] 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>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 ce915f326e..f09c1acae2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2180,6 +2180,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]));
@@ -2586,6 +2589,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..03b2640934 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 %zu)"
 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.30.2

[PATCH v3 09/14] scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh

[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.

The work at [1] suggests that 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
if the SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk is enabled, whilst also 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    | 7 ++++++-
 hw/scsi/trace-events   | 1 +
 include/hw/scsi/scsi.h | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index f09c1acae2..b225ec1a4c 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1552,7 +1552,10 @@ 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;
+            if (!(s->quirks & SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED)) {
+                goto invalid_param_len;
+            }
+            trace_scsi_disk_mode_select_page_truncated(page, page_len, len);
         }
 
         if (!change) {
@@ -3152,6 +3155,8 @@ static Property scsi_cd_properties[] = {
     DEFINE_PROP_BIT("quirk_mode_page_vendor_specific_apple", SCSIDiskState,
                     quirks, SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE,
                     0),
+    DEFINE_PROP_BIT("quirk_mode_page_truncated", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 03b2640934..8e927ff62d 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"
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 011cb84753..e284e3a4ec 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -230,5 +230,6 @@ extern const SCSIReqOps scsi_generic_req_ops;
 #define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR             0
 #define SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD             1
 #define SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE    2
+#define SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED                3
 
 #endif
-- 
2.30.2

[PATCH v3 10/14] q800: implement compat_props to enable quirk_mode_page_truncated for scsi-cd devices

[Mark Cave-Ayland]

By default quirk_mode_page_truncated should be enabled for all scsi-cd devices
connected to the q800 machine to allow A/UX to enumerate SCSI CDROM devices
without hanging.

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 b774a5b20a..3254ffb5c4 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -691,6 +691,7 @@ static GlobalProperty hw_compat_q800[] = {
     { "scsi-cd", "quirk_mode_page_apple_vendor", "on"},
     { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
     { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
+    { "scsi-cd", "quirk_mode_page_truncated", "on"},
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.30.2

[PATCH v3 11/14] 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 b225ec1a4c..3659583e13 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1188,6 +1188,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.30.2

[PATCH v3 12/14] scsi-disk: allow MODE SELECT block descriptor to set the block size

[Mark Cave-Ayland]

The MODE SELECT command can contain an optional block descriptor that can be used
to set the device block size. If the block descriptor is present then update the
block size on the SCSI device accordingly.

This allows CDROMs to be used with A/UX which requires a CDROM drive which is
capable of switching from a 2048 byte sector size to a 512 byte sector size.

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

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 3659583e13..9c8bad0283 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1616,6 +1616,12 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
         goto invalid_param;
     }
 
+    /* Allow changing the block size */
+    if (bd_len && p[6] != (s->qdev.blocksize >> 8)) {
+        s->qdev.blocksize = p[6] << 8;
+        trace_scsi_disk_mode_select_set_blocksize(s->qdev.blocksize);
+    }
+
     len -= bd_len;
     p += bd_len;
 
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 8e927ff62d..ab238293f0 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_set_blocksize(int blocksize) "set 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.30.2

[PATCH v3 13/14] 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 3254ffb5c4..dccf192e55 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_vendor_specific_apple", "on"},
+    { "scsi-hd", "vendor", " SEAGATE" },
+    { "scsi-hd", "product", "          ST225N" },
+    { "scsi-hd", "ver", "1.0 " },
     { "scsi-cd", "quirk_mode_page_apple_vendor", "on"},
     { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
     { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
-- 
2.30.2

[PATCH v3 14/14] 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>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/m68k/q800.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index dccf192e55..101ab0f803 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -695,6 +695,9 @@ static GlobalProperty hw_compat_q800[] = {
     { "scsi-cd", "quirk_mode_sense_rom_use_dbd", "on"},
     { "scsi-cd", "quirk_mode_page_vendor_specific_apple", "on"},
     { "scsi-cd", "quirk_mode_page_truncated", "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.30.2

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

[Mark Cave-Ayland]

On 22/06/2022 11:53, 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.
> 
> Patch 1 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 2 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 3 adds compat_props to the q800 machine which enable the new
> MODE_PAGE_APPLE_VENDOR quirk for all scsi-cd devices attached to the machine.
> 
> Patch 4 adds a new quirk to force SCSI CDROMs to always honour the block
> descriptor for a MODE SENSE command which is expected by A/UX, whilst patch 5
> enables the quirk for all scsi-cd devices on the q800 machine.
> 
> Patches 6 and 7 implement a new MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk to
> allow PF=0 MODE SELECT requests which are used by both MacOS and A/UX, along
> with a MODE_PAGE_VENDOR_SPECIFIC (0x0) mode page compatible with MacOS. Once
> again this quirk is only enabled for SCSI devices on the q800 machine.
> 
> Patch 8 implements a dummy FORMAT UNIT command which is used by the Apple HD SC
> Setup program when preparing an empty disk to install MacOS.
> 
> Patches 9 and 10 add support for allowing truncated MODE SELECT requests which are
> sent by A/UX when enumerating a SCSI CDROM device. Allowing these broken requests
> is protected by a new MODE_PAGE_TRUNCATED quirk which is only enabled for SCSI
> CDROM devices attached to the q800 machine.
> 
> Patch 11 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 12 adds support for setting the SCSI block size via a MODE SELECT request
> which is most commonly used 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 13 and 14 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 detected and initialised during the
> installation process.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> v3:
> [Note from v2: this series has changed in structure and functionality based upon
> bug reports from Howard off-list regarding detection/changing of CDROM media in
> both A/UX and MacOS]
> 
> - Rearrange order to aid bisecting differences between CDROM and DISK quirks
> - Add R-B tags from Laurent and Phil
> - Replace %zd with %zu in trace-events in patch 8
> - Add a new SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk to handle truncated MODE SELECT
>    requests
> - Rename SCSI_DISK_QUIRK_MODE_SENSE_ROM_FORCE_DBD quirk to
>    SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD since due to additional changes in this series
>    the DBD bit can be honoured rather than forced off
> - Add support for PF=0 MODE SELECT commands a and new MODE_PAGE_VENDOR_SPECIFIC (0x0) page
>    with a suitable implementation for MacOS protected by a new
>    SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk (this fixes detection of CDROM
>    media in some cases)
> - Allow the SCSI block size to be set for both CDROMs and DISKs as requested by Paolo
> 
> 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 (14):
>    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-cd devices
>    scsi-disk: add SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD quirk for
>      Macintosh
>    q800: implement compat_props to enable quirk_mode_sense_rom_use_dbd
>      for scsi-cd devices
>    scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk
>      for Macintosh
>    q800: implement compat_props to enable
>      quirk_mode_page_vendor_specific_apple for scsi devices
>    scsi-disk: add FORMAT UNIT command
>    scsi-disk: add SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk for Macintosh
>    q800: implement compat_props to enable quirk_mode_page_truncated for
>      scsi-cd devices
>    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 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           | 16 +++++++
>   hw/scsi/scsi-disk.c      | 96 +++++++++++++++++++++++++++++++++++++---
>   hw/scsi/trace-events     |  3 ++
>   include/hw/scsi/scsi.h   |  6 +++
>   include/scsi/constants.h |  2 +
>   5 files changed, 116 insertions(+), 7 deletions(-)

Ping?


ATB,

Mark.

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

[Paolo Bonzini]

Queued, thanks (I was on vacation last week).

I am a bit scared about the mode_select_truncated quirk.  My reading
of the code is that the MODE SELECT would fail anyway because the
page length does not match in scsi_disk_check_mode_select:

    len = mode_sense_page(s, page, &p, 0);
    if (len < 0 || len != expected_len) {
        return -1;
    }

Is that correct?  If not, I'm not sure where I am wrong.  If so,
I wonder if it is enough for the quirk to do just a "goto invalid_param;" 
in place of invalid_param_len.

Thanks,

Paolo

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

[Mark Cave-Ayland]

On 12/07/2022 15:48, Paolo Bonzini wrote:

> Queued, thanks (I was on vacation last week).
> 
> I am a bit scared about the mode_select_truncated quirk.  My reading
> of the code is that the MODE SELECT would fail anyway because the
> page length does not match in scsi_disk_check_mode_select:
> 
>      len = mode_sense_page(s, page, &p, 0);
>      if (len < 0 || len != expected_len) {
>          return -1;
>      }
> 
> Is that correct?  If not, I'm not sure where I am wrong.  If so,
> I wonder if it is enough for the quirk to do just a "goto invalid_param;"
> in place of invalid_param_len.

(goes and looks)

The truncated MODE SELECT request in question seems to be only used by the initial 
A/UX installation image and looks like this:

scsi_req_parsed target 3 lun 0 tag 0 command 21 dir 2 length 20
scsi_req_parsed_lba target 3 lun 0 tag 0 command 21 lba 0
scsi_req_alloc target 3 lun 0 tag 0
scsi_disk_new_request Command: lun=0 tag=0x0 data= 0x15 0x00 0x00 0x00 0x14 0x00
scsi_disk_emulate_command_MODE_SELECT Mode Select(6) (len 20)
scsi_req_continue target 3 lun 0 tag 0
scsi_disk_emulate_write_data Write buf_len=20
scsi_req_data target 3 lun 0 tag 0 len 20
scsi_req_continue target 3 lun 0 tag 0

This includes an 8 byte block descriptor which is used to set the CDROM sector size 
to 512 bytes and so the request content looks like:

4 bytes hdr_len for MODE SELECT(6)
8 bytes bd_len for the block descriptor
8 bytes of data for page 0 (MODE_PAGE_R_W_ERROR) data with page_len = 10

Stepping through mode_select_pages() in the debugger shows that since page_len is set 
correctly to 10 bytes (which matches the expected length in mode_sense_page()) the 
above check succeeds.

I suspect what happened is that the original author built the MODE_PAGE_R_W_ERROR 
page data correctly but miscalculated the length of the request in the CDB. Given 
that the truncated request is seemingly accepted on real hardware, no-one actually 
noticed until now :)


ATB,

Mark.