On 07/21/2011 04:17 PM, Jan Kiszka wrote:
On 2011-07-21 14:58, Avi Kivity wrote: > On 07/21/2011 03:52 PM, Jan Kiszka wrote: >>> >>> The problem is that "update" can change lots of things. offset, size, >>> whether it's mmio or RAM, read-onlyness, even the wierd things like >>> coalesced mmio. So it's either a function with 324.2 parameters (or a >>> large struct), or it's a series of functions with demarcation as to >>> where the update begins and ends. >> >> We do not need to provide update support for each and every bit, but for >> the common cases. memory_region_update_alias(region, offset, size) would >> be an excellent first candidate IMO. > > It's not enough, look at cirrus and PAM. It's a perfect fit for cirrus, but PAM indeed requires set_readonly in addition.
It isn't a pefect fit for cirrus. If the mode changes in a way that makes mapping the map as RAM possible, or vice versa, and if the banks are contiguous, then _update() results in two mappings or unmappings, while _commit() results in just one (since m_r_update_topology() merges the two adjacent regions).
I also think now that describing a memory region offline via a struct and then passing that to an atomic add/del/update would be a more handy and future-proof API than an increasing number set functions.
Maybe. But it's not sufficient for atomic changes involving multiple regions.
btw, there is another implementation issue involving SMP - if a region that obscures the middle of another region is removed, we'll have two regions removed and replaced with a larger one. That causes some memory to be temporarily inaccessible. I don't think it's a problem in practice, but if it is, we can fix it by stopping all vcpus if we detect this condition, and by adding an atomic change-memory-map-and-get-dirty-log ioctl to kvm.
-- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
