qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: "Emilio G. Cota" <cota@braap.org>,
	qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>,
	Thomas Huth <thuth@redhat.com>,
	qemu-arm@nongnu.org, Igor Mitsyanko <i.mitsyanko@gmail.com>,
	Alistair Francis <alistair.francis@xilinx.com>,
	"Edgar E . Iglesias" <edgar.iglesias@gmail.com>,
	Marcel Apfelbaum <marcel@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class
Date: Wed, 8 Nov 2017 19:52:12 -0200	[thread overview]
Message-ID: <20171108215212.GW3111@localhost.localdomain> (raw)
In-Reply-To: <73e9c472-82da-b293-27cd-ca9e0ff93179@linaro.org>

On Wed, Nov 08, 2017 at 10:29:43PM +0100, Richard Henderson wrote:
> On 11/06/2017 09:13 PM, Emilio G. Cota wrote:
> > Subject: [PATCH] qom: move CPUClass.tcg_initialize to a global
> > 
> > 55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
> > introduces a per-CPUClass bool that we check so that the target CPU
> > is initialized for TCG only once. This works well except when
> > we end up creating more than one CPUClass, in which case we end
> > up incorrectly initializing TCG more than once, i.e. once for
> > each CPUClass.
> > 
> > This can be replicated with:
> >   $ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
> >       -global driver=xlnx,,zynqmp,property=has_rpu,value=on
> > In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
> > whereas the "regular" CPUs are prefixed by "cortex-a53-". This
> > results in two CPUClass instances being created.
> > 
> > Fix it by introducing a static variable, so that only the first
> > target CPU being initialized will initialize the target-dependent
> > part of TCG, regardless of CPUClass instances.
> 
> Hah!
> 
> So, I had been thinking of the xylinx ARM + Microblaze case, where we really do
> need two different initializations.  I never imagined that two different ARM
> parts had different CPUClasses.

Is xylinx ARM + Microblaze something that already works (and
would be broken by this patch), or something planned for the
future?

> 
> So I guess it's my initial patch that unified this that's more buggy than not.

We still have the option of reverting the original patch, but (if
it doesn't break anything) this patch looks like a simpler fix
for 2.11 than a full revert.

-- 
Eduardo

  reply	other threads:[~2017-11-08 21:52 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-03 18:47 [Qemu-devel] [PATCH] hw: add .min_cpus and .default_cpus fields to machine_class Emilio G. Cota
2017-11-03 18:56 ` Emilio G. Cota
2017-11-03 20:02   ` Eduardo Habkost
2017-11-03 22:24     ` Emilio G. Cota
2017-11-06 14:10       ` Eduardo Habkost
2017-11-06 20:13         ` Emilio G. Cota
2017-11-07  0:43           ` Alistair Francis
2017-11-07 12:31             ` Eduardo Habkost
2017-11-08 21:29           ` Richard Henderson
2017-11-08 21:52             ` Eduardo Habkost [this message]
2017-11-08 22:08               ` Alistair Francis
2017-11-06 21:54         ` Emilio G. Cota
2017-11-06 22:32           ` Alistair Francis
2017-11-06 23:21             ` Emilio G. Cota
2017-11-06 23:33               ` Alistair Francis
2017-11-07  0:54                 ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2017-11-07  1:19                   ` Alistair Francis
2017-11-07 16:15           ` Philippe Mathieu-Daudé
2017-11-07 19:32           ` [Qemu-devel] " Eduardo Habkost
2017-11-07 19:48             ` Alistair Francis
2017-11-07 19:54               ` Eduardo Habkost
2017-11-07 20:15 ` Eduardo Habkost
2017-11-10 19:23   ` Emilio G. Cota

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171108215212.GW3111@localhost.localdomain \
    --to=ehabkost@redhat.com \
    --cc=alistair.francis@xilinx.com \
    --cc=cota@braap.org \
    --cc=edgar.iglesias@gmail.com \
    --cc=i.mitsyanko@gmail.com \
    --cc=imammedo@redhat.com \
    --cc=marcel@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).