Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 2 Aug 2023 16:13:04 -0700
From:   Jerry Snitselaar <jsnitsel@redhat.com>
To:     Jarkko Sakkinen <jarkko@kernel.org>,
        Stefan Berger <stefanb@linux.ibm.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        Daniil Stas <daniil.stas@posteo.net>,
        Mario Limonciello <mario.limonciello@amd.com>,
        James.Bottomley@hansenpartnership.com, Jason@zx2c4.com,
        linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
        regressions@leemhuis.info, stable@vger.kernel.org
Subject: Re: [PATCH 1/1] tpm: disable hwrng for fTPM on some AMD designs
Message-ID: <ejqnhdrhktfrhcb76bxx6y73hydjov34t7m4wrvzjc22gdtzts@p4g7yqxdnijn>
References: <65a1c307-826d-4ca3-0336-07a185684e5d@amd.com>
 <20230727195019.41abb48d@g14>
 <67eefe98-e6df-e152-3169-44329e22478d@amd.com>
 <20230727200527.4080c595@g14>
 <CAHk-=whqT0PxBazwfjWwoHQQFzZt50tV6Jfgq3iYceKMJtyuUg@mail.gmail.com>
 <CUGAV1Y993FB.1O2Q691015Z2C@seitikki>
 <CAHk-=whphk8Jp=NYmnm7Qv+vZ6ScYCz+rV8a2G1nD-AQY3z+mQ@mail.gmail.com>
 <CUHF67ZOFOTN.1UFE7Q1IFRQMX@suppilovahvero>
 <CAHk-=wgK0Z-LrJGExwG=e=oxjD93LJhY3jMmi_2O2_Pkjf8Tsg@mail.gmail.com>
 <CUHG1TB7IELF.PVXOXEXBGEPP@suppilovahvero>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CUHG1TB7IELF.PVXOXEXBGEPP@suppilovahvero>
Precedence: bulk

On Tue, Aug 01, 2023 at 10:09:58PM +0300, Jarkko Sakkinen wrote:
> On Tue Aug 1, 2023 at 9:42 PM EEST, Linus Torvalds wrote:
> > On Tue, 1 Aug 2023 at 11:28, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > >
> > > I would disable it inside tpm_crb driver, which is the driver used
> > > for fTPM's: they are identified by MSFT0101 ACPI identifier.
> > >
> > > I think the right scope is still AMD because we don't have such
> > > regressions with Intel fTPM.
> >
> > I'm ok with that.
> >
> > > I.e. I would move the helper I created inside tpm_crb driver, and
> > > a new flag, let's say "TPM_CHIP_FLAG_HWRNG_DISABLED", which tpm_crb
> > > sets before calling tpm_chip_register().
> > >
> > > Finally, tpm_add_hwrng() needs the following invariant:
> > >
> > >         if (chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
> > >                 return 0;
> > >
> > > How does this sound? I can refine this quickly from my first trial.
> >
> > Sounds fine.
> 
> Mario, it would be good if you could send a fix candidate but take my
> suggestion for a new TPM chip flag into account, while doing it. Please
> send it as a separate patch, not attachment to this thread.
> 
> I can test and ack it, if it looks reasonable.
> 
> > My only worry comes from my ignorance: do these fTPM devices *always*
> > end up being enumerated through CRB, or do they potentially look
> > "normal enough" that you can actually end up using them even without
> > having that CRB driver loaded?
> 
> I know that QEMU has TPM passthrough but I don't know how it behaves
> exactly.
> 

I just created a passthrough tpm device with a guest which it is using
the tis driver, while the host is using crb (and apparently one of the
amd devices that has an impacted fTPM). It looks like there is a
complete separation between the frontend and backends, with the front
end providing either a tis or crb interface to the guest, and then the
backend sending commands by writing to the passthrough device that was
given, such as /dev/tpm0, or an emulator such as swtpm. Stefan can
probably explain it much better than I.

Regards,
Jerry

> > Put another way: is the CRB driver the _only_ way they are visible, or
> > could some people hit on this through the TPM TIS interface if they
> > have CRB disabled?
> 
> I'm not aware of such implementations.
> 
> > I see, for example, that qemu ends up emulating the TIS layer, and it
> > might end up forwarding the TPM requests to something that is natively
> > CRB?
> >
> > But again: I don't know enough about CRB vs TIS, so the above may be a
> > stupid question.
> >
> >            Linus
> 
> I would focus exactly what is known not to work and disable exactly
> that.
> 
> If someone still wants to enable TPM on such hardware, we can later
> on add a kernel command-line flag to enforce hwrng. This ofc based
> on user feedback, not something I would add right now.
> 
> BR, Jarkko