On Mon, Feb 25, 2019 at 11:17:20AM -0800, Matthew Garrett wrote: > On Mon, Feb 25, 2019 at 7:36 AM James Bottomley > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > 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? > > The existing hypervisor drivers expose hypervisor-specific details. > This proposed driver provides an abstract interface that is usable by > other hypervisors. It allows building a VM that exposes TPM > functionality without requiring additional hardware emulation, > reducing the hypervisor attack surface. > > > > 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. > > Well, no - in general there's no need to have more than one virtio > driver for any /class/ of hardware. For various unfortunate accidents > of history we've ended up with multiple cases where we have > hypervisor-specific drivers. Using the more generic virtio > infrastructure reduces the need for that, since any hypervisor should > be able to implement the backend (eg, in this case it'd be very easy > to add support for this driver to qemu, which would allow the use of > TPMs without needing to enable a whole bunch of additional qemu > features). This isn't a discussion we'd be having if we'd pushed back > more strongly against hypervisor-specific solutions in the past. > > (While I work for Google, I'm not involved in crosvm development and > this shouldn't be interpreted as the position of anyone on that team) OK, this clears a lot of misconceptions thank you. The commit message itself fails to explain almost all of these details that you just wrote. The commit is also pretty badly formed so I grew a lot of initial mistrust, which I think is perfectly understandable. Putting links to random source files to a commit message is a sign of unwanted rush (i.e. time has not been taken to write down in plain English what has been done). Once the commit is in a better condition and some documentation how the stack is implemented from end to end we can better evaluate this. Some instructions how to compile and run crosvm would be also nice. /Jarkko
