From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42444)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Zvk5d-00069B-9N
	for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:22 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Zvk5c-0000hQ-CI
	for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:21 -0500
Received: from mail-yk0-x233.google.com ([2607:f8b0:4002:c07::233]:35950)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Zvk5c-0000h0-8u
	for qemu-devel@nongnu.org; Mon, 09 Nov 2015 05:54:20 -0500
Received: by ykba4 with SMTP id a4so257381777ykb.3
	for <qemu-devel@nongnu.org>; Mon, 09 Nov 2015 02:54:20 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <56407A2C.7050906@redhat.com>
References: <1446747358-18214-1-git-send-email-peter.maydell@linaro.org>
	<1446747358-18214-9-git-send-email-peter.maydell@linaro.org>
	<56407A2C.7050906@redhat.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Mon, 9 Nov 2015 10:54:00 +0000
Message-ID: <CAFEAcA-W1forLUTHf=NdqoiBUDT35PBAzveKq3XVC+xZb3QtTA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH 08/16] exec.c: Have one io_mem_watch per
	AddressSpace
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: Paolo Bonzini <pbonzini@redhat.com>
Cc: Patch Tracking <patches@linaro.org>, QEMU Developers <qemu-devel@nongnu.org>, qemu-arm@nongnu.org, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, =?UTF-8?B?QWxleCBCZW5uw6ll?= <alex.bennee@linaro.org>, =?UTF-8?Q?Andreas_F=C3=A4rber?= <afaerber@suse.de>

On 9 November 2015 at 10:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 05/11/2015 19:15, Peter Maydell wrote:
>> @@ -2045,17 +2037,18 @@ static MemTxResult watch_mem_read(void *opaque, hwaddr addr, uint64_t *pdata,
>>  {
>>      MemTxResult res;
>>      uint64_t data;
>> +    AddressSpace *as = opaque;
>>
>>      check_watchpoint(addr & ~TARGET_PAGE_MASK, size, attrs, BP_MEM_READ);
>>      switch (size) {
>>      case 1:
>> -        data = address_space_ldub(&address_space_memory, addr, attrs, &res);
>> +        data = address_space_ldub(as, addr, attrs, &res);
>>          break;
>>      case 2:
>> -        data = address_space_lduw(&address_space_memory, addr, attrs, &res);
>> +        data = address_space_lduw(as, addr, attrs, &res);
>>          break;
>>      case 4:
>> -        data = address_space_ldl(&address_space_memory, addr, attrs, &res);
>> +        data = address_space_ldl(as, addr, attrs, &res);
>>          break;
>>      default: abort();
>>      }
>
> current_cpu is available here, so it should be possible to have only one
> io_mem_watch per CPU address space index (i.e. two).

So the opaque gives you the asidx and then you look at current_cpu's
cpu_ases[asidx]? Yeah, that works and is simpler. (Good argument for
making "number of ASes per CPU" a compile-time constant I guess.)

> But actually, if the address space can be derived from the attributes,
> you just need to fish the right address space out of current_cpu.
>
>> +    as->io_mem_watch = g_new0(MemoryRegion, 1);
>
> This is leaked when the address space goes away.  You can allocate it
> statically inside AddressSpace if my alternative plan from above doesn't
> work out.

Oh, right, it's not sufficient to unref it, it doesn't auto-free
itself when the refcount drops to 0.

thanks
-- PMM