On Tue, 13 May 2025 at 16:39, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> On 13/5/25 16:14, Clément Chigot wrote:
> > From: Frederic Konrad <konrad.frede...@yahoo.fr>
> >
> > This introduces a first-cpu-index property to the arm-gic, as some SOCs
> > could have two separate GIC (ie: the zynqmp).
> >
> > Signed-off-by: Clément Chigot <chi...@adacore.com>
> > ---
> >   hw/intc/arm_gic.c                | 2 +-
> >   hw/intc/arm_gic_common.c         | 1 +
> >   include/hw/intc/arm_gic_common.h | 2 ++
> >   3 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> > index d18bef40fc..899f133363 100644
> > --- a/hw/intc/arm_gic.c
> > +++ b/hw/intc/arm_gic.c
> > @@ -59,7 +59,7 @@ static const uint8_t gic_id_gicv2[] = {
> >   static inline int gic_get_current_cpu(GICState *s)
> >   {
> >       if (!qtest_enabled() && s->num_cpu > 1) {
> > -        return current_cpu->cpu_index;
> > +        return current_cpu->cpu_index - s->first_cpu_index;
>
> Note, CPUState::cpu_index is meant for accelerators code and shouldn't
> be used in hw/ (in particular because it vary when using hotplug).

Mmm. I think my ideal solution for that would be that we
put the CPU index into the MemTxAttrs (requester_id, perhaps
with some extra flag that it's not a PCI requester ID).
Then every device that's looking at current_cpu to figure
out which CPU actually called it would be able to stop
doing that. We'd still have the "OK, so what number does
the GIC think this CPU is?" question, though.

For telling the GIC which CPUs it has connected to it, my
instinct is to say that ideally we'd have the GIC have
something like an array of link properties, and the board
or SoC code passes the GIC a pointer to each CPU in the
order it wants them to be.

But that would be quite a bit of fiddling around to achieve,
so I think I'm OK with the approach in this patch, especially
since the GICv2 is pretty well obsolete at this point.

(GICv3 also assumes "starting from zero", in several places
where it loops from 0 to s->num_cpu calling qemu_get_cpu().
Link properties is probably what I'm going to end up doing with
the GICv5 design.)

One note: if you add a new property to the GIC, please
also add documentation of it to the "QEMU interface" comment
in include/hw/intc/arm_gic.h.

thanks
-- PMM

Reply via email to