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

[Stefan Berger]

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.

     {
+        .name       = "query-tpm",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_tpm,
+    },
+
+    {
+        .name       = "query-tpm-models",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_tpm_models,
+    },
+
+    {
+        .name       = "query-tpm-types",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_tpm_types,
+    },
+
+    {
         .name       = "chardev-add",
         .args_type  = "id:s,backend:q",
         .mhandler.cmd_new = qmp_marshal_input_chardev_add,

> You imitated the bad examples that lack documentation instead the good
> ones that have it.  Please fix that in a followup patch.

Will do by sending it to the list and cc'ing you.

+{
+    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.

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?

    Stefan