Re: [PATCH v3 1/3] firmware: xilinx: Add support to clear TF-A PM state

[Jay Buddhabhatti]

Hi Sudeep,

Thanks for the review. Replies inline.

On 7/24/2026 4:43 PM, Sudeep Holla wrote:
> On Fri, Jul 24, 2026 at 03:19:46AM -0700, Jay Buddhabhatti wrote:
> > Currently, during a kexec restart, only the kernel is reloaded, while the
> > TF-A specific data remain unchanged. This leads to a mismatch between the
> > kernel state and the TF-A firmware state like SGI number and shutdown
> > scope variable.
>
> Avoid using TF-A in general as it can be any other firmware as well.
> You can always generalise as PSCI or EL3 firmware.

Agreed. Updated the commit message and in-patch comments to refer to
"EL3 firmware" / "secure firmware" rather than TF-A.

> > For example, the kernel registers an SGI number with TF-A so that TF-A can
> > notify the kernel of events via that SGI. TF-A stores this SGI number in
> > its internal state. After a kexec, the newly loaded kernel re-registers
> > and may request a different SGI number, but the stale value programmed in
> > TF-A remains, so event notifications are delivered on the old SGI and are
> > missed by the new kernel. The shutdown scope variable has a similar stale
> > state problem.
>
> Doesn't it reset it every time CPU_OFF or CPU_ON is called ?

No. The SGI registration is done once via zynqmp_pm_register_sgi() when
the event manager driver probes; it programs the SGI number into EL3 PM
state. On CPU hotplug, the kernel only enables/disables the per-CPU IRQ
(enable_percpu_irq() / disable_percpu_irq() in xlnx_event_manager.c);
it does not re-register the SGI with EL3 on CPU_ON or clear that
registration on CPU_OFF.

So while SGI delivery is per-CPU from the GIC side, the SGI number EL3
uses to raise the interrupt is subsystem-global state held in EL3, not
per-CPU state that is reset by CPU hotplug.

> > To resolve this, the TF_A_CLEAR_PM_STATE PM API is introduced to clear
> > TF-A-specific state during kexec. On a graceful reboot, this API is
> > triggered by zynqmp_firmware_shutdown(), while in a crash kernel scenario,
> > it is invoked by zynqmp_firmware_probe() in the reloaded kernel.
>
> I don't see the need for such an API just for kexec needs. You must deal
> with it as part of CPU_ON as it should start with clean slate as SGIs are
> per CPU, no ?

Handling this purely in the CPU_ON path would require either EL3 to
clear/re-init this PM state on every CPU bring-up, or the kernel to
re-register the SGI on every CPU_ON. Neither happens today. CPU_ON is
not the point at which this state is established or torn down; driver
probe/shutdown is. That is why the platform firmware exposes
TF_A_CLEAR_PM_STATE - to reset EL3 PM subsystem state when the OS is
replaced without a full firmware reload.

The kexec cleanup is therefore a targeted reset at the OS transition
boundary (zynqmp_firmware_shutdown() on graceful kexec, 
zynqmp_firmware_probe() on kdump), not a general CPU hotplug hook. 
Given the above, I believe adding a dedicated API and invoking it at the 
kexec boundary is the correct approach.

Thanks,
Jay

>