Re: [Qemu-devel] [PATCH for-2.12 v3 05/11] spapr: introduce an IRQ allocator using a bitmap

[David Gibson]

On Fri, Nov 17, 2017 at 08:19:23AM +0100, Cédric Le Goater wrote:
> On 11/17/2017 05:50 AM, David Gibson wrote:
> > On Tue, Nov 14, 2017 at 10:42:24AM +0100, Greg Kurz wrote:
> >> On Fri, 10 Nov 2017 15:20:11 +0000
> >> Cédric Le Goater <address@hidden> wrote:
> >>
> >>> Let's define a new set of XICSFabric IRQ operations for the latest
> >>> pseries machine. These simply use a a bitmap 'irq_map' as a IRQ number
> >>> allocator.
> >>>
> >>> The previous pseries machines keep the old set of IRQ operations using
> >>> the ICSIRQState array.
> >>>
> >>> Signed-off-by: Cédric Le Goater <address@hidden>
> >>> ---
> >>>
> >>>  Changes since v2 :
> >>>
> >>>  - introduced a second set of XICSFabric IRQ operations for older
> >>>    pseries machines
> >>>
> >>>  hw/ppc/spapr.c         | 76 
> >>> ++++++++++++++++++++++++++++++++++++++++++++++----
> >>>  include/hw/ppc/spapr.h |  3 ++
> >>>  2 files changed, 74 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 4bdceb45a14f..4ef0b73559ca 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -1681,6 +1681,22 @@ static const VMStateDescription 
> >>> vmstate_spapr_patb_entry = {
> >>>      },
> >>>  };
> >>>  
> >>> +static bool spapr_irq_map_needed(void *opaque)
> >>> +{
> >>> +    return true;
> >>
> >> I see that the next patch adds some code to avoid sending the
> >> bitmap if it doesn't contain state, but I guess you should also
> >> explicitly have this function to return false for older machine
> >> types (see remark below).
> > 
> > I don't see that you should need to migrate this at all.  The machine
> > needs to reliably allocate the same interrupts each time, and that
> > means source and dest should have the same allocations without
> > migrating data.
> 
> ok. so we need to make sure that hot plugging devices or CPUs does
> not break that scheme. This is not the case today if you don't follow
> the exact same order on the monitor.

Ok, that's already broken then :/.

AFAIK plugging CPUs shouldn't matter though.  Plugging devices might -
which is exactly why the approach of using an irq "allocator" as such
isn't a very good one.  We might have to have something for MSI-X,
which will then require migration of some data, but we shouldn't need
it for the intx and other fixed interrupts.

-- 
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