* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.