Re: [PATCH v4 07/25] ppc/spapr: Implement the XiveFabric interface

[David Gibson]

On Wed, Sep 18, 2019 at 06:06:27PM +0200, Cédric Le Goater wrote:
> The CAM line matching sequence in the pseries machine does not change
> much apart from the use of the new QOM interfaces. There is an extra
> indirection because of the sPAPR IRQ backend of the machine. Only the
> XIVE backend implements the new 'match_nvt' handler.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  include/hw/ppc/spapr_irq.h |  6 ++++++
>  hw/ppc/spapr.c             | 34 ++++++++++++++++++++++++++++++++++
>  hw/ppc/spapr_irq.c         | 25 +++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 5db305165ce2..859780efaf95 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -31,6 +31,8 @@ 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);
>  
> +struct XiveTCTXMatch;
> +
>  typedef struct SpaprIrq {
>      uint32_t    nr_irqs;
>      uint32_t    nr_msis;
> @@ -50,6 +52,10 @@ typedef struct SpaprIrq {
>      void (*set_irq)(void *opaque, int srcno, int val);
>      const char *(*get_nodename)(SpaprMachineState *spapr);
>      void (*init_kvm)(SpaprMachineState *spapr, Error **errp);
> +    int (*match_nvt)(SpaprMachineState *spapr, uint8_t format,
> +                     uint8_t nvt_blk, uint32_t nvt_idx,
> +                     bool cam_ignore, uint8_t priority,
> +                     uint32_t logic_serv, struct XiveTCTXMatch *match);

Obviously this will need some rework against my stuff.

But more importantly, I don't see the point of indirecting via here,
when the method is only relevant for the xive case.  Why not just
assert that XIVE is in use in the XiveFabric hook, and go directly  to
the XIVE matching code.

>  } SpaprIrq;
>  
>  extern SpaprIrq spapr_irq_xics;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2725b139a7f0..90f6f5fb9536 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4360,6 +4360,37 @@ static void 
> spapr_pic_print_info(InterruptStatsProvider *obj,
>                     kvm_irqchip_in_kernel() ? "in-kernel" : "emulated");
>  }
>  
> +static int spapr_xive_match_nvt(XiveFabric *xfb, uint8_t format,
> +                                uint8_t nvt_blk, uint32_t nvt_idx,
> +                                bool cam_ignore, uint8_t priority,
> +                                uint32_t logic_serv, XiveTCTXMatch *match)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(xfb);
> +    int count;
> +
> +    count = spapr->irq->match_nvt(spapr, format, nvt_blk, nvt_idx, 
> cam_ignore,
> +                                  priority, logic_serv, match);
> +    if (count < 0) {
> +        return count;
> +    }
> +
> +    /*
> +     * When we implement the save and restore of the thread interrupt
> +     * contexts in the enter/exit CPU handlers of the machine and the
> +     * escalations in QEMU, we should be able to handle non dispatched
> +     * vCPUs.
> +     *
> +     * Until this is done, the sPAPR machine should find at least one
> +     * matching context always.
> +     */
> +    if (count == 0) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "XIVE: NVT %x/%x is not dispatched\n",
> +                      nvt_blk, nvt_idx);
> +    }
> +
> +    return count;
> +}
> +
>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
>  {
>      return cpu->vcpu_id;
> @@ -4456,6 +4487,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>      PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_CLASS(oc);
>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
> +    XiveFabricClass *xfc = XIVE_FABRIC_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant)";
>      mc->ignore_boot_device_suffixes = true;
> @@ -4514,6 +4546,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>       */
>      mc->numa_mem_align_shift = 28;
>      mc->numa_mem_supported = true;
> +    xfc->match_nvt = spapr_xive_match_nvt;
>  
>      smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
>      smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
> @@ -4547,6 +4580,7 @@ static const TypeInfo spapr_machine_info = {
>          { TYPE_PPC_VIRTUAL_HYPERVISOR },
>          { TYPE_XICS_FABRIC },
>          { TYPE_INTERRUPT_STATS_PROVIDER },
> +        { TYPE_XIVE_FABRIC },
>          { }
>      },
>  };
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d8f46b6797f8..8a6d79a59af2 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -257,6 +257,7 @@ SpaprIrq spapr_irq_xics = {
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_nodename = spapr_irq_get_nodename_xics,
>      .init_kvm    = spapr_irq_init_kvm_xics,
> +    .match_nvt   = NULL, /* should not be used */
>  };
>  
>  /*
> @@ -406,6 +407,18 @@ static void spapr_irq_init_kvm_xive(SpaprMachineState 
> *spapr, Error **errp)
>      }
>  }
>  
> +static int spapr_irq_match_nvt_xive(SpaprMachineState *spapr, uint8_t format,
> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
> +                                    bool cam_ignore, uint8_t priority,
> +                                    uint32_t logic_serv, XiveTCTXMatch 
> *match)
> +{
> +    XivePresenter *xptr = XIVE_PRESENTER(spapr->xive);
> +    XivePresenterClass *xpc = XIVE_PRESENTER_GET_CLASS(xptr);
> +
> +    return xpc->match_nvt(xptr, format, nvt_blk, nvt_idx, cam_ignore,
> +                          priority, logic_serv, match);
> +}
> +
>  /*
>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>   * with XICS.
> @@ -431,6 +444,7 @@ SpaprIrq spapr_irq_xive = {
>      .set_irq     = spapr_irq_set_irq_xive,
>      .get_nodename = spapr_irq_get_nodename_xive,
>      .init_kvm    = spapr_irq_init_kvm_xive,
> +    .match_nvt   = spapr_irq_match_nvt_xive,
>  };
>  
>  /*
> @@ -585,6 +599,15 @@ static const char 
> *spapr_irq_get_nodename_dual(SpaprMachineState *spapr)
>      return spapr_irq_current(spapr)->get_nodename(spapr);
>  }
>  
> +static int spapr_irq_match_nvt_dual(SpaprMachineState *spapr, uint8_t format,
> +                                    uint8_t nvt_blk, uint32_t nvt_idx,
> +                                    bool cam_ignore, uint8_t priority,
> +                                    uint32_t logic_serv, XiveTCTXMatch 
> *match)
> +{
> +    return spapr_irq_current(spapr)->match_nvt(spapr, format, nvt_blk, 
> nvt_idx,
> +                                     cam_ignore, priority, logic_serv, 
> match);
> +}
> +
>  /*
>   * Define values in sync with the XIVE and XICS backend
>   */
> @@ -608,6 +631,7 @@ SpaprIrq spapr_irq_dual = {
>      .set_irq     = spapr_irq_set_irq_dual,
>      .get_nodename = spapr_irq_get_nodename_dual,
>      .init_kvm    = NULL, /* should not be used */
> +    .match_nvt   = spapr_irq_match_nvt_dual,
>  };
>  
>  
> @@ -825,4 +849,5 @@ SpaprIrq spapr_irq_xics_legacy = {
>      .set_irq     = spapr_irq_set_irq_xics,
>      .get_nodename = spapr_irq_get_nodename_xics,
>      .init_kvm    = spapr_irq_init_kvm_xics,
> +    .match_nvt   = NULL, /* should not be used */
>  };

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

signature.asc
Description: PGP signature