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

[KONRAD Frederic]

On 07/20/2017 12:02 PM, Dr. David Alan Gilbert wrote:
> * Peter Maydell (address@hidden) wrote:
> > On 17 July 2017 at 19:58, Dr. David Alan Gilbert <address@hidden> wrote:
> > > * Edgar E. Iglesias (address@hidden) 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?

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

Thanks,
Fred

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