linux-kernel - Re: [PATCH v2 2/3] soc: qcom: socinfo: Add support for new fields in revision 21

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: <aB3TkK7wEjdxSSvQ@hu-mojha-hyd.qualcomm.com>
Date: Fri, 9 May 2025 15:36:08 +0530
From: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] soc: qcom: socinfo: Add support for new fields in
 revision 21

On Thu, May 08, 2025 at 08:01:44PM +0200, Konrad Dybcio wrote:
> On 5/8/25 6:48 PM, Mukesh Ojha wrote:
> > On Thu, May 08, 2025 at 06:56:47PM +0300, Dmitry Baryshkov wrote:
> >> On Thu, May 08, 2025 at 09:07:03PM +0530, Mukesh Ojha wrote:
> >>> On Fri, Apr 25, 2025 at 08:28:51PM +0300, Dmitry Baryshkov wrote:
> >>>> On Fri, Apr 25, 2025 at 07:29:45PM +0530, Mukesh Ojha wrote:
> >>>>> Add the subpartfeature offset field to the socinfo structure
> >>>>> which came for version 21 of socinfo structure.
> >>>>>
> >>>>> Subpart_feat_offset is subpart like camera, display, etc.,
> >>>>> and its internal feature available on a bin.
> >>>>>
> >>>>> Signed-off-by: Mukesh Ojha <mukesh.ojha@....qualcomm.com>
> >>>>> ---
> >>>>> Changes in v2:
> >>>>>  - Added debugfs entry and described more about the field in commit.
> >>>>>
> >>>>>  drivers/soc/qcom/socinfo.c       | 6 ++++++
> >>>>>  include/linux/soc/qcom/socinfo.h | 2 ++
> >>>>>  2 files changed, 8 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> >>>>> index 5800ebf9ceea..bac1485f1b27 100644
> >>>>> --- a/drivers/soc/qcom/socinfo.c
> >>>>> +++ b/drivers/soc/qcom/socinfo.c
> >>>>> @@ -154,6 +154,7 @@ struct socinfo_params {
> >>>>>  	u32 boot_cluster;
> >>>>>  	u32 boot_core;
> >>>>>  	u32 raw_package_type;
> >>>>> +	u32 nsubpart_feat_array_offset;
> >>>>>  };
> >>>>>  
> >>>>>  struct smem_image_version {
> >>>>> @@ -608,6 +609,11 @@ static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo,
> >>>>>  			   &qcom_socinfo->info.fmt);
> >>>>>  
> >>>>>  	switch (qcom_socinfo->info.fmt) {
> >>>>> +	case SOCINFO_VERSION(0, 21):
> >>>>> +		qcom_socinfo->info.nsubpart_feat_array_offset =
> >>>>> +				   __le32_to_cpu(info->nsubpart_feat_array_offset);
> >>>>> +		debugfs_create_u32("nsubpart_feat_array_offset", 0444, qcom_socinfo->dbg_root,
> >>>>> +				   &qcom_socinfo->info.nsubpart_feat_array_offset);
> >>>>
> >>>> An offset into what? If this provides additional data, then the data
> >>>> should be visible in the debugfs. Not sure, what's the point in dumping
> >>>> the offset here.
> >>>
> >>> offset into info(struct socinfo) object.
> >>>
> >>> I agree to you and I said the same in first version this is just offset
> >>> and does not provide any debug info we would look from userspace.  For
> >>> parity with other fields I did it for all newly added fields.
> >>> I have dropped it in latest patch.
> >>
> >> I'd rather see the decoded structure that is being pointed by this
> >> offset.
> > 
> > You mean info + info->nsubpart_feat_array_offset ? 
> > 
> > There is more to it which I don't want to mention as they are not
> > upstreamed yet and unrelated to this change.
> > 
> > data = info + (offset + (part * sizeof(u32)));
> > 
> > e.g., Here, part is a enum represents camera, display etc., and data
> > represents their feature presents. Since, part is not upstream yet I
> > don't feel we should expose this information to debugfs. We could always
> > add them in debugfs when such things are standardized and upstreamed.
> 
> That's what Dmitry's saying - just add support for them

We definitely add support for this in the future.  In the meantime, does
the absence of the support prevent this socinfo field from being merged?
Without it, there could be inconsistencies between the boot firmware and
Linux for the SM8750 platform.

-Mukesh

> 
> Konrad