Message ID | 20230818095041.1973309-1-xiaoyao.li@intel.com (mailing list archive) |
---|---|
Headers | show |
Series | TDX QEMU support | expand |
On 8/18/2023 5:50 PM, Xiaoyao Li wrote: > From: Chenyi Qiang <chenyi.qiang@intel.com> > > To avoid no response from QGS server, setup a timer for the transaction. If > timeout, make it an error and interrupt guest. Define the threshold of time > to 30s at present, maybe change to other value if not appropriate. > > Extract the common cleanup code to make it more clear. > > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > target/i386/kvm/tdx.c | 151 ++++++++++++++++++++++++------------------ > 1 file changed, 85 insertions(+), 66 deletions(-) > > diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c > index 3cb2163a0335..fa658ce1f2e4 100644 > --- a/target/i386/kvm/tdx.c > +++ b/target/i386/kvm/tdx.c > @@ -1002,6 +1002,7 @@ struct tdx_get_quote_task { > struct tdx_get_quote_header hdr; > int event_notify_interrupt; > QIOChannelSocket *ioc; > + QEMUTimer timer; > }; > > struct x86_msi { > @@ -1084,13 +1085,48 @@ static void tdx_td_notify(struct tdx_get_quote_task *t) > } > } > > +static void tdx_getquote_task_cleanup(struct tdx_get_quote_task *t, bool outlen_overflow) > +{ > + MachineState *ms; > + TdxGuest *tdx; > + > + if (t->hdr.error_code != cpu_to_le64(TDX_VP_GET_QUOTE_SUCCESS) && !outlen_overflow) { > + t->hdr.out_len = cpu_to_le32(0); > + } > + > + /* Publish the response contents before marking this request completed. */ > + smp_wmb(); > + if (address_space_write( > + &address_space_memory, t->gpa, > + MEMTXATTRS_UNSPECIFIED, &t->hdr, sizeof(t->hdr)) != MEMTX_OK) { > + error_report("TDX: failed to update GetQuote header."); > + } > + tdx_td_notify(t); > + > + if (t->ioc->fd > 0) { > + qemu_set_fd_handler(t->ioc->fd, NULL, NULL, NULL); > + } > + qio_channel_close(QIO_CHANNEL(t->ioc), NULL); > + object_unref(OBJECT(t->ioc)); > + timer_del(&t->timer); Xiaoyao, I guess you missed a bug fix patch here as t->timer could be uninitialized and then timer_del() will cause segv. > + g_free(t->out_data); > + g_free(t); > + > + /* Maintain the number of in-flight requests. */ > + ms = MACHINE(qdev_get_machine()); > + tdx = TDX_GUEST(ms->cgs); > + qemu_mutex_lock(&tdx->lock); > + tdx->quote_generation_num--; > + qemu_mutex_unlock(&tdx->lock); > +} > +
On 8/24/2023 3:21 PM, Chenyi Qiang wrote: > > > On 8/18/2023 5:50 PM, Xiaoyao Li wrote: >> From: Chenyi Qiang <chenyi.qiang@intel.com> >> >> To avoid no response from QGS server, setup a timer for the transaction. If >> timeout, make it an error and interrupt guest. Define the threshold of time >> to 30s at present, maybe change to other value if not appropriate. >> >> Extract the common cleanup code to make it more clear. >> >> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> target/i386/kvm/tdx.c | 151 ++++++++++++++++++++++++------------------ >> 1 file changed, 85 insertions(+), 66 deletions(-) >> >> diff --git a/target/i386/kvm/tdx.c b/target/i386/kvm/tdx.c >> index 3cb2163a0335..fa658ce1f2e4 100644 >> --- a/target/i386/kvm/tdx.c >> +++ b/target/i386/kvm/tdx.c >> @@ -1002,6 +1002,7 @@ struct tdx_get_quote_task { >> struct tdx_get_quote_header hdr; >> int event_notify_interrupt; >> QIOChannelSocket *ioc; >> + QEMUTimer timer; >> }; >> >> struct x86_msi { >> @@ -1084,13 +1085,48 @@ static void tdx_td_notify(struct tdx_get_quote_task *t) >> } >> } >> >> +static void tdx_getquote_task_cleanup(struct tdx_get_quote_task *t, bool outlen_overflow) >> +{ >> + MachineState *ms; >> + TdxGuest *tdx; >> + >> + if (t->hdr.error_code != cpu_to_le64(TDX_VP_GET_QUOTE_SUCCESS) && !outlen_overflow) { >> + t->hdr.out_len = cpu_to_le32(0); >> + } >> + >> + /* Publish the response contents before marking this request completed. */ >> + smp_wmb(); >> + if (address_space_write( >> + &address_space_memory, t->gpa, >> + MEMTXATTRS_UNSPECIFIED, &t->hdr, sizeof(t->hdr)) != MEMTX_OK) { >> + error_report("TDX: failed to update GetQuote header."); >> + } >> + tdx_td_notify(t); >> + >> + if (t->ioc->fd > 0) { >> + qemu_set_fd_handler(t->ioc->fd, NULL, NULL, NULL); >> + } >> + qio_channel_close(QIO_CHANNEL(t->ioc), NULL); >> + object_unref(OBJECT(t->ioc)); >> + timer_del(&t->timer); > > Xiaoyao, I guess you missed a bug fix patch here as t->timer could be > uninitialized and then timer_del() will cause segv. Thanks for the reminding. I'll update this patch to include the fix. Thanks, -Xiaoyao >> + g_free(t->out_data); >> + g_free(t); >> + >> + /* Maintain the number of in-flight requests. */ >> + ms = MACHINE(qdev_get_machine()); >> + tdx = TDX_GUEST(ms->cgs); >> + qemu_mutex_lock(&tdx->lock); >> + tdx->quote_generation_num--; >> + qemu_mutex_unlock(&tdx->lock); >> +} >> + >