Re: [virtio-comment] Re: [PATCH v1 3/8] device-context: Define the device context fields for device migration

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 10/23/2023 7:32 PM, Michael S. Tsirkin wrote:

> On Mon, Oct 23, 2023 at 06:03:10PM +0800, Zhu, Lingshan wrote:
> > config space, MMIO, registers work for years, what is wrong with them?
> >       
> Nothing as such. They don't seem to be appropriate for all use-case
> where people want to utilize virtio. I think a new transport
> will be needed to address these.
>     
    New transport for new type of devices for
      sure, like transport vq for SIOV.
      
      I agree admin vq or admin cmds are useful in some use cases, that
      is
      another story, should be case by case.
      
      For now, let's don't talk about all-use cases, just for current
      task, for live migration.
      
      So IMHO, I still think we should use config space registers to
      control live migration process.
    

> > > > Config space is control path, DMA is data-path, let's better not mix them,
> > > > we never expect to use config space to transfer data.
> > > >
> > > > So we need DMA to transfer data, for example I take advantages of device DMA
> > > > to logging dirty pages, This also applies to in-flight descriptors.
> > > >           
> > > As long as you do, I personally see little benefit to retrieve parts of
> > > state with memory mapped accesses.
> > >         
> > registers only control, and I personally believe a single register is much
> > better
> > than processing admin commands, more light-weight, more reliable, working
> > for years.
> >       
> Yea. It would be, if we could do everything through that register.
> But we can't really. Migration has too much data to pass around
> for that to be reasonable.
>     
    data are not transferred by registers, they
      only control.
      
      We transfer data by DMA, the device writes DMA dirty pages
      information(bitmap)
      to host isolated memory region.
    
> > Config space interfaces are fundamental for virtio-pci.
> >       
>
> They are in fact fundamental to virtio. Multiple transports to
> use config space are also fundamental.
>     
    I agree. So I also agree to build admin vq
      live migration solution based on our
      basic facilities, as Jason ever proposed.
    
> > > > And we are implementing virito live migration, not only for PCI.
> > > >
> > > > So both me and Jason keep repeating: We are implementing basic facilities,
> > > > and the implementation is transport specific.
> > > >           
> > > But the register based facilities you proposed are extremely limited and
> > > seem to only work for migration. For example, it seems mostly useless for
> > > debugging because retrieving state is rather complex and would
> > > interfere with normal working of the device.
> > >         
> > If you want to prove the register controlling interfaces are extremely
> > limited than admin vq or admin cmds,
> > you are also proving config space registers are extremely limited than
> > admin vq.
> >       
> Yes. Migration needs ability to pass large amounts of data around, and
> is too complex a functionality to work reliably without ability to
> report errors.
>     
    what errors? when device DMA?
      missing some dirty pages? If the device can detect such errors, it
      can recover by itself,
      or how can driver fix this?
      
      for control path, virtio uses re-read for many years and it works
      well. I believe we have
      went through this issue before.
      Â
> > So the question still here: do you want to replace current virtio-pci common
> > cfg
> > with admin vq or admin cmds?
> >       
> I think we need to add a new transport that will use admin commands.
> Which one to use would be up to a specific device.
>     
    For new device type like SIOV, yes we need a
      new transport, transport vq.
      
      Let's focus on this live migration feature, if there are new
      features in the future
      requires admin vq, let's discuss when they proposed.
    
> > And debug what? If you want to introduce more functionalities, we should
> > discuss
> > case by case.
> >
> > If debugging vq state, it is as easy as read queue_size, I don't see the
> > limitations
> > as queue_size work for years.
> >       
> No one reads queue_size. In fact for years we didn't have any debugging
> functionality and we are fine. If we are adding it, it really needs to
> be accessible when driver and device are wedged.
>     
    OK, I don't disagree to implement new device
      debugging features.
      
      But let's focus on current live migration task.
    
> > I still believe our goal is to do our best, with our capabilities, to build
> > the most optimal virtio spec
> > as we can do. Not other goals.
> >
> > Thanks
> > Zhu Lingshan
> > > > We have proposed to build admin vq based on our register solution, this can
> > > > somehow even help tp resolve the nested issue.
> > > >
> > > > But I see the proposed has been rejected.
> > > >
> > > > I still believe the goal is to build a best spec, not "just can work" with
> > > > limitations.
> > > >
> > > >
> > > >
> > > > >             
> > > >           
> > >         
> >       
>