| Message ID | 1358524968-22297-9-git-send-email-stefanb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 01/18/2013 01:18 PM, Eric Blake wrote: > On 01/18/2013 09:02 AM, Stefan Berger wrote: >> Enable the passing of a file descriptor via fd=<..> to access the host's >> TPM device using the TPM passthrough driver. > Do we still need this, or is it sufficient to use path=/dev/fdset/nnn, > so that we are reusing common fd passing mechanisms without inventing > yet more variants? Well, it's similar to -netdev tap,fd=27,id=xyz > >> +++ b/qapi-schema.json >> @@ -3033,11 +3033,13 @@ >> # >> # @cancel_path: #optional Path to TPM backend device's cancel sysfs entry >> # >> +# @fd: #optional File descriptor for the TPM backend device >> +# >> # Since: 1.5.0 >> ## >> { 'type': 'TPMInfo', >> 'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str', >> - '*cancel_path': 'str' } } >> + '*cancel_path': 'str', '*fd' : 'int' } } > Besides, what integer value would you use for fd? Older commands that > support fd passing did so via 'int' on the command line, but via a 'str' > via QMP (the name associated with the fd when using 'getfd'), since QMP > does not have a way to expose _which_ fd is the right number from qemu's When libvirt forks, the child process inherits the file descriptors, among them those of the taps and /dev/tpm0. The subsequent execve keeps the file descriptor open. QEMU then reads the TPM file descriptor from the command line into above TPMInfo->fd. This also works with 'exec 100<>/dev/tpm0' via command line. Similar to the SELinux labeling of all the other file descriptors I also use the one for the TPM device for SELinux labeling. > perspective (it's not necessarily the same fd as in the management process). Hm. > I think this patch should just be dropped. > Regards, Stefan
On 01/18/2013 07:14 PM, Stefan Berger wrote: > On 01/18/2013 01:18 PM, Eric Blake wrote: >> On 01/18/2013 09:02 AM, Stefan Berger wrote: > When libvirt forks, the child process inherits the file descriptors, > among them those of the taps and /dev/tpm0. The subsequent execve > keeps the file descriptor open. QEMU then reads the TPM file > descriptor from the command line into above TPMInfo->fd. This also > works with 'exec 100<>/dev/tpm0' via command line. > Similar to the SELinux labeling of all the other file descriptors I > also use the one for the TPM device for SELinux labeling. > I have to correct this: The libvirt patches for this use path= on the command line and also apply the SELinux label on the path rather than the fd. So, this patch then adds file descriptor passing support to have equivalent functionality to other devices. Regards, Stefan
On 01/18/2013 05:55 PM, Stefan Berger wrote: > On 01/18/2013 07:14 PM, Stefan Berger wrote: >> On 01/18/2013 01:18 PM, Eric Blake wrote: >>> On 01/18/2013 09:02 AM, Stefan Berger wrote: >> When libvirt forks, the child process inherits the file descriptors, >> among them those of the taps and /dev/tpm0. The subsequent execve >> keeps the file descriptor open. QEMU then reads the TPM file >> descriptor from the command line into above TPMInfo->fd. This also >> works with 'exec 100<>/dev/tpm0' via command line. >> Similar to the SELinux labeling of all the other file descriptors I >> also use the one for the TPM device for SELinux labeling. >> > I have to correct this: The libvirt patches for this use path= on the > command line and also apply the SELinux label on the path rather than > the fd. So, this patch then adds file descriptor passing support to have > equivalent functionality to other devices. You _still_ don't need extra handling for fd passing; neither on the command line, nor in QMP. Remember, we added --add-fd to the command line, precisely so we could use: qemu -add-fd set=1,fd=100 -tpmdev passthrough,path=/dev/fdset/1 \ 100<>/dev/tpm0 See - by making fd passing universally accessible under pathname processing, we've made it easier to add new commands that don't have to special-case fd handling.
On 01/19/2013 10:31 AM, Eric Blake wrote: > On 01/18/2013 05:55 PM, Stefan Berger wrote: >> On 01/18/2013 07:14 PM, Stefan Berger wrote: >>> On 01/18/2013 01:18 PM, Eric Blake wrote: >>>> On 01/18/2013 09:02 AM, Stefan Berger wrote: >>> When libvirt forks, the child process inherits the file descriptors, >>> among them those of the taps and /dev/tpm0. The subsequent execve >>> keeps the file descriptor open. QEMU then reads the TPM file >>> descriptor from the command line into above TPMInfo->fd. This also >>> works with 'exec 100<>/dev/tpm0' via command line. >>> Similar to the SELinux labeling of all the other file descriptors I >>> also use the one for the TPM device for SELinux labeling. >>> >> I have to correct this: The libvirt patches for this use path= on the >> command line and also apply the SELinux label on the path rather than >> the fd. So, this patch then adds file descriptor passing support to have >> equivalent functionality to other devices. > You _still_ don't need extra handling for fd passing; neither on the > command line, nor in QMP. Remember, we added --add-fd to the command > line, precisely so we could use: > > qemu -add-fd set=1,fd=100 -tpmdev passthrough,path=/dev/fdset/1 \ > 100<>/dev/tpm0 This helps. /dev/fdset/1 is just a string and not a real device following what I see in the code. Then the problem seems to be solved by replacing open() with qemu_open() and we can drop this patch. Thanks for the hint. Obviously I don't follow all the developments in QEMU close enough... Regards, Stefan
diff --git a/hmp.c b/hmp.c index 8d63d03..ee6ab54 100644 --- a/hmp.c +++ b/hmp.c @@ -647,12 +647,17 @@ void hmp_info_tpm(Monitor *mon) TPMInfo *ti = info->value; monitor_printf(mon, " tpm%d: model=%s\n", c, ti->model); - monitor_printf(mon, " \\ %s: type=%s%s%s%s%s\n", + monitor_printf(mon, " \\ %s: type=%s%s%s%s%s", ti->id, ti->type, ti->has_path ? ",path=" : "", ti->has_path ? ti->path : "", ti->has_cancel_path ? ",cancel_path=" : "", ti->has_cancel_path ? ti->cancel_path : ""); + if (ti->has_fd) { + monitor_printf(mon, ",fd=%" PRId64, + ti->fd); + } + monitor_printf(mon, "\n"); c++; } qapi_free_TPMInfoList(info_list); diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c index 1b17c30..3843268 100644 --- a/hw/tpm_passthrough.c +++ b/hw/tpm_passthrough.c @@ -420,26 +420,54 @@ exit: static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) { const char *value; + struct stat statbuf; - value = qemu_opt_get(opts, "path"); - if (!value) { - value = TPM_PASSTHROUGH_DEFAULT_DEVICE; - } + value = qemu_opt_get(opts, "fd"); + if (value) { + if (qemu_opt_get(opts, "path")) { + error_report("fd= is invalid with path="); + goto err_exit; + } + + tb->s.tpm_pt->tpm_fd = qemu_parse_fd(value); + if (tb->s.tpm_pt->tpm_fd < 0) { + error_report("Illegal file descriptor for TPM device.\n"); + goto err_exit; + } + + tb->tpm_fd = &tb->s.tpm_pt->tpm_fd; + } else { + value = qemu_opt_get(opts, "path"); + if (!value) { + value = TPM_PASSTHROUGH_DEFAULT_DEVICE; + } - tb->s.tpm_pt->tpm_dev = g_strdup(value); + tb->s.tpm_pt->tpm_dev = g_strdup(value); - tb->path = g_strdup(tb->s.tpm_pt->tpm_dev); + tb->path = g_strdup(value); + + tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR); + if (tb->s.tpm_pt->tpm_fd < 0) { + error_report("Cannot access TPM device using '%s': %s\n", + tb->s.tpm_pt->tpm_dev, strerror(errno)); + goto err_free_parameters; + } + } + + if (fstat(tb->s.tpm_pt->tpm_fd, &statbuf) != 0) { + error_report("Cannot determine file descriptor type for TPM " + "device: %s", strerror(errno)); + goto err_close_tpmdev; + } - tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR); - if (tb->s.tpm_pt->tpm_fd < 0) { - error_report("Cannot access TPM device using '%s': %s\n", - tb->s.tpm_pt->tpm_dev, strerror(errno)); + /* only allow character devices for now */ + if (!S_ISCHR(statbuf.st_mode)) { + error_report("TPM file descriptor is not a character device"); goto err_free_parameters; } if (tpm_passthrough_test_tpmdev(tb->s.tpm_pt->tpm_fd)) { - error_report("'%s' is not a TPM device.\n", - tb->s.tpm_pt->tpm_dev); + error_report("Device is not a TPM.\n"); goto err_close_tpmdev; } @@ -456,6 +484,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) g_free(tb->s.tpm_pt->tpm_dev); tb->s.tpm_pt->tpm_dev = NULL; + err_exit: return 1; } diff --git a/qapi-schema.json b/qapi-schema.json index 82f753b..43b46e7 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3033,11 +3033,13 @@ # # @cancel_path: #optional Path to TPM backend device's cancel sysfs entry # +# @fd: #optional File descriptor for the TPM backend device +# # Since: 1.5.0 ## { 'type': 'TPMInfo', 'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str', - '*cancel_path': 'str' } } + '*cancel_path': 'str', '*fd' : 'int' } } ## # @query-tpm diff --git a/qemu-config.c b/qemu-config.c index 59f605a..e0e1c6f 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -706,6 +706,11 @@ static QemuOptsList qemu_tpmdev_opts = { .type = QEMU_OPT_STRING, .help = "Persistent storage for TPM state", }, + { + .name = "fd", + .type = QEMU_OPT_STRING, + .help = "File descriptor for accessing the TPM", + }, { /* end of list */ } }, }; diff --git a/qemu-options.hx b/qemu-options.hx index 225191f..bb7fe44 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2163,8 +2163,9 @@ DEFHEADING() DEFHEADING(TPM device options:) DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \ - "-tpmdev passthrough,id=id[,path=path]\n" - " use path to provide path to a character device; default is /dev/tpm0\n", + "-tpmdev passthrough,id=id[,path=path][,fd=h]\n" + " use path to provide path to a character device; default is /dev/tpm0\n" + " use fd to provide a file descriptor to a character device\n", QEMU_ARCH_ALL) STEXI @@ -2186,7 +2187,7 @@ Use ? to print all available TPM backend types. qemu -tpmdev ? @end example -@item -tpmdev passthrough, id=@var{id}, path=@var{path} +@item -tpmdev passthrough, id=@var{id}, path=@var{path}, fd=@var{h} (Linux-host only) Enable access to the host's TPM using the passthrough driver. @@ -2195,6 +2196,10 @@ driver. a Linux host this would be @code{/dev/tpm0}. @option{path} is optional and by default @code{/dev/tpm0} is used. +@option{fd} specifies the file descriptor of the host's TPM device. +@option{fd} and @option{path} are mutually exclusive. +@option{fd} is optional. + Some notes about using the host's TPM with the passthrough driver: The TPM device accessed by the passthrough driver must not be diff --git a/tpm.c b/tpm.c index 4730039..5cb9772 100644 --- a/tpm.c +++ b/tpm.c @@ -204,6 +204,10 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv) res->cancel_path = g_strdup(drv->cancel_path); res->has_cancel_path = true; } + if (drv->tpm_fd != NULL && *drv->tpm_fd >= 0) { + res->fd = *drv->tpm_fd; + res->has_fd = true; + } res->type = g_strdup(drv->ops->type); return res; diff --git a/tpm.h b/tpm.h index b4f7774..7d3dcac 100644 --- a/tpm.h +++ b/tpm.h @@ -25,6 +25,7 @@ typedef struct TPMBackend { const char *fe_model; char *path; char *cancel_path; + int *tpm_fd; const TPMDriverOps *ops; union {
Enable the passing of a file descriptor via fd=<..> to access the host's TPM device using the TPM passthrough driver. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- hmp.c | 7 ++++++- hw/tpm_passthrough.c | 53 ++++++++++++++++++++++++++++++++++++++++------------ qapi-schema.json | 4 +++- qemu-config.c | 5 +++++ qemu-options.hx | 11 ++++++++--- tpm.c | 4 ++++ tpm.h | 1 + 7 files changed, 68 insertions(+), 17 deletions(-)