On Mon, Dec 17, 2018 at 11:41:34PM +0100, Cédric Le Goater wrote: > 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.
It's a pointer, so you can just declare the type without a
definition. That should be ok.
> > 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 <c...@kaod.org>
> >>> ---
> >>>
> >>> 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));
> >>> }
> >>>
> >>
> >
>
--
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
