On Mon, May 19, 2025 at 5:26 PM Peter Maydell <peter.mayd...@linaro.org>
wrote:
> 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.
>
>
>
Yes, I agree with Peter.
Another option is to have the GIC expose a per-cpu MR for the GICC
interfaces.
We would need to map these onto a per-cpu address-space. I think this is
the way
it works on real HW GICv2 with a per-cpu AMBA port. A problem with this
approach
is that it (in QEMU) doesn't scale very well to other GIC's with large
numbers of cores.
So the side-band signals with CPU index in memtxattrs sounds good to me!
Cheers,
Edgar
