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

[Blue Swirl]

On Tue, Oct 9, 2012 at 5:04 PM, Vasilis Liaskovitis
<address@hidden> wrote:
> Hi,
>
> sorry for the delayed answer.
>
> On Sat, Sep 29, 2012 at 11:13:04AM +0000, Blue Swirl wrote:
>> >
>> > 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.
>
> interesting, I haven't thought about this alternative. It does sounds overly
> complex, but a dimmslot / dimmdevice splitup could consolidate hotplug 
> semantic
> differences between populated=on/off. Something similar to the dimmslot device
> is already present in v3 (dimmcfg structure), but it's not a qdev visible 
> device.
> I 'd rather avoid the complication, but i might revisit this idea.

The memory controller could be able to also enable and disable slots
independently to their population state.

>
>>
>> >
>> > - 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.
>
> hmm the problem with using only i440fx properties, is that size/nodes look
> dimm specific to me, not chipset-memcontroller specific. Unless we only allow
> uniform size dimms. Is it possible to have a dynamic list of sizes/nodes 
> pairs as
> properties of a qdev device?

I don't think so, but probably there's a limit of DIMMs that real
controllers have, something like 8 max.

>
> Also if there is no dimmbus, and instead we have only links<> from i440fx to 
> dimm-devices,
> would the current qdev hotplug API be enough?

I'd just disable hotplug if there is no dimmbus (ISA PC?).

>
> I am currently leaning towards this: i440fx unconditionally creates the 
> dimmbus. Users
> don't have to specify the bus (i assume this is what you mean by "dimmbus 
> should
> be invisible to the users")
>
> We only use "-device dimm" to describe dimms. With "-device 
> dimm,populated=off", only
> the dimm config layout will be saved in the dimmbus. The hotplug is triggered 
> from a normal
> device_add later (same as pci hotplug).

OK.

>
> thanks,
>
> - Vasilis