Re: [PATCH 2/3] irqchip/gic: Use 0x10000 offset to access GICC_DIR on STM32MP2

[Christian Bruel]

On 4/4/25 15:36, Marc Zyngier wrote:
> On Fri, 04 Apr 2025 13:15:05 +0100,
> Christian Bruel <christian.bruel@xxxxxxxxxxx> wrote:
> > On 4/3/25 19:50, Marc Zyngier wrote:
> > > On Thu, 03 Apr 2025 13:28:04 +0100,
> > > Christian Bruel <christian.bruel@xxxxxxxxxxx> wrote:
> > > > When GIC_4KNOT64K bit in the GIC configuration register is
> > > > 0 (64KB), address block is modified in such a way than only the
> > > > first 4KB of the GIC cpu interface are accessible with default
> > > > offsets.
> > > > With this bit mapping GICC_DIR register is accessible at
> > > > offset 0x10000 instead of 0x1000, thus remap accordingly
> > >
> > > And I'm pretty sure the whole of the GICC range is correctly
> > > accessible at offset 0xF000, giving you the full 8kB you need. That's
> > > because each page of the GIC is aliased over two 64kB blocks, as per
> > > the integration guidelines so that MMU isolation can be provided on a
> > > 64kB boundary.
> >
> > Thanks a lot for this explanation, indeed this works like a charm.
> >
> > > Funnily enough, all it takes is to adjust GICC region. You can either:
> > >
> > > - make it 128kB wide, and the driver will take care of it (details in
> > >     gic_check_eoimode()). On one of my boxes that is similarly
> > >     configured, I get:
> > >
> > >     [    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> > >     [    0.000000] GIC: Adjusting CPU interface base to 0x00000000780af000
> > >     [    0.000000] Root IRQ handler: gic_handle_irq
> > >     [    0.000000] GIC: Using split EOI/Deactivate mode
> > >
> > >     See below for what I expect to be the correct fix.
> > >     - make it 8kB wide from offset 0xF000.
> >
> > I checked both and they work. I will go for the former to show real
> > 8kB size to be exposed in the DT. And there are a few other
> > platforms that use this alias
>
> I think 8kB the wrong option. The GIC *is* supposed to be integrated
> over 128kB on arm64 platforms (there was some documentation about that
> back in the days, but it has become impossible to search anything on
> ARM's stupidly broken website.  My recollection is that it was bundled
> with the GICv2m "specification" (only half a page!). >
> Furthermore, you are supposed to describe the HW. Not your
> interpretation of it. Correctly written SW targeting arm64 know about
> this anyway.

greping other platforms there are a bunch 0xf000 offset 8KB mapped:

 amd/amd-seattle-soc.dtsi
 arm/corstone1000.dtsi
 arm/foundation-v8-gicv3.dtsi
 arm/juno-base.dtsi
 mediatek/mt8516.dtsi

but, looking at the stm32mp25 memory map (1) page 239:

0x4AC22000 - 0x4AC3FFFF 120 Reserved -
0x4AC20000 - 0x4AC21FFF 8   GICC

I can know guess that the "Reserved" 120kB is for aliasing the 64kB 
blocks. Thus describing the GICC 128KB range size makes sense

similarly 4KB + 120KB Reserved for GICH and 8KB + 120KB Reserved for GICV

(1) 
https://www.st.com/resource/en/reference_manual/rm0457-stm32mp25xx-advanced-armbased-3264bit-mpus-stmicroelectronics.pdf


> > > Unless the ST HW folks have been even more creative, none of this
> > > overly complicated stuff should be necessary. Just describe the HW
> > > correctly.
> >
> > I was unable to find this information in the GIC-400 trm
> > (https://developer.arm.com/documentation/ddi0471/b/programmers-model/gic-400-register-map). Now
> > I also prefer to use GICC alias at
> > offset 0xf000 as suggested rather than the quirk solution
>
> Again, this isn't a quirk. It's the one true way for 64bit platforms
> that can use pages bigger than 4kB. That's the purpose of the 4Kn64K
> parameter in the integration, dropping bits [15:12] from the PA
> presented to the CPU interface.

there might be a misunderstanding, I was referring to my dropped quirk 
that I now dropped, not your options

thanks

Christian
> 	M.