Re: [Qemu-devel] [PATCH 1/7] virtio-pmem: add virtio device

[Pankaj Gupta <pagupta@redhat.com>]

> 
> On 5/23/19 5:24 AM, Pankaj Gupta wrote:
> > This is the implementation of virtio-pmem device. Support will require
> > machine changes for the architectures that will support it, so it will
> > not yet be compiled. It can be unlocked with VIRTIO_PMEM_SUPPORTED per
> > machine and disabled globally via VIRTIO_PMEM.
> > 
> > We cannot use the "addr" property as that is already used e.g. for
> > virtio-pci/pci devices. And we will have e.g. virtio-pmem-pci as a proxy.
> > So we have to choose a different one (unfortunately). "memaddr" it is.
> > That name should ideally be used by all other virtio-* based memory
> > devices in the future.
> >     -device virtio-pmem-pci,id=p0,bus=bux0,addr=0x01,memaddr=0x1000000...
> > 
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > [ QAPI bits ]
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > [ MemoryDevice/MemoryRegion changes, cleanups, addr property "memaddr",
> >   split up patches, unplug handler ]
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> 
> > +++ b/qapi/misc.json
> > @@ -2742,16 +2742,42 @@
> >            }
> >  }
> >  
> > +##
> > +# @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: 4.0
> 
> You've missed 4.0; this should be 4.1.

Yes. Will update.

> 
> > +##
> > +{ 'struct': 'VirtioPMEMDeviceInfo',
> > +  'data': { '*id': 'str',
> 
> Why is id optional? Does it make sense to have a device without an id?

I think that's how it has been for both NVDIMM and DIMM. And it works
fine with optional 'id', but requires unique 'id' for underlying memory device. 
That means its okay to keep 'id' optional for root dimm/nvdimm/virtio-pmem 
devices. 

Thanks,
Pankaj

> 
> > +            'memaddr': 'size',
> > +            'size': 'size',
> > +            'memdev': 'str'
> > +          }
> > +}
> > +
> >  ##
> >  # @MemoryDeviceInfo:
> >  #
> >  # Union containing information about a memory device
> >  #
> > +# nvdimm is included since 2.12. virtio-pmem is included since 4.0.
> 
> 4.1

o.k

> 
> > +#
> >  # Since: 2.1
> >  ##
> >  { 'union': 'MemoryDeviceInfo',
> >    'data': { 'dimm': 'PCDIMMDeviceInfo',
> > -            'nvdimm': 'PCDIMMDeviceInfo'
> > +            'nvdimm': 'PCDIMMDeviceInfo',
> > +            'virtio-pmem': 'VirtioPMEMDeviceInfo'
> >            }
> >  }
> >  
> > 
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 
>