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.
-Scott
--
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
