On Mon, Dec 30, 2019 at 06:59:35PM +1000, Jonathan Matthew wrote:
> Here's the Solaris explanation:
>
> https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/scsi/adapters/mpt_sas/mptsas_var.h#L195
Thanks for digging.
> I think we should copy what they're doing here, that is, replace the high bits
> with 3 rather than adding 3 to it. I'm really not sure where we should do
> this though. Maybe in mpii, but only on sparc64?
As that is now a general quirk in all RAID volumes and no longer
specific to bootpath handling, mpii seems only appropiate and autoconf
must not know about this.
Since Illumos does exactly that with mptsas_get_raid_wwid() in
mptsas_raidconf_page_0_cb(), which after a brief look seems like the
code path equivalent to our recent WWID addition in mpii_scsi_probe(),
I'm inclined to just do it there.
Diff below doas that.
> As far as I can tell, the raid controller generates 128 bit WWIDs for raid
> volumes, but only has a 64 bit field to report the ID to the host, so it only
> puts the vendor-specified part here (you can see the last half of the ID
> string
> printed when sd0 attaches matches sl->port_wwn in reverse). I think the
> purpose of setting the high 4 bits to 3 is that 3 is not a defined NAA value,
> so you're not going to find a proper WWID coming from other device that will
> match that.
I did not manage to recognise this detail of the reversed ID, indeed:
sd0 at scsibus1 targ 0 lun 0: <LSI, Logical Volume, 3000>
naa.600508e0000000006cd1dcd59022a30a
Feedback? OK?
Index: dev/pci/mpii.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/mpii.c,v
retrieving revision 1.123
diff -u -p -r1.123 mpii.c
--- dev/pci/mpii.c 29 Dec 2019 21:30:21 -0000 1.123
+++ dev/pci/mpii.c 30 Dec 2019 14:26:57 -0000
@@ -930,6 +930,15 @@ mpii_scsi_probe(struct scsi_link *link)
return (EINVAL);
link->port_wwn = letoh64(vpg.wwid);
+#ifdef __sparc64__
+ /*
+ * WWIDs generated by LSI firmware are not IEEE NAA compliant
+ * so historical practise in OBP is to set the top nibble to 3
+ * to indicate that this is a RAID volume.
+ */
+ link->port_wwn &= 0x0fffffffffffffff;
+ link->port_wwn |= 0x3000000000000000;
+#endif
return (0);
}
Index: arch/sparc64/sparc64/autoconf.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/autoconf.c,v
retrieving revision 1.133
diff -u -p -r1.133 autoconf.c
--- arch/sparc64/sparc64/autoconf.c 15 Oct 2019 05:21:16 -0000 1.133
+++ arch/sparc64/sparc64/autoconf.c 30 Dec 2019 14:27:17 -0000
@@ -1455,7 +1455,7 @@ device_register(struct device *dev, void
u_int lun = bp->val[1];
if (bp->val[0] & 0xffffffff00000000 && bp->val[0] != -1) {
- /* Fibre channel? */
+ /* Hardware RAID or Fibre channel? */
if (bp->val[0] == sl->port_wwn && lun == sl->lun) {
nail_bootdev(dev, bp);
}
