On Wed, Oct 03, 2018 at 01:06:58PM +0200, Paolo Bonzini wrote:
> On 21/09/2018 10:22, Roman Kagan wrote:
> > +typedef struct HvSintStagedMesage {
> > + /* message content staged by hyperv_post_msg */
> > + struct hyperv_message msg;
> > + /* callback + data (r/o) to complete the processing in a BH */
> > + HvSintMsgCb cb;
> > + void *cb_data;
> > + /* message posting status filled by cpu_post_msg */
> > + int status;
> > + /* passing the buck: */
> > + enum {
> > + /* initial state */
> > + HV_STAGED_MSG_FREE,
> > + /*
> > + * hyperv_post_msg (e.g. in main loop) grabs the staged area (FREE
> > ->
> > + * BUSY), copies msg, and schedules cpu_post_msg on the assigned
> > cpu
> > + */
> > + HV_STAGED_MSG_BUSY,
> > + /*
> > + * cpu_post_msg (vcpu thread) tries to copy staged msg to msg slot,
> > + * notify the guest, records the status, marks the posting done
> > (BUSY
> > + * -> POSTED), and schedules sint_msg_bh BH
> > + */
> > + HV_STAGED_MSG_POSTED,
> > + /*
> > + * sint_msg_bh (BH) verifies that the posting is done, runs the
> > + * callback, and starts over (POSTED -> FREE)
> > + */
> > + } state;
> > +} HvSintStagedMesage;
>
> s/Mesage/Message/
:facepalm:
> > + if (atomic_read(&staged_msg->state) != HV_STAGED_MSG_POSTED) {
> > + /* status nor ready yet (spurious ack from guest?), ignore */
> > + return;
> > + }
> > +
>
> Can this actually happen? It seems scary...
I don't see off-hand what can stop the guest from wrmsr HV_X64_MSR_EOM
at an arbitrary moment, which AFAICS will result in this eventfd being
signaled. I haven't seen this codepath being taken in real life but
I think it provides enough protection for the internal state from being
screwed up by such an unexpected signal.
Thanks,
Roman.
