[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 11/34] hyperv: add synic message delivery
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC PATCH 11/34] hyperv: add synic message delivery |
Date: |
Wed, 7 Feb 2018 11:58:08 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 06/02/2018 21:30, Roman Kagan wrote:
> +
> + HvSintMsgCb msg_cb;
> + void *msg_cb_data;
> + struct hyperv_message *msg;
> + /*
> + * the state of the message staged in .msg:
> + * 0 - the staging area is not in use (after init or message
> + * successfully delivered to guest)
> + * -EBUSY - the staging area is being used in vcpu thread
> + * -EAGAIN - delivery attempt failed due to slot being busy, retry
> + * -EXXXX - error
> + */
> + int msg_status;
> +
Access to these fields needs to be protected by a mutex (the refcount is
okay because it is only released in the bottom half). Please also add
comments regarding which fields are read-only, which are accessed under
BQL, etc.
Also, msg_status's handling of EBUSY/EAGAIN is a bit strange, like:
+ if (ret == -EBUSY) {
+ return -EAGAIN;
+ }
+ if (ret) {
+ return ret;
+ }
I wonder if it would be better to change -EBUSY to 1, or to split
msg_status into two fields, such as msg_status (filled by the vCPU
thread) and msg_state which can be HYPERV_MSG_STATE_{FREE,BUSY,POSTED}.
msg_status is only valid if the state is POSTED, and sint_msg_bh then
moves the state back to FREE.
Thanks,
Paolo
[Qemu-devel] [RFC PATCH 12/34] hyperv: add synic event flag signaling, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 13/34] hyperv: process SIGNAL_EVENT hypercall, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 14/34] hyperv: process POST_MESSAGE hypercall, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 16/34] hyperv: update copyright notices, Roman Kagan, 2018/02/06
[Qemu-devel] [RFC PATCH 15/34] hyperv_testdev: add SynIC message and event testmodes, Roman Kagan, 2018/02/06