| Message ID | 20170606181948.16238-14-rkagan@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote: > Make Hyper-V SynIC a device which is attached as a child to X86CPU. For > now it only makes SynIC visibile in the qom hierarchy and exposes a few > properties which are maintained in sync with the respecitve msrs of the > parent cpu. > > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> > --- > target/i386/hyperv.h | 4 ++ > target/i386/hyperv.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++- > target/i386/kvm.c | 5 ++ > target/i386/machine.c | 9 ++++ > 4 files changed, 147 insertions(+), 2 deletions(-) > > diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h > index ca5a32d..9dd5ca0 100644 > --- a/target/i386/hyperv.h > +++ b/target/i386/hyperv.h > @@ -34,4 +34,8 @@ int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route); > uint32_t hyperv_vp_index(X86CPU *cpu); > X86CPU *hyperv_find_vcpu(uint32_t vcpu_id); > > +void hyperv_synic_add(X86CPU *cpu); > +void hyperv_synic_reset(X86CPU *cpu); > +void hyperv_synic_update(X86CPU *cpu); > + > #endif > diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c > index ae67f82..2d9e9fe 100644 > --- a/target/i386/hyperv.c > +++ b/target/i386/hyperv.c > @@ -13,12 +13,27 @@ > > #include "qemu/osdep.h" > #include "qemu/main-loop.h" > +#include "qapi/error.h" > +#include "hw/qdev-properties.h" > #include "hyperv.h" > #include "hyperv_proto.h" > > +typedef struct SynICState { > + DeviceState parent_obj; > + > + X86CPU *cpu; > + > + bool enabled; > + hwaddr msg_page_addr; > + hwaddr evt_page_addr; > +} SynICState; > + > +#define TYPE_SYNIC "hyperv-synic" > +#define SYNIC(obj) OBJECT_CHECK(SynICState, (obj), TYPE_SYNIC) > + > struct HvSintRoute { > uint32_t sint; > - X86CPU *cpu; > + SynICState *synic; > int gsi; > EventNotifier sint_set_notifier; > EventNotifier sint_ack_notifier; > @@ -37,6 +52,37 @@ X86CPU *hyperv_find_vcpu(uint32_t vp_index) > return X86_CPU(qemu_get_cpu(vp_index)); > } > > +static SynICState *get_synic(X86CPU *cpu) > +{ > + SynICState *synic = > + SYNIC(object_resolve_path_component(OBJECT(cpu), "synic")); > + assert(synic); > + return synic; How often this will run? I think a new X86CPU struct field would be acceptable to avoid resolving a QOM path on every hyperv exit. Do you even need QOM for this? > +} > + > +static void synic_update_msg_page_addr(SynICState *synic) > +{ > + uint64_t msr = synic->cpu->env.msr_hv_synic_msg_page; > + hwaddr new_addr = (msr & HV_SIMP_ENABLE) ? (msr & TARGET_PAGE_MASK) : 0; > + > + synic->msg_page_addr = new_addr; > +} > + > +static void synic_update_evt_page_addr(SynICState *synic) > +{ > + uint64_t msr = synic->cpu->env.msr_hv_synic_evt_page; > + hwaddr new_addr = (msr & HV_SIEFP_ENABLE) ? (msr & TARGET_PAGE_MASK) : 0; > + > + synic->evt_page_addr = new_addr; > +} > + > +static void synic_update(SynICState *synic) > +{ > + synic->enabled = synic->cpu->env.msr_hv_synic_control & HV_SYNIC_ENABLE; > + synic_update_msg_page_addr(synic); > + synic_update_evt_page_addr(synic); > +} > + > int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit) > { > CPUX86State *env = &cpu->env; > @@ -65,6 +111,7 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit) > default: > return -1; > } > + synic_update(get_synic(cpu)); > return 0; > case KVM_EXIT_HYPERV_HCALL: { > uint16_t code; > @@ -95,10 +142,13 @@ HvSintRoute *hyperv_sint_route_new(X86CPU *cpu, uint32_t sint, > HvSintAckClb sint_ack_clb, > void *sint_ack_clb_data) > { > + SynICState *synic; > HvSintRoute *sint_route; > EventNotifier *ack_notifier; > int r, gsi; > > + synic = get_synic(cpu); > + > sint_route = g_new0(HvSintRoute, 1); > r = event_notifier_init(&sint_route->sint_set_notifier, false); > if (r) { > @@ -129,7 +179,7 @@ HvSintRoute *hyperv_sint_route_new(X86CPU *cpu, uint32_t sint, > sint_route->gsi = gsi; > sint_route->sint_ack_clb = sint_ack_clb; > sint_route->sint_ack_clb_data = sint_ack_clb_data; > - sint_route->cpu = cpu; > + sint_route->synic = synic; > sint_route->sint = sint; > sint_route->refcount = 1; > > @@ -183,3 +233,80 @@ int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route) > { > return event_notifier_set(&sint_route->sint_set_notifier); > } > + > +static Property synic_props[] = { > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), What do you need the QOM properties for? Are they supposed to be configurable by the user? Are they supposed to be queried from outside QEMU? On which cases? > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void synic_realize(DeviceState *dev, Error **errp) > +{ > + Object *obj = OBJECT(dev); > + SynICState *synic = SYNIC(dev); > + > + synic->cpu = X86_CPU(obj->parent); > +} > + > +static void synic_reset(DeviceState *dev) > +{ > + SynICState *synic = SYNIC(dev); > + synic_update(synic); > +} > + > +static void synic_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->props = synic_props; > + dc->realize = synic_realize; > + dc->reset = synic_reset; > + dc->user_creatable = false; > +} > + > +void hyperv_synic_add(X86CPU *cpu) > +{ > + Object *obj; > + > + if (!cpu->hyperv_synic) { > + return; > + } > + > + obj = object_new(TYPE_SYNIC); > + object_property_add_child(OBJECT(cpu), "synic", obj, &error_abort); > + object_unref(obj); > + object_property_set_bool(obj, true, "realized", &error_abort); > +} > + > +void hyperv_synic_reset(X86CPU *cpu) > +{ > + if (!cpu->hyperv_synic) { > + return; > + } > + > + device_reset(DEVICE(get_synic(cpu))); > +} > + > +void hyperv_synic_update(X86CPU *cpu) > +{ > + if (!cpu->hyperv_synic) { > + return; > + } > + > + synic_update(get_synic(cpu)); > +} > + > +static const TypeInfo synic_type_info = { > + .name = TYPE_SYNIC, > + .parent = TYPE_DEVICE, > + .instance_size = sizeof(SynICState), > + .class_init = synic_class_init, > +}; > + > +static void synic_register_types(void) > +{ > + type_register_static(&synic_type_info); > +} > + > +type_init(synic_register_types) > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index eb9cde4..433c912 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -688,6 +688,9 @@ static int hyperv_handle_properties(CPUState *cs) > } > env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE; > } > + > + hyperv_synic_add(cpu); > + The purpose of hyperv_handle_properties() is to just initialize env->features based on hyperv flags, and it might even go away some day. I suggest moving this to x86_cpu_realizefn(). > return 0; > } > > @@ -1065,6 +1068,8 @@ void kvm_arch_reset_vcpu(X86CPU *cpu) > for (i = 0; i < ARRAY_SIZE(env->msr_hv_synic_sint); i++) { > env->msr_hv_synic_sint[i] = HV_SINT_MASKED; > } > + > + hyperv_synic_reset(cpu); > } > } > > diff --git a/target/i386/machine.c b/target/i386/machine.c > index eb00b19..8022c24 100644 > --- a/target/i386/machine.c > +++ b/target/i386/machine.c > @@ -7,6 +7,7 @@ > #include "hw/i386/pc.h" > #include "hw/isa/isa.h" > #include "migration/cpu.h" > +#include "hyperv.h" > > #include "sysemu/kvm.h" > > @@ -633,11 +634,19 @@ static bool hyperv_synic_enable_needed(void *opaque) > return false; > } > > +static int hyperv_synic_post_load(void *opaque, int version_id) > +{ > + X86CPU *cpu = opaque; > + hyperv_synic_update(cpu); > + return 0; > +} > + > static const VMStateDescription vmstate_msr_hyperv_synic = { > .name = "cpu/msr_hyperv_synic", > .version_id = 1, > .minimum_version_id = 1, > .needed = hyperv_synic_enable_needed, > + .post_load = hyperv_synic_post_load, > .fields = (VMStateField[]) { > VMSTATE_UINT64(env.msr_hv_synic_control, X86CPU), > VMSTATE_UINT64(env.msr_hv_synic_evt_page, X86CPU), > -- > 2.9.4 >
On Tue, Jun 13, 2017 at 03:34:34PM -0300, Eduardo Habkost wrote: > On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote: > > Make Hyper-V SynIC a device which is attached as a child to X86CPU. For > > now it only makes SynIC visibile in the qom hierarchy and exposes a few > > properties which are maintained in sync with the respecitve msrs of the > > parent cpu. [...] > > +static SynICState *get_synic(X86CPU *cpu) > > +{ > > + SynICState *synic = > > + SYNIC(object_resolve_path_component(OBJECT(cpu), "synic")); > > + assert(synic); > > + return synic; > > How often this will run? I think a new X86CPU struct field would > be acceptable to avoid resolving a QOM path on every hyperv exit. It's only used at the points where synic state is updated: at realize/save/load/unrealize and when the guest configures synics via msrs (i.e. a few times per cpu at guest startup). None of those is performance-critical. > Do you even need QOM for this? I need to reach the synic from the cpu on slow paths, and IMHO the pointer on X86CPU is not justified here. Besides there are two memory regions associated with every synic (in a followup patch) and I thought it made sense to have a dedicated parent QOM node for them. > > +static Property synic_props[] = { > > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), > > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), > > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), > > What do you need the QOM properties for? Are they supposed to be > configurable by the user? No. Actually I wanted to define them as read-only, but I failed to find a way to do so. > Are they supposed to be queried from outside QEMU? On which cases? ATM I only see them as informational fields that may prove useful during debugging. Roman.
On Wed, Jun 14, 2017 at 12:58:04PM +0300, Roman Kagan wrote: > On Tue, Jun 13, 2017 at 03:34:34PM -0300, Eduardo Habkost wrote: > > On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote: > > > Make Hyper-V SynIC a device which is attached as a child to X86CPU. For > > > now it only makes SynIC visibile in the qom hierarchy and exposes a few > > > properties which are maintained in sync with the respecitve msrs of the > > > parent cpu. > [...] > > > +static SynICState *get_synic(X86CPU *cpu) > > > +{ > > > + SynICState *synic = > > > + SYNIC(object_resolve_path_component(OBJECT(cpu), "synic")); > > > + assert(synic); > > > + return synic; > > > > How often this will run? I think a new X86CPU struct field would > > be acceptable to avoid resolving a QOM path on every hyperv exit. > > It's only used at the points where synic state is updated: at > realize/save/load/unrealize and when the guest configures synics via > msrs (i.e. a few times per cpu at guest startup). None of those is > performance-critical. > > > Do you even need QOM for this? > > I need to reach the synic from the cpu on slow paths, and IMHO the > pointer on X86CPU is not justified here. Besides there are two memory > regions associated with every synic (in a followup patch) and I thought > it made sense to have a dedicated parent QOM node for them. Makes sense. > > > > +static Property synic_props[] = { > > > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), > > > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), > > > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), > > > > What do you need the QOM properties for? Are they supposed to be > > configurable by the user? > > No. Actually I wanted to define them as read-only, but I failed to find > a way to do so. > > > Are they supposed to be queried from outside QEMU? On which cases? > > ATM I only see them as informational fields that may prove useful during > debugging. > You can use object_property_add_uint64_ptr() on instance_init to add read-only properties. If the enabled/msg_page_addr/evt_page_addr struct fields exist only because of the properties, you can also use object_property_add() and write your own getter that will query the MSRs only when reading the property. I normally don't like custom getters/setters, but it sounds acceptable for a read-only property that's only for debugging. Either way you choose to implement it, I would add a comment noting that the properties are there just for debugging. BTW, would you still want to add this amount of boilerplate code just for debugging if we had a generic msr_get HMP command? There's a series in the list (submitted on April) adding msr_get and msr_set HMP commands, but it was never merged.
On Wed, Jun 14, 2017 at 09:46:20AM -0300, Eduardo Habkost wrote: > On Wed, Jun 14, 2017 at 12:58:04PM +0300, Roman Kagan wrote: > > On Tue, Jun 13, 2017 at 03:34:34PM -0300, Eduardo Habkost wrote: > > > On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote: > > > > +static Property synic_props[] = { > > > > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), > > > > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), > > > > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), > > > > > > What do you need the QOM properties for? Are they supposed to be > > > configurable by the user? > > > > No. Actually I wanted to define them as read-only, but I failed to find > > a way to do so. > > > > > Are they supposed to be queried from outside QEMU? On which cases? > > > > ATM I only see them as informational fields that may prove useful during > > debugging. > > > > You can use object_property_add_uint64_ptr() on instance_init to > add read-only properties. > > If the enabled/msg_page_addr/evt_page_addr struct fields exist > only because of the properties, you can also use > object_property_add() and write your own getter that will query > the MSRs only when reading the property. I normally don't like > custom getters/setters, but it sounds acceptable for a read-only > property that's only for debugging. Actually that was what I did at first, but then I decided it was useful to have the fields directly on SynICState (see followup patches). > Either way you choose to implement it, I would add a comment > noting that the properties are there just for debugging. > > BTW, would you still want to add this amount of boilerplate code > just for debugging if we had a generic msr_get HMP command? > There's a series in the list (submitted on April) adding msr_get > and msr_set HMP commands, but it was never merged. Yes I guess it would do. So I'm now tempted to just drop the properties, and leave the fields invisible through QOM. Thanks, Roman.
On Wed, Jun 14, 2017 at 06:11:03PM +0300, Roman Kagan wrote: > On Wed, Jun 14, 2017 at 09:46:20AM -0300, Eduardo Habkost wrote: > > On Wed, Jun 14, 2017 at 12:58:04PM +0300, Roman Kagan wrote: > > > On Tue, Jun 13, 2017 at 03:34:34PM -0300, Eduardo Habkost wrote: > > > > On Tue, Jun 06, 2017 at 09:19:38PM +0300, Roman Kagan wrote: > > > > > +static Property synic_props[] = { > > > > > + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), > > > > > + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), > > > > > + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), > > > > > > > > What do you need the QOM properties for? Are they supposed to be > > > > configurable by the user? > > > > > > No. Actually I wanted to define them as read-only, but I failed to find > > > a way to do so. > > > > > > > Are they supposed to be queried from outside QEMU? On which cases? > > > > > > ATM I only see them as informational fields that may prove useful during > > > debugging. > > > > > > > You can use object_property_add_uint64_ptr() on instance_init to > > add read-only properties. > > > > If the enabled/msg_page_addr/evt_page_addr struct fields exist > > only because of the properties, you can also use > > object_property_add() and write your own getter that will query > > the MSRs only when reading the property. I normally don't like > > custom getters/setters, but it sounds acceptable for a read-only > > property that's only for debugging. > > Actually that was what I did at first, but then I decided it was useful > to have the fields directly on SynICState (see followup patches). > > > Either way you choose to implement it, I would add a comment > > noting that the properties are there just for debugging. > > > > BTW, would you still want to add this amount of boilerplate code > > just for debugging if we had a generic msr_get HMP command? > > There's a series in the list (submitted on April) adding msr_get > > and msr_set HMP commands, but it was never merged. > > Yes I guess it would do. So I'm now tempted to just drop the > properties, and leave the fields invisible through QOM. If you are going to keep the fields on SynICState, then you just need one object_property_add_*_ptr() line for each property, so maybe it's still worth it. It's up to you.
diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h index ca5a32d..9dd5ca0 100644 --- a/target/i386/hyperv.h +++ b/target/i386/hyperv.h @@ -34,4 +34,8 @@ int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route); uint32_t hyperv_vp_index(X86CPU *cpu); X86CPU *hyperv_find_vcpu(uint32_t vcpu_id); +void hyperv_synic_add(X86CPU *cpu); +void hyperv_synic_reset(X86CPU *cpu); +void hyperv_synic_update(X86CPU *cpu); + #endif diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c index ae67f82..2d9e9fe 100644 --- a/target/i386/hyperv.c +++ b/target/i386/hyperv.c @@ -13,12 +13,27 @@ #include "qemu/osdep.h" #include "qemu/main-loop.h" +#include "qapi/error.h" +#include "hw/qdev-properties.h" #include "hyperv.h" #include "hyperv_proto.h" +typedef struct SynICState { + DeviceState parent_obj; + + X86CPU *cpu; + + bool enabled; + hwaddr msg_page_addr; + hwaddr evt_page_addr; +} SynICState; + +#define TYPE_SYNIC "hyperv-synic" +#define SYNIC(obj) OBJECT_CHECK(SynICState, (obj), TYPE_SYNIC) + struct HvSintRoute { uint32_t sint; - X86CPU *cpu; + SynICState *synic; int gsi; EventNotifier sint_set_notifier; EventNotifier sint_ack_notifier; @@ -37,6 +52,37 @@ X86CPU *hyperv_find_vcpu(uint32_t vp_index) return X86_CPU(qemu_get_cpu(vp_index)); } +static SynICState *get_synic(X86CPU *cpu) +{ + SynICState *synic = + SYNIC(object_resolve_path_component(OBJECT(cpu), "synic")); + assert(synic); + return synic; +} + +static void synic_update_msg_page_addr(SynICState *synic) +{ + uint64_t msr = synic->cpu->env.msr_hv_synic_msg_page; + hwaddr new_addr = (msr & HV_SIMP_ENABLE) ? (msr & TARGET_PAGE_MASK) : 0; + + synic->msg_page_addr = new_addr; +} + +static void synic_update_evt_page_addr(SynICState *synic) +{ + uint64_t msr = synic->cpu->env.msr_hv_synic_evt_page; + hwaddr new_addr = (msr & HV_SIEFP_ENABLE) ? (msr & TARGET_PAGE_MASK) : 0; + + synic->evt_page_addr = new_addr; +} + +static void synic_update(SynICState *synic) +{ + synic->enabled = synic->cpu->env.msr_hv_synic_control & HV_SYNIC_ENABLE; + synic_update_msg_page_addr(synic); + synic_update_evt_page_addr(synic); +} + int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit) { CPUX86State *env = &cpu->env; @@ -65,6 +111,7 @@ int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit) default: return -1; } + synic_update(get_synic(cpu)); return 0; case KVM_EXIT_HYPERV_HCALL: { uint16_t code; @@ -95,10 +142,13 @@ HvSintRoute *hyperv_sint_route_new(X86CPU *cpu, uint32_t sint, HvSintAckClb sint_ack_clb, void *sint_ack_clb_data) { + SynICState *synic; HvSintRoute *sint_route; EventNotifier *ack_notifier; int r, gsi; + synic = get_synic(cpu); + sint_route = g_new0(HvSintRoute, 1); r = event_notifier_init(&sint_route->sint_set_notifier, false); if (r) { @@ -129,7 +179,7 @@ HvSintRoute *hyperv_sint_route_new(X86CPU *cpu, uint32_t sint, sint_route->gsi = gsi; sint_route->sint_ack_clb = sint_ack_clb; sint_route->sint_ack_clb_data = sint_ack_clb_data; - sint_route->cpu = cpu; + sint_route->synic = synic; sint_route->sint = sint; sint_route->refcount = 1; @@ -183,3 +233,80 @@ int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route) { return event_notifier_set(&sint_route->sint_set_notifier); } + +static Property synic_props[] = { + DEFINE_PROP_BOOL("enabled", SynICState, enabled, false), + DEFINE_PROP_UINT64("msg-page-addr", SynICState, msg_page_addr, 0), + DEFINE_PROP_UINT64("evt-page-addr", SynICState, evt_page_addr, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void synic_realize(DeviceState *dev, Error **errp) +{ + Object *obj = OBJECT(dev); + SynICState *synic = SYNIC(dev); + + synic->cpu = X86_CPU(obj->parent); +} + +static void synic_reset(DeviceState *dev) +{ + SynICState *synic = SYNIC(dev); + synic_update(synic); +} + +static void synic_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->props = synic_props; + dc->realize = synic_realize; + dc->reset = synic_reset; + dc->user_creatable = false; +} + +void hyperv_synic_add(X86CPU *cpu) +{ + Object *obj; + + if (!cpu->hyperv_synic) { + return; + } + + obj = object_new(TYPE_SYNIC); + object_property_add_child(OBJECT(cpu), "synic", obj, &error_abort); + object_unref(obj); + object_property_set_bool(obj, true, "realized", &error_abort); +} + +void hyperv_synic_reset(X86CPU *cpu) +{ + if (!cpu->hyperv_synic) { + return; + } + + device_reset(DEVICE(get_synic(cpu))); +} + +void hyperv_synic_update(X86CPU *cpu) +{ + if (!cpu->hyperv_synic) { + return; + } + + synic_update(get_synic(cpu)); +} + +static const TypeInfo synic_type_info = { + .name = TYPE_SYNIC, + .parent = TYPE_DEVICE, + .instance_size = sizeof(SynICState), + .class_init = synic_class_init, +}; + +static void synic_register_types(void) +{ + type_register_static(&synic_type_info); +} + +type_init(synic_register_types) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index eb9cde4..433c912 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -688,6 +688,9 @@ static int hyperv_handle_properties(CPUState *cs) } env->features[FEAT_HYPERV_EAX] |= HV_SYNTIMERS_AVAILABLE; } + + hyperv_synic_add(cpu); + return 0; } @@ -1065,6 +1068,8 @@ void kvm_arch_reset_vcpu(X86CPU *cpu) for (i = 0; i < ARRAY_SIZE(env->msr_hv_synic_sint); i++) { env->msr_hv_synic_sint[i] = HV_SINT_MASKED; } + + hyperv_synic_reset(cpu); } } diff --git a/target/i386/machine.c b/target/i386/machine.c index eb00b19..8022c24 100644 --- a/target/i386/machine.c +++ b/target/i386/machine.c @@ -7,6 +7,7 @@ #include "hw/i386/pc.h" #include "hw/isa/isa.h" #include "migration/cpu.h" +#include "hyperv.h" #include "sysemu/kvm.h" @@ -633,11 +634,19 @@ static bool hyperv_synic_enable_needed(void *opaque) return false; } +static int hyperv_synic_post_load(void *opaque, int version_id) +{ + X86CPU *cpu = opaque; + hyperv_synic_update(cpu); + return 0; +} + static const VMStateDescription vmstate_msr_hyperv_synic = { .name = "cpu/msr_hyperv_synic", .version_id = 1, .minimum_version_id = 1, .needed = hyperv_synic_enable_needed, + .post_load = hyperv_synic_post_load, .fields = (VMStateField[]) { VMSTATE_UINT64(env.msr_hv_synic_control, X86CPU), VMSTATE_UINT64(env.msr_hv_synic_evt_page, X86CPU),
Make Hyper-V SynIC a device which is attached as a child to X86CPU. For now it only makes SynIC visibile in the qom hierarchy and exposes a few properties which are maintained in sync with the respecitve msrs of the parent cpu. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- target/i386/hyperv.h | 4 ++ target/i386/hyperv.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++- target/i386/kvm.c | 5 ++ target/i386/machine.c | 9 ++++ 4 files changed, 147 insertions(+), 2 deletions(-)