Re: [RFC PATCH 0/1] xen/arm: smccc: preserve arguments before register setup

On 29/08/2026 3:12 am, Jan Setje-Eilers wrote:
> Hi Andrew,
>
> While investigating an SMC forwarding failure after updating to Xen 4.22,
> I found that firmware could receive different values from those passed to
> arm_smccc_1_1_smc().
>
> Several public Xen callers pass get_user_reg() calls directly to this
> helper.  This includes the SCMI, ZynqMP EEMI, and i.MX forwarding paths.
> The helper puts each value into its SMC argument register as the
> expression is evaluated.  A later get_user_reg() call can then overwrite a
> register prepared for an earlier argument.
>
> The change in generated code appears after:
>
> 67bcf5eae709 ("xen/arm: Simplify type handling for SMCCC declarations")
>
> I am not sure which behavior the helper is intended to provide. Should
> arm_smccc_1_1_smc() continue to accept arguments such as get_user_reg()
> calls, or should callers save those values in local variables first?
>
> The attached patch is a candidate for the first option. It restores
> temporary variables inside the helper, allowing all calls to finish before
> x0-x7 are prepared for the SMC instruction. I am not able to validate the
> public SCMI and ZynqMP paths on hardware, but their generated code now
> finishes every get_user_reg() call before the final register setup and
> smc. With this change, SMCs also work as intended on the device I'm
> using.
>
> If the intent is not to accept function calls as arguments, we should
> probably fix the SCMI, ZynqMP EEMI, and i.MX callers.
>
> Given the existing use cases, this seems like a bug, but your patch looks
> intentional. It's very possible I'm missing the intended design. I would
> appreciate your view on the intended contract.
>
> Thanks for any advice or help with this.

Yes you're right.  My change was buggy.   It was actually part of a
mutli-stage cleanup across several patches.

I'll submit my own patch.  I'm afraid your AI has put in whitespace
errors where a straight revert would have gotten it correct, and left
the 0 case still buggy.

~Andrew