All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.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>,
	Ira Weiny <ira.weiny@intel.com>, Jon Masters <jcm@jonmasters.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Rafael Wysocki <rafael.j.wysocki@intel.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Vishal Verma <vishal.l.verma@intel.com>,
	daniel.lll@alibaba-inc.com,
	"John Groves (jgroves)" <jgroves@micron.com>,
	"Kelley, Sean V" <sean.v.kelley@intel.com>
Subject: Re: [PATCH 07/14] cxl/mem: Add send command
Date: Mon, 1 Feb 2021 13:15:35 -0500	[thread overview]
Message-ID: <20210201181535.GI197521@fedora> (raw)
In-Reply-To: <20210130002438.1872527-8-ben.widawsky@intel.com>

> +/**
> + * struct cxl_send_command - Send a command to a memory device.
> + * @id: The command to send to the memory device. This must be one of the
> + *	commands returned by the query command.
> + * @flags: Flags for the command (input).
> + * @rsvd: Must be zero.
> + * @retval: Return value from the memory device (output).
> + * @size_in: Size of the payload to provide to the device (input).
> + * @size_out: Size of the payload received from the device (input/output). This
> + *	      field is filled in by userspace to let the driver know how much
> + *	      space was allocated for output. It is populated by the driver to
> + *	      let userspace know how large the output payload actually was.
> + * @in_payload: Pointer to memory for payload input (little endian order).
> + * @out_payload: Pointer to memory for payload output (little endian order).
> + *
> + * Mechanism for userspace to send a command to the hardware for processing. The
> + * driver will do basic validation on the command sizes. In some cases even the
> + * payload may be introspected. Userspace is required to allocate large
> + * enough buffers for size_out which can be variable length in certain
> + * situations.
> + */
I think (and this would help if you ran `pahole` on this structure) has
some gaps in it:

> +struct cxl_send_command {
> +	__u32 id;
> +	__u32 flags;
> +	__u32 rsvd;
> +	__u32 retval;
> +
> +	struct {
> +		__s32 size_in;

Here..Maybe just add:

__u32 rsv_2;
> +		__u64 in_payload;
> +	};
> +
> +	struct {
> +		__s32 size_out;

And here. Maybe just add:
__u32 rsv_2;
> +		__u64 out_payload;
> +	};
> +};

Perhaps to prepare for the future where this may need to be expanded, you
could add a size at the start of the structure, and
maybe what version of structure it is?

Maybe for all the new structs you are adding?

WARNING: multiple messages have this Message-ID (diff)
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.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>,
	Jon Masters <jcm@jonmasters.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Rafael Wysocki <rafael.j.wysocki@intel.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	daniel.lll@alibaba-inc.com,
	"John Groves (jgroves)" <jgroves@micron.com>,
	"Kelley, Sean V" <sean.v.kelley@intel.com>
Subject: Re: [PATCH 07/14] cxl/mem: Add send command
Date: Mon, 1 Feb 2021 13:15:35 -0500	[thread overview]
Message-ID: <20210201181535.GI197521@fedora> (raw)
In-Reply-To: <20210130002438.1872527-8-ben.widawsky@intel.com>

> +/**
> + * struct cxl_send_command - Send a command to a memory device.
> + * @id: The command to send to the memory device. This must be one of the
> + *	commands returned by the query command.
> + * @flags: Flags for the command (input).
> + * @rsvd: Must be zero.
> + * @retval: Return value from the memory device (output).
> + * @size_in: Size of the payload to provide to the device (input).
> + * @size_out: Size of the payload received from the device (input/output). This
> + *	      field is filled in by userspace to let the driver know how much
> + *	      space was allocated for output. It is populated by the driver to
> + *	      let userspace know how large the output payload actually was.
> + * @in_payload: Pointer to memory for payload input (little endian order).
> + * @out_payload: Pointer to memory for payload output (little endian order).
> + *
> + * Mechanism for userspace to send a command to the hardware for processing. The
> + * driver will do basic validation on the command sizes. In some cases even the
> + * payload may be introspected. Userspace is required to allocate large
> + * enough buffers for size_out which can be variable length in certain
> + * situations.
> + */
I think (and this would help if you ran `pahole` on this structure) has
some gaps in it:

> +struct cxl_send_command {
> +	__u32 id;
> +	__u32 flags;
> +	__u32 rsvd;
> +	__u32 retval;
> +
> +	struct {
> +		__s32 size_in;

Here..Maybe just add:

__u32 rsv_2;
> +		__u64 in_payload;
> +	};
> +
> +	struct {
> +		__s32 size_out;

And here. Maybe just add:
__u32 rsv_2;
> +		__u64 out_payload;
> +	};
> +};

Perhaps to prepare for the future where this may need to be expanded, you
could add a size at the start of the structure, and
maybe what version of structure it is?

Maybe for all the new structs you are adding?
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

  reply	other threads:[~2021-02-01 18:17 UTC|newest]

Thread overview: 193+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-30  0:24 [PATCH 00/14] CXL 2.0 Support Ben Widawsky
2021-01-30  0:24 ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 01/14] cxl/mem: Introduce a driver for CXL-2.0-Type-3 endpoints Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30 23:51   ` David Rientjes
2021-01-30 23:51     ` David Rientjes
2021-02-01 17:21   ` Jonathan Cameron
2021-02-01 17:21     ` Jonathan Cameron
2021-02-01 17:34   ` Konrad Rzeszutek Wilk
2021-02-01 17:34     ` Konrad Rzeszutek Wilk
2021-02-02 17:58     ` Christoph Hellwig
2021-02-02 17:58       ` Christoph Hellwig
2021-02-02 18:00   ` Christoph Hellwig
2021-02-02 18:00     ` Christoph Hellwig
2021-01-30  0:24 ` [PATCH 02/14] cxl/mem: Map memory device registers Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30 23:51   ` David Rientjes
2021-01-30 23:51     ` David Rientjes
2021-02-01 16:46     ` Ben Widawsky
2021-02-01 16:46       ` Ben Widawsky
2021-02-01 18:19       ` Jonathan Cameron
2021-02-01 18:19         ` Jonathan Cameron
2021-02-01 17:36   ` Konrad Rzeszutek Wilk
2021-02-01 17:36     ` Konrad Rzeszutek Wilk
2021-02-02 18:04   ` Christoph Hellwig
2021-02-02 18:04     ` Christoph Hellwig
2021-02-02 18:31     ` Ben Widawsky
2021-02-02 18:31       ` Ben Widawsky
2021-02-03 17:12       ` Christoph Hellwig
2021-02-03 17:12         ` Christoph Hellwig
2021-01-30  0:24 ` [PATCH 03/14] cxl/mem: Find device capabilities Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30 23:51   ` David Rientjes
2021-01-30 23:51     ` David Rientjes
2021-02-01 16:53     ` Ben Widawsky
2021-02-01 16:53       ` Ben Widawsky
2021-02-01 21:51       ` David Rientjes
2021-02-01 21:51         ` David Rientjes
2021-02-01 21:58         ` Ben Widawsky
2021-02-01 21:58           ` Ben Widawsky
2021-02-01 22:23           ` David Rientjes
2021-02-01 22:23             ` David Rientjes
2021-02-01 22:28             ` Ben Widawsky
2021-02-01 22:28               ` Ben Widawsky
2021-02-01 22:33               ` Ben Widawsky
2021-02-01 22:33                 ` Ben Widawsky
2021-02-01 22:45                 ` David Rientjes
2021-02-01 22:45                   ` David Rientjes
2021-02-01 22:50                   ` Ben Widawsky
2021-02-01 22:50                     ` Ben Widawsky
2021-02-01 23:09                     ` David Rientjes
2021-02-01 23:09                       ` David Rientjes
2021-02-01 23:17                       ` Ben Widawsky
2021-02-01 23:17                         ` Ben Widawsky
2021-02-01 23:58                         ` David Rientjes
2021-02-01 23:58                           ` David Rientjes
2021-02-02  0:11                           ` Ben Widawsky
2021-02-02  0:11                             ` Ben Widawsky
2021-02-02  0:14                             ` Dan Williams
2021-02-02  0:14                               ` Dan Williams
2021-02-02  1:09                               ` David Rientjes
2021-02-02  1:09                                 ` David Rientjes
2021-02-01 22:02         ` Dan Williams
2021-02-01 22:02           ` Dan Williams
2021-02-01 17:41   ` Konrad Rzeszutek Wilk
2021-02-01 17:41     ` Konrad Rzeszutek Wilk
2021-02-01 17:50     ` Ben Widawsky
2021-02-01 17:50       ` Ben Widawsky
2021-02-01 18:08       ` Konrad Rzeszutek Wilk
2021-02-01 18:08         ` Konrad Rzeszutek Wilk
2021-02-02 18:10   ` Christoph Hellwig
2021-02-02 18:10     ` Christoph Hellwig
2021-02-02 18:24     ` Ben Widawsky
2021-02-02 18:24       ` Ben Widawsky
2021-02-03 17:15       ` Christoph Hellwig
2021-02-03 17:15         ` Christoph Hellwig
2021-02-03 17:23         ` Ben Widawsky
2021-02-03 17:23           ` Ben Widawsky
2021-02-03 21:23           ` Dan Williams
2021-02-03 21:23             ` Dan Williams
2021-02-04  7:16             ` Christoph Hellwig
2021-02-04  7:16               ` Christoph Hellwig
2021-02-04 15:29               ` Ben Widawsky
2021-02-04 15:29                 ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 04/14] cxl/mem: Implement polled mode mailbox Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30 23:51   ` David Rientjes
2021-01-30 23:51     ` David Rientjes
2021-02-01 20:00     ` Dan Williams
2021-02-01 20:00       ` Dan Williams
2021-02-02 22:57       ` Ben Widawsky
2021-02-02 22:57         ` Ben Widawsky
2021-02-02 23:54         ` Dan Williams
2021-02-02 23:54           ` Dan Williams
2021-02-03  0:54           ` Ben Widawsky
2021-02-03  0:54             ` Ben Widawsky
2021-02-02 22:50     ` Ben Widawsky
2021-02-02 22:50       ` Ben Widawsky
2021-02-01 17:54   ` Konrad Rzeszutek Wilk
2021-02-01 17:54     ` Konrad Rzeszutek Wilk
2021-02-01 19:13     ` Ben Widawsky
2021-02-01 19:13       ` Ben Widawsky
2021-02-01 19:28       ` Dan Williams
2021-02-01 19:28         ` Dan Williams
2021-02-04 21:53         ` [EXT] " John Groves (jgroves)
2021-02-04 22:24           ` Ben Widawsky
2021-02-04 22:24             ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 05/14] cxl/mem: Register CXL memX devices Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30  0:31   ` Dan Williams
2021-01-30  0:31     ` Dan Williams
2021-01-30 23:52   ` David Rientjes
2021-01-30 23:52     ` David Rientjes
2021-02-01 17:10     ` Ben Widawsky
2021-02-01 17:10       ` Ben Widawsky
2021-02-01 21:53       ` David Rientjes
2021-02-01 21:53         ` David Rientjes
2021-02-01 21:55         ` Dan Williams
2021-02-01 21:55           ` Dan Williams
2021-02-02 18:13   ` Christoph Hellwig
2021-02-02 18:13     ` Christoph Hellwig
2021-01-30  0:24 ` [PATCH 06/14] cxl/mem: Add basic IOCTL interface Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-02-02 18:15   ` Christoph Hellwig
2021-02-02 18:15     ` Christoph Hellwig
2021-02-02 18:33     ` Ben Widawsky
2021-02-02 18:33       ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 07/14] cxl/mem: Add send command Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-02-01 18:15   ` Konrad Rzeszutek Wilk [this message]
2021-02-01 18:15     ` Konrad Rzeszutek Wilk
2021-02-02 23:08     ` Ben Widawsky
2021-02-02 23:08       ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 08/14] taint: add taint for direct hardware access Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-02-01 18:18   ` Konrad Rzeszutek Wilk
2021-02-01 18:18     ` Konrad Rzeszutek Wilk
2021-02-01 18:34     ` Ben Widawsky
2021-02-01 18:34       ` Ben Widawsky
2021-02-01 19:01       ` Dan Williams
2021-02-01 19:01         ` Dan Williams
2021-02-02  2:49         ` Konrad Rzeszutek Wilk
2021-02-02  2:49           ` Konrad Rzeszutek Wilk
2021-02-02 17:46           ` Dan Williams
2021-02-02 17:46             ` Dan Williams
2021-02-08 22:00   ` Dan Williams
2021-02-08 22:00     ` Dan Williams
2021-02-08 22:09     ` Kees Cook
2021-02-08 22:09       ` Kees Cook
2021-02-08 23:05       ` Ben Widawsky
2021-02-08 23:05         ` Ben Widawsky
2021-02-08 23:36       ` Dan Williams
2021-02-08 23:36         ` Dan Williams
2021-02-09  1:03         ` Dan Williams
2021-02-09  1:03           ` Dan Williams
2021-02-09  3:36           ` Ben Widawsky
2021-02-09  3:36             ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 09/14] cxl/mem: Add a "RAW" send command Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-02-01 18:24   ` Konrad Rzeszutek Wilk
2021-02-01 18:24     ` Konrad Rzeszutek Wilk
2021-02-01 19:27     ` Ben Widawsky
2021-02-01 19:27       ` Ben Widawsky
2021-02-01 19:34       ` Konrad Rzeszutek Wilk
2021-02-01 19:34         ` Konrad Rzeszutek Wilk
2021-02-01 21:20         ` Dan Williams
2021-02-01 21:20           ` Dan Williams
2021-01-30  0:24 ` [PATCH 10/14] cxl/mem: Create concept of enabled commands Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 11/14] cxl/mem: Use CEL for enabling commands Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 12/14] cxl/mem: Add set of informational commands Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-01-30  0:24 ` [PATCH 13/14] cxl/mem: Add limited Get Log command (0401h) Ben Widawsky
2021-01-30  0:24   ` Ben Widawsky
2021-02-01 18:28   ` Konrad Rzeszutek Wilk
2021-02-01 18:28     ` Konrad Rzeszutek Wilk
2021-02-02 23:51     ` Ben Widawsky
2021-02-02 23:51       ` Ben Widawsky
2021-02-02 23:57       ` Dan Williams
2021-02-02 23:57         ` Dan Williams
2021-02-03 17:16         ` Ben Widawsky
2021-02-03 17:16           ` Ben Widawsky
2021-02-03 18:14           ` Konrad Rzeszutek Wilk
2021-02-03 18:14             ` Konrad Rzeszutek Wilk
2021-02-03 20:31             ` Dan Williams
2021-02-03 20:31               ` Dan Williams
2021-02-04 18:55               ` Ben Widawsky
2021-02-04 18:55                 ` Ben Widawsky
2021-02-04 21:01                 ` Dan Williams
2021-02-04 21:01                   ` Dan Williams
2021-01-30  0:24 ` [PATCH 14/14] MAINTAINERS: Add maintainers of the CXL driver Ben Widawsky
2021-01-30  0:24   ` 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=20210201181535.GI197521@fedora \
    --to=konrad.wilk@oracle.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=ben.widawsky@intel.com \
    --cc=cbrowy@avery-design.com \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.lll@alibaba-inc.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=sean.v.kelley@intel.com \
    --cc=vishal.l.verma@intel.com \
    /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 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.