All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	Eduardo Habkost <ehabkost@redhat.com>,
	patches@linaro.org
Subject: Re: [Qemu-arm] [PATCH 0/4] cpu: Implement cpu_generic_new()
Date: Mon, 26 Jun 2017 14:28:13 +0100	[thread overview]
Message-ID: <87a84uyj2q.fsf@linaro.org> (raw)
In-Reply-To: <1486996099-15820-1-git-send-email-peter.maydell@linaro.org>


Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset adds a new function cpu_generic_new()
> which is similar to cpu_generic_init() except that it
> does not realize the created CPU object. This means that
> board code can do a "new cpu; set QOM properties; realize"
> sequence without having to do all the work of splitting
> the CPU model string and calling parse_features by hand.


Just going through my review queue and I see this needs re-basing. Is
there going to be another rev or was a different approach suggested?

>
> Patch 2 clarifies a TODO comment, hopefully correctly,
> based on an email conversation I had with Eduardo a
> little while back.
>
> Patches 3 and 4 change the ARM boards which currently
> call parse_features by hand to use the new function.
>
>
> If there's consensus that this is the right general
> direction to go in, then I think that some other
> architectures could also make cleanups to use this:
>  * cpu_s390x_create() is almost exactly this function,
>    give or take some fine detail of error handling
>  * ppc_cpu_parse_features is almost the same thing,
>    except that it doesn't actually create the CPU object,
>    it only calls parse_features
>  * hw/i386/pc.c does a manual parse_features
>
> I'm not strongly attached to this particular approach
> (though it seems like a reasonable one, especially given
> the proliferation of different arch-specific helpers
> listed above and the bugs in boards which don't call
> parse_features when they should), but I would like us to
> figure out and document what the right way for a board
> to create and configure its CPU objects is...
>
>
> Michael Davidsaver (1):
>   cpu: add cpu_generic_new()
>
> Peter Maydell (3):
>   cpu: Clarify TODO comment in cpu_generic_new()
>   hw/arm/integrator: Use new cpu_generic_new()
>   hw/arm/virt: Use new cpu_generic_new()
>
>  include/qom/cpu.h     | 17 +++++++++++++++++
>  hw/arm/integratorcp.c | 22 ++--------------------
>  hw/arm/virt.c         | 24 ++----------------------
>  qom/cpu.c             | 37 ++++++++++++++++++++++++++-----------
>  4 files changed, 47 insertions(+), 53 deletions(-)


--
Alex Bennée

WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	Eduardo Habkost <ehabkost@redhat.com>,
	patches@linaro.org
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH 0/4] cpu: Implement cpu_generic_new()
Date: Mon, 26 Jun 2017 14:28:13 +0100	[thread overview]
Message-ID: <87a84uyj2q.fsf@linaro.org> (raw)
In-Reply-To: <1486996099-15820-1-git-send-email-peter.maydell@linaro.org>


Peter Maydell <peter.maydell@linaro.org> writes:

> This patchset adds a new function cpu_generic_new()
> which is similar to cpu_generic_init() except that it
> does not realize the created CPU object. This means that
> board code can do a "new cpu; set QOM properties; realize"
> sequence without having to do all the work of splitting
> the CPU model string and calling parse_features by hand.


Just going through my review queue and I see this needs re-basing. Is
there going to be another rev or was a different approach suggested?

>
> Patch 2 clarifies a TODO comment, hopefully correctly,
> based on an email conversation I had with Eduardo a
> little while back.
>
> Patches 3 and 4 change the ARM boards which currently
> call parse_features by hand to use the new function.
>
>
> If there's consensus that this is the right general
> direction to go in, then I think that some other
> architectures could also make cleanups to use this:
>  * cpu_s390x_create() is almost exactly this function,
>    give or take some fine detail of error handling
>  * ppc_cpu_parse_features is almost the same thing,
>    except that it doesn't actually create the CPU object,
>    it only calls parse_features
>  * hw/i386/pc.c does a manual parse_features
>
> I'm not strongly attached to this particular approach
> (though it seems like a reasonable one, especially given
> the proliferation of different arch-specific helpers
> listed above and the bugs in boards which don't call
> parse_features when they should), but I would like us to
> figure out and document what the right way for a board
> to create and configure its CPU objects is...
>
>
> Michael Davidsaver (1):
>   cpu: add cpu_generic_new()
>
> Peter Maydell (3):
>   cpu: Clarify TODO comment in cpu_generic_new()
>   hw/arm/integrator: Use new cpu_generic_new()
>   hw/arm/virt: Use new cpu_generic_new()
>
>  include/qom/cpu.h     | 17 +++++++++++++++++
>  hw/arm/integratorcp.c | 22 ++--------------------
>  hw/arm/virt.c         | 24 ++----------------------
>  qom/cpu.c             | 37 ++++++++++++++++++++++++++-----------
>  4 files changed, 47 insertions(+), 53 deletions(-)


--
Alex Bennée

  parent reply	other threads:[~2017-06-26 13:27 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-13 14:28 [Qemu-arm] [PATCH 0/4] cpu: Implement cpu_generic_new() Peter Maydell
2017-02-13 14:28 ` [Qemu-devel] " Peter Maydell
2017-02-13 14:28 ` [Qemu-devel] [PATCH 1/4] cpu: add cpu_generic_new() Peter Maydell
2017-02-13 14:28   ` Peter Maydell
2017-02-21 18:35   ` [Qemu-arm] " Eduardo Habkost
2017-02-21 18:35     ` [Qemu-devel] " Eduardo Habkost
2017-02-13 14:28 ` [Qemu-arm] [PATCH 2/4] cpu: Clarify TODO comment in cpu_generic_new() Peter Maydell
2017-02-13 14:28   ` [Qemu-devel] " Peter Maydell
2017-02-13 14:28 ` [Qemu-arm] [PATCH 3/4] hw/arm/integrator: Use new cpu_generic_new() Peter Maydell
2017-02-13 14:28   ` [Qemu-devel] " Peter Maydell
2017-02-13 14:28 ` [Qemu-arm] [PATCH 4/4] hw/arm/virt: " Peter Maydell
2017-02-13 14:28   ` [Qemu-devel] " Peter Maydell
2017-02-20 19:10   ` [Qemu-arm] " Igor Mammedov
2017-02-20 19:10     ` Igor Mammedov
2017-02-21 16:09     ` [Qemu-arm] " Eduardo Habkost
2017-02-21 16:09       ` Eduardo Habkost
2017-02-16 16:40 ` [Qemu-arm] [PATCH 0/4] cpu: Implement cpu_generic_new() Peter Maydell
2017-02-16 16:40   ` [Qemu-devel] " Peter Maydell
2017-02-21 15:58   ` Eduardo Habkost
2017-02-21 15:58     ` [Qemu-devel] " Eduardo Habkost
2017-02-21 17:48     ` Peter Maydell
2017-02-21 17:48       ` [Qemu-devel] " Peter Maydell
2017-02-17 19:05 ` [Qemu-arm] [Qemu-devel] " Igor Mammedov
2017-02-17 19:05   ` Igor Mammedov
2017-02-21 16:01   ` [Qemu-arm] " Eduardo Habkost
2017-02-21 16:01     ` Eduardo Habkost
2017-06-26 13:28 ` Alex Bennée [this message]
2017-06-26 13:28   ` [Qemu-devel] [Qemu-arm] " Alex Bennée
2017-06-27  2:33   ` Eduardo Habkost
2017-06-27  2:33     ` [Qemu-devel] " Eduardo Habkost
2017-06-27  8:32     ` Igor Mammedov

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=87a84uyj2q.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=ehabkost@redhat.com \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.