On Fri, Dec 29, 2023 at 10:30:15AM +0800, Xiaoyao Li wrote:
> On 11/16/2023 1:58 AM, Daniel P. Berrangé wrote:
> > On Wed, Nov 15, 2023 at 02:15:01AM -0500, Xiaoyao Li wrote:
> > > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > >
> > > For GetQuote, delegate a request to Quote Generation Service.
> > > Add property "quote-generation-socket" to tdx-guest, whihc is a property
> > > of type SocketAddress to specify Quote Generation Service(QGS).
> > >
> > > On request, connect to the QGS, read request buffer from shared guest
> > > memory, send the request buffer to the server and store the response
> > > into shared guest memory and notify TD guest by interrupt.
> > >
> > > command line example:
> > > qemu-system-x86_64 \
> > > -object '{"qom-type":"tdx-guest","id":"tdx0","quote-generation-socket":{"type": "vsock", "cid":"2","port":"1234"}}' \
> > > -machine confidential-guest-support=tdx0
> > >
> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > > Codeveloped-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx>
> > > Signed-off-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx>
> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
> > > ---
> > > Changes in v3:
> > > - rename property "quote-generation-service" to "quote-generation-socket";
> > > - change the type of "quote-generation-socket" from str to
> > > SocketAddress;
> > > - squash next patch into this one;
> > > ---
> > > qapi/qom.json | 5 +-
> > > target/i386/kvm/tdx.c | 430 ++++++++++++++++++++++++++++++++++++++++++
> > > target/i386/kvm/tdx.h | 6 +
> > > 3 files changed, 440 insertions(+), 1 deletion(-)
> > >
> > > +static void tdx_handle_get_quote_connected(QIOTask *task, gpointer opaque)
> > > +{
> > > + struct tdx_get_quote_task *t = opaque;
> > > + Error *err = NULL;
> > > + char *in_data = NULL;
> > > + MachineState *ms;
> > > + TdxGuest *tdx;
> > > +
> > > + t->hdr.error_code = cpu_to_le64(TDX_VP_GET_QUOTE_ERROR);
> > > + if (qio_task_propagate_error(task, NULL)) {
> > > + t->hdr.error_code = cpu_to_le64(TDX_VP_GET_QUOTE_QGS_UNAVAILABLE);
> > > + goto error;
> > > + }
> > > +
> > > + in_data = g_malloc(le32_to_cpu(t->hdr.in_len));
> > > + if (!in_data) {
> > > + goto error;
> > > + }
> > > +
> > > + if (address_space_read(&address_space_memory, t->gpa + sizeof(t->hdr),
> > > + MEMTXATTRS_UNSPECIFIED, in_data,
> > > + le32_to_cpu(t->hdr.in_len)) != MEMTX_OK) {
> > > + goto error;
> > > + }
> > > +
> > > + qio_channel_set_blocking(QIO_CHANNEL(t->ioc), false, NULL);
> >
> > You've set the channel to non-blocking, but....
> >
> > > +
> > > + if (qio_channel_write_all(QIO_CHANNEL(t->ioc), in_data,
> > > + le32_to_cpu(t->hdr.in_len), &err) ||
> > > + err) {
> >
> > ...this method will block execution of this thread, by either
> > sleeping in poll() or doing a coroutine yield.
> >
> > I don't think this is in coroutine context, so presumably this
> > is just blocking. So what was the point in marking the channel
> > non-blocking ?
>
> Hi Dainel,
>
> First of all, I'm not good at socket or qio channel thing. Please correct me
> and teach me when I'm wrong.
>
> I'm not the author of this patch. My understanding is that, set it to
> non-blocking is for the qio_channel_write_all() to proceed immediately?
The '_all' suffixed methods are implemented such that they will
sleep in poll(), or a coroutine yield when seeing EAGAIN.
> If set non-blocking is not needed, I can remove it.
>
> > You are setting up a background watch to wait for the reply
> > so we don't block this thread, so you seem to want non-blocking
> > behaviour.
>
> Both sending and receiving are in a new thread created by
> qio_channel_socket_connect_async(). So I think both of then can be blocking
> and don't need to be in another background thread.
>
> what's your suggestion on it? Make both sending and receiving blocking or
> non-blocking?
I think the code /should/ be non-blocking, which would mean
using qio_channel_write, instead of qio_channel_write_all,
and using a .
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
