15165 SMN accesses are size-sensitive

Reviewed by: Robert Mustacchi <rm@fingolin.org>
Reviewed by: Andy Fiddaman <illumos@fiddaman.net>
Approved by: Richard Lowe <richlowe@richlowe.net>

9 files changed, 235 insertions, 72 deletions

diff --git a/usr/src/cmd/amdzen/usmn.c b/usr/src/cmd/amdzen/usmn.c
index 1397ab389d..ee49fe301d 100644
--- a/usr/src/cmd/amdzen/usmn.c
+++ b/usr/src/cmd/amdzen/usmn.c
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2021 Oxide Computer Company
+ * Copyright 2022 Oxide Computer Company
  */
 
 /*
@@ -51,11 +51,13 @@ usmn_parse_uint32(const char *str, uint32_t *valp)
 }
 
 static boolean_t
-usmn_op(boolean_t do_write, int fd, const char *addr, uint32_t value)
+usmn_op(boolean_t do_write, int fd, const char *addr, uint32_t length,
+    uint32_t value)
 {
 	usmn_reg_t usr;
 
 	usr.usr_data = value;
+	usr.usr_size = length;
 	if (!usmn_parse_uint32(addr, &usr.usr_addr)) {
 		return (B_FALSE);
 	}
@@ -78,12 +80,23 @@ main(int argc, char *argv[])
 	const char *device = NULL;
 	boolean_t do_write = B_FALSE;
 	uint32_t wval = 0;
+	uint32_t length = 4;
 
-	while ((c = getopt(argc, argv, "d:w:")) != -1) {
+	while ((c = getopt(argc, argv, "d:L:w:")) != -1) {
 		switch (c) {
 		case 'd':
 			device = optarg;
 			break;
+		case 'L':
+			if (!usmn_parse_uint32(optarg, &length)) {
+				return (EXIT_FAILURE);
+			}
+			if (length != 1 && length != 2 && length != 4) {
+				warnx("length %u is out of range {1,2,4}",
+				    length);
+				return (EXIT_FAILURE);
+			}
+			break;
 		case 'w':
 			do_write = B_TRUE;
 			if (!usmn_parse_uint32(optarg, &wval)) {
@@ -92,7 +105,7 @@ main(int argc, char *argv[])
 			break;
 		default:
 			(void) fprintf(stderr, "Usage: usmn -d device "
-			    "[-w value] addr [addr]...\n"
+			    "[-L length] [-w value] addr [addr]...\n"
 			    "Note: All addresses are interpreted as hex\n");
 			return (2);
 		}
@@ -119,7 +132,7 @@ main(int argc, char *argv[])
 
 	ret = EXIT_SUCCESS;
 	for (i = 0; i < argc; i++) {
-		if (!usmn_op(do_write, fd, argv[i], wval)) {
+		if (!usmn_op(do_write, fd, argv[i], length, wval)) {
 			ret = EXIT_FAILURE;
 		}
 	}
diff --git a/usr/src/uts/intel/io/amdzen/amdzen.c b/usr/src/uts/intel/io/amdzen/amdzen.c
index 8f430a6f6a..62f45ce057 100644
--- a/usr/src/uts/intel/io/amdzen/amdzen.c
+++ b/usr/src/uts/intel/io/amdzen/amdzen.c
@@ -193,6 +193,18 @@ static const amdzen_child_data_t amdzen_children[] = {
 	{ "zen_umc", AMDZEN_C_ZEN_UMC }
 };
 
+static uint8_t
+amdzen_stub_get8(amdzen_stub_t *stub, off_t reg)
+{
+	return (pci_config_get8(stub->azns_cfgspace, reg));
+}
+
+static uint16_t
+amdzen_stub_get16(amdzen_stub_t *stub, off_t reg)
+{
+	return (pci_config_get16(stub->azns_cfgspace, reg));
+}
+
 static uint32_t
 amdzen_stub_get32(amdzen_stub_t *stub, off_t reg)
 {
@@ -206,6 +218,18 @@ amdzen_stub_get64(amdzen_stub_t *stub, off_t reg)
 }
 
 static void
+amdzen_stub_put8(amdzen_stub_t *stub, off_t reg, uint8_t val)
+{
+	pci_config_put8(stub->azns_cfgspace, reg, val);
+}
+
+static void
+amdzen_stub_put16(amdzen_stub_t *stub, off_t reg, uint16_t val)
+{
+	pci_config_put16(stub->azns_cfgspace, reg, val);
+}
+
+static void
 amdzen_stub_put32(amdzen_stub_t *stub, off_t reg, uint32_t val)
 {
 	pci_config_put32(stub->azns_cfgspace, reg, val);
@@ -281,22 +305,59 @@ amdzen_df_read32_bcast(amdzen_t *azn, amdzen_df_t *df, const df_reg_def_t def)
 	return (amdzen_stub_get32(df->adf_funcs[def.drd_func], def.drd_reg));
 }
 
-
 static uint32_t
-amdzen_smn_read32(amdzen_t *azn, amdzen_df_t *df, const smn_reg_t reg)
+amdzen_smn_read(amdzen_t *azn, amdzen_df_t *df, const smn_reg_t reg)
 {
+	const uint32_t base_addr = SMN_REG_ADDR_BASE(reg);
+	const uint32_t addr_off = SMN_REG_ADDR_OFF(reg);
+
+	VERIFY(SMN_REG_IS_NATURALLY_ALIGNED(reg));
 	VERIFY(MUTEX_HELD(&azn->azn_mutex));
-	amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_ADDR, SMN_REG_ADDR(reg));
-	return (amdzen_stub_get32(df->adf_nb, AMDZEN_NB_SMN_DATA));
+	amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_ADDR, base_addr);
+
+	switch (SMN_REG_SIZE(reg)) {
+	case 1:
+		return ((uint32_t)amdzen_stub_get8(df->adf_nb,
+		    AMDZEN_NB_SMN_DATA + addr_off));
+	case 2:
+		return ((uint32_t)amdzen_stub_get16(df->adf_nb,
+		    AMDZEN_NB_SMN_DATA + addr_off));
+	case 4:
+		return (amdzen_stub_get32(df->adf_nb, AMDZEN_NB_SMN_DATA));
+	default:
+		panic("unreachable invalid SMN register size %u",
+		    SMN_REG_SIZE(reg));
+	}
 }
 
 static void
-amdzen_smn_write32(amdzen_t *azn, amdzen_df_t *df, const smn_reg_t reg,
+amdzen_smn_write(amdzen_t *azn, amdzen_df_t *df, const smn_reg_t reg,
     const uint32_t val)
 {
+	const uint32_t base_addr = SMN_REG_ADDR_BASE(reg);
+	const uint32_t addr_off = SMN_REG_ADDR_OFF(reg);
+
+	VERIFY(SMN_REG_IS_NATURALLY_ALIGNED(reg));
+	VERIFY(SMN_REG_VALUE_FITS(reg, val));
 	VERIFY(MUTEX_HELD(&azn->azn_mutex));
-	amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_ADDR, SMN_REG_ADDR(reg));
-	amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_DATA, val);
+	amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_ADDR, base_addr);
+
+	switch (SMN_REG_SIZE(reg)) {
+	case 1:
+		amdzen_stub_put8(df->adf_nb, AMDZEN_NB_SMN_DATA + addr_off,
+		    (uint8_t)val);
+		break;
+	case 2:
+		amdzen_stub_put16(df->adf_nb, AMDZEN_NB_SMN_DATA + addr_off,
+		    (uint16_t)val);
+		break;
+	case 4:
+		amdzen_stub_put32(df->adf_nb, AMDZEN_NB_SMN_DATA, val);
+		break;
+	default:
+		panic("unreachable invalid SMN register size %u",
+		    SMN_REG_SIZE(reg));
+	}
 }
 
 static amdzen_df_t *
@@ -328,11 +389,16 @@ amdzen_df_find(amdzen_t *azn, uint_t dfno)
  * Client functions that are used by nexus children.
  */
 int
-amdzen_c_smn_read32(uint_t dfno, const smn_reg_t reg, uint32_t *valp)
+amdzen_c_smn_read(uint_t dfno, const smn_reg_t reg, uint32_t *valp)
 {
 	amdzen_df_t *df;
 	amdzen_t *azn = amdzen_data;
 
+	if (!SMN_REG_SIZE_IS_VALID(reg))
+		return (EINVAL);
+	if (!SMN_REG_IS_NATURALLY_ALIGNED(reg))
+		return (EINVAL);
+
 	mutex_enter(&azn->azn_mutex);
 	df = amdzen_df_find(azn, dfno);
 	if (df == NULL) {
@@ -345,17 +411,24 @@ amdzen_c_smn_read32(uint_t dfno, const smn_reg_t reg, uint32_t *valp)
 		return (ENXIO);
 	}
 
-	*valp = amdzen_smn_read32(azn, df, reg);
+	*valp = amdzen_smn_read(azn, df, reg);
 	mutex_exit(&azn->azn_mutex);
 	return (0);
 }
 
 int
-amdzen_c_smn_write32(uint_t dfno, const smn_reg_t reg, const uint32_t val)
+amdzen_c_smn_write(uint_t dfno, const smn_reg_t reg, const uint32_t val)
 {
 	amdzen_df_t *df;
 	amdzen_t *azn = amdzen_data;
 
+	if (!SMN_REG_SIZE_IS_VALID(reg))
+		return (EINVAL);
+	if (!SMN_REG_IS_NATURALLY_ALIGNED(reg))
+		return (EINVAL);
+	if (!SMN_REG_VALUE_FITS(reg, val))
+		return (EOVERFLOW);
+
 	mutex_enter(&azn->azn_mutex);
 	df = amdzen_df_find(azn, dfno);
 	if (df == NULL) {
@@ -368,12 +441,11 @@ amdzen_c_smn_write32(uint_t dfno, const smn_reg_t reg, const uint32_t val)
 		return (ENXIO);
 	}
 
-	amdzen_smn_write32(azn, df, reg, val);
+	amdzen_smn_write(azn, df, reg, val);
 	mutex_exit(&azn->azn_mutex);
 	return (0);
 }
 
-
 uint_t
 amdzen_c_df_count(void)
 {
diff --git a/usr/src/uts/intel/io/amdzen/amdzen_client.h b/usr/src/uts/intel/io/amdzen/amdzen_client.h
index fc82c1039e..d7e2edbb74 100644
--- a/usr/src/uts/intel/io/amdzen/amdzen_client.h
+++ b/usr/src/uts/intel/io/amdzen/amdzen_client.h
@@ -52,8 +52,8 @@ extern int amdzen_c_df_fabric_decomp(df_fabric_decomp_t *);
 /*
  * SMN and DF access routines.
  */
-extern int amdzen_c_smn_read32(uint_t, const smn_reg_t, uint32_t *);
-extern int amdzen_c_smn_write32(uint_t, const smn_reg_t, const uint32_t);
+extern int amdzen_c_smn_read(uint_t, const smn_reg_t, uint32_t *);
+extern int amdzen_c_smn_write(uint_t, const smn_reg_t, const uint32_t);
 extern int amdzen_c_df_read32(uint_t, uint8_t, const df_reg_def_t, uint32_t *);
 extern int amdzen_c_df_read64(uint_t, uint8_t, const df_reg_def_t, uint64_t *);
 
diff --git a/usr/src/uts/intel/io/amdzen/smntemp.c b/usr/src/uts/intel/io/amdzen/smntemp.c
index 94b7aa8b83..43ef57f34e 100644
--- a/usr/src/uts/intel/io/amdzen/smntemp.c
+++ b/usr/src/uts/intel/io/amdzen/smntemp.c
@@ -116,7 +116,7 @@ smntemp_temp_update(smntemp_t *smn, smntemp_temp_t *stt)
 
 	ASSERT(MUTEX_HELD((&stt->stt_mutex)));
 
-	if ((ret = amdzen_c_smn_read32(stt->stt_dfno, SMN_SMU_THERMAL_CURTEMP,
+	if ((ret = amdzen_c_smn_read(stt->stt_dfno, SMN_SMU_THERMAL_CURTEMP,
 	    &reg)) != 0) {
 		return (ret);
 	}
diff --git a/usr/src/uts/intel/io/amdzen/usmn.c b/usr/src/uts/intel/io/amdzen/usmn.c
index 789e15830e..c9e1608b19 100644
--- a/usr/src/uts/intel/io/amdzen/usmn.c
+++ b/usr/src/uts/intel/io/amdzen/usmn.c
@@ -94,6 +94,13 @@ usmn_ioctl(dev_t dev, int cmd, intptr_t arg, int mode, cred_t *credp,
 		return (EFAULT);
 	}
 
+	/*
+	 * We don't need to check size and alignment here; the client access
+	 * routines do so for us and return EINVAL if violated.  The same goes
+	 * for the value to be written in the USMN_WRITE case below.
+	 */
+	const smn_reg_t reg = SMN_MAKE_REG_SIZED(usr.usr_addr, usr.usr_size);
+
 	if (cmd == USMN_READ) {
 		int ret;
 
@@ -101,8 +108,7 @@ usmn_ioctl(dev_t dev, int cmd, intptr_t arg, int mode, cred_t *credp,
 			return (EINVAL);
 		}
 
-		ret = amdzen_c_smn_read32(dfno, SMN_MAKE_REG(usr.usr_addr),
-		    &usr.usr_data);
+		ret = amdzen_c_smn_read(dfno, reg, &usr.usr_data);
 		if (ret != 0) {
 			return (ret);
 		}
@@ -113,8 +119,7 @@ usmn_ioctl(dev_t dev, int cmd, intptr_t arg, int mode, cred_t *credp,
 			return (EINVAL);
 		}
 
-		ret = amdzen_c_smn_write32(dfno, SMN_MAKE_REG(usr.usr_addr),
-		    usr.usr_data);
+		ret = amdzen_c_smn_write(dfno, reg, usr.usr_data);
 		if (ret != 0) {
 			return (ret);
 		}
diff --git a/usr/src/uts/intel/io/amdzen/usmn.h b/usr/src/uts/intel/io/amdzen/usmn.h
index 10f057525d..0ee86ff7d3 100644
--- a/usr/src/uts/intel/io/amdzen/usmn.h
+++ b/usr/src/uts/intel/io/amdzen/usmn.h
@@ -10,7 +10,7 @@
  */
 
 /*
- * Copyright 2020 Oxide Computer Company
+ * Copyright 2022 Oxide Computer Company
  */
 
 #ifndef _USMN_H
@@ -32,6 +32,7 @@ extern "C" {
 typedef struct usmn_reg {
 	uint32_t usr_addr;
 	uint32_t usr_data;
+	uint32_t usr_size;
 } usmn_reg_t;
 
 #ifdef __cplusplus
diff --git a/usr/src/uts/intel/io/amdzen/zen_umc.c b/usr/src/uts/intel/io/amdzen/zen_umc.c
index 947c17b4ff..1a00ec29c3 100644
--- a/usr/src/uts/intel/io/amdzen/zen_umc.c
+++ b/usr/src/uts/intel/io/amdzen/zen_umc.c
@@ -1863,7 +1863,7 @@ zen_umc_fill_dimm_common(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 	} else {
 		reg = UMC_DIMMCFG_DDR5(id, dimmno);
 	}
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read DIMM "
 		    "configuration register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -1939,7 +1939,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 		uint64_t addr;
 		const uint16_t reginst = i + dimmno * 2;
 		reg = UMC_BASE(id, reginst);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read base "
 			    "register %x: %d", SMN_REG_ADDR(reg), ret);
 			return (B_FALSE);
@@ -1950,7 +1950,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 		dimm->ud_cs[i].ucs_base.udb_valid = UMC_BASE_GET_EN(val);
 
 		reg = UMC_BASE_SEC(id, reginst);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read "
 			    "secondary base register %x: %d", SMN_REG_ADDR(reg),
 			    ret);
@@ -1963,7 +1963,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_MASK_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read mask register "
 		    "%x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -1979,7 +1979,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	cs1->ucs_base_mask = cs0->ucs_base_mask;
 
 	reg = UMC_MASK_SEC_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read secondary mask "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -1990,7 +1990,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	cs1->ucs_sec_mask = cs0->ucs_sec_mask;
 
 	reg = UMC_ADDRCFG_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read address config "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2026,7 +2026,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_ADDRSEL_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read bank address "
 		    "select register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2051,7 +2051,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	    sizeof (cs0->ucs_bank_bits));
 
 	reg = UMC_COLSEL_LO_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read column address "
 		    "select low register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2062,7 +2062,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_COLSEL_HI_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read column address "
 		    "select high register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2081,7 +2081,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	 * zero.
 	 */
 	reg = UMC_RMSEL_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read rank address "
 		    "select register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2097,7 +2097,7 @@ zen_umc_fill_chan_dimm_ddr4(zen_umc_t *umc, zen_umc_df_t *df,
 	bcopy(cs0->ucs_rm_bits, cs1->ucs_rm_bits, sizeof (cs0->ucs_rm_bits));
 
 	reg = UMC_RMSEL_SEC_DDR4(id, dimmno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read secondary rank "
 		    "address select register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2137,7 +2137,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	cs = &chan->chan_dimms[dimmno].ud_cs[rankno];
 
 	reg = UMC_BASE(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read base "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2149,7 +2149,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 		uint64_t addr;
 
 		reg = UMC_BASE_EXT_DDR5(id, regno);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) !=
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) !=
 		    0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read "
 			    "extended base register %x: %d", SMN_REG_ADDR(reg),
@@ -2163,7 +2163,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_BASE_SEC(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read secondary base "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2175,7 +2175,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 		uint64_t addr;
 
 		reg = UMC_BASE_EXT_SEC_DDR5(id, regno);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) !=
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) !=
 		    0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read "
 			    "extended secondary base register %x: %d",
@@ -2189,7 +2189,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_MASK_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read mask "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2201,7 +2201,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 		uint64_t addr;
 
 		reg = UMC_MASK_EXT_DDR5(id, regno);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) !=
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) !=
 		    0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read "
 			    "extended mask register %x: %d", SMN_REG_ADDR(reg),
@@ -2216,7 +2216,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 
 
 	reg = UMC_MASK_SEC_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read secondary mask "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2228,7 +2228,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 		uint64_t addr;
 
 		reg = UMC_MASK_EXT_SEC_DDR5(id, regno);
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) !=
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) !=
 		    0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read "
 			    "extended mask register %x: %d", SMN_REG_ADDR(reg),
@@ -2242,7 +2242,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_ADDRCFG_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read address config "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2263,7 +2263,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	cs->ucs_nbank_groups = UMC_ADDRCFG_GET_NBANKGRP_BITS(val);
 
 	reg = UMC_ADDRSEL_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read address select "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2283,7 +2283,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	    UMC_ADDRSEL_BANK_BASE;
 
 	reg = UMC_COLSEL_LO_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read column address "
 		    "select low register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2294,7 +2294,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	}
 
 	reg = UMC_COLSEL_HI_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read column address "
 		    "select high register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2311,7 +2311,7 @@ zen_umc_fill_chan_rank_ddr5(zen_umc_t *umc, zen_umc_df_t *df,
 	 * unless something actually points us there.
 	 */
 	reg = UMC_RMSEL_DDR5(id, regno);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read rank multiply "
 		    "select register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2408,7 +2408,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 				reg = UMC_BANK_HASH_DDR5(id, i);
 			}
 
-			if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg,
+			if ((ret = amdzen_c_smn_read(df->zud_dfno, reg,
 			    &val)) != 0) {
 				dev_err(umc->umc_dip, CE_WARN, "failed to read "
 				    "bank hash register %x: %d",
@@ -2433,7 +2433,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 				reg = UMC_RANK_HASH_DDR5(id, i);
 			}
 
-			if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg,
+			if ((ret = amdzen_c_smn_read(df->zud_dfno, reg,
 			    &val)) != 0) {
 				dev_err(umc->umc_dip, CE_WARN, "failed to read "
 				    "rm hash register %x: %d",
@@ -2451,7 +2451,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 			}
 
 			reg = UMC_RANK_HASH_EXT_DDR5(id, i);
-			if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg,
+			if ((ret = amdzen_c_smn_read(df->zud_dfno, reg,
 			    &val)) != 0) {
 				dev_err(umc->umc_dip, CE_WARN, "failed to read "
 				    "rm hash ext register %x: %d",
@@ -2474,7 +2474,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 			reg = UMC_PC_HASH_DDR5(id);
 		}
 
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read pc hash "
 			    "register %x: %d", SMN_REG_ADDR(reg), ret);
 			return (B_FALSE);
@@ -2490,7 +2490,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 			reg = UMC_PC_HASH2_DDR5(id);
 		}
 
-		if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+		if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 			dev_err(umc->umc_dip, CE_WARN, "failed to read pc hash "
 			    "2 register %x: %d", SMN_REG_ADDR(reg), ret);
 			return (B_FALSE);
@@ -2510,7 +2510,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 				reg = UMC_CS_HASH_DDR5(id, i);
 			}
 
-			if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg,
+			if ((ret = amdzen_c_smn_read(df->zud_dfno, reg,
 			    &val)) != 0) {
 				dev_err(umc->umc_dip, CE_WARN, "failed to read "
 				    "cs hash register %x", SMN_REG_ADDR(reg));
@@ -2527,7 +2527,7 @@ zen_umc_fill_chan_hash(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan,
 			}
 
 			reg = UMC_CS_HASH_EXT_DDR5(id, i);
-			if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg,
+			if ((ret = amdzen_c_smn_read(df->zud_dfno, reg,
 			    &val)) != 0) {
 				dev_err(umc->umc_dip, CE_WARN, "failed to read "
 				    "cs hash ext register %x",
@@ -2573,7 +2573,7 @@ zen_umc_fill_chan(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan)
 	}
 
 	reg = UMC_UMCCFG(id);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read UMC "
 		    "configuration register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2596,7 +2596,7 @@ zen_umc_fill_chan(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan)
 	 * encrypting regions of memory.
 	 */
 	reg = UMC_DATACTL(id);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read data control "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2617,7 +2617,7 @@ zen_umc_fill_chan(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan)
 	 * cache it for future us and observability.
 	 */
 	reg = UMC_ECCCTL(id);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read ECC control "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2629,7 +2629,7 @@ zen_umc_fill_chan(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan)
 	 * future.
 	 */
 	reg = UMC_UMCCAP(id);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read UMC cap"
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
@@ -2637,7 +2637,7 @@ zen_umc_fill_chan(zen_umc_t *umc, zen_umc_df_t *df, zen_umc_chan_t *chan)
 	chan->chan_umccap_raw = val;
 
 	reg = UMC_UMCCAP_HI(id);
-	if ((ret = amdzen_c_smn_read32(df->zud_dfno, reg, &val)) != 0) {
+	if ((ret = amdzen_c_smn_read(df->zud_dfno, reg, &val)) != 0) {
 		dev_err(umc->umc_dip, CE_WARN, "failed to read UMC cap high "
 		    "register %x: %d", SMN_REG_ADDR(reg), ret);
 		return (B_FALSE);
diff --git a/usr/src/uts/intel/sys/amdzen/smn.h b/usr/src/uts/intel/sys/amdzen/smn.h
index 3f8250a158..8187042b2f 100644
--- a/usr/src/uts/intel/sys/amdzen/smn.h
+++ b/usr/src/uts/intel/sys/amdzen/smn.h
@@ -17,6 +17,7 @@
 #define	_SYS_AMDZEN_SMN_H
 
 #include <sys/debug.h>
+#include <sys/sysmacros.h>
 #include <sys/types.h>
 
 /*
@@ -351,11 +352,72 @@ extern "C" {
  */
 typedef struct smn_reg {
 	uint32_t sr_addr;
+	uint8_t sr_size;	/* Not size_t: can't ever be that big. */
 } smn_reg_t;
 
-/*CSTYLED*/
-#define	SMN_MAKE_REG(x)	((const smn_reg_t){ .sr_addr = (x) })
-#define	SMN_REG_ADDR(x)	((x).sr_addr)
+/*
+ * These are intended to be macro-like (and indeed some used to be macros) but
+ * are implemented as inline functions so that we can use compound statements
+ * without extensions and don't have to worry about multiple evaluation.  Hence
+ * their capitalised names.
+ */
+static inline smn_reg_t
+SMN_MAKE_REG_SIZED(const uint32_t addr, const uint8_t size)
+{
+	const uint8_t size_always = (size == 0) ? 4 : size;
+	const smn_reg_t rv = {
+	    .sr_addr = addr,
+	    .sr_size = size_always
+	};
+
+	return (rv);
+}
+
+#define	SMN_MAKE_REG(x)		SMN_MAKE_REG_SIZED(x, 4)
+#define	SMN_REG_ADDR(x)		((x).sr_addr)
+#define	SMN_REG_SIZE(x)		((x).sr_size)
+
+static inline boolean_t
+SMN_REG_SIZE_IS_VALID(const smn_reg_t reg)
+{
+	return (reg.sr_size == 1 || reg.sr_size == 2 || reg.sr_size == 4);
+}
+
+/* Is this register suitably aligned for access of <size> bytes? */
+#define	SMN_REG_IS_ALIGNED(x, size)	IS_P2ALIGNED(SMN_REG_ADDR(x), size)
+
+/* Is this register naturally aligned with respect to its own width? */
+static inline boolean_t
+SMN_REG_IS_NATURALLY_ALIGNED(const smn_reg_t reg)
+{
+	return (SMN_REG_IS_ALIGNED(reg, reg.sr_size));
+}
+
+/* Does <val> fit into SMN register <x>? */
+#define	SMN_REG_VALUE_FITS(x, val)	\
+	(((val) & ~(0xffffffffU >> ((4 - SMN_REG_SIZE(x)) << 3))) == 0)
+
+/*
+ * Retrieve the base address of the register.  This is the address that will
+ * actually be set in the index register when performing a read or write of the
+ * underlying register via SMN.  It must always be 32-bit aligned.
+ */
+static inline uint32_t
+SMN_REG_ADDR_BASE(const smn_reg_t reg)
+{
+	return (reg.sr_addr & ~3);
+}
+
+/*
+ * The offset address is the byte offset into the 32-bit-wide data register that
+ * will be returned by a read or set by a write, if the register is smaller than
+ * 32 bits wide.  For registers that are 32 bits wide, this is always 0.
+ */
+static inline uint32_t
+SMN_REG_ADDR_OFF(const smn_reg_t reg)
+{
+	return (reg.sr_addr & 3);
+}
 
 /*
  * This exists so that address calculation functions can check that the register
@@ -396,10 +458,12 @@ typedef enum smn_unit {
  * aperture described above or a smaller one if a unit has been broken down
  * logically into smaller units).  srd_nents is optional; if not set, all
  * existing consumers assume a value of 0 is equivalent to 1: the register has
- * but a single instance in each unit.  srd_stride is ignored if srd_nents is 0
- * or 1 and optional otherwise; it describes the number of bytes to be added to
- * the previous instance's address to obtain that of the next instance.  If left
- * at 0 it is assumed to be 4 bytes.
+ * but a single instance in each unit.  srd_size is the width of the register in
+ * bytes, which must be 0, 1, 2, or 4.  If 0, the size is assumed to be 4 bytes.
+ * srd_stride is ignored if srd_nents is 0 or 1 and optional otherwise; it
+ * describes the number of bytes to be added to the previous instance's address
+ * to obtain that of the next instance.  If left at 0 it is assumed to be equal
+ * to the width of the register.
  *
  * There are units in which registers have more complicated collections of
  * instances that cannot be represented perfectly by this simple descriptor;
@@ -412,6 +476,7 @@ typedef struct smn_reg_def {
 	uint32_t	srd_reg;
 	uint32_t	srd_stride;
 	uint16_t	srd_nents;
+	uint8_t		srd_size;
 } smn_reg_def_t;
 
 /*
@@ -431,7 +496,12 @@ _fn(const uint8_t unitno, const smn_reg_def_t def, const uint16_t reginst) \
 {									\
 	const uint32_t unit32 = (const uint32_t)unitno;			\
 	const uint32_t reginst32 = (const uint32_t)reginst;		\
-	const uint32_t stride = (def.srd_stride == 0) ? 4 : def.srd_stride; \
+	const uint32_t size32 = (def.srd_size == 0) ? 4 :		\
+	    (const uint32_t)def.srd_size;				\
+	ASSERT(size32 == 1 || size32 == 2 || size32 == 4);		\
+	const uint32_t stride = (def.srd_stride == 0) ? size32 :	\
+	    def.srd_stride;						\
+	ASSERT3U(stride, >=, size32);					\
 	const uint32_t nents = (def.srd_nents == 0) ? 1 :		\
 	    (const uint32_t)def.srd_nents;				\
 									\
@@ -449,9 +519,9 @@ _fn(const uint8_t unitno, const smn_reg_def_t def, const uint16_t reginst) \
 	ASSERT0(aperture & ~(_mask));					\
 									\
 	const uint32_t reg = def.srd_reg + reginst32 * stride;		\
-	ASSERT0(reg & (_mask));				\
+	ASSERT0(reg & (_mask));						\
 									\
-	return (SMN_MAKE_REG(aperture + reg));				\
+	return (SMN_MAKE_REG_SIZED(aperture + reg, size32));		\
 }
 
 #ifdef __cplusplus
diff --git a/usr/src/uts/intel/sys/amdzen/umc.h b/usr/src/uts/intel/sys/amdzen/umc.h
index a06c2021eb..ca018c89af 100644
--- a/usr/src/uts/intel/sys/amdzen/umc.h
+++ b/usr/src/uts/intel/sys/amdzen/umc.h
@@ -76,7 +76,8 @@ extern "C" {
  * UMC Channel registers. These are in SMN Space. DDR4 and DDR5 based UMCs share
  * the same base address, somewhat surprisingly. This constructs the appropriate
  * offset and ensures that a caller doesn't exceed the number of known instances
- * of the register.  See smn.h for additional details on SMN addressing.
+ * of the register.  See smn.h for additional details on SMN addressing.  All
+ * UMC registers are 32 bits wide; we check for violations.
  */
 
 static inline smn_reg_t
@@ -93,6 +94,7 @@ amdzen_umc_smn_reg(const uint8_t umcno, const smn_reg_def_t def,
 	const uint32_t nents = (def.srd_nents == 0) ? 1 :
 	    (const uint32_t)def.srd_nents;
 
+	ASSERT0(def.srd_size);
 	ASSERT3S(def.srd_unit, ==, SMN_UNIT_UMC);
 	ASSERT0(def.srd_reg & APERTURE_MASK);
 	ASSERT3U(umc32, <, 12);