diff mbox series

[4/4] cxl/security: Drop security command ioctl uapi

Message ID 167030056464.4044561.11486507095384253833.stgit@dwillia2-xfh.jf.intel.com
State Accepted
Commit 7fe898041fb0c8e630504ecc2cb8805651ac85c1
Headers show
Series cxl/mbox: Output payload validation reworks | expand

Commit Message

Dan Williams Dec. 6, 2022, 4:22 a.m. UTC 
CXL PMEM security operations are routed through the NVDIMM sysfs
interface. For this reason the corresponding commands are marked
"exclusive" to preclude collisions between the ioctl ABI and the sysfs
ABI. However, a better way to preclude that collision is to simply
remove the ioctl ABI (command-id definitions) for those operations.

Now that cxl_internal_send_cmd() (formerly cxl_mbox_send_cmd()) no
longer needs to talk the cxl_mem_commands array, all of the uapi
definitions for the security commands can be dropped.

These never appeared in a released kernel, so no regression risk.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/cxl/core/mbox.c      |   17 -----------------
 include/uapi/linux/cxl_mem.h |    6 ------
 2 files changed, 23 deletions(-)

Comments

Ira Weiny Dec. 6, 2022, 6:38 a.m. UTC | #1 
On Mon, Dec 05, 2022 at 08:22:44PM -0800, Dan Williams wrote:
> CXL PMEM security operations are routed through the NVDIMM sysfs
> interface. For this reason the corresponding commands are marked
> "exclusive" to preclude collisions between the ioctl ABI and the sysfs
> ABI. However, a better way to preclude that collision is to simply
> remove the ioctl ABI (command-id definitions) for those operations.
> 
> Now that cxl_internal_send_cmd() (formerly cxl_mbox_send_cmd()) no
> longer needs to talk the cxl_mem_commands array, all of the uapi
> definitions for the security commands can be dropped.
> 
> These never appeared in a released kernel, so no regression risk.
> 

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/mbox.c      |   17 -----------------
>  include/uapi/linux/cxl_mem.h |    6 ------
>  2 files changed, 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index c36a3589377a..b03fba212799 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -65,12 +65,6 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>  	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
>  	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> -	CXL_CMD(GET_SECURITY_STATE, 0, 0x4, 0),
> -	CXL_CMD(SET_PASSPHRASE, 0x60, 0, 0),
> -	CXL_CMD(DISABLE_PASSPHRASE, 0x40, 0, 0),
> -	CXL_CMD(FREEZE_SECURITY, 0, 0, 0),
> -	CXL_CMD(UNLOCK, 0x20, 0, 0),
> -	CXL_CMD(PASSPHRASE_SECURE_ERASE, 0x40, 0, 0),
>  };
>  
>  /*
> @@ -717,17 +711,6 @@ int cxl_enumerate_cmds(struct cxl_dev_state *cxlds)
>  		/* Found the required CEL */
>  		rc = 0;
>  	}
> -
> -	/*
> -	 * Setup permanently kernel exclusive commands, i.e. the
> -	 * mechanism is driven through sysfs, keyctl, etc...
> -	 */
> -	set_bit(CXL_MEM_COMMAND_ID_SET_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_DISABLE_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_UNLOCK, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_PASSPHRASE_SECURE_ERASE,
> -		cxlds->exclusive_cmds);
> -
>  out:
>  	kvfree(gsl);
>  	return rc;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index 82bdad4ce5de..c71021a2a9ed 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -41,12 +41,6 @@
>  	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
>  	___C(SCAN_MEDIA, "Scan Media"),                                   \
>  	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
> -	___C(GET_SECURITY_STATE, "Get Security State"),			  \
> -	___C(SET_PASSPHRASE, "Set Passphrase"),				  \
> -	___C(DISABLE_PASSPHRASE, "Disable Passphrase"),			  \
> -	___C(FREEZE_SECURITY, "Freeze Security"),			  \
> -	___C(UNLOCK, "Unlock"),						  \
> -	___C(PASSPHRASE_SECURE_ERASE, "Passphrase Secure Erase"),	  \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
>
Dave Jiang Dec. 6, 2022, 4:56 p.m. UTC | #2 
On 12/5/2022 9:22 PM, Dan Williams wrote:
> CXL PMEM security operations are routed through the NVDIMM sysfs
> interface. For this reason the corresponding commands are marked
> "exclusive" to preclude collisions between the ioctl ABI and the sysfs
> ABI. However, a better way to preclude that collision is to simply
> remove the ioctl ABI (command-id definitions) for those operations.
> 
> Now that cxl_internal_send_cmd() (formerly cxl_mbox_send_cmd()) no
> longer needs to talk the cxl_mem_commands array, all of the uapi
> definitions for the security commands can be dropped.
> 
> These never appeared in a released kernel, so no regression risk.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   drivers/cxl/core/mbox.c      |   17 -----------------
>   include/uapi/linux/cxl_mem.h |    6 ------
>   2 files changed, 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index c36a3589377a..b03fba212799 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -65,12 +65,6 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>   	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>   	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
>   	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> -	CXL_CMD(GET_SECURITY_STATE, 0, 0x4, 0),
> -	CXL_CMD(SET_PASSPHRASE, 0x60, 0, 0),
> -	CXL_CMD(DISABLE_PASSPHRASE, 0x40, 0, 0),
> -	CXL_CMD(FREEZE_SECURITY, 0, 0, 0),
> -	CXL_CMD(UNLOCK, 0x20, 0, 0),
> -	CXL_CMD(PASSPHRASE_SECURE_ERASE, 0x40, 0, 0),
>   };
>   
>   /*
> @@ -717,17 +711,6 @@ int cxl_enumerate_cmds(struct cxl_dev_state *cxlds)
>   		/* Found the required CEL */
>   		rc = 0;
>   	}
> -
> -	/*
> -	 * Setup permanently kernel exclusive commands, i.e. the
> -	 * mechanism is driven through sysfs, keyctl, etc...
> -	 */
> -	set_bit(CXL_MEM_COMMAND_ID_SET_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_DISABLE_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_UNLOCK, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_PASSPHRASE_SECURE_ERASE,
> -		cxlds->exclusive_cmds);
> -
>   out:
>   	kvfree(gsl);
>   	return rc;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index 82bdad4ce5de..c71021a2a9ed 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -41,12 +41,6 @@
>   	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
>   	___C(SCAN_MEDIA, "Scan Media"),                                   \
>   	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
> -	___C(GET_SECURITY_STATE, "Get Security State"),			  \
> -	___C(SET_PASSPHRASE, "Set Passphrase"),				  \
> -	___C(DISABLE_PASSPHRASE, "Disable Passphrase"),			  \
> -	___C(FREEZE_SECURITY, "Freeze Security"),			  \
> -	___C(UNLOCK, "Unlock"),						  \
> -	___C(PASSPHRASE_SECURE_ERASE, "Passphrase Secure Erase"),	  \
>   	___C(MAX, "invalid / last command")
>   
>   #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
>
Jonathan Cameron Dec. 8, 2022, 10:51 a.m. UTC | #3 
On Mon, 05 Dec 2022 20:22:44 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> CXL PMEM security operations are routed through the NVDIMM sysfs
> interface. For this reason the corresponding commands are marked
> "exclusive" to preclude collisions between the ioctl ABI and the sysfs
> ABI. However, a better way to preclude that collision is to simply
> remove the ioctl ABI (command-id definitions) for those operations.
> 
> Now that cxl_internal_send_cmd() (formerly cxl_mbox_send_cmd()) no
> longer needs to talk the cxl_mem_commands array, all of the uapi
> definitions for the security commands can be dropped.
> 
> These never appeared in a released kernel, so no regression risk.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Makes sense

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/core/mbox.c      |   17 -----------------
>  include/uapi/linux/cxl_mem.h |    6 ------
>  2 files changed, 23 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index c36a3589377a..b03fba212799 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -65,12 +65,6 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
>  	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
>  	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
> -	CXL_CMD(GET_SECURITY_STATE, 0, 0x4, 0),
> -	CXL_CMD(SET_PASSPHRASE, 0x60, 0, 0),
> -	CXL_CMD(DISABLE_PASSPHRASE, 0x40, 0, 0),
> -	CXL_CMD(FREEZE_SECURITY, 0, 0, 0),
> -	CXL_CMD(UNLOCK, 0x20, 0, 0),
> -	CXL_CMD(PASSPHRASE_SECURE_ERASE, 0x40, 0, 0),
>  };
>  
>  /*
> @@ -717,17 +711,6 @@ int cxl_enumerate_cmds(struct cxl_dev_state *cxlds)
>  		/* Found the required CEL */
>  		rc = 0;
>  	}
> -
> -	/*
> -	 * Setup permanently kernel exclusive commands, i.e. the
> -	 * mechanism is driven through sysfs, keyctl, etc...
> -	 */
> -	set_bit(CXL_MEM_COMMAND_ID_SET_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_DISABLE_PASSPHRASE, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_UNLOCK, cxlds->exclusive_cmds);
> -	set_bit(CXL_MEM_COMMAND_ID_PASSPHRASE_SECURE_ERASE,
> -		cxlds->exclusive_cmds);
> -
>  out:
>  	kvfree(gsl);
>  	return rc;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index 82bdad4ce5de..c71021a2a9ed 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -41,12 +41,6 @@
>  	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
>  	___C(SCAN_MEDIA, "Scan Media"),                                   \
>  	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
> -	___C(GET_SECURITY_STATE, "Get Security State"),			  \
> -	___C(SET_PASSPHRASE, "Set Passphrase"),				  \
> -	___C(DISABLE_PASSPHRASE, "Disable Passphrase"),			  \
> -	___C(FREEZE_SECURITY, "Freeze Security"),			  \
> -	___C(UNLOCK, "Unlock"),						  \
> -	___C(PASSPHRASE_SECURE_ERASE, "Passphrase Secure Erase"),	  \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index c36a3589377a..b03fba212799 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -65,12 +65,6 @@  static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
 	CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0),
 	CXL_CMD(SCAN_MEDIA, 0x11, 0, 0),
 	CXL_CMD(GET_SCAN_MEDIA, 0, CXL_VARIABLE_PAYLOAD, 0),
-	CXL_CMD(GET_SECURITY_STATE, 0, 0x4, 0),
-	CXL_CMD(SET_PASSPHRASE, 0x60, 0, 0),
-	CXL_CMD(DISABLE_PASSPHRASE, 0x40, 0, 0),
-	CXL_CMD(FREEZE_SECURITY, 0, 0, 0),
-	CXL_CMD(UNLOCK, 0x20, 0, 0),
-	CXL_CMD(PASSPHRASE_SECURE_ERASE, 0x40, 0, 0),
 };
 
 /*
@@ -717,17 +711,6 @@  int cxl_enumerate_cmds(struct cxl_dev_state *cxlds)
 		/* Found the required CEL */
 		rc = 0;
 	}
-
-	/*
-	 * Setup permanently kernel exclusive commands, i.e. the
-	 * mechanism is driven through sysfs, keyctl, etc...
-	 */
-	set_bit(CXL_MEM_COMMAND_ID_SET_PASSPHRASE, cxlds->exclusive_cmds);
-	set_bit(CXL_MEM_COMMAND_ID_DISABLE_PASSPHRASE, cxlds->exclusive_cmds);
-	set_bit(CXL_MEM_COMMAND_ID_UNLOCK, cxlds->exclusive_cmds);
-	set_bit(CXL_MEM_COMMAND_ID_PASSPHRASE_SECURE_ERASE,
-		cxlds->exclusive_cmds);
-
 out:
 	kvfree(gsl);
 	return rc;
diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index 82bdad4ce5de..c71021a2a9ed 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -41,12 +41,6 @@ 
 	___C(GET_SCAN_MEDIA_CAPS, "Get Scan Media Capabilities"),         \
 	___C(SCAN_MEDIA, "Scan Media"),                                   \
 	___C(GET_SCAN_MEDIA, "Get Scan Media Results"),                   \
-	___C(GET_SECURITY_STATE, "Get Security State"),			  \
-	___C(SET_PASSPHRASE, "Set Passphrase"),				  \
-	___C(DISABLE_PASSPHRASE, "Disable Passphrase"),			  \
-	___C(FREEZE_SECURITY, "Freeze Security"),			  \
-	___C(UNLOCK, "Unlock"),						  \
-	___C(PASSPHRASE_SECURE_ERASE, "Passphrase Secure Erase"),	  \
 	___C(MAX, "invalid / last command")
 
 #define ___C(a, b) CXL_MEM_COMMAND_ID_##a