nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm
@ 2022-09-21 21:02 Dave Jiang
  2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dave Jiang @ 2022-09-21 21:02 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: vishal.l.verma

This ndctl series add support to test cxl pmem devices through the nvdimm
interface. A new shell script is added for security test due to the
discovery of cxl_test dimms are different than nfit_test based dimms.

---

Dave Jiang (4):
      ndctl: add cxl bus detection
      ndctl/libndctl: Add bus_prefix for cxl
      ndctl/libndctl: Add retrieving of unique_id for cxl mem dev
      ndctl/test: Add CXL test for security


 ndctl/lib/libndctl.c   |  87 +++++++++++++
 ndctl/lib/libndctl.sym |   1 +
 ndctl/lib/private.h    |   1 +
 ndctl/libndctl.h       |   1 +
 test/common            |   7 +
 test/meson.build       |   7 +
 test/security-cxl.sh   | 282 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 386 insertions(+)
 create mode 100755 test/security-cxl.sh

--


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] ndctl: add cxl bus detection
  2022-09-21 21:02 [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm Dave Jiang
@ 2022-09-21 21:02 ` Dave Jiang
  2022-12-13 23:07   ` Verma, Vishal L
  2022-12-14  2:40   ` Alison Schofield
  2022-09-21 21:02 ` [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl Dave Jiang
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2022-09-21 21:02 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: vishal.l.verma

Add helper function to detect that the bus is cxl based.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 ndctl/lib/libndctl.c   |   53 ++++++++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym |    1 +
 ndctl/lib/private.h    |    1 +
 ndctl/libndctl.h       |    1 +
 4 files changed, 56 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ad54f0626510..10422e24d38b 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -12,6 +12,7 @@
 #include <ctype.h>
 #include <fcntl.h>
 #include <dirent.h>
+#include <libgen.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
@@ -876,6 +877,48 @@ static enum ndctl_fwa_method fwa_method_to_method(const char *fwa_method)
 	return NDCTL_FWA_METHOD_RESET;
 }
 
+static int is_ndbus_cxl(const char *ctl_base)
+{
+	char *path, *ppath, *subsys;
+	char tmp_path[PATH_MAX];
+	int rc;
+
+	/* get the real path of ctl_base */
+	path = realpath(ctl_base, NULL);
+	if (!path)
+		return -errno;
+
+	/* setup to get the nd bridge device backing the ctl */
+	sprintf(tmp_path, "%s/device", path);
+	free(path);
+
+	path = realpath(tmp_path, NULL);
+	if (!path)
+		return -errno;
+
+	/* get the parent dir of the ndbus, which should be the nvdimm-bridge */
+	ppath = dirname(path);
+
+	/* setup to get the subsystem of the nvdimm-bridge */
+	sprintf(tmp_path, "%s/%s", ppath, "subsystem");
+	free(path);
+
+	path = realpath(tmp_path, NULL);
+	if (!path)
+		return -errno;
+
+	subsys = basename(path);
+
+	/* check if subsystem is cxl */
+	if (!strcmp(subsys, "cxl"))
+		rc = 1;
+	else
+		rc = 0;
+
+	free(path);
+	return rc;
+}
+
 static void *add_bus(void *parent, int id, const char *ctl_base)
 {
 	char buf[SYSFS_ATTR_SIZE];
@@ -919,6 +962,11 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 	else
 		bus->has_of_node = 1;
 
+	if (is_ndbus_cxl(ctl_base))
+		bus->has_cxl = 1;
+	else
+		bus->has_cxl = 0;
+
 	sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		bus->nfit_dsm_mask = 0;
@@ -1050,6 +1098,11 @@ NDCTL_EXPORT int ndctl_bus_has_of_node(struct ndctl_bus *bus)
 	return bus->has_of_node;
 }
 
+NDCTL_EXPORT int ndctl_bus_has_cxl(struct ndctl_bus *bus)
+{
+	return bus->has_cxl;
+}
+
 NDCTL_EXPORT int ndctl_bus_is_papr_scm(struct ndctl_bus *bus)
 {
 	char buf[SYSFS_ATTR_SIZE];
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index c933163c0380..3a3e8bbd63ef 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -465,4 +465,5 @@ LIBNDCTL_27 {
 
 LIBNDCTL_28 {
 	ndctl_dimm_disable_master_passphrase;
+	ndctl_bus_has_cxl;
 } LIBNDCTL_27;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index e5c56295556d..46bc8908bd90 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -163,6 +163,7 @@ struct ndctl_bus {
 	int regions_init;
 	int has_nfit;
 	int has_of_node;
+	int has_cxl;
 	char *bus_path;
 	char *bus_buf;
 	size_t buf_len;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index c52e82a6f826..91ef0f42f654 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -133,6 +133,7 @@ struct ndctl_bus *ndctl_bus_get_next(struct ndctl_bus *bus);
 struct ndctl_ctx *ndctl_bus_get_ctx(struct ndctl_bus *bus);
 int ndctl_bus_has_nfit(struct ndctl_bus *bus);
 int ndctl_bus_has_of_node(struct ndctl_bus *bus);
+int ndctl_bus_has_cxl(struct ndctl_bus *bus);
 int ndctl_bus_is_papr_scm(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_major(struct ndctl_bus *bus);
 unsigned int ndctl_bus_get_minor(struct ndctl_bus *bus);



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl
  2022-09-21 21:02 [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm Dave Jiang
  2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
@ 2022-09-21 21:02 ` Dave Jiang
  2022-12-13 23:10   ` Verma, Vishal L
  2022-09-21 21:02 ` [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev Dave Jiang
  2022-09-21 21:02 ` [PATCH 4/4] ndctl/test: Add CXL test for security Dave Jiang
  3 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2022-09-21 21:02 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: vishal.l.verma

With support of being able to detect the cxl bus, setup the bus_prefix
for cxl bus.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 ndctl/lib/libndctl.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 10422e24d38b..d2e800bc840a 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2012,6 +2012,12 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 			goto out;
 		}
 		rc =  add_papr_dimm(dimm, dimm_base);
+	} else if (ndctl_bus_has_cxl(bus)) {
+		dimm->bus_prefix = strdup("cxl");
+		if (!dimm->bus_prefix) {
+			rc = -ENOMEM;
+			goto out;
+		}
 	}
 
 	if (rc == -ENODEV) {



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev
  2022-09-21 21:02 [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm Dave Jiang
  2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
  2022-09-21 21:02 ` [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl Dave Jiang
@ 2022-09-21 21:02 ` Dave Jiang
  2022-12-13 23:14   ` Verma, Vishal L
  2022-09-21 21:02 ` [PATCH 4/4] ndctl/test: Add CXL test for security Dave Jiang
  3 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2022-09-21 21:02 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: vishal.l.verma

With bus_prefix, retrieve the unique_id of cxl mem device. This will
allow selecting a specific cxl mem device for the security test code.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 ndctl/lib/libndctl.c |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index d2e800bc840a..c569178b9a3a 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1749,6 +1749,33 @@ NDCTL_EXPORT void ndctl_dimm_refresh_flags(struct ndctl_dimm *dimm)
 		parse_papr_flags(dimm, buf);
 }
 
+static int populate_cxl_dimm_attributes(struct ndctl_dimm *dimm,
+					const char *dimm_base)
+{
+	int rc = 0;
+	char buf[SYSFS_ATTR_SIZE];
+	struct ndctl_ctx *ctx = dimm->bus->ctx;
+	char *path = calloc(1, strlen(dimm_base) + 100);
+	const char *bus_prefix = dimm->bus_prefix;
+
+	if (!path)
+		return -ENOMEM;
+
+	sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		dimm->unique_id = strdup(buf);
+		if (!dimm->unique_id) {
+			rc = -ENOMEM;
+			goto err_read;
+		}
+	}
+
+ err_read:
+
+	free(path);
+	return rc;
+}
+
 static int populate_dimm_attributes(struct ndctl_dimm *dimm,
 				    const char *dimm_base)
 {
@@ -2018,6 +2045,7 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 			rc = -ENOMEM;
 			goto out;
 		}
+		rc = populate_cxl_dimm_attributes(dimm, dimm_base);
 	}
 
 	if (rc == -ENODEV) {



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] ndctl/test: Add CXL test for security
  2022-09-21 21:02 [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm Dave Jiang
                   ` (2 preceding siblings ...)
  2022-09-21 21:02 ` [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev Dave Jiang
@ 2022-09-21 21:02 ` Dave Jiang
  2022-12-13 23:22   ` Verma, Vishal L
  2022-12-14  1:02   ` Dan Williams
  3 siblings, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2022-09-21 21:02 UTC (permalink / raw)
  To: linux-cxl, nvdimm; +Cc: vishal.l.verma

Create security-cxl.sh based off of security.sh for nfit security testing.
The test will test a cxl_test based security commands enabling through
nvdimm.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 test/common          |    7 +
 test/meson.build     |    7 +
 test/security-cxl.sh |  282 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 296 insertions(+)
 create mode 100755 test/security-cxl.sh

diff --git a/test/common b/test/common
index 65615cc09a3e..e13b79728b0c 100644
--- a/test/common
+++ b/test/common
@@ -47,6 +47,7 @@ fi
 #
 NFIT_TEST_BUS0="nfit_test.0"
 NFIT_TEST_BUS1="nfit_test.1"
+CXL_TEST_BUS="cxl_test"
 ACPI_BUS="ACPI.NFIT"
 E820_BUS="e820"
 
@@ -125,6 +126,12 @@ _cleanup()
 	modprobe -r nfit_test
 }
 
+_cxl_cleanup()
+{
+	$NDCTL disable-region -b $CXL_TEST_BUS all
+	modprobe -r cxl_test
+}
+
 # json2var
 # stdin: json
 #
diff --git a/test/meson.build b/test/meson.build
index 5953c286d13f..485deb89bbe2 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -219,6 +219,13 @@ if get_option('keyutils').enabled()
   ]
 endif
 
+if get_option('keyutils').enabled()
+  security_cxl = find_program('security-cxl.sh')
+  tests += [
+    [ 'security-cxl.sh', security_cxl, 'ndctl' ]
+  ]
+endif
+
 foreach t : tests
   test(t[0], t[1],
     is_parallel : false,
diff --git a/test/security-cxl.sh b/test/security-cxl.sh
new file mode 100755
index 000000000000..0ec9b335bf41
--- /dev/null
+++ b/test/security-cxl.sh
@@ -0,0 +1,282 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 Intel Corporation. All rights reserved.
+
+rc=77
+dev=""
+id=""
+keypath="/etc/ndctl/keys"
+masterkey="nvdimm-master"
+masterpath="$keypath/$masterkey.blob"
+backup_key=0
+backup_handle=0
+
+. $(dirname $0)/common
+
+trap 'err $LINENO' ERR
+
+setup()
+{
+	$NDCTL disable-region -b "$CXL_TEST_BUS" all
+}
+
+detect()
+{
+	dev="$($NDCTL list -b "$CXL_TEST_BUS" -D | jq -r 'sort_by(.id) | .[0].dev')"
+	[ -n "$dev" ] || err "$LINENO"
+	id="$($NDCTL list -b "$CXL_TEST_BUS" -D | jq -r 'sort_by(.id) | .[0].id')"
+	[ -n "$id" ] || err "$LINENO"
+}
+
+setup_keys()
+{
+	if [ ! -d "$keypath" ]; then
+		mkdir -p "$keypath"
+	fi
+
+	if [ -f "$masterpath" ]; then
+		mv "$masterpath" "$masterpath.bak"
+		backup_key=1
+	fi
+	if [ -f "$keypath/tpm.handle" ]; then
+		mv "$keypath/tpm.handle" "$keypath/tpm.handle.bak"
+		backup_handle=1
+	fi
+
+	dd if=/dev/urandom bs=1 count=32 2>/dev/null | keyctl padd user "$masterkey" @u
+	keyctl pipe "$(keyctl search @u user $masterkey)" > "$masterpath"
+}
+
+test_cleanup()
+{
+	if keyctl search @u encrypted nvdimm:"$id"; then
+		keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
+	fi
+
+	if keyctl search @u user "$masterkey"; then
+		keyctl unlink "$(keyctl search @u user "$masterkey")"
+	fi
+
+	if [ -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob ]; then
+		rm -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob
+	fi
+}
+
+post_cleanup()
+{
+	if [ -f $masterpath ]; then
+		rm -f "$masterpath"
+	fi
+	if [ "$backup_key" -eq 1 ]; then
+		mv "$masterpath.bak" "$masterpath"
+	fi
+	if [ "$backup_handle" -eq 1 ]; then
+		mv "$keypath/tpm.handle.bak" "$keypath/tpm.handle"
+	fi
+}
+
+lock_dimm()
+{
+	$NDCTL disable-dimm "$dev"
+	test_dimm_path=""
+
+	nmem_rpath=$(readlink -f "/sys/bus/nd/devices/${dev}")
+	nmem_bus=$(dirname ${nmem_rpath});
+	bus_provider_path="${nmem_bus}/provider"
+	test -e "$bus_provider_path" || err "$LINENO"
+	bus_provider=$(cat ${bus_provider_path})
+
+	[[ "$bus_provider" == "$CXL_TEST_BUS" ]] || err "$LINENO"
+	bus="cxl"
+	nmem_provider_path="/sys/bus/nd/devices/${dev}/${bus}/provider"
+	nmem_provider=$(cat ${nmem_provider_path})
+
+	test_dimm_path=$(readlink -f /sys/bus/$bus/devices/${nmem_provider})
+	test_dimm_path=$(dirname $(dirname ${test_dimm_path}))/security_lock
+
+	test -e "$test_dimm_path"
+
+	# now lock the dimm
+	echo 1 > "${test_dimm_path}"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "locked" ]; then
+		echo "Incorrect security state: $sstate expected: locked"
+		err "$LINENO"
+	fi
+}
+
+get_frozen_state()
+{
+	$NDCTL list -i -b "$CXL_TEST_BUS" -d "$dev" | jq -r .[].dimms[0].security_frozen
+}
+
+get_security_state()
+{
+	$NDCTL list -i -b "$CXL_TEST_BUS" -d "$dev" | jq -r .[].dimms[0].security
+}
+
+setup_passphrase()
+{
+	$NDCTL setup-passphrase "$dev" -k user:"$masterkey"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		err "$LINENO"
+	fi
+}
+
+remove_passphrase()
+{
+	$NDCTL remove-passphrase "$dev"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "disabled" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		err "$LINENO"
+	fi
+}
+
+erase_security()
+{
+	$NDCTL sanitize-dimm -c "$dev"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "disabled" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		err "$LINENO"
+	fi
+}
+
+update_security()
+{
+	$NDCTL update-passphrase "$dev"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		err "$LINENO"
+	fi
+}
+
+freeze_security()
+{
+	$NDCTL freeze-security "$dev"
+}
+
+test_1_security_setup_and_remove()
+{
+	setup_passphrase
+	remove_passphrase
+}
+
+test_2_security_setup_and_update()
+{
+	setup_passphrase
+	update_security
+	remove_passphrase
+}
+
+test_3_security_setup_and_erase()
+{
+	setup_passphrase
+	erase_security
+}
+
+test_4_security_unlock()
+{
+	setup_passphrase
+	lock_dimm
+	$NDCTL enable-dimm "$dev"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		err "$LINENO"
+	fi
+	$NDCTL disable-region -b "$CXL_TEST_BUS" all
+	remove_passphrase
+}
+
+# This should always be the last nvdimm security test.
+# with security frozen, cxl_test must be removed and is no longer usable
+test_5_security_freeze()
+{
+	setup_passphrase
+	freeze_security
+	sstate="$(get_security_state)"
+	fstate="$(get_frozen_state)"
+	if [ "$fstate" != "true" ]; then
+		echo "Incorrect security state: expected: frozen"
+		err "$LINENO"
+	fi
+
+	# need to simulate a soft reboot here to clean up
+	lock_dimm
+	$NDCTL enable-dimm "$dev"
+	sstate="$(get_security_state)"
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		err "$LINENO"
+	fi
+}
+
+test_6_load_keys()
+{
+	if keyctl search @u encrypted nvdimm:"$id"; then
+		keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
+	fi
+
+	if keyctl search @u user "$masterkey"; then
+		keyctl unlink "$(keyctl search @u user "$masterkey")"
+	fi
+
+	$NDCTL load-keys
+
+	if keyctl search @u user "$masterkey"; then
+		echo "master key loaded"
+	else
+		echo "master key failed to loaded"
+		err "$LINENO"
+	fi
+
+	if keyctl search @u encrypted nvdimm:"$id"; then
+		echo "dimm key loaded"
+	else
+		echo "dimm key failed to load"
+		err "$LINENO"
+	fi
+}
+
+check_min_kver "5.0" || do_skip "may lack security handling"
+uid="$(keyctl show | grep -Eo "_uid.[0-9]+" | head -1 | cut -d. -f2-)"
+if [ "$uid" -ne 0 ]; then
+	do_skip "run as root or with a sudo login shell for test to work"
+fi
+
+modprobe cxl_test
+setup
+check_prereq "keyctl"
+rc=1
+detect
+test_cleanup
+setup_keys
+echo "Test 1, security setup and remove"
+test_1_security_setup_and_remove
+echo "Test 2, security setup, update, and remove"
+test_2_security_setup_and_update
+echo "Test 3, security setup and erase"
+test_3_security_setup_and_erase
+echo "Test 4, unlock dimm"
+test_4_security_unlock
+
+# Freeze should always be the last nvdimm security test because it locks
+# security state and require cxl_test module unload. However, this does
+# not impact any key management testing via libkeyctl.
+echo "Test 5, freeze security"
+test_5_security_freeze
+
+# Load-keys is independent of actual nvdimm security and is part of key
+# mangement testing.
+echo "Test 6, test load-keys"
+test_6_load_keys
+
+test_cleanup
+post_cleanup
+_cxl_cleanup
+exit 0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] ndctl: add cxl bus detection
  2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
@ 2022-12-13 23:07   ` Verma, Vishal L
  2022-12-14  2:40   ` Alison Schofield
  1 sibling, 0 replies; 11+ messages in thread
From: Verma, Vishal L @ 2022-12-13 23:07 UTC (permalink / raw)
  To: Jiang, Dave, linux-cxl, nvdimm

On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote:
> Add helper function to detect that the bus is cxl based.

Capitalize CXL in subject and here. This can probably be "Add a CXL bus
type, and detect whether a 'dimm' is backed by the CXL subsystem"

Otherwise looks good.

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/lib/libndctl.c   |   53 ++++++++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym |    1 +
>  ndctl/lib/private.h    |    1 +
>  ndctl/libndctl.h       |    1 +
>  4 files changed, 56 insertions(+)
> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ad54f0626510..10422e24d38b 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -12,6 +12,7 @@
>  #include <ctype.h>
>  #include <fcntl.h>
>  #include <dirent.h>
> +#include <libgen.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/ioctl.h>
> @@ -876,6 +877,48 @@ static enum ndctl_fwa_method fwa_method_to_method(const char *fwa_method)
>         return NDCTL_FWA_METHOD_RESET;
>  }
>  
> +static int is_ndbus_cxl(const char *ctl_base)
> +{
> +       char *path, *ppath, *subsys;
> +       char tmp_path[PATH_MAX];
> +       int rc;
> +
> +       /* get the real path of ctl_base */
> +       path = realpath(ctl_base, NULL);
> +       if (!path)
> +               return -errno;
> +
> +       /* setup to get the nd bridge device backing the ctl */
> +       sprintf(tmp_path, "%s/device", path);
> +       free(path);
> +
> +       path = realpath(tmp_path, NULL);
> +       if (!path)
> +               return -errno;
> +
> +       /* get the parent dir of the ndbus, which should be the nvdimm-bridge */
> +       ppath = dirname(path);
> +
> +       /* setup to get the subsystem of the nvdimm-bridge */
> +       sprintf(tmp_path, "%s/%s", ppath, "subsystem");
> +       free(path);
> +
> +       path = realpath(tmp_path, NULL);
> +       if (!path)
> +               return -errno;
> +
> +       subsys = basename(path);
> +
> +       /* check if subsystem is cxl */
> +       if (!strcmp(subsys, "cxl"))
> +               rc = 1;
> +       else
> +               rc = 0;
> +
> +       free(path);
> +       return rc;
> +}
> +
>  static void *add_bus(void *parent, int id, const char *ctl_base)
>  {
>         char buf[SYSFS_ATTR_SIZE];
> @@ -919,6 +962,11 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
>         else
>                 bus->has_of_node = 1;
>  
> +       if (is_ndbus_cxl(ctl_base))
> +               bus->has_cxl = 1;
> +       else
> +               bus->has_cxl = 0;
> +
>         sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
>         if (sysfs_read_attr(ctx, path, buf) < 0)
>                 bus->nfit_dsm_mask = 0;
> @@ -1050,6 +1098,11 @@ NDCTL_EXPORT int ndctl_bus_has_of_node(struct ndctl_bus *bus)
>         return bus->has_of_node;
>  }
>  
> +NDCTL_EXPORT int ndctl_bus_has_cxl(struct ndctl_bus *bus)
> +{
> +       return bus->has_cxl;
> +}
> +
>  NDCTL_EXPORT int ndctl_bus_is_papr_scm(struct ndctl_bus *bus)
>  {
>         char buf[SYSFS_ATTR_SIZE];
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index c933163c0380..3a3e8bbd63ef 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -465,4 +465,5 @@ LIBNDCTL_27 {
>  
>  LIBNDCTL_28 {
>         ndctl_dimm_disable_master_passphrase;
> +       ndctl_bus_has_cxl;
>  } LIBNDCTL_27;
> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index e5c56295556d..46bc8908bd90 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -163,6 +163,7 @@ struct ndctl_bus {
>         int regions_init;
>         int has_nfit;
>         int has_of_node;
> +       int has_cxl;
>         char *bus_path;
>         char *bus_buf;
>         size_t buf_len;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index c52e82a6f826..91ef0f42f654 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -133,6 +133,7 @@ struct ndctl_bus *ndctl_bus_get_next(struct ndctl_bus *bus);
>  struct ndctl_ctx *ndctl_bus_get_ctx(struct ndctl_bus *bus);
>  int ndctl_bus_has_nfit(struct ndctl_bus *bus);
>  int ndctl_bus_has_of_node(struct ndctl_bus *bus);
> +int ndctl_bus_has_cxl(struct ndctl_bus *bus);
>  int ndctl_bus_is_papr_scm(struct ndctl_bus *bus);
>  unsigned int ndctl_bus_get_major(struct ndctl_bus *bus);
>  unsigned int ndctl_bus_get_minor(struct ndctl_bus *bus);
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl
  2022-09-21 21:02 ` [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl Dave Jiang
@ 2022-12-13 23:10   ` Verma, Vishal L
  0 siblings, 0 replies; 11+ messages in thread
From: Verma, Vishal L @ 2022-12-13 23:10 UTC (permalink / raw)
  To: Jiang, Dave, linux-cxl, nvdimm

On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote:
> With support of being able to detect the cxl bus, setup the bus_prefix
> for cxl bus.

Same comment as patch 1 about 'CXL'. This might also be reworded as:

When the 'ndbus' is backed by CXL, set up the bus_prefix for the dimm
object appropriately.

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/lib/libndctl.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index 10422e24d38b..d2e800bc840a 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -2012,6 +2012,12 @@ static void *add_dimm(void *parent, int id,
> const char *dimm_base)
>                         goto out;
>                 }
>                 rc =  add_papr_dimm(dimm, dimm_base);
> +       } else if (ndctl_bus_has_cxl(bus)) {
> +               dimm->bus_prefix = strdup("cxl");
> +               if (!dimm->bus_prefix) {
> +                       rc = -ENOMEM;
> +                       goto out;
> +               }
>         }
>  
>         if (rc == -ENODEV) {
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev
  2022-09-21 21:02 ` [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev Dave Jiang
@ 2022-12-13 23:14   ` Verma, Vishal L
  0 siblings, 0 replies; 11+ messages in thread
From: Verma, Vishal L @ 2022-12-13 23:14 UTC (permalink / raw)
  To: Jiang, Dave, linux-cxl, nvdimm

Also s/cxl/CXL/On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote:

> Subject: [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev

"Allow retrieving" or just "Retrieve unique_id for.."

> With bus_prefix, retrieve the unique_id of cxl mem device. This will
> allow selecting a specific cxl mem device for the security test code.

Also s/cxl/CXL/

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/lib/libndctl.c |   28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index d2e800bc840a..c569178b9a3a 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -1749,6 +1749,33 @@ NDCTL_EXPORT void ndctl_dimm_refresh_flags(struct ndctl_dimm *dimm)
>                 parse_papr_flags(dimm, buf);
>  }
>  
> +static int populate_cxl_dimm_attributes(struct ndctl_dimm *dimm,
> +                                       const char *dimm_base)
> +{
> +       int rc = 0;
> +       char buf[SYSFS_ATTR_SIZE];
> +       struct ndctl_ctx *ctx = dimm->bus->ctx;
> +       char *path = calloc(1, strlen(dimm_base) + 100);
> +       const char *bus_prefix = dimm->bus_prefix;
> +
> +       if (!path)
> +               return -ENOMEM;
> +
> +       sprintf(path, "%s/%s/id", dimm_base, bus_prefix);
> +       if (sysfs_read_attr(ctx, path, buf) == 0) {
> +               dimm->unique_id = strdup(buf);
> +               if (!dimm->unique_id) {
> +                       rc = -ENOMEM;
> +                       goto err_read;
> +               }
> +       }
> +
> + err_read:
> +
> +       free(path);
> +       return rc;
> +}
> +
>  static int populate_dimm_attributes(struct ndctl_dimm *dimm,
>                                     const char *dimm_base)
>  {
> @@ -2018,6 +2045,7 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
>                         rc = -ENOMEM;
>                         goto out;
>                 }
> +               rc = populate_cxl_dimm_attributes(dimm, dimm_base);
>         }
>  
>         if (rc == -ENODEV) {
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] ndctl/test: Add CXL test for security
  2022-09-21 21:02 ` [PATCH 4/4] ndctl/test: Add CXL test for security Dave Jiang
@ 2022-12-13 23:22   ` Verma, Vishal L
  2022-12-14  1:02   ` Dan Williams
  1 sibling, 0 replies; 11+ messages in thread
From: Verma, Vishal L @ 2022-12-13 23:22 UTC (permalink / raw)
  To: Jiang, Dave, linux-cxl, nvdimm

On Wed, 2022-09-21 at 14:02 -0700, Dave Jiang wrote:
> Create security-cxl.sh based off of security.sh for nfit security testing.
> The test will test a cxl_test based security commands enabling through
> nvdimm.

Since the new test is largely a copy of the NFIT security test, I think
it might be better to split out the common portions into a script that
can be sourced, and then from the main tests, call functions defined by
the sourced script with appropriate arguments to specify the module
(cxl_test) and bus ($CXL_TEST_BUS).

If the actual test sequence of tests 1 through 6 is the same between
NFIT and CXL, those could be wrapped in a run_security_tests wrapper
which can be called as:

  run_security_tests "cxl_test" "$CXL_TEST_BUS"

or

  run_security_tests "nfit_test" "$NFIT_TEST_BUS0"


This would allow Jeff's recent fix to get pulled in for the CXL test
too.

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  test/common          |    7 +
>  test/meson.build     |    7 +
>  test/security-cxl.sh |  282 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 296 insertions(+)
>  create mode 100755 test/security-cxl.sh

I think in line with the other cxl tests, a better name would be
cxl-security.sh

> 
> diff --git a/test/common b/test/common
> index 65615cc09a3e..e13b79728b0c 100644
> --- a/test/common
> +++ b/test/common
> @@ -47,6 +47,7 @@ fi
>  #
>  NFIT_TEST_BUS0="nfit_test.0"
>  NFIT_TEST_BUS1="nfit_test.1"
> +CXL_TEST_BUS="cxl_test"
>  ACPI_BUS="ACPI.NFIT"
>  E820_BUS="e820"
>  
> @@ -125,6 +126,12 @@ _cleanup()
>         modprobe -r nfit_test
>  }
>  
> +_cxl_cleanup()
> +{
> +       $NDCTL disable-region -b $CXL_TEST_BUS all
> +       modprobe -r cxl_test
> +}
> +
>  # json2var
>  # stdin: json
>  #
> diff --git a/test/meson.build b/test/meson.build
> index 5953c286d13f..485deb89bbe2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -219,6 +219,13 @@ if get_option('keyutils').enabled()
>    ]
>  endif
>  
> +if get_option('keyutils').enabled()
> +  security_cxl = find_program('security-cxl.sh')
> +  tests += [
> +    [ 'security-cxl.sh', security_cxl, 'ndctl' ]
> +  ]
> +endif
> +
>  foreach t : tests
>    test(t[0], t[1],
>      is_parallel : false,
> diff --git a/test/security-cxl.sh b/test/security-cxl.sh
> new file mode 100755
> index 000000000000..0ec9b335bf41
> --- /dev/null
> +++ b/test/security-cxl.sh
> @@ -0,0 +1,282 @@
> +#!/bin/bash -Ex
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 Intel Corporation. All rights reserved.
> +
> +rc=77
> +dev=""
> +id=""
> +keypath="/etc/ndctl/keys"
> +masterkey="nvdimm-master"
> +masterpath="$keypath/$masterkey.blob"
> +backup_key=0
> +backup_handle=0
> +
> +. $(dirname $0)/common
> +
> +trap 'err $LINENO' ERR
> +
> +setup()
> +{
> +       $NDCTL disable-region -b "$CXL_TEST_BUS" all
> +}
> +
> +detect()
> +{
> +       dev="$($NDCTL list -b "$CXL_TEST_BUS" -D | jq -r 'sort_by(.id) | .[0].dev')"
> +       [ -n "$dev" ] || err "$LINENO"
> +       id="$($NDCTL list -b "$CXL_TEST_BUS" -D | jq -r 'sort_by(.id) | .[0].id')"
> +       [ -n "$id" ] || err "$LINENO"
> +}
> +
> +setup_keys()
> +{
> +       if [ ! -d "$keypath" ]; then
> +               mkdir -p "$keypath"
> +       fi
> +
> +       if [ -f "$masterpath" ]; then
> +               mv "$masterpath" "$masterpath.bak"
> +               backup_key=1
> +       fi
> +       if [ -f "$keypath/tpm.handle" ]; then
> +               mv "$keypath/tpm.handle" "$keypath/tpm.handle.bak"
> +               backup_handle=1
> +       fi
> +
> +       dd if=/dev/urandom bs=1 count=32 2>/dev/null | keyctl padd user "$masterkey" @u
> +       keyctl pipe "$(keyctl search @u user $masterkey)" > "$masterpath"
> +}
> +
> +test_cleanup()
> +{
> +       if keyctl search @u encrypted nvdimm:"$id"; then
> +               keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
> +       fi
> +
> +       if keyctl search @u user "$masterkey"; then
> +               keyctl unlink "$(keyctl search @u user "$masterkey")"
> +       fi
> +
> +       if [ -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob ]; then
> +               rm -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob
> +       fi
> +}
> +
> +post_cleanup()
> +{
> +       if [ -f $masterpath ]; then
> +               rm -f "$masterpath"
> +       fi
> +       if [ "$backup_key" -eq 1 ]; then
> +               mv "$masterpath.bak" "$masterpath"
> +       fi
> +       if [ "$backup_handle" -eq 1 ]; then
> +               mv "$keypath/tpm.handle.bak" "$keypath/tpm.handle"
> +       fi
> +}
> +
> +lock_dimm()
> +{
> +       $NDCTL disable-dimm "$dev"
> +       test_dimm_path=""
> +
> +       nmem_rpath=$(readlink -f "/sys/bus/nd/devices/${dev}")
> +       nmem_bus=$(dirname ${nmem_rpath});
> +       bus_provider_path="${nmem_bus}/provider"
> +       test -e "$bus_provider_path" || err "$LINENO"
> +       bus_provider=$(cat ${bus_provider_path})
> +
> +       [[ "$bus_provider" == "$CXL_TEST_BUS" ]] || err "$LINENO"
> +       bus="cxl"
> +       nmem_provider_path="/sys/bus/nd/devices/${dev}/${bus}/provider"
> +       nmem_provider=$(cat ${nmem_provider_path})
> +
> +       test_dimm_path=$(readlink -f /sys/bus/$bus/devices/${nmem_provider})
> +       test_dimm_path=$(dirname $(dirname ${test_dimm_path}))/security_lock
> +
> +       test -e "$test_dimm_path"
> +
> +       # now lock the dimm
> +       echo 1 > "${test_dimm_path}"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "locked" ]; then
> +               echo "Incorrect security state: $sstate expected: locked"
> +               err "$LINENO"
> +       fi
> +}
> +
> +get_frozen_state()
> +{
> +       $NDCTL list -i -b "$CXL_TEST_BUS" -d "$dev" | jq -r .[].dimms[0].security_frozen
> +}
> +
> +get_security_state()
> +{
> +       $NDCTL list -i -b "$CXL_TEST_BUS" -d "$dev" | jq -r .[].dimms[0].security
> +}
> +
> +setup_passphrase()
> +{
> +       $NDCTL setup-passphrase "$dev" -k user:"$masterkey"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "unlocked" ]; then
> +               echo "Incorrect security state: $sstate expected: unlocked"
> +               err "$LINENO"
> +       fi
> +}
> +
> +remove_passphrase()
> +{
> +       $NDCTL remove-passphrase "$dev"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "disabled" ]; then
> +               echo "Incorrect security state: $sstate expected: disabled"
> +               err "$LINENO"
> +       fi
> +}
> +
> +erase_security()
> +{
> +       $NDCTL sanitize-dimm -c "$dev"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "disabled" ]; then
> +               echo "Incorrect security state: $sstate expected: disabled"
> +               err "$LINENO"
> +       fi
> +}
> +
> +update_security()
> +{
> +       $NDCTL update-passphrase "$dev"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "unlocked" ]; then
> +               echo "Incorrect security state: $sstate expected: unlocked"
> +               err "$LINENO"
> +       fi
> +}
> +
> +freeze_security()
> +{
> +       $NDCTL freeze-security "$dev"
> +}
> +
> +test_1_security_setup_and_remove()
> +{
> +       setup_passphrase
> +       remove_passphrase
> +}
> +
> +test_2_security_setup_and_update()
> +{
> +       setup_passphrase
> +       update_security
> +       remove_passphrase
> +}
> +
> +test_3_security_setup_and_erase()
> +{
> +       setup_passphrase
> +       erase_security
> +}
> +
> +test_4_security_unlock()
> +{
> +       setup_passphrase
> +       lock_dimm
> +       $NDCTL enable-dimm "$dev"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "unlocked" ]; then
> +               echo "Incorrect security state: $sstate expected: unlocked"
> +               err "$LINENO"
> +       fi
> +       $NDCTL disable-region -b "$CXL_TEST_BUS" all
> +       remove_passphrase
> +}
> +
> +# This should always be the last nvdimm security test.
> +# with security frozen, cxl_test must be removed and is no longer usable
> +test_5_security_freeze()
> +{
> +       setup_passphrase
> +       freeze_security
> +       sstate="$(get_security_state)"
> +       fstate="$(get_frozen_state)"
> +       if [ "$fstate" != "true" ]; then
> +               echo "Incorrect security state: expected: frozen"
> +               err "$LINENO"
> +       fi
> +
> +       # need to simulate a soft reboot here to clean up
> +       lock_dimm
> +       $NDCTL enable-dimm "$dev"
> +       sstate="$(get_security_state)"
> +       if [ "$sstate" != "unlocked" ]; then
> +               echo "Incorrect security state: $sstate expected: unlocked"
> +               err "$LINENO"
> +       fi
> +}
> +
> +test_6_load_keys()
> +{
> +       if keyctl search @u encrypted nvdimm:"$id"; then
> +               keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
> +       fi
> +
> +       if keyctl search @u user "$masterkey"; then
> +               keyctl unlink "$(keyctl search @u user "$masterkey")"
> +       fi
> +
> +       $NDCTL load-keys
> +
> +       if keyctl search @u user "$masterkey"; then
> +               echo "master key loaded"
> +       else
> +               echo "master key failed to loaded"
> +               err "$LINENO"
> +       fi
> +
> +       if keyctl search @u encrypted nvdimm:"$id"; then
> +               echo "dimm key loaded"
> +       else
> +               echo "dimm key failed to load"
> +               err "$LINENO"
> +       fi
> +}
> +
> +check_min_kver "5.0" || do_skip "may lack security handling"
> +uid="$(keyctl show | grep -Eo "_uid.[0-9]+" | head -1 | cut -d. -f2-)"
> +if [ "$uid" -ne 0 ]; then
> +       do_skip "run as root or with a sudo login shell for test to work"
> +fi
> +
> +modprobe cxl_test
> +setup
> +check_prereq "keyctl"
> +rc=1
> +detect
> +test_cleanup
> +setup_keys
> +echo "Test 1, security setup and remove"
> +test_1_security_setup_and_remove
> +echo "Test 2, security setup, update, and remove"
> +test_2_security_setup_and_update
> +echo "Test 3, security setup and erase"
> +test_3_security_setup_and_erase
> +echo "Test 4, unlock dimm"
> +test_4_security_unlock
> +
> +# Freeze should always be the last nvdimm security test because it locks
> +# security state and require cxl_test module unload. However, this does
> +# not impact any key management testing via libkeyctl.
> +echo "Test 5, freeze security"
> +test_5_security_freeze
> +
> +# Load-keys is independent of actual nvdimm security and is part of key
> +# mangement testing.
> +echo "Test 6, test load-keys"
> +test_6_load_keys
> +
> +test_cleanup
> +post_cleanup
> +_cxl_cleanup
> +exit 0
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 4/4] ndctl/test: Add CXL test for security
  2022-09-21 21:02 ` [PATCH 4/4] ndctl/test: Add CXL test for security Dave Jiang
  2022-12-13 23:22   ` Verma, Vishal L
@ 2022-12-14  1:02   ` Dan Williams
  1 sibling, 0 replies; 11+ messages in thread
From: Dan Williams @ 2022-12-14  1:02 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl, nvdimm; +Cc: vishal.l.verma

Dave Jiang wrote:
> Create security-cxl.sh based off of security.sh for nfit security testing.
> The test will test a cxl_test based security commands enabling through
> nvdimm.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  test/common          |    7 +
>  test/meson.build     |    7 +
>  test/security-cxl.sh |  282 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 296 insertions(+)
>  create mode 100755 test/security-cxl.sh
> 
> diff --git a/test/common b/test/common
> index 65615cc09a3e..e13b79728b0c 100644
> --- a/test/common
> +++ b/test/common
> @@ -47,6 +47,7 @@ fi
>  #
>  NFIT_TEST_BUS0="nfit_test.0"
>  NFIT_TEST_BUS1="nfit_test.1"
> +CXL_TEST_BUS="cxl_test"
>  ACPI_BUS="ACPI.NFIT"
>  E820_BUS="e820"
>  
> @@ -125,6 +126,12 @@ _cleanup()
>  	modprobe -r nfit_test
>  }
>  
> +_cxl_cleanup()
> +{
> +	$NDCTL disable-region -b $CXL_TEST_BUS all
> +	modprobe -r cxl_test
> +}
> +
>  # json2var
>  # stdin: json
>  #
> diff --git a/test/meson.build b/test/meson.build
> index 5953c286d13f..485deb89bbe2 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -219,6 +219,13 @@ if get_option('keyutils').enabled()
>    ]
>  endif
>  
> +if get_option('keyutils').enabled()
> +  security_cxl = find_program('security-cxl.sh')
> +  tests += [
> +    [ 'security-cxl.sh', security_cxl, 'ndctl' ]
> +  ]
> +endif
> +

I had this folded on top for my local testing:

diff --git a/test/meson.build b/test/meson.build
index c9853421ce69..1df115f82fef 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -216,15 +216,10 @@ endif
 
 if get_option('keyutils').enabled()
   security = find_program('security.sh')
-  tests += [
-    [ 'security.sh', security, 'ndctl' ]
-  ]
-endif
-
-if get_option('keyutils').enabled()
   security_cxl = find_program('security-cxl.sh')
   tests += [
-    [ 'security-cxl.sh', security_cxl, 'ndctl' ]
+    [ 'security.sh', security, 'ndctl' ],
+    [ 'security-cxl.sh', security_cxl, 'cxl' ],
   ]
 endif

...although I like Vishal's suggestion to name this cxl-security.sh to
match the other cxl test in the suite.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] ndctl: add cxl bus detection
  2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
  2022-12-13 23:07   ` Verma, Vishal L
@ 2022-12-14  2:40   ` Alison Schofield
  1 sibling, 0 replies; 11+ messages in thread
From: Alison Schofield @ 2022-12-14  2:40 UTC (permalink / raw)
  To: Dave Jiang; +Cc: linux-cxl, nvdimm, vishal.l.verma

On Wed, Sep 21, 2022 at 02:02:42PM -0700, Dave Jiang wrote:
> Add helper function to detect that the bus is cxl based.
> 

A couple of wonderings below about int vs bool usage.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>


> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/lib/libndctl.c   |   53 ++++++++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym |    1 +
>  ndctl/lib/private.h    |    1 +
>  ndctl/libndctl.h       |    1 +
>  4 files changed, 56 insertions(+)
> 
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ad54f0626510..10422e24d38b 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -12,6 +12,7 @@
>  #include <ctype.h>
>  #include <fcntl.h>
>  #include <dirent.h>
> +#include <libgen.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/ioctl.h>
> @@ -876,6 +877,48 @@ static enum ndctl_fwa_method fwa_method_to_method(const char *fwa_method)
>  	return NDCTL_FWA_METHOD_RESET;
>  }
>  
> +static int is_ndbus_cxl(const char *ctl_base)

is_ndbus_cxl()  sounds like a boolean function.

> +{
> +	char *path, *ppath, *subsys;
> +	char tmp_path[PATH_MAX];
> +	int rc;
> +
> +	/* get the real path of ctl_base */
> +	path = realpath(ctl_base, NULL);
> +	if (!path)
> +		return -errno;
> +
> +	/* setup to get the nd bridge device backing the ctl */
> +	sprintf(tmp_path, "%s/device", path);
> +	free(path);
> +
> +	path = realpath(tmp_path, NULL);
> +	if (!path)
> +		return -errno;
> +
> +	/* get the parent dir of the ndbus, which should be the nvdimm-bridge */
> +	ppath = dirname(path);
> +
> +	/* setup to get the subsystem of the nvdimm-bridge */
> +	sprintf(tmp_path, "%s/%s", ppath, "subsystem");
> +	free(path);
> +
> +	path = realpath(tmp_path, NULL);
> +	if (!path)
> +		return -errno;
> +
> +	subsys = basename(path);
> +
> +	/* check if subsystem is cxl */
> +	if (!strcmp(subsys, "cxl"))
> +		rc = 1;
> +	else
> +		rc = 0;
> +
> +	free(path);
> +	return rc;
> +}
> +
>  static void *add_bus(void *parent, int id, const char *ctl_base)
>  {
>  	char buf[SYSFS_ATTR_SIZE];
> @@ -919,6 +962,11 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
>  	else
>  		bus->has_of_node = 1;
>  
> +	if (is_ndbus_cxl(ctl_base))
> +		bus->has_cxl = 1;
> +	else
> +		bus->has_cxl = 0;
> +
>  	sprintf(path, "%s/device/nfit/dsm_mask", ctl_base);
>  	if (sysfs_read_attr(ctx, path, buf) < 0)
>  		bus->nfit_dsm_mask = 0;
> @@ -1050,6 +1098,11 @@ NDCTL_EXPORT int ndctl_bus_has_of_node(struct ndctl_bus *bus)
>  	return bus->has_of_node;
>  }
>  
> +NDCTL_EXPORT int ndctl_bus_has_cxl(struct ndctl_bus *bus)
> +{
> +	return bus->has_cxl;
> +}
> +
>  NDCTL_EXPORT int ndctl_bus_is_papr_scm(struct ndctl_bus *bus)
>  {
>  	char buf[SYSFS_ATTR_SIZE];
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index c933163c0380..3a3e8bbd63ef 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -465,4 +465,5 @@ LIBNDCTL_27 {
>  
>  LIBNDCTL_28 {
>  	ndctl_dimm_disable_master_passphrase;
> +	ndctl_bus_has_cxl;
>  } LIBNDCTL_27;
> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index e5c56295556d..46bc8908bd90 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -163,6 +163,7 @@ struct ndctl_bus {
>  	int regions_init;
>  	int has_nfit;
>  	int has_of_node;
> +	int has_cxl;

I was going to say boolean again, but I see the pattern above.
I guess, When in Rome...

>  	char *bus_path;
>  	char *bus_buf;
>  	size_t buf_len;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index c52e82a6f826..91ef0f42f654 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -133,6 +133,7 @@ struct ndctl_bus *ndctl_bus_get_next(struct ndctl_bus *bus);
>  struct ndctl_ctx *ndctl_bus_get_ctx(struct ndctl_bus *bus);
>  int ndctl_bus_has_nfit(struct ndctl_bus *bus);
>  int ndctl_bus_has_of_node(struct ndctl_bus *bus);
> +int ndctl_bus_has_cxl(struct ndctl_bus *bus);
>  int ndctl_bus_is_papr_scm(struct ndctl_bus *bus);
>  unsigned int ndctl_bus_get_major(struct ndctl_bus *bus);
>  unsigned int ndctl_bus_get_minor(struct ndctl_bus *bus);
> 
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-12-14  2:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 21:02 [PATCH 0/4] ndctl: Add security test for cxl devices through nvdimm Dave Jiang
2022-09-21 21:02 ` [PATCH 1/4] ndctl: add cxl bus detection Dave Jiang
2022-12-13 23:07   ` Verma, Vishal L
2022-12-14  2:40   ` Alison Schofield
2022-09-21 21:02 ` [PATCH 2/4] ndctl/libndctl: Add bus_prefix for cxl Dave Jiang
2022-12-13 23:10   ` Verma, Vishal L
2022-09-21 21:02 ` [PATCH 3/4] ndctl/libndctl: Add retrieving of unique_id for cxl mem dev Dave Jiang
2022-12-13 23:14   ` Verma, Vishal L
2022-09-21 21:02 ` [PATCH 4/4] ndctl/test: Add CXL test for security Dave Jiang
2022-12-13 23:22   ` Verma, Vishal L
2022-12-14  1:02   ` Dan Williams

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).