On Mon, Mar 06, 2017 at 10:06:27AM +1100, David Gibson wrote:
> On Fri, Mar 03, 2017 at 11:58:07AM -0300, Eduardo Habkost wrote:
> > On Tue, Jan 31, 2017 at 02:11:59PM +0100, Thomas Huth wrote:
> > > When running with KVM on POWER, we register some CPU types during
> > > the initialization function of the ppc64 KVM code (which unfortunately
> > > also can not be done via a type_init() like it is done on x86).
> >
> > Can you elaborate why it can't be done via type_init()? If the
> > QOM type hierarchy depends on any runtime data unavailable at
> > type_init(), we should fix that.
>
> Hmm.. how? This is specifically for the special 'host' cpu in the
> case of KVM. We can't use a static configuration here, because there
> are things on the host that could limit what features of the CPU are
> available for guest use. So, we need KVM to be initialized in order
> to query that information.
There's nothing saying you need to query that information before
type_register() or during class_init, does it? The behavior of
TYPE_HOST_POWERPC_CPU after object_new() is called can be as
dynamic as you want it to, but the QOM type hierarchy is supposed
to be static.
Registering QOM types outside type_init() is only causing us
problems and we should stop doing that.
>
> > > So to
> > > be able to see these updates in the CPU help text, the code that calls
> > > list_cpus() has to be run after configure_accelerator(). This move should
> > > be fine since the "cpu_model" variable is also never used before the call
> > > to configure_accelerator(), and thus there should not be any unwanted
> > > side effects in the code before configure_accelerator() if the user
> > > started QEMU with "-cpu ?" or "-cpu help".
> > >
> > > Signed-off-by: Thomas Huth <th...@redhat.com>
> >
> > I am not convinced that the output of "-cpu help" and
> > "-cpu help -machine accel=kvm" should look different. Do you have
> > an example of what exactly is wrong with the output currently?
> >
> > I actually believe list_cpus() needs to be called _earlier_, not
> > later. Otherwise we won't be able to fix this bug:
> >
> > $ qemu-system-arm -cpu help
> > qemu-system-arm:/usr/local/etc/qemu/qemu.conf:1: No machine specified,
> > and there is no default
> > Use -machine help to list supported machines
> > $
> >
> > > ---
> > > vl.c | 10 +++++-----
> > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/vl.c b/vl.c
> > > index 0b72b12..315c5c3 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -4055,11 +4055,6 @@ int main(int argc, char **argv, char **envp)
> > > qemu_set_hw_version(machine_class->hw_version);
> > > }
> > >
> > > - if (cpu_model && is_help_option(cpu_model)) {
> > > - list_cpus(stdout, &fprintf, cpu_model);
> > > - exit(0);
> > > - }
> > > -
> > > if (!trace_init_backends()) {
> > > exit(1);
> > > }
> > > @@ -4298,6 +4293,11 @@ int main(int argc, char **argv, char **envp)
> > >
> > > configure_accelerator(current_machine);
> > >
> > > + if (cpu_model && is_help_option(cpu_model)) {
> > > + list_cpus(stdout, &fprintf, cpu_model);
> > > + exit(0);
> > > + }
> > > +
> > > if (qtest_chrdev) {
> > > qtest_init(qtest_chrdev, qtest_log, &error_fatal);
> > > }
> >
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_
> _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
--
Eduardo
