[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_r
From: |
Eduardo Habkost |
Subject: |
Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn() |
Date: |
Thu, 17 Dec 2020 20:05:40 -0500 |
On Fri, Dec 18, 2020 at 01:07:33AM +0100, Claudio Fontana wrote:
> On 12/18/20 12:47 AM, Eduardo Habkost wrote:
> > On Fri, Dec 18, 2020 at 12:34:46AM +0100, Claudio Fontana wrote:
> >> On 12/17/20 11:53 PM, Eduardo Habkost wrote:
> >>> On Thu, Dec 17, 2020 at 11:33:57PM +0100, Claudio Fontana wrote:
> >>>> Hello all,
> >>>>
> >>>> On 12/17/20 7:46 PM, Eduardo Habkost wrote:
> >>>>> From: Vitaly Kuznetsov <vkuznets@redhat.com>
> >>>>>
> >>>>> As a preparation to expanding Hyper-V CPU features early, move
> >>>>> hyperv_vendor_id initialization to x86_cpu_realizefn(). Introduce
> >>>>> x86_cpu_hyperv_realize() to not not pollute x86_cpu_realizefn()
> >>>>> itself.
> >>>>
> >>>> this seems to fit very well the ongoing work on separating accelerator
> >>>> specific realize functions;
> >>>>
> >>>> related to the previous discussions about the class hierarchies,
> >>>> do you think that we should have a separate class in target/i386/kvm/
> >>>> for a hyperv variant of the kvm-cpu.c?
> >>>>
> >>>> Should it be a separate class or a subclass of "kvm-accel-x86_64-cpu" ?
> >>>
> >>> I don't see how a separate QOM class for Hyper-V would be helpful
> >>> here. What would be the problem you are trying to solve in this
> >>> case?
> >>
> >> there is now a call to accelerator specific code x86_hyperv_cpu_realize
> >> just before cpu_exec_realize,
> >> tying the generic target/i386/cpu.c code to kvm/hyperv-specific accel
> >> initialization.
> >>
> >> if this is just a feature of the kvm accel, maybe I should just move all
> >> to kvm-cpu.c and that's it.
> >
> > That would make sense. If we decide this is a KVM-specific
> > feature, this code can be moved to kvm_cpu_realizefn(), provided
> > by the kvm-accel-x86_64-cpu class added by your series.
> >
> > However, I'm not sure we can say this is a KVM-specific feature.
> > The feature is currently only supported by the KVM accelerator,
> > but I'd say it is a generic feature.
> >
>
> Maybe in the future it will be a generic feature, and we can export it the
> right way?
> It will be super-easy if the feature is well contained.
>
> Until it really is generic though, should it really appear in the middle of
> x86_cpu_realizefn?
> currently the generic code in target/i386/cpu.c contains an unconditional
> call to:
>
> x86_cpu_hyperv_realize(cpu);
>
> before the call to cpu_exec_realizefn().
>
> the function is defined in this very same file before as:
>
> static void x86_cpu_hyperv_realize(X86CPU *cpu)
> {
> ...
> }
>
> with a bunch of initializations of hyperv specific data, for example
> cpu->hyperv_interface_id, cpu->hyperv_vendor_id, cpu->hyperv_version_id.
>
> That data is ever only used in kvm/kvm.c, which is built conditionally under
> CONFIG_KVM.
>
> So the existing situation is fairly inconsistent in my view, at least for how
> things are now, and in principle the extra code of the initializations for
> hyperv should never be executed if !CONFIG_KVM.
What's the problem you are trying to solve? Speed? Binary size?
Something else?
Moving the code in x86_cpu_hyperv_realize() to kvm-cpu.c sounds
like a drop in the bucket. It wouldn't get rid of
hyperv-specific code in cpu.c, which includes:
- the "hv-*" properties in x86_cpu_properties[];
- FEAT_HYPERV_* CPUID leaf definitions;
- hyperv code in x86_cpu_get_crash_info().
I wouldn't object if somebody really wants to move it, but I
don't see it as a problem.
--
Eduardo
- [PULL 02/17] i386: move whpx accel files into whpx/, (continued)
- [PULL 02/17] i386: move whpx accel files into whpx/, Eduardo Habkost, 2020/12/17
- [PULL 04/17] i386: hvf: remove stale MAINTAINERS entry for old hvf stubs, Eduardo Habkost, 2020/12/17
- [PULL 05/17] i386: move TCG accel files into tcg/, Eduardo Habkost, 2020/12/17
- [PULL 06/17] i386: move cpu dump out of helper.c into cpu-dump.c, Eduardo Habkost, 2020/12/17
- [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Claudio Fontana, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Claudio Fontana, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Claudio Fontana, 2020/12/17
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(),
Eduardo Habkost <=
- Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn(), Claudio Fontana, 2020/12/18
[PULL 09/17] i386: move hyperv_version_id initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
[PULL 08/17] i386: move hyperv_interface_id initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
[PULL 13/17] i386: tcg: remove inline from cpu_load_eflags, Eduardo Habkost, 2020/12/17
[PULL 10/17] i386: move hyperv_limits initialization to x86_cpu_realizefn(), Eduardo Habkost, 2020/12/17
[PULL 12/17] i386: move TCG cpu class initialization to tcg/, Eduardo Habkost, 2020/12/17
[PULL 15/17] tcg: make CPUClass.cpu_exec_* optional, Eduardo Habkost, 2020/12/17
[PULL 11/17] x86/cpu: Add AVX512_FP16 cpu feature, Eduardo Habkost, 2020/12/17
[PULL 14/17] tcg: cpu_exec_{enter,exit} helpers, Eduardo Habkost, 2020/12/17
[PULL 17/17] cpu: Remove unnecessary noop methods, Eduardo Habkost, 2020/12/17