Re: [Qemu-devel] [patch v4 10/16] memory: introduce lock ops for MemoryRegionOps

[liu ping fan]

On Mon, Oct 22, 2012 at 6:30 PM, Avi Kivity <address@hidden> wrote:
> On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
>> This can help memory core to use mr's fine lock to mmio dispatch.
>>
>> diff --git a/memory.c b/memory.c
>> index d528d1f..86d5623 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1505,13 +1505,27 @@ void set_system_io_map(MemoryRegion *mr)
>>
>>  uint64_t io_mem_read(MemoryRegion *mr, target_phys_addr_t addr, unsigned 
>> size)
>>  {
>> -    return memory_region_dispatch_read(mr, addr, size);
>> +    uint64_t ret;
>> +    if (mr->ops->lock) {
>> +        mr->ops->lock(mr);
>> +    }
>> +    ret = memory_region_dispatch_read(mr, addr, size);
>> +    if (mr->ops->lock) {
>> +        mr->ops->unlock(mr);
>> +    }
>> +    return ret;
>>  }
>>
>>  void io_mem_write(MemoryRegion *mr, target_phys_addr_t addr,
>>                    uint64_t val, unsigned size)
>>  {
>> +    if (mr->ops->lock) {
>> +        mr->ops->lock(mr);
>> +    }
>>      memory_region_dispatch_write(mr, addr, val, size);
>> +    if (mr->ops->lock) {
>> +        mr->ops->unlock(mr);
>> +    }
>>  }
>>
>>  typedef struct MemoryRegionList MemoryRegionList;
>> diff --git a/memory.h b/memory.h
>> index 9039411..5d00066 100644
>> --- a/memory.h
>> +++ b/memory.h
>> @@ -69,6 +69,8 @@ struct MemoryRegionOps {
>>                    unsigned size);
>>      int (*ref)(MemoryRegion *mr);
>>      void (*unref)(MemoryRegion *mr);
>> +    void (*lock)(MemoryRegion *mr);
>> +    void (*unlock)(MemoryRegion *mr);
>>
>>      enum device_endian endianness;
>>      /* Guest-visible constraints: */
>>
>
> Is this really needed?  Can't read/write callbacks lock and unlock
> themselves?
>
We can. But then, we need to expand the logic there, and use addr to
tell which fine lock to snatch and release.   Which one do you prefer,
fold the logic into memory core or spread it into devices?

> --
> error compiling committee.c: too many arguments to function
>