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

efi-api.c: check availability of TPM2 device also from devicetree #32314

Conversation

mikkorapeli-linaro
Copy link

Either ACPI table or device tree entry exists if firmware has TPM2 support and those are visible also before TPM2 kernel modules are loaded.

Fixes detection of TPM2 device on qemu with swtpm when TPM kernel drivers are compiled as modules and loaded by udev. Support is correctly detected and tpm2.target triggered. Without this patch TPM2 device support needs to be visible from the firmware ACPI tables or the kernel drivers need to be compiled into the kernel.

See also:
https://lists.freedesktop.org/archives/systemd-devel/2024-April/050206.html

Either ACPI table or device tree entry exists if firmware has
TPM2 support and those are visible also before TPM2 kernel
modules are loaded.

Fixes detection of TPM2 device on qemu with swtpm when TPM kernel
drivers are compiled as modules and loaded by udev. Support is correctly
detected and tpm2.target triggered. Without this patch TPM2 device
support needs to be visible from the firmware ACPI tables or the kernel
drivers need to be compiled into the kernel.

See also:
https://lists.freedesktop.org/archives/systemd-devel/2024-April/050206.html
@github-actions github-actions bot added util-lib please-review PR is ready for (re-)review by a maintainer labels Apr 17, 2024
@poettering poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed tpm2 and removed please-review PR is ready for (re-)review by a maintainer labels Apr 17, 2024
@poettering
Copy link
Member

lgtm

@@ -503,6 +503,12 @@ bool efi_has_tpm2(void) {
if (errno != ENOENT)
log_debug_errno(errno, "Unable to test whether /sys/firmware/acpi/tables/TPM2 exists, assuming it doesn't: %m");

cache = access("/proc/device-tree/tpm-event-log", F_OK) >= 0;
Copy link

Choose a reason for hiding this comment

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

This is not standardized and we are better off avoiding it.
This DT entry comes from TF-A [0] only for QEMU and is not enabled by default.
Since BL2 doesn't have access to TPM drivers, it instead creates an EventLog and expects BL32/33 to replay it.
That entry holds the address of the EventLog

[0] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/qemu/qemu/qemu_helpers.c#L41

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @apalos. I don't know how to parse devicetree in systemd userspace pid 1 side. There doesn't seem to be kernel interfaces for this until the correct TPM devices have been loaded. Closing this MR.

I noticed at least on arm64 machine, AVA, which reports a TPM device in ACPI but it's not working with our kernel drivers currently.

Copy link

@apalos apalos Apr 17, 2024

Choose a reason for hiding this comment

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

@mikkorapeli-linaro I am not too familiar with systemd internal APIs but the tpm node name is standardized in the DT-bindings [0] and should match '^tpm(@[0-9a-f]+)?$'

The QEMU entry looks like this
/proc/device-tree/platform-bus@c000000/tpm_tis@0/

Could you perhaps 'grep' for a tpm and then look at the compatible or the name? e.g

$~ cat /proc/device-tree/platform-bus@c000000/tpm_tis@0/name
tpm_tis
$~ cat /proc/device-tree/platform-bus@c000000/tpm_tis@0/compatible 
tcg,tpm-tis-mmio

[0] https://www.kernel.org/doc/Documentation/devicetree/bindings/tpm/tpm-common.yaml

Copy link
Member

Choose a reason for hiding this comment

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

urgh, iterating through device trees to find a node is a bit much for a quick feature check like we need it. Ideally we'd have a single access() check only like for the ACPI node.

Isn't there any file/dir in sysfs, procfs or elsewhere that either exists on such arm platforms or does not exist if the firmware is providing TPM measured boot? there must be something?

Copy link
Member

Choose a reason for hiding this comment

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

I mean, iterating through the tree is not totally impossible, we do have feature check code that looks for virtio devices by scanning through parts of sysfs. But it's a last-resort thing really, it would be way better if we had a single flag file we can look for that we don't have to scan for.

(See src/core/kmod-setup.c has_virtio_feature() for an example how to do that)

Copy link
Author

Choose a reason for hiding this comment

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

Problem with this /proc interface is that it's not a single file or directory. Traversing the tree for file matching a regexp is not nice in early boot fast path. I think there should be a better interface for early usespace to detect firmware TPM support via devicetree. I think the ACPI table check for server grade HW is enough for now, though there can be HW where this info is wrong and firmware and/or TPM device is actually broken or kernel side support is missing, like for AVA in our setup currently. For embedded boards, it feels ok to require drivers to be built into the kernel if there is no ACPI table entry. Not sure exactly what the Arm SystemReady variants require from certified HW. https://www.arm.com/architecture/system-architectures/systemready-certification-program

Copy link

Choose a reason for hiding this comment

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

I mean, iterating through the tree is not totally impossible, we do have feature check code that looks for virtio devices by scanning through parts of sysfs. But it's a last-resort thing really, it would be way better if we had a single flag file we can look for that we don't have to scan for.

(See src/core/kmod-setup.c has_virtio_feature() for an example how to do that)

It depends on when we expect systemd to run and detect all that.
There is /sys/kernel/security/tpm0/binary_bios_measurements. We could either scan for the folder or the file.
The file though is the EventLog and might not always be there. It also depends on the tpm being built-in or a loaded module.

Copy link
Author

Choose a reason for hiding this comment

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

/sys/kernel/security/tpm0/binary_bios_measurements is not available before kernel modules for the TPM device are loaded.

@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Apr 17, 2024
@mikkorapeli-linaro
Copy link
Author

Different kernel interface based on TPM Event Log proposed here

https://lore.kernel.org/all/20240422112711.362779-1-mikko.rapeli@linaro.org/T/#u

and systemd side here: #32400

Could not reopen this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants