On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote:
> Stefan Berger <stefanb@xxxxxxxxxxxxx> writes:
> > 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@xxxxxxxxxxxxx>
> > ---
> > 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.
> 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.
>
> 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?
> + 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: false
>
>
> cheers
