> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, October 03, 2013 12:04 AM
> To: Alexander Graf
> Cc: Bhushan Bharat-R65777; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Bhushan
> Bharat-R65777
> Subject: Re: [PATCH 1/2] kvm/powerpc: rename kvm_hypercall() to
> epapr_hypercall()
>
> On Wed, 2013-10-02 at 19:54 +0200, Alexander Graf wrote:
> > 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.
>
> 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 ?
Thanks
-Bharat
��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
