On Fri, 10 Nov 2017 15:20:11 +0000 Cédric Le Goater <c...@kaod.org> 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 <c...@kaod.org> > --- > > 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). > +} > + > +static const VMStateDescription vmstate_spapr_irq_map = { > + .name = "spapr_irq_map", > + .version_id = 0, > + .minimum_version_id = 0, > + .needed = spapr_irq_map_needed, > + .fields = (VMStateField[]) { > + VMSTATE_BITMAP(irq_map, sPAPRMachineState, 0, nr_irqs), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > static const VMStateDescription vmstate_spapr = { > .name = "spapr", > .version_id = 3, > @@ -1700,6 +1716,7 @@ static const VMStateDescription vmstate_spapr = { > &vmstate_spapr_ov5_cas, > &vmstate_spapr_patb_entry, > &vmstate_spapr_pending_events, > + &vmstate_spapr_irq_map, > NULL > } > }; > @@ -2337,8 +2354,12 @@ static void ppc_spapr_init(MachineState *machine) > /* Setup a load limit for the ramdisk leaving room for SLOF and FDT */ > load_limit = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FW_OVERHEAD; > > + /* Initialize the IRQ allocator */ > + spapr->nr_irqs = XICS_IRQS_SPAPR; > + spapr->irq_map = bitmap_new(spapr->nr_irqs); > + I think you should introduce a sPAPRMachineClass::has_irq_bitmap boolean so that the bitmap is only allocated for newer machine types. And you should then use this flag in spapr_irq_map_needed() above. Apart from that, the rest of the patch looks good. > /* Set up Interrupt Controller before we create the VCPUs */ > - xics_system_init(machine, XICS_IRQS_SPAPR, &error_fatal); > + xics_system_init(machine, spapr->nr_irqs, &error_fatal); > > /* Set up containers for ibm,client-architecture-support negotiated > options > */ > @@ -3560,7 +3581,7 @@ static int ics_find_free_block(ICSState *ics, int num, > int alignnum) > return -1; > } > > -static bool spapr_irq_test(XICSFabric *xi, int irq) > +static bool spapr_irq_test_2_11(XICSFabric *xi, int irq) > { > sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > ICSState *ics = spapr->ics; > @@ -3569,7 +3590,7 @@ static bool spapr_irq_test(XICSFabric *xi, int irq) > return !ICS_IRQ_FREE(ics, srcno); > } > > -static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align) > +static int spapr_irq_alloc_block_2_11(XICSFabric *xi, int count, int align) > { > sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > ICSState *ics = spapr->ics; > @@ -3583,7 +3604,7 @@ static int spapr_irq_alloc_block(XICSFabric *xi, int > count, int align) > return srcno + ics->offset; > } > > -static void spapr_irq_free_block(XICSFabric *xi, int irq, int num) > +static void spapr_irq_free_block_2_11(XICSFabric *xi, int irq, int num) > { > sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > ICSState *ics = spapr->ics; > @@ -3601,6 +3622,46 @@ static void spapr_irq_free_block(XICSFabric *xi, int > irq, int num) > } > } > > +static bool spapr_irq_test(XICSFabric *xi, int irq) > +{ > + sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > + int srcno = irq - spapr->ics->offset; > + > + return test_bit(srcno, spapr->irq_map); > +} > + > +static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align) > +{ > + sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > + int start = 0; > + int srcno; > + > + /* > + * The 'align_mask' parameter of bitmap_find_next_zero_area() > + * should be one less than a power of 2; 0 means no > + * alignment. Adapt the 'align' value of the former allocator to > + * fit the requirements of bitmap_find_next_zero_area() > + */ > + align -= 1; > + > + srcno = bitmap_find_next_zero_area(spapr->irq_map, spapr->nr_irqs, start, > + count, align); > + if (srcno == spapr->nr_irqs) { > + return -1; > + } > + > + bitmap_set(spapr->irq_map, srcno, count); > + return srcno + spapr->ics->offset; > +} > + > +static void spapr_irq_free_block(XICSFabric *xi, int irq, int num) > +{ > + sPAPRMachineState *spapr = SPAPR_MACHINE(xi); > + int srcno = irq - spapr->ics->offset; > + > + bitmap_clear(spapr->irq_map, srcno, num); > +} > + > static void spapr_pic_print_info(InterruptStatsProvider *obj, > Monitor *mon) > { > @@ -3778,7 +3839,12 @@ static void > spapr_machine_2_11_instance_options(MachineState *machine) > > static void spapr_machine_2_11_class_options(MachineClass *mc) > { > - /* Defaults for the latest behaviour inherited from the base class */ > + XICSFabricClass *xic = XICS_FABRIC_CLASS(mc); > + > + spapr_machine_2_12_class_options(mc); > + xic->irq_test = spapr_irq_test_2_11; > + xic->irq_alloc_block = spapr_irq_alloc_block_2_11; > + xic->irq_free_block = spapr_irq_free_block_2_11; > } > > DEFINE_SPAPR_MACHINE(2_11, "2.11", false); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 9d21ca9bde3a..5835c694caff 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -7,6 +7,7 @@ > #include "hw/ppc/spapr_drc.h" > #include "hw/mem/pc-dimm.h" > #include "hw/ppc/spapr_ovec.h" > +#include "qemu/bitmap.h" > > struct VIOsPAPRBus; > struct sPAPRPHBState; > @@ -78,6 +79,8 @@ struct sPAPRMachineState { > struct VIOsPAPRBus *vio_bus; > QLIST_HEAD(, sPAPRPHBState) phbs; > struct sPAPRNVRAM *nvram; > + int32_t nr_irqs; > + unsigned long *irq_map; > ICSState *ics; > sPAPRRTCState rtc; >