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

[Michael S. Tsirkin]

On Tue, Nov 28, 2017 at 05:43:16PM +0000, Dr. David Alan Gilbert wrote:
> * Victor Kaplansky (address@hidden) wrote:
> > > 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.
> 
> OK thanks for the clarification Michael, Victor!
> 
> Dave

Sorry, what I said was completely wrong. Was mixing up vhost and
vhost-user.

> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK