linux-kernel - Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150401130113.GL7031@thinpad.lan.raisama.net>
Date:	Wed, 1 Apr 2015 10:01:13 -0300
From:	Eduardo Habkost <ehabkost@...hat.com>
To:	Michael Mueller <mimu@...ux.vnet.ibm.com>
Cc:	qemu-devel@...gnu.org, kvm@...r.kernel.org,
	linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
	Gleb Natapov <gleb@...nel.org>, Alexander Graf <agraf@...e.de>,
	Christian Borntraeger <borntraeger@...ibm.com>,
	"Jason J. Herne" <jjherne@...ux.vnet.ibm.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Andreas Faerber <afaerber@...e.de>,
	Richard Henderson <rth@...ddle.net>,
	Daniel Hansel <daniel.hansel@...ux.vnet.ibm.com>
Subject: Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model

On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote:
> On Tue, 31 Mar 2015 15:35:26 -0300
> Eduardo Habkost <ehabkost@...hat.com> wrote:
> 
> > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > > This patch implements a new QMP request named 'query-cpu-model'.
> > > It returns the cpu model of cpu 0 and its backing accelerator.
> > > 
> > > request:
> > >   {"execute" : "query-cpu-model" }
> > > 
> > > answer:
> > >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > > 
> > > Alias names are resolved to their respective machine type and GA names
> > > already during cpu instantiation. Thus, also a cpu model like 'host'
> > > which is implemented as alias will return its normalized cpu model name.
> > > 
> > > Furthermore the patch implements the following function:
> > > 
> > > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > > 
> > > Signed-off-by: Michael Mueller <mimu@...ux.vnet.ibm.com>
> > > ---
> > [...]
> > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > > +{
> > > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > > +}
> > 
> > How exactly is this information going to be used by clients? If getting
> > the correct type and ga values is important for them, maybe you could
> > add them as integer fields, instead of requiring clients to parse the
> > CPU model name?
> 
> The consumer don't need to parse the name, it is just important for them to have
> distinctive names that correlate with the names returned by query-cpu-definitions.
> Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential
> migration target can be verified, i.e. its query-cpu-definitions answer for "kvm"
> has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism
> also the largest common denominator can be calculated. That model will be used then.

Understood. So the point is to really have a name that can be found at
query-cpu-definitions. Makes sense.

(BTW, if you reused strdup_s390_cpu_name() inside
s390_cpu_compare_class_name() too, you would automatically ensure that
query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will
always agree with each other).

> 
> I also changed the above mentioned routine to map the cpu model none case:
> 
> static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> {
>     if (cpuid(cc->proc)) {
>         return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
>     } else {
>         return g_strdup("none");
>     }
> }

What about:

  static const char *s390_cpu_name(S390CPUClass *cc)
  {
      return cc->model_name;
  }

And then you can just set cc->model_name=_name inside S390_PROC_DEF (and
set it to "none" inside s390_cpu_class_init()).

I wonder if this class->model_name conversion could be made generic
inside the CPU class. We already have a CPU::class_by_name() method, so
it makes sense to have the opposite function too.

(But I wouldn't mind making this s390-specific first, and converted
later to generic code if appropriate).

> 
> This implicitly will fail a comparison for cpu model ("none", "kvm") as that will
> never be part of the query-cpu-definitions answer.

I am not sure I follow. If ("none", "kvm") is never in the list, is
"-cpu none -machine accel=kvm" always an invalid use case?

(I don't understand completely the meaning of "-cpu none" yet. How does
the CPU look like for the guest in this case? Is it possible to
live-migrate when using -cpu none?)

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/