Re: [Qemu-devel] [for-2.11 PATCH 25/26] spapr_pci: drop abusive sanity check when migrating the LSI table

[David Gibson]

On Tue, Jul 25, 2017 at 08:03:21PM +0200, Greg Kurz wrote:
> The guest can allocate blocks of IRQs when calling the ibm,change-msi
> RTAS call. This has an impact on the IRQ numbers returned by subsequent
> calls to spapr_ics_alloc_block().
> 
> It doesn't cause any problem right now because PHB are cold plugged and
> the LSI table have the same numbers at the destination and the source.
> 
> But this won't be the case anymore when we support PHB hotplug. In this
> case the IRQ numbers in the LSI table are state that we should migrate.
> 
> This patch hence changes the destination to always accept the LSI table
> from the migration stream, like it is already done for MSIs. No effort
> is made to preserve the current behavior of failing migration when LSI
> numbers are different with older machine types, for simplicity.
> 
> Signed-off-by: Greg Kurz <address@hidden>

Urgh.  So, yes, there's a real problem here.  But fundamentally it's
because we're dynamically allocating the LSIs from a pool, rather than
having a fixed mapping (mea culpa, I did this in a bunch of places
before I knew better and it's come to bite us several times since).

Migrating the value like this will fix the problem, but I'm going to
have to think if it's the best way.  If we really do need an allocator
or something like it, then we'll have to migrate something - although
the fact that the LSIs will depend on the order you hotplug things is
kinda nasty still.

Making the LSIs fixed will address several other oddities in this
area, but will obviously been a wider ranging rework of the irq
assignment in the machine (and some machine version compatibility
cruft, of course).

> ---
>  hw/ppc/spapr_pci.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58406a1b7e93..157867af8178 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1887,8 +1887,7 @@ static const VMStateDescription vmstate_spapr_pci_lsi = 
> {
>      .version_id = 1,
>      .minimum_version_id = 1,
>      .fields = (VMStateField[]) {
> -        VMSTATE_UINT32_EQUAL(irq, struct spapr_pci_lsi, NULL),
> -
> +        VMSTATE_UINT32(irq, struct spapr_pci_lsi),
>          VMSTATE_END_OF_LIST()
>      },
>  };
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature