linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Clément Léger" <clement.leger@bootlin.com>
To: Mark Brown <broonie@kernel.org>
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:53:15 +0200	[thread overview]
Message-ID: <20210723175315.3eb149c7@fixe.home> (raw)
In-Reply-To: <20210723144317.GF5221@sirena.org.uk>

Hi Mark,

Le Fri, 23 Jul 2021 15:43:18 +0100,
Mark Brown <broonie@kernel.org> a écrit :

> On Fri, Jul 23, 2021 at 03:52:37PM +0200, Clément Léger wrote:
> 
> > When running under secure monitor control, some controllers can be
> > placed in secure world and their access is thus not possible from
> > normal world. However, these controllers frequently contain
> > registers than are needed by the normal world for a few specific
> > operations.  
> 
> > This patch adds a regmap where registers are accessed using SMCs.
> > The secure monitor is then responsible to allow or deny access to
> > the requested registers.  
> 
> 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.

> 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.

> 
> If the end user really is just twiddling a few bits here and there I'd
> expect those functionality specific services to be pretty simple to
> do, slightly more effort on the secure monitor side but a lot safer.

The SMC id is supposed to be unique for a given device. The TEE check is
merely a register offset check and a value check. But I agree that the
attack surface is larger than with a SMC targeted for a single
functionality though.

> If there is a use case for passing through an entire device for some
> reason (ran out of controllers or something?) then I think we
> probably want an abstraction at the bus level so we don't need to add
> custom support to every device that we want to pass through and it's
> clear what's going on.

In our use case, only a few registers located in a secure controller
is needed to be done. We don't have a use case for an entire device
access.

Clément


  reply	other threads:[~2021-07-23 15:53 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 [this message]
2021-07-23 16:37       ` Mark Brown
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=20210723175315.3eb149c7@fixe.home \
    --to=clement.leger@bootlin.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).