linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: "Clément Léger" <clement.leger@bootlin.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peng Fan <peng.fan@nxp.com>, Sudeep Holla <sudeep.holla@arm.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Subject: Re: [PATCH 1/3] regmap: add regmap using ARM SMCCC
Date: Fri, 23 Jul 2021 17:37:59 +0100	[thread overview]
Message-ID: <20210723163759.GI5221@sirena.org.uk> (raw)
In-Reply-To: <20210723175315.3eb149c7@fixe.home>

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

On Fri, Jul 23, 2021 at 05:53:15PM +0200, Clément Léger wrote:
> Mark Brown <broonie@kernel.org> a écrit :

> > I can't see any SMC specification for this interface?  Frankly I have
> > some very substantial concerns about the use case for this over
> > exposing the functionality of whatever device the SMC is gating
> > access to through SMC interfaces specific to that functionality.

> This would require to modify drivers to check if the access should be
> done using SMCs, parse the device tree to find appropriate SMC ids for
> each functionality, add dependencies in KConfig on
> HAVE_ARM_SMCCC_DISCOVERY, and do SMC calls instead of regmap access.
> I'm not saying this is not the way to go but this is clearly more
> intrusive than keeping the existing syscon support.

You're not doing this at the syscon level, you're doing this at the
regmap level.  Any user of this code is going to have to be modified to
use the SMCCC regmap and discover the relevant SMCCC interfaces no
matter what, but by having it we're saying that that's a sensible and
reasonable thing to do and encouraging implementations as a result.

Device specific regmap interfaces do not require adding anything to the
core, there's the reg_read() and reg_write() callbacks for this, if
there is a sensible use case for this at the syscon level and only the
syscon level (but I really do strongly question if it's a good idea at
all) then you can use those without adding a generic interface for
defining SMCCC conduits as regmaps.  TBH what's being added to the
regmap core is so trival that I don't see what we'd be gaining anyway
even if this was widely used, it's not helping with the SMCCC
enumeration side at all.

> > Exposing raw access to a (presumed?) subset of whatever device
> > functionality feels like the wrong abstraction level to be working at
> > and like an invitation to system integrators to do things that are
> > going to get them into trouble down the line.

> Indeed, access is reduced to a subset of registers offset which are
> checked by the TEE.

I really think it would be clearer and safer to have the TEE expose
specific operations that encode the intent of whatever it is trying to
accomplish rather than just expose the register map and then audit the
operations that are going on in the register map after the fact.  It
seems like it's going to be more error prone to do things this way,
especially as this starts getting used as a generic pipe for exposing
things and things get built up - as well as auditing concerns if any
problems are identified it's going to be harder to track the intent of
what the non-secure world is doing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2021-07-23 16:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23 13:52 [PATCH 0/3] add SMC based regmap driver for secure syscon access Clément Léger
2021-07-23 13:52 ` [PATCH 1/3] regmap: add regmap using ARM SMCCC Clément Léger
2021-07-23 14:43   ` Mark Brown
2021-07-23 15:53     ` Clément Léger
2021-07-23 16:37       ` Mark Brown [this message]
2021-07-23 13:52 ` [PATCH 2/3] syscon: add support for "syscon-smc" compatible Clément Léger
2021-07-23 15:27   ` Lee Jones
2021-07-23 15:56     ` Clément Léger
2021-07-23 16:07   ` Arnd Bergmann
2021-07-23 16:41     ` Mark Brown
2021-07-24 12:36     ` Peng Fan (OSS)
2021-07-24  7:00   ` kernel test robot
2021-07-23 13:52 ` [PATCH 3/3] dt-bindings: mfd: add "syscon-smc" YAML description Clément Léger
2021-07-29 21:19   ` Rob Herring
2021-07-30  7:21     ` Clément Léger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210723163759.GI5221@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=arnd@arndb.de \
    --cc=clement.leger@bootlin.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peng.fan@nxp.com \
    --cc=rafael@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=sudeep.holla@arm.com \
    --subject='Re: [PATCH 1/3] regmap: add regmap using ARM SMCCC' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).