Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 17/22] virtio-pmem: prototype

[David Hildenbrand]

On 21/09/2018 10:07, Markus Armbruster wrote:
> Quick review of just the QAPI part.
> 
> David Hildenbrand <address@hidden> writes:
> 
>> From: Pankaj Gupta <address@hidden>
>>
>> This is the current protoype of virtio-pmem. Support will require
>> machine changes for the architectures that will support it, so it will
>> not yet be compiled.
>>
>> Signed-off-by: Pankaj Gupta <address@hidden>
>> [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr",
>>   split up patches ]
>> Signed-off-by: David Hildenbrand <address@hidden>
> [...]
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index 7c36de0464..cadbca26ac 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -2907,6 +2907,29 @@
>>            }
>>  }
>>  
>> +##
>> +# @VirtioPMemDeviceInfo:
>> +#
>> +# VirtioPMem state information
>> +#
>> +# @id: device's ID
>> +#
>> +# @memaddr: physical address in memory, where device is mapped
>> +#
>> +# @size: size of memory that the device provides
>> +#
>> +# @memdev: memory backend linked with device
>> +#
>> +# Since: 3.1
>> +##
>> +{ 'struct': 'VirtioPMemDeviceInfo',
>> +  'data': { '*id': 'str',
>> +            'memaddr': 'size',
>> +            'size': 'size',
>> +            'memdev': 'str'
>> +          }
>> +}
> 
> This set of members is a proper subset of PCDIMMDeviceInfo's, except
> 
> * It uses 'size' instead of 'int', which is an improvement.  Improve
>   PCDIMMDeviceInfo as well?

That certainly makes sense.

@Pankaj do you want to take care of that?

> 
> * The physical address member is called 'memaddr' instead of 'addr'.
>   Why?
> 

Very good point. Have a look at "memory-device: add device class
function set_addr()" (patch #10).

Summary: The property name on the device will be called "memaddr", as
"addr" is already (unfortunately) used for virtio addressing, that's why
I feel like we should name it here "memaddr", too.

("addr" is too generic, a collision had to happen :( )

-- 

Thanks,

David / dhildenb