On Wed, 30 Nov 2022 12:23:18 -0700
Dave Jiang <dave.ji...@intel.com> wrote:
> Bypass cpu_cache_invalidate_memregion() and checks when doing testing
> using CONFIG_NVDIMM_SECURITY_TEST flag. The bypass allows testing on
> QEMU where cpu_cache_has_invalidate_memregion() fails.
We should probably look at what is required to get that to not fail on QEMU
at least when running fully emulated on x86.
Longer term we'll figure out other architecture solutions for this...
FWIW this looks consistent with what you are aiming to do so.
So subject to the usual I've no idea how this intel specific code works in
general, so only focusing on the corner this patch touches...
Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
Jonathan
> Usage of
> cpu_cache_invalidate_memregion() is not needed for nfit_test security
> testing.
>
> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
> ---
> drivers/acpi/nfit/intel.c | 51
> +++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
> index fa0e57e35162..38069f10c316 100644
> --- a/drivers/acpi/nfit/intel.c
> +++ b/drivers/acpi/nfit/intel.c
> @@ -191,6 +191,39 @@ static int intel_security_change_key(struct nvdimm
> *nvdimm,
> }
> }
>
> +static bool intel_has_invalidate_memregion(struct nvdimm *nvdimm)
> +{
> + struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> + struct acpi_nfit_desc *acpi_desc = nfit_mem->acpi_desc;
> + struct device *dev = acpi_desc->dev;
> +
> + if (!cpu_cache_has_invalidate_memregion()) {
> + if (IS_ENABLED(CONFIG_NVDIMM_SECURITY_TEST)) {
> + dev_warn_once(dev,
> + "Bypassing
> cpu_cache_has_invalidate_memregion() check for testing!\n");
> + return true;
> + }
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static void intel_invalidate_memregion(struct nvdimm *nvdimm)
> +{
> + struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> + struct acpi_nfit_desc *acpi_desc = nfit_mem->acpi_desc;
> + struct device *dev = acpi_desc->dev;
> +
> + if (IS_ENABLED(CONFIG_NVDIMM_SECURITY_TEST)) {
> + dev_warn_once(dev,
> + "Bypassing cpu_cache_invalidate_memergion() for
> testing!\n");
> + return;
> + }
> +
> + cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> +}
> +
> static int __maybe_unused intel_security_unlock(struct nvdimm *nvdimm,
> const struct nvdimm_key_data *key_data)
> {
> @@ -212,7 +245,7 @@ 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())
> + if (!intel_has_invalidate_memregion(nvdimm))
> return -EINVAL;
>
> memcpy(nd_cmd.cmd.passphrase, key_data->data,
> @@ -230,7 +263,7 @@ static int __maybe_unused intel_security_unlock(struct
> nvdimm *nvdimm,
> }
>
> /* DIMM unlocked, invalidate all CPU caches before we read it */
> - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> + intel_invalidate_memregion(nvdimm);
>
> return 0;
> }
> @@ -299,11 +332,11 @@ 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())
> + if (!intel_has_invalidate_memregion(nvdimm))
> return -EINVAL;
>
> /* flush all cache before we erase DIMM */
> - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> + intel_invalidate_memregion(nvdimm);
> 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);
> @@ -323,7 +356,7 @@ static int __maybe_unused intel_security_erase(struct
> nvdimm *nvdimm,
> }
>
> /* DIMM erased, invalidate all CPU caches before we read it */
> - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> + intel_invalidate_memregion(nvdimm);
> return 0;
> }
>
> @@ -346,7 +379,7 @@ 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())
> + if (!intel_has_invalidate_memregion(nvdimm))
> return -EINVAL;
>
> rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
> @@ -363,7 +396,7 @@ static int __maybe_unused
> intel_security_query_overwrite(struct nvdimm *nvdimm)
> }
>
> /* flush all cache before we make the nvdimms available */
> - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> + intel_invalidate_memregion(nvdimm);
> return 0;
> }
>
> @@ -388,11 +421,11 @@ 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())
> + if (!intel_has_invalidate_memregion(nvdimm))
> return -EINVAL;
>
> /* flush all cache before we erase DIMM */
> - cpu_cache_invalidate_memregion(IORES_DESC_PERSISTENT_MEMORY);
> + intel_invalidate_memregion(nvdimm);
> 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);
>
>
