[PATCH] drivers: tee: i2c trampoline driver

[Jorge Ramirez-Ortiz, Foundries]

On 29/12/20, Simon Glass wrote:
> Hi Jorge,
> 
> On Tue, 29 Dec 2020 at 01:30, Jorge Ramirez-Ortiz, Foundries
> <jorge at foundries.io> wrote:
> >
> > On 28/12/20, Simon Glass wrote:
> > > Hi Jorge,
> > >
> > > On Mon, 21 Dec 2020 at 11:15, Jorge Ramirez-Ortiz <jorge at foundries.io> wrote:
> > > >
> > > > This commit gives the secure world access to the I2C bus so it can
> > > > communicate with I2C slaves (tipically those would be secure elements
> > >
> > > typically
> >
> > ok
> >
> > >
> > > > like the NXP SE050).
> > > >
> > > > Tested on imx8mmevk.
> > >
> > > We don't seem to have any optee tests in U-Boot at present. I vaguely
> > > recall they were coming at some point. I think we need:
> > >
> > > - a sandbox fake drive for optee, that understands and responds to the
> > > 6 uclass calls at a basic level
> > > - an update to get_invoke_func() that provides a sandbox function too
> > >
> > > Then we should be able to run optee tests in CI.
> > >
> > > It is not a lot of work, but I don't think we should add to optee
> > > until this is resolved.
> >
> > um, ok but shouldnt this infrastructure better rest on a maintainer's
> > roadmap rather than on an off-the-blue request? I mean, had I known I
> > could have done it in parallel but now I'll need to find the time to
> > do this.
> 
> We always need tests in U-Boot, so if you are not writing a test it
> would be a good question to ask as to how you can do that. Actually
> patman sometimes warns about that, but perhaps only in certain
> situations.
> 
> Actually I see that there is a test - it is hidden under the generic
> unit tests so I didn't see it. See dm/test/tee.c
> 
> I'll make some comments on the patch.
> 
> >
> > also notice that Linux's equivalent patchset was merged back in the
> > summer (ie, this is not untested code).
> >
> > https://lkml.org/lkml/2020/8/12/276
> 
> I don't see any tests in that patch though...are they somewhere
else?

hey

no you are right, I didnt post any tests to linux. And the more I
think about it the less convinced I am that we needed one.

This commit is nothing more than a RPC TEE service that shares a
buffer with U-boot and then sends requests to an I2C chip.

> Or do you justmean people have been running similar code?

I am not sure many people is using it yet other than a number of our
customers at Foundries.io across different projects. But the number of
users is at a steady growth (everybody seems to need this
functionality).

In our use case this trampoline code is used to access the NXP SE050
from OP-TEE over I2C via:
https://github.com/foundriesio/plug-and-trust

(from that link there is access to a lot of information if you are
interested)

But of course, it can be used by OP-TEE to access (or even just probe)
any chip.

> If so,
> that's fair enough but it doesn't really help us much. Lots of people
> test code manually before submitting patches, at least for their use
> case, but this is an open-source project. Over time people want to
> change and expand the code, and it is very hard for them to do that if
> there are no automated tests.

Right. And I dont disagree (everything should be testable)
jfyi I maintain a couple of platforms here so I am familiar with this
project (been using it on/off for a couple of decades already...umm
time flies, seems like yesterday).

But I dont think we need a test that verifies this service since
this is just an amalgamation of two other functions that should be
tested somewhere else (ie TEE RPC and I2C transfers).

IMO, if they dont exist already, u-boot would benefit from:
- a test that verifies TEE RPC
- a test that verifies TEE buffer sharing with U-BOOT
- a test that verifies I2C reading/writing

But not so much from
- a test that verifies TEE I2C trampoline service

I am going to repost the patch addressing some of your concerns (I
found some other issues) and if you still think that having a test
will be convenient sure we can go ahead and work on it.

> 
> Regards,
> Simon