Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 May 2017 19:14:44 +0000 (UTC)
From:      Stephen McConnell <slm@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r318895 - in head: share/man/man4 sys/dev/mps
Message-ID:  <201705251914.v4PJEi3l075183@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: slm
Date: Thu May 25 19:14:44 2017
New Revision: 318895
URL: https://svnweb.freebsd.org/changeset/base/318895

Log:
  Fix several problems with mapping code.
  
  Reviewed by:    ken, scottl, asomers, ambrisko, mav
  Approved by:	ken, mav
  MFC after:      1 week
  Differential Revision: https://reviews.freebsd.org/D10878

Modified:
  head/share/man/man4/mps.4
  head/sys/dev/mps/mps.c
  head/sys/dev/mps/mps_mapping.c
  head/sys/dev/mps/mps_sas.c
  head/sys/dev/mps/mps_sas_lsi.c
  head/sys/dev/mps/mps_user.c
  head/sys/dev/mps/mpsvar.h

Modified: head/share/man/man4/mps.4
==============================================================================
--- head/share/man/man4/mps.4	Thu May 25 19:02:54 2017	(r318894)
+++ head/share/man/man4/mps.4	Thu May 25 19:14:44 2017	(r318895)
@@ -1,8 +1,8 @@
 .\"
 .\" Copyright (c) 2010 Spectra Logic Corporation
 .\" Copyright (c) 2014 LSI Corp
-.\" Copyright (c) 2016 Avago Technologies
-.\" Copyright (c) 2016 Broadcom Ltd.
+.\" Copyright (c) 2015-2017 Avago Technologies
+.\" Copyright (c) 2015-2017 Broadcom Ltd.
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -38,7 +38,7 @@
 .\" $Id: //depot/SpectraBSD/head/share/man/man4/mps.4#6 $
 .\" $FreeBSD$
 .\"
-.Dd July 5, 2016
+.Dd May 25, 2017
 .Dt MPS 4
 .Os
 .Sh NAME
@@ -241,13 +241,13 @@ Send SSU to HDDs, but not to SSDs.
 Send SSU to both HDDs and SSDs.
 .El
 .Pp
-To control the feature for a specific adapter, set this tunable value in
+To control this feature for a specific adapter, set this tunable value in
 .Xr loader.conf 5 :
 .Bd -literal -offset indent
 dev.mps.X.enable_ssu
 .Ed
 .Pp
-The same set of values are valid when setting this tunable for all adapters.
+The same set of values are valid as when setting this tunable for all adapters.
 .Pp
 SATA disks that take several seconds to spin up and fail the SATA Identify
 command might not be discovered by the driver.
@@ -273,6 +273,45 @@ dev.mps.X.spinup_wait_time=NNNN
 tunable.
 NNNN is the number of seconds to wait for SATA devices to spin up when they fail
 the initial SATA Identify command.
+.Pp
+The driver can map devices discovered by the adapter so that target IDs
+corresponding to a specific device persist across resets and reboots.
+In some cases it is possible for devices to lose their mapped IDs due to
+unexpected behavior from certain hardware, such as some types of enclosures.
+To overcome this problem, a tunable is provided that will force the driver to
+map devices using the Phy number associated with the device.
+This feature is not recommended if the topology includes multiple
+enclosures/expanders.
+If multiple enclosures/expanders are present in the topology, Phy numbers are
+repeated, causing all devices at these Phy numbers except the first device to
+fail enumeration.
+To control this feature for all adapters, set the
+.Bd -literal -offset indent
+hw.mps.use_phy_num
+.Ed
+.Pp
+tunable in
+.Xr loader.conf 5
+to one of these values:
+.Bl -tag -width 6n -offset indent
+.It -1
+Only use Phy numbers to map devices and bypass the driver's mapping logic.
+.It 0
+Never use Phy numbers to map devices.
+.It 1
+Use Phy numbers to map devices, but only if the driver's mapping logic fails
+to map the device that is being enumerated.
+This is the default value.
+.El
+.Pp
+To control this feature for a specific adapter, set this tunable value in
+.Xr loader.conf 5 :
+.Bd -literal -offset indent
+dev.mps.X.use_phy_num
+.Ed
+.Pp
+The same set of values are valid as when setting this tunable for all adapters.
+.Pp
 .Sh DEBUGGING
 To enable debugging prints from the
 .Nm

Modified: head/sys/dev/mps/mps.c
==============================================================================
--- head/sys/dev/mps/mps.c	Thu May 25 19:02:54 2017	(r318894)
+++ head/sys/dev/mps/mps.c	Thu May 25 19:14:44 2017	(r318895)
@@ -505,7 +505,8 @@ mps_iocfacts_allocate(struct mps_softc *
 	 */
 	if (reallocating) {
 		mps_iocfacts_free(sc);
-		mpssas_realloc_targets(sc, saved_facts.MaxTargets);
+		mpssas_realloc_targets(sc, saved_facts.MaxTargets +
+		    saved_facts.MaxVolumes);
 	}
 
 	/*
@@ -1518,6 +1519,7 @@ mps_attach(struct mps_softc *sc)
 
 	mtx_init(&sc->mps_mtx, "MPT2SAS lock", NULL, MTX_DEF);
 	callout_init_mtx(&sc->periodic, &sc->mps_mtx, 0);
+	callout_init_mtx(&sc->device_check_callout, &sc->mps_mtx, 0);
 	TAILQ_INIT(&sc->event_list);
 	timevalclear(&sc->lastfail);
 
@@ -1682,6 +1684,7 @@ mps_free(struct mps_softc *sc)
 	mps_unlock(sc);
 	/* Lock must not be held for this */
 	callout_drain(&sc->periodic);
+	callout_drain(&sc->device_check_callout);
 
 	if (((error = mps_detach_log(sc)) != 0) ||
 	    ((error = mps_detach_sas(sc)) != 0))

Modified: head/sys/dev/mps/mps_mapping.c
==============================================================================
--- head/sys/dev/mps/mps_mapping.c	Thu May 25 19:02:54 2017	(r318894)
+++ head/sys/dev/mps/mps_mapping.c	Thu May 25 19:14:44 2017	(r318895)
@@ -60,7 +60,7 @@ __FBSDID("$FreeBSD$");
 #include <dev/mps/mps_mapping.h>
 
 /**
- * _mapping_clear_entry - Clear a particular mapping entry.
+ * _mapping_clear_map_entry - Clear a particular mapping entry.
  * @map_entry: map table entry
  *
  * Returns nothing.
@@ -73,7 +73,6 @@ _mapping_clear_map_entry(struct dev_mapp
 	map_entry->phy_bits = 0;
 	map_entry->dpm_entry_num = MPS_DPM_BAD_IDX;
 	map_entry->dev_handle = 0;
-	map_entry->channel = -1;
 	map_entry->id = -1;
 	map_entry->missing_count = 0;
 	map_entry->init_complete = 0;
@@ -140,12 +139,15 @@ _mapping_commit_enc_entry(struct mps_sof
 	dpm_entry->PhysicalBitsMapping = htole32(et_entry->phy_bits);
 	dpm_entry->Reserved1 = 0;
 
+	mps_dprint(sc, MPS_MAPPING, "%s: Writing DPM entry %d for enclosure.\n",
+	    __func__, et_entry->dpm_entry_num);
 	memcpy(&config_page.Entry, (u8 *)dpm_entry,
 	    sizeof(Mpi2DriverMap0Entry_t));
 	if (mps_config_set_dpm_pg0(sc, &mpi_reply, &config_page,
 	    et_entry->dpm_entry_num)) {
-		printf("%s: write of dpm entry %d for enclosure failed\n",
-		    __func__, et_entry->dpm_entry_num);
+		mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: Write of DPM "
+		    "entry %d for enclosure failed.\n", __func__,
+		    et_entry->dpm_entry_num);
 		dpm_entry->MappingInformation = le16toh(dpm_entry->
 		    MappingInformation);
 		dpm_entry->DeviceIndex = le16toh(dpm_entry->DeviceIndex);
@@ -164,7 +166,7 @@ _mapping_commit_enc_entry(struct mps_sof
 /**
  * _mapping_commit_map_entry - write a particular map table entry in DPM page0.
  * @sc: per adapter object
- * @enc_entry: enclosure table entry
+ * @mt_entry: mapping table entry
  *
  * Returns 0 for success, non-zero for failure.
  */
@@ -180,6 +182,19 @@ _mapping_commit_map_entry(struct mps_sof
 	if (!sc->is_dpm_enable)
 		return 0;
 
+	/*
+	 * It's possible that this Map Entry points to a BAD DPM index. This
+	 * can happen if the Map Entry is a for a missing device and the DPM
+	 * entry that was being used by this device is now being used by some
+	 * new device. So, check for a BAD DPM index and just return if so.
+	 */
+	if (mt_entry->dpm_entry_num == MPS_DPM_BAD_IDX) {
+		mps_dprint(sc, MPS_MAPPING, "%s: DPM entry location for target "
+		    "%d is invalid. DPM will not be written.\n", __func__,
+		    mt_entry->id);
+		return 0;
+	}
+
 	memset(&config_page, 0, sizeof(Mpi2DriverMappingPage0_t));
 	memcpy(&config_page.Header, (u8 *)sc->dpm_pg0,
 	    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
@@ -193,13 +208,16 @@ _mapping_commit_map_entry(struct mps_sof
 	dpm_entry->MappingInformation = htole16(mt_entry->missing_count);
 	dpm_entry->PhysicalBitsMapping = 0;
 	dpm_entry->Reserved1 = 0;
-	dpm_entry->MappingInformation = htole16(dpm_entry->MappingInformation);
 	memcpy(&config_page.Entry, (u8 *)dpm_entry,
 	    sizeof(Mpi2DriverMap0Entry_t));
+
+	mps_dprint(sc, MPS_MAPPING, "%s: Writing DPM entry %d for target %d.\n",
+	    __func__, mt_entry->dpm_entry_num, mt_entry->id);
 	if (mps_config_set_dpm_pg0(sc, &mpi_reply, &config_page,
 	    mt_entry->dpm_entry_num)) {
-		printf("%s: write of dpm entry %d for device failed\n",
-		    __func__, mt_entry->dpm_entry_num);
+		mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: Write of DPM "
+		    "entry %d for target %d failed.\n", __func__,
+		    mt_entry->dpm_entry_num, mt_entry->id);
 		dpm_entry->MappingInformation = le16toh(dpm_entry->
 		    MappingInformation);
 		dpm_entry->DeviceIndex = le16toh(dpm_entry->DeviceIndex);
@@ -307,7 +325,7 @@ _mapping_get_high_missing_et_idx(struct 
 		et_entry = &sc->enclosure_table[enc_idx];
 		if ((et_entry->missing_count > high_missing_count) &&
 		    !et_entry->skip_search) {
-			high_missing_count =  et_entry->missing_count;
+			high_missing_count = et_entry->missing_count;
 			high_idx = enc_idx;
 		}
 	}
@@ -326,7 +344,7 @@ _mapping_get_high_missing_et_idx(struct 
 static u32
 _mapping_get_high_missing_mt_idx(struct mps_softc *sc)
 {
-	u32 map_idx, high_idx = MPS_ENCTABLE_BAD_IDX;
+	u32 map_idx, high_idx = MPS_MAPTABLE_BAD_IDX;
 	u8 high_missing_count = 0;
 	u32 start_idx, end_idx, start_idx_ir, end_idx_ir;
 	struct dev_mapping_table *mt_entry;
@@ -370,7 +388,7 @@ _mapping_get_ir_mt_idx_from_wwid(struct 
 
 	_mapping_get_ir_maprange(sc, &start_idx, &end_idx);
 	mt_entry = &sc->mapping_table[start_idx];
-	for (map_idx  = start_idx; map_idx <= end_idx; map_idx++, mt_entry++)
+	for (map_idx = start_idx; map_idx <= end_idx; map_idx++, mt_entry++)
 		if (mt_entry->physical_id == wwid)
 			return map_idx;
 
@@ -458,20 +476,32 @@ _mapping_get_free_ir_mt_idx(struct mps_s
 	u32 high_idx = MPS_MAPTABLE_BAD_IDX;
 	struct dev_mapping_table *mt_entry;
 
+	/*
+	 * The IN_USE flag should be clear if the entry is available to use.
+	 * This flag is cleared on initialization and and when a volume is
+	 * deleted. All other times this flag should be set. If, for some
+	 * reason, a free entry cannot be found, look for the entry with the
+	 * highest missing count just in case there is one.
+	 */
 	_mapping_get_ir_maprange(sc, &start_idx, &end_idx);
 
 	mt_entry = &sc->mapping_table[start_idx];
-	for (map_idx  = start_idx; map_idx <= end_idx; map_idx++, mt_entry++)
+	for (map_idx = start_idx; map_idx <= end_idx; map_idx++, mt_entry++) {
 		if (!(mt_entry->device_info & MPS_MAP_IN_USE))
 			return map_idx;
 
-	mt_entry = &sc->mapping_table[start_idx];
-	for (map_idx  = start_idx; map_idx <= end_idx; map_idx++, mt_entry++) {
 		if (mt_entry->missing_count > high_missing_count) {
 			high_missing_count = mt_entry->missing_count;
 			high_idx = map_idx;
 		}
 	}
+
+	if (high_idx == MPS_MAPTABLE_BAD_IDX) {
+		mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: Could not find a "
+		    "free entry in the mapping table for a Volume. The mapping "
+		    "table is probably corrupt.\n", __func__);
+	}
+	
 	return high_idx;
 }
 
@@ -494,6 +524,7 @@ _mapping_get_free_mt_idx(struct mps_soft
 	if (sc->ir_firmware && (volume_mapping_flags ==
 	    MPI2_IOCPAGE8_IRFLAGS_HIGH_VOLUME_MAPPING))
 		max_idx -= sc->max_volumes;
+
 	for (map_idx  = start_idx; map_idx < max_idx; map_idx++, mt_entry++)
 		if (!(mt_entry->device_info & (MPS_MAP_IN_USE |
 		    MPS_DEV_RESERVED)))
@@ -542,12 +573,66 @@ static u32
 _mapping_get_free_dpm_idx(struct mps_softc *sc)
 {
 	u16 entry_num;
+	Mpi2DriverMap0Entry_t *dpm_entry;
+	u16 current_entry = MPS_DPM_BAD_IDX, missing_cnt, high_missing_cnt = 0;
+	u64 physical_id;
+	struct dev_mapping_table *mt_entry;
+	u32 map_idx;
 
-	for (entry_num = 0; entry_num < sc->max_dpm_entries; entry_num++) {
-		if (!sc->dpm_entry_used[entry_num])
-			return entry_num;
+ 	for (entry_num = 0; entry_num < sc->max_dpm_entries; entry_num++) {
+		dpm_entry = (Mpi2DriverMap0Entry_t *) ((u8 *)sc->dpm_pg0 +
+		    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
+		dpm_entry += entry_num;
+		missing_cnt = dpm_entry->MappingInformation &
+		    MPI2_DRVMAP0_MAPINFO_MISSING_MASK;
+
+		/*
+		 * If entry is used and not missing, then this entry can't be
+		 * used. Look at next one.
+		 */
+		if (sc->dpm_entry_used[entry_num] && !missing_cnt)
+			continue;
+
+		/*
+		 * If this entry is not used at all, then the missing count
+		 * doesn't matter. Just use this one. Otherwise, keep looking
+		 * and make sure the entry with the highest missing count is
+		 * used.
+		 */
+		if (!sc->dpm_entry_used[entry_num]) {
+			current_entry = entry_num;
+			break;
+		}
+		if ((current_entry == MPS_DPM_BAD_IDX) ||
+		    (missing_cnt > high_missing_cnt)) {
+			current_entry = entry_num;
+			high_missing_cnt = missing_cnt;
+		}
+ 	}
+
+	/*
+	 * If an entry has been found to use and it's already marked as used
+	 * it means that some device was already using this entry but it's
+	 * missing, and that means that the connection between the missing
+	 * device's DPM entry and the mapping table needs to be cleared. To do
+	 * this, use the Physical ID of the old device still in the DPM entry
+	 * to find its mapping table entry, then mark its DPM entry as BAD.
+	 */
+	if ((current_entry != MPS_DPM_BAD_IDX) &&
+	    sc->dpm_entry_used[current_entry]) {
+		dpm_entry = (Mpi2DriverMap0Entry_t *) ((u8 *)sc->dpm_pg0 +
+		    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
+		dpm_entry += current_entry;
+		physical_id = dpm_entry->PhysicalIdentifier.High;
+		physical_id = (physical_id << 32) |
+		    dpm_entry->PhysicalIdentifier.Low;
+		map_idx = _mapping_get_mt_idx_from_id(sc, physical_id);
+		if (map_idx != MPS_MAPTABLE_BAD_IDX) {
+			mt_entry = &sc->mapping_table[map_idx];
+			mt_entry->dpm_entry_num = MPS_DPM_BAD_IDX;
+		}
 	}
-	return MPS_DPM_BAD_IDX;
+	return current_entry;
 }
 
 /**
@@ -566,40 +651,57 @@ _mapping_update_ir_missing_cnt(struct mp
     Mpi2EventIrConfigElement_t *element, u64 wwid)
 {
 	struct dev_mapping_table *mt_entry;
-	u8 missing_cnt, reason = element->ReasonCode;
+	u8 missing_cnt, reason = element->ReasonCode, update_dpm = 1;
 	u16 dpm_idx;
 	Mpi2DriverMap0Entry_t *dpm_entry;
 
-	if (!sc->is_dpm_enable)
-		return;
+	/*
+	 * Depending on the reason code, update the missing count. Always set
+	 * the init_complete flag when here, so just do it first. That flag is
+	 * used for volumes to make sure that the DPM entry has been updated.
+	 * When a volume is deleted, clear the map entry's IN_USE flag so that
+	 * the entry can be used again if another volume is created. Also clear
+	 * its dev_handle entry so that other functions can't find this volume
+	 * by the handle, since it's not defined any longer.
+	 */
 	mt_entry = &sc->mapping_table[map_idx];
-	if (reason == MPI2_EVENT_IR_CHANGE_RC_ADDED) {
+	mt_entry->init_complete = 1;
+	if ((reason == MPI2_EVENT_IR_CHANGE_RC_ADDED) ||
+	    (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_CREATED)) {
 		mt_entry->missing_count = 0;
-	} else if (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_CREATED) {
-		mt_entry->missing_count = 0;
-		mt_entry->init_complete = 0;
-	} else if ((reason == MPI2_EVENT_IR_CHANGE_RC_REMOVED) ||
-	    (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_DELETED)) {
-		if (!mt_entry->init_complete) {
-			if (mt_entry->missing_count < MPS_MAX_MISSING_COUNT)
-				mt_entry->missing_count++;
-			else
-				mt_entry->init_complete = 1;
-		}
-		if (!mt_entry->missing_count)
+	} else if (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_DELETED) {
+		if (mt_entry->missing_count < MPS_MAX_MISSING_COUNT)
 			mt_entry->missing_count++;
+
+		mt_entry->device_info &= ~MPS_MAP_IN_USE;
 		mt_entry->dev_handle = 0;
 	}
 
+	/*
+	 * If persistent mapping is enabled, update the DPM with the new missing
+	 * count for the volume. If the DPM index is bad, get a free one. If
+	 * it's bad for a volume that's being deleted do nothing because that
+	 * volume doesn't have a DPM entry. 
+	 */
+	if (!sc->is_dpm_enable)
+		return;
 	dpm_idx = mt_entry->dpm_entry_num;
 	if (dpm_idx == MPS_DPM_BAD_IDX) {
-		if ((reason == MPI2_EVENT_IR_CHANGE_RC_ADDED) ||
-		    (reason == MPI2_EVENT_IR_CHANGE_RC_REMOVED))
-			dpm_idx = _mapping_get_dpm_idx_from_id(sc,
-			    mt_entry->physical_id, 0);
-		else if (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_DELETED)
+		if (reason == MPI2_EVENT_IR_CHANGE_RC_VOLUME_DELETED)
+		{
+			mps_dprint(sc, MPS_MAPPING, "%s: Volume being deleted "
+			    "is not in DPM so DPM missing count will not be "
+			    "updated.\n", __func__);
 			return;
+		}
 	}
+	if (dpm_idx == MPS_DPM_BAD_IDX)
+		dpm_idx = _mapping_get_free_dpm_idx(sc);
+
+	/*
+	 * Got the DPM entry for the volume or found a free DPM entry if this is
+	 * a new volume. Check if the current information is outdated.
+	 */
 	if (dpm_idx != MPS_DPM_BAD_IDX) {
 		dpm_entry = (Mpi2DriverMap0Entry_t *)((u8 *)sc->dpm_pg0 +
 		    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
@@ -607,17 +709,25 @@ _mapping_update_ir_missing_cnt(struct mp
 		missing_cnt = dpm_entry->MappingInformation &
 		    MPI2_DRVMAP0_MAPINFO_MISSING_MASK;
 		if ((mt_entry->physical_id ==
-		    le64toh((u64)dpm_entry->PhysicalIdentifier.High |
-		    dpm_entry->PhysicalIdentifier.Low)) && (missing_cnt ==
-		    mt_entry->missing_count))
-			mt_entry->init_complete = 1;
-	} else {
-		dpm_idx = _mapping_get_free_dpm_idx(sc);
-		mt_entry->init_complete = 0;
+		    le64toh(((u64)dpm_entry->PhysicalIdentifier.High << 32) |
+		    (u64)dpm_entry->PhysicalIdentifier.Low)) && (missing_cnt ==
+		    mt_entry->missing_count)) {
+			mps_dprint(sc, MPS_MAPPING, "%s: DPM entry for volume "
+			   "with target ID %d does not require an update.\n",
+			    __func__, mt_entry->id);
+			update_dpm = 0;
+		}
 	}
 
-	if ((dpm_idx != MPS_DPM_BAD_IDX) && !mt_entry->init_complete) {
-		mt_entry->init_complete = 1;
+	/*
+	 * Update the volume's persistent info if it's new or the ID or missing
+	 * count has changed. If a good DPM index has not been found by now,
+	 * there is no space left in the DPM table.
+	 */
+	if ((dpm_idx != MPS_DPM_BAD_IDX) && update_dpm) {
+		mps_dprint(sc, MPS_MAPPING, "%s: Update DPM entry for volume "
+		    "with target ID %d.\n", __func__, mt_entry->id);
+
 		mt_entry->dpm_entry_num = dpm_idx;
 		dpm_entry = (Mpi2DriverMap0Entry_t *)((u8 *)sc->dpm_pg0 +
 		    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
@@ -633,44 +743,47 @@ _mapping_update_ir_missing_cnt(struct mp
 		sc->dpm_flush_entry[dpm_idx] = 1;
 		sc->dpm_entry_used[dpm_idx] = 1;
 	} else if (dpm_idx == MPS_DPM_BAD_IDX) {
-		printf("%s: no space to add entry in DPM table\n", __func__);
-		mt_entry->init_complete = 1;
+		mps_dprint(sc, MPS_INFO | MPS_MAPPING, "%s: No space to add an "
+		    "entry in the DPM table for volume with target ID %d.\n",
+		    __func__, mt_entry->id);
 	}
 }
 
 /**
- * _mapping_add_to_removal_table - mark an entry for removal
+ * _mapping_add_to_removal_table - add DPM index to the removal table
  * @sc: per adapter object
- * @handle: Handle of enclosures/device/volume
+ * @dpm_idx: Index of DPM entry to remove
  *
- * Adds the handle or DPM entry number in removal table.
+ * Adds a DPM entry number to the removal table.
  *
  * Returns nothing.
  */
 static void
-_mapping_add_to_removal_table(struct mps_softc *sc, u16 handle,
-    u16 dpm_idx)
+_mapping_add_to_removal_table(struct mps_softc *sc, u16 dpm_idx)
 {
 	struct map_removal_table *remove_entry;
 	u32 i;
-	u16 ioc_pg8_flags = le16toh(sc->ioc_pg8.Flags);
 
+	/*
+	 * This is only used to remove entries from the DPM in the controller.
+	 * If DPM is not enabled, just return.
+	 */
+	if (!sc->is_dpm_enable)
+		return;
+
+	/*
+	 * Find the first available removal_table entry and add the new entry
+	 * there.
+	 */
 	remove_entry = sc->removal_table;
 
 	for (i = 0; i < sc->max_devices; i++, remove_entry++) {
-		if (remove_entry->dev_handle || remove_entry->dpm_entry_num !=
-		    MPS_DPM_BAD_IDX)
+		if (remove_entry->dpm_entry_num != MPS_DPM_BAD_IDX)
 			continue;
-		if ((ioc_pg8_flags & MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
-		    MPI2_IOCPAGE8_FLAGS_ENCLOSURE_SLOT_MAPPING) {
-			if (dpm_idx)
-				remove_entry->dpm_entry_num = dpm_idx;
-			if (remove_entry->dpm_entry_num == MPS_DPM_BAD_IDX)
-				remove_entry->dev_handle = handle;
-		} else if ((ioc_pg8_flags &
-		    MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
-		    MPI2_IOCPAGE8_FLAGS_DEVICE_PERSISTENCE_MAPPING)
-			remove_entry->dev_handle = handle;
+ 
+		mps_dprint(sc, MPS_MAPPING, "%s: Adding DPM entry %d to table "
+		    "for removal.\n", __func__, dpm_idx);
+		remove_entry->dpm_entry_num = dpm_idx;
 		break;
 	}
 
@@ -681,8 +794,13 @@ _mapping_add_to_removal_table(struct mps
  * @sc: per adapter object
  * @topo_change: Topology change event entry
  *
- * Search through the topology change list and if any device is found not
- * responding it's associated map table entry and DPM entry is updated
+ * Increment the missing count in the mapping table for a device that is not
+ * responding. If Persitent Mapping is used, increment the DPM entry as well.
+ * Currently, this function only increments the missing count if the device 
+ * goes missing, so after initialization has completed. This means that the
+ * missing count can only go from 0 to 1 here. The missing count is incremented
+ * during initialization as well, so that's where a target's missing count can
+ * go past 1.
  *
  * Returns nothing.
  */
@@ -706,34 +824,45 @@ _mapping_update_missing_count(struct mps
 		    dev_handle);
 		phy_change->is_processed = 1;
 		if (map_idx == MPS_MAPTABLE_BAD_IDX) {
-			printf("%s: device is already removed from mapping "
-			    "table\n", __func__);
+			mps_dprint(sc, MPS_INFO | MPS_MAPPING, "%s: device is "
+			    "already removed from mapping table\n", __func__);
 			continue;
 		}
 		mt_entry = &sc->mapping_table[map_idx];
-		if (!mt_entry->init_complete) {
-			if (mt_entry->missing_count < MPS_MAX_MISSING_COUNT)
-				mt_entry->missing_count++;
-			else
-				mt_entry->init_complete = 1;
-		}
-		if (!mt_entry->missing_count)
+		if (mt_entry->missing_count < MPS_MAX_MISSING_COUNT)
 			mt_entry->missing_count++;
-		_mapping_add_to_removal_table(sc, mt_entry->dev_handle, 0);
-		mt_entry->dev_handle = 0;
 
+		/*
+		 * When using Enc/Slot mapping, when a device is removed, it's
+		 * mapping table information should be cleared. Otherwise, the
+		 * target ID will be incorrect if this same device is re-added
+		 * to a different slot.
+		 */
+		if ((ioc_pg8_flags & MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
+		    MPI2_IOCPAGE8_FLAGS_ENCLOSURE_SLOT_MAPPING) {
+			_mapping_clear_map_entry(mt_entry);
+		}
+
+		/*
+		 * When using device mapping, update the missing count in the
+		 * DPM entry, but only if the missing count has changed.
+		 */
 		if (((ioc_pg8_flags & MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
 		    MPI2_IOCPAGE8_FLAGS_DEVICE_PERSISTENCE_MAPPING) &&
-		    sc->is_dpm_enable && !mt_entry->init_complete &&
+		    sc->is_dpm_enable &&
 		    mt_entry->dpm_entry_num != MPS_DPM_BAD_IDX) {
 			dpm_entry =
 			    (Mpi2DriverMap0Entry_t *) ((u8 *)sc->dpm_pg0 +
 			    sizeof(MPI2_CONFIG_EXTENDED_PAGE_HEADER));
 			dpm_entry += mt_entry->dpm_entry_num;
-			dpm_entry->MappingInformation = mt_entry->missing_count;
-			sc->dpm_flush_entry[mt_entry->dpm_entry_num] = 1;
+			if (dpm_entry->MappingInformation !=
+			    mt_entry->missing_count) {
+				dpm_entry->MappingInformation =
+				    mt_entry->missing_count;
+				sc->dpm_flush_entry[mt_entry->dpm_entry_num] =
+				    1;
+			}
 		}
-		mt_entry->init_complete = 1;
 	}
 }
 
@@ -766,6 +895,10 @@ _mapping_find_enc_map_space(struct mps_s
 	vol_mapping_flags = le16toh(sc->ioc_pg8.IRVolumeMappingFlags) &
 	    MPI2_IOCPAGE8_IRFLAGS_MASK_VOLUME_MAPPING_MODE;
 
+	/*
+	 * The end of the mapping table depends on where volumes are kept, if
+	 * IR is enabled.
+	 */
 	if (!sc->ir_firmware)
 		end_of_table = sc->max_devices;
 	else if (vol_mapping_flags == MPI2_IOCPAGE8_IRFLAGS_LOW_VOLUME_MAPPING)
@@ -773,6 +906,17 @@ _mapping_find_enc_map_space(struct mps_s
 	else
 		end_of_table = sc->max_devices - sc->max_volumes;
 
+	/*
+	 * The skip_count is the number of entries that are reserved at the
+	 * beginning of the mapping table. But, it does not include the number
+	 * of Physical IDs that are reserved for direct attached devices. Look
+	 * through the mapping table after these reserved entries to see if 
+	 * the devices for this enclosure are already mapped. The PHY bit check
+	 * is used to make sure that at least one PHY bit is common between the
+	 * enclosure and the device that is already mapped.
+	 */
+	mps_dprint(sc, MPS_MAPPING, "%s: Looking for space in the mapping "
+	    "table for added enclosure.\n", __func__);
 	for (map_idx = (max_num_phy_ids + skip_count);
 	    map_idx < end_of_table; map_idx++) {
 		mt_entry = &sc->mapping_table[map_idx];
@@ -782,11 +926,21 @@ _mapping_find_enc_map_space(struct mps_s
 			num_found += 1;
 			if (num_found == et_entry->num_slots) {
 				start_idx = (map_idx - num_found) + 1;
+				mps_dprint(sc, MPS_MAPPING, "%s: Found space "
+				    "in the mapping for enclosure at map index "
+				    "%d.\n", __func__, start_idx);
 				return start_idx;
 			}
 		} else
 			num_found = 0;
 	}
+
+	/*
+	 * If the enclosure's devices are not mapped already, look for
+	 * contiguous entries in the mapping table that are not reserved. If
+	 * enough entries are found, return the starting index for that space.
+	 */
+	num_found = 0;
 	for (map_idx = (max_num_phy_ids + skip_count);
 	    map_idx < end_of_table; map_idx++) {
 		mt_entry = &sc->mapping_table[map_idx];
@@ -794,40 +948,91 @@ _mapping_find_enc_map_space(struct mps_s
 			num_found += 1;
 			if (num_found == et_entry->num_slots) {
 				start_idx = (map_idx - num_found) + 1;
+				mps_dprint(sc, MPS_MAPPING, "%s: Found space "
+				    "in the mapping for enclosure at map index "
+				    "%d.\n", __func__, start_idx);
 				return start_idx;
 			}
 		} else
 			num_found = 0;
 	}
 
+	/*
+	 * If here, it means that not enough space in the mapping table was
+	 * found to support this enclosure, so go through the enclosure table to
+	 * see if any enclosure entries have a missing count. If so, get the
+	 * enclosure with the highest missing count and check it to see if there
+	 * is enough space for the new enclosure.
+	 */
 	while (!done_flag) {
 		enc_idx = _mapping_get_high_missing_et_idx(sc);
-		if (enc_idx == MPS_ENCTABLE_BAD_IDX)
+		if (enc_idx == MPS_ENCTABLE_BAD_IDX) {
+			mps_dprint(sc, MPS_MAPPING, "%s: Not enough space was "
+			    "found in the mapping for the added enclosure.\n",
+			    __func__);
 			return MPS_MAPTABLE_BAD_IDX;
+		}
+
+		/*
+		 * Found a missing enclosure. Set the skip_search flag so this
+		 * enclosure is not checked again for a high missing count if
+		 * the loop continues. This way, all missing enclosures can
+		 * have their space added together to find enough space in the
+		 * mapping table for the added enclosure. The space must be
+		 * contiguous.
+		 */
+		mps_dprint(sc, MPS_MAPPING, "%s: Space from a missing "
+		    "enclosure was found.\n", __func__);
 		enc_entry = &sc->enclosure_table[enc_idx];
-		/*VSP FIXME*/
 		enc_entry->skip_search = 1;
+
+		/*
+		 * Unmark all of the missing enclosure's device's reserved
+		 * space. These will be remarked as reserved if this missing
+		 * enclosure's space is not used.
+		 */
+		mps_dprint(sc, MPS_MAPPING, "%s: Clear the reserved flag for "
+		    "all of the map entries for the enclosure.\n", __func__);
 		mt_entry = &sc->mapping_table[enc_entry->start_index];
 		for (map_idx = enc_entry->start_index; map_idx <
 		    (enc_entry->start_index + enc_entry->num_slots); map_idx++,
 		    mt_entry++)
-			mt_entry->device_info  &= ~MPS_DEV_RESERVED;
+			mt_entry->device_info &= ~MPS_DEV_RESERVED;
+
+		/*
+		 * Now that space has been unreserved, check again to see if
+		 * enough space is available for the new enclosure.
+		 */
+		mps_dprint(sc, MPS_MAPPING, "%s: Check if new mapping space is "
+		    "enough for the new enclosure.\n", __func__);
 		found_space = 0;
-		for (map_idx = (max_num_phy_ids +
-		    skip_count); map_idx < end_of_table; map_idx++) {
+		num_found = 0;
+		for (map_idx = (max_num_phy_ids + skip_count);
+		    map_idx < end_of_table; map_idx++) {
 			mt_entry = &sc->mapping_table[map_idx];
 			if (!(mt_entry->device_info & MPS_DEV_RESERVED)) {
 				num_found += 1;
 				if (num_found == et_entry->num_slots) {
 					start_idx = (map_idx - num_found) + 1;
 					found_space = 1;
+					break;
 				}
 			} else
 				num_found = 0;
 		}
-
 		if (!found_space)
 			continue;
+
+		/*
+		 * If enough space was found, all of the missing enclosures that
+		 * will be used for the new enclosure must be added to the
+		 * removal table. Then all mappings for the enclosure's devices
+		 * and for the enclosure itself need to be cleared. There may be
+		 * more than one enclosure to add to the removal table and
+		 * clear.
+		 */
+		mps_dprint(sc, MPS_MAPPING, "%s: Found space in the mapping "
+		    "for enclosure at map index %d.\n", __func__, start_idx);
 		for (map_idx = start_idx; map_idx < (start_idx + num_found);
 		    map_idx++) {
 			enc_entry = sc->enclosure_table;
@@ -838,26 +1043,38 @@ _mapping_find_enc_map_space(struct mps_s
 				    enc_entry->num_slots))
 					continue;
 				if (!enc_entry->removal_flag) {
+					mps_dprint(sc, MPS_MAPPING, "%s: "
+					    "Enclosure %d will be removed from "
+					    "the mapping table.\n", __func__,
+					    enc_idx);
 					enc_entry->removal_flag = 1;
-					_mapping_add_to_removal_table(sc, 0,
+					_mapping_add_to_removal_table(sc,
 					    enc_entry->dpm_entry_num);
 				}
 				mt_entry = &sc->mapping_table[map_idx];
-				if (mt_entry->device_info &
-				    MPS_MAP_IN_USE) {
-					_mapping_add_to_removal_table(sc,
-					    mt_entry->dev_handle, 0);
-					_mapping_clear_map_entry(mt_entry);
-				}
+				_mapping_clear_map_entry(mt_entry);
 				if (map_idx == (enc_entry->start_index +
 				    enc_entry->num_slots - 1))
 					_mapping_clear_enc_entry(et_entry);
 			}
 		}
+
+		/*
+		 * During the search for space for this enclosure, some entries
+		 * in the mapping table may have been unreserved. Go back and
+		 * change all of these to reserved again. Only the enclosures
+		 * with the removal_flag set should be left as unreserved. The
+		 * skip_search flag needs to be cleared as well so that the
+		 * enclosure's space will be looked at the next time space is
+		 * needed.
+		 */ 
 		enc_entry = sc->enclosure_table;
 		for (enc_idx = 0; enc_idx < sc->num_enc_table_entries;
 		    enc_idx++, enc_entry++) {
 			if (!enc_entry->removal_flag) {
+				mps_dprint(sc, MPS_MAPPING, "%s: Reset the "
+				    "reserved flag for all of the map entries "
+				    "for enclosure %d.\n", __func__, enc_idx);
 				mt_entry = &sc->mapping_table[enc_entry->
 				    start_index];
 				for (map_idx = enc_entry->start_index; map_idx <
@@ -905,6 +1122,7 @@ _mapping_get_dev_info(struct mps_softc *
 		if (phy_change->is_processed || !phy_change->dev_handle ||
 		    phy_change->reason != MPI2_EVENT_SAS_TOPO_RC_TARG_ADDED)
 			continue;
+
 		if (mps_config_get_sas_device_pg0(sc, &mpi_reply,
 		    &sas_device_pg0, MPI2_SAS_DEVICE_PGAD_FORM_HANDLE,
 		    phy_change->dev_handle)) {
@@ -918,9 +1136,9 @@ _mapping_get_dev_info(struct mps_softc *
 		 * when the system is shutdown.
 		 */
 		device_info = le32toh(sas_device_pg0.DeviceInfo);
-		sas_address = sas_device_pg0.SASAddress.High;
+		sas_address = le32toh(sas_device_pg0.SASAddress.High);
 		sas_address = (sas_address << 32) |
-		    sas_device_pg0.SASAddress.Low;
+		    le32toh(sas_device_pg0.SASAddress.Low);
 		if ((device_info & MPI2_SAS_DEVICE_INFO_END_DEVICE) &&
 		    (device_info & MPI2_SAS_DEVICE_INFO_SATA_DEVICE)) {
 			rc = mpssas_get_sas_address_for_sata_disk(sc,
@@ -939,18 +1157,29 @@ _mapping_get_dev_info(struct mps_softc *
 
 		phy_change->physical_id = sas_address;
 		phy_change->slot = le16toh(sas_device_pg0.Slot);
-		phy_change->device_info = le32toh(sas_device_pg0.DeviceInfo);
+		phy_change->device_info = device_info;
 
+		/*
+		 * When using Enc/Slot mapping, if this device is an enclosure
+		 * make sure that all of its slots can fit into the mapping
+		 * table.
+		 */
 		if ((ioc_pg8_flags & MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
 		    MPI2_IOCPAGE8_FLAGS_ENCLOSURE_SLOT_MAPPING) {
+			/*
+			 * The enclosure should already be in the enclosure
+			 * table due to the Enclosure Add event. If not, just
+			 * continue, nothing can be done.
+			 */
 			enc_idx = _mapping_get_enc_idx_from_handle(sc,
 			    topo_change->enc_handle);
 			if (enc_idx == MPS_ENCTABLE_BAD_IDX) {
 				phy_change->is_processed = 1;
-				mps_dprint(sc, MPS_MAPPING, "%s: failed to add "
-				    "the device with handle 0x%04x because the "
-				    "enclosure is not in the mapping table\n",
-				    __func__, phy_change->dev_handle);
+				mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: "
+				    "failed to add the device with handle "
+				    "0x%04x because the enclosure is not in "
+				    "the mapping table\n", __func__,
+				    phy_change->dev_handle);
 				continue;
 			}
 			if (!((phy_change->device_info &
@@ -963,8 +1192,20 @@ _mapping_get_dev_info(struct mps_softc *
 				continue;
 			}
 			et_entry = &sc->enclosure_table[enc_idx];
+
+			/*
+			 * If the enclosure already has a start_index, it's been
+			 * mapped, so go to the next Topo change.
+			 */
 			if (et_entry->start_index != MPS_MAPTABLE_BAD_IDX)
 				continue;
+
+			/*
+			 * If the Expander Handle is 0, the devices are direct
+			 * attached. In that case, the start_index must be just 
+			 * after the reserved entries. Otherwise, find space in
+			 * the mapping table for the enclosure's devices.
+			 */ 
 			if (!topo_change->exp_handle) {
 				map_idx	= sc->num_rsvd_entries;
 				et_entry->start_index = map_idx;
@@ -972,8 +1213,26 @@ _mapping_get_dev_info(struct mps_softc *
 				map_idx = _mapping_find_enc_map_space(sc,
 				    et_entry);
 				et_entry->start_index = map_idx;
+
+				/*
+				 * If space cannot be found to hold all of the
+				 * enclosure's devices in the mapping table,
+				 * there's no need to continue checking the
+				 * other devices in this event. Set all of the
+				 * phy_details for this event (if the change is
+				 * for an add) as already processed because none
+				 * of these devices can be added to the mapping
+				 * table.
+				 */
 				if (et_entry->start_index ==
 				    MPS_MAPTABLE_BAD_IDX) {
+					mps_dprint(sc, MPS_ERROR | MPS_MAPPING,
+					    "%s: failed to add the enclosure "
+					    "with ID 0x%016jx because there is "
+					    "no free space available in the "
+					    "mapping table for all of the "
+					    "enclosure's devices.\n", __func__,
+					    (uintmax_t)et_entry->enclosure_id);
 					phy_change->is_processed = 1;
 					for (phy_idx = 0; phy_idx <
 					    topo_change->num_entries;
@@ -989,12 +1248,22 @@ _mapping_get_dev_info(struct mps_softc *
 					break;
 				}
 			}
+
+			/*
+			 * Found space in the mapping table for this enclosure.
+			 * Initialize each mapping table entry for the
+			 * enclosure.
+			 */
+			mps_dprint(sc, MPS_MAPPING, "%s: Initialize %d map "
+			    "entries for the enclosure, starting at map index "
+			    " %d.\n", __func__, et_entry->num_slots, map_idx);
 			mt_entry = &sc->mapping_table[map_idx];
 			for (index = map_idx; index < (et_entry->num_slots
 			    + map_idx); index++, mt_entry++) {
 				mt_entry->device_info = MPS_DEV_RESERVED;
 				mt_entry->physical_id = et_entry->enclosure_id;
 				mt_entry->phy_bits = et_entry->phy_bits;
+				mt_entry->missing_count = 0;
 			}
 		}
 	}
@@ -1014,6 +1283,7 @@ _mapping_set_mid_to_eid(struct mps_softc
 	struct dev_mapping_table *mt_entry;
 	u16 slots = et_entry->num_slots, map_idx;
 	u32 start_idx = et_entry->start_index;
+
 	if (start_idx != MPS_MAPTABLE_BAD_IDX) {
 		mt_entry = &sc->mapping_table[start_idx];
 		for (map_idx = 0; map_idx < slots; map_idx++, mt_entry++)
@@ -1063,6 +1333,13 @@ _mapping_clear_removed_entries(struct mp
 			}
 		}
 	}
+
+	/*
+	 * When using Enc/Slot mapping, if a new enclosure was added and old
+	 * enclosure space was needed, the enclosure table may now have gaps
+	 * that need to be closed. All enclosure mappings need to be contiguous
+	 * so that space can be reused correctly if available.
+	 */
 	if ((ioc_pg8_flags & MPI2_IOCPAGE8_FLAGS_MASK_MAPPING_MODE) ==
 	    MPI2_IOCPAGE8_FLAGS_ENCLOSURE_SLOT_MAPPING) {
 		num_entries = sc->num_enc_table_entries;
@@ -1105,8 +1382,8 @@ _mapping_clear_removed_entries(struct mp
  * @sc: per adapter object
  * @topo_change: Topology change event entry
  *
- * Search through the topology change event list and updates map table,
- * enclosure table and DPM pages for for the newly added devices.
+ * Search through the topology change event list and update map table,
+ * enclosure table and DPM pages for the newly added devices.
  *
  * Returns nothing
  */
@@ -1143,30 +1420,41 @@ _mapping_add_new_device(struct mps_softc
 			    (sc, topo_change->enc_handle);
 			if (enc_idx == MPS_ENCTABLE_BAD_IDX) {
 				phy_change->is_processed = 1;
-				printf("%s: failed to add the device with "
-				    "handle 0x%04x because the enclosure is "
-				    "not in the mapping table\n", __func__,
+				mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: "
+				    "failed to add the device with handle "
+				    "0x%04x because the enclosure is not in "
+				    "the mapping table\n", __func__,
 				    phy_change->dev_handle);
 				continue;
 			}
+
+			/*
+			 * If the enclosure's start_index is BAD here, it means
+			 * that there is no room in the mapping table to cover
+			 * all of the devices that could be in the enclosure.
+			 * There's no reason to process any of the devices for
+			 * this enclosure since they can't be mapped.
+			 */
 			et_entry = &sc->enclosure_table[enc_idx];
 			if (et_entry->start_index == MPS_MAPTABLE_BAD_IDX) {
 				phy_change->is_processed = 1;
-				if (!sc->mt_full_retry) {
-					sc->mt_add_device_failed = 1;
-					continue;
-				}
-				printf("%s: failed to add the device with "
-				    "handle 0x%04x because there is no free "
-				    "space available in the mapping table\n",
+				mps_dprint(sc, MPS_ERROR | MPS_MAPPING, "%s: "
+				    "failed to add the device with handle "
+				    "0x%04x because there is no free space "
+				    "available in the mapping table\n",
 				    __func__, phy_change->dev_handle);
 				continue;
 			}
+
+			/*
+			 * Add this device to the mapping table at the correct
+			 * offset where space was found to map the enclosure.
+			 * Then setup the DPM entry information if being used.
+			 */
 			map_idx = et_entry->start_index + phy_change->slot -
 			    et_entry->start_slot;

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201705251914.v4PJEi3l075183>