All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Bolognani <abologna@redhat.com>
To: Wei Huang <wei@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jones <drjones@redhat.com>, qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Shannon Zhao <shannon.zhao@linaro.org>
Subject: Re: [Qemu-devel] [PATCH RFC 1/1] arm64: add an option to turn on/off vpmu support
Date: Mon, 15 Aug 2016 11:24:47 +0200	[thread overview]
Message-ID: <1471253087.3003.3.camel@redhat.com> (raw)
In-Reply-To: <da84a3fb-79d9-bb15-0784-9f635b6363cf@redhat.com>

On Sat, 2016-08-13 at 01:06 -0500, Wei Huang wrote:
> > > Wouldn't that mean that you'd be unable to use
> > > 
> > >   -cpu foo,pmu=off
> > > 
> > > if CPU model 'foo' doesn't support a PMU? I'd expect that
> > > to work.
> > 
> > The current precedent (has_el3) doesn't work like that: if
> > foo isn't a CPU which can support EL3 then the property doesn't
> > exist, and it's an error to try to set it.
> 
> V1 sent. I tried to follow everyone's advice. See the following:
> 
> * set default pmu=off
> * like el3, add a new feature ARM_FEATURE_HOST_PMU
> * "pmu" property becomes CPU dependent. Only cortex-a53/cortex-a57/host
> under certain mode support this option
> * change struct ARMCPU field name "has_pmu" ==> "has_host_pmu" because
> IMO "has_pmu" is misleading
> 
> BTW answering Andrea's question above: "-cpu foo,pmu=off" won't be
> allowed in this patch if CPU "foo" doesn't support host-backed PMU. QEMU
> will fail to run in this case. Maybe this is what we want?

After discussing this a bit offline, I came to the conclusion
that there isn't a Single Right Way™ to handle this - both my
proposal and what you implemented are reasonable behaviors
one could expect.

On the other hand, what you implemented:

  * matches x86
  * is more strict than what I proposed, so there's room to
    change it later without breaking any existing guest

so I'm happy with it :)

-- 
Andrea Bolognani / Red Hat / Virtualization

  reply	other threads:[~2016-08-15  9:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-28 16:38 [Qemu-devel] [PATCH RFC 1/1] arm64: add an option to turn on/off vpmu support Wei Huang
2016-07-29  0:59 ` Shannon Zhao
2016-07-29  6:54 ` Andrew Jones
2016-07-29 15:07   ` Wei Huang
2016-07-29 15:29   ` Peter Maydell
2016-08-01 12:04   ` Andrea Bolognani
2016-08-01 13:08     ` Andrew Jones
2016-08-01 13:16       ` Peter Maydell
2016-08-01 13:26       ` Andrea Bolognani
2016-08-01 13:32         ` Peter Maydell
2016-08-01 14:55           ` Andrea Bolognani
2016-08-13  6:06           ` Wei Huang
2016-08-15  9:24             ` Andrea Bolognani [this message]
2016-07-29  7:57 ` Peter Maydell
2016-07-29 15:08   ` Wei Huang
2016-07-29 15:25     ` Peter Maydell

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=1471253087.3003.3.camel@redhat.com \
    --to=abologna@redhat.com \
    --cc=drjones@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhao@linaro.org \
    --cc=wei@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.