| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b361d492-1cc1-4057-8114-851240dd64d6@intel.com>
Date: Tue, 4 Nov 2025 16:02:04 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Robert Richter <rrichter@....com>,
Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
Dan Williams <dan.j.williams@...el.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Davidlohr Bueso <dave@...olabs.net>
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
Gregory Price <gourry@...rry.net>,
"Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>,
Terry Bowman <terry.bowman@....com>, Joshua Hahn <joshua.hahnjy@...il.com>
Subject: Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and
handling
On 11/3/25 2:53 PM, Dave Jiang wrote:
>
>
> On 11/3/25 11:47 AM, Robert Richter wrote:
>> A root port's callback handlers are collected in struct cxl_root_ops.
>> The structure is dynamically allocated, though it contains only a
>> single pointer in it. This also requires to check two pointers to
>> check for the existance of a callback.
>>
>> Simplify the allocation, release and handler check by embedding the
>> ops statical in struct cxl_root.
>>
>> Signed-off-by: Robert Richter <rrichter@....com>
>
> Reviewed-by: Dave Jiang <dave.jiang@...el.com>
Thought about it for a bit, should the callback be with 'cxl_rd_ops' and the rest of the translation functions under the root decoder rather than cxl_root_ops with the cxl root? That seems to be the better fit.
DJ
>> drivers/cxl/acpi.c | 7 ++-----
>> drivers/cxl/core/cdat.c | 8 ++++----
>> drivers/cxl/core/port.c | 9 ++-------
>> drivers/cxl/cxl.h | 19 ++++++++++---------
>> 4 files changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index bd2e282ca93a..1ab780edf141 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -299,10 +299,6 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
>> return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
>> }
>>
>> -static const struct cxl_root_ops acpi_root_ops = {
>> - .qos_class = cxl_acpi_qos_class,
>> -};
>> -
>> static void del_cxl_resource(struct resource *res)
>> {
>> if (!res)
>> @@ -914,9 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>> cxl_res->end = -1;
>> cxl_res->flags = IORESOURCE_MEM;
>>
>> - cxl_root = devm_cxl_add_root(host, &acpi_root_ops);
>> + cxl_root = devm_cxl_add_root(host);
>> if (IS_ERR(cxl_root))
>> return PTR_ERR(cxl_root);
>> + cxl_root->ops.qos_class = cxl_acpi_qos_class;
>> root_port = &cxl_root->port;
>>
>> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
>> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
>> index c4bd6e8a0cf0..b84a9b52942c 100644
>> --- a/drivers/cxl/core/cdat.c
>> +++ b/drivers/cxl/core/cdat.c
>> @@ -213,7 +213,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>> if (!cxl_root)
>> return -ENODEV;
>>
>> - if (!cxl_root->ops || !cxl_root->ops->qos_class)
>> + if (!cxl_root->ops.qos_class)
>> return -EOPNOTSUPP;
>>
>> xa_for_each(dsmas_xa, index, dent) {
>> @@ -221,9 +221,9 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>>
>> cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
>> dent->entries = 1;
>> - rc = cxl_root->ops->qos_class(cxl_root,
>> - &dent->coord[ACCESS_COORDINATE_CPU],
>> - 1, &qos_class);
>> + rc = cxl_root->ops.qos_class(cxl_root,
>> + &dent->coord[ACCESS_COORDINATE_CPU],
>> + 1, &qos_class);
>> if (rc != 1)
>> continue;
>>
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 8128fd2b5b31..2338d146577c 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -459,7 +459,6 @@ static void cxl_root_decoder_release(struct device *dev)
>> if (atomic_read(&cxlrd->region_id) >= 0)
>> memregion_free(atomic_read(&cxlrd->region_id));
>> __cxl_decoder_release(&cxlrd->cxlsd.cxld);
>> - kfree(cxlrd->ops);
>> kfree(cxlrd);
>> }
>>
>> @@ -955,19 +954,15 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>> }
>> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_port, "CXL");
>>
>> -struct cxl_root *devm_cxl_add_root(struct device *host,
>> - const struct cxl_root_ops *ops)
>> +struct cxl_root *devm_cxl_add_root(struct device *host)
>> {
>> - struct cxl_root *cxl_root;
>> struct cxl_port *port;
>>
>> port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
>> if (IS_ERR(port))
>> return ERR_CAST(port);
>>
>> - cxl_root = to_cxl_root(port);
>> - cxl_root->ops = ops;
>> - return cxl_root;
>> + return to_cxl_root(port);
>> }
>> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_root, "CXL");
>>
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index b57cfa4273b9..9a381c827416 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -638,6 +638,14 @@ struct cxl_port {
>> resource_size_t component_reg_phys;
>> };
>>
>> +struct cxl_root;
>> +
>> +struct cxl_root_ops {
>> + int (*qos_class)(struct cxl_root *cxl_root,
>> + struct access_coordinate *coord, int entries,
>> + int *qos_class);
>> +};
>> +
>> /**
>> * struct cxl_root - logical collection of root cxl_port items
>> *
>> @@ -646,7 +654,7 @@ struct cxl_port {
>> */
>> struct cxl_root {
>> struct cxl_port port;
>> - const struct cxl_root_ops *ops;
>> + struct cxl_root_ops ops;
>> };
>>
>> static inline struct cxl_root *
>> @@ -655,12 +663,6 @@ to_cxl_root(const struct cxl_port *port)
>> return container_of(port, struct cxl_root, port);
>> }
>>
>> -struct cxl_root_ops {
>> - int (*qos_class)(struct cxl_root *cxl_root,
>> - struct access_coordinate *coord, int entries,
>> - int *qos_class);
>> -};
>> -
>> static inline struct cxl_dport *
>> cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>> {
>> @@ -755,8 +757,7 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>> struct device *uport_dev,
>> resource_size_t component_reg_phys,
>> struct cxl_dport *parent_dport);
>> -struct cxl_root *devm_cxl_add_root(struct device *host,
>> - const struct cxl_root_ops *ops);
>> +struct cxl_root *devm_cxl_add_root(struct device *host);
>> struct cxl_root *find_cxl_root(struct cxl_port *port);
>>
>> DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
>
>
Powered by blists - more mailing lists