Messages in this thread |  | | Date | Tue, 12 Mar 2024 10:12:41 -0400 | | Subject | Re: [RFC PATCH v2 2/3] dt-bindings: tpm: Add linux,sml-log to ibm,vtpm.yaml | | From | Stefan Berger <> |
| |
On 3/12/24 07:11, Lukas Wunner wrote: > On Mon, Mar 11, 2024 at 09:20:29AM -0400, Stefan Berger wrote: >> Add linux,sml-log, which carries the firmware TPM log in a uint8-array, to >> the properties. Either this property is required or both linux,sml-base and >> linux,sml-size are required. Add a test case for verification. >> >> Fixes: 82003e0487fb ("Documentation: tpm: add the IBM Virtual TPM device tree binding documentation") > > The Fixes tag is confusing. The patch won't even apply cleanly to the > v4.10 commit referenced here as the conversion to yaml happened only > recently with v6.8.
Then that's as far back (6.8) as the series may be applied. I put the Fixes tag on the first appearance of sml-base/sml-size since for kexec this was never correct.
> > Why is the Fixes tag necessary in the first place? Same question for > the other patches in the series. This looks like feature work rather > than a fix. Not sure whether it satisfies the "obviously correct" > rule per Documentation/process/stable-kernel-rules.rst.
It is a fix for the interaction of the TPM firmware log with kexec. The sml-base buffer pointer was never protected across a kexec.
> > >> --- 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 > > I assume that either these two or the new "linux,sml-log" property > are (still) required? If so, a quick grep through the bindings > (e.g. auxdisplay/img,ascii-lcd.yaml) shows that the following > might work: > > required: > - ... > > oneOf: > - required: > - linux,sml-base > - required: > - linux,sml-log > You're right, they need to be here since examples could now omit sml-base or sml-log. I added them. Thanks.
> >> --- 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: >> + Content of firmware event log > > Please add one or two sentences of context so that readers don't > need to use git blame + git log to find out what this is for. > (Mention at least that the property may be used to pass the log > to a kexec kernel.)
Ok, will do:
Content of firmware event log embedded in device tree to be safely carried across a kexec soft reboot.
> > >> -# 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 > > Could you add "linux,sml-size: false" to "memory-region" as well > while at it for consistency?
Done. Thanks.
Stefan > > Thanks, > > Lukas
|  |