qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Igor Mammedov <imammedo@redhat.com>
Cc: peter.maydell@linaro.org, stefanha@redhat.com,
	stefanb@linux.vnet.ibm.com, qemu-devel@nongnu.org,
	hutao@cn.fujitsu.com, mst@redhat.com, mjt@tls.msk.ru,
	mdroth@linux.vnet.ibm.com, armbru@redhat.com,
	vasilis.liaskovitis@profitbricks.com, quintela@redhat.com,
	kraxel@redhat.com, aliguori@amazon.com, pbonzini@redhat.com,
	marcel.a@redhat.com, lcapitulino@redhat.com, chegu_vinod@hp.com,
	Li Guang <lig.fnst@cn.fujitsu.com>
Subject: Re: [Qemu-devel] [PATCH 21/27] pc: add memory hotplug 440fx machine
Date: Mon, 25 Nov 2013 18:00:56 +0100	[thread overview]
Message-ID: <52938248.8090105@suse.de> (raw)
In-Reply-To: <20131125114150.08e05144@nial.usersys.redhat.com>

Am 25.11.2013 11:41, schrieb Igor Mammedov:
> On Thu, 21 Nov 2013 17:09:27 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
>> Am 21.11.2013 15:34, schrieb Igor Mammedov:
>>> On Thu, 21 Nov 2013 15:13:12 +0100
>>> Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 21.11.2013 06:48, schrieb Li Guang:
>>>>> Why not give the memory that not be hot-added a chance to be placed on
>>>>> one memory slot?
>>>>
>>>> Seconded, I believe I requested that on the previous version already.
>>> Because current initial memory allocation is a mess and this already
>>> large series would become even more large and intrusive, so far series
>>> it relatively not intrusive and self contained.
>>>
>>> I believe re-factoring of initial memory to use Dimm devices should be
>>> done later on top of infrastructure this series provides.
>>
>> My problem with that is that that would be a semantically incompatible
>> modeling change. With your series we might have /machine/dimm.0/child[0]
>> be the first hot-plugged memory and once such a refactoring is done,
>> child[0] silently becomes -m and child[1] is the hot-added one.
> 
> I think there won't be silent change in child[0], since most likely
> initial RAM would require additional DimmBus (maybe even 2)
> for it's devices.
> 
> But anyway, why would this be an issue?
> 
>> So if we know that we want/need to change the infrastructure, why not
>> add a single patch (?) to allocate one additional object on the bus now?
>> Unless we actually write the code, we won't know whether there are some
>> incorrect hot-plug assumptions in the dimm code.
> It wouldn't be a single simple patch for PC, I'm afraid.
> I don't see point in adding dummy DIMM device for initial memory and then
> do re-aliasing of its memory region in GPA as it's done in current code.
> 
> As I see it (taking in account Marcolo's/Paolo's alignment path), current
> single MR for initial RAM should be split in 1-4 separate MRs depending on
> initial RAM amount and alignment requirements between HPA/GPA addresses.
> 
> That would probably introduce additional, non hotlugable DimmBus-es (1-2)
> for low and high memory, which would be incompatible with old machine types
> devices and GPA layout, so why care about what
> /machine/dimm.0/child[0] would be in new machine version?

I feel we're talking about two very different things here.

What I am talking about is the user experience. A mainboard has 4 or
maybe 8 DIMM slots where the user can plug in greenish memory bars.
That's what I would like to see implemented in QEMU because that's
understandable without reading code and ACPI specs.

What you seem to be talking about by contrast is your DimmBus
implementation and its limitations/assumptions. You can easily use
dev->hotplugged to distinguish between initial and hot-plugged devices
as done elsewhere, including PCI and ICC bus, no?

In short, what I am fighting against is having a machine with 4 slots:

slot[0] = 42
slot[1] = 0
slot[2] = 0
slot[3] = 0

meaning 42 + implicit -m now, later getting fixed to explicit:

slot[0] = -m
slot[1] = 42
slot[2] = 0
slot[3] = 0

Whether -m maps to one or more slots can easily be scaled in the
example, I had previously asked whether there were upper limits per slot
but believe that got denied from an ACPI perspective; my point is the
slot offset and the inconsistent sum exposed via QOM/QMP.

On your ICC bus we had the initial -smp CPUs alongside hot-plugged CPUs
right from the start.

I can't think of a reason why there would be multiple DimmBuses in a
single machine from a user's point of view.
Different implementations for different memory controllers / machines
make perfect sense of course. But even from a developer's point of view
multiple buses don't make that much sense either if we keep
http://wiki.qemu.org/Features/QOM#TODO in mind - devices exposing
multiple buses need to be split up and in the end we just have named
link<> properties on some container object as sketched in the example
above - question then becomes should we have multiple containers, and I
think the answer for a PC will be no.

Embedded systems with a mix of small on-chip SRAM and on-board SDRAM may
be a different beast to model, but well beyond the scope of this series
anyway, which IIUC doesn't expose any DimmBus outside of the two PCs.

Also, once memory has been hot-plugged and the machine gets rebooted,
shouldn't that be the same to BIOS/ACPI as if the memory was cold-plugged?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

  reply	other threads:[~2013-11-25 17:01 UTC|newest]

Thread overview: 143+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-21  2:38 [Qemu-devel] [PATCH 00/27 RFC v7] ACPI memory hotplug Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 01/27] acpi: factor out common pm_update_sci() into acpi core Igor Mammedov
2013-12-05 12:37   ` Michael S. Tsirkin
2013-12-05 15:11     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 02/27] rename pci_hotplug_fn to hotplug_fn and make it available for other devices Igor Mammedov
2013-11-25 12:49   ` Paolo Bonzini
2013-11-25 13:11     ` Paolo Bonzini
2013-11-25 15:57     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 03/27] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS Igor Mammedov
2013-12-19 14:35   ` Michael S. Tsirkin
2013-12-20 12:48     ` Igor Mammedov
2013-12-22 11:20       ` Michael S. Tsirkin
2013-11-21  2:38 ` [Qemu-devel] [PATCH 04/27] vl: convert -m to qemu_opts_parse() Igor Mammedov
2013-11-21  6:01   ` Li Guang
2013-11-21 13:45     ` Igor Mammedov
2013-11-21 10:12   ` Markus Armbruster
2013-11-26 13:17     ` Igor Mammedov
2013-11-26 14:49       ` Markus Armbruster
2013-11-26 16:55         ` Igor Mammedov
2013-11-27 14:35           ` Markus Armbruster
2013-11-27 15:28             ` Igor Mammedov
2013-11-27 17:31               ` Markus Armbruster
2013-11-27  0:27         ` [Qemu-devel] [PATCH 04/28] vl: convert -m to QemuOpts Igor Mammedov
2013-11-27  0:27           ` [Qemu-devel] [PATCH 05/28] vl.c: extend -m option to support options for memory hotplug Igor Mammedov
2013-12-10  7:23           ` [Qemu-devel] [PATCH 04/28] vl: convert -m to QemuOpts Paolo Bonzini
2013-12-10 10:53             ` Igor Mammedov
2013-11-25 12:51   ` [Qemu-devel] [PATCH 04/27] vl: convert -m to qemu_opts_parse() Paolo Bonzini
2013-11-27  0:32     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor Igor Mammedov
2013-11-21 10:15   ` Markus Armbruster
2013-11-25 15:36     ` Igor Mammedov
2013-11-25 16:04       ` Michael S. Tsirkin
2013-11-25 16:32         ` Paolo Bonzini
2013-11-25 16:43           ` Eric Blake
2013-11-25 17:01             ` Paolo Bonzini
2013-11-27 14:15             ` Markus Armbruster
2013-11-27 15:17               ` Paolo Bonzini
2013-11-27 17:02                 ` Markus Armbruster
2013-11-27 17:10                   ` Paolo Bonzini
2013-12-19 14:40             ` Michael S. Tsirkin
2013-12-19 15:14               ` Markus Armbruster
2013-12-19 15:32                 ` Michael S. Tsirkin
2013-12-19 15:31               ` Paolo Bonzini
2013-12-19 15:40                 ` Michael S. Tsirkin
2013-12-19 15:46                   ` Paolo Bonzini
2013-11-21  2:38 ` [Qemu-devel] [PATCH 06/27] get reference to /backend container via qemu_get_backend() Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 07/27] add memdev backend infrastructure Igor Mammedov
2013-11-25 12:54   ` Paolo Bonzini
2013-11-25 16:01     ` Igor Mammedov
2013-11-25 16:09       ` Paolo Bonzini
2013-11-27 14:37         ` Igor Mammedov
2013-11-27 15:21           ` Paolo Bonzini
2013-11-27 15:57             ` Igor Mammedov
2013-11-27 15:25   ` Eric Blake
2013-11-27 16:09     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 08/27] dimm: implement dimm device abstraction Igor Mammedov
2013-11-25 12:57   ` Paolo Bonzini
2013-11-25 15:10     ` Igor Mammedov
2013-11-25 15:16       ` Paolo Bonzini
2013-11-21  2:38 ` [Qemu-devel] [PATCH 09/27] dimm: map DimmDevice into DimBus provided address space Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 10/27] dimm: add busy slot check and slot auto-allocation Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 11/27] dimm: add busy address check and address auto-allocation Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 12/27] dimm: add hotplug callback to DimmBus Igor Mammedov
2013-11-25 13:01   ` Paolo Bonzini
2013-11-25 14:40     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 13/27] acpi: memory hotplug ACPI hardware implementation Igor Mammedov
2013-11-21  9:42   ` Michael S. Tsirkin
2013-11-21 14:21     ` Igor Mammedov
2013-11-21 14:38       ` Michael S. Tsirkin
2013-11-22 17:14         ` Igor Mammedov
2013-11-24 10:58           ` Michael S. Tsirkin
2013-11-25  7:27             ` Markus Armbruster
2013-11-25 13:45               ` Paolo Bonzini
2013-11-25 14:18                 ` Igor Mammedov
2013-11-25 14:31                   ` Paolo Bonzini
2013-11-25 14:57                     ` Igor Mammedov
2013-11-25 13:56             ` Igor Mammedov
2013-11-27 17:59   ` Eric Blake
2013-11-21  2:38 ` [Qemu-devel] [PATCH 14/27] acpi: initialize memory hotplug ACPI PIIX4 hardware Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 15/27] acpi: piix4: add memory-hotplug-io-base property to piix4_pm Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 16/27] acpi: ich9: allow guest to clear SCI rised by GPE Igor Mammedov
2013-11-21  7:14   ` Michael S. Tsirkin
2013-11-21  8:12     ` Hu Tao
2013-11-21  8:18       ` Li Guang
2013-11-21  8:29         ` Michael S. Tsirkin
2013-11-21  8:32           ` Li Guang
2013-11-21  9:43             ` Michael S. Tsirkin
2013-11-22  0:57               ` Li Guang
2013-11-25 11:13                 ` Igor Mammedov
2013-11-26  0:29                   ` Li Guang
2013-11-26 22:36                     ` Igor Mammedov
2013-11-27  0:15                       ` Li Guang
2013-11-27  0:57                         ` Igor Mammedov
2013-11-27  1:48                           ` Li Guang
2013-11-27  9:43                             ` Paolo Bonzini
2013-11-21 13:19             ` Igor Mammedov
2013-11-22  1:03               ` Li Guang
2013-11-21  8:26       ` Michael S. Tsirkin
2013-11-21  8:28         ` Hu Tao
2013-11-21 13:31           ` Igor Mammedov
2013-11-21 13:21         ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 17/27] acpi: initialize memory hotplug ACPI ICH9 hardware Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 18/27] acpi: ich9: add memory-hotplug-io-base property to ich9_pm Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 19/27] acpi: piix4/ich9: add optional vmstate field for MemHotplugState migration Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 20/27] pc: piix: make PCII440FXState type public Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 21/27] pc: add memory hotplug 440fx machine Igor Mammedov
2013-11-21  5:48   ` Li Guang
2013-11-21 14:13     ` Andreas Färber
2013-11-21 14:34       ` Igor Mammedov
2013-11-21 14:39         ` Michael S. Tsirkin
2013-11-21 16:09         ` Andreas Färber
2013-11-21 16:17           ` Michael S. Tsirkin
2013-11-25 10:41           ` Igor Mammedov
2013-11-25 17:00             ` Andreas Färber [this message]
2013-11-26 20:26               ` Igor Mammedov
2013-11-22 14:23   ` Gerd Hoffmann
2013-11-25 11:00     ` Igor Mammedov
2013-11-25 11:39       ` Gerd Hoffmann
2013-11-25 13:34         ` Igor Mammedov
2013-11-25 13:35         ` Paolo Bonzini
2013-11-25 14:24           ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 22/27] pc: add memory hotplug Q35 machine Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface Igor Mammedov
2013-11-21  9:37   ` Michael S. Tsirkin
2013-11-25 14:39     ` Igor Mammedov
2013-12-16 19:50       ` Michael S. Tsirkin
2013-12-16 21:42         ` Igor Mammedov
2013-11-25 13:42   ` Paolo Bonzini
2013-11-25 14:26     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 24/27] pc: ACPI BIOS: add ssdt-mem.hex.generated and update ssdt-misc.hex.generated Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 25/27] pc: ACPI BIOS: use enum for defining memory affinity flags Igor Mammedov
2013-11-21  7:20   ` Michael S. Tsirkin
2013-11-25 10:11     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 26/27] pc: ACPI BIOS: reserve SRAT entry for hotplug mem hole Igor Mammedov
2013-11-21  7:18   ` Michael S. Tsirkin
2013-11-25 10:11     ` Igor Mammedov
2013-11-21  2:38 ` [Qemu-devel] [PATCH 27/27] pc: ACPI BIOS: make GPE.3 handle memory hotplug event on PIIX and Q35 machines Igor Mammedov
2013-11-21  6:20 ` [Qemu-devel] [PATCH 00/27 RFC v7] ACPI memory hotplug Michael S. Tsirkin
2013-11-21 13:39   ` Igor Mammedov
2013-11-21 13:43     ` Michael S. Tsirkin
2013-11-21 14:37       ` Igor Mammedov
2013-11-21 14:45         ` Michael S. Tsirkin
2013-11-25 10:09           ` 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=52938248.8090105@suse.de \
    --to=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=armbru@redhat.com \
    --cc=chegu_vinod@hp.com \
    --cc=hutao@cn.fujitsu.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=lig.fnst@cn.fujitsu.com \
    --cc=marcel.a@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mjt@tls.msk.ru \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanb@linux.vnet.ibm.com \
    --cc=stefanha@redhat.com \
    --cc=vasilis.liaskovitis@profitbricks.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).