Re: [PATCH 1/2] kvm/powerpc: rename kvm_hypercall() to epapr_hypercall()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07.10.2013, at 17:43, Bhushan Bharat-R65777 <R65777@xxxxxxxxxxxxx> 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()

I prefer b, Scott prefers b.


Alex

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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux