Message ID | 1256047441-9846-1-git-send-email-glommer@redhat.com |
---|---|
State | New |
Headers | show |
On 10/20/09 16:04, Glauber Costa wrote: > Currently, the msrs involved in setting up pvclock are not saved over > migration and/or save/restore. This patch puts their value in special > fields in our CPUState, and deal with them using vmstate. > > kvm also has to account for it, by including them in the msr list > for the ioctls. Fails to build. Fedora 11 machine with distro kernel. http://repo.or.cz/w/qemu/pdb.git?a=blob;f=build.log;h=5b8a2e83116dc6a74e5045e30c3efe71fa7397ef;hb=0b4f913c89b89b45d3844dc1cf321e3d94871ecb cheers, Gerd
On Wed, Oct 21, 2009 at 03:38:31PM +0200, Gerd Hoffmann wrote: > On 10/20/09 16:04, Glauber Costa wrote: >> Currently, the msrs involved in setting up pvclock are not saved over >> migration and/or save/restore. This patch puts their value in special >> fields in our CPUState, and deal with them using vmstate. >> >> kvm also has to account for it, by including them in the msr list >> for the ioctls. > > Fails to build. Fedora 11 machine with distro kernel. > > http://repo.or.cz/w/qemu/pdb.git?a=blob;f=build.log;h=5b8a2e83116dc6a74e5045e30c3efe71fa7397ef;hb=0b4f913c89b89b45d3844dc1cf321e3d94871ecb strange message. Do we need --kerneldir for that? kvm_para.h should be available...
On 10/21/09 16:32, Glauber Costa wrote: > On Wed, Oct 21, 2009 at 03:38:31PM +0200, Gerd Hoffmann wrote: >> On 10/20/09 16:04, Glauber Costa wrote: >>> Currently, the msrs involved in setting up pvclock are not saved over >>> migration and/or save/restore. This patch puts their value in special >>> fields in our CPUState, and deal with them using vmstate. >>> >>> kvm also has to account for it, by including them in the msr list >>> for the ioctls. >> >> Fails to build. Fedora 11 machine with distro kernel. >> >> http://repo.or.cz/w/qemu/pdb.git?a=blob;f=build.log;h=5b8a2e83116dc6a74e5045e30c3efe71fa7397ef;hb=0b4f913c89b89b45d3844dc1cf321e3d94871ecb > strange message. Do we need --kerneldir for that? Requiring --kerneldir for a successful build isn't going to fly. > kvm_para.h should be available... Nope: $ rpm -ql kernel-headers | grep kvm /usr/include/asm/kvm.h /usr/include/linux/kvm.h Did you ever build this? You should improve your patch qa ... cheers, Gerd
On Wed, Oct 21, 2009 at 04:52:00PM +0200, Gerd Hoffmann wrote: > On 10/21/09 16:32, Glauber Costa wrote: >> On Wed, Oct 21, 2009 at 03:38:31PM +0200, Gerd Hoffmann wrote: >>> On 10/20/09 16:04, Glauber Costa wrote: >>>> Currently, the msrs involved in setting up pvclock are not saved over >>>> migration and/or save/restore. This patch puts their value in special >>>> fields in our CPUState, and deal with them using vmstate. >>>> >>>> kvm also has to account for it, by including them in the msr list >>>> for the ioctls. >>> >>> Fails to build. Fedora 11 machine with distro kernel. >>> >>> http://repo.or.cz/w/qemu/pdb.git?a=blob;f=build.log;h=5b8a2e83116dc6a74e5045e30c3efe71fa7397ef;hb=0b4f913c89b89b45d3844dc1cf321e3d94871ecb >> strange message. Do we need --kerneldir for that? > > Requiring --kerneldir for a successful build isn't going to fly. > >> kvm_para.h should be available... > > Nope: > > $ rpm -ql kernel-headers | grep kvm > /usr/include/asm/kvm.h > /usr/include/linux/kvm.h > > Did you ever build this? > You should improve your patch qa ... built and tested. I was using --kerneldir all along, however. One option I see is to redefine the values of msrs in this file. It seems it already happens for tsc anyway.
On Wed, Oct 21, 2009 at 04:52:00PM +0200, Gerd Hoffmann wrote: > On 10/21/09 16:32, Glauber Costa wrote: >> On Wed, Oct 21, 2009 at 03:38:31PM +0200, Gerd Hoffmann wrote: >>> On 10/20/09 16:04, Glauber Costa wrote: >>>> Currently, the msrs involved in setting up pvclock are not saved over >>>> migration and/or save/restore. This patch puts their value in special >>>> fields in our CPUState, and deal with them using vmstate. >>>> >>>> kvm also has to account for it, by including them in the msr list >>>> for the ioctls. >>> >>> Fails to build. Fedora 11 machine with distro kernel. >>> >>> http://repo.or.cz/w/qemu/pdb.git?a=blob;f=build.log;h=5b8a2e83116dc6a74e5045e30c3efe71fa7397ef;hb=0b4f913c89b89b45d3844dc1cf321e3d94871ecb >> strange message. Do we need --kerneldir for that? > > Requiring --kerneldir for a successful build isn't going to fly. > >> kvm_para.h should be available... > > Nope: > > $ rpm -ql kernel-headers | grep kvm > /usr/include/asm/kvm.h > /usr/include/linux/kvm.h > > Did you ever build this? > You should improve your patch qa ... btw, it builds fine for me even without --kerneldir, just tested. and the <linux/kvm_para.h> include is there since Feb08, a while now. /me wonders what the heck is going on...
On 10/21/09 21:39, Glauber Costa wrote: >> $ rpm -ql kernel-headers | grep kvm >> /usr/include/asm/kvm.h >> /usr/include/linux/kvm.h >> >> Did you ever build this? >> You should improve your patch qa ... > btw, it builds fine for me even without --kerneldir, just tested. > > and the<linux/kvm_para.h> include is there since Feb08, a while now. Which doesn't imply "make headers" actually includes that file. Even rawhide aka soon-to-be-F12 ships no kvm_para.h header file in kernel-headers. Just defining the msrs instead of using kvm_para.h should be fine, the numbers will not change ... cheers, Gerd
On Thu, Oct 22, 2009 at 10:00:37AM +0200, Gerd Hoffmann wrote: > On 10/21/09 21:39, Glauber Costa wrote: >>> $ rpm -ql kernel-headers | grep kvm >>> /usr/include/asm/kvm.h >>> /usr/include/linux/kvm.h >>> >>> Did you ever build this? >>> You should improve your patch qa ... >> btw, it builds fine for me even without --kerneldir, just tested. >> >> and the<linux/kvm_para.h> include is there since Feb08, a while now. > > Which doesn't imply "make headers" actually includes that file. Even > rawhide aka soon-to-be-F12 ships no kvm_para.h header file in > kernel-headers. > > Just defining the msrs instead of using kvm_para.h should be fine, the > numbers will not change ... That's not the problem. If one removes kvm_para.h, a lot of other things will not build. For example, the capabilities list. And it has been like this for almost a year now! Without that header, your system should have never successfully built a qemu-kvm instance. This is what happens here, if I try to remove it: error: ‘KVM_FEATURE_CLOCKSOURCE’ undeclared here (not in a function) error: ‘KVM_FEATURE_NOP_IO_DELAY’ undeclared here (not in a function) error: ‘KVM_FEATURE_MMU_OP’ undeclared here (not in a function) So again, wth?
>> Just defining the msrs instead of using kvm_para.h should be fine, the >> numbers will not change ... > That's not the problem. If one removes kvm_para.h, a lot of other things > will not build. For example, the capabilities list. > And it has been like this for almost a year now! > > Without that header, your system should have never successfully built a > qemu-kvm instance. This is what happens here, if I try to remove it: qemu != qemu-kvm. qemu-kvm ships with a copy of the kvm kernel header files. upstream qemu doesn't. cheers, Gerd
On Thu, Oct 22, 2009 at 02:15:06PM +0200, Gerd Hoffmann wrote: >>> Just defining the msrs instead of using kvm_para.h should be fine, the >>> numbers will not change ... >> That's not the problem. If one removes kvm_para.h, a lot of other things >> will not build. For example, the capabilities list. >> And it has been like this for almost a year now! >> >> Without that header, your system should have never successfully built a >> qemu-kvm instance. This is what happens here, if I try to remove it: > > qemu != qemu-kvm. > > qemu-kvm ships with a copy of the kvm kernel header files. > upstream qemu doesn't. ouch. so this is the source of the confusion. My bad!
On Thu, Oct 22, 2009 at 02:15:06PM +0200, Gerd Hoffmann wrote: >>> Just defining the msrs instead of using kvm_para.h should be fine, the >>> numbers will not change ... >> That's not the problem. If one removes kvm_para.h, a lot of other things >> will not build. For example, the capabilities list. >> And it has been like this for almost a year now! >> >> Without that header, your system should have never successfully built a >> qemu-kvm instance. This is what happens here, if I try to remove it: > > qemu != qemu-kvm. > > qemu-kvm ships with a copy of the kvm kernel header files. > upstream qemu doesn't. btw: I did tested qemu.git itself, but then I was using --kerneldir.
Glauber Costa wrote: > That's not the problem. If one removes kvm_para.h, a lot of other things > will not build. For example, the capabilities list. > And it has been like this for almost a year now! > > Without that header, your system should have never successfully built a > qemu-kvm instance. This is what happens here, if I try to remove it: > > error: ‘KVM_FEATURE_CLOCKSOURCE’ undeclared here (not in a function) > error: ‘KVM_FEATURE_NOP_IO_DELAY’ undeclared here (not in a function) > error: ‘KVM_FEATURE_MMU_OP’ undeclared here (not in a function) > > So again, wth? > FWIW, I always build with --kerneldir. I'd like to pull in KVM headers but every time we discuss it it seems to rat hole.
diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 5929d28..f589044 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -651,6 +651,8 @@ typedef struct CPUX86State { target_ulong fmask; target_ulong kernelgsbase; #endif + uint64_t system_time_msr; + uint64_t wall_clock_msr; uint64_t tsc; diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 0e69b57..8a6fa29 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -18,6 +18,7 @@ #include <sys/utsname.h> #include <linux/kvm.h> +#include <linux/kvm_para.h> #include "qemu-common.h" #include "sysemu.h" @@ -500,6 +501,9 @@ static int kvm_put_msrs(CPUState *env) kvm_msr_entry_set(&msrs[n++], MSR_LSTAR, env->lstar); } #endif + kvm_msr_entry_set(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_time_msr); + kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr); + msr_data.info.nmsrs = n; return kvm_vcpu_ioctl(env, KVM_SET_MSRS, &msr_data); @@ -634,6 +638,9 @@ static int kvm_get_msrs(CPUState *env) msrs[n++].index = MSR_LSTAR; } #endif + msrs[n++].index = MSR_KVM_SYSTEM_TIME; + msrs[n++].index = MSR_KVM_WALL_CLOCK; + msr_data.info.nmsrs = n; ret = kvm_vcpu_ioctl(env, KVM_GET_MSRS, &msr_data); if (ret < 0) @@ -670,6 +677,12 @@ static int kvm_get_msrs(CPUState *env) case MSR_IA32_TSC: env->tsc = msrs[i].data; break; + case MSR_KVM_SYSTEM_TIME: + env->system_time_msr = msrs[i].data; + break; + case MSR_KVM_WALL_CLOCK: + env->wall_clock_msr = msrs[i].data; + break; } } diff --git a/target-i386/machine.c b/target-i386/machine.c index b13eff4..f6fe216 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -475,6 +475,9 @@ const VMStateDescription vmstate_cpu = { VMSTATE_UINT64_ARRAY_V(mce_banks, CPUState, MCE_BANKS_DEF *4, 10), /* rdtscp */ VMSTATE_UINT64_V(tsc_aux, CPUState, 11), + /* KVM pvclock msr */ + VMSTATE_UINT64_V(system_time_msr, CPUState, 11), + VMSTATE_UINT64_V(wall_clock_msr, CPUState, 11), VMSTATE_END_OF_LIST() } };