Re: [Qemu-devel] [PATCH v8 09/12] spapr: set the interrupt presenter at reset

[Cédric Le Goater]

On 12/17/18 7:01 AM, David Gibson wrote:
> On Thu, Dec 13, 2018 at 01:52:14PM +0100, Cédric Le Goater wrote:
>> On 12/11/18 11:38 PM, Cédric Le Goater wrote:
>>> Currently, the interrupt presenter of the vCPU is set at realize
>>> time. Setting it at reset will become necessary when the new machine
>>> supporting both interrupt modes is introduced. In this machine, the
>>> interrupt mode is chosen at CAS time and activated after a reset.
>>
>> I think we need a second '->intc' pointer specific to XIVE instead. 
>> How about ->intc_xive ?
> 
> Ah, interesting.  If we're going to need that, we might as well go
> to specific ->icp and ->tctx pointers with the right types.

Hmm, PowerPCCPU is defined under target/ppc and adding the type might 
add some noise. I will check.

> I don't particularly like having those machine specific pointers
> within the cpu structure, but we can look at fixing that later by
> reviving my patches to move various machine specific stuff within the
> cpu into a separate sub-allocation.

ok.

Thanks,

C. 

>>
>> We could just drop this patch and easly get rid of the XiveTCTX object 
>> leak in spapr_unrealize_vcpu().
>>
>> C. 
>>
>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>> ---
>>>
>>>  Changes since v7:
>>>
>>>  - introduced spapr_irq_reset_xics(). 
>>>
>>>  include/hw/ppc/spapr_cpu_core.h |  2 ++
>>>  hw/ppc/spapr_cpu_core.c         | 26 ++++++++++++++++++++++++++
>>>  hw/ppc/spapr_irq.c              | 13 +++++++++++++
>>>  3 files changed, 41 insertions(+)
>>>
>>> diff --git a/include/hw/ppc/spapr_cpu_core.h 
>>> b/include/hw/ppc/spapr_cpu_core.h
>>> index 9e2821e4b31f..fc8ea9021656 100644
>>> --- a/include/hw/ppc/spapr_cpu_core.h
>>> +++ b/include/hw/ppc/spapr_cpu_core.h
>>> @@ -53,4 +53,6 @@ static inline sPAPRCPUState *spapr_cpu_state(PowerPCCPU 
>>> *cpu)
>>>      return (sPAPRCPUState *)cpu->machine_data;
>>>  }
>>>  
>>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type);
>>> +
>>>  #endif
>>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>>> index 82666436e9b4..afc5d4f4e739 100644
>>> --- a/hw/ppc/spapr_cpu_core.c
>>> +++ b/hw/ppc/spapr_cpu_core.c
>>> @@ -397,3 +397,29 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
>>>  };
>>>  
>>>  DEFINE_TYPES(spapr_cpu_core_type_infos)
>>> +
>>> +typedef struct ForeachFindIntCArgs {
>>> +    const char *intc_type;
>>> +    Object *intc;
>>> +} ForeachFindIntCArgs;
>>> +
>>> +static int spapr_cpu_core_find_intc(Object *child, void *opaque)
>>> +{
>>> +    ForeachFindIntCArgs *args = opaque;
>>> +
>>> +    if (object_dynamic_cast(child, args->intc_type)) {
>>> +        args->intc = child;
>>> +    }
>>> +
>>> +    return args->intc != NULL;
>>> +}
>>> +
>>> +void spapr_cpu_core_set_intc(PowerPCCPU *cpu, const char *intc_type)
>>> +{
>>> +    ForeachFindIntCArgs args = { intc_type, NULL };
>>> +
>>> +    object_child_foreach(OBJECT(cpu), spapr_cpu_core_find_intc, &args);
>>> +    g_assert(args.intc);
>>> +
>>> +    cpu->intc = args.intc;
>>> +}
>>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>>> index 0999a2b2d69c..814500f22d34 100644
>>> --- a/hw/ppc/spapr_irq.c
>>> +++ b/hw/ppc/spapr_irq.c
>>> @@ -12,6 +12,7 @@
>>>  #include "qemu/error-report.h"
>>>  #include "qapi/error.h"
>>>  #include "hw/ppc/spapr.h"
>>> +#include "hw/ppc/spapr_cpu_core.h"
>>>  #include "hw/ppc/spapr_xive.h"
>>>  #include "hw/ppc/xics.h"
>>>  #include "sysemu/kvm.h"
>>> @@ -208,6 +209,15 @@ static int spapr_irq_post_load_xics(sPAPRMachineState 
>>> *spapr, int version_id)
>>>      return 0;
>>>  }
>>>  
>>> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
>>> +{
>>> +   CPUState *cs;
>>> +
>>> +    CPU_FOREACH(cs) {
>>> +        spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
>>> +    }
>>> +}
>>> +
>>>  #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 +235,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,
>>>  };
>>>  
>>>  /*
>>> @@ -325,6 +336,8 @@ static void spapr_irq_reset_xive(sPAPRMachineState 
>>> *spapr, Error **errp)
>>>      CPU_FOREACH(cs) {
>>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>  
>>> +        spapr_cpu_core_set_intc(cpu, TYPE_XIVE_TCTX);
>>> +
>>>          /* (TCG) Set the OS CAM line of the thread interrupt context. */
>>>          spapr_xive_set_tctx_os_cam(XIVE_TCTX(cpu->intc));
>>>      }
>>>
>>
>