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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
