From: Dan Williams <dan.j.williams@intel.com>
To: linux-cxl@vger.kernel.org
Cc: Jonathan.Cameron@huawei.com, dave.jiang@intel.com,
nvdimm@lists.linux.dev, dave@stgolabs.net
Subject: [PATCH 4/5] nvdimm/region: Move cache management to the region driver
Date: Thu, 01 Dec 2022 14:03:35 -0800 [thread overview]
Message-ID: <166993221550.1995348.16843505129579060258.stgit@dwillia2-xfh.jf.intel.com> (raw)
In-Reply-To: <166993219354.1995348.12912519920112533797.stgit@dwillia2-xfh.jf.intel.com>
Now that cpu_cache_invalidate_memregion() is generically available, use
it to centralize CPU cache management in the nvdimm region driver.
This trades off removing redundant per-dimm CPU cache flushing with an
opportunistic flush on every region disable event to cover the case of
sensitive dirty data in the cache being written back to media after a
secure erase / overwrite event.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/acpi/nfit/intel.c | 25 ---------------------
drivers/nvdimm/region.c | 11 +++++++++
drivers/nvdimm/region_devs.c | 49 +++++++++++++++++++++++++++++++++++++++++-
drivers/nvdimm/security.c | 6 +++++
include/linux/libnvdimm.h | 5 ++++
5 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index fa0e57e35162..3902759abcba 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -212,9 +212,6 @@ static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm,
if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, &nfit_mem->dsm_mask))
return -ENOTTY;
- if (!cpu_cache_has_invalidate_memregion())
- return -EINVAL;
-
memcpy(nd_cmd.cmd.passphrase, key_data->data,
sizeof(nd_cmd.cmd.passphrase));
rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
@@ -229,9 +226,6 @@ static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm,
return -EIO;
}
- /* DIMM unlocked, invalidate all CPU caches before we read it */
- cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
-
return 0;
}
@@ -299,11 +293,6 @@ static int __maybe_unused intel_security_erase(struct nvdimm *nvdimm,
if (!test_bit(cmd, &nfit_mem->dsm_mask))
return -ENOTTY;
- if (!cpu_cache_has_invalidate_memregion())
- return -EINVAL;
-
- /* flush all cache before we erase DIMM */
- cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
memcpy(nd_cmd.cmd.passphrase, key->data,
sizeof(nd_cmd.cmd.passphrase));
rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
@@ -322,8 +311,6 @@ static int __maybe_unused intel_security_erase(struct nvdimm *nvdimm,
return -ENXIO;
}
- /* DIMM erased, invalidate all CPU caches before we read it */
- cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
return 0;
}
@@ -346,9 +333,6 @@ static int __maybe_unused intel_security_query_overwrite(struct nvdimm *nvdimm)
if (!test_bit(NVDIMM_INTEL_QUERY_OVERWRITE, &nfit_mem->dsm_mask))
return -ENOTTY;
- if (!cpu_cache_has_invalidate_memregion())
- return -EINVAL;
-
rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
if (rc < 0)
return rc;
@@ -362,8 +346,6 @@ static int __maybe_unused intel_security_query_overwrite(struct nvdimm *nvdimm)
return -ENXIO;
}
- /* flush all cache before we make the nvdimms available */
- cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
return 0;
}
@@ -388,11 +370,6 @@ static int __maybe_unused intel_security_overwrite(struct nvdimm *nvdimm,
if (!test_bit(NVDIMM_INTEL_OVERWRITE, &nfit_mem->dsm_mask))
return -ENOTTY;
- if (!cpu_cache_has_invalidate_memregion())
- return -EINVAL;
-
- /* flush all cache before we erase DIMM */
- cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
memcpy(nd_cmd.cmd.passphrase, nkey->data,
sizeof(nd_cmd.cmd.passphrase));
rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
@@ -770,5 +747,3 @@ static const struct nvdimm_fw_ops __intel_fw_ops = {
};
const struct nvdimm_fw_ops *intel_fw_ops = &__intel_fw_ops;
-
-MODULE_IMPORT_NS(DEVMEM);
diff --git a/drivers/nvdimm/region.c b/drivers/nvdimm/region.c
index 390123d293ea..88dc062af5f8 100644
--- a/drivers/nvdimm/region.c
+++ b/drivers/nvdimm/region.c
@@ -2,6 +2,7 @@
/*
* Copyright(c) 2013-2015 Intel Corporation. All rights reserved.
*/
+#include <linux/memregion.h>
#include <linux/cpumask.h>
#include <linux/module.h>
#include <linux/device.h>
@@ -100,6 +101,16 @@ static void nd_region_remove(struct device *dev)
*/
sysfs_put(nd_region->bb_state);
nd_region->bb_state = NULL;
+
+ /*
+ * Try to flush caches here since a disabled region may be subject to
+ * secure erase while disabled, and previous dirty data should not be
+ * written back to a new instance of the region. This only matters on
+ * bare metal where security commands are available, so silent failure
+ * here is ok.
+ */
+ if (cpu_cache_has_invalidate_memregion())
+ cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
}
static int child_notify(struct device *dev, void *data)
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e0875d369762..c73e3b1fd0a6 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -59,13 +59,57 @@ static int nvdimm_map_flush(struct device *dev, struct nvdimm *nvdimm, int dimm,
return 0;
}
+static int nd_region_invalidate_memregion(struct nd_region *nd_region)
+{
+ int i, incoherent = 0;
+
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ if (test_bit(NDD_INCOHERENT, &nvdimm->flags))
+ incoherent++;
+ }
+
+ if (!incoherent)
+ return 0;
+
+ if (!cpu_cache_has_invalidate_memregion()) {
+ if (IS_ENABLED(CONFIG_NVDIMM_SECURITY_TEST)) {
+ dev_warn(
+ &nd_region->dev,
+ "Bypassing cpu_cache_invalidate_memergion() for testing!\n");
+ goto out;
+ } else {
+ dev_err(&nd_region->dev,
+ "Failed to synchronize CPU cache state\n");
+ return -ENXIO;
+ }
+ }
+
+ cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
+out:
+ for (i = 0; i < nd_region->ndr_mappings; i++) {
+ struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+ struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+ clear_bit(NDD_INCOHERENT, &nvdimm->flags);
+ }
+
+ return 0;
+}
+
int nd_region_activate(struct nd_region *nd_region)
{
- int i, j, num_flush = 0;
+ int i, j, rc, num_flush = 0;
struct nd_region_data *ndrd;
struct device *dev = &nd_region->dev;
size_t flush_data_size = sizeof(void *);
+ rc = nd_region_invalidate_memregion(nd_region);
+ if (rc)
+ return rc;
+
nvdimm_bus_lock(&nd_region->dev);
for (i = 0; i < nd_region->ndr_mappings; i++) {
struct nd_mapping *nd_mapping = &nd_region->mapping[i];
@@ -85,6 +129,7 @@ int nd_region_activate(struct nd_region *nd_region)
}
nvdimm_bus_unlock(&nd_region->dev);
+
ndrd = devm_kzalloc(dev, sizeof(*ndrd) + flush_data_size, GFP_KERNEL);
if (!ndrd)
return -ENOMEM;
@@ -1222,3 +1267,5 @@ int nd_region_conflict(struct nd_region *nd_region, resource_size_t start,
return device_for_each_child(&nvdimm_bus->dev, &ctx, region_conflict);
}
+
+MODULE_IMPORT_NS(DEVMEM);
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index 6814339b3dab..a03e3c45f297 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -208,6 +208,8 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
rc = nvdimm->sec.ops->unlock(nvdimm, data);
dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
rc == 0 ? "success" : "fail");
+ if (rc == 0)
+ set_bit(NDD_INCOHERENT, &nvdimm->flags);
nvdimm_put_key(key);
nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
@@ -374,6 +376,8 @@ static int security_erase(struct nvdimm *nvdimm, unsigned int keyid,
return -ENOKEY;
rc = nvdimm->sec.ops->erase(nvdimm, data, pass_type);
+ if (rc == 0)
+ set_bit(NDD_INCOHERENT, &nvdimm->flags);
dev_dbg(dev, "key: %d erase%s: %s\n", key_serial(key),
pass_type == NVDIMM_MASTER ? "(master)" : "(user)",
rc == 0 ? "success" : "fail");
@@ -408,6 +412,8 @@ static int security_overwrite(struct nvdimm *nvdimm, unsigned int keyid)
return -ENOKEY;
rc = nvdimm->sec.ops->overwrite(nvdimm, data);
+ if (rc == 0)
+ set_bit(NDD_INCOHERENT, &nvdimm->flags);
dev_dbg(dev, "key: %d overwrite submission: %s\n", key_serial(key),
rc == 0 ? "success" : "fail");
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 3bf658a74ccb..af38252ad704 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -35,6 +35,11 @@ enum {
NDD_WORK_PENDING = 4,
/* dimm supports namespace labels */
NDD_LABELING = 6,
+ /*
+ * dimm contents have changed requiring invalidation of CPU caches prior
+ * to activation of a region that includes this device
+ */
+ NDD_INCOHERENT = 7,
/* need to set a limit somewhere, but yes, this is likely overkill */
ND_IOCTL_MAX_BUFLEN = SZ_4M,
next prev parent reply other threads:[~2022-12-01 22:04 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-01 22:03 [PATCH 0/5] cxl, nvdimm: Move CPU cache management to region drivers Dan Williams
2022-12-01 22:03 ` [PATCH 1/5] cxl: add dimm_id support for __nvdimm_create() Dan Williams
2022-12-01 22:03 ` [PATCH 2/5] cxl/region: Fix missing probe failure Dan Williams
2022-12-01 22:30 ` Dave Jiang
2022-12-02 1:45 ` Davidlohr Bueso
2022-12-02 14:23 ` Jonathan Cameron
2022-12-03 8:03 ` Dan Williams
2022-12-01 22:03 ` [PATCH 3/5] cxl/pmem: Enforce keyctl ABI for PMEM security Dan Williams
2022-12-01 22:32 ` Dave Jiang
2022-12-01 22:44 ` Dan Williams
2022-12-02 1:49 ` Davidlohr Bueso
2022-12-02 14:24 ` Jonathan Cameron
2022-12-01 22:03 ` Dan Williams [this message]
2022-12-01 23:00 ` [PATCH 4/5] nvdimm/region: Move cache management to the region driver Dave Jiang
2022-12-02 3:21 ` Davidlohr Bueso
2022-12-03 8:01 ` Dan Williams
2022-12-01 22:03 ` [PATCH 5/5] cxl/region: Manage CPU caches relative to DPA invalidation events Dan Williams
2022-12-01 23:04 ` Dave Jiang
2022-12-05 19:20 ` Davidlohr Bueso
2022-12-05 20:10 ` Dan Williams
2022-12-06 9:47 ` Jonathan Cameron
2022-12-06 15:17 ` James Morse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=166993221550.1995348.16843505129579060258.stgit@dwillia2-xfh.jf.intel.com \
--to=dan.j.williams@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=linux-cxl@vger.kernel.org \
--cc=nvdimm@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).