From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:48018)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UZJdR-00017j-OJ
	for qemu-devel@nongnu.org; Mon, 06 May 2013 07:31:15 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UZJdN-0006rz-1t
	for qemu-devel@nongnu.org; Mon, 06 May 2013 07:31:13 -0400
Received: from mx1.redhat.com ([209.132.183.28]:40920)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UZJdM-0006rh-R3
	for qemu-devel@nongnu.org; Mon, 06 May 2013 07:31:08 -0400
Message-ID: <5187946F.6030700@redhat.com>
Date: Mon, 06 May 2013 13:30:55 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1353808984-22368-1-git-send-email-qemulist@gmail.com>
	<1353808984-22368-6-git-send-email-qemulist@gmail.com>
	<51879242.7030402@redhat.com> <51879326.9040607@siemens.com>
In-Reply-To: <51879326.9040607@siemens.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v7 5/7] memory: make mmio dispatch able to
	be out of biglock
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, "gleb@redhat.com" <gleb@redhat.com>, Stefan Hajnoczi <stefanha@gmail.com>, Marcelo Tosatti <mtosatti@redhat.com>, Liu Ping Fan <qemulist@gmail.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Anthony Liguori <anthony@codemonkey.ws>

Il 06/05/2013 13:25, Jan Kiszka ha scritto:
> On 2013-05-06 13:21, Paolo Bonzini wrote:
>> Il 25/11/2012 03:03, Liu Ping Fan ha scritto:
>>> @@ -3550,12 +3668,11 @@ void *address_space_map(AddressSpace *as,
>>>                          target_phys_addr_t *plen,
>>>                          bool is_write)
>>>  {
>>> -    AddressSpaceDispatch *d = as->dispatch;
>>>      target_phys_addr_t len = *plen;
>>>      target_phys_addr_t todo = 0;
>>>      int l;
>>>      target_phys_addr_t page;
>>> -    MemoryRegionSection *section;
>>> +    MemoryRegionSection *section, mr_obj;
>>>      ram_addr_t raddr = RAM_ADDR_MAX;
>>>      ram_addr_t rlen;
>>>      void *ret;
>>> @@ -3565,7 +3682,8 @@ void *address_space_map(AddressSpace *as,
>>>          l = (page + TARGET_PAGE_SIZE) - addr;
>>>          if (l > len)
>>>              l = len;
>>> -        section = phys_page_find(d, page >> TARGET_PAGE_BITS);
>>> +        address_space_section_lookup_ref(as, page >> TARGET_PAGE_BITS, &mr_obj);
>>> +        section = &mr_obj;
>>>  
>>>          if (!(memory_region_is_ram(section->mr) && !section->readonly)) {
>>>              if (todo || bounce.buffer) {
>>> @@ -3579,6 +3697,7 @@ void *address_space_map(AddressSpace *as,
>>>              }
>>>  
>>>              *plen = l;
>>> +            memory_region_section_unref(&mr_obj);
>>>              return bounce.buffer;
>>>          }
>>>          if (!todo) {
>>> @@ -3589,6 +3708,7 @@ void *address_space_map(AddressSpace *as,
>>>          len -= l;
>>>          addr += l;
>>>          todo += l;
>>> +        memory_region_section_unref(&mr_obj);
>>>      }
>>>      rlen = todo;
>>>      ret = qemu_ram_ptr_length(raddr, &rlen);
>>
>> I think this unref is wrong.  You need to delay it to the
>> address_space_unmap, and this in turns requires changing the signature
>> of address_space_map.
> 
> Can't RAMBlock hold a reference to the associated region? Then this
> could be retrieved on unmap without bothering the caller.

Right you are. :)  In fact, RAMBlock already does have block->mr.

Paolo