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

[Markus Armbruster]

Stefan Berger <address@hidden> writes:

> On 03/15/2013 03:36 AM, Markus Armbruster wrote:
>> I missed this one, because it wasn't cc'ed to QMP maintainers, the
>> subject mentions only command line, not QMP, and even the body talks
>> only about the human monitor command, not QMP.  Noticed it only when
>> git-pull touched qapi-schema.json.  Please try harder to help Luiz and
>> me keep track of QMP changes.
>
> I apologize for that.
>
>>
>> I gave the QMP interface and its documentation a look-over now.  It's
>> just a look-over, because passthrough requires a box with TPM enabled,
>> which I don't have handy, so I can't test anything.
>>
>> A few comments inline.
>>
>> Stefan Berger <address@hidden> writes:
>> +
>> address@hidden -tpmdev @var{backend} ,address@hidden [,@var{options}]
>> address@hidden -tpmdev
>> +Backend type must be:
>> +
>> +The specific backend type will determine the applicable options.
>> +The @code{-tpmdev} option requires a @code{-device} option.
>> You mean -tpmdev creates just a backend, so for a usable device you also
>> need to create a frontend with -device?
>
> Yes, just like any other device.

Okay.  Corey already posted a patch to clarify the documentation here.
Appreciated!

[...]
> +{
> +    if (qemu_opts_foreach(qemu_find_opts("tpmdev"),
> +                          tpm_init_tpmdev, NULL, 1) != 0) {
> +        return -1;
> +    }
> +
> +    atexit(tpm_cleanup);
>
>> Routine atexit() question: what happens when the program terminates
>> abnormally?  atext() callbacks don't run then.  Impact of not doing
>> cleanup on the system?
>
> 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?

Therefore, we try to clean it up manually.  Actual cleanup code is
tpm_passthrough_cancel_cmd().  Correct?

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?

> What other choices do we have? Do we need a signal handler that runs
> on SIGSEGV and provide a registration function for those functions
> that need to run on abnormal termination?

Not worth it.