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

[Christian Bruel <christian.bruel@foss.st.com>]

On 4/4/25 15:36, Marc Zyngier wrote:
> On Fri, 04 Apr 2025 13:15:05 +0100,
> Christian Bruel <christian.bruel@foss.st.com> wrote:
>>
>>
>>
>> On 4/3/25 19:50, Marc Zyngier wrote:
>>> On Thu, 03 Apr 2025 13:28:04 +0100,
>>> Christian Bruel <christian.bruel@foss.st.com> 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)

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 range at 128kB makes sense

similarly 4 kB + 120 Reserved for GICH and 8kB + 120 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.

sorry, misunderstanding, I was referring about my dropped quirk that I 
now dropped, not your options

thanks

Christian
> 
> 	M.
>