Re: [Qemu-devel] [PATCH for-2.10 2/2] xilinx-spips: add a migration blocker when using mmio_execution

[KONRAD Frederic]

On 08/01/2017 11:30 AM, Edgar E. Iglesias wrote:
> On Tue, Aug 01, 2017 at 11:13:56AM +0200, KONRAD Frederic wrote:
> > On 08/01/2017 11:00 AM, Peter Maydell wrote:
> > > On 1 August 2017 at 09:10, KONRAD Frederic <address@hidden> wrote:
> > > > This adds a migration blocker when mmio_execution has been used.
> > > >
> > > > Signed-off-by: KONRAD Frederic <address@hidden>
> > > > ---
> > > >   hw/ssi/xilinx_spips.c | 11 +++++++++++
> > > >   1 file changed, 11 insertions(+)
> > > >
> > > > diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> > > > index e833028..d46491f 100644
> > > > --- a/hw/ssi/xilinx_spips.c
> > > > +++ b/hw/ssi/xilinx_spips.c
> > > > @@ -31,6 +31,8 @@
> > > >   #include "hw/ssi/ssi.h"
> > > >   #include "qemu/bitops.h"
> > > >   #include "hw/ssi/xilinx_spips.h"
> > > > +#include "qapi/error.h"
> > > > +#include "migration/blocker.h"
> > > >
> > > >   #ifndef XILINX_SPIPS_ERR_DEBUG
> > > >   #define XILINX_SPIPS_ERR_DEBUG 0
> > > > @@ -139,6 +141,7 @@ typedef struct {
> > > >
> > > >       uint8_t lqspi_buf[LQSPI_CACHE_SIZE];
> > > >       hwaddr lqspi_cached_addr;
> > > > +    Error *migration_blocker;
> > > >   } XilinxQSPIPS;
> > > >
> > > >   typedef struct XilinxSPIPSClass {
> > > > @@ -603,6 +606,14 @@ static void *lqspi_request_mmio_ptr(void *opaque, hwaddr 
> > > > addr, unsigned *size,
> > > >       XilinxQSPIPS *q = opaque;
> > > >       hwaddr offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1);
> > > >
> > > > +    /* mmio_execution breaks migration better aborting than having strange
> > > > +     * bugs.
> > > > +     */
> > > > +    if (!q->migration_blocker) {
> > > > +        error_setg(&q->migration_blocker, "booting from SPI breaks migration");
> > > > +        migrate_add_blocker(q->migration_blocker, &error_fatal);
> > > > +    }
> > > > +
> > >
> > > This doesn't handle the case when migration is already in progress
> > > and this function is called (which will cause migrate_add_blocker
> > > to fail).
> >
> > Maybe I can make the request_ptr to fail if migration is in
> > progress.. But is that safe or do I risk a race.
>
> Hi Fred,
>
> At this stage, perhaps we should just register the blocker when this dev 
> realizes.
>
> If a request_ptr comes in during migration, the VM will fail either way...
>
> Cheers,
> Edgar

Hi Edgar,

Yes but this will breaks migration for the spips device everytime
and not only when mmio-execution is used?

Thanks,
Fred