On Fri, Nov 10, 2023 at 10:37 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > On Fri, Nov 10, 2023 at 1:51 AM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: > > > > On 10/11/2023 05:32, Yi Chou wrote: > > > The necessary fields to initialize the widevine related functions in > > > OP-TEE. > > > > > > Signed-off-by: Yi Chou <yich@xxxxxxxxxxxx> > > > Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx> > > > > > > --- > > > > Please provide version log and proper background for this patch. This > > was already sent to Linux kernel, so we should not spend time to figure > > it out. > > Sent out a new patch that has the version log and background for this patch. https://lore.kernel.org/all/20231113075249.3807225-1-yich@xxxxxxxxxxxx > > > There's no plan to add more optee stuff now. > > > This is it. > > > > > > dtschema/schemas/options/op-tee/widevine.yaml | 73 +++++++++++++++++++ > > > 1 file changed, 73 insertions(+) > > > create mode 100644 dtschema/schemas/options/op-tee/widevine.yaml > > > > > > diff --git a/dtschema/schemas/options/op-tee/widevine.yaml b/dtschema/schemas/options/op-tee/widevine.yaml > > > new file mode 100644 > > > index 0000000..925c214 > > > --- /dev/null > > > +++ b/dtschema/schemas/options/op-tee/widevine.yaml > > > > Missing vendor prefix - google, > > > > > @@ -0,0 +1,73 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/options/op-tee/widevine.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Google Widevine initialization parameters > > > > What is Widevine? You write it once "widevine", once "Widevine" but > > never explain what it is. > > > > > + > > > +maintainers: > > > + - Jeffrey Kardatzke <jkardatzke@xxxxxxxxxxxx> > > > + - Yi Chou <yich@xxxxxxxxxxxx> > > > + > > > +description: > > > + The necessary fields to initialize the widevine related functions in > > > + OP-TEE. This node does not represent a real device, but serves as a > > > + place for passing data between firmware and OP-TEE. > > > + > > > +properties: > > > + google,optee-hardware-unique-key: > > > + $ref: /schemas/types.yaml#/definitions/uint8-array > > > + maxItems: 32 > > > + description: | > > > + The hardware-unique key of the Widevine OP-TEE. It will be used > > > + to derive the secure storage key. > > > + For more information, please reference: > > > + https://optee.readthedocs.io/en/latest/architecture/porting_guidelines.html#hardware-unique-key > > Reading this, it says: "in the best case the HUK should never ever be > readable directly from software, not even from the secure side" > > Putting it in DT is certainly "readable directly from software". > > Also, wouldn't widevine be calling tee_otp_get_hw_unique_key(...)? So > if you do put this in DT, it should be consumed by optee rather than > widevine? > > I'd like to see an ack from Jens or other optee maintainer on this > before merging. Yes, it will be consumed by the OP-TEE. The OP-TEE side PR is this: https://github.com/OP-TEE/optee_os/pull/6379 There is a new flag in OP-TEE called "CFG_WIDEVINE_HUK", and it will let the OP-TEE consume the HUK in DT. And in our use case, it is not "readable directly from software", because the DT will be generated at the runtime in TF-A, and we will only pass it into the OP-TEE. https://github.com/OP-TEE/optee_os/pull/6379#discussion_r1368844405 > > > > Blank line > > > > > + google,tcg-tpm-auth-public-key: > > > > Missing tcg prefix as asked by Rob. > > > > > + $ref: /schemas/types.yaml#/definitions/uint8-array > > > + maxItems: 1024 > > > + description: | > > > + The TPM auth public key. Used to communicate the TPM from OP-TEE. > > > + The format of data should be TPM2B_PUBLIC. > > > + For more information, please reference the 12.2.5 section: > > > + https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf > > > > Blank line > > > > > > > + google,widevine-root-of-trust-ecc-p256: > > > + $ref: /schemas/types.yaml#/definitions/uint8-array > > > + maxItems: 32 > > > + description: | > > > + The Widevine root of trust secret. Used to sign the widevine > > > + request in OP-TEE. The value is an ECC NIST P-256 scalar. > > > + For more information, please reference the G.1.2 section: > > > + https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-186.pdf > > > > Blank line > > > > > > > +required: > > > + - google,optee-hardware-unique-key > > > + - google,tcg-tpm-auth-public-key > > > + - google,widevine-root-of-trust-ecc-p256 > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + options { > > > + widevine { > > > > Node names should be generic. See also an explanation and list of > > examples (not exhaustive) in DT specification: > > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > Yes, but 'options' nodes are for software components and named for the > component. So I think 'widevine' is okay here. > > However, I think the schema path/filename should match the DT path. So > either need to add 'optee' to the DT path or remove it from the schema > path. It's really up to the optee community to decide. They are the > ones who will be stuck with whatever is created here. > The previous discussion in the optee: https://github.com/OP-TEE/optee_os/pull/6418#pullrequestreview-1714399006 I believe they prefer the "/options/op-tee/widevine" path. > Rob Thanks, Yi