| Message ID | 6776cc5c00f4d06d23c11b68d18fd96e27f71718.1328438750.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
Jan Kiszka <jan.kiszka@web.de> wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > To be used for in-kernel PIT emulation. .... > + int pit_state2; This is used as a bool. > int xsave, xcrs; > int many_ioeventfds; > int irqchip_inject_ioctl; > @@ -954,6 +955,10 @@ int kvm_init(void) > s->xcrs = kvm_check_extension(s, KVM_CAP_XCRS); > #endif > > +#ifdef KVM_CAP_PIT_STATE2 > + s->pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2); > +#endif > + [ this happened to me when I was reviewing this patch, but culprit is not this patch] really kvm_check_extension() should also return a bool, but that is a bigger change that this patch series tend to introduce. So, I went to "man ioctl" > RETURN VALUE > Usually, on success zero is returned. A few ioctl() requests use the > return value as an output parameter and return a nonnegative value on > success. On error, -1 is returned, and errno is set appropriately. Usually is the important word there. And then went to kvm-all.c int kvm_check_extension(KVMState *s, unsigned int extension) { int ret; ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, extension); if (ret < 0) { ret = 0; } return ret; } What? it allways return zero? Something should be wrong here. I will expect kvm_check_extension() to work by now. Yes, kvm_ioctl() return 1 went the extension is there, just for the people confused like me. Later, Juan.
On 2012-02-05 21:03, Juan Quintela wrote: > Jan Kiszka <jan.kiszka@web.de> wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> To be used for in-kernel PIT emulation. > > .... > >> + int pit_state2; > > This is used as a bool. > >> int xsave, xcrs; >> int many_ioeventfds; >> int irqchip_inject_ioctl; >> @@ -954,6 +955,10 @@ int kvm_init(void) >> s->xcrs = kvm_check_extension(s, KVM_CAP_XCRS); >> #endif >> >> +#ifdef KVM_CAP_PIT_STATE2 >> + s->pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2); >> +#endif >> + > > [ this happened to me when I was reviewing this patch, but culprit is > not this patch] > > really kvm_check_extension() should also return a bool, but that is a > bigger change that this patch series tend to introduce. > > So, I went to "man ioctl" > >> RETURN VALUE >> Usually, on success zero is returned. A few ioctl() requests use the >> return value as an output parameter and return a nonnegative value on >> success. On error, -1 is returned, and errno is set appropriately. > > Usually is the important word there. Right, because the driver decides in the end what is returned. In case of KVM_CHECK_EXTENSION, it is > 0 for supported. Sometimes it returns an integer value that encode "how much" is supported, e.g. KVM_CAP_IRQ_ROUTING provides the number of supported GSI routes. That said, all those boolean caps could indeed be encoded as such. Some smarter way to initialize and retrieve them would also be nice. Too much boilerplate code. Thanks for bending your brain around this :) Jan
diff --git a/kvm-all.c b/kvm-all.c index c4babda..bddf922 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -75,6 +75,7 @@ struct KVMState struct kvm_sw_breakpoint_head kvm_sw_breakpoints; #endif int pit_in_kernel; + int pit_state2; int xsave, xcrs; int many_ioeventfds; int irqchip_inject_ioctl; @@ -954,6 +955,10 @@ int kvm_init(void) s->xcrs = kvm_check_extension(s, KVM_CAP_XCRS); #endif +#ifdef KVM_CAP_PIT_STATE2 + s->pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2); +#endif + ret = kvm_arch_init(s); if (ret < 0) { goto err; @@ -1291,6 +1296,11 @@ int kvm_has_xcrs(void) return kvm_state->xcrs; } +int kvm_has_pit_state2(void) +{ + return kvm_state->pit_state2; +} + int kvm_has_many_ioeventfds(void) { if (!kvm_enabled()) { diff --git a/kvm-stub.c b/kvm-stub.c index f63a0d2..1f1c686 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -78,6 +78,11 @@ int kvm_allows_irq0_override(void) return 1; } +int kvm_has_pit_state2(void) +{ + return 0; +} + void kvm_setup_guest_memory(void *start, size_t size) { } diff --git a/kvm.h b/kvm.h index f9f1dc8..8ef4476 100644 --- a/kvm.h +++ b/kvm.h @@ -54,6 +54,7 @@ int kvm_has_robust_singlestep(void); int kvm_has_debugregs(void); int kvm_has_xsave(void); int kvm_has_xcrs(void); +int kvm_has_pit_state2(void); int kvm_has_many_ioeventfds(void); int kvm_has_gsi_routing(void);