Re: [Qemu-devel] [PATCH v1 02/11] ppc/xics: Move SPAPR specific code to a separate file

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4496 bytes --]

On Fri, Jun 24, 2016 at 11:27:58AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Thu, Jun 23, 2016 at 11:17:21PM +0530, Nikunj A Dadhania wrote:
> >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> 
> >> Leave the core ICP/ICS logic in xics.c and move the top level
> >> class wrapper, hypercall and RTAS handlers to xics_spapr.c
> >> 
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> [add cpu.h in xics_spapr.c, move set_nr_irqs and set_nr_servers to
> >>  xics_spapr.c]
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> ---
> >>  default-configs/ppc64-softmmu.mak |   1 +
> >>  hw/intc/Makefile.objs             |   1 +
> >>  hw/intc/xics.c                    | 418 +-----------------------------------
> >>  hw/intc/xics_spapr.c              | 432 ++++++++++++++++++++++++++++++++++++++
> >>  include/hw/ppc/xics.h             |  21 ++
> >>  5 files changed, 464 insertions(+), 409 deletions(-)
> >>  create mode 100644 hw/intc/xics_spapr.c
> >> 
> >> diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
> >> index bb71b23..c4be59f 100644
> >> --- a/default-configs/ppc64-softmmu.mak
> >> +++ b/default-configs/ppc64-softmmu.mak
> >> @@ -49,6 +49,7 @@ CONFIG_ETSEC=y
> >>  CONFIG_LIBDECNUMBER=y
> >>  # For pSeries
> >>  CONFIG_XICS=$(CONFIG_PSERIES)
> >> +CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
> >>  CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
> >>  # For PReP
> >>  CONFIG_MC146818RTC=y
> >> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> >> index c7bbf88..530df2e 100644
> >> --- a/hw/intc/Makefile.objs
> >> +++ b/hw/intc/Makefile.objs
> >> @@ -30,6 +30,7 @@ obj-$(CONFIG_OPENPIC_KVM) += openpic_kvm.o
> >>  obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
> >>  obj-$(CONFIG_SH4) += sh_intc.o
> >>  obj-$(CONFIG_XICS) += xics.o
> >> +obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
> >>  obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> >>  obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
> >>  obj-$(CONFIG_S390_FLIC) += s390_flic.o
> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> >> index a715532..6ca391f 100644
> >> --- a/hw/intc/xics.c
> >> +++ b/hw/intc/xics.c
> >> @@ -32,12 +32,11 @@
> >>  #include "hw/hw.h"
> >>  #include "trace.h"
> >>  #include "qemu/timer.h"
> >> -#include "hw/ppc/spapr.h"
> >>  #include "hw/ppc/xics.h"
> >>  #include "qemu/error-report.h"
> >>  #include "qapi/visitor.h"
> >>  
> >> -static int get_cpu_index_by_dt_id(int cpu_dt_id)
> >> +int get_cpu_index_by_dt_id(int cpu_dt_id)
> >
> > If this is made public it needs  xics_*() name the current one is too
> > generic for a global symbol.
> 
> Sure. Should we also make  icp_set_*  as xics_icp_set_* then ?

I'm happy enough to treat icp_() as sufficient namespacing on its own.

> 
> >
> >>  {
> >>      PowerPCCPU *cpu = ppc_get_vcpu_by_dt_id(cpu_dt_id);
> >>  
> >> @@ -242,7 +241,7 @@ static void icp_resend(XICSState *icp, int server)
> >>      ics_resend(icp->ics);
> >>  }
> >>  
> >> -static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
> >> +void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
> >>  {
> >>      ICPState *ss = icp->ss + server;
> >>      uint8_t old_cppr;
> >> @@ -266,7 +265,7 @@ static void icp_set_cppr(XICSState *icp, int server, uint8_t cppr)
> >>      }
> >>  }
> >>  
> >> -static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
> >> +void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
> >>  {
> >>      ICPState *ss = icp->ss + server;
> >>  
> >> @@ -276,7 +275,7 @@ static void icp_set_mfrr(XICSState *icp, int server, uint8_t mfrr)
> >>      }
> >>  }
> >>  
> >> -static uint32_t icp_accept(ICPState *ss)
> >> +uint32_t icp_accept(ICPState *ss)
> >>  {
> >>      uint32_t xirr = ss->xirr;
> >>  
> >> @@ -289,7 +288,7 @@ static uint32_t icp_accept(ICPState *ss)
> >>      return xirr;
> >>  }
> >>  
> >> -static void icp_eoi(XICSState *icp, int server, uint32_t xirr)
> >> +void icp_eoi(XICSState *icp, int server, uint32_t xirr)
> >>  {
> >>      ICPState *ss = icp->ss + server;
> >>  
> 
> Regards
> Nikunj
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]