* [PATCH 0/2] Preserve TPM log across kexec @ 2024-03-06 15:55 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash, Stefan Berger This series resolves an issue on PowerVM and KVM on Power where the memory the TPM log was held in may become inaccessible or corrupted after a kexec soft reboot. The solution on these two platforms is to store the whole log in the device tree because the device tree is preserved across a kexec with either of the two kexec syscalls. Regards, Stefan Stefan Berger (2): powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log tpm: of: If available Use linux,sml-log to get the log and its size arch/powerpc/kernel/prom_init.c | 8 ++------ drivers/char/tpm/eventlog/of.c | 36 ++++++++++----------------------- 2 files changed, 13 insertions(+), 31 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 0/2] Preserve TPM log across kexec @ 2024-03-06 15:55 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: rnsastry, linux-kernel, jarkko, peterhuewe, viparash, Stefan Berger This series resolves an issue on PowerVM and KVM on Power where the memory the TPM log was held in may become inaccessible or corrupted after a kexec soft reboot. The solution on these two platforms is to store the whole log in the device tree because the device tree is preserved across a kexec with either of the two kexec syscalls. Regards, Stefan Stefan Berger (2): powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log tpm: of: If available Use linux,sml-log to get the log and its size arch/powerpc/kernel/prom_init.c | 8 ++------ drivers/char/tpm/eventlog/of.c | 36 ++++++++++----------------------- 2 files changed, 13 insertions(+), 31 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-06 15:55 ` Stefan Berger @ 2024-03-06 15:55 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash, Stefan Berger 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); prom_debug("sml size = 0x%x\n", size); prom_debug("prom_instantiate_sml: end...\n"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-06 15:55 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: rnsastry, linux-kernel, jarkko, peterhuewe, viparash, Stefan Berger 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); prom_debug("sml size = 0x%x\n", size); prom_debug("prom_instantiate_sml: end...\n"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-06 15:55 ` Stefan Berger @ 2024-03-07 10:41 ` Michael Ellerman -1 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-07 10:41 UTC (permalink / raw) To: Stefan Berger, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash, Stefan Berger Stefan Berger <stefanb@linux.ibm.com> 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@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. 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 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: + 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 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 10:41 ` Michael Ellerman 0 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-07 10:41 UTC (permalink / raw) To: Stefan Berger, linux-integrity, linuxppc-dev Cc: rnsastry, linux-kernel, jarkko, peterhuewe, viparash, Stefan Berger Stefan Berger <stefanb@linux.ibm.com> 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@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. 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 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: + 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 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 10:41 ` Michael Ellerman @ 2024-03-07 15:11 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 15:11 UTC (permalink / raw) To: Michael Ellerman, linux-integrity, linuxppc-dev, conor.dooley, nayna, Lukas Wunner Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/7/24 05:41, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> 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@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. Done. > > Also adding the new linux,sml-log property should be accompanied by a > change to the device tree binding. See my proposal below. > > 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 > > 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: > + 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 > > I have been working with this patch here now and it passes the following test: make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml index 50a3fd31241c..cacf6c3082de 100644 --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml @@ -74,8 +74,12 @@ required: - ibm,my-dma-window - ibm,my-drc-index - ibm,loc-code - - linux,sml-base - - linux,sml-size +oneOf: + - required: + - linux,sml-base + - linux,sml-size + - required: + - linux,sml-log allOf: - $ref: tpm-common.yaml# @@ -102,3 +106,21 @@ examples: linux,sml-size = <0xbce10200>; }; }; + - | + soc { + #address-cells = <1>; + #size-cells = <0>; + + tpm@30000003 { + compatible = "IBM,vtpm"; + device_type = "IBM,vtpm"; + reg = <0x30000003>; + interrupts = <0xa0003 0x0>; + ibm,#dma-address-cells = <0x2>; + ibm,#dma-size-cells = <0x2>; + ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>; + ibm,my-drc-index = <0x30000003>; + ibm,loc-code = "U8286.41A.10082DV-V3-C3"; + linux,sml-log = <00 00 00 00 03 00 00>; + }; + }; diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml b/Documentation/devicetree/bindings/tpm/tpm-common.yaml index 3c1241b2a43f..591c48f8cb74 100644 --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml @@ -30,6 +30,11 @@ properties: size of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint32 + linux,sml-log: + description: + firmware event log + $ref: /schemas/types.yaml#/definitions/uint8-array + memory-region: description: reserved memory allocated for firmware event log maxItems: 1 Is my patch missing something? ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 15:11 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 15:11 UTC (permalink / raw) To: Michael Ellerman, linux-integrity, linuxppc-dev, conor.dooley, nayna, Lukas Wunner Cc: jarkko, viparash, linux-kernel, peterhuewe, rnsastry On 3/7/24 05:41, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> 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@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. Done. > > Also adding the new linux,sml-log property should be accompanied by a > change to the device tree binding. See my proposal below. > > 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 > > 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: > + 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 > > I have been working with this patch here now and it passes the following test: make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml index 50a3fd31241c..cacf6c3082de 100644 --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml @@ -74,8 +74,12 @@ required: - ibm,my-dma-window - ibm,my-drc-index - ibm,loc-code - - linux,sml-base - - linux,sml-size +oneOf: + - required: + - linux,sml-base + - linux,sml-size + - required: + - linux,sml-log allOf: - $ref: tpm-common.yaml# @@ -102,3 +106,21 @@ examples: linux,sml-size = <0xbce10200>; }; }; + - | + soc { + #address-cells = <1>; + #size-cells = <0>; + + tpm@30000003 { + compatible = "IBM,vtpm"; + device_type = "IBM,vtpm"; + reg = <0x30000003>; + interrupts = <0xa0003 0x0>; + ibm,#dma-address-cells = <0x2>; + ibm,#dma-size-cells = <0x2>; + ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>; + ibm,my-drc-index = <0x30000003>; + ibm,loc-code = "U8286.41A.10082DV-V3-C3"; + linux,sml-log = <00 00 00 00 03 00 00>; + }; + }; diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml b/Documentation/devicetree/bindings/tpm/tpm-common.yaml index 3c1241b2a43f..591c48f8cb74 100644 --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml @@ -30,6 +30,11 @@ properties: size of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint32 + linux,sml-log: + description: + firmware event log + $ref: /schemas/types.yaml#/definitions/uint8-array + memory-region: description: reserved memory allocated for firmware event log maxItems: 1 Is my patch missing something? ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 15:11 ` Stefan Berger @ 2024-03-07 20:39 ` Conor Dooley -1 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 20:39 UTC (permalink / raw) To: Stefan Berger Cc: Michael Ellerman, linux-integrity, linuxppc-dev, conor.dooley, nayna, Lukas Wunner, linux-kernel, jarkko, rnsastry, peterhuewe, viparash [-- Attachment #1: Type: text/plain, Size: 5497 bytes --] On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: > On 3/7/24 05:41, Michael Ellerman wrote: > > Stefan Berger <stefanb@linux.ibm.com> writes: > > > > Also adding the new linux,sml-log property should be accompanied by a > > change to the device tree binding. > > > See my proposal below. > > > > > 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 > > 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: > > + 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 > > > > > > I have been working with this patch here now and it passes the following > test: > > make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml > > > diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > index 50a3fd31241c..cacf6c3082de 100644 > --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > @@ -74,8 +74,12 @@ required: > - ibm,my-dma-window > - ibm,my-drc-index > - ibm,loc-code > - - linux,sml-base > - - linux,sml-size > +oneOf: > + - required: > + - linux,sml-base > + - linux,sml-size > + - required: > + - linux,sml-log > > allOf: > - $ref: tpm-common.yaml# > @@ -102,3 +106,21 @@ examples: > linux,sml-size = <0xbce10200>; > }; > }; > + - | > + soc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + tpm@30000003 { > + compatible = "IBM,vtpm"; > + device_type = "IBM,vtpm"; > + reg = <0x30000003>; > + interrupts = <0xa0003 0x0>; > + ibm,#dma-address-cells = <0x2>; > + ibm,#dma-size-cells = <0x2>; > + ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>; > + ibm,my-drc-index = <0x30000003>; > + ibm,loc-code = "U8286.41A.10082DV-V3-C3"; > + linux,sml-log = <00 00 00 00 03 00 00>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > index 3c1241b2a43f..591c48f8cb74 100644 > --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > @@ -30,6 +30,11 @@ properties: > size of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint32 > > + linux,sml-log: > + description: > + firmware event log Can you provide a more complete description here please as to what the different between this and the other property? If I was populating a DT I would have absolutely no idea whether or not to use this or the other property, nor how to go about actually populating it. The "log" in your example doesn't look like an actual log of any sort, but I know nothing about TPMs so I'll take your word for it that that's what a TPM log looks like. > + $ref: /schemas/types.yaml#/definitions/uint8-array > + > memory-region: > description: reserved memory allocated for firmware event log > maxItems: 1 > > > Is my patch missing something? I think you also need the dependantSchema stuff you had in your original snippet that makes the linux,* properties mutually exclusive with memory-region (or at least something like that). Please make sure you CC the DT maintainers and list on the v2 and Lukas Wunner too. Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 20:39 ` Conor Dooley 0 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 20:39 UTC (permalink / raw) To: Stefan Berger Cc: rnsastry, nayna, linux-kernel, conor.dooley, Lukas Wunner, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe [-- Attachment #1: Type: text/plain, Size: 5497 bytes --] On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: > On 3/7/24 05:41, Michael Ellerman wrote: > > Stefan Berger <stefanb@linux.ibm.com> writes: > > > > Also adding the new linux,sml-log property should be accompanied by a > > change to the device tree binding. > > > See my proposal below. > > > > > 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 > > 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: > > + 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 > > > > > > I have been working with this patch here now and it passes the following > test: > > make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml > > > diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > index 50a3fd31241c..cacf6c3082de 100644 > --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml > @@ -74,8 +74,12 @@ required: > - ibm,my-dma-window > - ibm,my-drc-index > - ibm,loc-code > - - linux,sml-base > - - linux,sml-size > +oneOf: > + - required: > + - linux,sml-base > + - linux,sml-size > + - required: > + - linux,sml-log > > allOf: > - $ref: tpm-common.yaml# > @@ -102,3 +106,21 @@ examples: > linux,sml-size = <0xbce10200>; > }; > }; > + - | > + soc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + tpm@30000003 { > + compatible = "IBM,vtpm"; > + device_type = "IBM,vtpm"; > + reg = <0x30000003>; > + interrupts = <0xa0003 0x0>; > + ibm,#dma-address-cells = <0x2>; > + ibm,#dma-size-cells = <0x2>; > + ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>; > + ibm,my-drc-index = <0x30000003>; > + ibm,loc-code = "U8286.41A.10082DV-V3-C3"; > + linux,sml-log = <00 00 00 00 03 00 00>; > + }; > + }; > diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > index 3c1241b2a43f..591c48f8cb74 100644 > --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > @@ -30,6 +30,11 @@ properties: > size of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint32 > > + linux,sml-log: > + description: > + firmware event log Can you provide a more complete description here please as to what the different between this and the other property? If I was populating a DT I would have absolutely no idea whether or not to use this or the other property, nor how to go about actually populating it. The "log" in your example doesn't look like an actual log of any sort, but I know nothing about TPMs so I'll take your word for it that that's what a TPM log looks like. > + $ref: /schemas/types.yaml#/definitions/uint8-array > + > memory-region: > description: reserved memory allocated for firmware event log > maxItems: 1 > > > Is my patch missing something? I think you also need the dependantSchema stuff you had in your original snippet that makes the linux,* properties mutually exclusive with memory-region (or at least something like that). Please make sure you CC the DT maintainers and list on the v2 and Lukas Wunner too. Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 20:39 ` Conor Dooley @ 2024-03-07 21:15 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 21:15 UTC (permalink / raw) To: Conor Dooley Cc: Michael Ellerman, linux-integrity, linuxppc-dev, conor.dooley, nayna, Lukas Wunner, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/7/24 15:39, Conor Dooley wrote: > On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: >> On 3/7/24 05:41, Michael Ellerman wrote: >>> Stefan Berger <stefanb@linux.ibm.com> writes: > >>> >> diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> b/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> index 3c1241b2a43f..591c48f8cb74 100644 >> --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> @@ -30,6 +30,11 @@ properties: >> size of reserved memory allocated for firmware event log >> $ref: /schemas/types.yaml#/definitions/uint32 >> >> + linux,sml-log: >> + description: >> + firmware event log > > Can you provide a more complete description here please as to what the > different between this and the other property? If I was populating a DT > I would have absolutely no idea whether or not to use this or the other > property, nor how to go about actually populating it. > The "log" in your example doesn't look like an actual log of any sort, > but I know nothing about TPMs so I'll take your word for it that that's > what a TPM log looks like. In the example I cannot give a log but only a part of it. The log is in binary format and in case of TPM 2.0 starts with a header followed by log entries about what was measured. I don't think it's necessary to even give the full log header here. You do need some TPM specific knowledge about the 'firmware even log'. The existing properties are described like this: linux,sml-base: description: base address of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint64 linux,sml-size: description: size of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint32 Would this describe the new property 'better' by prefixing it with 'embedded'? linux,sml-log: description: embedded firmware event log $ref: /schemas/types.yaml#/definitions/uint8-array > >> + $ref: /schemas/types.yaml#/definitions/uint8-array >> + >> memory-region: >> description: reserved memory allocated for firmware event log >> maxItems: 1 >> >> >> Is my patch missing something? > > I think you also need the dependantSchema stuff you had in your original > snippet that makes the linux,* properties mutually exclusive with > memory-region (or at least something like that). > I modified my new example now like this: ... ibm,loc-code = "U9080.HEX.134CA08-V7-C3"; linux,sml-log = <00 00 00 00 03 00 00>; linux,sml-size = <0xbce10200>; <-- added The check fails like this: # make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dts DTC_CHK Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: 'linux,sml-base' is a dependency of 'linux,sml-size' from schema $id: http://devicetree.org/schemas/tpm/tpm-common.yaml# /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: 'linux,sml-base' is a dependency of 'linux,sml-size' from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: Unevaluated properties are not allowed ('interrupts', 'linux,sml-log', 'linux,sml-size' were unexpected) from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# When I modify the existing example like this: ibm,loc-code = "U8286.41A.10082DV-V3-C3"; linux,sml-base = <0xc60e 0x0>; linux,sml-size = <0xbce10200>; linux,sml-log = <00 00 00 00 03 00 00>; <- added The check fails like this: # make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dts DTC_CHK Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: More than one condition true in oneOf schema: {'$filename': '/root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml', '$id': 'http://devicetree.org/schemas/tpm/ibm,vtpm.yaml#', '$schema': 'http://devicetree.org/meta-schemas/core.yaml#', 'allOf': [{'$ref': 'tpm-common.yaml#'}], 'oneOf': [{'required': ['linux,sml-base', 'linux,sml-size']}, {'required': ['linux,sml-log']}], 'patternProperties': {'pinctrl-[0-9]+': True}, 'properties': {'$nodename': True, 'bootph-all': True, 'bootph-pre-ram': True, 'bootph-pre-sram': True, 'bootph-some-ram': True, 'bootph-verify': True, 'compatible': {'items': [{'enum': ['IBM,vtpm', 'IBM,vtpm20']}], 'maxItems': 1, 'minItems': 1, 'type': 'array'}, 'device_type': {'items': [{'enum': ['IBM,vtpm', 'IBM,vtpm20']}], 'maxItems': 1, 'minItems': 1, 'type': 'array'}, 'ibm,#dma-address-cells': {'$ref': '/schemas/types.yaml#/definitions/uint32-array'}, 'ibm,#dma-size-cells': {'$ref': '/schemas/types.yaml#/definitions/uint32-array'}, 'ibm,loc-code': {'$ref': '/schemas/types.yaml#/definitions/string'}, 'ibm,my-dma-window': {'$ref': '/schemas/types.yaml#/definitions/uint32-array', 'items': {'maxItems': 5, 'minItems': 5}, 'maxItems': 1, 'type': 'array'}, 'ibm,my-drc-index': {'$ref': '/schemas/types.yaml#/definitions/uint32'}, 'phandle': True, 'pinctrl-names': True, 'reg': {'maxItems': 1, 'minItems': 1}, 'secure-status': True, 'status': True}, 'required': ['compatible', 'device_type', 'reg', 'interrupts', 'ibm,#dma-address-cells', 'ibm,#dma-size-cells', 'ibm,my-dma-window', 'ibm,my-drc-index', 'ibm,loc-code'], 'select': {'properties': {'compatible': {'contains': {'enum': ['IBM,vtpm', 'IBM,vtpm20']}}}, 'required': ['compatible']}, 'title': 'IBM Virtual Trusted Platform Module (vTPM)', 'type': 'object', 'unevaluatedProperties': False} from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# It errors out on bad examples, which is good. > Please make sure you CC the DT maintainers and list on the v2 and Lukas > Wunner too. Yes, I have them already cc'ed here. > > Thanks, > Conor. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 21:15 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 21:15 UTC (permalink / raw) To: Conor Dooley Cc: rnsastry, nayna, linux-kernel, conor.dooley, Lukas Wunner, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On 3/7/24 15:39, Conor Dooley wrote: > On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: >> On 3/7/24 05:41, Michael Ellerman wrote: >>> Stefan Berger <stefanb@linux.ibm.com> writes: > >>> >> diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> b/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> index 3c1241b2a43f..591c48f8cb74 100644 >> --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml >> @@ -30,6 +30,11 @@ properties: >> size of reserved memory allocated for firmware event log >> $ref: /schemas/types.yaml#/definitions/uint32 >> >> + linux,sml-log: >> + description: >> + firmware event log > > Can you provide a more complete description here please as to what the > different between this and the other property? If I was populating a DT > I would have absolutely no idea whether or not to use this or the other > property, nor how to go about actually populating it. > The "log" in your example doesn't look like an actual log of any sort, > but I know nothing about TPMs so I'll take your word for it that that's > what a TPM log looks like. In the example I cannot give a log but only a part of it. The log is in binary format and in case of TPM 2.0 starts with a header followed by log entries about what was measured. I don't think it's necessary to even give the full log header here. You do need some TPM specific knowledge about the 'firmware even log'. The existing properties are described like this: linux,sml-base: description: base address of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint64 linux,sml-size: description: size of reserved memory allocated for firmware event log $ref: /schemas/types.yaml#/definitions/uint32 Would this describe the new property 'better' by prefixing it with 'embedded'? linux,sml-log: description: embedded firmware event log $ref: /schemas/types.yaml#/definitions/uint8-array > >> + $ref: /schemas/types.yaml#/definitions/uint8-array >> + >> memory-region: >> description: reserved memory allocated for firmware event log >> maxItems: 1 >> >> >> Is my patch missing something? > > I think you also need the dependantSchema stuff you had in your original > snippet that makes the linux,* properties mutually exclusive with > memory-region (or at least something like that). > I modified my new example now like this: ... ibm,loc-code = "U9080.HEX.134CA08-V7-C3"; linux,sml-log = <00 00 00 00 03 00 00>; linux,sml-size = <0xbce10200>; <-- added The check fails like this: # make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dts DTC_CHK Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: 'linux,sml-base' is a dependency of 'linux,sml-size' from schema $id: http://devicetree.org/schemas/tpm/tpm-common.yaml# /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: 'linux,sml-base' is a dependency of 'linux,sml-size' from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: Unevaluated properties are not allowed ('interrupts', 'linux,sml-log', 'linux,sml-size' were unexpected) from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# When I modify the existing example like this: ibm,loc-code = "U8286.41A.10082DV-V3-C3"; linux,sml-base = <0xc60e 0x0>; linux,sml-size = <0xbce10200>; linux,sml-log = <00 00 00 00 03 00 00>; <- added The check fails like this: # make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dts DTC_CHK Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb /root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.example.dtb: tpm@30000003: More than one condition true in oneOf schema: {'$filename': '/root/linux/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml', '$id': 'http://devicetree.org/schemas/tpm/ibm,vtpm.yaml#', '$schema': 'http://devicetree.org/meta-schemas/core.yaml#', 'allOf': [{'$ref': 'tpm-common.yaml#'}], 'oneOf': [{'required': ['linux,sml-base', 'linux,sml-size']}, {'required': ['linux,sml-log']}], 'patternProperties': {'pinctrl-[0-9]+': True}, 'properties': {'$nodename': True, 'bootph-all': True, 'bootph-pre-ram': True, 'bootph-pre-sram': True, 'bootph-some-ram': True, 'bootph-verify': True, 'compatible': {'items': [{'enum': ['IBM,vtpm', 'IBM,vtpm20']}], 'maxItems': 1, 'minItems': 1, 'type': 'array'}, 'device_type': {'items': [{'enum': ['IBM,vtpm', 'IBM,vtpm20']}], 'maxItems': 1, 'minItems': 1, 'type': 'array'}, 'ibm,#dma-address-cells': {'$ref': '/schemas/types.yaml#/definitions/uint32-array'}, 'ibm,#dma-size-cells': {'$ref': '/schemas/types.yaml#/definitions/uint32-array'}, 'ibm,loc-code': {'$ref': '/schemas/types.yaml#/definitions/string'}, 'ibm,my-dma-window': {'$ref': '/schemas/types.yaml#/definitions/uint32-array', 'items': {'maxItems': 5, 'minItems': 5}, 'maxItems': 1, 'type': 'array'}, 'ibm,my-drc-index': {'$ref': '/schemas/types.yaml#/definitions/uint32'}, 'phandle': True, 'pinctrl-names': True, 'reg': {'maxItems': 1, 'minItems': 1}, 'secure-status': True, 'status': True}, 'required': ['compatible', 'device_type', 'reg', 'interrupts', 'ibm,#dma-address-cells', 'ibm,#dma-size-cells', 'ibm,my-dma-window', 'ibm,my-drc-index', 'ibm,loc-code'], 'select': {'properties': {'compatible': {'contains': {'enum': ['IBM,vtpm', 'IBM,vtpm20']}}}, 'required': ['compatible']}, 'title': 'IBM Virtual Trusted Platform Module (vTPM)', 'type': 'object', 'unevaluatedProperties': False} from schema $id: http://devicetree.org/schemas/tpm/ibm,vtpm.yaml# It errors out on bad examples, which is good. > Please make sure you CC the DT maintainers and list on the v2 and Lukas > Wunner too. Yes, I have them already cc'ed here. > > Thanks, > Conor. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 21:15 ` Stefan Berger @ 2024-03-07 21:29 ` Conor Dooley -1 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 21:29 UTC (permalink / raw) To: Stefan Berger Cc: Michael Ellerman, linux-integrity, linuxppc-dev, conor.dooley, nayna, Lukas Wunner, linux-kernel, jarkko, rnsastry, peterhuewe, viparash [-- Attachment #1: Type: text/plain, Size: 4578 bytes --] On Thu, Mar 07, 2024 at 04:15:01PM -0500, Stefan Berger wrote: > > > On 3/7/24 15:39, Conor Dooley wrote: > > On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: > > > On 3/7/24 05:41, Michael Ellerman wrote: > > > > Stefan Berger <stefanb@linux.ibm.com> writes: > > > > > > > > > diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > index 3c1241b2a43f..591c48f8cb74 100644 > > > --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > @@ -30,6 +30,11 @@ properties: > > > size of reserved memory allocated for firmware event log > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > + linux,sml-log: > > > + description: > > > + firmware event log > > > > Can you provide a more complete description here please as to what the > > different between this and the other property? If I was populating a DT > > I would have absolutely no idea whether or not to use this or the other > > property, nor how to go about actually populating it. > > The "log" in your example doesn't look like an actual log of any sort, > > but I know nothing about TPMs so I'll take your word for it that that's > > what a TPM log looks like. > > In the example I cannot give a log but only a part of it. The log is in > binary format and in case of TPM 2.0 starts with a header followed by log > entries about what was measured. I don't think it's necessary to even give > the full log header here. You do need some TPM specific knowledge about the > 'firmware even log'. > > > The existing properties are described like this: > > linux,sml-base: > description: > base address of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint64 > > linux,sml-size: > description: > size of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint32 > > Would this describe the new property 'better' by prefixing it with > 'embedded'? IMO, no that's not any better. Spell it out so that someone who doesn't know his arse from his elbow when it comes to tpm immediately knows that this means the entire tpm log is inside the dtb. The paragraph you wrote above gives more information about what this property is populated with than the property description does. > linux,sml-log: > description: > embedded firmware event log > $ref: /schemas/types.yaml#/definitions/uint8-array > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint8-array > > > + > > > memory-region: > > > description: reserved memory allocated for firmware event log > > > maxItems: 1 > > > > > > > > > Is my patch missing something? > > > > I think you also need the dependantSchema stuff you had in your original > > snippet that makes the linux,* properties mutually exclusive with > > memory-region (or at least something like that). > > > I modified my new example now like this: > ... > ibm,loc-code = "U9080.HEX.134CA08-V7-C3"; > linux,sml-log = <00 00 00 00 03 00 00>; > linux,sml-size = <0xbce10200>; <-- added > ibm,loc-code = "U8286.41A.10082DV-V3-C3"; > linux,sml-base = <0xc60e 0x0>; > linux,sml-size = <0xbce10200>; > linux,sml-log = <00 00 00 00 03 00 00>; <- added > > It errors out on bad examples, which is good. Aye, that is covered by your new oneOf for this one binding. The dependantSchema bit in tpm-common.yaml enforces it for all tpm devices. It also covers the memory-region property being mutually exclusive with the linux,sml-{base,size} properties so I think you need to extend that to also cover linux,sml-lof property. > > Please make sure you CC the DT maintainers and list on the v2 and Lukas > > Wunner too. > > Yes, I have them already cc'ed here. To: Conor Dooley <conor@kernel.org> Cc: Michael Ellerman <mpe@ellerman.id.au>, linux-integrity@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, conor.dooley@microchip.com, nayna@linux.ibm.com, Lukas Wunner <lukas@wunner.de>, linux-kernel@vger.kernel.org, jarkko@kernel.org, rnsastry@linux.ibm.com, peterhuewe@gmx.de, viparash@in.ibm.com You have Lukas, one of the three DT maintainers and not the list as far as I can see. Correct me please if I am wrong. Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 21:29 ` Conor Dooley 0 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 21:29 UTC (permalink / raw) To: Stefan Berger Cc: rnsastry, nayna, linux-kernel, conor.dooley, Lukas Wunner, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe [-- Attachment #1: Type: text/plain, Size: 4578 bytes --] On Thu, Mar 07, 2024 at 04:15:01PM -0500, Stefan Berger wrote: > > > On 3/7/24 15:39, Conor Dooley wrote: > > On Thu, Mar 07, 2024 at 10:11:03AM -0500, Stefan Berger wrote: > > > On 3/7/24 05:41, Michael Ellerman wrote: > > > > Stefan Berger <stefanb@linux.ibm.com> writes: > > > > > > > > > diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > index 3c1241b2a43f..591c48f8cb74 100644 > > > --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml > > > @@ -30,6 +30,11 @@ properties: > > > size of reserved memory allocated for firmware event log > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > > > + linux,sml-log: > > > + description: > > > + firmware event log > > > > Can you provide a more complete description here please as to what the > > different between this and the other property? If I was populating a DT > > I would have absolutely no idea whether or not to use this or the other > > property, nor how to go about actually populating it. > > The "log" in your example doesn't look like an actual log of any sort, > > but I know nothing about TPMs so I'll take your word for it that that's > > what a TPM log looks like. > > In the example I cannot give a log but only a part of it. The log is in > binary format and in case of TPM 2.0 starts with a header followed by log > entries about what was measured. I don't think it's necessary to even give > the full log header here. You do need some TPM specific knowledge about the > 'firmware even log'. > > > The existing properties are described like this: > > linux,sml-base: > description: > base address of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint64 > > linux,sml-size: > description: > size of reserved memory allocated for firmware event log > $ref: /schemas/types.yaml#/definitions/uint32 > > Would this describe the new property 'better' by prefixing it with > 'embedded'? IMO, no that's not any better. Spell it out so that someone who doesn't know his arse from his elbow when it comes to tpm immediately knows that this means the entire tpm log is inside the dtb. The paragraph you wrote above gives more information about what this property is populated with than the property description does. > linux,sml-log: > description: > embedded firmware event log > $ref: /schemas/types.yaml#/definitions/uint8-array > > > > > > > + $ref: /schemas/types.yaml#/definitions/uint8-array > > > + > > > memory-region: > > > description: reserved memory allocated for firmware event log > > > maxItems: 1 > > > > > > > > > Is my patch missing something? > > > > I think you also need the dependantSchema stuff you had in your original > > snippet that makes the linux,* properties mutually exclusive with > > memory-region (or at least something like that). > > > I modified my new example now like this: > ... > ibm,loc-code = "U9080.HEX.134CA08-V7-C3"; > linux,sml-log = <00 00 00 00 03 00 00>; > linux,sml-size = <0xbce10200>; <-- added > ibm,loc-code = "U8286.41A.10082DV-V3-C3"; > linux,sml-base = <0xc60e 0x0>; > linux,sml-size = <0xbce10200>; > linux,sml-log = <00 00 00 00 03 00 00>; <- added > > It errors out on bad examples, which is good. Aye, that is covered by your new oneOf for this one binding. The dependantSchema bit in tpm-common.yaml enforces it for all tpm devices. It also covers the memory-region property being mutually exclusive with the linux,sml-{base,size} properties so I think you need to extend that to also cover linux,sml-lof property. > > Please make sure you CC the DT maintainers and list on the v2 and Lukas > > Wunner too. > > Yes, I have them already cc'ed here. To: Conor Dooley <conor@kernel.org> Cc: Michael Ellerman <mpe@ellerman.id.au>, linux-integrity@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, conor.dooley@microchip.com, nayna@linux.ibm.com, Lukas Wunner <lukas@wunner.de>, linux-kernel@vger.kernel.org, jarkko@kernel.org, rnsastry@linux.ibm.com, peterhuewe@gmx.de, viparash@in.ibm.com You have Lukas, one of the three DT maintainers and not the list as far as I can see. Correct me please if I am wrong. Thanks, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 10:41 ` Michael Ellerman @ 2024-03-07 21:52 ` Rob Herring -1 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-07 21:52 UTC (permalink / raw) To: Michael Ellerman Cc: Stefan Berger, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> 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@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. > 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 ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 21:52 ` Rob Herring 0 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-07 21:52 UTC (permalink / raw) To: Michael Ellerman Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe, Stefan Berger On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> 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@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. > 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 ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-07 21:52 ` Rob Herring @ 2024-03-08 12:23 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 12:23 UTC (permalink / raw) To: Rob Herring, Michael Ellerman Cc: linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/7/24 16:52, Rob Herring wrote: > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>> --- >>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>> 1 file changed, 2 insertions(+), 6 deletions(-) >>> > > >> 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. 1) On PowerVM and KVM on Power these two properties were added in the Linux code. I replaced the creation of these properties with creation of linux,sml-log (1/2 in this series). I also replaced the handling of these two (2/2 in this series) for these two platforms but leaving it for powernv systems where the firmware creates these. https://elixir.bootlin.com/linux/latest/source/arch/powerpc/kernel/prom_init.c#L1959 2) There is an example in the ibm,vtpm.yaml file that has both of these and the test case still passes the check when the two entries above are removed. I will post v2 with the changes to the DT bindings for linux,sml-log including an example for linux,sml-log. [The test cases fail, as expected, when an additional property is added, such as when linux,sml-base is added when linux,sml-log is there or linux,sml-log is added when linux,sml-base is there.] > >> >> 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? I am not sure about the criteria when to prefix with 'linux,' and when not to. In this case I did it because it is created by Linux and because of existing linux,sml-base and linux,sml-size. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-08 12:23 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 12:23 UTC (permalink / raw) To: Rob Herring, Michael Ellerman Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On 3/7/24 16:52, Rob Herring wrote: > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>> --- >>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>> 1 file changed, 2 insertions(+), 6 deletions(-) >>> > > >> 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. 1) On PowerVM and KVM on Power these two properties were added in the Linux code. I replaced the creation of these properties with creation of linux,sml-log (1/2 in this series). I also replaced the handling of these two (2/2 in this series) for these two platforms but leaving it for powernv systems where the firmware creates these. https://elixir.bootlin.com/linux/latest/source/arch/powerpc/kernel/prom_init.c#L1959 2) There is an example in the ibm,vtpm.yaml file that has both of these and the test case still passes the check when the two entries above are removed. I will post v2 with the changes to the DT bindings for linux,sml-log including an example for linux,sml-log. [The test cases fail, as expected, when an additional property is added, such as when linux,sml-base is added when linux,sml-log is there or linux,sml-log is added when linux,sml-base is there.] > >> >> 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? I am not sure about the criteria when to prefix with 'linux,' and when not to. In this case I did it because it is created by Linux and because of existing linux,sml-base and linux,sml-size. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-08 12:23 ` Stefan Berger @ 2024-03-08 20:57 ` Rob Herring -1 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-08 20:57 UTC (permalink / raw) To: Stefan Berger Cc: Michael Ellerman, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: > > > On 3/7/24 16:52, Rob Herring wrote: > > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> > > > > --- > > > > arch/powerpc/kernel/prom_init.c | 8 ++------ > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > > > > > 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. > > 1) On PowerVM and KVM on Power these two properties were added in the Linux > code. I replaced the creation of these properties with creation of > linux,sml-log (1/2 in this series). I also replaced the handling of > these two (2/2 in this series) for these two platforms but leaving it for > powernv systems where the firmware creates these. Okay, I guess your case is not a ABI break if the kernel is populating it and the same kernel consumes it. You failed to answer my question on using /reserved-memory. Again, why can't that be used? That is the standard way we prevent chunks of memory from being clobbered. There's already support for describing the TPM log that way anyways. The only reasoning I can see writing out a node for that is harder than just adding a property, but that's not a great argument IMO. > 2) There is an example in the ibm,vtpm.yaml file that has both of these > and the test case still passes the check when the two entries above are > removed. I will post v2 with the changes to the DT bindings for > linux,sml-log including an example for linux,sml-log. [The test cases fail, > as expected, when an additional property is added, such as when > linux,sml-base is added when linux,sml-log is there or linux,sml-log is > added when linux,sml-base is there.] Sure, removing a required property is never going to break the DT checks. What would break is a client (OS) version that only understands linux,sml-base and can no longer get the log assuming getting the log itself was required. Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-08 20:57 ` Rob Herring 0 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-08 20:57 UTC (permalink / raw) To: Stefan Berger Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: > > > On 3/7/24 16:52, Rob Herring wrote: > > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> > > > > --- > > > > arch/powerpc/kernel/prom_init.c | 8 ++------ > > > > 1 file changed, 2 insertions(+), 6 deletions(-) > > > > > > > > > > > > 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. > > 1) On PowerVM and KVM on Power these two properties were added in the Linux > code. I replaced the creation of these properties with creation of > linux,sml-log (1/2 in this series). I also replaced the handling of > these two (2/2 in this series) for these two platforms but leaving it for > powernv systems where the firmware creates these. Okay, I guess your case is not a ABI break if the kernel is populating it and the same kernel consumes it. You failed to answer my question on using /reserved-memory. Again, why can't that be used? That is the standard way we prevent chunks of memory from being clobbered. There's already support for describing the TPM log that way anyways. The only reasoning I can see writing out a node for that is harder than just adding a property, but that's not a great argument IMO. > 2) There is an example in the ibm,vtpm.yaml file that has both of these > and the test case still passes the check when the two entries above are > removed. I will post v2 with the changes to the DT bindings for > linux,sml-log including an example for linux,sml-log. [The test cases fail, > as expected, when an additional property is added, such as when > linux,sml-base is added when linux,sml-log is there or linux,sml-log is > added when linux,sml-base is there.] Sure, removing a required property is never going to break the DT checks. What would break is a client (OS) version that only understands linux,sml-base and can no longer get the log assuming getting the log itself was required. Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-08 20:57 ` Rob Herring @ 2024-03-08 21:26 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 21:26 UTC (permalink / raw) To: Rob Herring Cc: Michael Ellerman, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/8/24 15:57, Rob Herring wrote: > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >> >> >> On 3/7/24 16:52, Rob Herring wrote: >>> On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >>>> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>>>> --- >>>>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>>> >> >>> >>> >>>> 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. >> >> 1) On PowerVM and KVM on Power these two properties were added in the Linux >> code. I replaced the creation of these properties with creation of >> linux,sml-log (1/2 in this series). I also replaced the handling of >> these two (2/2 in this series) for these two platforms but leaving it for >> powernv systems where the firmware creates these. > > Okay, I guess your case is not a ABI break if the kernel is populating > it and the same kernel consumes it. > > You failed to answer my question on using /reserved-memory. Again, why I am not familiar with /reserved-memory and whether it is supported on the targeted platforms. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-08 21:26 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 21:26 UTC (permalink / raw) To: Rob Herring Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On 3/8/24 15:57, Rob Herring wrote: > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >> >> >> On 3/7/24 16:52, Rob Herring wrote: >>> On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >>>> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>>>> --- >>>>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>>> >> >>> >>> >>>> 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. >> >> 1) On PowerVM and KVM on Power these two properties were added in the Linux >> code. I replaced the creation of these properties with creation of >> linux,sml-log (1/2 in this series). I also replaced the handling of >> these two (2/2 in this series) for these two platforms but leaving it for >> powernv systems where the firmware creates these. > > Okay, I guess your case is not a ABI break if the kernel is populating > it and the same kernel consumes it. > > You failed to answer my question on using /reserved-memory. Again, why I am not familiar with /reserved-memory and whether it is supported on the targeted platforms. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-08 20:57 ` Rob Herring @ 2024-03-12 10:32 ` Michael Ellerman -1 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-12 10:32 UTC (permalink / raw) To: Rob Herring, Stefan Berger Cc: linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash Rob Herring <robh@kernel.org> writes: > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >> On 3/7/24 16:52, Rob Herring wrote: >> > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >> > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >> > > > --- >> > > > arch/powerpc/kernel/prom_init.c | 8 ++------ >> > > > 1 file changed, 2 insertions(+), 6 deletions(-) >> > > > >> >> > >> > >> > > 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. >> >> 1) On PowerVM and KVM on Power these two properties were added in the Linux >> code. I replaced the creation of these properties with creation of >> linux,sml-log (1/2 in this series). I also replaced the handling of >> these two (2/2 in this series) for these two platforms but leaving it for >> powernv systems where the firmware creates these. > > Okay, I guess your case is not a ABI break if the kernel is populating > it and the same kernel consumes it. > > You failed to answer my question on using /reserved-memory. Again, why > can't that be used? That is the standard way we prevent chunks of memory > from being clobbered. Yes I think that would mostly work. I don't see support for /reserved-memory in kexec-tools, so that would need fixing I think. My logic was that the memory is not special. It's just a buffer we allocated during early boot to store the log. There isn't anything else in the system that relies on that memory remaining untouched. So it seemed cleaner to just put the log in the device tree, rather than a pointer to it. Having the log external to the device tree creates several problems, like the crash kernel region colliding with it, it being clobbered by kexec, etc. cheers ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-12 10:32 ` Michael Ellerman 0 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-12 10:32 UTC (permalink / raw) To: Rob Herring, Stefan Berger Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe Rob Herring <robh@kernel.org> writes: > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >> On 3/7/24 16:52, Rob Herring wrote: >> > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >> > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >> > > > --- >> > > > arch/powerpc/kernel/prom_init.c | 8 ++------ >> > > > 1 file changed, 2 insertions(+), 6 deletions(-) >> > > > >> >> > >> > >> > > 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. >> >> 1) On PowerVM and KVM on Power these two properties were added in the Linux >> code. I replaced the creation of these properties with creation of >> linux,sml-log (1/2 in this series). I also replaced the handling of >> these two (2/2 in this series) for these two platforms but leaving it for >> powernv systems where the firmware creates these. > > Okay, I guess your case is not a ABI break if the kernel is populating > it and the same kernel consumes it. > > You failed to answer my question on using /reserved-memory. Again, why > can't that be used? That is the standard way we prevent chunks of memory > from being clobbered. Yes I think that would mostly work. I don't see support for /reserved-memory in kexec-tools, so that would need fixing I think. My logic was that the memory is not special. It's just a buffer we allocated during early boot to store the log. There isn't anything else in the system that relies on that memory remaining untouched. So it seemed cleaner to just put the log in the device tree, rather than a pointer to it. Having the log external to the device tree creates several problems, like the crash kernel region colliding with it, it being clobbered by kexec, etc. cheers ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-12 10:32 ` Michael Ellerman @ 2024-03-12 16:22 ` Rob Herring -1 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-12 16:22 UTC (permalink / raw) To: Michael Ellerman Cc: Stefan Berger, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On Tue, Mar 12, 2024 at 09:32:50PM +1100, Michael Ellerman wrote: > Rob Herring <robh@kernel.org> writes: > > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: > >> On 3/7/24 16:52, Rob Herring wrote: > >> > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > >> > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> > >> > > > --- > >> > > > arch/powerpc/kernel/prom_init.c | 8 ++------ > >> > > > 1 file changed, 2 insertions(+), 6 deletions(-) > >> > > > > >> > >> > > >> > > >> > > 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. > >> > >> 1) On PowerVM and KVM on Power these two properties were added in the Linux > >> code. I replaced the creation of these properties with creation of > >> linux,sml-log (1/2 in this series). I also replaced the handling of > >> these two (2/2 in this series) for these two platforms but leaving it for > >> powernv systems where the firmware creates these. > > > > Okay, I guess your case is not a ABI break if the kernel is populating > > it and the same kernel consumes it. > > > > You failed to answer my question on using /reserved-memory. Again, why > > can't that be used? That is the standard way we prevent chunks of memory > > from being clobbered. > > Yes I think that would mostly work. I don't see support for > /reserved-memory in kexec-tools, so that would need fixing I think. > > My logic was that the memory is not special. It's just a buffer we > allocated during early boot to store the log. There isn't anything else > in the system that relies on that memory remaining untouched. So it > seemed cleaner to just put the log in the device tree, rather than a > pointer to it. My issue is we already have 2 ways to describe the log to the OS. I don't see a good reason to add a 3rd way. (Though it might actually be a 4th way, because the chosen property for the last attempt was accepted to dtschema yet the code has been abandoned.) If you put the log into the DT, then the memory for the log remains untouched too because the FDT remains untouched. For reserved-memory regions, the OS is free to free them if it knows what the region is and that it is no longer needed. IOW, if freeing the log memory is desired, then the suggested approach doesn't work. > > Having the log external to the device tree creates several problems, > like the crash kernel region colliding with it, it being clobbered by > kexec, etc. We have multiple regions to pass/maintain thru kexec, so how does having one less really matter? Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-12 16:22 ` Rob Herring 0 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-12 16:22 UTC (permalink / raw) To: Michael Ellerman Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe, Stefan Berger On Tue, Mar 12, 2024 at 09:32:50PM +1100, Michael Ellerman wrote: > Rob Herring <robh@kernel.org> writes: > > On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: > >> On 3/7/24 16:52, Rob Herring wrote: > >> > On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: > >> > > Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> > >> > > > --- > >> > > > arch/powerpc/kernel/prom_init.c | 8 ++------ > >> > > > 1 file changed, 2 insertions(+), 6 deletions(-) > >> > > > > >> > >> > > >> > > >> > > 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. > >> > >> 1) On PowerVM and KVM on Power these two properties were added in the Linux > >> code. I replaced the creation of these properties with creation of > >> linux,sml-log (1/2 in this series). I also replaced the handling of > >> these two (2/2 in this series) for these two platforms but leaving it for > >> powernv systems where the firmware creates these. > > > > Okay, I guess your case is not a ABI break if the kernel is populating > > it and the same kernel consumes it. > > > > You failed to answer my question on using /reserved-memory. Again, why > > can't that be used? That is the standard way we prevent chunks of memory > > from being clobbered. > > Yes I think that would mostly work. I don't see support for > /reserved-memory in kexec-tools, so that would need fixing I think. > > My logic was that the memory is not special. It's just a buffer we > allocated during early boot to store the log. There isn't anything else > in the system that relies on that memory remaining untouched. So it > seemed cleaner to just put the log in the device tree, rather than a > pointer to it. My issue is we already have 2 ways to describe the log to the OS. I don't see a good reason to add a 3rd way. (Though it might actually be a 4th way, because the chosen property for the last attempt was accepted to dtschema yet the code has been abandoned.) If you put the log into the DT, then the memory for the log remains untouched too because the FDT remains untouched. For reserved-memory regions, the OS is free to free them if it knows what the region is and that it is no longer needed. IOW, if freeing the log memory is desired, then the suggested approach doesn't work. > > Having the log external to the device tree creates several problems, > like the crash kernel region colliding with it, it being clobbered by > kexec, etc. We have multiple regions to pass/maintain thru kexec, so how does having one less really matter? Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-12 16:22 ` Rob Herring @ 2024-03-12 19:15 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-12 19:15 UTC (permalink / raw) To: Rob Herring, Michael Ellerman Cc: linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/12/24 12:22, Rob Herring wrote: > On Tue, Mar 12, 2024 at 09:32:50PM +1100, Michael Ellerman wrote: >> Rob Herring <robh@kernel.org> writes: >>> On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >>>> On 3/7/24 16:52, Rob Herring wrote: >>>>> On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >>>>>> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>>>>>> --- >>>>>>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>>>>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>>>>> >>>> >>>>> >>>>> >>>>>> 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. >>>> >>>> 1) On PowerVM and KVM on Power these two properties were added in the Linux >>>> code. I replaced the creation of these properties with creation of >>>> linux,sml-log (1/2 in this series). I also replaced the handling of >>>> these two (2/2 in this series) for these two platforms but leaving it for >>>> powernv systems where the firmware creates these. >>> >>> Okay, I guess your case is not a ABI break if the kernel is populating >>> it and the same kernel consumes it. >>> >>> You failed to answer my question on using /reserved-memory. Again, why >>> can't that be used? That is the standard way we prevent chunks of memory >>> from being clobbered. >> >> Yes I think that would mostly work. I don't see support for >> /reserved-memory in kexec-tools, so that would need fixing I think. >> >> My logic was that the memory is not special. It's just a buffer we >> allocated during early boot to store the log. There isn't anything else >> in the system that relies on that memory remaining untouched. So it >> seemed cleaner to just put the log in the device tree, rather than a >> pointer to it. > > My issue is we already have 2 ways to describe the log to the OS. I > don't see a good reason to add a 3rd way. (Though it might actually be a > 4th way, because the chosen property for the last attempt was accepted > to dtschema yet the code has been abandoned.) I have a revert for this in my tree... > > If you put the log into the DT, then the memory for the log remains > untouched too because the FDT remains untouched. For reserved-memory > regions, the OS is free to free them if it knows what the region is and > that it is no longer needed. IOW, if freeing the log memory is desired, > then the suggested approach doesn't work. The log, once embedded in the FDT, would stay there forever. The TPM subsystem looks at it when initializing and will look at it again when initializing after a kexec soft reboot. > >> >> Having the log external to the device tree creates several problems, >> like the crash kernel region colliding with it, it being clobbered by >> kexec, etc. > > We have multiple regions to pass/maintain thru kexec, so how does having > one less really matter? I had tried it with the newer kexec syscall. Yes, there was a way of doing it but the older kexec call is still around and since that one is also still being used the problems with corrupted memory for example still persisted. So that's why we now embed it in the FDT because both syscalls carry that across unharmed. > > Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-12 19:15 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-12 19:15 UTC (permalink / raw) To: Rob Herring, Michael Ellerman Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On 3/12/24 12:22, Rob Herring wrote: > On Tue, Mar 12, 2024 at 09:32:50PM +1100, Michael Ellerman wrote: >> Rob Herring <robh@kernel.org> writes: >>> On Fri, Mar 08, 2024 at 07:23:35AM -0500, Stefan Berger wrote: >>>> On 3/7/24 16:52, Rob Herring wrote: >>>>> On Thu, Mar 07, 2024 at 09:41:31PM +1100, Michael Ellerman wrote: >>>>>> Stefan Berger <stefanb@linux.ibm.com> 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@linux.ibm.com> >>>>>>> --- >>>>>>> arch/powerpc/kernel/prom_init.c | 8 ++------ >>>>>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>>>>> >>>> >>>>> >>>>> >>>>>> 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. >>>> >>>> 1) On PowerVM and KVM on Power these two properties were added in the Linux >>>> code. I replaced the creation of these properties with creation of >>>> linux,sml-log (1/2 in this series). I also replaced the handling of >>>> these two (2/2 in this series) for these two platforms but leaving it for >>>> powernv systems where the firmware creates these. >>> >>> Okay, I guess your case is not a ABI break if the kernel is populating >>> it and the same kernel consumes it. >>> >>> You failed to answer my question on using /reserved-memory. Again, why >>> can't that be used? That is the standard way we prevent chunks of memory >>> from being clobbered. >> >> Yes I think that would mostly work. I don't see support for >> /reserved-memory in kexec-tools, so that would need fixing I think. >> >> My logic was that the memory is not special. It's just a buffer we >> allocated during early boot to store the log. There isn't anything else >> in the system that relies on that memory remaining untouched. So it >> seemed cleaner to just put the log in the device tree, rather than a >> pointer to it. > > My issue is we already have 2 ways to describe the log to the OS. I > don't see a good reason to add a 3rd way. (Though it might actually be a > 4th way, because the chosen property for the last attempt was accepted > to dtschema yet the code has been abandoned.) I have a revert for this in my tree... > > If you put the log into the DT, then the memory for the log remains > untouched too because the FDT remains untouched. For reserved-memory > regions, the OS is free to free them if it knows what the region is and > that it is no longer needed. IOW, if freeing the log memory is desired, > then the suggested approach doesn't work. The log, once embedded in the FDT, would stay there forever. The TPM subsystem looks at it when initializing and will look at it again when initializing after a kexec soft reboot. > >> >> Having the log external to the device tree creates several problems, >> like the crash kernel region colliding with it, it being clobbered by >> kexec, etc. > > We have multiple regions to pass/maintain thru kexec, so how does having > one less really matter? I had tried it with the newer kexec syscall. Yes, there was a way of doing it but the older kexec call is still around and since that one is also still being used the problems with corrupted memory for example still persisted. So that's why we now embed it in the FDT because both syscalls carry that across unharmed. > > Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log 2024-03-06 15:55 ` Stefan Berger @ 2024-03-07 20:11 ` Jarkko Sakkinen -1 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 20:11 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > 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. So, I see only description of the problem but nothing how it gets addressed. > > 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); > prom_debug("sml size = 0x%x\n", size); > > prom_debug("prom_instantiate_sml: end...\n"); BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log @ 2024-03-07 20:11 ` Jarkko Sakkinen 0 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 20:11 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > 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. So, I see only description of the problem but nothing how it gets addressed. > > 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); > prom_debug("sml size = 0x%x\n", size); > > prom_debug("prom_instantiate_sml: end...\n"); BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-06 15:55 ` Stefan Berger @ 2024-03-06 15:55 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash, Stefan Berger If linux,sml-log is available use it to get the TPM log rather than the pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM on Power where after a kexec the memory pointed to by linux,sml-base may have been corrupted. Also, linux,sml-log has replaced linux,sml-base and linux,sml-size on these two platforms. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c index 930fe43d5daf..e37196e64ef1 100644 --- a/drivers/char/tpm/eventlog/of.c +++ b/drivers/char/tpm/eventlog/of.c @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) const u32 *sizep; const u64 *basep; struct tpm_bios_log *log; + const void *logp; u32 size; - u64 base; log = &chip->log; if (chip->dev.parent && chip->dev.parent->of_node) @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) if (of_property_read_bool(np, "powered-while-suspended")) chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; - sizep = of_get_property(np, "linux,sml-size", NULL); - basep = of_get_property(np, "linux,sml-base", NULL); - if (sizep == NULL && basep == NULL) - return tpm_read_log_memory_region(chip); - if (sizep == NULL || basep == NULL) - return -EIO; - - /* - * For both vtpm/tpm, firmware has log addr and log size in big - * endian format. But in case of vtpm, there is a method called - * sml-handover which is run during kernel init even before - * device tree is setup. This sml-handover function takes care - * of endianness and writes to sml-base and sml-size in little - * endian format. For this reason, vtpm doesn't need conversion - * but physical tpm needs the conversion. - */ - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { + logp = of_get_property(np, "linux,sml-log", &size); + if (logp == NULL) { + sizep = of_get_property(np, "linux,sml-size", NULL); + basep = of_get_property(np, "linux,sml-base", NULL); + if (sizep == NULL && basep == NULL) + return tpm_read_log_memory_region(chip); + if (sizep == NULL || basep == NULL) + return -EIO; + logp = __va(be64_to_cpup((__force __be64 *)basep)); size = be32_to_cpup((__force __be32 *)sizep); - base = be64_to_cpup((__force __be64 *)basep); - } else { - size = *sizep; - base = *basep; } - if (size == 0) { dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); return -EIO; } - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); if (!log->bios_event_log) return -ENOMEM; -- 2.43.0 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-06 15:55 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 15:55 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: rnsastry, linux-kernel, jarkko, peterhuewe, viparash, Stefan Berger If linux,sml-log is available use it to get the TPM log rather than the pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM on Power where after a kexec the memory pointed to by linux,sml-base may have been corrupted. Also, linux,sml-log has replaced linux,sml-base and linux,sml-size on these two platforms. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c index 930fe43d5daf..e37196e64ef1 100644 --- a/drivers/char/tpm/eventlog/of.c +++ b/drivers/char/tpm/eventlog/of.c @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) const u32 *sizep; const u64 *basep; struct tpm_bios_log *log; + const void *logp; u32 size; - u64 base; log = &chip->log; if (chip->dev.parent && chip->dev.parent->of_node) @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) if (of_property_read_bool(np, "powered-while-suspended")) chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; - sizep = of_get_property(np, "linux,sml-size", NULL); - basep = of_get_property(np, "linux,sml-base", NULL); - if (sizep == NULL && basep == NULL) - return tpm_read_log_memory_region(chip); - if (sizep == NULL || basep == NULL) - return -EIO; - - /* - * For both vtpm/tpm, firmware has log addr and log size in big - * endian format. But in case of vtpm, there is a method called - * sml-handover which is run during kernel init even before - * device tree is setup. This sml-handover function takes care - * of endianness and writes to sml-base and sml-size in little - * endian format. For this reason, vtpm doesn't need conversion - * but physical tpm needs the conversion. - */ - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { + logp = of_get_property(np, "linux,sml-log", &size); + if (logp == NULL) { + sizep = of_get_property(np, "linux,sml-size", NULL); + basep = of_get_property(np, "linux,sml-base", NULL); + if (sizep == NULL && basep == NULL) + return tpm_read_log_memory_region(chip); + if (sizep == NULL || basep == NULL) + return -EIO; + logp = __va(be64_to_cpup((__force __be64 *)basep)); size = be32_to_cpup((__force __be32 *)sizep); - base = be64_to_cpup((__force __be64 *)basep); - } else { - size = *sizep; - base = *basep; } - if (size == 0) { dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); return -EIO; } - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); if (!log->bios_event_log) return -ENOMEM; -- 2.43.0 ^ permalink raw reply related [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-06 15:55 ` Stefan Berger @ 2024-03-07 10:42 ` Michael Ellerman -1 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-07 10:42 UTC (permalink / raw) To: Stefan Berger, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash, Stefan Berger Stefan Berger <stefanb@linux.ibm.com> writes: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. It would be good to mention that powernv platforms (sometimes called Open Power or bare metal) still use linux,sml-base/size, via skiboot. cheers > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0 ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-07 10:42 ` Michael Ellerman 0 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-07 10:42 UTC (permalink / raw) To: Stefan Berger, linux-integrity, linuxppc-dev Cc: rnsastry, linux-kernel, jarkko, peterhuewe, viparash, Stefan Berger Stefan Berger <stefanb@linux.ibm.com> writes: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. It would be good to mention that powernv platforms (sometimes called Open Power or bare metal) still use linux,sml-base/size, via skiboot. cheers > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0 ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-07 10:42 ` Michael Ellerman @ 2024-03-07 15:00 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 15:00 UTC (permalink / raw) To: Michael Ellerman, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/7/24 05:42, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: >> If linux,sml-log is available use it to get the TPM log rather than the >> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >> on Power where after a kexec the memory pointed to by linux,sml-base may >> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >> linux,sml-size on these two platforms. > > It would be good to mention that powernv platforms (sometimes called > Open Power or bare metal) still use linux,sml-base/size, via skiboot. Will do in v2. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-07 15:00 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 15:00 UTC (permalink / raw) To: Michael Ellerman, linux-integrity, linuxppc-dev Cc: jarkko, viparash, linux-kernel, peterhuewe, rnsastry On 3/7/24 05:42, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: >> If linux,sml-log is available use it to get the TPM log rather than the >> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >> on Power where after a kexec the memory pointed to by linux,sml-base may >> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >> linux,sml-size on these two platforms. > > It would be good to mention that powernv platforms (sometimes called > Open Power or bare metal) still use linux,sml-base/size, via skiboot. Will do in v2. ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-06 15:55 ` Stefan Berger @ 2024-03-07 11:35 ` Conor Dooley -1 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 11:35 UTC (permalink / raw) To: Stefan Berger Cc: mpe, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash [-- Attachment #1: Type: text/plain, Size: 3296 bytes --] On Wed, Mar 06, 2024 at 10:55:11AM -0500, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. Those two properties are documented, but linux,sml-log is not, nor can I find patches on the list documenting it. There should be a patch adding this to tmp-common.yaml. Cheers, Conor. > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-07 11:35 ` Conor Dooley 0 siblings, 0 replies; 58+ messages in thread From: Conor Dooley @ 2024-03-07 11:35 UTC (permalink / raw) To: Stefan Berger Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe [-- Attachment #1: Type: text/plain, Size: 3296 bytes --] On Wed, Mar 06, 2024 at 10:55:11AM -0500, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. Those two properties are documented, but linux,sml-log is not, nor can I find patches on the list documenting it. There should be a patch adding this to tmp-common.yaml. Cheers, Conor. > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-06 15:55 ` Stefan Berger @ 2024-03-07 19:57 ` Jarkko Sakkinen -1 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 19:57 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash in short summary: s/Use/use/ On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> So shouldn't this have a fixed tag, or not? > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > Looks pretty good other than that. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-07 19:57 ` Jarkko Sakkinen 0 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 19:57 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry in short summary: s/Use/use/ On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> So shouldn't this have a fixed tag, or not? > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > Looks pretty good other than that. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-07 19:57 ` Jarkko Sakkinen @ 2024-03-07 20:00 ` Jarkko Sakkinen -1 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 20:00 UTC (permalink / raw) To: Jarkko Sakkinen, Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > in short summary: s/Use/use/ > > On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > > If linux,sml-log is available use it to get the TPM log rather than the > > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > > on Power where after a kexec the memory pointed to by linux,sml-base may > > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > > linux,sml-size on these two platforms. > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > So shouldn't this have a fixed tag, or not? In English: do we want this to be backported to stable kernel releases or not? BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-07 20:00 ` Jarkko Sakkinen 0 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-07 20:00 UTC (permalink / raw) To: Jarkko Sakkinen, Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > in short summary: s/Use/use/ > > On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > > If linux,sml-log is available use it to get the TPM log rather than the > > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > > on Power where after a kexec the memory pointed to by linux,sml-base may > > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > > linux,sml-size on these two platforms. > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > So shouldn't this have a fixed tag, or not? In English: do we want this to be backported to stable kernel releases or not? BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-07 20:00 ` Jarkko Sakkinen @ 2024-03-08 12:17 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 12:17 UTC (permalink / raw) To: Jarkko Sakkinen, mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On 3/7/24 15:00, Jarkko Sakkinen wrote: > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >> in short summary: s/Use/use/ >> >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>> If linux,sml-log is available use it to get the TPM log rather than the >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>> on Power where after a kexec the memory pointed to by linux,sml-base may >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>> linux,sml-size on these two platforms. >>> >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> >> So shouldn't this have a fixed tag, or not? > > In English: do we want this to be backported to stable kernel releases or not? Ideally, yes. v3 will have 3 patches and all 3 of them will have to be backported *together* and not applied otherwise if any one of them fails. Can this be 'guaranteed'? > > BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-08 12:17 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-08 12:17 UTC (permalink / raw) To: Jarkko Sakkinen, mpe, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On 3/7/24 15:00, Jarkko Sakkinen wrote: > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >> in short summary: s/Use/use/ >> >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>> If linux,sml-log is available use it to get the TPM log rather than the >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>> on Power where after a kexec the memory pointed to by linux,sml-base may >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>> linux,sml-size on these two platforms. >>> >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> >> So shouldn't this have a fixed tag, or not? > > In English: do we want this to be backported to stable kernel releases or not? Ideally, yes. v3 will have 3 patches and all 3 of them will have to be backported *together* and not applied otherwise if any one of them fails. Can this be 'guaranteed'? > > BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-08 12:17 ` Stefan Berger @ 2024-03-11 20:09 ` Jarkko Sakkinen -1 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-11 20:09 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On Fri Mar 8, 2024 at 2:17 PM EET, Stefan Berger wrote: > > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >> in short summary: s/Use/use/ > >> > >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>> If linux,sml-log is available use it to get the TPM log rather than the > >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>> linux,sml-size on these two platforms. > >>> > >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >> > >> So shouldn't this have a fixed tag, or not? > > > > In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? All of them will end up to stable if the following conditions hold: - All have a fixes tag. - All have "Cc: stable@vger.kernel.org". - We agree in the review process that they are all legit fixes. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-11 20:09 ` Jarkko Sakkinen 0 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-11 20:09 UTC (permalink / raw) To: Stefan Berger, mpe, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On Fri Mar 8, 2024 at 2:17 PM EET, Stefan Berger wrote: > > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >> in short summary: s/Use/use/ > >> > >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>> If linux,sml-log is available use it to get the TPM log rather than the > >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>> linux,sml-size on these two platforms. > >>> > >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >> > >> So shouldn't this have a fixed tag, or not? > > > > In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? All of them will end up to stable if the following conditions hold: - All have a fixes tag. - All have "Cc: stable@vger.kernel.org". - We agree in the review process that they are all legit fixes. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-08 12:17 ` Stefan Berger @ 2024-03-12 10:35 ` Michael Ellerman -1 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-12 10:35 UTC (permalink / raw) To: Stefan Berger, Jarkko Sakkinen, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash Stefan Berger <stefanb@linux.ibm.com> writes: > On 3/7/24 15:00, Jarkko Sakkinen wrote: >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>> in short summary: s/Use/use/ >>> >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>> If linux,sml-log is available use it to get the TPM log rather than the >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>> linux,sml-size on these two platforms. >>>> >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>> >>> So shouldn't this have a fixed tag, or not? >> >> In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? You can use Depends-on: <previous commit SHA> to indicate the relationship. cheers ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-12 10:35 ` Michael Ellerman 0 siblings, 0 replies; 58+ messages in thread From: Michael Ellerman @ 2024-03-12 10:35 UTC (permalink / raw) To: Stefan Berger, Jarkko Sakkinen, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry Stefan Berger <stefanb@linux.ibm.com> writes: > On 3/7/24 15:00, Jarkko Sakkinen wrote: >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>> in short summary: s/Use/use/ >>> >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>> If linux,sml-log is available use it to get the TPM log rather than the >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>> linux,sml-size on these two platforms. >>>> >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>> >>> So shouldn't this have a fixed tag, or not? >> >> In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? You can use Depends-on: <previous commit SHA> to indicate the relationship. cheers ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-12 10:35 ` Michael Ellerman @ 2024-03-12 15:50 ` Jarkko Sakkinen -1 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-12 15:50 UTC (permalink / raw) To: Michael Ellerman, Stefan Berger, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >>> in short summary: s/Use/use/ > >>> > >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>>> If linux,sml-log is available use it to get the TPM log rather than the > >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>>> linux,sml-size on these two platforms. > >>>> > >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >>> > >>> So shouldn't this have a fixed tag, or not? > >> > >> In English: do we want this to be backported to stable kernel releases or not? > > > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > > backported *together* and not applied otherwise if any one of them > > fails. Can this be 'guaranteed'? > > You can use Depends-on: <previous commit SHA> to indicate the relationship. > > cheers Thanks, I've missed depends-on tag. Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure that I don't forget to add it. Right, and since these are so small scoped commits, and bug fixes in particular, it is also possible to do PR during the release cycle. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-12 15:50 ` Jarkko Sakkinen 0 siblings, 0 replies; 58+ messages in thread From: Jarkko Sakkinen @ 2024-03-12 15:50 UTC (permalink / raw) To: Michael Ellerman, Stefan Berger, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >>> in short summary: s/Use/use/ > >>> > >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>>> If linux,sml-log is available use it to get the TPM log rather than the > >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>>> linux,sml-size on these two platforms. > >>>> > >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >>> > >>> So shouldn't this have a fixed tag, or not? > >> > >> In English: do we want this to be backported to stable kernel releases or not? > > > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > > backported *together* and not applied otherwise if any one of them > > fails. Can this be 'guaranteed'? > > You can use Depends-on: <previous commit SHA> to indicate the relationship. > > cheers Thanks, I've missed depends-on tag. Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure that I don't forget to add it. Right, and since these are so small scoped commits, and bug fixes in particular, it is also possible to do PR during the release cycle. BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size 2024-03-12 15:50 ` Jarkko Sakkinen @ 2024-03-12 19:08 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-12 19:08 UTC (permalink / raw) To: Jarkko Sakkinen, Michael Ellerman, linux-integrity, linuxppc-dev Cc: linux-kernel, rnsastry, peterhuewe, viparash On 3/12/24 11:50, Jarkko Sakkinen wrote: > On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: >> Stefan Berger <stefanb@linux.ibm.com> writes: >>> On 3/7/24 15:00, Jarkko Sakkinen wrote: >>>> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>>>> in short summary: s/Use/use/ >>>>> >>>>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>>>> If linux,sml-log is available use it to get the TPM log rather than the >>>>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>>>> linux,sml-size on these two platforms. >>>>>> >>>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>>>> >>>>> So shouldn't this have a fixed tag, or not? >>>> >>>> In English: do we want this to be backported to stable kernel releases or not? >>> >>> Ideally, yes. v3 will have 3 patches and all 3 of them will have to be >>> backported *together* and not applied otherwise if any one of them >>> fails. Can this be 'guaranteed'? >> >> You can use Depends-on: <previous commit SHA> to indicate the relationship. >> >> cheers > > Thanks, I've missed depends-on tag. > > Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure > that I don't forget to add it. Yeah, once we know whether this is the way forward or not... I posted v2 as RFC to figure this out. v2's 2/3 patch will only apply to 6.8. To avoid any inconsistencies between code and bindings we cannot even go further back with this series (IFF it's the way forward at all). So I am inclined to remove the Fixes tags. I also find little under Documentation about the Depends-on tag and what it's supposed to be formatted like -- a commit hash of 1/3 appearing in 2/3 for example? The commit hash is not stable at this point so I couldn't created it. > Right, and since these are so small scoped commits, and bug fixes in > particular, it is also possible to do PR during the release cycle. > > BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size @ 2024-03-12 19:08 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-12 19:08 UTC (permalink / raw) To: Jarkko Sakkinen, Michael Ellerman, linux-integrity, linuxppc-dev Cc: peterhuewe, viparash, linux-kernel, rnsastry On 3/12/24 11:50, Jarkko Sakkinen wrote: > On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: >> Stefan Berger <stefanb@linux.ibm.com> writes: >>> On 3/7/24 15:00, Jarkko Sakkinen wrote: >>>> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>>>> in short summary: s/Use/use/ >>>>> >>>>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>>>> If linux,sml-log is available use it to get the TPM log rather than the >>>>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>>>> linux,sml-size on these two platforms. >>>>>> >>>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>>>> >>>>> So shouldn't this have a fixed tag, or not? >>>> >>>> In English: do we want this to be backported to stable kernel releases or not? >>> >>> Ideally, yes. v3 will have 3 patches and all 3 of them will have to be >>> backported *together* and not applied otherwise if any one of them >>> fails. Can this be 'guaranteed'? >> >> You can use Depends-on: <previous commit SHA> to indicate the relationship. >> >> cheers > > Thanks, I've missed depends-on tag. > > Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure > that I don't forget to add it. Yeah, once we know whether this is the way forward or not... I posted v2 as RFC to figure this out. v2's 2/3 patch will only apply to 6.8. To avoid any inconsistencies between code and bindings we cannot even go further back with this series (IFF it's the way forward at all). So I am inclined to remove the Fixes tags. I also find little under Documentation about the Depends-on tag and what it's supposed to be formatted like -- a commit hash of 1/3 appearing in 2/3 for example? The commit hash is not stable at this point so I couldn't created it. > Right, and since these are so small scoped commits, and bug fixes in > particular, it is also possible to do PR during the release cycle. > > BR, Jarkko ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec 2024-03-06 15:55 ` Stefan Berger @ 2024-03-06 16:08 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 16:08 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/6/24 10:55, Stefan Berger wrote: > This series resolves an issue on PowerVM and KVM on Power where the memory > the TPM log was held in may become inaccessible or corrupted after a kexec > soft reboot. The solution on these two platforms is to store the whole log > in the device tree because the device tree is preserved across a kexec with > either of the two kexec syscalls. > FYI: This was the previous attempt that didn't work with the older kexec syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 > Regards, > Stefan > > Stefan Berger (2): > powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log > tpm: of: If available Use linux,sml-log to get the log and its size > > arch/powerpc/kernel/prom_init.c | 8 ++------ > drivers/char/tpm/eventlog/of.c | 36 ++++++++++----------------------- > 2 files changed, 13 insertions(+), 31 deletions(-) > ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec @ 2024-03-06 16:08 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-06 16:08 UTC (permalink / raw) To: mpe, linux-integrity, linuxppc-dev Cc: jarkko, viparash, linux-kernel, peterhuewe, rnsastry On 3/6/24 10:55, Stefan Berger wrote: > This series resolves an issue on PowerVM and KVM on Power where the memory > the TPM log was held in may become inaccessible or corrupted after a kexec > soft reboot. The solution on these two platforms is to store the whole log > in the device tree because the device tree is preserved across a kexec with > either of the two kexec syscalls. > FYI: This was the previous attempt that didn't work with the older kexec syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 > Regards, > Stefan > > Stefan Berger (2): > powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log > tpm: of: If available Use linux,sml-log to get the log and its size > > arch/powerpc/kernel/prom_init.c | 8 ++------ > drivers/char/tpm/eventlog/of.c | 36 ++++++++++----------------------- > 2 files changed, 13 insertions(+), 31 deletions(-) > ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec 2024-03-06 16:08 ` Stefan Berger @ 2024-03-07 21:42 ` Rob Herring -1 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-07 21:42 UTC (permalink / raw) To: Stefan Berger Cc: mpe, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On Wed, Mar 06, 2024 at 11:08:20AM -0500, Stefan Berger wrote: > > > On 3/6/24 10:55, Stefan Berger wrote: > > This series resolves an issue on PowerVM and KVM on Power where the memory > > the TPM log was held in may become inaccessible or corrupted after a kexec > > soft reboot. The solution on these two platforms is to store the whole log > > in the device tree because the device tree is preserved across a kexec with > > either of the two kexec syscalls. > > > FYI: This was the previous attempt that didn't work with the older kexec > syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 Doesn't everyone else still need that? Is powerpc the only ones that care about the old kexec syscall? Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec @ 2024-03-07 21:42 ` Rob Herring 0 siblings, 0 replies; 58+ messages in thread From: Rob Herring @ 2024-03-07 21:42 UTC (permalink / raw) To: Stefan Berger Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On Wed, Mar 06, 2024 at 11:08:20AM -0500, Stefan Berger wrote: > > > On 3/6/24 10:55, Stefan Berger wrote: > > This series resolves an issue on PowerVM and KVM on Power where the memory > > the TPM log was held in may become inaccessible or corrupted after a kexec > > soft reboot. The solution on these two platforms is to store the whole log > > in the device tree because the device tree is preserved across a kexec with > > either of the two kexec syscalls. > > > FYI: This was the previous attempt that didn't work with the older kexec > syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 Doesn't everyone else still need that? Is powerpc the only ones that care about the old kexec syscall? Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec 2024-03-07 21:42 ` Rob Herring @ 2024-03-07 22:32 ` Stefan Berger -1 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 22:32 UTC (permalink / raw) To: Rob Herring Cc: mpe, linux-integrity, linuxppc-dev, linux-kernel, jarkko, rnsastry, peterhuewe, viparash On 3/7/24 16:42, Rob Herring wrote: > On Wed, Mar 06, 2024 at 11:08:20AM -0500, Stefan Berger wrote: >> >> >> On 3/6/24 10:55, Stefan Berger wrote: >>> This series resolves an issue on PowerVM and KVM on Power where the memory >>> the TPM log was held in may become inaccessible or corrupted after a kexec >>> soft reboot. The solution on these two platforms is to store the whole log >>> in the device tree because the device tree is preserved across a kexec with >>> either of the two kexec syscalls. >>> >> FYI: This was the previous attempt that didn't work with the older kexec >> syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 > > Doesn't everyone else still need that? Is powerpc the only ones that Are you referring to the old series with 'that' ? I more or less had to abandon it because it wouldn't solve the problem for the old kexec syscall, so that's why I am embedding the whole log now in the devicetree since the DT is properly carried across the kexec soft reboot on PowerVM and KVM for Power. Maybe other platforms will (have to) follow, but I don't know. > care about the old kexec syscall? > > Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH 0/2] Preserve TPM log across kexec @ 2024-03-07 22:32 ` Stefan Berger 0 siblings, 0 replies; 58+ messages in thread From: Stefan Berger @ 2024-03-07 22:32 UTC (permalink / raw) To: Rob Herring Cc: rnsastry, linux-kernel, jarkko, linux-integrity, viparash, linuxppc-dev, peterhuewe On 3/7/24 16:42, Rob Herring wrote: > On Wed, Mar 06, 2024 at 11:08:20AM -0500, Stefan Berger wrote: >> >> >> On 3/6/24 10:55, Stefan Berger wrote: >>> This series resolves an issue on PowerVM and KVM on Power where the memory >>> the TPM log was held in may become inaccessible or corrupted after a kexec >>> soft reboot. The solution on these two platforms is to store the whole log >>> in the device tree because the device tree is preserved across a kexec with >>> either of the two kexec syscalls. >>> >> FYI: This was the previous attempt that didn't work with the older kexec >> syscall: https://lore.kernel.org/lkml/4afde78d-e138-9eee-50e0-dbd32f4dcfe0@linux.ibm.com/T/#m158630d214837e41858b03d4b025e6f96cb8f251 > > Doesn't everyone else still need that? Is powerpc the only ones that Are you referring to the old series with 'that' ? I more or less had to abandon it because it wouldn't solve the problem for the old kexec syscall, so that's why I am embedding the whole log now in the devicetree since the DT is properly carried across the kexec soft reboot on PowerVM and KVM for Power. Maybe other platforms will (have to) follow, but I don't know. > care about the old kexec syscall? > > Rob ^ permalink raw reply [flat|nested] 58+ messages in thread
end of thread, other threads:[~2024-03-12 19:16 UTC | newest] Thread overview: 58+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-06 15:55 [PATCH 0/2] Preserve TPM log across kexec Stefan Berger 2024-03-06 15:55 ` Stefan Berger 2024-03-06 15:55 ` [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log Stefan Berger 2024-03-06 15:55 ` Stefan Berger 2024-03-07 10:41 ` Michael Ellerman 2024-03-07 10:41 ` Michael Ellerman 2024-03-07 15:11 ` Stefan Berger 2024-03-07 15:11 ` Stefan Berger 2024-03-07 20:39 ` Conor Dooley 2024-03-07 20:39 ` Conor Dooley 2024-03-07 21:15 ` Stefan Berger 2024-03-07 21:15 ` Stefan Berger 2024-03-07 21:29 ` Conor Dooley 2024-03-07 21:29 ` Conor Dooley 2024-03-07 21:52 ` Rob Herring 2024-03-07 21:52 ` Rob Herring 2024-03-08 12:23 ` Stefan Berger 2024-03-08 12:23 ` Stefan Berger 2024-03-08 20:57 ` Rob Herring 2024-03-08 20:57 ` Rob Herring 2024-03-08 21:26 ` Stefan Berger 2024-03-08 21:26 ` Stefan Berger 2024-03-12 10:32 ` Michael Ellerman 2024-03-12 10:32 ` Michael Ellerman 2024-03-12 16:22 ` Rob Herring 2024-03-12 16:22 ` Rob Herring 2024-03-12 19:15 ` Stefan Berger 2024-03-12 19:15 ` Stefan Berger 2024-03-07 20:11 ` Jarkko Sakkinen 2024-03-07 20:11 ` Jarkko Sakkinen 2024-03-06 15:55 ` [PATCH 2/2] tpm: of: If available Use linux,sml-log to get the log and its size Stefan Berger 2024-03-06 15:55 ` Stefan Berger 2024-03-07 10:42 ` Michael Ellerman 2024-03-07 10:42 ` Michael Ellerman 2024-03-07 15:00 ` Stefan Berger 2024-03-07 15:00 ` Stefan Berger 2024-03-07 11:35 ` Conor Dooley 2024-03-07 11:35 ` Conor Dooley 2024-03-07 19:57 ` Jarkko Sakkinen 2024-03-07 19:57 ` Jarkko Sakkinen 2024-03-07 20:00 ` Jarkko Sakkinen 2024-03-07 20:00 ` Jarkko Sakkinen 2024-03-08 12:17 ` Stefan Berger 2024-03-08 12:17 ` Stefan Berger 2024-03-11 20:09 ` Jarkko Sakkinen 2024-03-11 20:09 ` Jarkko Sakkinen 2024-03-12 10:35 ` Michael Ellerman 2024-03-12 10:35 ` Michael Ellerman 2024-03-12 15:50 ` Jarkko Sakkinen 2024-03-12 15:50 ` Jarkko Sakkinen 2024-03-12 19:08 ` Stefan Berger 2024-03-12 19:08 ` Stefan Berger 2024-03-06 16:08 ` [PATCH 0/2] Preserve TPM log across kexec Stefan Berger 2024-03-06 16:08 ` Stefan Berger 2024-03-07 21:42 ` Rob Herring 2024-03-07 21:42 ` Rob Herring 2024-03-07 22:32 ` Stefan Berger 2024-03-07 22:32 ` Stefan Berger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.