13714 pcieadm pci check misses renamed nodes

13715 pcieadm save-cfgspace doesn't work with b/d/f
13716 pcieadm save-cfgspace -a can't open files
Reviewed by: C Fraire <cfraire@me.com>
Reviewed by: Andy Fiddaman <andy@omnios.org>
Approved by: Dan McDonald <danmcd@joyent.com>

7 files changed, 184 insertions, 16 deletions

diff --git a/usr/src/cmd/pcieadm/pcieadm.c b/usr/src/cmd/pcieadm/pcieadm.c
index edcad6e4d8..f1af0da485 100644
--- a/usr/src/cmd/pcieadm/pcieadm.c
+++ b/usr/src/cmd/pcieadm/pcieadm.c
@@ -129,13 +129,49 @@ pcieadm_ofmt_errx(const char *fmt, ...)
 	verrx(EXIT_FAILURE, fmt, ap);
 }
 
-boolean_t
+/*
+ * We determine if a node is PCI in a two step process. The first is to see if
+ * the node's name starts with pci, and has an additional character that
+ * indicates it's not the synthetic root of the tree. However, the node name
+ * changes for some classes of devices such as GPUs. As such, for those we try
+ * to look at the compatible property and see if we have a pciexclass or
+ * pciclass entry. We look specifically for the class to make sure that we don't
+ * fall for the synthetic nodes that have a compatible property of
+ * 'pciex_root_complex'.
+ *
+ * The compatible property is a single string that is actually a compressed
+ * string. That is, there are multiple strings concatenated together in a single
+ * pointer.
+ */
+static boolean_t
 pcieadm_di_node_is_pci(di_node_t node)
 {
 	const char *name;
+	char *compat;
+	int nents;
 
 	name = di_node_name(node);
-	return (strncmp("pci", name, 3) == 0);
+	if (strncmp("pci", name, 3) == 0) {
+		return (name[3] != '\0');
+	}
+
+	nents = di_prop_lookup_strings(DDI_DEV_T_ANY, node, "compatible",
+	    &compat);
+	if (nents <= 0) {
+		return (B_FALSE);
+	}
+
+	for (int i = 0; i < nents; i++) {
+		if (strncmp("pciclass,", compat, strlen("pciclass,")) == 0 ||
+		    strncmp("pciexclass,", compat, strlen("pciexclass,")) ==
+		    0) {
+			return (B_TRUE);
+		}
+
+		compat += strlen(compat) + 1;
+	}
+
+	return (B_FALSE);
 }
 
 static int
@@ -147,15 +183,6 @@ pcieadm_di_walk_cb(di_node_t node, void *arg)
 		return (DI_WALK_CONTINUE);
 	}
 
-	/*
-	 * We create synthetic nodes for the root of PCIe tree basically
-	 * functions as all the resources available for one or more bridges.
-	 * When we encounter that top-level node skip it.
-	 */
-	if (strcmp("pci", di_node_name(node)) == 0) {
-		return (DI_WALK_CONTINUE);
-	}
-
 	return (walk->pdw_func(node, walk->pdw_arg));
 }
 
@@ -216,7 +243,7 @@ pcieadm_find_dip_cb(di_node_t node, void *arg)
 	}
 	(void) snprintf(bdf, sizeof (bdf), "%x/%x/%x", PCI_REG_BUS_G(regs[0]),
 	    PCI_REG_DEV_G(regs[0]), PCI_REG_FUNC_G(regs[0]));
-	(void) snprintf(bdf, sizeof (bdf), "%02x/%02x/%02x",
+	(void) snprintf(altbdf, sizeof (altbdf), "%02x/%02x/%02x",
 	    PCI_REG_BUS_G(regs[0]), PCI_REG_DEV_G(regs[0]),
 	    PCI_REG_FUNC_G(regs[0]));
 
diff --git a/usr/src/cmd/pcieadm/pcieadm.h b/usr/src/cmd/pcieadm/pcieadm.h
index b5d44ef970..7ebdc148ca 100644
--- a/usr/src/cmd/pcieadm/pcieadm.h
+++ b/usr/src/cmd/pcieadm/pcieadm.h
@@ -71,7 +71,6 @@ extern void pcieadm_init_cfgspace_file(pcieadm_t *, const char *,
 extern void pcieadm_fini_cfgspace_file(void *);
 extern void pcieadm_find_nexus(pcieadm_t *);
 extern void pcieadm_find_dip(pcieadm_t *, const char *);
-extern boolean_t pcieadm_di_node_is_pci(di_node_t);
 
 /*
  * Output related
diff --git a/usr/src/cmd/pcieadm/pcieadm_cfgspace.c b/usr/src/cmd/pcieadm/pcieadm_cfgspace.c
index 50d98c5ec9..32c4be06f4 100644
--- a/usr/src/cmd/pcieadm/pcieadm_cfgspace.c
+++ b/usr/src/cmd/pcieadm/pcieadm_cfgspace.c
@@ -4869,8 +4869,17 @@ pcieadm_save_cfgspace_cb(di_node_t devi, void *arg)
 	    PCI_REG_BUS_G(regs[0]), PCI_REG_DEV_G(regs[0]),
 	    PCI_REG_FUNC_G(regs[0]));
 
-	if ((fd = openat(psc->psc_dirfd, fname, O_WRONLY | O_TRUNC | O_CREAT,
-	    0666)) < 0) {
+	if (setppriv(PRIV_SET, PRIV_EFFECTIVE, psc->psc_pci->pia_priv_eff) !=
+	    0) {
+		err(EXIT_FAILURE, "failed to raise privileges");
+	}
+	fd = openat(psc->psc_dirfd, fname, O_WRONLY | O_TRUNC | O_CREAT, 0666);
+	if (setppriv(PRIV_SET, PRIV_EFFECTIVE, psc->psc_pci->pia_priv_min) !=
+	    0) {
+		err(EXIT_FAILURE, "failed to reduce privileges");
+	}
+
+	if (fd < 0) {
 		warn("failed to create output file %s", fname);
 		psc->psc_ret = EXIT_FAILURE;
 		return (DI_WALK_CONTINUE);
diff --git a/usr/src/pkg/manifests/system-test-utiltest.mf b/usr/src/pkg/manifests/system-test-utiltest.mf
index 44cd0ece10..3bfae2f782 100644
--- a/usr/src/pkg/manifests/system-test-utiltest.mf
+++ b/usr/src/pkg/manifests/system-test-utiltest.mf
@@ -1692,6 +1692,7 @@ file path=opt/util-tests/tests/pci/igb-ltr-p.out mode=0444
 file path=opt/util-tests/tests/pci/igb-ltr.out mode=0444
 file path=opt/util-tests/tests/pci/igb.pci mode=0444
 file path=opt/util-tests/tests/pcidbtest mode=0555
+file path=opt/util-tests/tests/pcieadm-priv mode=0555
 file path=opt/util-tests/tests/pcieadmtest mode=0555
 file path=opt/util-tests/tests/printf_test mode=0555
 file path=opt/util-tests/tests/sed/multi_test mode=0555
diff --git a/usr/src/test/util-tests/runfiles/default.run b/usr/src/test/util-tests/runfiles/default.run
index 248f5b5b82..e65e60b10f 100644
--- a/usr/src/test/util-tests/runfiles/default.run
+++ b/usr/src/test/util-tests/runfiles/default.run
@@ -84,3 +84,5 @@ tests = ['sed_addr', 'multi_test']
 
 [/opt/util-tests/tests/pcidbtest]
 [/opt/util-tests/tests/pcieadmtest]
+[/opt/util-tests/tests/pcieadm-priv]
+user = root
diff --git a/usr/src/test/util-tests/tests/pcieadm/Makefile b/usr/src/test/util-tests/tests/pcieadm/Makefile
index 59a995f0ea..dff5374614 100644
--- a/usr/src/test/util-tests/tests/pcieadm/Makefile
+++ b/usr/src/test/util-tests/tests/pcieadm/Makefile
@@ -18,7 +18,7 @@ include $(SRC)/test/Makefile.com
 
 ROOTOPTPKG = $(ROOT)/opt/util-tests/tests
 ROOTOPTPCI = $(ROOT)/opt/util-tests/tests/pci
-PROG = pcieadmtest
+PROG = pcieadmtest pcieadm-priv
 DATAFILES = bridge.pci igb.pci \
 	header0-basic.out \
 	header0-basic-L.out \
diff --git a/usr/src/test/util-tests/tests/pcieadm/pcieadm-priv.ksh b/usr/src/test/util-tests/tests/pcieadm/pcieadm-priv.ksh
new file mode 100644
index 0000000000..0e9c8bc796
--- /dev/null
+++ b/usr/src/test/util-tests/tests/pcieadm/pcieadm-priv.ksh
@@ -0,0 +1,130 @@
+#!/usr/bin/ksh
+#
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2021 Oxide Computer Company
+#
+
+#
+# Additional testing for pcieadm that requires us to actually have
+# privileges rather than relying on existing pieces.
+#
+
+unalias -a
+set -o pipefail
+
+pcieadm_arg0="$(basename $0)"
+pcieadm_prog="/usr/lib/pci/pcieadm"
+pcieadm_tmp="/tmp/pcieadm-priv.$$"
+pcieadm_bdf=""
+pcieadm_dev=""
+pcieadm_path=""
+
+warn()
+{
+	typeset msg="$*"
+	[[ -z "$msg" ]] && msg="failed"
+	echo "TEST FAILED: $pcieadm_arg0: $msg" >&2
+	pcieadm_exit=1
+}
+
+pcieadm_validate_filter()
+{
+	typeset filter="$1"
+
+	if ! $pcieadm_prog show-devs $filter >/dev/null; then
+		warn "failed to show-devs with filter $filter"
+	else
+		printf "TEST PASSED: show-devs $filter\n"
+	fi
+
+	if ! $pcieadm_prog show-cfgspace -d $filter >/dev/null; then
+		warn "failed to show-cfgspace with filter $filter"
+	else
+		printf "TEST PASSED: show-cfgspace -d $filter\n"
+	fi
+
+	if ! $pcieadm_prog save-cfgspace -d $filter "$pcieadm_tmp/out.bin"; then
+		warn "failed to use save-cfgspace -d $filter"
+	else
+		printf "TEST PASSED: save-cfgspace -d $filter\n"
+	fi
+}
+
+#
+# Before we begin execution, set up the environment such that we have a
+# standard locale and that umem will help us catch mistakes.
+#
+export LC_ALL=C.UTF-8
+export LD_PRELOAD=libumem.so
+export UMEM_DEBUG=default
+
+if [[ -n $PCIEADM ]]; then
+	pcieadm_prog=$PCIEADM
+fi
+
+if ! $pcieadm_prog show-devs >/dev/null; then
+	warn "failed to show devices"
+else
+	printf "successfully listed devices\n"
+fi
+
+if ! mkdir "$pcieadm_tmp"; then
+	warn "failed to create temporary directory"
+	exit $pcieadm_exit
+fi
+
+#
+# Verify that we can grab things based on bdf
+#
+pcieadm_bdf=$($pcieadm_prog show-devs -p -o bdf | \
+    awk '{ print $1; exit 0 }')
+if [[ -z "$pcieadm_bdf" ]]; then
+	warn "failed to obtain bdf based filter"
+else
+	pcieadm_validate_filter "$pcieadm_bdf"
+fi
+
+#
+# Verify based on device.
+#
+pcieadm_path=$($pcieadm_prog show-devs -p -o path | \
+    awk '{ print $1; exit 0 }')
+if [[ -z "$pcieadm_path" ]]; then
+	warn "failed to obtain path based filter"
+else
+	pcieadm_validate_filter "$pcieadm_path"
+fi
+
+#
+# Do the same based on the device name
+#
+pcieadm_dev=$($pcieadm_prog show-devs -p -o driver | \
+    awk '{ if ($1 != "--") { print $1; exit 0 } }')
+if [[ -z "$pcieadm_dev" ]]; then
+	warn "failed to obtain driver based filter"
+else
+	pcieadm_validate_filter "$pcieadm_dev"
+fi
+
+if ! $pcieadm_prog save-cfgspace -a "$pcieadm_tmp" > /dev/null; then
+	warn "failed to save all devices"
+else
+	printf "TEST PASSED: save-cfgspace -a\n"
+fi
+
+if (( pcieadm_exit == 0 )); then
+	printf "All tests passed successfully!\n"
+fi
+rm -rf "$pcieadm_tmp"
+exit $pcieadm_exit