linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Ben Widawsky <ben.widawsky@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <linux-acpi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-nvdimm@lists.01.org>,
	<linux-pci@vger.kernel.org>, Bjorn Helgaas <helgaas@kernel.org>,
	"Chris Browy" <cbrowy@avery-design.com>,
	Christoph Hellwig <hch@infradead.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	David Hildenbrand <david@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Ira Weiny <ira.weiny@intel.com>,
	"Jon Masters" <jcm@jonmasters.org>,
	Rafael Wysocki <rafael.j.wysocki@intel.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Vishal Verma <vishal.l.verma@intel.com>,
	"John Groves (jgroves)" <jgroves@micron.com>,
	"Kelley, Sean V" <sean.v.kelley@intel.com>
Subject: Re: [PATCH v2 7/8] cxl/mem: Add set of informational commands
Date: Thu, 11 Feb 2021 12:07:36 +0000	[thread overview]
Message-ID: <20210211120736.00006778@Huawei.com> (raw)
In-Reply-To: <20210210000259.635748-8-ben.widawsky@intel.com>

On Tue, 9 Feb 2021 16:02:58 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> Add initial set of formal commands beyond basic identify and command
> enumeration.
> 
> Of special note is the Get Log Command which is only specified to return
> 2 log types, CEL and VENDOR_DEBUG. Given that VENDOR_DEBUG is already a
> large catch all for vendor specific information there is no known reason
> for devices to be implementing other log types. Unknown log types are
> included in the "vendor passthrough shenanigans" safety regime like raw
> commands and blocked by default.

As mentioned in previous patch comments, the way that is worded in the spec
suggests to me that what we might see if other specifications providing
more UUIDs to define other 'standard' info.  Maybe something else was
intended...   Still what you have done here makes sense to me.

> 
> Up to this point there has been no reason to inspect payload data.
> Given the need to check the log type add a new "validate_payload"
> operation to define a generic mechanism to restrict / filter commands.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/mem.c            | 55 +++++++++++++++++++++++++++++++++++-
>  include/uapi/linux/cxl_mem.h |  5 ++++
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index e9aa6ca18d99..e8cc076b9f1b 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -44,12 +44,16 @@
>  enum opcode {
>  	CXL_MBOX_OP_INVALID		= 0x0000,
>  	CXL_MBOX_OP_RAW			= CXL_MBOX_OP_INVALID,
> +	CXL_MBOX_OP_GET_FW_INFO		= 0x0200,
>  	CXL_MBOX_OP_ACTIVATE_FW		= 0x0202,
>  	CXL_MBOX_OP_GET_SUPPORTED_LOGS	= 0x0400,
>  	CXL_MBOX_OP_GET_LOG		= 0x0401,
>  	CXL_MBOX_OP_IDENTIFY		= 0x4000,
> +	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
>  	CXL_MBOX_OP_SET_PARTITION_INFO	= 0x4101,
> +	CXL_MBOX_OP_GET_LSA		= 0x4102,
>  	CXL_MBOX_OP_SET_LSA		= 0x4103,
> +	CXL_MBOX_OP_GET_HEALTH_INFO	= 0x4200,
>  	CXL_MBOX_OP_SET_SHUTDOWN_STATE	= 0x4204,
>  	CXL_MBOX_OP_SCAN_MEDIA		= 0x4304,
>  	CXL_MBOX_OP_GET_SCAN_MEDIA	= 0x4305,
> @@ -118,6 +122,9 @@ static const uuid_t log_uuid[] = {
>  					0xd6, 0x07, 0x19, 0x40, 0x3d, 0x86)
>  };
>  
> +static int validate_log_uuid(struct cxl_mem *cxlm, void __user *payload,
> +			     size_t size);
> +
>  /**
>   * struct cxl_mem_command - Driver representation of a memory device command
>   * @info: Command information as it exists for the UAPI
> @@ -129,6 +136,10 @@ static const uuid_t log_uuid[] = {
>   *  * %CXL_CMD_INTERNAL_FLAG_PSEUDO: This is a pseudo command which doesn't have
>   *    a direct mapping to hardware. They are implicitly always enabled.
>   *
> + * @validate_payload: A function called after the command is validated but
> + * before it's sent to the hardware. The primary purpose is to validate, or
> + * fixup the actual payload.
> + *
>   * The cxl_mem_command is the driver's internal representation of commands that
>   * are supported by the driver. Some of these commands may not be supported by
>   * the hardware. The driver will use @info to validate the fields passed in by
> @@ -139,9 +150,12 @@ static const uuid_t log_uuid[] = {
>  struct cxl_mem_command {
>  	struct cxl_command_info info;
>  	enum opcode opcode;
> +
> +	int (*validate_payload)(struct cxl_mem *cxlm, void __user *payload,
> +				size_t size);
>  };
>  
> -#define CXL_CMD(_id, _flags, sin, sout)                                        \
> +#define CXL_CMD_VALIDATE(_id, _flags, sin, sout, v)                            \
>  	[CXL_MEM_COMMAND_ID_##_id] = {                                         \
>  	.info =	{                                                              \
>  			.id = CXL_MEM_COMMAND_ID_##_id,                        \
> @@ -150,8 +164,12 @@ struct cxl_mem_command {
>  			.size_out = sout,                                      \
>  		},                                                             \
>  	.opcode = CXL_MBOX_OP_##_id,                                           \
> +	.validate_payload = v,                                                 \
>  	}
>  
> +#define CXL_CMD(_id, _flags, sin, sout)                                        \
> +	CXL_CMD_VALIDATE(_id, _flags, sin, sout, NULL)
> +
>  /*
>   * This table defines the supported mailbox commands for the driver. This table
>   * is made up of a UAPI structure. Non-negative values as parameters in the
> @@ -164,6 +182,11 @@ static struct cxl_mem_command mem_commands[] = {
>  	CXL_CMD(RAW, NONE, ~0, ~0),
>  #endif
>  	CXL_CMD(GET_SUPPORTED_LOGS, NONE, 0, ~0),
> +	CXL_CMD(GET_FW_INFO, NONE, 0, 0x50),
> +	CXL_CMD(GET_PARTITION_INFO, NONE, 0, 0x20),
> +	CXL_CMD(GET_LSA, NONE, 0x8, ~0),
> +	CXL_CMD(GET_HEALTH_INFO, NONE, 0, 0x12),
> +	CXL_CMD_VALIDATE(GET_LOG, NONE, 0x18, ~0, validate_log_uuid),
>  };
>  
>  /*
> @@ -492,6 +515,14 @@ static int handle_mailbox_cmd_from_user(struct cxl_memdev *cxlmd,
>  		mbox_cmd.payload_out = kvzalloc(cxlm->payload_size, GFP_KERNEL);
>  
>  	if (cmd->info.size_in) {
> +		if (cmd->validate_payload) {
> +			rc = cmd->validate_payload(cxlm,
> +						   u64_to_user_ptr(in_payload),
> +						   cmd->info.size_in);
> +			if (rc)
> +				goto out;
> +		}
> +
>  		mbox_cmd.payload_in = kvzalloc(cmd->info.size_in, GFP_KERNEL);
>  		if (!mbox_cmd.payload_in) {
>  			rc = -ENOMEM;
> @@ -1124,6 +1155,28 @@ struct cxl_mbox_get_log {
>  	__le32 length;
>  } __packed;
>  
> +static int validate_log_uuid(struct cxl_mem *cxlm, void __user *input,
> +			     size_t size)
> +{
> +	struct cxl_mbox_get_log __user *get_log = input;
> +	uuid_t payload_uuid;
> +
> +	if (copy_from_user(&payload_uuid, &get_log->uuid, sizeof(uuid_t)))
> +		return -EFAULT;
> +
> +	if (uuid_equal(&payload_uuid, &log_uuid[CEL_UUID]))
> +		return 0;
> +	if (uuid_equal(&payload_uuid, &log_uuid[VENDOR_DEBUG_UUID]))
> +		return 0;
> +
> +	/* All unspec'd logs shall taint */
> +	if (WARN_ONCE(!cxl_mem_raw_command_allowed(CXL_MBOX_OP_RAW),
> +		      "Unknown log UUID %pU used\n", &payload_uuid))
> +		return -EPERM;
> +
> +	return 0;
> +}
> +
>  static int cxl_xfer_log(struct cxl_mem *cxlm, uuid_t *uuid, u32 size, u8 *out)
>  {
>  	u32 remaining = size;
> diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
> index c5e75b9dad9d..ba4d3b4d6b7d 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -24,6 +24,11 @@
>  	___C(IDENTIFY, "Identify Command"),                               \
>  	___C(RAW, "Raw device command"),                                  \
>  	___C(GET_SUPPORTED_LOGS, "Get Supported Logs"),                   \
> +	___C(GET_FW_INFO, "Get FW Info"),                                 \
> +	___C(GET_PARTITION_INFO, "Get Partition Information"),            \
> +	___C(GET_LSA, "Get Label Storage Area"),                          \
> +	___C(GET_HEALTH_INFO, "Get Health Info"),                         \
> +	___C(GET_LOG, "Get Log"),                                         \
>  	___C(MAX, "Last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a


  reply	other threads:[~2021-02-11 12:11 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10  0:02 [PATCH v2 0/8] CXL 2.0 Support Ben Widawsky
2021-02-10  0:02 ` [PATCH v2 1/8] cxl/mem: Introduce a driver for CXL-2.0-Type-3 endpoints Ben Widawsky
2021-02-10 16:17   ` Jonathan Cameron
2021-02-10 17:12     ` Ben Widawsky
2021-02-10 17:23       ` Jonathan Cameron
2021-02-10  0:02 ` [PATCH v2 2/8] cxl/mem: Find device capabilities Ben Widawsky
2021-02-10 13:32   ` Jonathan Cameron
2021-02-10 15:07     ` Jonathan Cameron
2021-02-10 16:55       ` Ben Widawsky
2021-02-10 17:30         ` Jonathan Cameron
2021-02-10 18:16         ` Ben Widawsky
2021-02-11  9:55           ` Jonathan Cameron
2021-02-11 15:55             ` Ben Widawsky
2021-02-12 13:27               ` Jonathan Cameron
2021-02-12 15:54                 ` Ben Widawsky
2021-02-11 18:27             ` Ben Widawsky
2021-02-12 13:23               ` Jonathan Cameron
2021-02-10 19:32     ` Ben Widawsky
2021-02-10 17:41   ` Jonathan Cameron
2021-02-10 18:53     ` Ben Widawsky
2021-02-10 19:54       ` Dan Williams
2021-02-11 10:01         ` Jonathan Cameron
2021-02-11 16:04           ` Ben Widawsky
2021-02-10  0:02 ` [PATCH v2 3/8] cxl/mem: Register CXL memX devices Ben Widawsky
2021-02-10 18:17   ` Jonathan Cameron
2021-02-11 10:17     ` Jonathan Cameron
2021-02-11 20:40       ` Dan Williams
2021-02-12 13:33         ` Jonathan Cameron
2021-02-10  0:02 ` [PATCH v2 4/8] cxl/mem: Add basic IOCTL interface Ben Widawsky
2021-02-10 18:45   ` Jonathan Cameron
2021-02-10 20:22     ` Ben Widawsky
2021-02-11  4:40     ` Dan Williams
2021-02-11 10:06       ` Jonathan Cameron
2021-02-11 16:54         ` Ben Widawsky
2021-02-14 16:30   ` Al Viro
2021-02-14 23:14     ` Ben Widawsky
2021-02-14 23:50       ` Al Viro
2021-02-14 23:57         ` Al Viro
2021-02-10  0:02 ` [PATCH v2 5/8] cxl/mem: Add a "RAW" send command Ben Widawsky
2021-02-10 15:26   ` Ariel.Sibley
2021-02-10 16:49     ` Ben Widawsky
2021-02-10 18:03       ` Ariel.Sibley
2021-02-10 18:11         ` Ben Widawsky
2021-02-10 18:46           ` Ariel.Sibley
2021-02-10 19:12             ` Ben Widawsky
2021-02-11 16:43     ` Dan Williams
2021-02-11 11:19   ` Jonathan Cameron
2021-02-11 16:01     ` Ben Widawsky
2021-02-12 13:40       ` Jonathan Cameron
2021-02-10  0:02 ` [PATCH v2 6/8] cxl/mem: Enable commands via CEL Ben Widawsky
2021-02-11 12:02   ` Jonathan Cameron
2021-02-11 17:45     ` Ben Widawsky
2021-02-11 20:34       ` Dan Williams
2021-02-16 13:43     ` Bartosz Golaszewski
2021-02-10  0:02 ` [PATCH v2 7/8] cxl/mem: Add set of informational commands Ben Widawsky
2021-02-11 12:07   ` Jonathan Cameron [this message]
2021-02-10  0:02 ` [PATCH v2 8/8] MAINTAINERS: Add maintainers of the CXL driver Ben Widawsky

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=20210211120736.00006778@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=ben.widawsky@intel.com \
    --cc=cbrowy@avery-design.com \
    --cc=dan.j.williams@intel.com \
    --cc=david@redhat.com \
    --cc=hch@infradead.org \
    --cc=helgaas@kernel.org \
    --cc=ira.weiny@intel.com \
    --cc=jcm@jonmasters.org \
    --cc=jgroves@micron.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rdunlap@infradead.org \
    --cc=rientjes@google.com \
    --cc=sean.v.kelley@intel.com \
    --cc=vishal.l.verma@intel.com \
    --subject='Re: [PATCH v2 7/8] cxl/mem: Add set of informational commands' \
    /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

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).