Received-SPF: pass (google.com: domain of linux-kernel+bounces-96241-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <71c151b2-b03a-49a7-87b9-fc902b0cf328@linux.ibm.com>
Date: Thu, 7 Mar 2024 16:15:01 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size
 with linux,sml-log
Content-Language: en-US
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
References: <20240306155511.974517-1-stefanb@linux.ibm.com>
 <20240306155511.974517-2-stefanb@linux.ibm.com> <87jzmenx2c.fsf@mail.lhotse>
 <768fc5f1-3919-477e-a8e6-16a7e8536add@linux.ibm.com>
 <20240307-cytoplasm-compare-6656aae737ac@spud>
From: Stefan Berger <stefanb@linux.ibm.com>
In-Reply-To: <20240307-cytoplasm-compare-6656aae737ac@spud>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0


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.