Hi Kevin,


On Mon, 10 Jul 2023 at 12:39, Kevin Brodsky <kevin.brodsky@arm.com> wrote:
On 06/07/2023 23:16, Menna Mahmoud wrote:
> got this error when enabling 'CONFIG_TEE' and 'CONFIG_OPTEE':
>
> /home/morello/workspace/linux/drivers/tee/tee_core.c:851:20: error: incompatible function
> pointer types initializing 'long (*)(struct file *, unsigned int, user_uintptr_t)'
> (aka 'long (*)(struct file *, unsigned int, unsigned __intcap)') with an expression
> of type 'long (struct file *, unsigned int, unsigned long)' [-Werror,-Wincompatible-function-pointer-types]
>         .unlocked_ioctl = tee_ioctl,
>                           ^~~~~~~~~
> 1 error generated.
> make[4]: *** [/home/morello/workspace/linux/scripts/Makefile.build:250: drivers/tee/tee_core.o] Error 1
> make[3]: *** [/home/morello/workspace/linux/scripts/Makefile.build:500: drivers/tee] Error 2
> make[2]: *** [/home/morello/workspace/linux/scripts/Makefile.build:500: drivers] Error 2
> make[1]: *** [/home/morello/workspace/linux/Makefile:1999: .] Error 2
> make[1]: Leaving directory '/home/morello/workspace/linux-out'
> make: *** [Makefile:231: __sub-make] Error 2

Hi Menna,

Thanks for this patch! The diff looks right to me, the native and compat
ioctl handlers already handle pointers correctly so we just need to
change this prototype. Just a few comments on the commit message:
- Make sure to start sentences with capital letters.
- There is no need to include the full error message, as it is an
expected issue when building a new driver in PCuABI, see [1]. Pawel
recently fixed one of those in another driver [2], his commit message
can provide some inspiration.

 
Thanks, Kevin for the feedback, I will work on it and resend the patch.
 
More importantly, did you have the chance to test OP-TEE?

No, I didn't. I just enabled it and then built the kernel. I want to test it but when trying to insert  
OP-TEE module, I got this error:
```
root@morello:~# lsmod
Module                  Size  Used by
root@morello:~# insmod optee.ko
insmod: ERROR: could not load module optee.ko: No such file or directory
root@morello:~# modprobe OP-TEE
modprobe: FATAL: Module OP-TEE not found in directory /lib/modules/6.1.0OPTEE-Test-g1eebcc168af4-dirty

```
I found it should be loaded by default but I don't why I couldn't find it with `lsmod`?
I found that I should build OP-TEE first in the kernel [1]

But need to double-check with you, Is it correct? if yes, should I follow the steps related to FVP?
and according to our discussion here [2], should I install the prerequisites [3] in the Morello-Linux container, 
and this step [4] will be in the workspace dir, then build it in the Morello-Linux container? if yes, should I use the `mount` command 
to do this or what?
or everything will be in the Morello-Linux container?
or are all these not needed and there is another way?


If so, it
would be nice to add the corresponding options to the Morello defconfig,
in a follow-up patch, as they already are in the standard arm64
defconfig. Pawel's patch from the same series [3] is a good example of
how to do this (make sure to use `make savedefconfig` to generate a
minimal defconfig file).

got it, I will do this.
 

Cheers,
Kevin

[1]
https://git.morello-project.org/morello/kernel/linux/-/blob/morello/master/Documentation/cheri/pcuabi-porting.rst
[2]
https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/message/GFCIMXVQ5SLX673Z7U4WOZAZRHTPGFFQ/
[3]
https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/message/V5LZUHNZ4DKFIWPSGUICNZOZCVQ6D3ON/

Thanks,
Menna 

[1]
https://optee.readthedocs.io/en/latest/building/index.html
[2] 
https://op-lists.linaro.org/archives/list/linux-morello@op-lists.linaro.org/message/K5343M5TXJLJRU4RDO6OKMI73OT7OO4L/
[3]
https://optee.readthedocs.io/en/latest/building/prerequisites.html
[4]
https://optee.readthedocs.io/en/latest/building/gits/build.html#step-3-get-the-source-code 



>
> fix it by changing type 'unsigned long' to 'user_uintptr_t'.
>
> Signed-off-by: Menna Mahmoud <eng.mennamahmoud.mm@gmail.com>
> ---
>  drivers/tee/tee_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 98da206cd761..780a094f16ac 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -815,7 +815,7 @@ static int tee_ioctl_supp_send(struct tee_context *ctx,
>       return rc;
>  }

> -static long tee_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +static long tee_ioctl(struct file *filp, unsigned int cmd, user_uintptr_t arg)
>  {
>       struct tee_context *ctx = filp->private_data;
>       void __user *uarg = (void __user *)arg;