Re: [Qemu-devel] [PATCH for-2.10 2/8] ppc/xics: add an ics_eoi() handler to XICSFabric

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

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

On Wed, Mar 08, 2017 at 11:52:45AM +0100, Cédric Le Goater wrote:
> This handler will be required by PowerPC machines using multiple ICS
> objects, like this is the case for PowerNV. Also update the sPAPR
> machine to use the new handler.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

I don't see why you need a new callback for this.  In this and the
subsequent patches, it looks like the callback is always just use the
fabric's ics_get() to find the right ICS state, then call ics_eoi() on
it.


> ---
>  hw/intc/xics.c        |  9 +++------
>  hw/ppc/spapr.c        | 11 +++++++++++
>  include/hw/ppc/xics.h |  2 ++
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 209e1a75ecb9..e6fecd6e1a89 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -169,7 +169,7 @@ void ics_resend(ICSState *ics)
>      }
>  }
>  
> -static void ics_eoi(ICSState *ics, int nr)
> +void ics_eoi(ICSState *ics, int nr)
>  {
>      ICSStateClass *k = ICS_BASE_GET_CLASS(ics);
>  
> @@ -268,7 +268,6 @@ void icp_eoi(ICPState *icp, uint32_t xirr)
>  {
>      XICSFabric *xi = icp->xics;
>      XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(xi);
> -    ICSState *ics;
>      uint32_t irq;
>  
>      /* Send EOI -> ICS */
> @@ -276,10 +275,8 @@ void icp_eoi(ICPState *icp, uint32_t xirr)
>      trace_xics_icp_eoi(icp->cs->cpu_index, xirr, icp->xirr);
>      irq = xirr & XISR_MASK;
>  
> -    ics = xic->ics_get(xi, irq);
> -    if (ics) {
> -        ics_eoi(ics, irq);
> -    }
> +    xic->ics_eoi(xi, irq);
> +
>      if (!XISR(icp)) {
>          icp_resend(icp);
>      }
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c3bb99160545..043629cc5c54 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3024,6 +3024,16 @@ static void spapr_ics_resend(XICSFabric *dev)
>      ics_resend(spapr->ics);
>  }
>  
> +static void spapr_ics_eoi(XICSFabric *xi, int irq)
> +{
> +    ICSState *ics;
> +
> +    ics = spapr_ics_get(xi, irq);
> +    if (ics) {
> +        ics_eoi(ics, irq);
> +    }
> +}
> +
>  static ICPState *spapr_icp_get(XICSFabric *xi, int server)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(xi);
> @@ -3094,6 +3104,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      vhc->get_patbe = spapr_get_patbe;
>      xic->ics_get = spapr_ics_get;
>      xic->ics_resend = spapr_ics_resend;
> +    xic->ics_eoi = spapr_ics_eoi;
>      xic->icp_get = spapr_icp_get;
>      ispc->print_info = spapr_pic_print_info;
>  }
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 42bd24e975cb..00b003b2392d 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -155,6 +155,7 @@ typedef struct XICSFabricClass {
>      InterfaceClass parent;
>      ICSState *(*ics_get)(XICSFabric *xi, int irq);
>      void (*ics_resend)(XICSFabric *xi);
> +    void (*ics_eoi)(XICSFabric *xi, int irq);
>      ICPState *(*icp_get)(XICSFabric *xi, int server);
>  } XICSFabricClass;
>  
> @@ -189,6 +190,7 @@ void icp_pic_print_info(ICPState *icp, Monitor *mon);
>  void ics_pic_print_info(ICSState *ics, Monitor *mon);
>  
>  void ics_resend(ICSState *ics);
> +void ics_eoi(ICSState *ics, int irq);
>  void icp_resend(ICPState *ss);
>  
>  typedef struct sPAPRMachineState sPAPRMachineState;

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