Re: [Qemu-ppc] [PATCH v3 02/19] spapr: Rename xics to intc in interrupt controller agnostic code

[Greg Kurz]

On Fri, 18 Jan 2019 14:56:51 +0100
Cédric Le Goater <address@hidden> wrote:

> On 1/17/19 6:14 PM, Greg Kurz wrote:
> > All this code is used with both the XICS and XIVE interrupt controllers.
> > 
> > Signed-off-by: Greg Kurz <address@hidden>
> > Reviewed-by: Cédric Le Goater <address@hidden>
> > ---
> > v3: - s/spapr_dt_intc_irq/spapr_dt_irq
> > ---
> >  hw/ppc/spapr.c              |    6 +++---
> >  hw/ppc/spapr_events.c       |    2 +-
> >  hw/ppc/spapr_pci.c          |    6 +++---
> >  hw/ppc/spapr_vio.c          |    2 +-
> >  include/hw/pci-host/spapr.h |    2 +-
> >  include/hw/ppc/spapr.h      |    2 +-
> >  6 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 83081defde4e..26f8e55cc25e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -96,7 +96,7 @@
> >  
> >  #define MIN_RMA_SLOF            128UL
> >  
> > -#define PHANDLE_XICP            0x00001111
> > +#define PHANDLE_INTC            0x00001111  
> 
> Shouln't that define be under a spapr->irq field ? 
> 

You're probably right but that's not in the scope of this patch
which only does a global and trivial renaming. I'll have a closer
look at your suggestion anyway since there will be a v4 :)

> >  
> >  /* These two functions implement the VCPU id numbering: one to compute them
> >   * all and one to identify thread 0 of a VCORE. Any change to the first one
> > @@ -1276,7 +1276,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >  
> >      /* /interrupt controller */
> >      spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
> > -                          PHANDLE_XICP);
> > +                          PHANDLE_INTC);
> >  
> >      ret = spapr_populate_memory(spapr, fdt);
> >      if (ret < 0) {
> > @@ -1296,7 +1296,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
> >      }
> >  
> >      QLIST_FOREACH(phb, &spapr->phbs, list) {
> > -        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
> > +        ret = spapr_populate_pci_dt(phb, PHANDLE_INTC, fdt,
> >                                      spapr->irq->nr_msis);  
> 
> and there we could pass spapr->irq.
> 

Yeah.

> 
> >          if (ret < 0) {
> >              error_report("couldn't setup PCI devices in fdt");
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index 32719a1b72d0..b9c7ecb9e987 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -282,7 +282,7 @@ void spapr_dt_events(sPAPRMachineState *spapr, void 
> > *fdt)
> >              continue;
> >          }
> >  
> > -        spapr_dt_xics_irq(interrupts, source->irq, false);
> > +        spapr_dt_irq(interrupts, source->irq, false);
> >  
> >          _FDT(node_offset = fdt_add_subnode(fdt, event_sources, 
> > source_name));
> >          _FDT(fdt_setprop(fdt, node_offset, "interrupts", interrupts,
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index b74f2632ecc6..ccdaf2c9a606 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -2066,7 +2066,7 @@ static void spapr_phb_pci_enumerate(sPAPRPHBState 
> > *phb)
> >  
> >  }
> >  
> > -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void 
> > *fdt,
> > +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void 
> > *fdt,
> >                            uint32_t nr_msis)
> >  {
> >      int bus_off, i, j, ret;
> > @@ -2164,8 +2164,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb, 
> > uint32_t xics_phandle, void *fdt,
> >              irqmap[1] = 0;
> >              irqmap[2] = 0;
> >              irqmap[3] = cpu_to_be32(j+1);
> > -            irqmap[4] = cpu_to_be32(xics_phandle);
> > -            spapr_dt_xics_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, 
> > true);
> > +            irqmap[4] = cpu_to_be32(intc_phandle);
> > +            spapr_dt_irq(&irqmap[5], phb->lsi_table[lsi_num].irq, true);
> >          }
> >      }
> >      /* Write interrupt map */
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index 7e8a9ad09337..f80b70a39c46 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -158,7 +158,7 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
> >      if (dev->irq) {
> >          uint32_t ints_prop[2];
> >  
> > -        spapr_dt_xics_irq(ints_prop, dev->irq, false);
> > +        spapr_dt_irq(ints_prop, dev->irq, false);
> >          ret = fdt_setprop(fdt, node_off, "interrupts", ints_prop,
> >                            sizeof(ints_prop));
> >          if (ret < 0) {
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index 4eb3a2ce3eb8..e0e683c32469 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -113,7 +113,7 @@ static inline qemu_irq spapr_phb_lsi_qirq(struct 
> > sPAPRPHBState *phb, int pin)
> >      return spapr_qirq(spapr, phb->lsi_table[pin].irq);
> >  }
> >  
> > -int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t xics_phandle, void 
> > *fdt,
> > +int spapr_populate_pci_dt(sPAPRPHBState *phb, uint32_t intc_phandle, void 
> > *fdt,
> >                            uint32_t nr_msis);
> >  
> >  void spapr_pci_rtas_init(void);
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index 9e01a5a12e4a..f988dc6924b8 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -682,7 +682,7 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void 
> > *fdt, hwaddr addr);
> >   * "interrupt-controller" node has its "#interrupt-cells" property set to 
> > 2 (ie,
> >   * VIO devices, RTAS event sources and PHBs).
> >   */
> > -static inline void spapr_dt_xics_irq(uint32_t *intspec, int irq, bool 
> > is_lsi)
> > +static inline void spapr_dt_irq(uint32_t *intspec, int irq, bool is_lsi)
> >  {
> >      intspec[0] = cpu_to_be32(irq);
> >      intspec[1] = is_lsi ? cpu_to_be32(1) : 0;
> >   
>