linux-kernel - Re: [PATCH v8 1/2] soc: qcom: Add SoC info driver

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] [day] [month] [year] [list]

Message-ID: <20170216043238.GB23255@minitux>
Date:   Wed, 15 Feb 2017 20:32:38 -0800
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Imran Khan <kimran@...eaurora.org>
Cc:     andy.gross@...aro.org, david.brown@...aro.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-soc@...r.kernel.org
Subject: Re: [PATCH v8 1/2] soc: qcom: Add SoC info driver

On Tue 14 Feb 21:31 PST 2017, Imran Khan wrote:

> On 2/15/2017 5:54 AM, Bjorn Andersson wrote:
[..]
> > 
> > I thought the conclusion was to drop the "hw_platform",
> > "qrd_hw_platform_subtype" and hw_platform_subtype" lists, but to keep
> > the cpu_of_id (although named soc_of_id).
> > 
> > The hw_platform ids are re-used by ODMs and might have different
> > meaning, but the SoC name is quite useful. So please put the soc_of_id
> > back in here.
> > 
> 
> cpu_of_id was mapping soc-id read from SMEM into SoC name, which in turn
> was being used as machine name in soc_device_attribute. But we can also
> read machine name from DT. Reading the machine name from DT would make the 
> solution more flexible as we don't have to make an entry in soc_of_id every
> time we get a new SoC.

I don't think we need a more flexible solution, as the purpose of this
driver is to expose the socinfo information to user space; i.e. there's
nothing flexible about that.

If we just expose this information as a proprietary magic number, then
it will be up to each consumer to keep some mapping table. So you will
not get rid of the work of keeping things updated, you will just push
the problem to somewhere else.

> Also as soc_of_id uses soc-id as index, there is theoretically no limit on how
> many elements it may end up having.

There's a practical - and reasonable - limit on how much space this data
will take, one could however argue that it's unwise to keep the sparse
data in a simple array. A better solution would be to maintain an array
of id,soc pairs and loop through that on lookup.

> Because of the above reasons, I wanted to get rid off soc_of_id (or cpu_of_id)
> array and use DT to get machine name as shown in the following snippet:
> 
> +	attr->family = "Snapdragon";
> +	prop = fdt_get_property(initial_boot_params, 0, "model", NULL);
> +	if (prop)
> +		attr->machine = kasprintf(GFP_KERNEL, "%s", prop->data);

The data is already in socinfo, we should not manually duplicate it in
DT.

> 
> Could you please let me know if this approach looks fine to you?
> 

I would prefer that the socinfo doesn't expose magic numbers without a
publicly available lookup table. And I don't see a need for the added
flexibility.

Regards,
Bjorn