Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log
flat view
From: Rob Herring <robh@kernel.org>
Date: 2024-03-07 21:52:19
Also in:
linux-integrity, lkml
On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote:
Stefan Berger [off-list ref] writes:quoted
linux,sml-base holds the address of a buffer with the TPM log. This buffer may become invalid after a kexec and therefore embed the whole TPM log in linux,sml-log. This helps to protect the log since it is properly carried across a kexec with both of the kexec syscalls. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- arch/powerpc/kernel/prom_init.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index e67effdba85c..41268c30de4c 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c@@ -1956,12 +1956,8 @@ static void __init prom_instantiate_sml(void) reserve_mem(base, size); - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", - &base, sizeof(base)); - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", - &size, sizeof(size)); - - prom_debug("sml base = 0x%llx\n", base); + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log", + (void *)base, size);As we discussed via chat, doing it this way sucks the full content of the log back into Open Firmware. That relies on OF handling such big properties, and also means more memory will be consumed, which can cause problems early in boot. A better solution is to explicitly add the log to the FDT in the flattening phase.
Why can't you just use /reserved-memory here? That should be preserved from one kernel entry to the next.
quoted hunk ↗ jump to hunk
Also adding the new linux,sml-log property should be accompanied by a change to the device tree binding. The syntax is not very obvious to me, but possibly something like?diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml index 50a3fd31241c..cd75037948bc 100644 --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml@@ -74,8 +74,6 @@ required: - ibm,my-dma-window - ibm,my-drc-index - ibm,loc-code - - linux,sml-base - - linux,sml-size
Dropping required properties is an ABI break. If you drop them, an older OS version won't work.
quoted hunk ↗ jump to hunk
allOf: - $ref: tpm-common.yaml#diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml b/Documentation/devicetree/bindings/tpm/tpm-common.yaml index 3c1241b2a43f..616604707c95 100644 --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml@@ -25,6 +25,11 @@ properties: base address of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint64 + linux,sml-log:
Why is this Linux specific?
quoted hunk ↗ jump to hunk
+ description: + Content of firmware event log + $ref: /schemas/types.yaml#/definitions/uint8-array + linux,sml-size: description: size of reserved memory allocated for firmware event log@@ -53,15 +58,22 @@ dependentRequired: linux,sml-base: ['linux,sml-size'] linux,sml-size: ['linux,sml-base'] -# must only have either memory-region or linux,sml-base +# must only have either memory-region or linux,sml-base/size or linux,sml-log # as well as either resets or reset-gpios dependentSchemas: memory-region: properties: linux,sml-base: false + linux,sml-log: false linux,sml-base: properties: memory-region: false + linux,sml-log: false + linux,sml-log: + properties: + memory-region: false + linux,sml-base: false + linux,sml-size: false resets: properties: reset-gpios: falsecheers