Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tee-supplicant: fix build with kernel < 4.16 #369

Merged
merged 1 commit into from
Jan 8, 2024
Merged

tee-supplicant: fix build with kernel < 4.16 #369

merged 1 commit into from
Jan 8, 2024

Conversation

ffontaine
Copy link
Contributor

@ffontaine ffontaine commented Jan 4, 2024

Commit 3ac968e moved linux/tee.h from libteec/include to libteec/src resulting in the following build failure with any kernel < 4.16 (i.e before torvalds/linux@033ddf1):

/home/buildroot/autobuild/instance-3/output-1/build/optee-client-4.0.0/tee-supplicant/src/tee_supplicant.c: In function 'register_local_shm': /home/buildroot/autobuild/instance-3/output-1/build/optee-client-4.0.0/tee-supplicant/src/tee_supplicant.c:356:44: error: storage size of 'data' isn't known
  356 |         struct tee_ioctl_shm_register_data data;
      |                                            ^~~~

To fix this build failure, update CMakeLists.txt and Makefile of tee-supplicant to add libteec/src to the include directories.

Fixes: 3ac968e ("Makefile, cmake: move teec related headers")

@jenswi-linaro
Copy link
Contributor

Wouldn't that mean exporting that .h file to other packages?

@ffontaine
Copy link
Contributor Author

ffontaine commented Jan 4, 2024

I'm not an expert in optee-client, I don't even understand why a local copy of linux/tee.h is embedded instead of using the one provided by the kernel. An other option on buildroot side would be to add a dependency on kernel >= 4.16 to optee-client.

@jenswi-linaro
Copy link
Contributor

The copy is there to provide early access to new features. It needs to be put somewhere reachable by tee-supplicant too, but it should at the same time not be exported to other packages.

@ffontaine
Copy link
Contributor Author

Then, I would suggest to replace

target_include_directories(${PROJECT_NAME} PRIVATE src)

by

target_include_directories(${PROJECT_NAME}
	PRIVATE src
	PRIVATE ../libteec/src
)

Makefile will also have to be updated. Does it seems acceptable?

@jenswi-linaro
Copy link
Contributor

Works for me.

@ffontaine ffontaine changed the title libteec: move linux/tee.h back to include tee-supplicant: fix build with kernel < 4.16 Jan 5, 2024
@ffontaine
Copy link
Contributor Author

PR updated

@jenswi-linaro
Copy link
Contributor

Please add a fixes tag Fixes: 3ac968ee7c92 ("Makefile, cmake: move teec related headers")

It would be nice with a short description of what the patch does also apart from all the trouble that the offending commit caused.

@ffontaine
Copy link
Contributor Author

I updated the PR as requested

Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acked-by: Jerome Forissier <[email protected]>

@jenswi-linaro
Copy link
Contributor

Acked-by: Jens Wiklander <[email protected]>

@ffontaine
Copy link
Contributor Author

I updated the PR to also fix tee_supplicant_android.mk

@jforissier
Copy link
Contributor

I updated the PR to also fix tee_supplicant_android.mk

Good. Please add the Acked-by: tags, thanks.

Commit 3ac968e moved linux/tee.h from
libteec/include to libteec/src resulting in the following build failure
with any kernel < 4.16 (i.e before
torvalds/linux@033ddf1):

/home/buildroot/autobuild/instance-3/output-1/build/optee-client-4.0.0/tee-supplicant/src/tee_supplicant.c: In function 'register_local_shm':
/home/buildroot/autobuild/instance-3/output-1/build/optee-client-4.0.0/tee-supplicant/src/tee_supplicant.c:356:44: error: storage size of 'data' isn't known
  356 |         struct tee_ioctl_shm_register_data data;
      |                                            ^~~~

To fix this build failure, update CMakeLists.txt and Makefile of
tee-supplicant to add libteec/src to the include directories.

Fixes: 3ac968e ("Makefile, cmake: move teec related headers")

Acked-by: Jerome Forissier <[email protected]>
Acked-by: Jens Wiklander <[email protected]>
Signed-off-by: Fabrice Fontaine <[email protected]>
@jforissier jforissier merged commit f467ad3 into OP-TEE:master Jan 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants