linux-kernel - Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

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]

Date:	Fri, 14 Nov 2014 09:11:15 +0200
From:	Ohad Ben-Cohen <ohad@...ery.com>
To:	Suman Anna <s-anna@...com>
Cc:	Mark Rutland <mark.rutland@....com>,
	Kumar Gala <galak@...eaurora.org>,
	Tony Lindgren <tony@...mide.com>,
	Josh Cartwright <joshc@...eaurora.org>,
	Bjorn Andersson <bjorn@...o.se>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>,
	linux-arm <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCHv6 4/5] hwspinlock/core: add common OF helpers

Hi Suman,

On Thu, Nov 13, 2014 at 11:02 PM, Suman Anna <s-anna@...com> wrote:
> OK, lets take an example. I have say 2 device instances, say
>         hwlock1: hwlock@0 {
>                 hwlock-num-locks = <32>
>                 hwlock-base-id = <0>;
>                 #hwlock-cells = <1>;
>         };
>         hwlock2: hwlock@0 {
>                 hwlock-num-locks = <32>
>                 hwlock-base-id = <32>;
>                 #hwlock-cells = <1>;
>         };
>
>         some-client {
>                 hwlocks = <&hwlock1 32>, <&hwlock2 0>;
>         };
>
> The first args value is incorrect, and I expect the API to return an
> error. I don't think the API can make assumptions that all passed in
> values will be correct. What do you suggest that the API do here?

I think this is just one example of many potential mistakes the DT
developer can have, and that there's nothing really special about it.

What if the '5' digit below is a typo, and the actual hardware
assignment was supposed to be '4' ?

         some-client {
                 hwlocks = <&hwlock1 5>, <&hwlock2 5>;
         };

I don't see how much different this mistake is from the one you
mentioned above. There's a limit to how much we can really catch DT
mistakes in the code, just like we couldn't really catch past mistakes
in the platform data structures.

If we can easily add some validations then great, but if we have to go
to great lengths just to gain very limited validations, then I'd vote
against doing so.

> One solution to handle #1 is to also make the hwlock-num-locks property
> also mandatory (even though it could have been read from a register

Yeah, this is awkward. We shouldn't impose this on implementations
like OMAP which don't need it.

Again I think for the very limited validation this buys us - it's not worth it.

> And, how do you propose we solve the problem of deferred probing?

It seems to me that hwspin_lock_request_specific failures should be
used by clients to defer their probing. Why wouldn't such a simple
solution work?

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/