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@xxxxxxxxxxxxx> 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.