Re: [Qemu-devel] [PATCH 3/3] spapr/irq: Drop spapr_irq_msi_reset()

[Cédric Le Goater]

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.

Reviewed-by: Cédric Le Goater <address@hidden>

Thanks,

C.

> Signed-off-by: Greg Kurz <address@hidden>
> ---
>  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;
>