Hello arm-soc maintainers,
Please pull these tee driver changes. This implements support for dynamic
shared memory support in OP-TEE. More specifically is enables mapping of
user space memory in secure world to be used as shared memory.
This has been reviewed and refined by the OP-TEE community at various
places on Github during the last year. An earlier version of this pull
request is used in the latest OP-TEE release (2.6.0). This has also been
reviewed recently at the kernel mailing lists, with all comments from
Mark Rutland <[email protected]> and Yury Norov
<[email protected]> addressed as far as I can tell.
This isn't a bugfix so I'm aiming for the next merge window.
Thanks,
Jens
The following changes since commit 50c4c4e268a2d7a3e58ebb698ac74da0de40ae36:
Linux 4.15-rc3 (2017-12-10 17:56:26 -0800)
are available in the git repository at:
https://git.linaro.org/people/jens.wiklander/linux-tee.git tags/tee-drv-dynamic-shm-for-v4.16
for you to fetch changes up to ef8e08d24ca84846ce639b835ebd2f15a943f42b:
tee: shm: inline tee_shm_get_id() (2017-12-15 13:36:21 +0100)
----------------------------------------------------------------
This pull request enables dynamic shared memory support in the TEE
subsystem as a whole and in OP-TEE in particular.
Global Platform TEE specification [1] allows client applications
to register part of own memory as a shared buffer between
application and TEE. This allows fast zero-copy communication between
TEE and REE. But current implementation of TEE in Linux does not support
this feature.
Also, current implementation of OP-TEE transport uses fixed size
pre-shared buffer for all communications with OP-TEE OS. This is okay
in the most use cases. But this prevents use of OP-TEE in virtualized
environments, because:
a) We can't share the same buffer between different virtual machines
b) Physically contiguous memory as seen by VM can be non-contiguous
in reality (and as seen by OP-TEE OS) due to second stage of
MMU translation.
c) Size of this pre-shared buffer is limited.
So, first part of this pull request adds generic register/unregister
interface to tee subsystem. The second part adds necessary features into
OP-TEE driver, so it can use not only static pre-shared buffer, but
whole RAM to communicate with OP-TEE OS.
This change is backwards compatible allowing older secure world or
user space to work with newer kernels and vice versa.
[1] https://www.globalplatform.org/specificationsdevice.asp
----------------------------------------------------------------
Jens Wiklander (2):
tee: flexible shared memory pool creation
tee: add register user memory
Volodymyr Babchuk (12):
tee: shm: add accessors for buffer size and page offset
tee: shm: add page accessor functions
tee: optee: Update protocol definitions
tee: optee: add page list manipulation functions
tee: optee: add shared buffer registration functions
tee: optee: add registered shared parameters handling
tee: optee: add registered buffers handling into RPC calls
tee: optee: store OP-TEE capabilities in private data
tee: optee: add optee-specific shared pool implementation
tee: optee: enable dynamic SHM support
tee: use reference counting for tee_context
tee: shm: inline tee_shm_get_id()
drivers/tee/optee/Makefile | 1 +
drivers/tee/optee/call.c | 179 +++++++++++++++++++++++++++++-
drivers/tee/optee/core.c | 152 +++++++++++++++++++------
drivers/tee/optee/optee_msg.h | 38 ++++++-
drivers/tee/optee/optee_private.h | 27 ++++-
drivers/tee/optee/optee_smc.h | 7 ++
drivers/tee/optee/rpc.c | 77 +++++++++++--
drivers/tee/optee/shm_pool.c | 75 +++++++++++++
drivers/tee/optee/shm_pool.h | 23 ++++
drivers/tee/tee_core.c | 81 ++++++++++++--
drivers/tee/tee_private.h | 60 +---------
drivers/tee/tee_shm.c | 228 +++++++++++++++++++++++++++++++-------
drivers/tee/tee_shm_pool.c | 165 ++++++++++++++++-----------
include/linux/tee_drv.h | 183 +++++++++++++++++++++++++++++-
include/uapi/linux/tee.h | 30 +++++
15 files changed, 1105 insertions(+), 221 deletions(-)
create mode 100644 drivers/tee/optee/shm_pool.c
create mode 100644 drivers/tee/optee/shm_pool.h
On Fri, Dec 15, 2017 at 2:21 PM, Jens Wiklander
<[email protected]> wrote:
> Hello arm-soc maintainers,
>
> Please pull these tee driver changes. This implements support for dynamic
> shared memory support in OP-TEE. More specifically is enables mapping of
> user space memory in secure world to be used as shared memory.
>
> This has been reviewed and refined by the OP-TEE community at various
> places on Github during the last year. An earlier version of this pull
> request is used in the latest OP-TEE release (2.6.0). This has also been
> reviewed recently at the kernel mailing lists, with all comments from
> Mark Rutland <[email protected]> and Yury Norov
> <[email protected]> addressed as far as I can tell.
>
> This isn't a bugfix so I'm aiming for the next merge window.
Given that Mark and Yury reviewed this, I'm assuming this is all
good and have now merged it. However I missed the entire discussion
about it, so I have one question about the implementation:
What happens when user space passes a buffer that is not
backed by regular memory but instead is something it has itself
mapped from a device with special page attributes or physical
properties? Could this be inconsistent when optee and user
space disagree on the caching attributes? Can you get into
trouble if you pass an area from a device that is read-only
in user space but writable from secure world?
Arnd
On 2017年12月21日 08:30, Arnd Bergmann wrote:
> On Fri, Dec 15, 2017 at 2:21 PM, Jens Wiklander
> <[email protected]> wrote:
>> Hello arm-soc maintainers,
>>
>> Please pull these tee driver changes. This implements support for dynamic
>> shared memory support in OP-TEE. More specifically is enables mapping of
>> user space memory in secure world to be used as shared memory.
>>
>> This has been reviewed and refined by the OP-TEE community at various
>> places on Github during the last year. An earlier version of this pull
>> request is used in the latest OP-TEE release (2.6.0). This has also been
>> reviewed recently at the kernel mailing lists, with all comments from
>> Mark Rutland <[email protected]> and Yury Norov
>> <[email protected]> addressed as far as I can tell.
>>
>> This isn't a bugfix so I'm aiming for the next merge window.
> Given that Mark and Yury reviewed this, I'm assuming this is all
> good and have now merged it. However I missed the entire discussion
> about it, so I have one question about the implementation:
>
> What happens when user space passes a buffer that is not
> backed by regular memory but instead is something it has itself
> mapped from a device with special page attributes or physical
> properties? Could this be inconsistent when optee and user
> space disagree on the caching attributes? Can you get into
> trouble if you pass an area from a device that is read-only
> in user space but writable from secure world?
Just recently, we have started to kick the tires of these "shm" related
Gen Tee Driver patches. And we have in the past encountered real world
scenarios requiring some of the shared memory regions to be marked as
"normal IC=0 and OC=0" in EL2 or SEL1, or else HW would misbehave. We
worked around by hacking the boot code but that works if the regions are
pre-allocated. Since now these regions can also be managed dynamically,
we definitely agree with Arnd Bergmann that the dynamic registration SMC
commands, and potention the SHM IOCTL commands, must convey cache
intentions. Is it possible to take this requirement into consideration,
in this iteration or the follow on?
>
> Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Dec 25, 2017 at 01:22:18PM -0800, thomas zeng wrote:
>
>
> On 2017年12月21日 08:30, Arnd Bergmann wrote:
> > On Fri, Dec 15, 2017 at 2:21 PM, Jens Wiklander
> > <[email protected]> wrote:
> > > Hello arm-soc maintainers,
> > >
> > > Please pull these tee driver changes. This implements support for dynamic
> > > shared memory support in OP-TEE. More specifically is enables mapping of
> > > user space memory in secure world to be used as shared memory.
> > >
> > > This has been reviewed and refined by the OP-TEE community at various
> > > places on Github during the last year. An earlier version of this pull
> > > request is used in the latest OP-TEE release (2.6.0). This has also been
> > > reviewed recently at the kernel mailing lists, with all comments from
> > > Mark Rutland <[email protected]> and Yury Norov
> > > <[email protected]> addressed as far as I can tell.
> > >
> > > This isn't a bugfix so I'm aiming for the next merge window.
> > Given that Mark and Yury reviewed this, I'm assuming this is all
> > good and have now merged it. However I missed the entire discussion
> > about it, so I have one question about the implementation:
> >
> > What happens when user space passes a buffer that is not
> > backed by regular memory but instead is something it has itself
> > mapped from a device with special page attributes or physical
> > properties? Could this be inconsistent when optee and user
> > space disagree on the caching attributes? Can you get into
> > trouble if you pass an area from a device that is read-only
> > in user space but writable from secure world?
Read-only memory is dealt with by calling get_user_pages_fast() with
the 'write' parameter set to 1.
Mismatch in cache attributes isn't addressed though. This is something
that should be checked in the OP-TEE driver, typically
drivers/tee/optee/core.c.
I would like to add another patch on top of this patch series to guard
against cache attributes which aren't normal cached memory. So far I
haven't been able to find a nice way of doing that, I'd appreciate any
advice of idea of how to deal with this.
>
> Just recently, we have started to kick the tires of these "shm" related Gen
> Tee Driver patches. And we have in the past encountered real world
> scenarios requiring some of the shared memory regions to be marked as
> "normal IC=0 and OC=0" in EL2 or SEL1, or else HW would misbehave. We worked
> around by hacking the boot code but that works if the regions are
> pre-allocated. Since now these regions can also be managed dynamically, we
> definitely agree with Arnd Bergmann that the dynamic registration SMC
> commands, and potention the SHM IOCTL commands, must convey cache
> intentions. Is it possible to take this requirement into consideration, in
> this iteration or the follow on?
I'd be happy to discuss using different cache attributes outside this
patch series. We have so far avoided specifying cache attributes by
calling it normal cached memory. Now that we have one use case we're
able take the next step here.
Thanks,
Jens