Re: [Qemu-devel] [PATCH v7 0/7] push mmio dispatch out of big lock

[liu ping fan]

On Fri, May 3, 2013 at 4:04 PM, Jan Kiszka <address@hidden> wrote:
> On 2013-05-03 09:37, liu ping fan wrote:
>> On Fri, May 3, 2013 at 12:58 AM, Jan Kiszka <address@hidden> wrote:
>>> Hi Pingfan,
>>>
>>> On 2012-12-06 08:28, liu ping fan wrote:
>>>> Any suggestion? Or new design idea for this?
>>>
>>> Finally... I'm getting back to this. I'm currently trying to make use of
>>> this series, adapting it to my needs (selective BQL-free dispatching of
>>> PIO regions).
>>>
>> Glad that you are back :)
>>
>>> Is there a newer version available on your side? This one obviously no
>>
>> No, but I can see the code and rebase next week.
>
> I've already rebased and started to fix/cleanup. Today I will look into
> the ref/unref vs. Object topic and try a first unlocked device model. I
> can share afterward so that we do not need to do the work twice.
>
Ok, thanks.
>>
>>> longer applies due to all the code movements in QEMU. But it also seems
>>> to contain some bugs, at least in patch 5 (mixed up page number vs. page
>>> address around for address_space_section_lookup_ref).
>>>
>> Will pay some time to see it.
>
> Fixed. Specifically subpages were broken. As I've converted portio to
> use the memory core dispatcher, I'm getting a lot of them now and
> triggered the bugs immediately.
>
> There is still some nesting issue around coalesced MMIO flushing. Need
> to look into this today as well. So far I've worked around it by
> assuming that nesting is fine if we enter the dispatcher with BQL held.
>
>>
>>> Then we should get rid of the ref/unref callbacks. Making a memory
>>> region BQL-free must be as simple as setting a flag or (more likely)
>>> adding a reference to the owning QOM object in the region.
>>> Reimplementing ref/unref in device models over and over again is clearly
>>> a no-go. Maybe I'm currently forgetting a use case where overloading the
>>
>> At the beginning, Avi suggest to enforce mr->opaque to be Device
>> object, but due to the nested embedded Object, we fail. And finally
>> Avi suggest ref/unref interface.
>> From my point,  we can save lots of reimplementing ref/unref in device
>> models by telling whether mr->opauque is Object or not.  And leave not
>> object case to reimplement ref/unref.
>
> We can't change the semantics of opaque as long as old_mmio / old_portio
> are around. But we need a flag anyway to indicate if a region is
> depending on BQL or not. Adding a separate "Object *owner" to
> MemoryRegion can serve both purposes. Then we define something like
>
> void memory_region_set_local_locking(MemoryRegion *mr,
>                                      bool local_locking,
>                                      Object *owner);
>
I think, owner imply local_locking, so we need not call this function
for each device,
just those we plan to push out of biglock

> to control the property (if local_locking is true, owner must be
> non-NULL, of course). That's quite similar to my old prototype here that
> had memory_region_set/clear_global_locking.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux