> > On 21/09/2018 10:07, Markus Armbruster wrote: > > Quick review of just the QAPI part. > > > > David Hildenbrand <da...@redhat.com> writes: > > > >> From: Pankaj Gupta <pagu...@redhat.com> > >> > >> 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 <pagu...@redhat.com> > >> [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr", > >> split up patches ] > >> Signed-off-by: David Hildenbrand <da...@redhat.com> > > [...] > >> 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?
Sure. Thanks, Pankaj > > > > > * 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 >