Re: [Qemu-devel] [RFC PATCH v3 06/19] Implement "-dimm" command line option

[Blue Swirl]

On Mon, Sep 24, 2012 at 10:42 AM, Vasilis Liaskovitis
<address@hidden> wrote:
> On Sat, Sep 22, 2012 at 01:46:57PM +0000, Blue Swirl wrote:
>> On Fri, Sep 21, 2012 at 11:17 AM, Vasilis Liaskovitis
>> <address@hidden> wrote:
>> > Example:
>> > "-dimm id=dimm0,size=512M,node=0,populated=off"
>>
>> There should not be a need to introduce a new top level option,
>> instead you should just use -device, like
>> -device dimm,base=0,id=dimm0,size=512M,node=0,populated=off
>>
>> That would also specify the start address.
>
> What is "base"? the start address? I think the start address should be 
> calculated by the
> chipset / board, not by the user.

Yes.

>
> The "-dimm" option is supposed to specify the dimm/memory layout, and not 
> create
> any devices.
>
> If we don't want this new option, I have a question:
>
> A "-device/device_add" means we create a new qdev device at startup or as a
> hotplug operation respectively. So, the semantics of
> "-device dimm,id=dimm0,size=512M,node=0,populated=on" are clear to me.
>
> What does "-device dimm,populated=off" mean from a qdev perspective? There 
> are 2
> alternatives:
>
> - The device is created on the dimmbus, but is not used/populated yet. Than 
> the
> activation/acpi-hotplug of the dimm may require a separate command (we used 
> to have
> "dimm_add" in versions < 3). "device_add" handling always hotplugs a new qdev
> device, so this wouldn't fit this usecase, because the device already exists. 
> In
> this case, the actual "acpi hotplug" operation is decoupled from qdev device
> creation.

The bus exists but the devices do not, device_add would add DIMMs to
the bus. This matches PCI bus created by the host bridge and PCI
device hotplug.

A more complex setup would be dimm bus, dimm slot devices and DIMM
devices. The intermediate slot device would contain one DIMM device if
plugged.

>
> - The dimmdevice is not created when "-device dimm,populated=off" (this would
> require some ugly checking in normal -device argument handling). Only the dimm
> layout is saved. The hotplug is triggered from a normal device_add later. So 
> in
> this case, the "acpi hotplug" happens at the same time as the qdev hotplug.
>
> Do you see a simpler alternative without introducing a new option?
>
> Using the "-dimm" option follows the second semantic and avoids changing the 
> "-device"
> semantics. Dimm layout description is decoupled from dimmdevice creation, and 
> qdev
> hotplug coincides with acpi hotplug.

Maybe even the dimmbus device shouldn't exist by itself after all, or
it should be pretty much invisible to users. On real HW, the memory
controller or south bridge handles the memory. For i440fx, it's part
of the same chipset. So I think we should just add qdev properties to
i440fx to specify the sizes, nodes etc. Then i440fx should create the
dimmbus device unconditionally using the properties. The default
properties should create a sane configuration, otherwise -global
i440fx.dimm_size=512M etc. could be used. Then the bus would be
populated as before or with device_add.

>
> thanks,
>
> - Vasilis