Received-SPF: pass (google.com: domain of linux-kernel+bounces-137890-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 9 Apr 2024 21:35:40 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: Dave Jiang <dave.jiang@intel.com>, <ira.weiny@intel.com>, Fan Ni
	<fan.ni@samsung.com>, Jonathan Cameron <Jonathan.Cameron@huawei.com>, Navneet
 Singh <navneet.singh@intel.com>
CC: Dan Williams <dan.j.williams@intel.com>, Davidlohr Bueso
	<dave@stgolabs.net>, Alison Schofield <alison.schofield@intel.com>, "Vishal
 Verma" <vishal.l.verma@intel.com>, <linux-btrfs@vger.kernel.org>,
	<linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 09/26] cxl/region: Add Dynamic Capacity CXL region support
Message-ID: <6616171c47d0c_e9f9f29460@iweiny-mobl.notmuch>
References: <20240324-dcd-type2-upstream-v1-0-b7b00d623625@intel.com>
 <20240324-dcd-type2-upstream-v1-9-b7b00d623625@intel.com>
 <dde02c34-fcf3-4adc-8920-7fd7fe202d54@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <dde02c34-fcf3-4adc-8920-7fd7fe202d54@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?X9kPh8qtqy0+5ItMYuM04sCf6lfUtuAAvInTmbyQKYlzJqfDEuOVtlCGg+YR?=
 =?us-ascii?Q?0LWr1a6axqqpYDMOBQVDrqvcvy3AwV5ZqDIDuQlJWgPxqZWBPNFfJSJjDY36?=
 =?us-ascii?Q?rrklfYqEye0D1w8Bi1m+bsH4DJeyTFJf3qnnOzpKp5x9U0jmrsdE1WwNnJa+?=
 =?us-ascii?Q?NFfPRyxDXfwYyhipojI4srVeVGJQhev94sHxQ/obKXag1qS7IvQY9BbXNZSP?=
 =?us-ascii?Q?L8fkCODqi8qXSEtHoxQBuILBg0Srwn6Je4EEEHL3GkOT+Gc9slrIbkplE2t5?=
 =?us-ascii?Q?jo5cj69CT54wcQ5HjNVc9vGHcwjiEgnrexVl+tZZ3GBK7/pyzBkhUOXGEoqR?=
 =?us-ascii?Q?GqOzHmrTGpOkNiz0P6eCxAZ3y+CymFlkC2jwVvSFYe1+RfytddRBkuIkO9Ya?=
 =?us-ascii?Q?/dy416mA/amrksHI38rJ8r7wS8SPOJDSxQsjrvP0ihxhYL/gZt0LO2wCZVPm?=
 =?us-ascii?Q?zU+R5hLXuXm/UazoyyR4LAYpvQJmUZNydF6YnZlhXSW52hrxxnK8RAWrWzRR?=
 =?us-ascii?Q?vBD4199f3QD9p2bp2PpEH/vDXwvarhqoTeXE/Eit5ulmsObGd1w6XWgKys9Q?=
 =?us-ascii?Q?iul3KgzdnOGhScxmE5PV410udvDepjxZN3z5LXcRZlQKOJ75/T1KG2fQkz2E?=
 =?us-ascii?Q?r/WFCaBZUBo+KZIBB1BYxShRcn1B4HS+buvTwlWSL9ZEOKxZ9tcYU/vdUTgb?=
 =?us-ascii?Q?Askk7Q/AGDqPkAFEPWCVNX9lKW2HdxDjkdNuO89hsXXnGXI2n0t6MKHTObai?=
 =?us-ascii?Q?Jl+Sh9t3KElGcouVPnTCRrhCCYDgQVk7PKOZH5ujMJBEGbp4tomlXuEK/ARU?=
 =?us-ascii?Q?PSqCgZsas/jerws+F6q2zz8gtktXIcxZDDjSyyE6piK5KrKfg1qvehM7tUiq?=
 =?us-ascii?Q?Sh91KIMptkQaXt1QDnMebmfsogYrce2VECnzaEmplLcDyor10DxZrSgnvOnU?=
 =?us-ascii?Q?8iPHhqivzzZCGsOH4Xx+Flvn7B9U4C8EuJ9blLMhGKxWnOTGO7ToZZ5P06nb?=
 =?us-ascii?Q?3mUDPNMm3X3zZBuE3+is3jcG2hUP2xJmOMJx+Q7Zeuc8Tn3is3JOhglHsFKz?=
 =?us-ascii?Q?//srMzx90yNpitIDc+cD22tDTbbY/yvqp31GsTz3fFZiebKUrMk2kYacSqQL?=
 =?us-ascii?Q?15CqwJfIr3mPlZWlkFWXpxhRLc/gYA4XMcDLyMcIQePebT2jpEA+BBmt0JFQ?=
 =?us-ascii?Q?GFh5p0dQsJk3wBOuQBDwEKKgHcdsaOCH9H7/OGjuUrJX/gmPnWQwoxLVUAZI?=
 =?us-ascii?Q?fGrdugIkApxGbsIAOEXEq2fRbzw5DWfZdJEOm4MRYwkmeFNBS6+9ti2wLeZN?=
 =?us-ascii?Q?1zTF/YmiKfzk3DD0OHC1UlJkH/CYw8KTA+4IMqczV84HeZ5ciAKIeJ00kj85?=
 =?us-ascii?Q?ZDb7Ix5fqoKb23Kz8wdhItWJDfIMMVDbko+PfS+nvd235sbxbC/G78xcjXzI?=
 =?us-ascii?Q?1d3FPhn2RSIo+MYnSd0QEFqw20Ps6MckaSaM38UvTIU+J6opOJ23GIRb2GtG?=
 =?us-ascii?Q?OjQKH5GrFlbntQb1PEIa+PLwkkVZdUud7dShz8Yhf9v5ISvdL1qY/OkDUtpn?=
 =?us-ascii?Q?LyP82S+gBkl5qqsbYUb3gzFS7LKEK+J0lXsG07Si?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f6ab09b7-bb4a-4676-4bf1-08dc5917aef3
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2024 04:35:43.5916
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4ioI8cZygcTHwxpHjFoKo7NXn3R4YILPCFAF5SOGKckS+TJHp6OXzQwsq9ZsoIbu9278oKLb+gg9OhQ8apGGDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB8279
X-OriginatorOrg: intel.com

Dave Jiang wrote:
> 
> 
> On 3/24/24 4:18 PM, ira.weiny@intel.com wrote:
> > From: Navneet Singh <navneet.singh@intel.com>
> > 

[snip]

> >  
> > -What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
> > -Date:		May, 2022, January, 2023
> > -KernelVersion:	v6.0 (pmem), v6.3 (ram)
> > +What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram,dc}_region
> > +Date:		May, 2022, January, 2023, June 2024
> > +KernelVersion:	v6.0 (pmem), v6.3 (ram), v6.10 (dc)
> >  Contact:	linux-cxl@vger.kernel.org
> >  Description:
> >  		(RW) Write a string in the form 'regionZ' to start the process
> > -		of defining a new persistent, or volatile memory region
> > -		(interleave-set) within the decode range bounded by root decoder
> > -		'decoderX.Y'. The value written must match the current value
> > -		returned from reading this attribute. An atomic compare exchange
> > -		operation is done on write to assign the requested id to a
> > -		region and allocate the region-id for the next creation attempt.
> > -		EBUSY is returned if the region name written does not match the
> > -		current cached value.
> > +		of defining a new persistent, volatile, or Dynamic Capacity
> > +		(DC) memory region (interleave-set) within the decode range
> > +		bounded by root decoder 'decoderX.Y'. The value written must
> > +		match the current value returned from reading this attribute.
> > +		An atomic compare exchange operation is done on write to assign
> > +		the requested id to a region and allocate the region-id for the
> > +		next creation attempt.  EBUSY is returned if the region name
> 
> -EBUSY?
>

To match the other documentation I would say no.  The other docs show
ENXIO/EBUSY/EINVAL without the negative indicator.


[snip]

> > diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> > index c696837ab23c..415d03fbf9b6 100644
> > --- a/drivers/dax/cxl.c
> > +++ b/drivers/dax/cxl.c
> > @@ -13,19 +13,30 @@ static int cxl_dax_region_probe(struct device *dev)
> >  	struct cxl_region *cxlr = cxlr_dax->cxlr;
> >  	struct dax_region *dax_region;
> >  	struct dev_dax_data data;
> > +	resource_size_t dev_size;
> > +	unsigned long flags;
> >  
> >  	if (nid == NUMA_NO_NODE)
> >  		nid = memory_add_physaddr_to_nid(cxlr_dax->hpa_range.start);
> >  
> > +	flags = IORESOURCE_DAX_KMEM;
> > +	if (cxlr->mode == CXL_REGION_DC)
> > +		flags |= IORESOURCE_DAX_SPARSE_CAP;
> > +
> >  	dax_region = alloc_dax_region(dev, cxlr->id, &cxlr_dax->hpa_range, nid,
> > -				      PMD_SIZE, IORESOURCE_DAX_KMEM);
> > +				      PMD_SIZE, flags);
> >  	if (!dax_region)
> >  		return -ENOMEM;
> >  
> > +	dev_size = range_len(&cxlr_dax->hpa_range);
> > +	/* Add empty seed dax device */
> > +	if (cxlr->mode == CXL_REGION_DC)
> > +		dev_size = 0;
> 
> Nit. Just do if/else so dev_size isn't set twice if mode is DC.

Ok yea.

Ira