Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 27 Apr 2022 18:55:02 +0100
From:   Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To:     <ira.weiny@intel.com>
CC:     Dan Williams <dan.j.williams@intel.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Alison Schofield <alison.schofield@intel.com>,
        "Vishal Verma" <vishal.l.verma@intel.com>,
        Ben Widawsky <ben.widawsky@intel.com>,
        <linux-kernel@vger.kernel.org>, <linux-cxl@vger.kernel.org>,
        <linux-pci@vger.kernel.org>
Subject: Re: [PATCH V8 07/10] cxl/mem: Read CDAT table
Message-ID: <20220427185502.00001fbf@Huawei.com>
In-Reply-To: <20220414203237.2198665-8-ira.weiny@intel.com>
References: <20220414203237.2198665-1-ira.weiny@intel.com>
        <20220414203237.2198665-8-ira.weiny@intel.com>
Organization: Huawei Technologies Research and Development (UK) Ltd.
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 14 Apr 2022 13:32:34 -0700
ira.weiny@intel.com wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> The OS will need CDAT data from CXL devices to properly set up
> interleave sets.  Currently this is supported by a through a DOE mailbox
> which supports CDAT.  But any cxl_mem object can provide this data later
> if need be, for example for testing.
> 
> Cache the CDAT data for later parsing.  Provide a sysfs binary attribute
> to allow dumping of the CDAT.
> 
> Binary dumping is modeled on /sys/firmware/ACPI/tables/
> 
> The ability to dump this table will be very useful for emulation of real
> devices once they become available as QEMU CXL type 3 device emulation will
> be able to load this file in.
> 
> This does not support table updates at runtime. It will always provide
> whatever was there when first cached. Handling of table updates can be
> implemented later.
> 
> Finally create a complete list of DOE defines within cdat.h for code
> wishing to decode the CDAT table.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Co-developed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
I'd have left the introduction of callbacks until they were actually needed,
but meh. That's minor.

On trivial below.

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

...


> +
> +static int read_cdat_data(struct cxl_dev_state *cxlds)
> +{
> +	struct device *dev = cxlds->dev;
> +	size_t cdat_length;
> +	int ret;
> +
> +	if (cxl_mem_cdat_get_length(cxlds, &cdat_length))
> +		return 0;
> +
> +	cxlds->cdat.table = devm_kzalloc(dev, cdat_length, GFP_KERNEL);
> +	if (!cxlds->cdat.table)
> +		return -ENOMEM;

Trivial but blank line here.

> +	cxlds->cdat.length = cdat_length;
> +	ret = cxl_mem_cdat_read_table(cxlds, &cxlds->cdat);
> +	if (ret) {
> +		devm_kfree(dev, cxlds->cdat.table);
> +		cxlds->cdat.table = NULL;
> +		cxlds->cdat.length = 0;
> +	}
> +	return ret;
> +}