> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Monday, October 07, 2013 9:43 PM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org
> Subject: Re: [PATCH 1/2] kvm/powerpc: rename kvm_hypercall() to
> epapr_hypercall()
>
>
> On 07.10.2013, at 18:04, Bhushan Bharat-R65777 <r65...@freescale.com> wrote:
>
> >
> >
> >> -----Original Message-----
> >> From: kvm-ppc-ow...@vger.kernel.org
> >> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Monday, October 07, 2013 9:16 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org
> >> Subject: Re: [PATCH 1/2] kvm/powerpc: rename kvm_hypercall() to
> >> epapr_hypercall()
> >>
> >>
> >> On 07.10.2013, at 17:43, Bhushan Bharat-R65777 <r65...@freescale.com>
> >> wrote:
> >>
> >>>>>>>>>>> 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:
> >>
> >> No, what I'm saying is that we either
> >>
> >> a) drop the whole #ifndef code path consciously. This would have to
> >> be a separate patch with a separate discussion. It's orthogonal to
> >> combining
> >> kvm_hypercall() and epapr_hypercall()
> >>
> >> b) add the #ifndef path to epapr_hypercall()
> >
> > Do you mean like this in arch/powerpc/include/asm/epapr_hcalls.h
> >
> > #ifdef CONFIG_KVM_GUEST
>
> CONFIG_EPAPR_PARAVIRT
Yes, I was getting confused why only KVM_GUEST as this not specific to
KVM-GUEST.
Thank you
>
> Apart from that, yes, I think that's what we want.
>
>
> Alex
>
> > static inline unsigned long epapr_hypercall(unsigned long *in,
> > unsigned long *out,
> > unsigned long nr) { // code for this
> > function } #else static inline unsigned long epapr_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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
