* [Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks @ 2009-11-09 21:05 Hollis Blanchard 2009-11-09 21:05 ` [Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code Hollis Blanchard 2009-11-09 21:05 ` [Qemu-devel] [PATCH 2 of 2] kvm ppc: Remove unused label Hollis Blanchard 0 siblings, 2 replies; 8+ messages in thread From: Hollis Blanchard @ 2009-11-09 21:05 UTC (permalink / raw) To: aliguori; +Cc: qemu-devel, kvm-ppc Hi Anthony, please apply these two patches to fix the PowerPC KVM build. -Hollis ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 21:05 [Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks Hollis Blanchard @ 2009-11-09 21:05 ` Hollis Blanchard 2009-11-09 22:12 ` [Qemu-devel] " Jan Kiszka 2009-11-09 21:05 ` [Qemu-devel] [PATCH 2 of 2] kvm ppc: Remove unused label Hollis Blanchard 1 sibling, 1 reply; 8+ messages in thread From: Hollis Blanchard @ 2009-11-09 21:05 UTC (permalink / raw) To: aliguori; +Cc: qemu-devel, kvm-ppc Unbreaks PowerPC and S390 KVM builds. Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> diff --git a/kvm-all.c b/kvm-all.c --- a/kvm-all.c +++ b/kvm-all.c @@ -207,26 +207,6 @@ err: return ret; } -int kvm_put_mp_state(CPUState *env) -{ - struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; - - return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); -} - -int kvm_get_mp_state(CPUState *env) -{ - struct kvm_mp_state mp_state; - int ret; - - ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); - if (ret < 0) { - return ret; - } - env->mp_state = mp_state.mp_state; - return 0; -} - /* * dirty pages logging control */ diff --git a/kvm.h b/kvm.h --- a/kvm.h +++ b/kvm.h @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, int kvm_vcpu_ioctl(CPUState *env, int type, ...); -int kvm_get_mp_state(CPUState *env); -int kvm_put_mp_state(CPUState *env); - /* Arch specific hooks */ int kvm_arch_post_run(CPUState *env, struct kvm_run *run); diff --git a/target-i386/kvm.c b/target-i386/kvm.c --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env) return 0; } +static int kvm_put_mp_state(CPUState *env) +{ + struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; + + return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); +} + +static int kvm_get_mp_state(CPUState *env) +{ + struct kvm_mp_state mp_state; + int ret; + + ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); + if (ret < 0) { + return ret; + } + env->mp_state = mp_state.mp_state; + return 0; +} + int kvm_arch_put_registers(CPUState *env) { int ret; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 21:05 ` [Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code Hollis Blanchard @ 2009-11-09 22:12 ` Jan Kiszka 2009-11-09 22:21 ` Alexander Graf ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Jan Kiszka @ 2009-11-09 22:12 UTC (permalink / raw) To: Hollis Blanchard; +Cc: Anthony Liguori, qemu-devel, kvm-ppc [-- Attachment #1: Type: text/plain, Size: 2271 bytes --] Hollis Blanchard wrote: > Unbreaks PowerPC and S390 KVM builds. What breaks precisely? Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be shared with ia64 - one day. We could still move things back then, but maybe we can handle the build issues already in place, specifically as qemu-kvm is carrying this in generic code since ages. Jan > > Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> > > diff --git a/kvm-all.c b/kvm-all.c > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -207,26 +207,6 @@ err: > return ret; > } > > -int kvm_put_mp_state(CPUState *env) > -{ > - struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; > - > - return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); > -} > - > -int kvm_get_mp_state(CPUState *env) > -{ > - struct kvm_mp_state mp_state; > - int ret; > - > - ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); > - if (ret < 0) { > - return ret; > - } > - env->mp_state = mp_state.mp_state; > - return 0; > -} > - > /* > * dirty pages logging control > */ > diff --git a/kvm.h b/kvm.h > --- a/kvm.h > +++ b/kvm.h > @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, > > int kvm_vcpu_ioctl(CPUState *env, int type, ...); > > -int kvm_get_mp_state(CPUState *env); > -int kvm_put_mp_state(CPUState *env); > - > /* Arch specific hooks */ > > int kvm_arch_post_run(CPUState *env, struct kvm_run *run); > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env) > return 0; > } > > +static int kvm_put_mp_state(CPUState *env) > +{ > + struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; > + > + return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); > +} > + > +static int kvm_get_mp_state(CPUState *env) > +{ > + struct kvm_mp_state mp_state; > + int ret; > + > + ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); > + if (ret < 0) { > + return ret; > + } > + env->mp_state = mp_state.mp_state; > + return 0; > +} > + > int kvm_arch_put_registers(CPUState *env) > { > int ret; [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 22:12 ` [Qemu-devel] " Jan Kiszka @ 2009-11-09 22:21 ` Alexander Graf 2009-11-09 22:23 ` Hollis Blanchard 2009-11-09 23:22 ` Anthony Liguori 2 siblings, 0 replies; 8+ messages in thread From: Alexander Graf @ 2009-11-09 22:21 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-ppc, Anthony Liguori, qemu-devel, Hollis Blanchard On 09.11.2009, at 23:12, Jan Kiszka wrote: > Hollis Blanchard wrote: >> Unbreaks PowerPC and S390 KVM builds. > > What breaks precisely? > > Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be > shared with ia64 - one day. We could still move things back then, but > maybe we can handle the build issues already in place, specifically as > qemu-kvm is carrying this in generic code since ages. The "generic" code tries to access env->mp_state which doesn't exist in PPC's env because it's only defined in the X86 env. I honestly don't care where to move things. And I don't think anyone else should. Either move it to CPU_COMMON if you think it's generic or move the code to the platform specific part. We've been going through this at least 2 times in previous threads. But qemu HEAD has been broken for PPC kvm since MP states got introduced and it's no fun talking against walls on this. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 22:12 ` [Qemu-devel] " Jan Kiszka 2009-11-09 22:21 ` Alexander Graf @ 2009-11-09 22:23 ` Hollis Blanchard 2009-11-09 23:03 ` Jan Kiszka 2009-11-09 23:22 ` Anthony Liguori 2 siblings, 1 reply; 8+ messages in thread From: Hollis Blanchard @ 2009-11-09 22:23 UTC (permalink / raw) To: Jan Kiszka; +Cc: kvm-ppc, Anthony Liguori, qemu-devel, Hollis Blanchard This is the technique Anthony requested this morning, rather than relocating mp_state. To be honest, I don't care which way it's fixed; this is at least the 3rd patch to solve this seemingly trivial problem. /home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state': /home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct CPUPPCState' has no member named 'mp_state' -Hollis On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote: > Hollis Blanchard wrote: >> Unbreaks PowerPC and S390 KVM builds. > > What breaks precisely? > > Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be > shared with ia64 - one day. We could still move things back then, but > maybe we can handle the build issues already in place, specifically as > qemu-kvm is carrying this in generic code since ages. > > Jan > >> >> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> >> >> diff --git a/kvm-all.c b/kvm-all.c >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -207,26 +207,6 @@ err: >> return ret; >> } >> >> -int kvm_put_mp_state(CPUState *env) >> -{ >> - struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; >> - >> - return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); >> -} >> - >> -int kvm_get_mp_state(CPUState *env) >> -{ >> - struct kvm_mp_state mp_state; >> - int ret; >> - >> - ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); >> - if (ret < 0) { >> - return ret; >> - } >> - env->mp_state = mp_state.mp_state; >> - return 0; >> -} >> - >> /* >> * dirty pages logging control >> */ >> diff --git a/kvm.h b/kvm.h >> --- a/kvm.h >> +++ b/kvm.h >> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, >> >> int kvm_vcpu_ioctl(CPUState *env, int type, ...); >> >> -int kvm_get_mp_state(CPUState *env); >> -int kvm_put_mp_state(CPUState *env); >> - >> /* Arch specific hooks */ >> >> int kvm_arch_post_run(CPUState *env, struct kvm_run *run); >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >> --- a/target-i386/kvm.c >> +++ b/target-i386/kvm.c >> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env) >> return 0; >> } >> >> +static int kvm_put_mp_state(CPUState *env) >> +{ >> + struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; >> + >> + return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); >> +} >> + >> +static int kvm_get_mp_state(CPUState *env) >> +{ >> + struct kvm_mp_state mp_state; >> + int ret; >> + >> + ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); >> + if (ret < 0) { >> + return ret; >> + } >> + env->mp_state = mp_state.mp_state; >> + return 0; >> +} >> + >> int kvm_arch_put_registers(CPUState *env) >> { >> int ret; > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 22:23 ` Hollis Blanchard @ 2009-11-09 23:03 ` Jan Kiszka 0 siblings, 0 replies; 8+ messages in thread From: Jan Kiszka @ 2009-11-09 23:03 UTC (permalink / raw) To: Hollis Blanchard; +Cc: kvm-ppc, Anthony Liguori, qemu-devel, Hollis Blanchard [-- Attachment #1: Type: text/plain, Size: 3157 bytes --] Hollis Blanchard wrote: > This is the technique Anthony requested this morning, rather than > relocating mp_state. Then let's move it. There is not that much code to share anyway. > To be honest, I don't care which way it's fixed; > this is at least the 3rd patch to solve this seemingly trivial > problem. That's indeed unfortunate, but I think to remember that not all of these attempts were clean. Jan > > /home/hollisb/source/qemu-fresh.hg/kvm-all.c: In function 'kvm_put_mp_state': > /home/hollisb/source/qemu-fresh.hg/kvm-all.c:212: error: 'struct > CPUPPCState' has no member named 'mp_state' > > -Hollis > > On Mon, Nov 9, 2009 at 2:12 PM, Jan Kiszka <jan.kiszka@web.de> wrote: >> Hollis Blanchard wrote: >>> Unbreaks PowerPC and S390 KVM builds. >> What breaks precisely? >> >> Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be >> shared with ia64 - one day. We could still move things back then, but >> maybe we can handle the build issues already in place, specifically as >> qemu-kvm is carrying this in generic code since ages. >> >> Jan >> >>> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> >>> >>> diff --git a/kvm-all.c b/kvm-all.c >>> --- a/kvm-all.c >>> +++ b/kvm-all.c >>> @@ -207,26 +207,6 @@ err: >>> return ret; >>> } >>> >>> -int kvm_put_mp_state(CPUState *env) >>> -{ >>> - struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; >>> - >>> - return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); >>> -} >>> - >>> -int kvm_get_mp_state(CPUState *env) >>> -{ >>> - struct kvm_mp_state mp_state; >>> - int ret; >>> - >>> - ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); >>> - if (ret < 0) { >>> - return ret; >>> - } >>> - env->mp_state = mp_state.mp_state; >>> - return 0; >>> -} >>> - >>> /* >>> * dirty pages logging control >>> */ >>> diff --git a/kvm.h b/kvm.h >>> --- a/kvm.h >>> +++ b/kvm.h >>> @@ -74,9 +74,6 @@ int kvm_vm_ioctl(KVMState *s, int type, >>> >>> int kvm_vcpu_ioctl(CPUState *env, int type, ...); >>> >>> -int kvm_get_mp_state(CPUState *env); >>> -int kvm_put_mp_state(CPUState *env); >>> - >>> /* Arch specific hooks */ >>> >>> int kvm_arch_post_run(CPUState *env, struct kvm_run *run); >>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c >>> --- a/target-i386/kvm.c >>> +++ b/target-i386/kvm.c >>> @@ -659,6 +659,26 @@ static int kvm_get_msrs(CPUState *env) >>> return 0; >>> } >>> >>> +static int kvm_put_mp_state(CPUState *env) >>> +{ >>> + struct kvm_mp_state mp_state = { .mp_state = env->mp_state }; >>> + >>> + return kvm_vcpu_ioctl(env, KVM_SET_MP_STATE, &mp_state); >>> +} >>> + >>> +static int kvm_get_mp_state(CPUState *env) >>> +{ >>> + struct kvm_mp_state mp_state; >>> + int ret; >>> + >>> + ret = kvm_vcpu_ioctl(env, KVM_GET_MP_STATE, &mp_state); >>> + if (ret < 0) { >>> + return ret; >>> + } >>> + env->mp_state = mp_state.mp_state; >>> + return 0; >>> +} >>> + >>> int kvm_arch_put_registers(CPUState *env) >>> { >>> int ret; >> >> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 257 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] Re: [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code 2009-11-09 22:12 ` [Qemu-devel] " Jan Kiszka 2009-11-09 22:21 ` Alexander Graf 2009-11-09 22:23 ` Hollis Blanchard @ 2009-11-09 23:22 ` Anthony Liguori 2 siblings, 0 replies; 8+ messages in thread From: Anthony Liguori @ 2009-11-09 23:22 UTC (permalink / raw) To: Jan Kiszka; +Cc: hollisb, kvm-ppc, qemu-devel Jan Kiszka wrote: > Hollis Blanchard wrote: > >> Unbreaks PowerPC and S390 KVM builds. >> > > What breaks precisely? > > Note that KVM_GET/SET_MP_STATE are generic IOCTLs and supposed to be > shared with ia64 - one day. We could still move things back then, but > maybe we can handle the build issues already in place, specifically as > qemu-kvm is carrying this in generic code since ages. > mp_state is pretty specific to the in-kernel apic. ia64 may share because it shares an interrupt controller but that does not make it generic. That just makes two architectures have a common field. I don't think there's anything wrong with kvm exposing this as a common ioctl but from a qemu perspective, it makes no sense as a common cpu field. It only makes sense when using kvm and when using an in-kernel apic. That's very architecture specific from an qemu perspective. -- Regards, Anthony Liguori ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2 of 2] kvm ppc: Remove unused label 2009-11-09 21:05 [Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks Hollis Blanchard 2009-11-09 21:05 ` [Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code Hollis Blanchard @ 2009-11-09 21:05 ` Hollis Blanchard 1 sibling, 0 replies; 8+ messages in thread From: Hollis Blanchard @ 2009-11-09 21:05 UTC (permalink / raw) To: aliguori; +Cc: qemu-devel, kvm-ppc Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com> diff --git a/target-ppc/kvm_ppc.c b/target-ppc/kvm_ppc.c --- a/target-ppc/kvm_ppc.c +++ b/target-ppc/kvm_ppc.c @@ -52,7 +52,6 @@ close: fclose(f); free: free(path); -out: return ret; } ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-11-09 23:22 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-09 21:05 [Qemu-devel] [PATCH 0 of 2] Fix PowerPC KVM build breaks Hollis Blanchard 2009-11-09 21:05 ` [Qemu-devel] [PATCH 1 of 2] kvm: Move KVM mp_state accessors to i386-specific code Hollis Blanchard 2009-11-09 22:12 ` [Qemu-devel] " Jan Kiszka 2009-11-09 22:21 ` Alexander Graf 2009-11-09 22:23 ` Hollis Blanchard 2009-11-09 23:03 ` Jan Kiszka 2009-11-09 23:22 ` Anthony Liguori 2009-11-09 21:05 ` [Qemu-devel] [PATCH 2 of 2] kvm ppc: Remove unused label Hollis Blanchard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).