From: David Rientjes <rientjes@google.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 03/14] cxl/mem: Find device capabilities
Date: Mon, 1 Feb 2021 15:09:45 -0800 (PST) [thread overview]
Message-ID: <79b98f60-151b-6c80-65c3-91a37699d121@google.com> (raw)
In-Reply-To: <20210201225052.vrrvuxrsgmddjzbb@intel.com>
On Mon, 1 Feb 2021, Ben Widawsky wrote:
> > I think that's what 8.2.8.4.3 says, no? And then 8.2.8.4.5 says you
> > can use up to Payload Size. That's why my recommendation was to enforce
> > this in cxl_mem_setup_mailbox() up front.
>
> Yeah. I asked our spec people to update 8.2.8.4.5 to make it clearer. I'd argue
> the intent is how you describe it, but the implementation isn't.
>
> My argument was silly anyway because if you specify greater than 1M as your
> payload, you will get EINVAL at the ioctl.
>
> The value of how it works today is the driver will at least bind and allow you
> to interact with it.
>
> How strongly do you feel about this?
>
I haven't seen the update to 8.2.8.4.5 to know yet :)
You make a good point of at least being able to interact with the driver.
I think you could argue that if the driver binds, then the payload size is
accepted, in which case it would be strange to get an EINVAL when using
the ioctl with anything >1MB.
Concern was that if we mask off the reserved bits from the command
register that we could be masking part of the payload size that is being
passed if the accepted max is >1MB. Idea was to avoid any possibility of
this inconsistency. If this is being checked for ioctl, seems like it's
checking reserved bits.
But maybe I should just wait for the spec update.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: David Rientjes <rientjes@google.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 03/14] cxl/mem: Find device capabilities
Date: Mon, 1 Feb 2021 15:09:45 -0800 (PST) [thread overview]
Message-ID: <79b98f60-151b-6c80-65c3-91a37699d121@google.com> (raw)
In-Reply-To: <20210201225052.vrrvuxrsgmddjzbb@intel.com>
On Mon, 1 Feb 2021, Ben Widawsky wrote:
> > I think that's what 8.2.8.4.3 says, no? And then 8.2.8.4.5 says you
> > can use up to Payload Size. That's why my recommendation was to enforce
> > this in cxl_mem_setup_mailbox() up front.
>
> Yeah. I asked our spec people to update 8.2.8.4.5 to make it clearer. I'd argue
> the intent is how you describe it, but the implementation isn't.
>
> My argument was silly anyway because if you specify greater than 1M as your
> payload, you will get EINVAL at the ioctl.
>
> The value of how it works today is the driver will at least bind and allow you
> to interact with it.
>
> How strongly do you feel about this?
>
I haven't seen the update to 8.2.8.4.5 to know yet :)
You make a good point of at least being able to interact with the driver.
I think you could argue that if the driver binds, then the payload size is
accepted, in which case it would be strange to get an EINVAL when using
the ioctl with anything >1MB.
Concern was that if we mask off the reserved bits from the command
register that we could be masking part of the payload size that is being
passed if the accepted max is >1MB. Idea was to avoid any possibility of
this inconsistency. If this is being checked for ioctl, seems like it's
checking reserved bits.
But maybe I should just wait for the spec update.
next prev parent reply other threads:[~2021-02-01 23:09 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 [this message]
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
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=79b98f60-151b-6c80-65c3-91a37699d121@google.com \
--to=rientjes@google.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=ben.widawsky@intel.com \
--cc=cbrowy@avery-design.com \
--cc=daniel.lll@alibaba-inc.com \
--cc=hch@infradead.org \
--cc=helgaas@kernel.org \
--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 \
/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.