On 02.10.2013, at 19:49, Scott Wood wrote: > On Wed, 2013-10-02 at 19:46 +0200, Alexander Graf wrote: >> On 02.10.2013, at 19:42, Scott Wood wrote: >> >>> On Wed, 2013-10-02 at 19:17 +0200, Alexander Graf wrote: >>>> On 02.10.2013, at 19:04, Scott Wood wrote: >>>> >>>>> On Wed, 2013-10-02 at 18:53 +0200, Alexander Graf wrote: >>>>>> On 02.10.2013, at 18:40, Scott Wood wrote: >>>>>> >>>>>>> On Wed, 2013-10-02 at 16:19 +0200, Alexander Graf wrote: >>>>>>>> Won't this break when CONFIG_EPAPR_PARAVIRT=n? We wouldn't have epapr_hcalls.S compiled into the code base then and the bl above would reference an unknown function. >>>>>>> >>>>>>> KVM_GUEST selects EPAPR_PARAVIRT. >>>>>> >>>>>> But you can not select KVM_GUEST and still call these inline functions, no? >>>>> >>>>> No. >>>>> >>>>>> Like kvm_arch_para_features(). >>>>> >>>>> Where does that get called without KVM_GUEST? >>>>> >>>>> How would that work currently, with the call to kvm_hypercall() in >>>>> arch/powerpc/kernel/kvm.c (which calls epapr_hypercall, BTW)? >>>> >>>> It wouldn't ever get called because kvm_hypercall() ends up always returning EV_UNIMPLEMENTED when #ifndef CONFIG_KVM_GUEST. >>> >>> OK, so the objection is to removing that stub? Where would we actually >>> want to call this without knowing that KVM_GUEST or EPAPR_PARAVIRT are >>> enabled? >> >> In probing code. I usually prefer >> >> if (kvm_feature_available(X)) { >> ... >> } >> >> over >> >> #ifdef CONFIG_KVM_GUEST >> if (kvm_feature_available(X)) { >> ... >> } >> #endif >> >> at least when I can avoid it. With the current code the compiler would be smart enough to just optimize out the complete branch. > > Sure. My point is, where would you be calling that where the entire > file isn't predicated on (or selecting) CONFIG_KVM_GUEST or similar? > > We don't do these stubs for every single function in the kernel -- only > ones where the above is a reasonable use case. Yeah, I'm fine on dropping it, but we need to make that a conscious decision and verify that no caller relies on it. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html