On 09/05/2012 02:11 PM, Jan Kiszka wrote:
> On 2012-09-05 12:53, Avi Kivity wrote:
>> On 09/05/2012 01:36 PM, Jan Kiszka wrote:
>>>>
>>>> My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a
>>>> corresponding unref), which has the following requirements:
>>>>
>>>> - if the refcount is nonzero, MemoryRegion::opaque is safe to use
>>>> - if the refcount is nonzero, the MemoryRegion itself is stable.
>>>
>>> The second point means that the memory subsystem will cache the region
>>> state and apply changes only after leaving a handler that performed them?
>>
>> No. I/O callbacks may be called after a region has been disabled.
>>
>> I guess we can restrict this to converted regions, so we don't introduce
>> regressions.
>
> s/can/have to/. This property change will require some special care,
> hopefully mostly at the memory layer. A simple scenario from recent patches:
>
> if (<enable>) {
> memory_region_set_address(&s->pm_io, pm_io_base);
> memory_region_set_enabled(&s->pm_io, true);
> } else {
> memory_region_set_enabled(&s->pm_io, false);
> }
I am unable to avoid pointing out that this can be collapsed to
memory_region_set_address(&s->pm_io, pm_io_base);
memory_region_set_enabled(&s->pm_io, <enable>);
as the address is meaningless when disabled. Sorry.
>
> We will have to ensure that set_enabled(..., true) will never cause a
> dispatch using an outdated base address.
No, this is entirely safe. If the guest changes a region offset
concurrently with issuing mmio on it, then it must expect either the old
or new offset to be used during dispatch. In either case, the correct
intra-region offset will be provided to the I/O callback (no volatile
MemoryRegion fields except ->readable (IIRC) are used during dispatch -
the rest are all copied into data structures used during dispatch (this
is part of what makes the whole thing so rcu friendly).
> I think discussing semantics and usage patterns of the new memory API -
> outside of the BQL - will be the next big topic. ;)
I hope it won't prove to be that complicated.
--
error compiling committee.c: too many arguments to function
