[Qemu-devel] Re: [PATCH 5/5] Port apic to new VMState design

[Juan Quintela]

Reply-to: address@hidden
Reimar Döffinger <address@hidden> wrote:
> On Tue, Aug 18, 2009 at 05:38:57PM +0200, Juan Quintela wrote:
>> Are your changes on upstream hw/eepro100.c?  I can't see anything there
>> that can't be done in a table approach.
>
> No, so far noone got around to taking my patches apart (and that is
> actually one I have not yet properly submitted, it is mangled into that
> patch: http://article.gmane.org/gmane.comp.emulators.qemu/49853
>
>> >> It is already that way.  This design don't change anything.  And I am
>> >> not sure how to fix it.  We don't have a "is this value safe for this
>> >> field", around yet.  It is possible to add some support for it, but I
>> >> would like to 1st have an use case.
>> >
>> > Well, I meant nowadays it is just possible to add a check in load_vm and
>> > fix any values that are off. While it is quite a bit of work there is
>> > nothing in the API stopping you from doing it, you even can return
>> > -EINVAL and hopefully the core will print some somewhat useful message.
>> 
>> I guess we are going to have an optional callback to be called
>> before/after loading the state.  You should be able to put your verify
>> there.
>
> Maybe I'm silly, but what would the callback for before loading state be
> good for?

qemu-kvm has in-kernel apic and pit (at least).  You just need to sync
state with the kernel after loading (the other way for saving).

>> > That is completely different from what I meant.
>> > Changing the RAM compromises the VM and only the VM, an exploit in a
>> > device emulation might allow to compromise the _host_. Is it now clearer
>> > what I meant?
>> 
>> yes, I see where you are meaning now.  But I guess that one is needed to
>> be solved, not only for migration.  Not sure what to do about this.
>
> I think it is mostly leg-work of finding the assumptions the emulations
> do. That really should be left to maintainers where available IMO.
> I'm just suggesting that it's better to design the API in a way that
> doesn't further discourage fixing this :-).
> If the patch is close to being accepted maybe I can help out by writing
> such verification code for vmware_vga, there e.g. depth, bypp, wred,
> wgreen and wblue must fit together as well as
> width/height/new_width/new_height and fb_size (I think)
> and width/height/bypp must be limit to ensure no integer overflows...

If you sent a patch, I will take a look.

Later, Juan.