Re: [Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* KONRAD Frederic (frederic.konrad@adacore.com) wrote:
> 
> 
> On 07/20/2017 12:02 PM, Dr. David Alan Gilbert wrote:
> > * Peter Maydell (peter.maydell@linaro.org) wrote:
> > > On 17 July 2017 at 19:58, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > > > * Edgar E. Iglesias (edgar.iglesias@gmail.com) wrote:
> > > > > Is there a way we can prevent migration of the RAMBlock?
> > > > 
> > > > Not yet, I think we'd have to:
> > > >     a) Add a flag to the RAMBlock
> > > >     b) Set it/clear it on registration
> > > >     c) Have a RAMBLOCK_FOREACH_MIGRATABLE macro
> > > >     d) Replace all of the RAMBLOCK_FOREACH (and the couple of hand coded
> > > >     cases) with the RAMBLOCK_FOREACH_MIGRATABLE
> > > >     e) Worry about the corner cases!
> > > > 
> > > > I've got a few worries about what happens when the kernel tries to
> > > > do dirty yncing - I'm not sure if we have to change anything on that
> > > > interface to skip those RAMBlocks.
> > > 
> > > OK, so what should we do for 2.10 ?
> > > 
> > > We could:
> > >   * implement the changes you suggest above, and mark only
> > >     vmstate_register_ram'd blocks as migratable
> > >     (would probably need to fix some places which buggily
> > >     don't call vmstate_register_ram)
> > >   * implement the changes above, but special case mmio-interface
> > >     so only its ramblock is marked unmigratable
> > 
> > I think either of these is too late for 2.10 - I don't fancy prodding
> > about in all of the migration RAM loops at this stage.
> > 
> > >   * postpone the changes above until 2.11, and for 2.10 register
> > >     a migration-blocker in mmio-interface so that we at least
> > >     give the user a useful error rather than having it fail
> > >     obscurely on vmload (and release note this)
> > 
> > I think that's best, especially because I've just thought of another nasty.
> > If I understand the way mmio-interface is working, you're dynamically
> > changing the RAMBlock list while the guest is running.
> > And while we are using QLIST_FOREACH_RCU I'm not convinced we're
> > actually safe against dynamic modification of that list.
> 
> Hi Dave,
> 
> Hmmm I'm not totally convinced..
> 
> Let's imagine somebody (eg: u-boot guest) wants to execute code
> from the LQSPI area.
> 
> memory_region_request_mmio_ptr is called (the guest is not
> running yet) it will create a mmio-interface which create the
> ram memory region with a pointer provided by the device.
> Then the guest can execute code from that and this somehow is
> breaking migration because this ram memory region is migrated.
> 
> Now something is writing to the device.. The cache is no longer
> valid, we have to drop this mmio-interface. This is done in an
> async_safe work so cpu are stopped at this moment. So I think we
> are safe for that.
> 
> > 
> > > (Or something else?)
> > > 
> > > I do think we definitely need to fix this for 2.11 at latest.
> > 
> > OK, I can do a-e OK, I'm more worried now about that dynamic
> > modification I just thought of.
> > 
> 
> What's an a-e?

Oh sorry, didn't answer that bit - it's the items a..e in the list
I posted above.

> BTW sorry for this pain :( I didn't figure out that the
> ram MemoryRegion would be migrated..

It's OK, these things happen.

Dave

> Thanks,
> Fred
> 
> > Dave
> > 
> > > thanks
> > > -- PMM
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> > 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK