Re: [PATCH] tpm: Add driver for TPM over virtio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/22/19 1:50 PM, Jarkko Sakkinen wrote:
> On Fri, Feb 22, 2019 at 04:25:08PM -0500, Michael S. Tsirkin wrote:
>> On Fri, Feb 22, 2019 at 09:33:05PM +0200, Jarkko Sakkinen wrote:
>>> On Fri, Feb 22, 2019 at 09:31:56PM +0200, Jarkko Sakkinen wrote:
>>>> On Fri, Feb 22, 2019 at 10:23:02AM -0500, Michael S. Tsirkin wrote:
>>>>> On Fri, Feb 22, 2019 at 12:26:10PM +0200, Jarkko Sakkinen wrote:
>>>>>> On Thu, Feb 21, 2019 at 06:14:02PM -0800, David Tolnay wrote:
>>>>>>> Add a config TCG_VIRTIO_VTPM which enables a driver providing the guest
>>>>>>> kernel side of TPM over virtio.
>>>>>>>
>>>>>>> Use case: TPM support is needed for performing trusted work from within
>>>>>>> a virtual machine launched by Chrome OS.
>>>>>>>
>>>>>>> Tested inside crosvm, the Chrome OS virtual machine monitor. Crosvm's
>>>>>>> implementation of the virtio TPM device can be found in these two source
>>>>>>> files:
>>>>>>>
>>>>>>> - https://chromium.googlesource.com/chromiumos/platform/crosvm/+/18ce5713e6cb99c40aafec52b67c28ba12a44f31/devices/src/virtio/tpm.rs
>>>>>>> - https://chromium.googlesource.com/chromiumos/platform/crosvm/+/18ce5713e6cb99c40aafec52b67c28ba12a44f31/tpm2/src/lib.rs
>>>>>>
>>>>>> These files/links do not make sense for kernel testing. Please remove
>>>>>> them from the next version.
>>>>>
>>>>> To clarify generally for a virtio device we want
>>>>> - guest support
>>>>> - device support
>>>>> - spec
>>>>>
>>>>> If the device is implemented in qemu and guest in linux kernel,
>>>>> then there are lots of people familiar with these
>>>>> programming environments, so sometimes we merge
>>>>> guest and host code even if spec isn't written up at all.
>>>>>
>>>>> If you don't want to do that there's a small number of people who can
>>>>> properly review code, e.g. I don't think lots of people on this list are
>>>>> familiar with crosvm.  One way to address this would be to build a QEMU
>>>>> implementation. Another would be to write up a spec.  You can do both
>>>>> too :)
>>>>
>>>> I don't really understand your arguments.
>>>
>>> ... and I did your response total three times and did not find any
>>> causality of any sort from anything.
>>>
>>> /Jarkko
>>
>> Thanks for spending the time reading my response.  What was included in
>> it was a general suggestion for a virtio based driver to be acceptable
>> in upstream Linux.
>>
>> You pointed out that a pointer to a prototype implementation in Rust
>> isn't relevant. However, FYI just posting guest code and asking for it
>> to be merged alone won't work for a virtio driver either. I am merely
>> trying to speed things up instead of having the contributor repost with
>> a tweaked commit log just to immediately get another set of nacks.
> 
> I did not say anything about relevance of any implementation. I tried to
> mainly point out that looking at random source files implemented with a
> relatively alien language to most does not really make a case.
> 
> Things that would help to move this forward:
> 
> - Documentation of the stack, nothing spectacular but more like how it
>   works in practical terms.
> - Sufficiently easy ways to test the code.
> - Explain in the commit message why we want this.
> 
> /Jarkko


Thanks Jarkko, I respect all of the points you've raised and you are absolutely
correct to ask for a spec and/or spec-quality documentation, detailed testing
steps for exercising this driver in qemu or crosvm, and strong justification for
the new driver.

- Regarding spec, at this point we'll follow up with OASIS to claim a device
  number and pin down the protocol in sufficient detail to make it clear when an
  implementation is conformant. As I noted in the patch message, at the very
  least the virtio device number will need to be resolved before this driver
  could be accepted.

  On the Chrome OS side we're comfortable pursuing review of the spec and review
  of the driver in parallel. At what point in the spec process the driver can be
  accepted is up to you folks.

- I will make sure to include detailed testing steps with the next patch.

- I'm glad we are hashing out why this driver is necessary (or why it is not
  necessary, if it turns out that way) in this discussion. You are right that it
  may have been better originally sent as an RFC. Once I understand there to be
  some agreement, I will write that up for the next patch.

Thanks,
David



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux