On 1/8/2024 10:44 PM, Daniel P. Berrangé wrote:
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 .
I see. will implement in the next version.
With regards,
Daniel
