From: Yi Chou <yich@chromium.org>
To: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
jkardatzke@google.com, yich@google.com,
krzysztof.kozlowski+dt@linaro.org,
devicetree-spec@vger.kernel.org, chenyian@google.com,
sjg@chromium.org, jwerner@chromium.org,
jens.wiklander@linaro.org
Subject: Re: [PATCH] schemas: Add Google Widevine initialization parameters
Date: Tue, 28 Nov 2023 18:29:12 +0800 [thread overview]
Message-ID: <CABOkjxLVU+YJODXBU=x8SXtOAyW86rePEkrw_cETzRJbAc+etg@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqL9aU+aC+WWn-6Q+Z-jiV8guS0-JQw2DVPYefVeiYKw8A@mail.gmail.com>
On Fri, Nov 10, 2023 at 10:37 PM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Nov 10, 2023 at 1:51 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> 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@chromium.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > ---
> >
> > 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@chromium.org
> > > 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@chromium.org>
> > > + - Yi Chou <yich@chromium.org>
> > > +
> > > +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
next prev parent reply other threads:[~2023-11-28 10:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-10 4:32 [PATCH] schemas: Add Google Widevine initialization parameters Yi Chou
2023-11-10 7:50 ` Krzysztof Kozlowski
2023-11-10 14:36 ` Rob Herring
2023-11-28 10:29 ` Yi Chou [this message]
2023-11-10 14:27 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABOkjxLVU+YJODXBU=x8SXtOAyW86rePEkrw_cETzRJbAc+etg@mail.gmail.com' \
--to=yich@chromium.org \
--cc=chenyian@google.com \
--cc=devicetree-spec@vger.kernel.org \
--cc=jens.wiklander@linaro.org \
--cc=jkardatzke@google.com \
--cc=jwerner@chromium.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=robh@kernel.org \
--cc=sjg@chromium.org \
--cc=yich@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).