> >>>>>>> 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.
> >>>>
> >>>> kvm_para_has_feature() is called from arch/powerpc/kernel/kvm.c,
> >>>> arch/x86/kernel/kvm.c, and arch/x86/kernel/kvmclock.c, all of which
> >>>> are enabled by CONFIG_KVM_GUEST.
> >>>>
> >>>> I did find one example of kvm_para_available() being used in an
> >>>> unexpected place
> >>>> -- sound/pci/intel8x0.c. It defines its own non-CONFIG_KVM_GUEST
> >>>> stub, even though x86 defines kvm_para_available() using inline
> >>>> CPUID stuff which should work without CONFIG_KVM_GUEST.
> >>>> I'm not sure why it even needs to do that, though -- shouldn't the
> >>>> subsequent PCI subsystem vendor/device check should be sufficient?
> >>>> No hypercalls are involved.
> >>>>
> >>>> That said, the possibility that some random driver might want to
> >>>> make use of paravirt features is a decent argument for keeping the stub.
> >>>>
> >>>
> >>> I am not sure where we are agreeing on?
> >>> Do we want to remove the stub in arch/powerpc/include/asm/kvm_para.h
> >>> ? as
> >> there is no caller without KVM_GUEST and in future caller ensure this
> >> to be called only from code selected by KVM_GUEST?
> >>>
> >>> Or let this stub stay to avoid any random driver calling this ?
> >>
> >> I think the most reasonable way forward is to add a stub for
> >> non-CONFIG_EPAPR to the epapr code, then replace the kvm bits with
> >> generic epapr bits (which your patches already do).
> >
> > Please describe which stub you are talking about.
>
> kvm_hypercall is always available, regardless of the config option, which makes
> all its subfunctions always available as well.
This patch renames kvm_hypercall() to epapr_hypercall() and which is always available. And the kvm_hypercall() friends now directly calls epapr_hypercall().
IIUC, So what you are trying to say is let the kvm_hypercall() friends keep on calling kvm_hypercall() itself and a sub something like this:
--------
#ifdef CONFIG_KVM_GUEST
static unsigned long kvm_hypercall(unsigned long *in,
unsigned long *out,
unsigned long nr)
{
return epapr_hypercall(in, out. nr);
}
#else
static unsigned long kvm_hypercall(unsigned long *in,
unsigned long *out,
unsigned long nr) {
return EV_UNIMPLEMENTED;
}
---------
I am still not really convinced about why we want to keep this stub where we know this is not called outside KVM_GUEST and calling this without KVM_GUEST is debatable.
Thanks
-Bharat
Thanks
-Bharat
>
>
> Alex
>
> ---
>
> #ifdef CONFIG_KVM_GUEST
>
> #include <linux/of.h>
>
> static inline int kvm_para_available(void) {
> struct device_node *hyper_node;
>
> hyper_node = of_find_node_by_path("/hypervisor");
> if (!hyper_node)
> return 0;
>
> if (!of_device_is_compatible(hyper_node, "linux,kvm"))
> return 0;
>
> return 1;
> }
>
> extern unsigned long kvm_hypercall(unsigned long *in,
> unsigned long *out,
> unsigned long nr);
>
> #else
>
> static inline int kvm_para_available(void) {
> return 0;
> }
>
> static unsigned long kvm_hypercall(unsigned long *in,
> unsigned long *out,
> unsigned long nr) {
> return EV_UNIMPLEMENTED;
> }
>
> #endif
>
--
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
