Re: [Qemu-devel] [PATCH V27 1/7] Support for TPM command line options

[Markus Armbruster]

Stefan Berger <address@hidden> writes:

> On 03/18/2013 09:10 AM, Markus Armbruster wrote:
>>>
>>> In case the TPM is currently not operating on a command there will be
>>> no impact. In case the TPM is operating on a command, it will hold the
>>> thread inside /dev/tpm0 until the command has finished. The solution
>>> here is to write a byte into sysfs file to terminate the TPM from
>>> further executing the command and return; this code exists and is
>>> being invoked in all other cases than abnormal termination obviously.
>> Unlike other kernel resources such as file descriptors, this one isn't
>> tied to a process and cleaned up automatically on process termination.
>> Correct?
>
> No, it's also a file descriptor and will be cleaned up upon process
> termination.

Closing the file descriptor does *not* free kernel resources associated
with it?!?

>> Therefore, we try to clean it up manually.  Actual cleanup code is
>> tpm_passthrough_cancel_cmd().  Correct?
>
> No, tpm_passthrough_cancel_cmd() sends a byte to the TPM's sysfs entry
> for canceling an ongoing command. QEMU has a thread that sends the
> commands to the TPM via write() and gets the responses in read(). The
> issue being the thread is held inside the write() (!) until the
> command has completed inside the host TPM.

You mean write() blocks?

>                                            Inside the passthrough
> driver we set a flag while the command is executing and if a command
> is executing when for example QEMU is reset, then the sysfs entry is
> written to to get the thread out of the write() due to termination of
> the ongoing TPM command.

I can understand a need to abort the write() on guest reset, but why on
QEMU exit()?  Shouldn't process termination abort the write() just fine?

>                          Depending on which version of Fedora is run,
> the termination of the command may or may not get invoked.

May or may not get invoked on what condition?

>                                                            F14 for
> example does not seem to care about waiting until all  user-level
> processes have properly shut down and would reset  while F16 seems to
> wait for all processes to properly terminate.

F14 is more than nine months past it's shelf date.  Why should we
support it when its makers don't?

> The cancellation of an ongoing command may be initiated inside the
> guest as well and will also cause tpm_passthrough_cancel_cmd() to be
> invoked if a command is executed by the host TPM on behalf of the
> guest.

Passing through the guest's cancellation makes sense to me.

> There were/are(?) some issues with cancellation of ongoing
> commands. Different vendors seem to set different bits to indicate to
> the driver as to when a command has been canceled. The specs are not
> clear enough in that area. We fixed this for the TPMs that we have
> access (to wake up the thread inside the write()).

I'm not doubting the need to cancel commands.  I'm just trying to
understand why we do it on exit().

>> The only existing cleanup function tpm_passthrough_destroy() does a
>> whole lot more.  Just to make sure I understand what's going on:
>> everything but tpm_passthrough_cancel_cmd() is effectively a no-op
>> there, correct?
>
> It depends on what you mean with no-op. In tpm_passthrough_destroy() a
> possibly ongoing command is canceled, then the thread is signaled to
> terminate and join()ed. The rest is closing of file descriptors and
> freeing of memory.

Closing file descriptors and freeing memory on exit() is a no-op at
best, and a source of bugs otherwise.  The kernel already cleans these
up just fine.

Unless the thread does something interesting in response to its
termination signal, making it terminate is also a no-op.  I can't see
anything interesting happening, please correct me if I'm wrong.