[PATCH 0/6] scsi: add support for FORMAT UNIT command and quirks

[PATCH 0/6] scsi: add support for FORMAT UNIT command and quirks

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

Patches 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 MODE_PAGE_APPLE
quirk for all scsi-hd devices attached to the machine.

Finally patches 5 and 6 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>


Mark Cave-Ayland (6):
  scsi-disk: add FORMAT UNIT command
  scsi-disk: add new quirks bitmap to SCSIDiskState
  scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh
  q800: implement compat_props to enable quirk_mode_page_apple for
    scsi-hd devices
  q800: add default vendor, product and version information for scsi-hd
    devices
  q800: add default vendor, product and version information for scsi-cd
    devices

 hw/m68k/q800.c           | 12 ++++++++++++
 hw/scsi/scsi-disk.c      | 24 ++++++++++++++++++++++++
 hw/scsi/trace-events     |  1 +
 include/hw/scsi/scsi.h   |  3 +++
 include/scsi/constants.h |  1 +
 5 files changed, 41 insertions(+)

-- 
2.20.1

[PATCH 1/6] 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 2/6] 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 3/6] scsi-disk: add MODE_PAGE_APPLE 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 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      | 19 +++++++++++++++++++
 include/hw/scsi/scsi.h   |  3 +++
 include/scsi/constants.h |  1 +
 3 files changed, 23 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d89cdd4e4a..37013756d5 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]                  = (1 << TYPE_ROM),
     };
 
     uint8_t *p = *p_outbuf + 2;
@@ -1229,6 +1230,22 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
         p[19] = (16 * 176) & 0xff;
         break;
 
+     case MODE_PAGE_APPLE:
+        if (s->qdev.type == TYPE_DISK &&
+            (s->quirks & SCSI_DISK_QUIRK_MODE_PAGE_APPLE)) {
+
+            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 +3059,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", SCSIDiskState, quirks,
+                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE, 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..f629706250 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 0
+
 #endif
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 2a32c08b5e..21ca7b50cd 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                       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 4/6] q800: implement compat_props to enable quirk_mode_page_apple for scsi-hd devices

[Mark Cave-Ayland]

By default quirk_mode_page_apple 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..cfab0c54fc 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", "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 5/6] q800: add default vendor, product and version 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 cfab0c54fc..a5d20bb64b 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", "on"},
+    { "scsi-hd", "vendor", " SEAGATE" },
+    { "scsi-hd", "product", "          ST225N" },
+    { "scsi-hd", "ver", "1.0 " },
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
-- 
2.20.1

[PATCH 6/6] q800: add default vendor, product and version 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 a5d20bb64b..e050d45040 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -691,6 +691,9 @@ static GlobalProperty hw_compat_q800[] = {
     { "scsi-hd", "vendor", " SEAGATE" },
     { "scsi-hd", "product", "          ST225N" },
     { "scsi-hd", "ver", "1.0 " },
+    { "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 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh

[Fam Zheng]

On 2022-04-21 07:51, Mark Cave-Ayland wrote:
> 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 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      | 19 +++++++++++++++++++
>  include/hw/scsi/scsi.h   |  3 +++
>  include/scsi/constants.h |  1 +
>  3 files changed, 23 insertions(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index d89cdd4e4a..37013756d5 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]                  = (1 << TYPE_ROM),
>      };
>  
>      uint8_t *p = *p_outbuf + 2;
> @@ -1229,6 +1230,22 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>          p[19] = (16 * 176) & 0xff;
>          break;
>  
> +     case MODE_PAGE_APPLE:
> +        if (s->qdev.type == TYPE_DISK &&
> +            (s->quirks & SCSI_DISK_QUIRK_MODE_PAGE_APPLE)) {

This is always false. SCSI_DISK_QUIRK_MODE_PAGE_APPLE is defined 0.

You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.

> +
> +            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 +3059,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", SCSIDiskState, quirks,
> +                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE, 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..f629706250 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 0
> +
>  #endif
> diff --git a/include/scsi/constants.h b/include/scsi/constants.h
> index 2a32c08b5e..21ca7b50cd 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                       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
> 
> 

Fam

Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh

[Mark Cave-Ayland]

On 21/04/2022 14:27, Fam Zheng wrote:

> On 2022-04-21 07:51, Mark Cave-Ayland wrote:
>> 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 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      | 19 +++++++++++++++++++
>>   include/hw/scsi/scsi.h   |  3 +++
>>   include/scsi/constants.h |  1 +
>>   3 files changed, 23 insertions(+)
>>
>> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>> index d89cdd4e4a..37013756d5 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]                  = (1 << TYPE_ROM),
>>       };
>>   
>>       uint8_t *p = *p_outbuf + 2;
>> @@ -1229,6 +1230,22 @@ static int mode_sense_page(SCSIDiskState *s, int page, uint8_t **p_outbuf,
>>           p[19] = (16 * 176) & 0xff;
>>           break;
>>   
>> +     case MODE_PAGE_APPLE:
>> +        if (s->qdev.type == TYPE_DISK &&
>> +            (s->quirks & SCSI_DISK_QUIRK_MODE_PAGE_APPLE)) {
> 
> This is always false. SCSI_DISK_QUIRK_MODE_PAGE_APPLE is defined 0.
> 
> You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.

Doh, you're absolutely right. I believe the current recommendation is to use the 
BIT() macro in these cases.

I'll wait to see if there is any further feedback on the whole quirks concept before 
posting an updated v2.

>> +
>> +            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 +3059,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", SCSIDiskState, quirks,
>> +                    SCSI_DISK_QUIRK_MODE_PAGE_APPLE, 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..f629706250 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 0
>> +
>>   #endif
>> diff --git a/include/scsi/constants.h b/include/scsi/constants.h
>> index 2a32c08b5e..21ca7b50cd 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                       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
>>
>>
> 
> Fam


ATB,

Mark.

Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh

[Richard Henderson]

On 4/21/22 08:29, Mark Cave-Ayland wrote:
>> You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.
> 
> Doh, you're absolutely right. I believe the current recommendation is to use the BIT() 
> macro in these cases.

We probably need to fix BIT() to use 1ULL.

At present it's using 1UL, to match the other (unfortunate) uses of unsigned long within 
bitops.h.  The use of BIT() for things unrelated to bitops.h just bit a recent risc-v pull 
request, in that it failed to build on all 32-bit hosts.


r~

Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh

[BALATON Zoltan]

On Thu, 21 Apr 2022, Richard Henderson wrote:
> On 4/21/22 08:29, Mark Cave-Ayland wrote:
>>> You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.
>> 
>> Doh, you're absolutely right. I believe the current recommendation is to 
>> use the BIT() macro in these cases.

I think it's not a recommendation (as in code style) but it often makes 
things simpler by reducing the number of parenthesis so using it is 
probably a good idea for readability. But if you never need the bit number 
only the value then you could define the quirks constants as that in the 
first place. (Otherwise if you want bit numbers maybe make it an enum.)

> We probably need to fix BIT() to use 1ULL.
>
> At present it's using 1UL, to match the other (unfortunate) uses of unsigned 
> long within bitops.h.  The use of BIT() for things unrelated to bitops.h just 
> bit a recent risc-v pull request, in that it failed to build on all 32-bit 
> hosts.

There's already a BIT_ULL(nr) when ULL is needed but in this case quirks 
was declared uint32_t so probably OK with UL as well. (Was this bitops.h 
taken from Linux? Keeping it compatible then may be a good idea to avoid 
confusion.)

Regards,
BALATON Zoltan

Re: [PATCH 3/6] scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh

[Mark Cave-Ayland]

On 21/04/2022 23:00, BALATON Zoltan wrote:

> On Thu, 21 Apr 2022, Richard Henderson wrote:
>> On 4/21/22 08:29, Mark Cave-Ayland wrote:
>>>> You need (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE) instead.
>>>
>>> Doh, you're absolutely right. I believe the current recommendation is to use the 
>>> BIT() macro in these cases.
> 
> I think it's not a recommendation (as in code style) but it often makes things 
> simpler by reducing the number of parenthesis so using it is probably a good idea for 
> readability. But if you never need the bit number only the value then you could 
> define the quirks constants as that in the first place. (Otherwise if you want bit 
> numbers maybe make it an enum.)
> 
>> We probably need to fix BIT() to use 1ULL.
>>
>> At present it's using 1UL, to match the other (unfortunate) uses of unsigned long 
>> within bitops.h.  The use of BIT() for things unrelated to bitops.h just bit a 
>> recent risc-v pull request, in that it failed to build on all 32-bit hosts.
> 
> There's already a BIT_ULL(nr) when ULL is needed but in this case quirks was declared 
> uint32_t so probably OK with UL as well. (Was this bitops.h taken from Linux? Keeping 
> it compatible then may be a good idea to avoid confusion.)

It seems there is still a bit of discussion around using BIT() here, so for v2 I'll 
add the shift directly with (1 << x). Then if the BIT() macro becomes suitable for 
more general use it can easily be updated as a separate patch later.


ATB,

Mark.

Re: [PATCH 0/6] scsi: add support for FORMAT UNIT command and quirks

[Mark Cave-Ayland]

On 21/04/2022 07:51, 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.
> 
> Patches 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 MODE_PAGE_APPLE
> quirk for all scsi-hd devices attached to the machine.
> 
> Finally patches 5 and 6 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>
> 
> 
> Mark Cave-Ayland (6):
>    scsi-disk: add FORMAT UNIT command
>    scsi-disk: add new quirks bitmap to SCSIDiskState
>    scsi-disk: add MODE_PAGE_APPLE quirk for Macintosh
>    q800: implement compat_props to enable quirk_mode_page_apple for
>      scsi-hd devices
>    q800: add default vendor, product and version information for scsi-hd
>      devices
>    q800: add default vendor, product and version information for scsi-cd
>      devices
> 
>   hw/m68k/q800.c           | 12 ++++++++++++
>   hw/scsi/scsi-disk.c      | 24 ++++++++++++++++++++++++
>   hw/scsi/trace-events     |  1 +
>   include/hw/scsi/scsi.h   |  3 +++
>   include/scsi/constants.h |  1 +
>   5 files changed, 41 insertions(+)

I was fortunate enough to find a really good reference to some work done over on 
68mla.org reverse engineering Apple's HD SC Setup and SCSI device detection. This 
pointed me towards a couple of additional SCSI changes for QEMU that also fix CDROM 
access under A/UX which I shall include in an updated v2.


ATB,

Mark.

Re: [PATCH 1/6] scsi-disk: add FORMAT UNIT command

[Philippe Mathieu-Daudé via]

On 21/4/22 08:51, Mark Cave-Ayland wrote:
> 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/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)"

%zu (%zd is for ssize_t), otherwise:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>