| Message ID | 20220216102500.692781-2-arilou@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | HyperV: Synthetic Debugging device | expand |
On 16/02/2022 11:24, Jon Doron wrote: > SynIc can be enabled regardless of the SControl mechanisim which can > register a GSI for a given SintRoute. > > This behaviour can achived by setting enabling SIMP and then the guest > will poll on the message slot. > > Once there is another message pending the host will set the message slot > with the pending flag. > When the guest polls from the message slot, in case the pending flag is > set it will write to the HV_X64_MSR_EOM indicating it has cleared the > slot and we can try and push our message again. > > Signed-off-by: Jon Doron <arilou@gmail.com> > --- > hw/hyperv/hyperv.c | 109 +++++++++++++++++++++++++++++++-------------- > 1 file changed, 76 insertions(+), 33 deletions(-) > > diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c > index cb1074f234..aaba6b4901 100644 > --- a/hw/hyperv/hyperv.c > +++ b/hw/hyperv/hyperv.c > @@ -27,13 +27,16 @@ struct SynICState { > > CPUState *cs; > > - bool enabled; > + bool sctl_enabled; > hwaddr msg_page_addr; > hwaddr event_page_addr; > MemoryRegion msg_page_mr; > MemoryRegion event_page_mr; > struct hyperv_message_page *msg_page; > struct hyperv_event_flags_page *event_page; > + > + QemuMutex sint_routes_mutex; > + QLIST_HEAD(, HvSintRoute) sint_routes; > }; > > #define TYPE_SYNIC "hyperv-synic" > @@ -51,11 +54,11 @@ static SynICState *get_synic(CPUState *cs) > return SYNIC(object_resolve_path_component(OBJECT(cs), "synic")); > } > > -static void synic_update(SynICState *synic, bool enable, > +static void synic_update(SynICState *synic, bool sctl_enable, > hwaddr msg_page_addr, hwaddr event_page_addr) > { > > - synic->enabled = enable; > + synic->sctl_enabled = sctl_enable; > if (synic->msg_page_addr != msg_page_addr) { > if (synic->msg_page_addr) { > memory_region_del_subregion(get_system_memory(), > @@ -80,7 +83,7 @@ static void synic_update(SynICState *synic, bool enable, > } > } > > -void hyperv_synic_update(CPUState *cs, bool enable, > +void hyperv_synic_update(CPUState *cs, bool sctl_enable, > hwaddr msg_page_addr, hwaddr event_page_addr) > { > SynICState *synic = get_synic(cs); > @@ -89,7 +92,7 @@ void hyperv_synic_update(CPUState *cs, bool enable, > return; > } > > - synic_update(synic, enable, msg_page_addr, event_page_addr); > + synic_update(synic, sctl_enable, msg_page_addr, event_page_addr); > } > > static void synic_realize(DeviceState *dev, Error **errp) > @@ -110,16 +113,20 @@ static void synic_realize(DeviceState *dev, Error **errp) > sizeof(*synic->event_page), &error_abort); > synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr); > synic->event_page = memory_region_get_ram_ptr(&synic->event_page_mr); > + qemu_mutex_init(&synic->sint_routes_mutex); > + QLIST_INIT(&synic->sint_routes); > > g_free(msgp_name); > g_free(eventp_name); > } > + > static void synic_reset(DeviceState *dev) > { > SynICState *synic = SYNIC(dev); > memset(synic->msg_page, 0, sizeof(*synic->msg_page)); > memset(synic->event_page, 0, sizeof(*synic->event_page)); > synic_update(synic, false, 0, 0); > + assert(QLIST_EMPTY(&synic->sint_routes)); > } > > static void synic_class_init(ObjectClass *klass, void *data) > @@ -214,6 +221,7 @@ struct HvSintRoute { > HvSintStagedMessage *staged_msg; > > unsigned refcount; > + QLIST_ENTRY(HvSintRoute) link; > }; > > static CPUState *hyperv_find_vcpu(uint32_t vp_index) > @@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, run_on_cpu_data data) > > assert(staged_msg->state == HV_STAGED_MSG_BUSY); > > - if (!synic->enabled || !synic->msg_page_addr) { > + if (!synic->msg_page_addr) { > staged_msg->status = -ENXIO; > goto posted; > } > @@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno) > if (eventno > HV_EVENT_FLAGS_COUNT) { > return -EINVAL; > } > - if (!synic->enabled || !synic->event_page_addr) { > + if (!synic->sctl_enabled || !synic->event_page_addr) { > return -ENXIO; > } > > @@ -364,11 +372,12 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno) > HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint, > HvSintMsgCb cb, void *cb_data) > { > - HvSintRoute *sint_route; > - EventNotifier *ack_notifier; > + HvSintRoute *sint_route = NULL; > + EventNotifier *ack_notifier = NULL; > int r, gsi; > CPUState *cs; > SynICState *synic; > + bool ack_event_initialized = false; > > cs = hyperv_find_vcpu(vp_index); > if (!cs) { > @@ -381,57 +390,77 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint, > } > > sint_route = g_new0(HvSintRoute, 1); > - r = event_notifier_init(&sint_route->sint_set_notifier, false); > - if (r) { > - goto err; > + if (!sint_route) { > + return NULL; > } > > + sint_route->synic = synic; > + sint_route->sint = sint; > + sint_route->refcount = 1; > > ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL; > if (ack_notifier) { > sint_route->staged_msg = g_new0(HvSintStagedMessage, 1); > + if (!sint_route->staged_msg) { > + goto cleanup_err_sint; > + } > sint_route->staged_msg->cb = cb; > sint_route->staged_msg->cb_data = cb_data; > > r = event_notifier_init(ack_notifier, false); > if (r) { > - goto err_sint_set_notifier; > + goto cleanup_err_sint; > } > - > event_notifier_set_handler(ack_notifier, sint_ack_handler); > + ack_event_initialized = true; > + } > + > + /* See if we are done or we need to setup a GSI for this SintRoute */ > + if (!synic->sctl_enabled) { > + goto cleanup; > + } > + > + /* We need to setup a GSI for this SintRoute */ > + r = event_notifier_init(&sint_route->sint_set_notifier, false); > + if (r) { > + goto cleanup_err_sint; > } > > gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint); > if (gsi < 0) { > - goto err_gsi; > + goto cleanup_err_sint_notifier; > } > > r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, > &sint_route->sint_set_notifier, > ack_notifier, gsi); > if (r) { > - goto err_irqfd; > + goto cleanup_err_irqfd; > } > sint_route->gsi = gsi; > - sint_route->synic = synic; > - sint_route->sint = sint; > - sint_route->refcount = 1; > - > +cleanup: > + qemu_mutex_lock(&synic->sint_routes_mutex); > + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link); > + qemu_mutex_unlock(&synic->sint_routes_mutex); > return sint_route; > > -err_irqfd: > +cleanup_err_irqfd: > kvm_irqchip_release_virq(kvm_state, gsi); > -err_gsi: > + > +cleanup_err_sint_notifier: > + event_notifier_cleanup(&sint_route->sint_set_notifier); > + > +cleanup_err_sint: > if (ack_notifier) { > - event_notifier_set_handler(ack_notifier, NULL); > - event_notifier_cleanup(ack_notifier); > + if (ack_event_initialized) { > + event_notifier_set_handler(ack_notifier, NULL); > + event_notifier_cleanup(ack_notifier); > + } > + > g_free(sint_route->staged_msg); > } > -err_sint_set_notifier: > - event_notifier_cleanup(&sint_route->sint_set_notifier); > -err: > - g_free(sint_route); > > + g_free(sint_route); > return NULL; > } > > @@ -442,6 +471,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route) > > void hyperv_sint_route_unref(HvSintRoute *sint_route) > { > + SynICState *synic; > + > if (!sint_route) { > return; > } > @@ -452,21 +483,33 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route) > return; > } > > - kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, > - &sint_route->sint_set_notifier, > - sint_route->gsi); > - kvm_irqchip_release_virq(kvm_state, sint_route->gsi); > + synic = sint_route->synic; > + qemu_mutex_lock(&synic->sint_routes_mutex); > + QLIST_REMOVE_RCU(sint_route, link); > + qemu_mutex_unlock(&synic->sint_routes_mutex); > + > + if (sint_route->gsi) { > + kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, > + &sint_route->sint_set_notifier, > + sint_route->gsi); > + kvm_irqchip_release_virq(kvm_state, sint_route->gsi); > + event_notifier_cleanup(&sint_route->sint_set_notifier); > + } > + > if (sint_route->staged_msg) { > event_notifier_set_handler(&sint_route->sint_ack_notifier, NULL); > event_notifier_cleanup(&sint_route->sint_ack_notifier); > g_free(sint_route->staged_msg); > } > - event_notifier_cleanup(&sint_route->sint_set_notifier); > g_free(sint_route); > } > > int hyperv_sint_route_set_sint(HvSintRoute *sint_route) > { > + if (!sint_route->gsi) { > + return 0; > + } > + > return event_notifier_set(&sint_route->sint_set_notifier); > } > > Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Thanks! is there an estimate when will this patchset be merged? On Thu, Feb 24, 2022, 18:36 Emanuele Giuseppe Esposito <eesposit@redhat.com> wrote: > > > On 16/02/2022 11:24, Jon Doron wrote: > > SynIc can be enabled regardless of the SControl mechanisim which can > > register a GSI for a given SintRoute. > > > > This behaviour can achived by setting enabling SIMP and then the guest > > will poll on the message slot. > > > > Once there is another message pending the host will set the message slot > > with the pending flag. > > When the guest polls from the message slot, in case the pending flag is > > set it will write to the HV_X64_MSR_EOM indicating it has cleared the > > slot and we can try and push our message again. > > > > Signed-off-by: Jon Doron <arilou@gmail.com> > > --- > > hw/hyperv/hyperv.c | 109 +++++++++++++++++++++++++++++++-------------- > > 1 file changed, 76 insertions(+), 33 deletions(-) > > > > diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c > > index cb1074f234..aaba6b4901 100644 > > --- a/hw/hyperv/hyperv.c > > +++ b/hw/hyperv/hyperv.c > > @@ -27,13 +27,16 @@ struct SynICState { > > > > CPUState *cs; > > > > - bool enabled; > > + bool sctl_enabled; > > hwaddr msg_page_addr; > > hwaddr event_page_addr; > > MemoryRegion msg_page_mr; > > MemoryRegion event_page_mr; > > struct hyperv_message_page *msg_page; > > struct hyperv_event_flags_page *event_page; > > + > > + QemuMutex sint_routes_mutex; > > + QLIST_HEAD(, HvSintRoute) sint_routes; > > }; > > > > #define TYPE_SYNIC "hyperv-synic" > > @@ -51,11 +54,11 @@ static SynICState *get_synic(CPUState *cs) > > return SYNIC(object_resolve_path_component(OBJECT(cs), "synic")); > > } > > > > -static void synic_update(SynICState *synic, bool enable, > > +static void synic_update(SynICState *synic, bool sctl_enable, > > hwaddr msg_page_addr, hwaddr event_page_addr) > > { > > > > - synic->enabled = enable; > > + synic->sctl_enabled = sctl_enable; > > if (synic->msg_page_addr != msg_page_addr) { > > if (synic->msg_page_addr) { > > memory_region_del_subregion(get_system_memory(), > > @@ -80,7 +83,7 @@ static void synic_update(SynICState *synic, bool > enable, > > } > > } > > > > -void hyperv_synic_update(CPUState *cs, bool enable, > > +void hyperv_synic_update(CPUState *cs, bool sctl_enable, > > hwaddr msg_page_addr, hwaddr event_page_addr) > > { > > SynICState *synic = get_synic(cs); > > @@ -89,7 +92,7 @@ void hyperv_synic_update(CPUState *cs, bool enable, > > return; > > } > > > > - synic_update(synic, enable, msg_page_addr, event_page_addr); > > + synic_update(synic, sctl_enable, msg_page_addr, event_page_addr); > > } > > > > static void synic_realize(DeviceState *dev, Error **errp) > > @@ -110,16 +113,20 @@ static void synic_realize(DeviceState *dev, Error > **errp) > > sizeof(*synic->event_page), &error_abort); > > synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr); > > synic->event_page = > memory_region_get_ram_ptr(&synic->event_page_mr); > > + qemu_mutex_init(&synic->sint_routes_mutex); > > + QLIST_INIT(&synic->sint_routes); > > > > g_free(msgp_name); > > g_free(eventp_name); > > } > > + > > static void synic_reset(DeviceState *dev) > > { > > SynICState *synic = SYNIC(dev); > > memset(synic->msg_page, 0, sizeof(*synic->msg_page)); > > memset(synic->event_page, 0, sizeof(*synic->event_page)); > > synic_update(synic, false, 0, 0); > > + assert(QLIST_EMPTY(&synic->sint_routes)); > > } > > > > static void synic_class_init(ObjectClass *klass, void *data) > > @@ -214,6 +221,7 @@ struct HvSintRoute { > > HvSintStagedMessage *staged_msg; > > > > unsigned refcount; > > + QLIST_ENTRY(HvSintRoute) link; > > }; > > > > static CPUState *hyperv_find_vcpu(uint32_t vp_index) > > @@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, > run_on_cpu_data data) > > > > assert(staged_msg->state == HV_STAGED_MSG_BUSY); > > > > - if (!synic->enabled || !synic->msg_page_addr) { > > + if (!synic->msg_page_addr) { > > staged_msg->status = -ENXIO; > > goto posted; > > } > > @@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, > unsigned eventno) > > if (eventno > HV_EVENT_FLAGS_COUNT) { > > return -EINVAL; > > } > > - if (!synic->enabled || !synic->event_page_addr) { > > + if (!synic->sctl_enabled || !synic->event_page_addr) { > > return -ENXIO; > > } > > > > @@ -364,11 +372,12 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, > unsigned eventno) > > HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint, > > HvSintMsgCb cb, void *cb_data) > > { > > - HvSintRoute *sint_route; > > - EventNotifier *ack_notifier; > > + HvSintRoute *sint_route = NULL; > > + EventNotifier *ack_notifier = NULL; > > int r, gsi; > > CPUState *cs; > > SynICState *synic; > > + bool ack_event_initialized = false; > > > > cs = hyperv_find_vcpu(vp_index); > > if (!cs) { > > @@ -381,57 +390,77 @@ HvSintRoute *hyperv_sint_route_new(uint32_t > vp_index, uint32_t sint, > > } > > > > sint_route = g_new0(HvSintRoute, 1); > > - r = event_notifier_init(&sint_route->sint_set_notifier, false); > > - if (r) { > > - goto err; > > + if (!sint_route) { > > + return NULL; > > } > > > > + sint_route->synic = synic; > > + sint_route->sint = sint; > > + sint_route->refcount = 1; > > > > ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL; > > if (ack_notifier) { > > sint_route->staged_msg = g_new0(HvSintStagedMessage, 1); > > + if (!sint_route->staged_msg) { > > + goto cleanup_err_sint; > > + } > > sint_route->staged_msg->cb = cb; > > sint_route->staged_msg->cb_data = cb_data; > > > > r = event_notifier_init(ack_notifier, false); > > if (r) { > > - goto err_sint_set_notifier; > > + goto cleanup_err_sint; > > } > > - > > event_notifier_set_handler(ack_notifier, sint_ack_handler); > > + ack_event_initialized = true; > > + } > > + > > + /* See if we are done or we need to setup a GSI for this SintRoute > */ > > + if (!synic->sctl_enabled) { > > + goto cleanup; > > + } > > + > > + /* We need to setup a GSI for this SintRoute */ > > + r = event_notifier_init(&sint_route->sint_set_notifier, false); > > + if (r) { > > + goto cleanup_err_sint; > > } > > > > gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint); > > if (gsi < 0) { > > - goto err_gsi; > > + goto cleanup_err_sint_notifier; > > } > > > > r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, > > > &sint_route->sint_set_notifier, > > ack_notifier, gsi); > > if (r) { > > - goto err_irqfd; > > + goto cleanup_err_irqfd; > > } > > sint_route->gsi = gsi; > > - sint_route->synic = synic; > > - sint_route->sint = sint; > > - sint_route->refcount = 1; > > - > > +cleanup: > > + qemu_mutex_lock(&synic->sint_routes_mutex); > > + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link); > > + qemu_mutex_unlock(&synic->sint_routes_mutex); > > return sint_route; > > > > -err_irqfd: > > +cleanup_err_irqfd: > > kvm_irqchip_release_virq(kvm_state, gsi); > > -err_gsi: > > + > > +cleanup_err_sint_notifier: > > + event_notifier_cleanup(&sint_route->sint_set_notifier); > > + > > +cleanup_err_sint: > > if (ack_notifier) { > > - event_notifier_set_handler(ack_notifier, NULL); > > - event_notifier_cleanup(ack_notifier); > > + if (ack_event_initialized) { > > + event_notifier_set_handler(ack_notifier, NULL); > > + event_notifier_cleanup(ack_notifier); > > + } > > + > > g_free(sint_route->staged_msg); > > } > > -err_sint_set_notifier: > > - event_notifier_cleanup(&sint_route->sint_set_notifier); > > -err: > > - g_free(sint_route); > > > > + g_free(sint_route); > > return NULL; > > } > > > > @@ -442,6 +471,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route) > > > > void hyperv_sint_route_unref(HvSintRoute *sint_route) > > { > > + SynICState *synic; > > + > > if (!sint_route) { > > return; > > } > > @@ -452,21 +483,33 @@ void hyperv_sint_route_unref(HvSintRoute > *sint_route) > > return; > > } > > > > - kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, > > - > &sint_route->sint_set_notifier, > > - sint_route->gsi); > > - kvm_irqchip_release_virq(kvm_state, sint_route->gsi); > > + synic = sint_route->synic; > > + qemu_mutex_lock(&synic->sint_routes_mutex); > > + QLIST_REMOVE_RCU(sint_route, link); > > + qemu_mutex_unlock(&synic->sint_routes_mutex); > > + > > + if (sint_route->gsi) { > > + kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, > > + > &sint_route->sint_set_notifier, > > + sint_route->gsi); > > + kvm_irqchip_release_virq(kvm_state, sint_route->gsi); > > + event_notifier_cleanup(&sint_route->sint_set_notifier); > > + } > > + > > if (sint_route->staged_msg) { > > event_notifier_set_handler(&sint_route->sint_ack_notifier, > NULL); > > event_notifier_cleanup(&sint_route->sint_ack_notifier); > > g_free(sint_route->staged_msg); > > } > > - event_notifier_cleanup(&sint_route->sint_set_notifier); > > g_free(sint_route); > > } > > > > int hyperv_sint_route_set_sint(HvSintRoute *sint_route) > > { > > + if (!sint_route->gsi) { > > + return 0; > > + } > > + > > return event_notifier_set(&sint_route->sint_set_notifier); > > } > > > > > > Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > >
On 2/16/22 11:24, Jon Doron wrote: > + qemu_mutex_lock(&synic->sint_routes_mutex); > + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link); > + qemu_mutex_unlock(&synic->sint_routes_mutex); Hi, I don't see any access to sint_routes outside hyperv_sint_route_new and hyperv_sint_route_unref. Am I missing something or is this just for debugging? If so, using the _RCU functions is not necessary. Otherwise everything looks good, thanks! Paolo
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c index cb1074f234..aaba6b4901 100644 --- a/hw/hyperv/hyperv.c +++ b/hw/hyperv/hyperv.c @@ -27,13 +27,16 @@ struct SynICState { CPUState *cs; - bool enabled; + bool sctl_enabled; hwaddr msg_page_addr; hwaddr event_page_addr; MemoryRegion msg_page_mr; MemoryRegion event_page_mr; struct hyperv_message_page *msg_page; struct hyperv_event_flags_page *event_page; + + QemuMutex sint_routes_mutex; + QLIST_HEAD(, HvSintRoute) sint_routes; }; #define TYPE_SYNIC "hyperv-synic" @@ -51,11 +54,11 @@ static SynICState *get_synic(CPUState *cs) return SYNIC(object_resolve_path_component(OBJECT(cs), "synic")); } -static void synic_update(SynICState *synic, bool enable, +static void synic_update(SynICState *synic, bool sctl_enable, hwaddr msg_page_addr, hwaddr event_page_addr) { - synic->enabled = enable; + synic->sctl_enabled = sctl_enable; if (synic->msg_page_addr != msg_page_addr) { if (synic->msg_page_addr) { memory_region_del_subregion(get_system_memory(), @@ -80,7 +83,7 @@ static void synic_update(SynICState *synic, bool enable, } } -void hyperv_synic_update(CPUState *cs, bool enable, +void hyperv_synic_update(CPUState *cs, bool sctl_enable, hwaddr msg_page_addr, hwaddr event_page_addr) { SynICState *synic = get_synic(cs); @@ -89,7 +92,7 @@ void hyperv_synic_update(CPUState *cs, bool enable, return; } - synic_update(synic, enable, msg_page_addr, event_page_addr); + synic_update(synic, sctl_enable, msg_page_addr, event_page_addr); } static void synic_realize(DeviceState *dev, Error **errp) @@ -110,16 +113,20 @@ static void synic_realize(DeviceState *dev, Error **errp) sizeof(*synic->event_page), &error_abort); synic->msg_page = memory_region_get_ram_ptr(&synic->msg_page_mr); synic->event_page = memory_region_get_ram_ptr(&synic->event_page_mr); + qemu_mutex_init(&synic->sint_routes_mutex); + QLIST_INIT(&synic->sint_routes); g_free(msgp_name); g_free(eventp_name); } + static void synic_reset(DeviceState *dev) { SynICState *synic = SYNIC(dev); memset(synic->msg_page, 0, sizeof(*synic->msg_page)); memset(synic->event_page, 0, sizeof(*synic->event_page)); synic_update(synic, false, 0, 0); + assert(QLIST_EMPTY(&synic->sint_routes)); } static void synic_class_init(ObjectClass *klass, void *data) @@ -214,6 +221,7 @@ struct HvSintRoute { HvSintStagedMessage *staged_msg; unsigned refcount; + QLIST_ENTRY(HvSintRoute) link; }; static CPUState *hyperv_find_vcpu(uint32_t vp_index) @@ -259,7 +267,7 @@ static void cpu_post_msg(CPUState *cs, run_on_cpu_data data) assert(staged_msg->state == HV_STAGED_MSG_BUSY); - if (!synic->enabled || !synic->msg_page_addr) { + if (!synic->msg_page_addr) { staged_msg->status = -ENXIO; goto posted; } @@ -343,7 +351,7 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno) if (eventno > HV_EVENT_FLAGS_COUNT) { return -EINVAL; } - if (!synic->enabled || !synic->event_page_addr) { + if (!synic->sctl_enabled || !synic->event_page_addr) { return -ENXIO; } @@ -364,11 +372,12 @@ int hyperv_set_event_flag(HvSintRoute *sint_route, unsigned eventno) HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint, HvSintMsgCb cb, void *cb_data) { - HvSintRoute *sint_route; - EventNotifier *ack_notifier; + HvSintRoute *sint_route = NULL; + EventNotifier *ack_notifier = NULL; int r, gsi; CPUState *cs; SynICState *synic; + bool ack_event_initialized = false; cs = hyperv_find_vcpu(vp_index); if (!cs) { @@ -381,57 +390,77 @@ HvSintRoute *hyperv_sint_route_new(uint32_t vp_index, uint32_t sint, } sint_route = g_new0(HvSintRoute, 1); - r = event_notifier_init(&sint_route->sint_set_notifier, false); - if (r) { - goto err; + if (!sint_route) { + return NULL; } + sint_route->synic = synic; + sint_route->sint = sint; + sint_route->refcount = 1; ack_notifier = cb ? &sint_route->sint_ack_notifier : NULL; if (ack_notifier) { sint_route->staged_msg = g_new0(HvSintStagedMessage, 1); + if (!sint_route->staged_msg) { + goto cleanup_err_sint; + } sint_route->staged_msg->cb = cb; sint_route->staged_msg->cb_data = cb_data; r = event_notifier_init(ack_notifier, false); if (r) { - goto err_sint_set_notifier; + goto cleanup_err_sint; } - event_notifier_set_handler(ack_notifier, sint_ack_handler); + ack_event_initialized = true; + } + + /* See if we are done or we need to setup a GSI for this SintRoute */ + if (!synic->sctl_enabled) { + goto cleanup; + } + + /* We need to setup a GSI for this SintRoute */ + r = event_notifier_init(&sint_route->sint_set_notifier, false); + if (r) { + goto cleanup_err_sint; } gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint); if (gsi < 0) { - goto err_gsi; + goto cleanup_err_sint_notifier; } r = kvm_irqchip_add_irqfd_notifier_gsi(kvm_state, &sint_route->sint_set_notifier, ack_notifier, gsi); if (r) { - goto err_irqfd; + goto cleanup_err_irqfd; } sint_route->gsi = gsi; - sint_route->synic = synic; - sint_route->sint = sint; - sint_route->refcount = 1; - +cleanup: + qemu_mutex_lock(&synic->sint_routes_mutex); + QLIST_INSERT_HEAD_RCU(&synic->sint_routes, sint_route, link); + qemu_mutex_unlock(&synic->sint_routes_mutex); return sint_route; -err_irqfd: +cleanup_err_irqfd: kvm_irqchip_release_virq(kvm_state, gsi); -err_gsi: + +cleanup_err_sint_notifier: + event_notifier_cleanup(&sint_route->sint_set_notifier); + +cleanup_err_sint: if (ack_notifier) { - event_notifier_set_handler(ack_notifier, NULL); - event_notifier_cleanup(ack_notifier); + if (ack_event_initialized) { + event_notifier_set_handler(ack_notifier, NULL); + event_notifier_cleanup(ack_notifier); + } + g_free(sint_route->staged_msg); } -err_sint_set_notifier: - event_notifier_cleanup(&sint_route->sint_set_notifier); -err: - g_free(sint_route); + g_free(sint_route); return NULL; } @@ -442,6 +471,8 @@ void hyperv_sint_route_ref(HvSintRoute *sint_route) void hyperv_sint_route_unref(HvSintRoute *sint_route) { + SynICState *synic; + if (!sint_route) { return; } @@ -452,21 +483,33 @@ void hyperv_sint_route_unref(HvSintRoute *sint_route) return; } - kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, - &sint_route->sint_set_notifier, - sint_route->gsi); - kvm_irqchip_release_virq(kvm_state, sint_route->gsi); + synic = sint_route->synic; + qemu_mutex_lock(&synic->sint_routes_mutex); + QLIST_REMOVE_RCU(sint_route, link); + qemu_mutex_unlock(&synic->sint_routes_mutex); + + if (sint_route->gsi) { + kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, + &sint_route->sint_set_notifier, + sint_route->gsi); + kvm_irqchip_release_virq(kvm_state, sint_route->gsi); + event_notifier_cleanup(&sint_route->sint_set_notifier); + } + if (sint_route->staged_msg) { event_notifier_set_handler(&sint_route->sint_ack_notifier, NULL); event_notifier_cleanup(&sint_route->sint_ack_notifier); g_free(sint_route->staged_msg); } - event_notifier_cleanup(&sint_route->sint_set_notifier); g_free(sint_route); } int hyperv_sint_route_set_sint(HvSintRoute *sint_route) { + if (!sint_route->gsi) { + return 0; + } + return event_notifier_set(&sint_route->sint_set_notifier); }
SynIc can be enabled regardless of the SControl mechanisim which can register a GSI for a given SintRoute. This behaviour can achived by setting enabling SIMP and then the guest will poll on the message slot. Once there is another message pending the host will set the message slot with the pending flag. When the guest polls from the message slot, in case the pending flag is set it will write to the HV_X64_MSR_EOM indicating it has cleared the slot and we can try and push our message again. Signed-off-by: Jon Doron <arilou@gmail.com> --- hw/hyperv/hyperv.c | 109 +++++++++++++++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 33 deletions(-)