* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On 12 April 2018 at 11:18, Cédric Le Goater <c...@kaod.org> wrote:
> > On the POWER9 processor, the XIVE interrupt controller can control
> > interrupt sources using MMIO to trigger events, to EOI or to turn off
> > the sources. Priority management and interrupt acknowledgment is also
> > controlled by MMIO in the presenter sub-engine.
> >
> > These MMIO regions are exposed to guests in QEMU with a set of 'ram
> > device' memory mappings, similarly to VFIO, and the VMAs are populated
> > dynamically with the appropriate pages using a fault handler.
> >
> > But, these regions are an issue for migration. We need to discard the
> > associated RAMBlocks from the RAM state on the source VM and let the
> > destination VM rebuild the memory mappings on the new host in the
> > post_load() operation just before resuming the system.
> >
> > To achieve this goal, the following introduces a new helper,
> > ram_block_is_migratable(), which identifies RAMBlocks to discard on
> > the source. Some checks are also performed on the destination to make
> > sure nothing invalid was sent.
>
> David suggested on IRC that we would want a flag on the ramblock
> for "not migratable", because there are other uses for "don't
> migrate this" than just "is this a ram device".
My original suggestion to your series was with a flag, but I'd forgotten
about that by the time I'd made the suggestion to Cédric.
In your case would just adding an extra term to the
ram_block_is_migratable function work, or do you really need a flag?
Dave
> > Signed-off-by: Cédric Le Goater <c...@kaod.org>
> > ---
> >
> > I am not sure we want to taker into account patchew complaint :
> >
> > ERROR: braces {} are necessary for all arms of this statement
> > #52: FILE: migration/ram.c:203:
> > + if (ram_block_is_migratable(block))
> > [...]
> >
> > total: 1 errors, 0 warnings, 136 lines checked
> >
> > migration/ram.c | 52 ++++++++++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 42 insertions(+), 10 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 0e90efa09236..32371950865b 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -188,6 +188,21 @@ void ramblock_recv_bitmap_set_range(RAMBlock *rb, void
> > *host_addr,
> > }
> >
> > /*
> > + * Identifies RAM blocks which should be discarded from migration. For
> > + * the moment, it only applies to blocks backed by a 'ram_device'
> > + * memory region.
> > + */
> > +static inline bool ram_block_is_migratable(RAMBlock *block)
> > +{
> > + return !memory_region_is_ram_device(block->mr);
> > +}
> > +
> > +/* Should be holding either ram_list.mutex, or the RCU lock. */
> > +#define RAMBLOCK_FOREACH_MIGRATABLE(block) \
> > + RAMBLOCK_FOREACH(block) \
> > + if (ram_block_is_migratable(block))
>
> This will mishandle some uses, like:
>
> if (foo)
> RAMBLOCK_FOREACH_MIGRATABLE(block)
> stuff;
> else
> morestuff;
>
> as the if() inside the macro will capture the else clause.
> (The lack of braces in the calling code would be against our
> coding style, of course, so not very likely.)
>
> Eric, is there a 'standard' trick for this? I thought of
> maybe
>
> #define RAMBLOCK_FOREACH_MIGRATABLE(block) \
> RAMBLOCK_FOREACH(block) \
> if (!ram_block_is_migratable(block)) {} else
>
> ?
>
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
