On Fri, 26 Jul 2019 17:01:36 +0200 Cédric Le Goater <c...@kaod.org> wrote:
> On 26/07/2019 16:44, Greg Kurz wrote: > > PHBs already take care of clearing the MSIs from the bitmap during reset > > or unplug. No need to do this globally from the machine code. Rather add > > an assert to ensure that PHBs have acted as expected. > > This works because spar_irq_reset() is called after qemu_devices_reset(). > I guess this is fine. > Yeah and we have this comment in spapr_machine_reset(): /* * This is fixing some of the default configuration of the XIVE * devices. To be called after the reset of the machine devices. */ spapr_irq_reset(spapr, &error_fatal); I guess this is enough to prevent someone to break things. > Reviewed-by: Cédric Le Goater <c...@kaod.org> > > Thanks, > > C. > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > --- > > hw/ppc/spapr.c | 4 ---- > > hw/ppc/spapr_irq.c | 7 ++----- > > include/hw/ppc/spapr_irq.h | 1 - > > 3 files changed, 2 insertions(+), 10 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 5894329f29a9..855e9fbd9805 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1739,10 +1739,6 @@ static void spapr_machine_reset(MachineState > > *machine) > > ppc_set_compat(first_ppc_cpu, spapr->max_compat_pvr, &error_fatal); > > } > > > > - if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) { > > - spapr_irq_msi_reset(spapr); > > - } > > - > > /* > > * NVLink2-connected GPU RAM needs to be placed on a separate NUMA > > node. > > * We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which > > is > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index d07aed8ca9f9..c72d8433681d 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -57,11 +57,6 @@ void spapr_irq_msi_free(SpaprMachineState *spapr, int > > irq, uint32_t num) > > bitmap_clear(spapr->irq_map, irq - SPAPR_IRQ_MSI, num); > > } > > > > -void spapr_irq_msi_reset(SpaprMachineState *spapr) > > -{ > > - bitmap_clear(spapr->irq_map, 0, spapr->irq_map_nr); > > -} > > - > > static void spapr_irq_init_kvm(SpaprMachineState *spapr, > > SpaprIrq *irq, Error **errp) > > { > > @@ -729,6 +724,8 @@ int spapr_irq_post_load(SpaprMachineState *spapr, int > > version_id) > > > > void spapr_irq_reset(SpaprMachineState *spapr, Error **errp) > > { > > + assert(bitmap_empty(spapr->irq_map, spapr->irq_map_nr)); > > + > > if (spapr->irq->reset) { > > spapr->irq->reset(spapr, errp); > > } > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > > index f965a58f8954..44fe4f9e0e2e 100644 > > --- a/include/hw/ppc/spapr_irq.h > > +++ b/include/hw/ppc/spapr_irq.h > > @@ -28,7 +28,6 @@ void spapr_irq_msi_init(SpaprMachineState *spapr, > > uint32_t nr_msis); > > int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, > > Error **errp); > > void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); > > -void spapr_irq_msi_reset(SpaprMachineState *spapr); > > > > typedef struct SpaprIrq { > > uint32_t nr_irqs; > > >