[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/7] cpu model bug fixes and definition correcti
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH 2/7] cpu model bug fixes and definition corrections: Allow an optional qemu_early_init_vcpu() |
Date: |
Tue, 24 May 2011 14:18:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
Hi John,
a general remark first: KVM related patches usually flow via uq/master
of the qemu-kvm.git. Maintainers are Avi and Marcelo, so you need to
address them and include the kvm mailing list.
On 2011-05-23 23:46, john cooper wrote:
> Allow an optional qemu_early_init_vcpu() such that
> kvm_arch_get_supported_cpuid() can be used from
> cpu_x86_register(). Without this minimal setup
> kvm_arch_get_supported_cpuid() gags kvm_ioctl() via
> passing a NULL initialized KVMState *.
>
> Signed-off-by: john cooper <address@hidden>
> ---
>
> diff --git a/cpus.c b/cpus.c
> index 1fc34b7..25122db 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -544,6 +544,14 @@ void qemu_main_loop_start(void)
> {
> }
>
> +void qemu_early_init_vcpu(void *_env)
> +{
> + CPUState *env = _env;
> +
> + if (kvm_enabled())
> + kvm_early_init_vcpu(env);
Please make sure that all your patches pass checkpatch.sh before posting.
> +}
> +
> void qemu_init_vcpu(void *_env)
> {
> CPUState *env = _env;
> diff --git a/kvm-all.c b/kvm-all.c
> index 106eb3a..dc846aa 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -195,24 +195,46 @@ int kvm_pit_in_kernel(void)
> return kvm_state->pit_in_kernel;
> }
>
> -int kvm_init_vcpu(CPUState *env)
> +/* env->kvm_state is needed early by kvm_check_extension()
> + * break it out so it may be setup early where needed
> + */
> +int kvm_early_init_vcpu(CPUState *env)
> +
> {
> KVMState *s = kvm_state;
> - long mmap_size;
> int ret;
>
> - DPRINTF("kvm_init_vcpu\n");
> + DPRINTF("kvm_early_init_vcpu\n");
> +
> + if (env->kvm_state) { /* already setup */
> + return 0;
> + }
>
> ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, env->cpu_index);
> if (ret < 0) {
> DPRINTF("kvm_create_vcpu failed\n");
> - goto err;
> + } else {
> + env->kvm_fd = ret;
> + env->kvm_state = s;
> + env->kvm_vcpu_dirty = 1;
> }
> + return ret;
> +}
> +
> +int kvm_init_vcpu(CPUState *env)
> +{
> + KVMState *s;
> + long mmap_size;
> + int ret;
> +
> + DPRINTF("kvm_init_vcpu\n");
>
> - env->kvm_fd = ret;
> - env->kvm_state = s;
> - env->kvm_vcpu_dirty = 1;
> + ret = kvm_early_init_vcpu(env);
> + if (ret < 0) {
> + goto err;
> + }
>
> + s = env->kvm_state;
> mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
> if (mmap_size < 0) {
> ret = mmap_size;
> diff --git a/kvm.h b/kvm.h
> index d565dba..fe0631b 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -53,6 +53,7 @@ int kvm_has_xcrs(void);
> int kvm_has_many_ioeventfds(void);
>
> #ifdef NEED_CPU_H
> +int kvm_early_init_vcpu(CPUState *env);
> int kvm_init_vcpu(CPUState *env);
>
> int kvm_cpu_exec(CPUState *env);
> diff --git a/qemu-common.h b/qemu-common.h
> index b851b20..2bea318 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -313,8 +313,10 @@ struct qemu_work_item {
> };
>
> #ifdef CONFIG_USER_ONLY
> +#define qemu_early_init_vcpu(env) do { } while (0)
> #define qemu_init_vcpu(env) do { } while (0)
> #else
> +void qemu_early_init_vcpu(void *env);
> void qemu_init_vcpu(void *env);
> #endif
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 89df997..73f44e8 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1261,6 +1261,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
> cpu_set_debug_excp_handler(breakpoint_handler);
> #endif
> }
> + qemu_early_init_vcpu(env);
Lacking return code evaluation.
> if (cpu_x86_register(env, cpu_model) < 0) {
> cpu_x86_close(env);
> return NULL;
However, this is the wrong approach IMHO. Neither
kvm_arch_get_supported_cpuid nor try_get_cpuid depend on CPUState or a
per-VCPU IOCTL. try_get_cpuid actually issues a KVM (ie. global) IOCTL.
So better pass KVMState around which is available even before VCPU creation.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux