| Message ID | 1365691918-30594-5-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 11, 2013 at 04:51:43PM +0200, Igor Mammedov wrote: > ... to synchronize CPU state to KVM > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > * linking kvm-stub.o to *-user target moved in separate patch > --- > include/sysemu/kvm.h | 18 ++++++++++-------- > kvm-all.c | 1 + > kvm-stub.c | 1 + > qom/cpu.c | 4 ++++ > vl.c | 1 - > 5 files changed, 16 insertions(+), 9 deletions(-) > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > index 93cef28..8fb2489 100644 > --- a/include/sysemu/kvm.h > +++ b/include/sysemu/kvm.h > @@ -250,8 +250,6 @@ int kvm_check_extension(KVMState *s, unsigned int extension); > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > uint32_t index, int reg); > void kvm_cpu_synchronize_state(CPUArchState *env); > -void kvm_cpu_synchronize_post_reset(CPUState *cpu); > -void kvm_cpu_synchronize_post_init(CPUState *cpu); > > /* generic hooks - to be moved/refactored once there are more users */ > > @@ -262,6 +260,16 @@ static inline void cpu_synchronize_state(CPUArchState *env) > } > } > > +#if !defined(CONFIG_USER_ONLY) > +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > + hwaddr *phys_addr); > +#endif > + > +#endif /* NEED_CPU_H */ > + > +void kvm_cpu_synchronize_post_reset(CPUState *cpu); > +void kvm_cpu_synchronize_post_init(CPUState *cpu); > + > static inline void cpu_synchronize_post_reset(CPUState *cpu) > { > if (kvm_enabled()) { > @@ -277,12 +285,6 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) > } > > > -#if !defined(CONFIG_USER_ONLY) > -int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > - hwaddr *phys_addr); > -#endif > - > -#endif > int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign, > uint32_t size); > > diff --git a/kvm-all.c b/kvm-all.c > index fc4e17c..1d17128 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; > bool kvm_irqfds_allowed; > bool kvm_msi_via_irqfd_allowed; > bool kvm_gsi_routing_allowed; > +bool kvm_allowed; > > static const KVMCapabilityInfo kvm_required_capabilites[] = { > KVM_CAP_INFO(USER_MEMORY), > diff --git a/kvm-stub.c b/kvm-stub.c > index b34064a..8cb81c4 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; > bool kvm_irqfds_allowed; > bool kvm_msi_via_irqfd_allowed; > bool kvm_gsi_routing_allowed; > +bool kvm_allowed; > > int kvm_init_vcpu(CPUState *cpu) > { > diff --git a/qom/cpu.c b/qom/cpu.c > index e242dcb..0c76712 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -20,6 +20,7 @@ > > #include "qom/cpu.h" > #include "qemu-common.h" > +#include "sysemu/kvm.h" > > void cpu_reset_interrupt(CPUState *cpu, int mask) > { > @@ -57,6 +58,9 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model) > > static void cpu_common_realizefn(DeviceState *dev, Error **errp) > { > + if (dev->hotplugged) { > + cpu_synchronize_post_init(CPU(dev)); > + } For reference: non-hotplugged CPUs have cpu_synchronize_post_init() called from cpu_synchronize_all_post_init(), that is called from main(), just after the machine->init() call. That said, have you considered changing the code so that's possible to call cpu_synchronize_post_init() for all CPUs, so we could kill the cpu_synchronize_all_post_init() call entirely? Maybe the solution for this is to realize non-hotplugged CPUs later, instead of realizing them very early inside the machine init function. If doing is not possible or not enough, we could use VM state notifiers to watch for the right moment to call cpu_synchronize_post_init(). > } > > static void cpu_class_init(ObjectClass *klass, void *data) > diff --git a/vl.c b/vl.c > index d694a90..21952c3 100644 > --- a/vl.c > +++ b/vl.c > @@ -267,7 +267,6 @@ static NotifierList machine_init_done_notifiers = > NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers); > > static bool tcg_allowed = true; > -bool kvm_allowed; > bool xen_allowed; > uint32_t xen_domid; > enum xen_mode xen_mode = XEN_EMULATE; > -- > 1.8.2 >
On Thu, 11 Apr 2013 15:33:01 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Apr 11, 2013 at 04:51:43PM +0200, Igor Mammedov wrote: > > ... to synchronize CPU state to KVM > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v2: > > * linking kvm-stub.o to *-user target moved in separate patch > > --- > > include/sysemu/kvm.h | 18 ++++++++++-------- > > kvm-all.c | 1 + > > kvm-stub.c | 1 + > > qom/cpu.c | 4 ++++ > > vl.c | 1 - > > 5 files changed, 16 insertions(+), 9 deletions(-) > > > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > > index 93cef28..8fb2489 100644 > > --- a/include/sysemu/kvm.h > > +++ b/include/sysemu/kvm.h > > @@ -250,8 +250,6 @@ int kvm_check_extension(KVMState *s, unsigned int extension); > > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > > uint32_t index, int reg); > > void kvm_cpu_synchronize_state(CPUArchState *env); > > -void kvm_cpu_synchronize_post_reset(CPUState *cpu); > > -void kvm_cpu_synchronize_post_init(CPUState *cpu); > > > > /* generic hooks - to be moved/refactored once there are more users */ > > > > @@ -262,6 +260,16 @@ static inline void cpu_synchronize_state(CPUArchState *env) > > } > > } > > > > +#if !defined(CONFIG_USER_ONLY) > > +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > > + hwaddr *phys_addr); > > +#endif > > + > > +#endif /* NEED_CPU_H */ > > + > > +void kvm_cpu_synchronize_post_reset(CPUState *cpu); > > +void kvm_cpu_synchronize_post_init(CPUState *cpu); > > + > > static inline void cpu_synchronize_post_reset(CPUState *cpu) > > { > > if (kvm_enabled()) { > > @@ -277,12 +285,6 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) > > } > > > > > > -#if !defined(CONFIG_USER_ONLY) > > -int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > > - hwaddr *phys_addr); > > -#endif > > - > > -#endif > > int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign, > > uint32_t size); > > > > diff --git a/kvm-all.c b/kvm-all.c > > index fc4e17c..1d17128 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; > > bool kvm_irqfds_allowed; > > bool kvm_msi_via_irqfd_allowed; > > bool kvm_gsi_routing_allowed; > > +bool kvm_allowed; > > > > static const KVMCapabilityInfo kvm_required_capabilites[] = { > > KVM_CAP_INFO(USER_MEMORY), > > diff --git a/kvm-stub.c b/kvm-stub.c > > index b34064a..8cb81c4 100644 > > --- a/kvm-stub.c > > +++ b/kvm-stub.c > > @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; > > bool kvm_irqfds_allowed; > > bool kvm_msi_via_irqfd_allowed; > > bool kvm_gsi_routing_allowed; > > +bool kvm_allowed; > > > > int kvm_init_vcpu(CPUState *cpu) > > { > > diff --git a/qom/cpu.c b/qom/cpu.c > > index e242dcb..0c76712 100644 > > --- a/qom/cpu.c > > +++ b/qom/cpu.c > > @@ -20,6 +20,7 @@ > > > > #include "qom/cpu.h" > > #include "qemu-common.h" > > +#include "sysemu/kvm.h" > > > > void cpu_reset_interrupt(CPUState *cpu, int mask) > > { > > @@ -57,6 +58,9 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model) > > > > static void cpu_common_realizefn(DeviceState *dev, Error **errp) > > { > > + if (dev->hotplugged) { > > + cpu_synchronize_post_init(CPU(dev)); > > + } > > For reference: non-hotplugged CPUs have cpu_synchronize_post_init() > called from cpu_synchronize_all_post_init(), that is called from main(), > just after the machine->init() call. > > That said, have you considered changing the code so that's possible to > call cpu_synchronize_post_init() for all CPUs, so we could kill the > cpu_synchronize_all_post_init() call entirely? > > Maybe the solution for this is to realize non-hotplugged CPUs later, > instead of realizing them very early inside the machine init function. > If doing is not possible or not enough, we could use VM state notifiers > to watch for the right moment to call cpu_synchronize_post_init(). I have not considered it, since question is out of scope CPU hot-plug. It might be feasible to do so later in separate patch and remove "if (dev->hotplugged)" condition tough. I think it will be more appropriate when machine will QOM-ified, then to make machine.realize() work it had to be done, but now I don't see any gain doing so (providing all the trouble it might cause). > > > } > > > > static void cpu_class_init(ObjectClass *klass, void *data) > > diff --git a/vl.c b/vl.c > > index d694a90..21952c3 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -267,7 +267,6 @@ static NotifierList machine_init_done_notifiers = > > NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers); > > > > static bool tcg_allowed = true; > > -bool kvm_allowed; > > bool xen_allowed; > > uint32_t xen_domid; > > enum xen_mode xen_mode = XEN_EMULATE; > > -- > > 1.8.2 > > > > -- > Eduardo
On Fri, Apr 12, 2013 at 01:34:09PM +0200, Igor Mammedov wrote: > On Thu, 11 Apr 2013 15:33:01 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Thu, Apr 11, 2013 at 04:51:43PM +0200, Igor Mammedov wrote: > > > ... to synchronize CPU state to KVM > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > v2: > > > * linking kvm-stub.o to *-user target moved in separate patch > > > --- > > > include/sysemu/kvm.h | 18 ++++++++++-------- > > > kvm-all.c | 1 + > > > kvm-stub.c | 1 + > > > qom/cpu.c | 4 ++++ > > > vl.c | 1 - > > > 5 files changed, 16 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h > > > index 93cef28..8fb2489 100644 > > > --- a/include/sysemu/kvm.h > > > +++ b/include/sysemu/kvm.h > > > @@ -250,8 +250,6 @@ int kvm_check_extension(KVMState *s, unsigned int extension); > > > uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, > > > uint32_t index, int reg); > > > void kvm_cpu_synchronize_state(CPUArchState *env); > > > -void kvm_cpu_synchronize_post_reset(CPUState *cpu); > > > -void kvm_cpu_synchronize_post_init(CPUState *cpu); > > > > > > /* generic hooks - to be moved/refactored once there are more users */ > > > > > > @@ -262,6 +260,16 @@ static inline void cpu_synchronize_state(CPUArchState *env) > > > } > > > } > > > > > > +#if !defined(CONFIG_USER_ONLY) > > > +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > > > + hwaddr *phys_addr); > > > +#endif > > > + > > > +#endif /* NEED_CPU_H */ > > > + > > > +void kvm_cpu_synchronize_post_reset(CPUState *cpu); > > > +void kvm_cpu_synchronize_post_init(CPUState *cpu); > > > + > > > static inline void cpu_synchronize_post_reset(CPUState *cpu) > > > { > > > if (kvm_enabled()) { > > > @@ -277,12 +285,6 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) > > > } > > > > > > > > > -#if !defined(CONFIG_USER_ONLY) > > > -int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, > > > - hwaddr *phys_addr); > > > -#endif > > > - > > > -#endif > > > int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign, > > > uint32_t size); > > > > > > diff --git a/kvm-all.c b/kvm-all.c > > > index fc4e17c..1d17128 100644 > > > --- a/kvm-all.c > > > +++ b/kvm-all.c > > > @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; > > > bool kvm_irqfds_allowed; > > > bool kvm_msi_via_irqfd_allowed; > > > bool kvm_gsi_routing_allowed; > > > +bool kvm_allowed; > > > > > > static const KVMCapabilityInfo kvm_required_capabilites[] = { > > > KVM_CAP_INFO(USER_MEMORY), > > > diff --git a/kvm-stub.c b/kvm-stub.c > > > index b34064a..8cb81c4 100644 > > > --- a/kvm-stub.c > > > +++ b/kvm-stub.c > > > @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; > > > bool kvm_irqfds_allowed; > > > bool kvm_msi_via_irqfd_allowed; > > > bool kvm_gsi_routing_allowed; > > > +bool kvm_allowed; > > > > > > int kvm_init_vcpu(CPUState *cpu) > > > { > > > diff --git a/qom/cpu.c b/qom/cpu.c > > > index e242dcb..0c76712 100644 > > > --- a/qom/cpu.c > > > +++ b/qom/cpu.c > > > @@ -20,6 +20,7 @@ > > > > > > #include "qom/cpu.h" > > > #include "qemu-common.h" > > > +#include "sysemu/kvm.h" > > > > > > void cpu_reset_interrupt(CPUState *cpu, int mask) > > > { > > > @@ -57,6 +58,9 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model) > > > > > > static void cpu_common_realizefn(DeviceState *dev, Error **errp) > > > { > > > + if (dev->hotplugged) { > > > + cpu_synchronize_post_init(CPU(dev)); > > > + } > > > > For reference: non-hotplugged CPUs have cpu_synchronize_post_init() > > called from cpu_synchronize_all_post_init(), that is called from main(), > > just after the machine->init() call. > > > > That said, have you considered changing the code so that's possible to > > call cpu_synchronize_post_init() for all CPUs, so we could kill the > > cpu_synchronize_all_post_init() call entirely? > > > > Maybe the solution for this is to realize non-hotplugged CPUs later, > > instead of realizing them very early inside the machine init function. > > If doing is not possible or not enough, we could use VM state notifiers > > to watch for the right moment to call cpu_synchronize_post_init(). > > I have not considered it, since question is out of scope CPU hot-plug. > It might be feasible to do so later in separate patch and remove > "if (dev->hotplugged)" condition tough. > > I think it will be more appropriate when machine will QOM-ified, then > to make machine.realize() work it had to be done, but now I don't see > any gain doing so (providing all the trouble it might cause). OK, sounds like a good justification to me. I would love to have a device model where there's no difference between hotplugged and non-hotplugged devices. But this is hard to achieve.
On Thu, Apr 11, 2013 at 04:51:43PM +0200, Igor Mammedov wrote: [...] > diff --git a/kvm-all.c b/kvm-all.c > index fc4e17c..1d17128 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; > bool kvm_irqfds_allowed; > bool kvm_msi_via_irqfd_allowed; > bool kvm_gsi_routing_allowed; > +bool kvm_allowed; > > static const KVMCapabilityInfo kvm_required_capabilites[] = { > KVM_CAP_INFO(USER_MEMORY), > diff --git a/kvm-stub.c b/kvm-stub.c > index b34064a..8cb81c4 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; > bool kvm_irqfds_allowed; > bool kvm_msi_via_irqfd_allowed; > bool kvm_gsi_routing_allowed; > +bool kvm_allowed; Why do you need kvm_allowed on kvm-stub.c? Isn't simpler and more efficient to define kvm_enabled() as (0) on CONFIG_USER_ONLY?
On Mon, 15 Apr 2013 16:57:10 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Apr 11, 2013 at 04:51:43PM +0200, Igor Mammedov wrote: > [...] > > diff --git a/kvm-all.c b/kvm-all.c > > index fc4e17c..1d17128 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; > > bool kvm_irqfds_allowed; > > bool kvm_msi_via_irqfd_allowed; > > bool kvm_gsi_routing_allowed; > > +bool kvm_allowed; > > > > static const KVMCapabilityInfo kvm_required_capabilites[] = { > > KVM_CAP_INFO(USER_MEMORY), > > diff --git a/kvm-stub.c b/kvm-stub.c > > index b34064a..8cb81c4 100644 > > --- a/kvm-stub.c > > +++ b/kvm-stub.c > > @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; > > bool kvm_irqfds_allowed; > > bool kvm_msi_via_irqfd_allowed; > > bool kvm_gsi_routing_allowed; > > +bool kvm_allowed; > > Why do you need kvm_allowed on kvm-stub.c? Isn't simpler and more > efficient to define kvm_enabled() as (0) on CONFIG_USER_ONLY? qom/cpu.o is build only one time for all targets, header ifdef won't work here. > -- > Eduardo
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index 93cef28..8fb2489 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -250,8 +250,6 @@ int kvm_check_extension(KVMState *s, unsigned int extension); uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function, uint32_t index, int reg); void kvm_cpu_synchronize_state(CPUArchState *env); -void kvm_cpu_synchronize_post_reset(CPUState *cpu); -void kvm_cpu_synchronize_post_init(CPUState *cpu); /* generic hooks - to be moved/refactored once there are more users */ @@ -262,6 +260,16 @@ static inline void cpu_synchronize_state(CPUArchState *env) } } +#if !defined(CONFIG_USER_ONLY) +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, + hwaddr *phys_addr); +#endif + +#endif /* NEED_CPU_H */ + +void kvm_cpu_synchronize_post_reset(CPUState *cpu); +void kvm_cpu_synchronize_post_init(CPUState *cpu); + static inline void cpu_synchronize_post_reset(CPUState *cpu) { if (kvm_enabled()) { @@ -277,12 +285,6 @@ static inline void cpu_synchronize_post_init(CPUState *cpu) } -#if !defined(CONFIG_USER_ONLY) -int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr, - hwaddr *phys_addr); -#endif - -#endif int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign, uint32_t size); diff --git a/kvm-all.c b/kvm-all.c index fc4e17c..1d17128 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -109,6 +109,7 @@ bool kvm_async_interrupts_allowed; bool kvm_irqfds_allowed; bool kvm_msi_via_irqfd_allowed; bool kvm_gsi_routing_allowed; +bool kvm_allowed; static const KVMCapabilityInfo kvm_required_capabilites[] = { KVM_CAP_INFO(USER_MEMORY), diff --git a/kvm-stub.c b/kvm-stub.c index b34064a..8cb81c4 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -23,6 +23,7 @@ bool kvm_async_interrupts_allowed; bool kvm_irqfds_allowed; bool kvm_msi_via_irqfd_allowed; bool kvm_gsi_routing_allowed; +bool kvm_allowed; int kvm_init_vcpu(CPUState *cpu) { diff --git a/qom/cpu.c b/qom/cpu.c index e242dcb..0c76712 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -20,6 +20,7 @@ #include "qom/cpu.h" #include "qemu-common.h" +#include "sysemu/kvm.h" void cpu_reset_interrupt(CPUState *cpu, int mask) { @@ -57,6 +58,9 @@ static ObjectClass *cpu_common_class_by_name(const char *cpu_model) static void cpu_common_realizefn(DeviceState *dev, Error **errp) { + if (dev->hotplugged) { + cpu_synchronize_post_init(CPU(dev)); + } } static void cpu_class_init(ObjectClass *klass, void *data) diff --git a/vl.c b/vl.c index d694a90..21952c3 100644 --- a/vl.c +++ b/vl.c @@ -267,7 +267,6 @@ static NotifierList machine_init_done_notifiers = NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers); static bool tcg_allowed = true; -bool kvm_allowed; bool xen_allowed; uint32_t xen_domid; enum xen_mode xen_mode = XEN_EMULATE;
... to synchronize CPU state to KVM Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v2: * linking kvm-stub.o to *-user target moved in separate patch --- include/sysemu/kvm.h | 18 ++++++++++-------- kvm-all.c | 1 + kvm-stub.c | 1 + qom/cpu.c | 4 ++++ vl.c | 1 - 5 files changed, 16 insertions(+), 9 deletions(-)