Re: [Qemu-devel] [PATCH v4 0/6] save/restore on Xen

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/24/2012 09:56 AM, Avi Kivity wrote:
> On 01/24/2012 03:14 PM, Anthony Liguori wrote:
>> On 01/24/2012 05:10 AM, Avi Kivity wrote:
>>> On 01/23/2012 07:18 PM, Anthony Liguori wrote:
>>>> Generally speaking, RAM is an independent device in most useful cases.
>>>
>>> Can you give examples?  Do you mean a subdevice with composition, or a
>>> really independent device?
>>
>> I expect we'll have one Ram device.  It's size will be configurable.
>> One Ram device will hang off of the machine which would be the main ram.
>
> We'll also have a hotpluggable variant which talks to some GPIOs.  A
> motherboard may support multiple slots with such devices.

Yup.  And boards can subclass the Ram device in order to be able to restrict the 
valid RAM sizes.

-m just becomes an analysis to set the ram size for a ram device.  The 
infrastructure around RAMBlock, etc. would go away as each RAMBlock would 
correspond to a Ram device.

>> A video card would have a Ram device via composition.
>
> IMO, overkill.

I think it's a lot of refactoring to get there and it's not the most critical 
infrastructure changes we have coming up, but I think it would be nice to get 
there eventually.

>
>>
>> The important consideration about reset is how it propagates.  My
>> expectation is that we'll propagate reset through the composition tree
>> in a preorder transversal.  That means in the VGA device reset
>> function, it's child device (Ram) has not been reset yet.
>
> Doesn't depth first make more sense?

Yes, it would be depth first, but you're looking for a preorder and post order 
hook.  In otherwords:

void do_reset(DeviceState *dev)
{
     dev->pre_reset(dev);  // this is preorder
     do_reset_all(dev->children);
     dev->post_reset(dev);  // this is postorder
}

The PCI bus overloads the reset handler and does a post-order reset.  I guess I 
can rationalize it after all but it's not entirely clear to me that a pre_reset 
hook isn't necessary too.

>  A bus is considered reset after
> all devices in the bus have been reset.
>
>
>>
>>>> We really should view RAM as just another device so I don't like the
>>>> idea of propagating a global concept of "when RAM is restored" because
>>>> that treats it specially compared to other devices.
>>>
>>> Agree.  In fact the first step has been taken as now creating a RAM
>>> region with memory_region_init_ram() doesn't register it for migration.
>>> The next step would be a VMSTATE_RAM() to make it part of the containing
>>> device.
>>
>> That's not necessary (or wise).
>>
>> Let's not confuse a Ram device with a MemoryRegion.  They are separate
>> things and should be treated as separate things.  I thought we
>> discussed that MemoryRegions are stateless (or at least, there state
>> is all derived) and don't need to be serialized?
>
> Well, the actual bits in memory are state.  All other attributes are
> indeed derived.
>
> I just think that making any device that has a bit of RAM a composed
> device is overkill.  What do we gain from it?  The cost is not trivial.

The cost of composition is pretty trivial.

struct VGADevice {
    DeviceState parent;

    Ram vga_ram;
};

static void vga_device_initfn(Object *obj)
{
    VGADevice *vga = VGA(obj);

    object_property_add_child(obj, "vram", (Object *)&vga_ram,
                              TYPE_RAM, NULL);
}

Now a user can control the VGA ram size by accessing vga/vram.size = 16M.

Regards,

Anthony Liguori