Re: [Qemu-devel] [PATCH] vhost-user spec: Clarify policy on setting log_base

[Victor Kaplansky]

> From: "Michael S. Tsirkin" <address@hidden>
> To: "Dr. David Alan Gilbert" <address@hidden>
> Cc: "Victor Kaplansky" <address@hidden>, address@hidden, "Maxime Coquelin" 
> <address@hidden>
> Sent: Tuesday, November 28, 2017 6:12:44 PM
> Subject: Re: [Qemu-devel] [PATCH] vhost-user spec: Clarify policy on setting 
> log_base
> 
> On Tue, Nov 28, 2017 at 04:04:21PM +0000, Dr. David Alan Gilbert wrote:
> > * Victor Kaplansky (address@hidden) wrote:
> > > From: Victor Kaplansky <address@hidden>
> > > 
> > > If we allow qemu to change logging area after it was already established,
> > > it may require from the backend to acquire a lock on each access to
> > > the log_base, which has a potential quite a big performance hit.
> > > 
> > > Thus we would like to clarify in the spec, that qemu is not expected
> > > to resize or remap the logging area, and backend implementations
> > > can safely ignore subsequent requests to log_base modifications.
> > 
> > There's quite a bit of code in vhost to cope with changes in mappings
> > of regions; and there's already code in there to handle log size
> > changes:
> > 
> > static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t
> > size)
> > {
> >     struct vhost_log *log = vhost_log_get(size,
> >     vhost_dev_log_is_shared(dev));
> >     uint64_t log_base = (uintptr_t)log->log;
> >     int r;
> > 
> >     /* inform backend of log switching, this must be done before
> >        releasing the current log, to ensure no logging is lost */
> >     r = dev->vhost_ops->vhost_set_log_base(dev, log_base, log);
> > 
> > so when is a log resize legal?
> > 
> > Dave
> 
> Log base has nothing to do with log resize.
> 
> It supplies the physical address of the ring
> for dirty logging purposes.
> 

Right. Just for complete picture, it also supplies the address for
dirty logging of where descriptors can point, in other words for logging
whole guest's physical memory. It is why a single log_base is enough for
whole device, and there is no need to specify it for every ring separately.

Having said that, there is an obscure possibility to place the ring in
a memory different from guest's physical memory, but as far as I know there
is no backend that supports this now.

The introduction of this clarification to the vhost-user spec, either with
*must* or without it, will enable us to fix in DPDK several BZs open for qemu
and related to migration failure of MQ vhost-user.