| Message ID | 1510323112-2207-10-git-send-email-stefanb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: Extend TPM with state migration support (not 2.11) | expand |
On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > Introduce a lock and a condition to notify anyone waiting for the completion > of the execution of a TPM command by the backend (thread). The backend > uses the condition to signal anyone waiting for command completion. > We need to place the condition in two locations: one is invoked by the > backend thread, the other by the bottom half thread. > We will use the signaling to wait for command completion before VM > suspend. > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > --- > hw/tpm/tpm_tis.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c > index 035c6ef..86e9a92 100644 > --- a/hw/tpm/tpm_tis.c > +++ b/hw/tpm/tpm_tis.c > @@ -80,6 +80,9 @@ typedef struct TPMState { > TPMVersion be_tpm_version; > > size_t be_buffer_size; > + > + QemuMutex state_lock; > + QemuCond cmd_complete; Looks like the cond is unused in the following patches. > } TPMState; > > #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS) > @@ -405,6 +408,8 @@ static void tpm_tis_request_completed(TPMIf *ti) > } > } > > + qemu_mutex_lock(&s->state_lock); > + I get grumpy with multiple threads... Potentially, many threads will want to use TPMState: main thread, backend thread (hopefully not with the bh), migration thread (with the next patches...), and vcpu thread. It's hard to review which part of TPMState needs mux and which doesn't. Why do you lock only in this function? After checking s->cmd.selftest_done & modifying s->loc[locty].sts... > tpm_tis_sts_set(&s->loc[locty], > TPM_TIS_STS_VALID | TPM_TIS_STS_DATA_AVAILABLE); > s->loc[locty].state = TPM_TIS_STATE_COMPLETION; > @@ -419,6 +424,10 @@ static void tpm_tis_request_completed(TPMIf *ti) > > tpm_tis_raise_irq(s, locty, > TPM_TIS_INT_DATA_AVAILABLE | TPM_TIS_INT_STS_VALID); > + > + /* notify of completed command */ > + qemu_cond_signal(&s->cmd_complete); > + qemu_mutex_unlock(&s->state_lock); > } > > /* > @@ -1046,6 +1055,9 @@ static void tpm_tis_initfn(Object *obj) > memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops, > s, "tpm-tis-mmio", > TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT); > + > + qemu_mutex_init(&s->state_lock); > + qemu_cond_init(&s->cmd_complete); > } > > static void tpm_tis_class_init(ObjectClass *klass, void *data) > -- > 2.5.5 > >
On 12/22/2017 08:24 AM, Marc-André Lureau wrote: > On Fri, Nov 10, 2017 at 3:11 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> Introduce a lock and a condition to notify anyone waiting for the completion >> of the execution of a TPM command by the backend (thread). The backend >> uses the condition to signal anyone waiting for command completion. >> We need to place the condition in two locations: one is invoked by the >> backend thread, the other by the bottom half thread. >> We will use the signaling to wait for command completion before VM >> suspend. >> >> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >> --- >> hw/tpm/tpm_tis.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c >> index 035c6ef..86e9a92 100644 >> --- a/hw/tpm/tpm_tis.c >> +++ b/hw/tpm/tpm_tis.c >> @@ -80,6 +80,9 @@ typedef struct TPMState { >> TPMVersion be_tpm_version; >> >> size_t be_buffer_size; >> + >> + QemuMutex state_lock; >> + QemuCond cmd_complete; > Looks like the cond is unused in the following patches. You are right. I will drop this patch. It may have been needed before the refactoring you did. Stefan
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index 035c6ef..86e9a92 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -80,6 +80,9 @@ typedef struct TPMState { TPMVersion be_tpm_version; size_t be_buffer_size; + + QemuMutex state_lock; + QemuCond cmd_complete; } TPMState; #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS) @@ -405,6 +408,8 @@ static void tpm_tis_request_completed(TPMIf *ti) } } + qemu_mutex_lock(&s->state_lock); + tpm_tis_sts_set(&s->loc[locty], TPM_TIS_STS_VALID | TPM_TIS_STS_DATA_AVAILABLE); s->loc[locty].state = TPM_TIS_STATE_COMPLETION; @@ -419,6 +424,10 @@ static void tpm_tis_request_completed(TPMIf *ti) tpm_tis_raise_irq(s, locty, TPM_TIS_INT_DATA_AVAILABLE | TPM_TIS_INT_STS_VALID); + + /* notify of completed command */ + qemu_cond_signal(&s->cmd_complete); + qemu_mutex_unlock(&s->state_lock); } /* @@ -1046,6 +1055,9 @@ static void tpm_tis_initfn(Object *obj) memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops, s, "tpm-tis-mmio", TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT); + + qemu_mutex_init(&s->state_lock); + qemu_cond_init(&s->cmd_complete); } static void tpm_tis_class_init(ObjectClass *klass, void *data)
Introduce a lock and a condition to notify anyone waiting for the completion of the execution of a TPM command by the backend (thread). The backend uses the condition to signal anyone waiting for command completion. We need to place the condition in two locations: one is invoked by the backend thread, the other by the bottom half thread. We will use the signaling to wait for command completion before VM suspend. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- hw/tpm/tpm_tis.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)