linux-kernel - Re: [PATCH 07/10] firmware: arm_scmi: Add System Telemetry ioctls support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aPdguGJ-fZFF7L3Y@pluto>
Date: Tue, 21 Oct 2025 11:30:16 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: Cristian Marussi <cristian.marussi@....com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	arm-scmi@...r.kernel.org, sudeep.holla@....com,
	james.quinlan@...adcom.com, f.fainelli@...il.com,
	vincent.guittot@...aro.org, etienne.carriere@...com,
	peng.fan@....nxp.com, michal.simek@....com, quic_sibis@...cinc.com,
	dan.carpenter@...aro.org, d-gole@...com, souvik.chakravarty@....com
Subject: Re: [PATCH 07/10] firmware: arm_scmi: Add System Telemetry ioctls
 support

On Mon, Oct 20, 2025 at 05:30:34PM +0100, Jonathan Cameron wrote:
> On Thu, 25 Sep 2025 21:35:51 +0100
> Cristian Marussi <cristian.marussi@....com> wrote:
> 
> > Extend the filesystem based interface with special 'control' file that can
> > be used to configure and retrieve SCMI Telemetry data in binary form using
> > the alternative ioctls-based ABI described in uapi/linux/scmi.h.
> Why you say alternative.  Why do you need both?
> 
> That's in the cover letter but I'd put something here as well.

Ok..indeed all my babbling in teh cover wont be anywhere once merged.

> > 
> > Signed-off-by: Cristian Marussi <cristian.marussi@....com>
> > ---
> >  .../firmware/arm_scmi/scmi_system_telemetry.c | 402 ++++++++++++++++++
> >  1 file changed, 402 insertions(+)
> > 
> > diff --git a/drivers/firmware/arm_scmi/scmi_system_telemetry.c b/drivers/firmware/arm_scmi/scmi_system_telemetry.c
> > index 2fec465b0f33..f591aad10302 100644
> > --- a/drivers/firmware/arm_scmi/scmi_system_telemetry.c
> > +++ b/drivers/firmware/arm_scmi/scmi_system_telemetry.c
> 
> > +static long scmi_tlm_des_read_ioctl(const struct scmi_tlm_inode *tlmi,
> > +				    unsigned long arg, bool single,
> > +				    bool is_group)
> > +{
> > +	const struct scmi_tlm_setup *tsp = tlmi->tsp;
> > +	void * __user uptr = (void * __user)arg;
> > +	struct scmi_tlm_data_read bulk, *bulk_ptr;
> > +	int ret, grp_id = SCMI_TLM_GRP_INVALID;
> > +
> > +	if (copy_from_user(&bulk, uptr, sizeof(bulk)))
> > +		return -EFAULT;
> > +
> > +	bulk_ptr = kzalloc(struct_size(bulk_ptr, samples, bulk.num_samples),
> 
> __free() would help here.
> 

Ok.

> > +			   GFP_KERNEL);
> > +	if (!bulk_ptr)
> > +		return -ENOMEM;
> > +
> > +	if (is_group) {
> > +		const struct scmi_telemetry_group *grp = tlmi->priv;
> > +
> > +		grp_id = grp->info->id;
> > +	}
> > +
> > +	bulk_ptr->num_samples = bulk.num_samples;
> > +	if (!single)
> > +		ret = tsp->ops->des_bulk_read(tsp->ph, grp_id,
> > +					      &bulk_ptr->num_samples,
> > +			  (struct scmi_telemetry_de_sample *)bulk_ptr->samples);
> > +	else
> > +		ret = tsp->ops->des_sample_get(tsp->ph, grp_id,
> > +					       &bulk_ptr->num_samples,
> > +			  (struct scmi_telemetry_de_sample *)bulk_ptr->samples);
> 
> That is very unusual code alignment.  Drag 2 lines above left to match one line above.

Trying to please checkpatch while staying under 80 cols...BUT I know is NOT a
good reason for the above mess...

Thanks,
Cristian