linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: David Tolnay <dtolnay@gmail.com>
Cc: Peter Huewe <peterhuewe@gmx.de>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	linux-integrity@vger.kernel.org,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	virtualization@lists.linux-foundation.org, dgreid@chromium.org,
	apronin@chromium.org
Subject: Re: [PATCH] tpm: Add driver for TPM over virtio
Date: Mon, 25 Feb 2019 07:36:09 -0800	[thread overview]
Message-ID: <1551108969.3226.26.camel@HansenPartnership.com> (raw)
In-Reply-To: <be2c8327-49d0-a1e4-cc22-38342ce06ed3@gmail.com>

On Sun, 2019-02-24 at 14:12 -0800, David Tolnay wrote:
> On 2/24/19 8:30 AM, James Bottomley wrote:
> > QEMU implements a virtual hardware emulation for discovery, but
> > once discovered all the packet communication is handed off to the
> > vTPM socket.
> > 
> > The virtual hardware emulation can be anything we have a driver
> > for. TIS is the simplest, which is why I think they used it.  TIS
> > is actually a simple interface specification, it supports discovery
> > over anything, but the discovery implemented in standard guest
> > drivers is over ACPI, OF and PNP.  If you want more esoteric
> > discovery methods, we also support i2c.  However, that latter is
> > really only for embedded.  I think QEMU chose TIS because it works
> > seamlessly on both Linux and Windows guests.
> > 
> > 
> > > All of this is what I would like to avoid by using a virtio
> > > driver.
> > 
> > How? Discovery is the part that you have to do, whether it's using
> > emulated physical mechanisms or virtual bus discovery.
> 
> What I mean is that we avoid the need for *TPM-specific virtual
> hardware emulation* for discovery by using a virtio driver.
> 
> We avoid implementing TIS or any other TPM-specific interface
> mechanism, and we avoid implementing ACPI or OF or PNP or I2C or any
> other additional bus necessitated by the existing set of Linux TPM
> drivers which we wouldn't otherwise need.
> 
> The virtio driver performs discovery via virtio, which crosvm
> implements already for all of its supported devices. This
> substantially reduces the amount of TPM-specific code compared to
> your suggestions, and lowers the barrier to entry for implementing
> TPM support in other hypervisors which I hope we agree is beneficial.

Well, that's somewhat misleading:  The reason we already have two
hypervisor specific drivers already is because every hypervisor has a
different  virtual discovery mechanism. You didn't find the other two
hypervisor drivers remotely useful, so why would another hypervisor
find yours useful?

> Turning this around a different way, suppose that there already was a
> virtio TPM driver in the kernel.

There already are two paravirt TPM drivers: xen-tpmfront and
tpm_ibmvtpm.

The reason we have so many is that every hypervisor implements a
different virtual bus mechanism.  So if we add this for you all we need
is an ESX driver to have the full complement; or at least for the four
main hypervisors, there are probably a huge number of minor ones, like
the parallels hypervisor, virtual box etc. ... by the time we're done
we'll have ten or so paravirt TPM drivers.

>  For me as a hypervisor implementer, what advantages do you see that
> would make me decide to implement TPM-specific virtual hardware
> emulation in the form of TIS rather than simply leveraging a virtio
> driver like for other virtual devices?

So your argument is that for every device we have in the Linux kernel,
we should have the N hypervisor paravirt variants for the same thing? 
I assure you that's not going to fly because paravirt drivers would
then outnumber real drivers by an order of magnitude.

> > If you want to make this more concrete: I once wrote a pure socsim
> > packet TPM driver:
> > 
> > https://patchwork.ozlabs.org/patch/712465/
> > 
> > Since you just point it at the network socket, it does no discovery
> > at all and works in any Linux environment that has net.  I actually
> > still use it because a socsim TPM is easier to debug from the
> > outside. However it was 230 lines.  Your device is 460 so that
> > means about half your driver is actually about discovery.
> 
> It looks like all the comments in the virtio driver account for the
> difference, not necessarily discovery.
> 
> But to put this in perspective, what we save is the 1000+ lines I see
> in QEMU dedicated to TIS. Without a virtio driver (or socsim, or
> something else that avoids TPM-specific hardware emulation for device
> discovery), QEMU and crosvm and other hypervisors need to reproduce a
> TIS implementation. Conceptually this is simple but it is still a
> quite substantial barrier compared to not needing any TPM-specific
> discovery.

Paravirt drivers are something we add when there's a pragmatic use
case.  Paravirt is not a panacea because it costs us in terms of
additional maintenance burden.  You also still need a receiver in the
hypervisor even for a paravirt driver.  We can argue about the amount
of code you need for the receiver, but without adding some code another
hypervisor can't make use of your paravirt driver.  And, of course, if
they use a different virtual bus implementation, as every hypervisor
seems to, it's quite an enormous amount of code to emulate your bus
implementation.

> > The only reasons I can see to use a virtual bus is either because
> > its way more efficient (the storage/network use case) or because
> > you've stripped down the hypervisor so far that it's incapable of
> > emulating any physical device (the firecracker use case).
> 
> Crosvm does fall under the Firecracker use case, I believe.

Well you just added USB emulation:

https://www.aboutchromebooks.com/news/project-crostini-usb-support-linux-chrome-os/

You didn't tell the kernel USB subsystem to add virtio USB drivers ...

What I've been fishing for for several emails is the pragmatic use case
... do you have one?

James


  parent reply	other threads:[~2019-02-25 15:36 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-22  2:14 [PATCH] tpm: Add driver for TPM over virtio David Tolnay
2019-02-22  5:51 ` Michael S. Tsirkin
2019-02-22 21:40   ` David Tolnay
2019-02-22 22:24     ` Michael S. Tsirkin
2019-02-23  1:23       ` David Tolnay
2019-02-25  9:58   ` Jarkko Sakkinen
2019-02-22 10:26 ` Jarkko Sakkinen
2019-02-22 15:23   ` Michael S. Tsirkin
2019-02-22 19:31     ` Jarkko Sakkinen
2019-02-22 19:33       ` Jarkko Sakkinen
2019-02-22 21:25         ` Michael S. Tsirkin
2019-02-22 21:50           ` Jarkko Sakkinen
2019-02-22 22:24             ` David Tolnay
2019-02-22 22:36               ` Jarkko Sakkinen
2019-02-22 23:05                 ` Michael S. Tsirkin
2019-02-24  9:33                   ` Jarkko Sakkinen
2019-02-22 20:55       ` Michael S. Tsirkin
2019-02-22 21:30         ` Jarkko Sakkinen
2019-02-22 10:30 ` Jarkko Sakkinen
2019-02-22 15:30 ` James Bottomley
2019-02-22 21:16   ` Michael S. Tsirkin
2019-02-22 21:31     ` Jason Gunthorpe
2019-02-22 21:59       ` Jarkko Sakkinen
2019-02-22 22:07         ` Michael S. Tsirkin
2019-02-22 22:14           ` Jarkko Sakkinen
2019-02-22 22:00   ` David Tolnay
2019-02-22 22:18     ` James Bottomley
2019-02-23  0:45       ` David Tolnay
2019-02-23  1:34         ` James Bottomley
2019-02-23  2:41           ` David Tolnay
2019-02-24 16:30             ` James Bottomley
2019-02-24 17:51               ` Jarkko Sakkinen
2019-02-24 22:12               ` David Tolnay
2019-02-25  9:55                 ` Jarkko Sakkinen
2019-02-25 15:36                 ` James Bottomley [this message]
2019-02-25 19:17                   ` Matthew Garrett
2019-02-25 19:54                     ` Jarkko Sakkinen
2019-02-25 20:20                     ` James Bottomley
2019-02-25 21:00                       ` Matthew Garrett
2019-02-25 21:02                         ` Matthew Garrett
2019-02-25 22:14                         ` James Bottomley
2019-02-25 22:24                           ` Matthew Garrett
2019-02-25 22:32                             ` James Bottomley
2019-02-25 22:43                               ` Matthew Garrett
2019-02-25 22:51                                 ` James Bottomley
2019-02-25 23:02                                   ` Matthew Garrett
2019-02-25 23:09                                     ` James Bottomley
2019-02-25 21:05                       ` Jarkko Sakkinen
2019-02-25 22:24                         ` James Bottomley

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=1551108969.3226.26.camel@HansenPartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=apronin@chromium.org \
    --cc=dgreid@chromium.org \
    --cc=dtolnay@gmail.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jasowang@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=linux-integrity@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=peterhuewe@gmx.de \
    --cc=virtualization@lists.linux-foundation.org \
    /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 NNTP newsgroup(s).