Re: [Qemu-devel] [PATCH 08/16] exec.c: Have one io_mem_watch per AddressSpace

[Peter Maydell]

On 9 November 2015 at 10:49, Paolo Bonzini <address@hidden> 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