Re: [PULL 07/17] i386: move hyperv_vendor_id initialization to x86_cpu_realizefn()

[Eduardo Habkost]

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.

-- 
Eduardo