> Date: Tue, 31 Dec 2019 09:12:56 +1000
> From: Jonathan Matthew <jonat...@d14n.org>
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Mon, Dec 30, 2019 at 03:36:54PM +0100, Klemens Nanni wrote:
> > 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?
>
> I think this is probably the most sensible thing we can do here.
> ok jmatthew@ but I'd wait and see if anyone has a better idea.
Can we leave out the #ifdef __sparc64__? Unless somebody can come up
with a really good reason for it...
> > 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);
> > }
>
>
