Re: [Qemu-ppc] [PATCH v7 12/19] spapr: add a 'reset' method to the sPAPR IRQ backend

[Cédric Le Goater]

On 12/10/18 7:42 AM, David Gibson wrote:
> On Sun, Dec 09, 2018 at 08:46:03PM +0100, Cédric Le Goater wrote:
>> For the time being, the XIVE reset handler updates the OS CAM line of
>> the vCPU as it is done under a real hypervisor when a vCPU is
>> scheduled to run on a HW thread.
>>
>> This handler will become even more useful when we introduce the
>> machine supporting both interrupt modes, XIVE and XICS. In this
>> machine, the interrupt mode is chosen by the CAS negotiation process
>> and activated after a reset.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  include/hw/ppc/spapr_irq.h  |  2 ++
>>  include/hw/ppc/spapr_xive.h |  1 +
>>  hw/intc/spapr_xive.c        | 24 ++++++++++++++++++++++++
>>  hw/ppc/spapr.c              |  5 +++++
>>  hw/ppc/spapr_irq.c          | 24 ++++++++++++++++++++++++
>>  5 files changed, 56 insertions(+)
>>
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index 84a25ffb6c65..63061a009b4c 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -44,6 +44,7 @@ typedef struct sPAPRIrq {
>>      Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
>>                                 Error **errp);
>>      int (*post_load)(sPAPRMachineState *spapr, int version_id);
>> +    void (*reset)(sPAPRMachineState *spapr, Error **errp);
>>  } sPAPRIrq;
>>  
>>  extern sPAPRIrq spapr_irq_xics;
>> @@ -55,6 +56,7 @@ int spapr_irq_claim(sPAPRMachineState *spapr, int irq, 
>> bool lsi, Error **errp);
>>  void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
>>  qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
>>  int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
>> +void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
>>  
>>  /*
>>   * XICS legacy routines
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index 728a5e8dc163..7244a6231ce6 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -47,5 +47,6 @@ typedef struct sPAPRMachineState sPAPRMachineState;
>>  void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>>                     uint32_t phandle);
>> +void spapr_xive_reset_tctx(sPAPRXive *xive);
>>  
>>  #endif /* PPC_SPAPR_XIVE_H */
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index a6d854b07690..560d8d031f74 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -179,6 +179,30 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
>>      sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
>>  }
>>  
>> +/*
>> + * When a Virtual Processor is scheduled to run on a HW thread, the
>> + * hypervisor pushes its identifier in the OS CAM line. Emulate the
>> + * same behavior under QEMU.
>> + */
>> +void spapr_xive_reset_tctx(sPAPRXive *xive)
>> +{
>> +    CPUState *cs;
>> +    uint8_t  nvt_blk;
>> +    uint32_t nvt_idx;
>> +    uint32_t nvt_cam;
>> +
>> +    CPU_FOREACH(cs) {
>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +        XiveTCTX *tctx = XIVE_TCTX(cpu->intc);
>> +
>> +        spapr_xive_cpu_to_nvt(cpu, &nvt_blk, &nvt_idx);
>> +
>> +        nvt_cam = cpu_to_be32(TM_QW1W2_VO |
>> +                              xive_nvt_cam_line(nvt_blk, nvt_idx));
>> +        memcpy(&tctx->regs[TM_QW1_OS + TM_WORD2], &nvt_cam, 4);
>> +    }
>> +}
>> +

I will need to rework that patch because the OS CAM line needs to be 
reseted when a CPU is hot plugged also.

C. 

>>  static void spapr_xive_end_reset(XiveEND *end)
>>  {
>>      memset(end, 0, sizeof(*end));
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 8cea4cad1732..98d69f09e080 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1619,6 +1619,11 @@ static void spapr_machine_reset(void)
>>  
>>      qemu_devices_reset();
>>  
>> +    /* This is fixing some of the default configuration of the XIVE
>> +     * devices. To be called after the reset of the machine devices.
>> +     */
>> +    spapr_irq_reset(spapr, &error_fatal);
>> +
>>      /* DRC reset may cause a device to be unplugged. This will cause 
>> troubles
>>       * if this device is used by another device (eg, a running vhost backend
>>       * will crash QEMU if the DIMM holding the vring goes away). To avoid 
>> such
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 35a067cad3f8..04f5c9665550 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -209,6 +209,10 @@ static int spapr_irq_post_load_xics(sPAPRMachineState 
>> *spapr, int version_id)
>>      return 0;
>>  }
>>  
>> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>> +{
>> +}
> 
> You already have a check for a NULL reset hook in spapr_irq_reset() so
> you could omit this empty function.
> 
>> +
>>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
>> @@ -225,6 +229,7 @@ sPAPRIrq spapr_irq_xics = {
>>      .dt_populate = spapr_dt_xics,
>>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>>      .post_load   = spapr_irq_post_load_xics,
>> +    .reset       = spapr_irq_reset_xics,
>>  };
>>  
>>  /*
>> @@ -333,6 +338,15 @@ static int spapr_irq_post_load_xive(sPAPRMachineState 
>> *spapr, int version_id)
>>      return 0;
>>  }
>>  
>> +static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
>> +{
>> +    /*
>> +     * Set the OS CAM line of the cpu interrupt thread context. Needs
>> +     * to come after the XiveTCTX reset handlers.
>> +     */
>> +    spapr_xive_reset_tctx(spapr->xive);
>> +}
>> +
>>  /*
>>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>>   * with XICS.
>> @@ -353,6 +367,7 @@ sPAPRIrq spapr_irq_xive = {
>>      .dt_populate = spapr_dt_xive,
>>      .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
>>      .post_load   = spapr_irq_post_load_xive,
>> +    .reset       = spapr_irq_reset_xive,
>>  };
>>  
>>  /*
>> @@ -398,6 +413,15 @@ int spapr_irq_post_load(sPAPRMachineState *spapr, int 
>> version_id)
>>      return smc->irq->post_load(spapr, version_id);
>>  }
>>  
>> +void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp)
>> +{
>> +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> +
>> +    if (smc->irq->reset) {
>> +        smc->irq->reset(spapr, errp);
>> +    }
>> +}
>> +
>>  /*
>>   * XICS legacy routines - to deprecate one day
>>   */
>